fix(adapters/gemini-local): address PR review feedback for skills and formatting

- Isolate skills injection using a temporary directory mapped via GEMINI_CLI_HOME, mirroring the claude-local sandbox approach instead of polluting the global ~/.gemini/skills directory. - Update the environment probe to use `--output-format stream-json` so the payload matches the downstream parseGeminiJsonl parser. - Deduplicate `firstNonEmptyLine` helper by extracting it to a shared `utils.ts` module. - Clean up orphaned internal exports and update adapter documentation.
2026-03-08 19:20:43 +05:30
parent af97259a9c
commit ec445e4cc9
5 changed files with 65 additions and 105 deletions
@@ -20,6 +20,7 @@ import {
 } from "@paperclipai/adapter-utils/server-utils";
 import { DEFAULT_GEMINI_LOCAL_MODEL } from "../index.js";
 import { isGeminiUnknownSessionError, parseGeminiJsonl } from "./parse.js";
+import { firstNonEmptyLine } from "./utils.js";

 const __moduleDir = path.dirname(fileURLToPath(import.meta.url));
 const PAPERCLIP_SKILLS_CANDIDATES = [
@@ -27,15 +28,6 @@ const PAPERCLIP_SKILLS_CANDIDATES = [
  path.resolve(__moduleDir, "../../../../../skills"),
 ];

-function firstNonEmptyLine(text: string): string {
-  return (
-    text
-      .split(/\r?\n/)
-      .map((line) => line.trim())
-      .find(Boolean) ?? ""
-  );
-}
-
 function hasNonEmptyEnvValue(env: Record<string, string>, key: string): boolean {
  const raw = env[key];
  return typeof raw === "string" && raw.trim().length > 0;
@@ -61,10 +53,6 @@ function renderPaperclipEnvNote(env: Record<string, string>): string {
  ].join("\n");
 }

-function geminiSkillsHome(): string {
-  return path.join(os.homedir(), ".gemini", "skills");
-}
-
 async function resolvePaperclipSkillsDir(): Promise<string | null> {
  for (const candidate of PAPERCLIP_SKILLS_CANDIDATES) {
    const isDir = await fs.stat(candidate).then((s) => s.isDirectory()).catch(() => false);
@@ -73,62 +61,27 @@ async function resolvePaperclipSkillsDir(): Promise<string | null> {
  return null;
 }

-type EnsureGeminiSkillsInjectedOptions = {
-  skillsDir?: string | null;
-  skillsHome?: string;
-  linkSkill?: (source: string, target: string) => Promise<void>;
-};
-
-export async function ensureGeminiSkillsInjected(
-  onLog: AdapterExecutionContext["onLog"],
-  options: EnsureGeminiSkillsInjectedOptions = {},
-) {
-  const skillsDir = options.skillsDir ?? await resolvePaperclipSkillsDir();
-  if (!skillsDir) return;
-
-  const skillsHome = options.skillsHome ?? geminiSkillsHome();
-  try {
-    await fs.mkdir(skillsHome, { recursive: true });
-  } catch (err) {
-    await onLog(
-      "stderr",
-      `[paperclip] Failed to prepare Gemini skills directory ${skillsHome}: ${err instanceof Error ? err.message : String(err)}\n`,
-    );
-    return;
-  }
-
-  let entries: Dirent[];
-  try {
-    entries = await fs.readdir(skillsDir, { withFileTypes: true });
-  } catch (err) {
-    await onLog(
-      "stderr",
-      `[paperclip] Failed to read Paperclip skills from ${skillsDir}: ${err instanceof Error ? err.message : String(err)}\n`,
-    );
-    return;
-  }
-
-  const linkSkill = options.linkSkill ?? ((source: string, target: string) => fs.symlink(source, target));
+/**
+ * Create a tmpdir with `.gemini/skills/` containing symlinks to skills from
+ * the repo's `skills/` directory, so `GEMINI_CLI_HOME` makes Gemini CLI discover
+ * them as proper registered skills.
+ */
+async function buildSkillsDir(): Promise<string> {
+  const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-gemini-skills-"));
+  const target = path.join(tmp, ".gemini", "skills");
+  await fs.mkdir(target, { recursive: true });
+  const skillsDir = await resolvePaperclipSkillsDir();
+  if (!skillsDir) return tmp;
+  const entries = await fs.readdir(skillsDir, { withFileTypes: true });
  for (const entry of entries) {
-    if (!entry.isDirectory()) continue;
-    const source = path.join(skillsDir, entry.name);
-    const target = path.join(skillsHome, entry.name);
-    const existing = await fs.lstat(target).catch(() => null);
-    if (existing) continue;
-
-    try {
-      await linkSkill(source, target);
-      await onLog(
-        "stderr",
-        `[paperclip] Injected Gemini skill "${entry.name}" into ${skillsHome}\n`,
-      );
-    } catch (err) {
-      await onLog(
-        "stderr",
-        `[paperclip] Failed to inject Gemini skill "${entry.name}" into ${skillsHome}: ${err instanceof Error ? err.message : String(err)}\n`,
+    if (entry.isDirectory()) {
+      await fs.symlink(
+        path.join(skillsDir, entry.name),
+        path.join(target, entry.name),
      );
    }
  }
+  return tmp;
 }

 export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExecutionResult> {
@@ -150,21 +103,22 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
  const workspaceRepoRef = asString(workspaceContext.repoRef, "");
  const workspaceHints = Array.isArray(context.paperclipWorkspaces)
    ? context.paperclipWorkspaces.filter(
-        (value): value is Record<string, unknown> => typeof value === "object" && value !== null,
-      )
+      (value): value is Record<string, unknown> => typeof value === "object" && value !== null,
+    )
    : [];
  const configuredCwd = asString(config.cwd, "");
  const useConfiguredInsteadOfAgentHome = workspaceSource === "agent_home" && configuredCwd.length > 0;
  const effectiveWorkspaceCwd = useConfiguredInsteadOfAgentHome ? "" : workspaceCwd;
  const cwd = effectiveWorkspaceCwd || configuredCwd || process.cwd();
  await ensureAbsoluteDirectory(cwd, { createIfMissing: true });
-  await ensureGeminiSkillsInjected(onLog);
+  const tmpHome = await buildSkillsDir();

  const envConfig = parseObject(config.env);
  const hasExplicitApiKey =
    typeof envConfig.PAPERCLIP_API_KEY === "string" && envConfig.PAPERCLIP_API_KEY.trim().length > 0;
  const env: Record<string, string> = { ...buildPaperclipEnv(agent) };
  env.PAPERCLIP_RUN_ID = runId;
+  if (tmpHome) env.GEMINI_CLI_HOME = tmpHome;
  const wakeTaskId =
    (typeof context.taskId === "string" && context.taskId.trim().length > 0 && context.taskId.trim()) ||
    (typeof context.issueId === "string" && context.issueId.trim().length > 0 && context.issueId.trim()) ||
@@ -350,12 +304,12 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
    const resolvedSessionId = attempt.parsed.sessionId ?? runtimeSessionId ?? runtime.sessionId ?? null;
    const resolvedSessionParams = resolvedSessionId
      ? ({
-          sessionId: resolvedSessionId,
-          cwd,
-          ...(workspaceId ? { workspaceId } : {}),
-          ...(workspaceRepoUrl ? { repoUrl: workspaceRepoUrl } : {}),
-          ...(workspaceRepoRef ? { repoRef: workspaceRepoRef } : {}),
-        } as Record<string, unknown>)
+        sessionId: resolvedSessionId,
+        cwd,
+        ...(workspaceId ? { workspaceId } : {}),
+        ...(workspaceRepoUrl ? { repoUrl: workspaceRepoUrl } : {}),
+        ...(workspaceRepoRef ? { repoRef: workspaceRepoRef } : {}),
+      } as Record<string, unknown>)
      : null;
    const parsedError = typeof attempt.parsed.errorMessage === "string" ? attempt.parsed.errorMessage.trim() : "";
    const stderrLine = firstNonEmptyLine(attempt.proc.stderr);
@@ -386,20 +340,26 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
    };
  };

-  const initial = await runAttempt(sessionId);
-  if (
-    sessionId &&
-    !initial.proc.timedOut &&
-    (initial.proc.exitCode ?? 0) !== 0 &&
-    isGeminiUnknownSessionError(initial.proc.stdout, initial.proc.stderr)
-  ) {
-    await onLog(
-      "stderr",
-      `[paperclip] Gemini resume session "${sessionId}" is unavailable; retrying with a fresh session.\n`,
-    );
-    const retry = await runAttempt(null);
-    return toResult(retry, true);
-  }
+  try {
+    const initial = await runAttempt(sessionId);
+    if (
+      sessionId &&
+      !initial.proc.timedOut &&
+      (initial.proc.exitCode ?? 0) !== 0 &&
+      isGeminiUnknownSessionError(initial.proc.stdout, initial.proc.stderr)
+    ) {
+      await onLog(
+        "stderr",
+        `[paperclip] Gemini resume session "${sessionId}" is unavailable; retrying with a fresh session.\n`,
+      );
+      const retry = await runAttempt(null);
+      return toResult(retry, true);
+    }

-  return toResult(initial);
+    return toResult(initial);
+  } finally {
+    if (tmpHome) {
+      fs.rm(tmpHome, { recursive: true, force: true }).catch(() => { });
+    }
+  }
 }
@@ -1,4 +1,4 @@
-export { execute, ensureGeminiSkillsInjected } from "./execute.js";
+export { execute } from "./execute.js";
 export { testEnvironment } from "./test.js";
 export { parseGeminiJsonl, isGeminiUnknownSessionError } from "./parse.js";
 import type { AdapterSessionCodec } from "@paperclipai/adapter-utils";
@@ -16,6 +16,7 @@ import {
 } from "@paperclipai/adapter-utils/server-utils";
 import { DEFAULT_GEMINI_LOCAL_MODEL } from "../index.js";
 import { parseGeminiJsonl } from "./parse.js";
+import { firstNonEmptyLine } from "./utils.js";

 function summarizeStatus(checks: AdapterEnvironmentCheck[]): AdapterEnvironmentTestResult["status"] {
  if (checks.some((check) => check.level === "error")) return "fail";
@@ -27,15 +28,6 @@ function isNonEmpty(value: unknown): value is string {
  return typeof value === "string" && value.trim().length > 0;
 }

-function firstNonEmptyLine(text: string): string {
-  return (
-    text
-      .split(/\r?\n/)
-      .map((line) => line.trim())
-      .find(Boolean) ?? ""
-  );
-}
-
 function commandLooksLike(command: string, expected: string): boolean {
  const base = path.basename(command).toLowerCase();
  return base === expected || base === `${expected}.cmd` || base === `${expected}.exe`;
@@ -146,7 +138,7 @@ export async function testEnvironment(
        return asStringArray(config.args);
      })();

-      const args = ["--output-format", "json"];
+      const args = ["--output-format", "stream-json"];
      if (model && model !== DEFAULT_GEMINI_LOCAL_MODEL) args.push("--model", model);
      if (yolo) args.push("--approval-mode", "yolo");
      if (extraArgs.length > 0) args.push(...extraArgs);
@@ -161,7 +153,7 @@ export async function testEnvironment(
          env,
          timeoutSec: 45,
          graceSec: 5,
-          onLog: async () => {},
+          onLog: async () => { },
        },
      );
      const parsed = parseGeminiJsonl(probe.stdout);
@@ -188,8 +180,8 @@ export async function testEnvironment(
          ...(hasHello
            ? {}
            : {
-                hint: "Try `gemini --output-format json \"Respond with hello.\"` manually to inspect full output.",
-              }),
+              hint: "Try `gemini --output-format json \"Respond with hello.\"` manually to inspect full output.",
+            }),
        });
      } else if (GEMINI_AUTH_REQUIRED_RE.test(authEvidence)) {
        checks.push({
@@ -0,0 +1,8 @@
+export function firstNonEmptyLine(text: string): string {
+    return (
+        text
+            .split(/\r?\n/)
+            .map((line) => line.trim())
+            .find(Boolean) ?? ""
+    );
+}