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/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/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..fa56f15 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,85 @@ 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); + }); + + 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 f33ab3f..098d847 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,26 @@ 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.warn(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 { + const lower = message.toLowerCase(); + return WORKSPACE_TRUST_PATTERNS.some((pattern) => lower.includes(pattern.toLowerCase())); +} + export async function executeGeminiCLI(options: GeminiExecutorOptions): Promise { + ensureWorkspaceTrustEnv(); const { model, sandbox, changeMode, sessionId, includeDirs, onProgress } = options; let promptProcessed = options.prompt; @@ -490,6 +514,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}.`); 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",