Skip to content

fix(gemini): externalize @google/genai for Electron and add relay HTT…#219

Open
nxsre wants to merge 3 commits into
OpenCoworkAI:mainfrom
nxsre:main
Open

fix(gemini): externalize @google/genai for Electron and add relay HTT…#219
nxsre wants to merge 3 commits into
OpenCoworkAI:mainfrom
nxsre:main

Conversation

@nxsre

@nxsre nxsre commented May 19, 2026

Copy link
Copy Markdown

fix(gemini): externalize @google/genai and add relay HTTP probes

Bundle @google/genai outside the pi-ai chunk so models.get works in diagnostics and runtime. Add HTTP fallbacks for one-api Gemini relays and ship genai deps in the installer.

Summary

Fixes broken Custom + Gemini + custom Base URL connectivity checks in the Electron main process (e.g. one-api at http://127.0.0.1:13000/gemini).

Problem: Auth and model diagnostic steps fail with Cannot read properties of undefined (reading 'get') within ~7ms. The same proxy works via gemini-cli and curl; the failure is a bundled SDK initialization issue, not a network timeout.

Root cause: Vite bundles @google/genai into the same mega-chunk as pi-ai. The resulting GoogleGenAI instance does not initialize client.models correctly, so models.get() and completeSimple both fail.

This PR:

  • Marks @google/genai as external in the main-process Vite build and loads the Node package at runtime via createRequire (google-genai-loader.ts).
  • Adds HTTP probe fallbacks for Gemini-compatible relays such as one-api (gemini-relay-probe.ts):
    • GET …/v1beta/models/{model} (auth / metadata)
    • POST …/models/{model}:generateContent (matches pi-ai when apiVersion is cleared for custom baseUrl)
  • Includes @google/genai and transitive dependencies in the installer (electron-builder.yml).

Recommended proxy settings (one-api):

Field Value
Provider Custom
Protocol Gemini
Base URL http://127.0.0.1:13000/gemini
API Key one-api sk-… token (not a Google AI Studio key)
Model e.g. gemini-2.5-flash, gemini-3.1-pro-preview (must be allowed on the token)

Type of change

  • Bug fix (fix)
  • New feature (feat)
  • Refactor / performance (refactor / perf)
  • Documentation (docs)
  • Tests (test)
  • Build / CI (build / ci)
  • Other:

Checklist

  • Code follows the project style (TypeScript strict, ESLint, Prettier)
  • Commit messages follow Conventional Commits (feat:, fix:, etc.)
  • Self-review completed — no debug logs, no commented-out code
  • Tests added or updated for the changed behaviour
  • npm run test passes locally
  • npm run lint passes locally
  • UI changes tested on both macOS and Windows (or noted as platform-specific)
  • New user-facing strings added to i18n files (en + zh) — N/A (no new UI copy)

Testing

  1. Unit tests

    npm run test -- --run src/tests/config/google-genai-loader.test.ts src/tests/config/api-diagnostics.test.ts
  2. Build verification

    npx vite build
    • Confirm dist-electron/main/google-*.js uses require("@google/genai") (external, not inlined).
    • Confirm api-diagnostics-*.js loads genai via createRequire("@google/genai").
  3. Manual (one-api relay)

    • Provider: Custom, Protocol: Gemini
    • Base URL: http://127.0.0.1:3000/gemini
    • API Key: one-api sk-… token
    • Model: gemini-2.5-flash or gemini-3.1-pro-preview
    • Test connection in settings and full diagnostics (dns → model) should pass.
    • Send a Gemini chat message to verify runtime inference via externalized pi-ai + genai.
  4. Fallback paths

    • If the SDK still reports reading 'get', auth should fall back to HTTP GET; model probe / test connection should fall back to HTTP generateContent and succeed.
  5. Regression (optional)

    • Official Google Gemini API without a custom Base URL should still work.

Screenshots / recordings (if applicable)

N/A — main-process diagnostics and connection testing only; no UI layout changes.

Example logs before fix:

[Diagnostics] Gemini auth probe unavailable, continuing to model check: Cannot read properties of undefined (reading 'get')
[OneShot] Provider error-as-resolve: error Cannot read properties of undefined (reading 'get')
[Diagnostics] Failed { failedAt: 'model', error: "Cannot read properties of undefined (reading 'get')" }

…P probes

Bundle @google/genai outside the pi-ai chunk so models.get works in diagnostics
and runtime. Add HTTP fallbacks for one-api Gemini relays and ship genai deps in
the installer.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review mode: initial

Findings

  • [MAJOR] isGeminiSdkProbeUnavailableError regex is too broad: /reading ['"]get['"]|reading get/i may match unrelated error messages (e.g., Something went wrong while reading getter). This could cause false positives where non-SDK-init errors trigger the HTTP fallback unnecessarily, potentially masking real network/auth issues. Narrow the pattern to require 'models.get' or 'client.models.get' context, or match the exact error string from the SDK (Cannot read properties of undefined (reading 'get')).
    src/main/config/gemini-relay-probe.ts:10
    Suggested fix:

    export function isGeminiSdkProbeUnavailableError(message: string): boolean {
      return /models\.get|client\.models\.get|cannot read properties of undefined \(reading 'get'\)/i.test(message);
    }
  • [MAJOR] fetchGeminiRelayModelMetadata throws an error with a status property attached, but the type assertion as Error & { status?: number } is not checked. Callers (api-diagnostics.ts line ~413) catch with httpErr typed as unknown and pass to getApiErrorInfo, which relies on err.status. If the error is not the constructed one (e.g., an AbortError), status will be undefined, potentially causing getApiErrorInfo to misreport. Ensure the thrown error always carries a status or handle undefined status gracefully.
    src/main/config/gemini-relay-probe.ts:110-113
    Suggested fix:

      if (!response.ok) {
        const body = await response.text();
        const err = new Error(body || `HTTP ${response.status}`);
        (err as any).status = response.status;
        throw err;
      }

    (or use a custom error class with a typed status field).

  • [MINOR] Missing unit tests for the critical functions in gemini-relay-probe.ts (probeGeminiRelayGenerateContent, fetchGeminiRelayModelMetadata, isGeminiSdkProbeUnavailableError). The integration code in api-diagnostics.ts and claude-sdk-one-shot.ts relies on these functions; without tests, regressions are harder to catch. Add tests mocking fetch and verifying fallback behavior, error mapping, and correct URL construction.
    src/tests/config/gemini-relay-probe.test.ts (new)

  • [MINOR] In api-diagnostics.ts, the fallback in stepAuth for Gemini auth (lines ~388-417) uses Date.now() - start to update step.latencyMs on success, but start is not defined in the provided patch. The variable start is declared earlier (likely in the surrounding function). For consistency, verify start captures the correct timestamp before the auth step, not before the outer diagnostic start. If start is not re-initialized per step, the latency may include time from earlier steps (DNS/TCP/TLS). Review of full api-diagnostics.ts needed.
    src/main/config/api-diagnostics.ts:399

  • [NIT] The geminiRelayMetadataBaseUrl function always appends /v1beta if the base does not end with /v1beta or /gemini. For custom base URLs that already contain a path (e.g., http://proxy.example.com/gemini/v1beta), an extra /v1beta may be appended, resulting in a malformed URL. Document this expectation or handle the case where the base already includes v1beta in a sub-path.
    src/main/config/gemini-relay-probe.ts:34-42

  • [NIT] The test for google-genai-loader mocks createRequire at module level, but the mock throws for unexpected require calls. This is brittle if the loader module ever imports other dependencies via createRequire. Consider using vi.mock('node:module', ...) with a spy that returns a function lazily, or use vi.importActual to preserve real behavior for non-@google/genai requires.
    src/tests/config/google-genai-loader.test.ts:12-18

Questions

  • Is the start variable in stepAuth intended to measure only the auth step, or the cumulative time including prior steps? If the latter, using Date.now() - start for step.latencyMs after fallback success may over-report. Please confirm.

Testing

  • Added tests: src/tests/config/google-genai-loader.test.ts (1 test, verifies loader returns mock and creates client)
  • Missing tests: No tests for gemini-relay-probe.ts (4 functions), no integration tests for the fallback logic in api-diagnostics.ts or claude-sdk-one-shot.ts.
  • Recommendation: Add unit tests for isGeminiSdkProbeUnavailableError, fetchGeminiRelayModelMetadata, probeGeminiRelayGenerateContent (mock global fetch), and a test for the diagnostics fallback flow.

Open Cowork Bot

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review mode: follow-up after new commits

What changed since last review

  • Removed unused import probeGeminiRelayGenerateContent from src/main/config/api-diagnostics.ts. This resolves the earlier minor issue about an unused import. ✅

Remaining Findings

  • [MAJOR] isGeminiSdkProbeUnavailableError regex is too broad: /reading ['"]get['"]|reading get/i may match many unrelated error strings (e.g., "Error reading getter"). This can cause false-positive HTTP fallback attempts, masking real SDK initialization failures. Narrow the pattern to require 'models.get' or 'client.models.get' context, or match the exact known error message.
    src/main/config/gemini-relay-probe.ts:10
    Suggested fix:

    export function isGeminiSdkProbeUnavailableError(message: string): boolean {
      return /models\.get|client\.models\.get|cannot read properties of undefined \(reading 'get'\)/i.test(message);
    }
  • [MAJOR] In fetchGeminiRelayModelMetadata (line 110-113), the thrown error is typed as Error & { status?: number } via assertion, but the status property is not guaranteed (e.g., AbortError from fetch timeout won't have status). Downstream getApiErrorInfo relies on err.status and may misreport. Ensure the error always carries a status or handle undefined gracefully.
    src/main/config/gemini-relay-probe.ts:110-113
    Suggested fix:

      if (!response.ok) {
        const body = await response.text();
        const err = new Error(body || `HTTP ${response.status}`);
        (err as any).status = response.status;  // or use a custom error class
        throw err;
      }
  • [MINOR] Missing unit tests for gemini-relay-probe.ts: probeGeminiRelayGenerateContent, fetchGeminiRelayModelMetadata, isGeminiSdkProbeUnavailableError, and error mapping functions. These are critical for fallback behavior; without tests, regressions are harder to catch.
    src/tests/config/gemini-relay-probe.test.ts (new)

  • [NIT] geminiRelayMetadataBaseUrl always appends /v1beta when the base does not end with /v1beta or /gemini. Custom base URLs that already contain a path like /custom/path will get an extra /v1beta, potentially forming a malformed URL. Either document this assumption or handle path segments more carefully.
    src/main/config/gemini-relay-probe.ts:34-42

  • [NIT] The test mock for createRequire in google-genai-loader.test.ts throws on unexpected require calls, making it brittle if the loader ever needs to require other modules. Consider using vi.mock with a spy that delegates to the real implementation for non-@google/genai calls.
    src/tests/config/google-genai-loader.test.ts:12-18

Testing

  • Existing test: src/tests/config/google-genai-loader.test.ts (1 test, passes).
  • Missing tests for gemini-relay-probe.ts as noted above. Recommended: unit tests mocking fetch and verifying URL construction + error mapping.

Open Cowork Bot

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review mode: follow-up after new commits

Findings

  • MAJOR Previous Gemini fix findings remain unaddressed. The issues identified in the initial review have not been fixed in the new commits:

    • isGeminiSdkProbeUnavailableError regex is too broad (src/main/config/gemini-relay-probe.ts:10). Narrow to require client.models.get context or exact known error message.
    • fetchGeminiRelayModelMetadata error handling (src/main/config/gemini-relay-probe.ts:110-113): status property not guaranteed on abort errors. Ensure it's always set or handle undefined.
    • Missing unit tests for gemini-relay-probe.ts.
    • geminiRelayMetadataBaseUrl always appends /v1beta (src/main/config/gemini-relay-probe.ts:34-42), potentially malforming custom paths.
    • The test mock in google-genai-loader.test.ts is brittle (src/tests/config/google-genai-loader.test.ts:12-18).
  • MAJOR New file agent-runner-loop-guard.ts introduces a complex loop detection mechanism without any unit tests. This is a significant new feature with zero test coverage. Additionally, the streak flags (streakWarnIssued, streakHaltIssued, streakAbortIssued) are not reset when a new hash streak begins, which could cause incorrect suppression of warnings in subsequent streaks. Add tests in src/tests/claude/agent-runner-loop-guard.test.ts and reset streak flags when currentHash changes in pushHash:

    } else {
      this.currentHash = hash;
      this.currentStreak = 1;
      this.streakWarnIssued = false;
      this.streakHaltIssued = false;
      this.streakAbortIssued = false;
    }
  • MINOR Version bump from 3.3.0 to 3.3.1 is included without justification. The PR description only references the Gemini fix, but the new loop guard feature is a significant addition that should be reflected in the description (and might warrant a minor version bump). Update the PR description to describe all changes.

  • MINOR The PR mixes two unrelated concerns: the Gemini relay fix (original changes) and the loop guard feature (new agent-runner-loop-guard.ts, version bump, patch updates). Consider separating into two focused PRs for clarity and easier review.

  • NIT In agent-runner-loop-guard.ts, the comment says "MD5 over stable keys" but the implementation uses MD5 over JSON-stringified sorted key fields. Update the comment for accuracy.

Summary

The PR includes two major categories of changes: (1) the original Gemini relay fix (externalizing @google/genai, adding HTTP fallback probes) and (2) an unrelated new loop guard feature (agent-runner-loop-guard.ts) plus infrastructure updates. The Gemini fix issues from the previous review remain entirely unaddressed. The loop guard is a new, untested feature with a potential bug in streak flag management. The PR mixes concerns and lacks tests for the new module.

Testing

  • Gemini fix: Missing tests for gemini-relay-probe.ts. The existing test for google-genai-loader.test.ts is brittle.
  • Loop guard: No tests added. Recommended: comprehensive unit tests for stableToolKey, messageCallsHash, and LoopGuard class covering streak, frequency, and edge cases.
  • Not run (automation)

Open Cowork Bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant