fix(routing): add agy to executor map so it uses AntigravityExecutor - #2957
Conversation
The agy provider was registered in providerRegistry.ts with
executor: "antigravity" but the executor map in executors/index.ts
only had an "antigravity" entry. getExecutor("agy") fell through to
DefaultExecutor, which returned undefined for baseUrl (agy only has
baseUrls), causing fetch(undefined) → TypeError: Cannot read properties
of undefined (reading 'toString').
Closes diegosouzapw#2932
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the agy alias for the AntigravityExecutor and introduces a new unit test file tests/unit/executor-agy.test.ts to verify its functionality. The review feedback correctly advises rewriting the new tests to use Vitest's native test and expect APIs instead of Node's native node:test and node:assert modules. This change is necessary to maintain consistency with the repository's test runner and ensure accurate test coverage tracking to meet the required 75% coverage threshold.
| import test from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
|
|
||
| import { getExecutor, AntigravityExecutor } from "../../open-sse/executors/index.ts"; | ||
|
|
||
| test("getExecutor('agy') returns AntigravityExecutor (not DefaultExecutor)", () => { | ||
| const executor = getExecutor("agy"); | ||
| assert.ok(executor instanceof AntigravityExecutor, "agy provider should use AntigravityExecutor"); | ||
| }); | ||
|
|
||
| test("getExecutor('antigravity') returns AntigravityExecutor", () => { | ||
| const executor = getExecutor("antigravity"); | ||
| assert.ok(executor instanceof AntigravityExecutor, "antigravity provider should use AntigravityExecutor"); | ||
| }); | ||
|
|
||
| test("getExecutor('agy') builds valid streaming URL", () => { | ||
| const executor = getExecutor("agy"); | ||
| const url = executor.buildUrl("gemini-3-flash", true); | ||
| assert.ok( | ||
| url.includes("streamGenerateContent?alt=sse"), | ||
| `expected streaming endpoint URL, got: ${url}` | ||
| ); | ||
| }); | ||
|
|
||
| test("getExecutor('agy') builds valid non-streaming URL", () => { | ||
| const executor = getExecutor("agy"); | ||
| const url = executor.buildUrl("gemini-3-flash", false); | ||
| // Antigravity executor always uses streaming endpoint (buildUrl ignores stream flag) | ||
| assert.ok( | ||
| url.includes("streamGenerateContent?alt=sse"), | ||
| `expected streaming endpoint URL (always), got: ${url}` | ||
| ); | ||
| }); | ||
|
|
||
| test("getExecutor('agy') buildHeaders returns Bearer auth", () => { | ||
| const executor = getExecutor("agy"); | ||
| const headers = executor.buildHeaders({ accessToken: "test-token" }); | ||
| assert.equal(headers.Authorization, "Bearer test-token"); | ||
| }); |
There was a problem hiding this comment.
The repository uses Vitest as its test runner (as indicated by vitest.config.ts in the root). Using Node's native node:test and node:assert modules can lead to inconsistency, potential issues with Vitest's test runner/matchers, and incorrect test coverage reporting (which is critical to maintain the ≥75% coverage requirement). Please rewrite the test to use Vitest's native test and expect APIs.
import { test, expect } from "vitest";
import { getExecutor, AntigravityExecutor } from "../../open-sse/executors/index.ts";
test("getExecutor('agy') returns AntigravityExecutor (not DefaultExecutor)", () => {
const executor = getExecutor("agy");
expect(executor).toBeInstanceOf(AntigravityExecutor);
});
test("getExecutor('antigravity') returns AntigravityExecutor", () => {
const executor = getExecutor("antigravity");
expect(executor).toBeInstanceOf(AntigravityExecutor);
});
test("getExecutor('agy') builds valid streaming URL", () => {
const executor = getExecutor("agy");
const url = executor.buildUrl("gemini-3-flash", true);
expect(url).toContain("streamGenerateContent?alt=sse");
});
test("getExecutor('agy') builds valid non-streaming URL", () => {
const executor = getExecutor("agy");
const url = executor.buildUrl("gemini-3-flash", false);
expect(url).toContain("streamGenerateContent?alt=sse");
});
test("getExecutor('agy') buildHeaders returns Bearer auth", () => {
const executor = getExecutor("agy");
const headers = executor.buildHeaders({ accessToken: "test-token" });
expect(headers.Authorization).toBe("Bearer test-token");
});References
- Coverage must stay ≥ 75%. Using Vitest APIs ensures proper coverage tracking and integration with the repository's test suite. (link)
There was a problem hiding this comment.
Following the existing pattern. Happy to rewrite, but should be for all then, which shouldn't be the scope of this PR imho.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Status: 1 Issues Found | Recommendation: Address before merge
Overview
| Severity | Count |
|---|---|
| CRITICAL | 0 |
| WARNING | 1 |
| SUGGESTION | 0 |
Issue Details (click to expand)
WARNING
| File | Line | Issue |
|---|---|---|
open-sse/executors/index.ts |
52 | Missing provider registration - The provider ID "agy" should be registered in src/shared/constants/providers.ts and, if OAuth-based, in src/lib/oauth/constants/oauth.ts. Without this, requests using the "agy" provider may fail validation. |
Other Observations (not in diff)
Issues found in unchanged code that cannot receive inline comments:
| File | Line | Issue |
|---|---|---|
| None |
Files Reviewed (2 files)
open-sse/executors/index.ts- 1 issuestests/unit/executor-agy.test.ts- 0 issues
|
Replying to #2957 (comment) Thanks for the review. I kept
All use |
|
Replying to @kilo-code-bot review (#pullrequestreview-4396079202) The WARNING flags that
This is pre-existing registration. The only missing piece was the executor map entry, which this PR adds. |
|
Thank you for your contribution! This PR has been reviewed and integrated into the upcoming |
…iegosouzapw#2957) Integrated into release/v3.8.8
…iegosouzapw#2957) Integrated into release/v3.8.8
…iegosouzapw#2957) Integrated into release/v3.8.8
2 participants
Summary
agyprovider was registered inproviderRegistry.tswithexecutor: "antigravity"but the executor map inexecutors/index.tsonly had an"antigravity"entry, no"agy"entry. This causedgetExecutor("agy")to fall through toDefaultExecutor, which tried to readthis.config.baseUrl(singular). Theagyprovider config only hasbaseUrls(plural), sobuildUrl()returnedundefined. Passingundefinedtofetch()triggered aTypeError: Cannot read properties of undefined (reading 'toString')inside Node's undici HTTP client, surfaced as a 502 to the client.Fix: Add
agy: new AntigravityExecutor()to the executor map. One line.Related Issues
Validation
npm run lintclean (0 warnings)npm run typecheck:corecleanagy/gemini-3-flash,agy/claude-sonnet-4-6,agy/gemini-3.5-flash-lowall return valid responses on patched dev serverTests Added Or Updated
tests/unit/executor-agy.test.tswith 5 assertions:getExecutor("agy")returnsAntigravityExecutor(notDefaultExecutor)getExecutor("antigravity")returnsAntigravityExecutorbuildUrl()returns valid streaming endpointbuildUrl()returns valid non-streaming endpoint (always streams)buildHeaders()sets Bearer auth fromaccessTokenCoverage Notes
open-sse/executors/index.ts(one map entry). The new test file covers executor resolution, URL building, and header construction for theagyprovider. Existing antigravity tests (antigravity-credits,antigravity-version,antigravity-projectid,executor-default-base) all pass unchanged.Reviewer Notes
agyprovider was unusable since its introduction because the executor was never wired up.AntigravityExecutorconstructor hardcodessuper("antigravity", PROVIDERS.antigravity), so both"antigravity"and"agy"share the same provider config. This is correct: identical base URLs, OAuth credentials, and request format.