From 99e71f1e056f9aba434d1f4436c301efda5ec865 Mon Sep 17 00:00:00 2001 From: Anton Lykhoyda Date: Mon, 27 Apr 2026 14:36:21 +0200 Subject: [PATCH 1/3] fix(gemini): handle workspace-trust gate from gemini-cli v0.39.1+ (#26, ADR-069) gemini-cli v0.39.1 introduced FatalUntrustedWorkspaceError that fires before any model call when invoked headlessly against a directory not marked trusted in interactive mode. Fresh installs of ask-gemini-mcp hit this with no actionable guidance, and the catch block would have triggered a useless Flash retry against the same untrusted directory. Set GEMINI_TRUST_WORKSPACE=true by default before spawn (forward- compatible env var; older Geminis silently ignore it). Detect trust errors in the catch block via FatalUntrustedWorkspaceError / "not running in a trusted directory" patterns, surface a friendly remediation message, and short-circuit the Flash retry. Opt-out via ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1 for users who want trust enforcement. Argv shape unchanged (env var, not flag) so all existing toEqual([...]) argv tests stay green. +6 tests in geminiExecutor.test.ts under a new "workspace trust handling" describe block. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/BUGS.md | 6 ++ docs/DECISIONS.md | 7 ++ docs/ROADMAP.md | 1 + packages/gemini-mcp/README.md | 2 +- packages/gemini-mcp/src/constants.ts | 6 ++ .../utils/__tests__/geminiExecutor.test.ts | 70 ++++++++++++++++++- .../gemini-mcp/src/utils/geminiExecutor.ts | 34 +++++++-- 7 files changed, 119 insertions(+), 7 deletions(-) diff --git a/docs/BUGS.md b/docs/BUGS.md index f3a45b4..c835eff 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -2,6 +2,12 @@ ## Known Bugs (inherited from upstream) +### ~~Gemini CLI v0.39.1 workspace-trust gate breaks fresh installs~~ FIXED +- **Severity:** Critical (issue #26) +- **Affected versions:** Gemini CLI v0.39.1+ +- **Description:** gemini-cli v0.39.1 ([upstream PR #25814](https://github.com/google-gemini/gemini-cli/pull/25814)) added a `FatalUntrustedWorkspaceError` gate that fires before any model call when `gemini -p` is invoked against a directory that was never marked trusted in interactive mode. Fresh installs of `ask-gemini-mcp` against a never-opened directory failed with a raw stderr dump bubbled through `createGeminiStderrHandler` (which only special-cased `RESOURCE_EXHAUSTED`). The catch block at `geminiExecutor.ts:491` would also have triggered a Flash retry against the *same* untrusted directory — guaranteed to fail identically. +- **Fix:** `geminiExecutor.executeGeminiCLI()` now sets `process.env.GEMINI_TRUST_WORKSPACE = "true"` by default before spawn (forward-compatible env var; older Geminis silently ignore it). Opt-out via `ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1`. Catch block detects `FatalUntrustedWorkspaceError` / `not running in a trusted directory` patterns and short-circuits with a friendly remediation message (no Flash retry). See ADR-069. + ### ~~Deprecated `-p` flag causes error~~ FIXED - **Severity:** Critical - **Upstream:** Issue #48, PRs #56, #43 diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 798b552..13cffd6 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -1,5 +1,12 @@ # Architectural Decisions +## ADR-069: Gemini Workspace-Trust Compatibility — Default-On Env Var, Friendly Error, No Flash Retry +- **Date:** 2026-04-27 +- **Status:** Accepted (resolves issue #26) +- **Context:** gemini-cli v0.39.1 (released 2026-04-24, upstream PR [#25814](https://github.com/google-gemini/gemini-cli/pull/25814)) shipped a `FatalUntrustedWorkspaceError` gate that fires before any model call when `gemini -p` is invoked against a directory that was never marked trusted in interactive mode. Fresh installs of `ask-gemini-mcp` on v0.39.1+ would fail with a raw stderr dump and no actionable guidance — the project's existing stderr handler in `geminiExecutor.ts:379` only specialized RESOURCE_EXHAUSTED, so trust errors fell through to a generic spawn failure surface. Worse, the catch block at `geminiExecutor.ts:491` would fire the quota-fallback path on any non-quota error mismatch, meaning a trust error on Pro would have triggered a Flash retry against the *same untrusted directory* — guaranteed to fail identically, just slower. Three fix approaches were considered: (A) add `--skip-trust` to argv; (B) set `GEMINI_TRUST_WORKSPACE=true` as a parent-process env var that propagates via `getSpawnEnv()`'s `{...process.env, ...}` spread; (C) extend `executeCommand` to accept per-call `extraEnv`. Option A is backward-incompatible — the local install at the time of writing was v0.38.2, which has no `--skip-trust` flag, so a fresh argv flag would error on older Geminis with "unknown option." Option B is the upstream-blessed compatibility surface (the gemini-cli's own CI uses `GEMINI_TRUST_WORKSPACE=true` in E2E workflows) and unknown env vars are silently ignored on older versions, making it forward-and-backward safe. Option C would be the cleanest abstraction but adds a parameter to a shared API for one caller's needs — explicit "no premature abstraction" violation per the project's CLAUDE.md. Option B was selected. +- **Decision:** (1) `packages/gemini-mcp/src/constants.ts` gains a `WORKSPACE_TRUST_PATTERNS` array containing both the typed error class name (`FatalUntrustedWorkspaceError`, stable identifier in stack traces) and the human-readable string (`not running in a trusted directory`, surfaces in stderr text) — matching either makes detection robust against upstream changing how the error surfaces in either channel. New `ERROR_MESSAGES.WORKSPACE_TRUST_REQUIRED` string lists three remediation paths in priority order: unset `ASK_GEMINI_REQUIRE_WORKSPACE_TRUST` (let us re-apply the safe default), set `GEMINI_TRUST_WORKSPACE=true` yourself, or run `gemini` interactively in this directory once and mark it trusted. New `STATUS_MESSAGES.WORKSPACE_TRUST_DETECTED` for the stderr-handler log line. (2) `packages/gemini-mcp/src/utils/geminiExecutor.ts` gains a private `ensureWorkspaceTrustEnv()` helper called at the top of `executeGeminiCLI`. The helper has three idempotency guards: `process.env.ASK_GEMINI_REQUIRE_WORKSPACE_TRUST === "1"` (user opted out — do nothing), `process.env.GEMINI_TRUST_WORKSPACE !== undefined` (user pre-set their own value — don't override), otherwise sets `process.env.GEMINI_TRUST_WORKSPACE = "true"`. The mutation propagates to all subsequent spawned children for free because `shellPath.ts:getSpawnEnv()` already does `{...process.env, PATH: resolveShellPath()}` — no `commandExecutor.ts` changes required. (3) `createGeminiStderrHandler` now tracks both `quotaLogged` and `trustLogged` flags so the handler logs each error class once per invocation. (4) The catch block inserts a workspace-trust check **before** the quota check via a new `isWorkspaceTrustError(message)` helper — this is critical, because without short-circuiting, a trust error on Pro would either silently fall through to the generic re-throw OR (worse) hit the Flash retry path which would just fail again on the same untrusted directory. When detected, the catch throws a fresh `Error(ERROR_MESSAGES.WORKSPACE_TRUST_REQUIRED)` instead of bubbling the raw upstream error. (5) 6 new tests in `geminiExecutor.test.ts` under a new `executeGeminiCLI workspace trust handling` describe block, with proper `process.env` save-and-restore in `beforeEach`/`afterEach` to prevent cross-test env leakage: default behavior sets the env var, `ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1` skips it, pre-existing user value isn't overridden, both error patterns (`FatalUntrustedWorkspaceError` and `not running in a trusted directory`) throw the friendly message and short-circuit Flash retry, and the orthogonality test that quota errors still trigger Flash fallback unchanged. **Explicitly out of scope:** Action items #2 (stdin piping for >16KiB prompts), #3 (Codex stabilized hooks smoke test), #4 (carried sessionId round-trip + untrusted-workspace integration test), #5–7 (carried-from-#24/#25 items) — each warrants its own focused PR rather than bundling. Issue #26 listed all 7 as candidates with #1 explicitly tagged "Hotfix required? Arguably yes for action #1" — this ADR ships only #1. +- **Consequences:** Fresh installs of `ask-gemini-mcp` on gemini-cli v0.39.1+ now work without manual workspace-trust setup, restoring parity with the ≤ v0.39.0 behavior the MCP server was written against. The env-var approach is forward-and-backward compatible: older Geminis (v0.38.x and earlier, which have no workspace-trust feature) silently ignore the unknown env var; newer Geminis (v0.39.1+) honor it. Power users who *want* workspace-trust enforcement (e.g., shared CI runners where directory provenance matters) can opt in by setting `ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1` in their MCP config — when they then hit an untrusted directory, they get the friendly remediation message instead of a raw upstream error. Zero argv shape change, so all 25+ argv-based `toEqual([...])` tests in `geminiExecutor.test.ts` stay green untouched — the brittleness of those exact-array assertions now only fires on real argv changes, not env-side tweaks. The Flash retry cascade is preserved for genuine quota errors (orthogonality test confirms) but correctly skipped for trust errors. Test count: 88 in gemini-mcp (was 82, +6 trust tests); 422 across all packages. Lint clean across all 6 packages, full build clean. The friendly error message is markdown-readable so MCP clients render it nicely; the three numbered remediation steps mean a user can copy-paste the right command without reading the upstream PR. Future work: actions #2–7 from issue #26 each get their own PR; the integration-test scaffold for an actual untrusted-workspace round-trip (action #4) is the natural next step but requires either a git-isolated tempdir fixture or a vi-mocked `which` to skip the real `gemini` binary path. Deferred. + ## ADR-068: `/codex-image` Plugin Skill — Skill-Only Architecture for Codex Image Generation - **Date:** 2026-04-24 - **Status:** Accepted diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index ef96af8..df01cf0 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -34,6 +34,7 @@ - See [design doc](plans/2026-02-26-ask-llm-mcp-design.md) ## Priority 9: Bug Fixes (GitHub Issues) +- [x] **#26 Gemini CLI v0.39.1 workspace-trust gate** — gemini-cli v0.39.1 added `FatalUntrustedWorkspaceError` to headless (`-p`) mode, breaking fresh installs in directories never marked trusted. Fix: `geminiExecutor` sets `GEMINI_TRUST_WORKSPACE=true` by default (forward-compatible env var, ignored on older Geminis); opt-out via `ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1`. Trust errors short-circuit before the Flash retry (same dir would just fail again) and surface a friendly remediation message. ADR-069 - [x] **npm 9 EUNSUPPORTEDPROTOCOL workspace:*** — `npx -y ask-llm-mcp` failed under Claude Desktop's Node 18 / npm 9.7.1. Root cause + fix in ADR-052; lifecycle corrected in 1.5.7 / 0.2.7 - [x] **MCP Registry publish failures** — `server.json` versions were all set to the gemini tag version, causing 404 validation errors for codex/ollama/llm on the MCP registry. Fixed by reading each package's own `package.json` version - [x] **Smoke test rate-limit-self-defeating loop** — Pre-push smoke tests burned the very Gemini quota the next push needed, causing intermittent push failures within ~10-minute windows. Added quota-detection escape (`scripts/smoke-test.sh`) that treats 429/quota errors as skip-with-warning, with `FORCE_SMOKE=1` opt-in to restore hard-fail (ADR-051) diff --git a/packages/gemini-mcp/README.md b/packages/gemini-mcp/README.md index 6273114..b8152f1 100644 --- a/packages/gemini-mcp/README.md +++ b/packages/gemini-mcp/README.md @@ -120,7 +120,7 @@ Add to `opencode.json` in your project (or `~/.config/opencode/opencode.json` fo ## Prerequisites - **[Node.js](https://nodejs.org/)** v20.0.0 or higher (LTS) -- **[Google Gemini CLI](https://github.com/google-gemini/gemini-cli)** installed and authenticated +- **[Google Gemini CLI](https://github.com/google-gemini/gemini-cli)** installed and authenticated — **v0.39.1+ recommended** (earlier versions still work; v0.39.1 introduced a workspace-trust gate that this server transparently handles via the `GEMINI_TRUST_WORKSPACE=true` environment variable; opt out with `ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1`) ## Tools diff --git a/packages/gemini-mcp/src/constants.ts b/packages/gemini-mcp/src/constants.ts index e6028f3..e7941b9 100644 --- a/packages/gemini-mcp/src/constants.ts +++ b/packages/gemini-mcp/src/constants.ts @@ -2,12 +2,16 @@ import type { BaseToolArguments } from "@ask-llm/shared"; export const QUOTA_PATTERNS = ["RESOURCE_EXHAUSTED", "TerminalQuotaError", "exhausted your capacity"] as const; +export const WORKSPACE_TRUST_PATTERNS = ["FatalUntrustedWorkspaceError", "not running in a trusted directory"] as const; + export const ERROR_MESSAGES = { QUOTA_EXCEEDED: "RESOURCE_EXHAUSTED", QUOTA_EXCEEDED_SHORT: "⚠️ Gemini Pro daily quota exceeded. Please retry with model: 'gemini-3-flash-preview'", TOOL_NOT_FOUND: "not found in registry", NO_PROMPT_PROVIDED: "Please provide a prompt for analysis. Use @ syntax to include files (e.g., '@largefile.js explain what this does') or ask general questions", + WORKSPACE_TRUST_REQUIRED: + "Gemini blocked this call because workspace-trust enforcement is enabled and the current directory is not trusted. To resolve, either (a) unset ASK_GEMINI_REQUIRE_WORKSPACE_TRUST so ask-gemini-mcp re-applies its safe default of GEMINI_TRUST_WORKSPACE=true, (b) export GEMINI_TRUST_WORKSPACE=true yourself, or (c) run `gemini` interactively in this directory once and mark it trusted.", } as const; export const STATUS_MESSAGES = { @@ -19,6 +23,8 @@ export const STATUS_MESSAGES = { PROCESSING_START: "🔍 Starting analysis (may take 5-15 minutes for large codebases)", PROCESSING_CONTINUE: "⏳ Still processing... Gemini is working on your request", PROCESSING_COMPLETE: "✅ Analysis completed successfully", + WORKSPACE_TRUST_DETECTED: + "🔒 Gemini reported a workspace-trust block; not retrying with Flash (same directory will fail again).", } as const; export const MODELS = { diff --git a/packages/gemini-mcp/src/utils/__tests__/geminiExecutor.test.ts b/packages/gemini-mcp/src/utils/__tests__/geminiExecutor.test.ts index 2c200ec..0179f84 100644 --- a/packages/gemini-mcp/src/utils/__tests__/geminiExecutor.test.ts +++ b/packages/gemini-mcp/src/utils/__tests__/geminiExecutor.test.ts @@ -1,5 +1,5 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { CLI, MODELS } from "../../constants.js"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CLI, ERROR_MESSAGES, MODELS } from "../../constants.js"; vi.mock("@ask-llm/shared", async (importOriginal) => { const actual = await importOriginal(); @@ -715,3 +715,69 @@ describe("executeGeminiCLI includeDirs support", () => { ]); }); }); + +describe("executeGeminiCLI workspace trust handling", () => { + let originalTrust: string | undefined; + let originalRequireTrust: string | undefined; + + beforeEach(() => { + originalTrust = process.env.GEMINI_TRUST_WORKSPACE; + originalRequireTrust = process.env.ASK_GEMINI_REQUIRE_WORKSPACE_TRUST; + delete process.env.GEMINI_TRUST_WORKSPACE; + delete process.env.ASK_GEMINI_REQUIRE_WORKSPACE_TRUST; + }); + + afterEach(() => { + if (originalTrust === undefined) delete process.env.GEMINI_TRUST_WORKSPACE; + else process.env.GEMINI_TRUST_WORKSPACE = originalTrust; + if (originalRequireTrust === undefined) delete process.env.ASK_GEMINI_REQUIRE_WORKSPACE_TRUST; + else process.env.ASK_GEMINI_REQUIRE_WORKSPACE_TRUST = originalRequireTrust; + }); + + it("sets GEMINI_TRUST_WORKSPACE=true by default", async () => { + await executeGeminiCLI({ prompt: "hello" }); + + expect(process.env.GEMINI_TRUST_WORKSPACE).toBe("true"); + }); + + it("does not set GEMINI_TRUST_WORKSPACE when ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1", async () => { + process.env.ASK_GEMINI_REQUIRE_WORKSPACE_TRUST = "1"; + + await executeGeminiCLI({ prompt: "hello" }); + + expect(process.env.GEMINI_TRUST_WORKSPACE).toBeUndefined(); + }); + + it("does not override a user-supplied GEMINI_TRUST_WORKSPACE value", async () => { + process.env.GEMINI_TRUST_WORKSPACE = "false"; + + await executeGeminiCLI({ prompt: "hello" }); + + expect(process.env.GEMINI_TRUST_WORKSPACE).toBe("false"); + }); + + it("throws friendly error and does not fall back to Flash on FatalUntrustedWorkspaceError", async () => { + mockExecuteCommand.mockRejectedValueOnce(new Error("FatalUntrustedWorkspaceError: workspace not trusted")); + + await expect(executeGeminiCLI({ prompt: "hello" })).rejects.toThrow(ERROR_MESSAGES.WORKSPACE_TRUST_REQUIRED); + expect(mockExecuteCommand).toHaveBeenCalledOnce(); + }); + + it("throws friendly error on the user-visible 'not running in a trusted directory' string", async () => { + mockExecuteCommand.mockRejectedValueOnce(new Error("Gemini CLI is not running in a trusted directory.")); + + await expect(executeGeminiCLI({ prompt: "hello" })).rejects.toThrow(ERROR_MESSAGES.WORKSPACE_TRUST_REQUIRED); + expect(mockExecuteCommand).toHaveBeenCalledOnce(); + }); + + it("does not flag quota errors as trust errors", async () => { + mockExecuteCommand + .mockRejectedValueOnce(new Error("RESOURCE_EXHAUSTED")) + .mockResolvedValueOnce(JSON.stringify({ response: "Flash response" })); + + const result = await executeGeminiCLI({ prompt: "hello" }); + + expect(result.response).toContain("Flash response"); + expect(mockExecuteCommand).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/gemini-mcp/src/utils/geminiExecutor.ts b/packages/gemini-mcp/src/utils/geminiExecutor.ts index f33ab3f..ffa7fcc 100644 --- a/packages/gemini-mcp/src/utils/geminiExecutor.ts +++ b/packages/gemini-mcp/src/utils/geminiExecutor.ts @@ -14,7 +14,14 @@ import { type UsageStats, validateChangeModeEdits, } from "@ask-llm/shared"; -import { CLI, MODELS, QUOTA_PATTERNS, STATUS_MESSAGES } from "../constants.js"; +import { + CLI, + ERROR_MESSAGES, + MODELS, + QUOTA_PATTERNS, + STATUS_MESSAGES, + WORKSPACE_TRUST_PATTERNS, +} from "../constants.js"; interface GeminiModelTokens { input?: number; @@ -378,11 +385,12 @@ function buildArgs( function createGeminiStderrHandler(): (chunk: string) => void { let buffer = ""; - let logged = false; + let quotaLogged = false; + let trustLogged = false; return (chunk: string) => { buffer += chunk; - if (!logged && buffer.includes("RESOURCE_EXHAUSTED")) { - logged = true; + if (!quotaLogged && buffer.includes("RESOURCE_EXHAUSTED")) { + quotaLogged = true; const modelMatch = buffer.match(/Quota exceeded for quota metric '([^']+)'/); const statusMatch = buffer.match(/status["\s]*[:=]\s*(\d+)/); const reasonMatch = buffer.match(/"reason":\s*"([^"]+)"/); @@ -391,10 +399,25 @@ function createGeminiStderrHandler(): (chunk: string) => void { const reason = reasonMatch ? reasonMatch[1] : "rateLimitExceeded"; Logger.error(`Gemini Quota Error: code=${status}, model=${model}, reason=${reason}`); } + if (!trustLogged && WORKSPACE_TRUST_PATTERNS.some((pattern) => buffer.includes(pattern))) { + trustLogged = true; + Logger.error(STATUS_MESSAGES.WORKSPACE_TRUST_DETECTED); + } }; } +function ensureWorkspaceTrustEnv(): void { + if (process.env.ASK_GEMINI_REQUIRE_WORKSPACE_TRUST === "1") return; + if (process.env.GEMINI_TRUST_WORKSPACE !== undefined) return; + process.env.GEMINI_TRUST_WORKSPACE = "true"; +} + +function isWorkspaceTrustError(message: string): boolean { + return WORKSPACE_TRUST_PATTERNS.some((pattern) => message.includes(pattern)); +} + export async function executeGeminiCLI(options: GeminiExecutorOptions): Promise { + ensureWorkspaceTrustEnv(); const { model, sandbox, changeMode, sessionId, includeDirs, onProgress } = options; let promptProcessed = options.prompt; @@ -490,6 +513,9 @@ ${promptProcessed} return result; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); + if (isWorkspaceTrustError(errorMessage)) { + throw new Error(ERROR_MESSAGES.WORKSPACE_TRUST_REQUIRED); + } const isQuotaError = QUOTA_PATTERNS.some((pattern) => errorMessage.toLowerCase().includes(pattern.toLowerCase())); if (isQuotaError && model !== MODELS.FLASH) { Logger.warn(`Gemini quota exceeded. Falling back to ${MODELS.FLASH}.`); From 458480e3f83ee9278b1cf0d9fb99672f78d338f4 Mon Sep 17 00:00:00 2001 From: Anton Lykhoyda Date: Mon, 27 Apr 2026 14:36:29 +0200 Subject: [PATCH 2/3] chore: bump ask-gemini-mcp to 1.6.1 and ask-llm-mcp to 0.3.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships the #26 workspace-trust fix to npm consumers. - packages/gemini-mcp: 1.6.0 -> 1.6.1 (carries the actual fix) - packages/llm-mcp: 0.3.1 -> 0.3.2 (orchestrator's prepack-bundle.mjs bundles gemini-mcp source at publish time, so the fix ships transitively to ask-llm-mcp users too) @ask-llm/plugin (also workspace-deps on gemini-mcp) is left at 0.3.1 — the plugin is distributed via marketplace on a separate cycle and was just bumped in f33b475. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/gemini-mcp/package.json | 2 +- packages/llm-mcp/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/gemini-mcp/package.json b/packages/gemini-mcp/package.json index b922ea9..c18287e 100644 --- a/packages/gemini-mcp/package.json +++ b/packages/gemini-mcp/package.json @@ -1,6 +1,6 @@ { "name": "ask-gemini-mcp", - "version": "1.6.0", + "version": "1.6.1", "mcpName": "io.github.Lykhoyda/ask-gemini", "description": "MCP server for Gemini CLI integration - for Claude IDE and other IDEs", "type": "module", diff --git a/packages/llm-mcp/package.json b/packages/llm-mcp/package.json index 2e074a7..9774124 100644 --- a/packages/llm-mcp/package.json +++ b/packages/llm-mcp/package.json @@ -1,6 +1,6 @@ { "name": "ask-llm-mcp", - "version": "0.3.1", + "version": "0.3.2", "mcpName": "io.github.Lykhoyda/ask-llm", "description": "Unified MCP server for multi-LLM consultation — registers tools from all available providers (Gemini, Codex, Ollama) behind runtime availability checks", "type": "module", From e03cfee141be132512d11c56209f10df1638d671 Mon Sep 17 00:00:00 2001 From: Anton Lykhoyda Date: Mon, 27 Apr 2026 15:21:20 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(gemini):=20apply=20PR=20#29=20review=20?= =?UTF-8?q?feedback=20=E2=80=94=20log=20severity,=20case-insensitive=20tru?= =?UTF-8?q?st=20check,=20opt-out=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from claude[bot] code review (#29): 1. Logger.error -> Logger.warn for the stderr-detected trust condition. The condition is detected and handled (the actual error surface is the thrown ERROR_MESSAGES.WORKSPACE_TRUST_REQUIRED in the catch block); warn matches severity better than error. 2. isWorkspaceTrustError now matches case-insensitively, mirroring the defensive normalization already applied to QUOTA_PATTERNS detection. Patterns are stable upstream identifiers today, but issue #21 (multi- pattern quota detection) is precedent that upstream error formatting does drift; the cost of the .toLowerCase() pair on a cold path is zero. 3. Two new tests: - opt-out path (ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1) combined with a workspace-trust error from gemini, asserts friendly remediation message AND that GEMINI_TRUST_WORKSPACE is not set. - case-insensitivity regression test (FATALUNTRUSTEDWORKSPACEERROR uppercase) locks in finding #2 so a future contributor cannot "simplify" the matching back to exact-case without breaking the test. Test count: gemini-mcp 88 -> 90; project 422 -> 424. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/utils/__tests__/geminiExecutor.test.ts | 16 ++++++++++++++++ packages/gemini-mcp/src/utils/geminiExecutor.ts | 5 +++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/gemini-mcp/src/utils/__tests__/geminiExecutor.test.ts b/packages/gemini-mcp/src/utils/__tests__/geminiExecutor.test.ts index 0179f84..fa56f15 100644 --- a/packages/gemini-mcp/src/utils/__tests__/geminiExecutor.test.ts +++ b/packages/gemini-mcp/src/utils/__tests__/geminiExecutor.test.ts @@ -780,4 +780,20 @@ describe("executeGeminiCLI workspace trust handling", () => { expect(result.response).toContain("Flash response"); expect(mockExecuteCommand).toHaveBeenCalledTimes(2); }); + + it("throws friendly error when ASK_GEMINI_REQUIRE_WORKSPACE_TRUST=1 and workspace is untrusted", async () => { + process.env.ASK_GEMINI_REQUIRE_WORKSPACE_TRUST = "1"; + mockExecuteCommand.mockRejectedValueOnce(new Error("FatalUntrustedWorkspaceError: workspace not trusted")); + + await expect(executeGeminiCLI({ prompt: "hello" })).rejects.toThrow(ERROR_MESSAGES.WORKSPACE_TRUST_REQUIRED); + expect(process.env.GEMINI_TRUST_WORKSPACE).toBeUndefined(); + expect(mockExecuteCommand).toHaveBeenCalledOnce(); + }); + + it("matches trust patterns case-insensitively (defends against upstream re-formatting)", async () => { + mockExecuteCommand.mockRejectedValueOnce(new Error("FATALUNTRUSTEDWORKSPACEERROR: capital edge case")); + + await expect(executeGeminiCLI({ prompt: "hello" })).rejects.toThrow(ERROR_MESSAGES.WORKSPACE_TRUST_REQUIRED); + expect(mockExecuteCommand).toHaveBeenCalledOnce(); + }); }); diff --git a/packages/gemini-mcp/src/utils/geminiExecutor.ts b/packages/gemini-mcp/src/utils/geminiExecutor.ts index ffa7fcc..098d847 100644 --- a/packages/gemini-mcp/src/utils/geminiExecutor.ts +++ b/packages/gemini-mcp/src/utils/geminiExecutor.ts @@ -401,7 +401,7 @@ function createGeminiStderrHandler(): (chunk: string) => void { } if (!trustLogged && WORKSPACE_TRUST_PATTERNS.some((pattern) => buffer.includes(pattern))) { trustLogged = true; - Logger.error(STATUS_MESSAGES.WORKSPACE_TRUST_DETECTED); + Logger.warn(STATUS_MESSAGES.WORKSPACE_TRUST_DETECTED); } }; } @@ -413,7 +413,8 @@ function ensureWorkspaceTrustEnv(): void { } function isWorkspaceTrustError(message: string): boolean { - return WORKSPACE_TRUST_PATTERNS.some((pattern) => message.includes(pattern)); + const lower = message.toLowerCase(); + return WORKSPACE_TRUST_PATTERNS.some((pattern) => lower.includes(pattern.toLowerCase())); } export async function executeGeminiCLI(options: GeminiExecutorOptions): Promise {