diff --git a/README.md b/README.md index 7d0257f..1f6faf5 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,7 @@ gitagent --repo https://github.com/org/repo "Add unit tests" | `--repo ` | `-r` | GitHub repo URL to clone and work on | | `--pat ` | | GitHub PAT (or set `GITHUB_TOKEN` / `GIT_TOKEN`) | | `--session ` | | Resume an existing session branch | +| `--read-only` | | With `--repo`: clone/checkout only, never commit or push | | `--model ` | `-m` | Override model (e.g. `anthropic:claude-sonnet-4-5-20250929`) | | `--sandbox` | `-s` | Run in sandbox VM | | `--prompt ` | `-p` | Single-shot prompt (skip REPL) | diff --git a/package-lock.json b/package-lock.json index 0901d27..54f09a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@open-gitagent/gitagent", - "version": "2.0.1", + "version": "2.0.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@open-gitagent/gitagent", - "version": "2.0.1", + "version": "2.0.2", "license": "MIT", "dependencies": { "@mariozechner/pi-agent-core": "^0.70.2", diff --git a/src/index.ts b/src/index.ts index ba4eeb2..ae6c0c4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -43,7 +43,7 @@ const green = (s: string) => `\x1b[32m${s}\x1b[0m`; interface ParsedArgs { model?: string; - dir: string; + dir?: string; prompt?: string; env?: string; sandbox?: boolean; @@ -52,13 +52,16 @@ interface ParsedArgs { repo?: string; pat?: string; session?: string; + readOnly?: boolean; voice?: string; } function parseArgs(argv: string[]): ParsedArgs { const args = argv.slice(2); let model: string | undefined; - let dir = process.cwd(); + // Left undefined unless --dir/-d is explicitly passed, so callers can tell + // "user gave no --dir" apart from "user's --dir value happens to equal cwd". + let dir: string | undefined; let prompt: string | undefined; let env: string | undefined; let sandbox = false; @@ -67,6 +70,7 @@ function parseArgs(argv: string[]): ParsedArgs { let repo: string | undefined; let pat: string | undefined; let session: string | undefined; + let readOnly = false; let voice: string | undefined; for (let i = 0; i < args.length; i++) { @@ -107,6 +111,9 @@ function parseArgs(argv: string[]): ParsedArgs { case "--session": session = args[++i]; break; + case "--read-only": + readOnly = true; + break; case "--voice": case "-v": // Accept optional backend name: --voice, --voice openai, --voice gemini @@ -124,7 +131,7 @@ function parseArgs(argv: string[]): ParsedArgs { } } - return { model, dir, prompt, env, sandbox, sandboxRepo, sandboxToken, repo, pat, session, voice }; + return { model, dir, prompt, env, sandbox, sandboxRepo, sandboxToken, repo, pat, session, readOnly, voice }; } function handleEvent( @@ -321,10 +328,10 @@ async function main(): Promise { return; } - const { model, dir: rawDir, prompt, env, sandbox: useSandbox, sandboxRepo, sandboxToken, repo, pat, session: sessionBranch, voice } = parseArgs(process.argv); + const { model, dir: rawDir, prompt, env, sandbox: useSandbox, sandboxRepo, sandboxToken, repo, pat, session: sessionBranch, readOnly, voice } = parseArgs(process.argv); // If --repo is given, derive a default dir from the repo URL (skip interactive prompt) - let dir = rawDir; + let dir = rawDir ?? process.cwd(); let localSession: LocalSession | undefined; if (repo) { @@ -340,8 +347,11 @@ async function main(): Promise { process.exit(1); } - // Default dir: /tmp/gitagent/ if no --dir given - if (dir === process.cwd()) { + // Default dir: /tmp/gitagent/ if --dir wasn't explicitly passed. + // Checked against `rawDir` (undefined unless the user passed --dir), not + // against `dir === process.cwd()` — a value-equality check can't tell + // "no --dir given" apart from "user explicitly passed --dir matching cwd". + if (rawDir === undefined) { const repoName = repo.split("/").pop()?.replace(/\.git$/, "") || "repo"; dir = resolve(`/tmp/gitagent/${repoName}`); } @@ -351,6 +361,7 @@ async function main(): Promise { token, dir, session: sessionBranch, + readOnly, }); dir = localSession.dir; console.log(dim(`Local session: ${localSession.branch} (${localSession.dir})`)); diff --git a/src/sdk-types.ts b/src/sdk-types.ts index 5ab79ca..d2528f8 100644 --- a/src/sdk-types.ts +++ b/src/sdk-types.ts @@ -112,6 +112,7 @@ export interface LocalRepoOptions { token: string; dir?: string; session?: string; + readOnly?: boolean; } // ── Sandbox options ───────────────────────────────────────────────────── diff --git a/src/sdk.ts b/src/sdk.ts index 55b941c..ce1b475 100644 --- a/src/sdk.ts +++ b/src/sdk.ts @@ -141,11 +141,18 @@ export function query(options: QueryOptions): Query { if (!token) { throw new Error("repo.token, GITHUB_TOKEN, or GIT_TOKEN is required with repo option"); } + const repoDir = options.repo.dir ?? options.dir; + if (!repoDir) { + throw new Error( + "repo mode requires an explicit `dir` (top-level `dir` or `repo.dir`) — refusing to default to the current working directory", + ); + } localSession = initLocalSession({ url: options.repo.url, token, - dir: options.repo.dir || dir, + dir: repoDir, session: options.repo.session, + readOnly: options.repo.readOnly, }); dir = localSession.dir; } diff --git a/src/session.ts b/src/session.ts index cdc6a45..f418b27 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1,5 +1,5 @@ -import { execSync } from "child_process"; -import { existsSync, mkdirSync, writeFileSync } from "fs"; +import { execSync, execFileSync } from "child_process"; +import { existsSync, mkdirSync, writeFileSync, realpathSync } from "fs"; import { resolve } from "path"; import { randomBytes } from "crypto"; @@ -10,6 +10,7 @@ export interface LocalRepoOptions { token: string; dir: string; session?: string; + readOnly?: boolean; } export interface LocalSession { @@ -29,7 +30,7 @@ function authedUrl(url: string, token: string): string { } function cleanUrl(url: string): string { - return url.replace(/^https:\/\/[^@]+@/, "https://"); + return url.replace(/^https:\/\/[^@]+@/, "https://").replace(/\.git$/, ""); } function git(args: string, cwd: string): string { @@ -56,6 +57,9 @@ function getDefaultBranch(cwd: string): string { export function initLocalSession(opts: LocalRepoOptions): LocalSession { const { url, token, session } = opts; + if (!opts.dir) { + throw new Error("repo.dir is required — refusing to guess a working directory for repo mode"); + } const dir = resolve(opts.dir); const aUrl = authedUrl(url, token); @@ -63,7 +67,39 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession { if (!existsSync(dir)) { execSync(`git clone --depth 1 --no-single-branch ${aUrl} ${dir}`, { stdio: "pipe" }); } else { - git(`remote set-url origin ${aUrl}`, dir); + // Refuse to operate unless `dir` is a git repo root in its own right. + // If `dir` has no local .git, git commands here would silently resolve + // to whatever ancestor repo does have one — e.g. a monorepo root. + let topLevel: string; + try { + topLevel = execSync("git rev-parse --show-toplevel", { cwd: dir, stdio: "pipe", encoding: "utf-8" }).trim(); + } catch { + throw new Error(`${dir} exists but is not a git repository — refusing to operate on it`); + } + if (realpathSync(topLevel) !== realpathSync(dir)) { + throw new Error( + `${dir} has no .git of its own — git commands here would escape to the ancestor repo at ${topLevel}. Refusing to run destructive commands.`, + ); + } + + // Refuse to reset a repo that's already tracking a different remote — + // it isn't the clone this call expects, even if the folder exists. + let existingOrigin = ""; + try { + existingOrigin = execSync("git remote get-url origin", { cwd: dir, stdio: "pipe", encoding: "utf-8" }).trim(); + } catch { + // no origin configured yet — fine, we're about to set one + } + if (existingOrigin && cleanUrl(existingOrigin) !== cleanUrl(url)) { + throw new Error( + `${dir} already tracks a different remote (${cleanUrl(existingOrigin)}), not ${url}. Refusing to overwrite it.`, + ); + } + + // `set-url` only updates an existing remote — it errors if `origin` + // isn't configured yet, which happens if `dir` is a repo the caller + // created themselves rather than one gitagent cloned previously. + git(`remote ${existingOrigin ? "set-url" : "add"} origin ${aUrl}`, dir); git("fetch origin", dir); // Reset local default branch to latest remote @@ -133,9 +169,12 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession { git("diff --cached --quiet", dir); // Nothing staged — skip } catch { - // There are staged changes + // There are staged changes. Use execFileSync with an argv array + // (not the shell-interpolated `git()` helper) since commitMsg may + // be an arbitrary caller-supplied string containing quotes, `$()`, + // backticks, etc. const commitMsg = msg || `gitagent: auto-commit (${branch})`; - git(`commit -m "${commitMsg}"`, dir); + execFileSync("git", ["commit", "-m", commitMsg], { cwd: dir, stdio: "pipe" }); } }, @@ -144,9 +183,11 @@ export function initLocalSession(opts: LocalRepoOptions): LocalSession { }, finalize() { - localSession.commitChanges(); - localSession.push(); - // Strip PAT from remote URL + if (!opts.readOnly) { + localSession.commitChanges(); + localSession.push(); + } + // Strip PAT from remote URL regardless of readOnly git(`remote set-url origin ${cleanUrl(url)}`, dir); }, }; diff --git a/test/mcp.test.ts b/test/mcp.test.ts index c9efb17..d2b890a 100644 --- a/test/mcp.test.ts +++ b/test/mcp.test.ts @@ -91,15 +91,15 @@ describe("buildToolsForConnection", () => { const echo = tools.find((t) => t.name === "srv__echo")!; const res = await echo.execute("call-1", { msg: "hello" }); - assert.equal(res, "hello"); + assert.equal(res.content[0].text, "hello"); const boom = tools.find((t) => t.name === "srv__boom")!; const boomRes = await boom.execute("call-2", {}); - assert.match(boomRes, /^Error: /); + assert.match(boomRes.content[0].text, /^Error: /); const pic = tools.find((t) => t.name === "srv__pic")!; const picRes = await pic.execute("call-3", {}); - assert.match(picRes, /\[image: image\/png/); + assert.match(picRes.content[0].text, /\[image: image\/png/); await conn.close(); }); @@ -142,7 +142,7 @@ describe("buildToolsForConnection — pagination & name sanitization", () => { }; const tools = await buildToolsForConnection({ name: "srv", client: mockClient, close: async () => {} }); const res = await tools[0].execute("c1", {}); - assert.match(res, /^Error: .*-32602/); + assert.match(res.content[0].text, /^Error: .*-32602/); }); });