diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 114c7b3ef..467263d15 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1871,6 +1871,7 @@ ### Fixed - Fixed `@`-mention auto-read injecting an unrelated, same-named file when a mention did not point at a real path — e.g. an npm scope like `@scope/`, a partial path, or a bare token. `generateFileMentionMessages` resolution previously fell back to prefix and repo-wide fuzzy matching (globbing the whole project on every such mention) and auto-read the single "best" guess. Resolution is now exact-only: a mention is auto-read only when it resolves to an existing file or directory; otherwise it is left as prose. The TUI `@`-selector already inserts the real, complete path before send, so post-send guessing was both unnecessary and the source of the wrong-file reads. Directories still resolve and are listed. Removes the per-mention `**/*` project scan. +- Fixed Git-mutating automation silently mis-handling pure Jujutsu workspaces (`.jj/repo/` present, no colocated `.git/`). `task/worktree.ts#getRepoRoot` previously threw a generic "Git repository not found" for isolated subagent setup, and `autoresearch/git.ts#ensureAutoresearchBranch` returned a soft "Not in a git repository" warning that let `/autoresearch` proceed with no branch isolation or auto-commits. Both paths now detect a pure jj workspace via the new `jj.isPureJjRepo` helper and surface an actionable Jujutsu-specific error pointing at `jj git init --colocate`. Colocated jj-git (both `.jj/` and `.git/` at the same root) and plain Git checkouts behave exactly as before ([#1935](https://github.com/can1357/oh-my-pi/issues/1935)). ## [15.9.2] - 2026-06-05 diff --git a/packages/coding-agent/src/autoresearch/git.ts b/packages/coding-agent/src/autoresearch/git.ts index 95c54700a..43cb0e886 100644 --- a/packages/coding-agent/src/autoresearch/git.ts +++ b/packages/coding-agent/src/autoresearch/git.ts @@ -1,5 +1,6 @@ import type { ExtensionAPI } from "../extensibility/extensions"; import * as git from "../utils/git"; +import * as jj from "../utils/jj"; import { normalizePathSpec } from "./helpers"; const AUTORESEARCH_BRANCH_PREFIX = "autoresearch/"; @@ -37,6 +38,17 @@ export async function ensureAutoresearchBranch( workDir: string, goal: string | null, ): Promise { + // Pure-jj check runs first so a jj workspace nested under an unrelated + // outer Git checkout is rejected at its own root rather than silently + // creating `autoresearch/*` branches and commits in the surrounding Git + // tree behind jj's back. + if (await jj.isPureJjRepo(workDir)) { + return { + ok: false, + error: "Autoresearch needs a Git checkout for branch isolation and baseline commits, but this workspace is pure Jujutsu (`.jj/` without a colocated `.git/`). Run `jj git init --colocate` to add a Git checkout before starting autoresearch.", + }; + } + const repoRoot = await git.repo.root(workDir); if (!repoRoot) { return { diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index a10220e41..f80b37ee7 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -6,6 +6,7 @@ import * as natives from "@oh-my-pi/pi-natives"; import { getWorktreeDir, hashPath, logger, Snowflake } from "@oh-my-pi/pi-utils"; import * as git from "../utils/git"; import { mapWithConcurrencyLimit } from "./parallel"; +import * as jj from "../utils/jj"; const { IsoBackendKind } = natives; type IsoBackendKind = natives.IsoBackendKind; @@ -28,12 +29,19 @@ export interface WorktreeBaseline { } export async function getRepoRoot(cwd: string): Promise { - const repoRoot = await git.repo.root(cwd); - if (!repoRoot) { - throw new Error("Git repository not found for isolated task execution."); + // Pure-jj check runs first so a jj workspace nested under an unrelated + // outer Git checkout is rejected at its own root rather than silently + // mutating the surrounding Git tree behind jj's back. + if (await jj.isPureJjRepo(cwd)) { + throw new Error( + "Isolated task execution requires a Git checkout, but this workspace is pure Jujutsu (`.jj/` without a colocated `.git/`). Run `jj git init --colocate` to add a Git checkout, or set `task.isolation.mode: none` to disable task isolation.", + ); } - return repoRoot; + const repoRoot = await git.repo.root(cwd); + if (repoRoot) return repoRoot; + + throw new Error("Git repository not found for isolated task execution."); } const GIT_NO_INDEX_NULL_PATH = process.platform === "win32" ? "NUL" : "/dev/null"; diff --git a/packages/coding-agent/src/utils/jj.ts b/packages/coding-agent/src/utils/jj.ts index 228325bff..655decf97 100644 --- a/packages/coding-agent/src/utils/jj.ts +++ b/packages/coding-agent/src/utils/jj.ts @@ -2,6 +2,7 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; import { $which } from "@oh-my-pi/pi-utils"; import { LRUCache } from "lru-cache/raw"; +import * as git from "./git"; // ════════════════════════════════════════════════════════════════════════════ // Types @@ -246,3 +247,49 @@ export const repo = { return (await repo.root(cwd)) !== null; }, }; + +/** + * Detect a "pure" Jujutsu workspace — one where Git-mutating automation has + * no safe Git target. Invoking `git checkout -b`, `git worktree add`, or + * `git apply` against a pure jj workspace either fails outright (no `.git/` + * present) or mutates state that jj itself cannot reconcile. + * + * `cwd` is "pure jj" iff its nearest jj workspace ancestor is **closer than** + * its nearest Git checkout ancestor (or no Git checkout is present at all). + * Both lookups walk upward from `cwd`, so the deeper ancestor is the one the + * user is actually working inside. + * + * Returns: + * - `false` for plain Git checkouts (no jj metadata anywhere up the tree). + * - `false` for colocated jj-git workspaces — `jj git init --colocate` keeps + * `.jj/` and `.git/` at the same root. + * - `false` when a nested Git checkout (e.g. a vendored repo or fixture) + * lives **under** an outer jj workspace; Git automation targets the inner + * repo and never touches the surrounding jj tree. + * - `true` when jj is the deeper ancestor — either a standalone pure jj + * workspace, or a jj workspace nested under an unrelated outer Git + * checkout, where Git automation against the outer root would silently + * bypass jj. + * - `false` for directories backed by neither tool. + */ +export async function isPureJjRepo(cwd: string): Promise { + const jjRoot = await repo.root(cwd); + if (jjRoot === null) return false; + const gitRoot = await git.repo.root(cwd); + if (gitRoot === null) return true; + return isStrictDescendant(path.resolve(jjRoot), path.resolve(gitRoot)); +} + +/** + * Return `true` when `child` is a strict descendant of `ancestor` (same path + * counts as `false`). Both arguments must already be resolved absolute paths. + */ +function isStrictDescendant(child: string, ancestor: string): boolean { + const rel = path.relative(ancestor, child); + if (rel === "" || rel === ".") return false; + if (rel.startsWith("..")) return false; + // `path.relative` returns an absolute path only when the two arguments + // live on different filesystem roots (Windows drives, UNC shares); not a + // real ancestor relationship. + return !path.isAbsolute(rel); +} diff --git a/packages/coding-agent/test/autoresearch-git.test.ts b/packages/coding-agent/test/autoresearch-git.test.ts new file mode 100644 index 000000000..503c2926e --- /dev/null +++ b/packages/coding-agent/test/autoresearch-git.test.ts @@ -0,0 +1,132 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { ensureAutoresearchBranch } from "../src/autoresearch/git"; +import type { ExtensionAPI } from "../src/extensibility/extensions"; +import * as jj from "../src/utils/jj"; + +const tempDirs: string[] = []; + +async function mkTempDir(prefix: string): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +async function runGit(cwd: string, args: string[]): Promise { + const env = { ...process.env, HOME: cwd, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null" }; + const proc = Bun.spawn(["git", "-C", cwd, ...args], { env, stdout: "ignore", stderr: "pipe" }); + const code = await proc.exited; + if (code !== 0) { + const stderr = await new Response(proc.stderr).text(); + throw new Error(`git ${args.join(" ")} failed (${code}): ${stderr}`); + } +} + +async function initGitWithCommit(dir: string): Promise { + await runGit(dir, ["init", "-q", "-b", "main"]); + await runGit(dir, ["config", "user.email", "test@example.com"]); + await runGit(dir, ["config", "user.name", "Test"]); + await fs.writeFile(path.join(dir, "README"), "seed\n"); + await runGit(dir, ["add", "."]); + await runGit(dir, ["commit", "-q", "-m", "init"]); +} + +// `ensureAutoresearchBranch` never invokes the `api` it receives — its two +// internal helpers (`readGitWorkDirPrefix`, `branchExists`) immediately +// `void api`. The stub keeps the signature honest without spinning up the +// real extension runtime. +const stubApi = {} as unknown as ExtensionAPI; + +afterEach(async () => { + jj.repo.clearRootCache(); + await Promise.all(tempDirs.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true }))); +}); + +describe("ensureAutoresearchBranch jj guardrails", () => { + it("rejects pure jj workspaces before touching git state", async () => { + const dir = await mkTempDir("omp-ar-purejj-"); + await fs.mkdir(path.join(dir, ".jj", "repo", "store"), { recursive: true }); + + const result = await ensureAutoresearchBranch(stubApi, dir, "demo"); + + expect(result.ok).toBe(false); + if (result.ok) throw new Error("unreachable"); + expect(result.error).toMatch(/pure Jujutsu/); + expect(result.error).toMatch(/jj git init --colocate/); + }); + + it("creates an autoresearch branch in a colocated jj-git workspace", async () => { + const dir = await mkTempDir("omp-ar-colocated-"); + await initGitWithCommit(dir); + await fs.mkdir(path.join(dir, ".jj", "repo", "store"), { recursive: true }); + + const result = await ensureAutoresearchBranch(stubApi, dir, "demo goal"); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("unreachable"); + expect(result.created).toBe(true); + expect(result.branchName).toMatch(/^autoresearch\/demo-goal-\d{8}$/); + }); + + it("creates an autoresearch branch in a plain git repo (unchanged behavior)", async () => { + const dir = await mkTempDir("omp-ar-plaingit-"); + await initGitWithCommit(dir); + + const result = await ensureAutoresearchBranch(stubApi, dir, "demo"); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("unreachable"); + expect(result.created).toBe(true); + expect(result.branchName).toMatch(/^autoresearch\/demo-\d{8}$/); + }); + + it("returns the soft no-git warning for directories backed by neither tool", async () => { + const dir = await mkTempDir("omp-ar-empty-"); + + const result = await ensureAutoresearchBranch(stubApi, dir, "demo"); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("unreachable"); + expect(result.branchName).toBeNull(); + expect(result.warning).toMatch(/Not in a git repository/); + }); + + it("rejects a pure jj workspace nested inside an unrelated outer git checkout", async () => { + // `git.repo.root(inner)` walks up and finds the outer .git — without + // the pure-jj check running first, autoresearch would create + // `autoresearch/*` branches and commits in the surrounding git tree + // behind jj's back. + const outer = await mkTempDir("omp-ar-nested-outer-"); + await initGitWithCommit(outer); + const inner = path.join(outer, "nested-jj"); + await fs.mkdir(path.join(inner, ".jj", "repo", "store"), { recursive: true }); + + const result = await ensureAutoresearchBranch(stubApi, inner, "demo"); + + expect(result.ok).toBe(false); + if (result.ok) throw new Error("unreachable"); + expect(result.error).toMatch(/pure Jujutsu/); + expect(result.error).toMatch(/jj git init --colocate/); + }); + + it("creates an autoresearch branch in a nested git checkout under an outer jj workspace", async () => { + // Mirror image of the case above: `jj.repo.root(inner)` finds the outer + // .jj, but `git.repo.root(inner)` finds the inner .git, so autoresearch + // safely targets the nested checkout and never touches the surrounding + // jj tree. + const outer = await mkTempDir("omp-ar-outerjj-"); + await fs.mkdir(path.join(outer, ".jj", "repo", "store"), { recursive: true }); + const inner = path.join(outer, "vendor"); + await fs.mkdir(inner, { recursive: true }); + await initGitWithCommit(inner); + + const result = await ensureAutoresearchBranch(stubApi, inner, "demo"); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("unreachable"); + expect(result.created).toBe(true); + expect(result.branchName).toMatch(/^autoresearch\/demo-\d{8}$/); + }); +}); diff --git a/packages/coding-agent/test/task/worktree.test.ts b/packages/coding-agent/test/task/worktree.test.ts index 958f20555..a5cad7d2d 100644 --- a/packages/coding-agent/test/task/worktree.test.ts +++ b/packages/coding-agent/test/task/worktree.test.ts @@ -7,10 +7,14 @@ import { captureDeltaPatch, ensureIsolation, getGitNoIndexNullPath, + getRepoRoot, mergeTaskBranches, parseIsolationMode, } from "@oh-my-pi/pi-coding-agent/task/worktree"; import * as natives from "@oh-my-pi/pi-natives"; +import * as jj from "@oh-my-pi/pi-coding-agent/utils/jj"; + +const tempDirs: string[] = []; async function runGit(repo: string, args: string[]): Promise { const proc = Bun.spawn(["git", ...args], { @@ -30,6 +34,27 @@ async function runGit(repo: string, args: string[]): Promise { return stdout.trim(); } +async function createGitRepo(): Promise<{ baseBranch: string; repo: string }> { + const repo = await fs.mkdtemp(path.join(os.tmpdir(), "omp-worktree-")); + tempDirs.push(repo); + await runGit(repo, ["init"]); + await runGit(repo, ["config", "user.email", "test@example.com"]); + await runGit(repo, ["config", "user.name", "Test User"]); + await fs.writeFile(path.join(repo, "merged.txt"), "base version\n"); + await fs.writeFile(path.join(repo, "staged.txt"), "base staged\n"); + await runGit(repo, ["add", "."]); + await runGit(repo, ["commit", "-m", "initial"]); + return { + baseBranch: await runGit(repo, ["branch", "--show-current"]), + repo, + }; +} + +afterEach(async () => { + vi.restoreAllMocks(); + jj.repo.clearRootCache(); + await Promise.all(tempDirs.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true }))); +}); describe("worktree isolation helpers", () => { it("returns platform-specific null path for git --no-index diffs", () => { const expected = process.platform === "win32" ? "NUL" : "/dev/null"; @@ -198,3 +223,59 @@ describe("worktree isolation helpers", () => { }); }); }); + +describe("getRepoRoot", () => { + it("returns the git root for a plain git checkout", async () => { + const { repo } = await createGitRepo(); + expect(await getRepoRoot(repo)).toBe(repo); + }); + + it("returns the git root for a colocated jj-git workspace", async () => { + const { repo } = await createGitRepo(); + await fs.mkdir(path.join(repo, ".jj", "repo", "store"), { recursive: true }); + expect(await getRepoRoot(repo)).toBe(repo); + }); + + it("rejects pure jj workspaces with an actionable Jujutsu message", async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-purejj-")); + tempDirs.push(dir); + await fs.mkdir(path.join(dir, ".jj", "repo", "store"), { recursive: true }); + await expect(getRepoRoot(dir)).rejects.toThrow(/pure Jujutsu/); + await expect(getRepoRoot(dir)).rejects.toThrow(/jj git init --colocate/); + }); + + it("preserves the generic git-not-found error for directories without any repo", async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-norepo-")); + tempDirs.push(dir); + await expect(getRepoRoot(dir)).rejects.toThrow("Git repository not found for isolated task execution."); + }); + + it("rejects a pure jj workspace nested inside an unrelated outer git checkout", async () => { + // `git.repo.root(inner)` walks up and finds the outer .git — without + // the pure-jj check running first, isolation would silently target the + // surrounding git tree behind jj's back. + const { repo: outer } = await createGitRepo(); + const inner = path.join(outer, "nested-jj"); + await fs.mkdir(path.join(inner, ".jj", "repo", "store"), { recursive: true }); + + await expect(getRepoRoot(inner)).rejects.toThrow(/pure Jujutsu/); + await expect(getRepoRoot(inner)).rejects.toThrow(/jj git init --colocate/); + }); + + it("returns the nested git root when a git checkout lives under an outer jj workspace", async () => { + // Mirror image of the case above: `jj.repo.root(inner)` finds the outer + // .jj, but `git.repo.root(inner)` finds the inner .git, so Git + // automation targets the nested checkout safely. Isolation must keep + // working here exactly as it did before the pure-jj guard landed. + const outer = await fs.mkdtemp(path.join(os.tmpdir(), "omp-outerjj-")); + tempDirs.push(outer); + await fs.mkdir(path.join(outer, ".jj", "repo", "store"), { recursive: true }); + const inner = path.join(outer, "vendor"); + await fs.mkdir(inner, { recursive: true }); + await runGit(inner, ["init", "-q", "-b", "main"]); + await runGit(inner, ["config", "user.email", "test@example.com"]); + await runGit(inner, ["config", "user.name", "Test"]); + + expect(await getRepoRoot(inner)).toBe(inner); + }); +}); diff --git a/packages/coding-agent/test/utils/jj.test.ts b/packages/coding-agent/test/utils/jj.test.ts index 9f5050120..1b0ff868a 100644 --- a/packages/coding-agent/test/utils/jj.test.ts +++ b/packages/coding-agent/test/utils/jj.test.ts @@ -77,3 +77,79 @@ describe("jj workspace detection", () => { expect(resolved?.storeDir).toBe(path.join(dir, ".jj", "repo", "store")); }); }); + +describe("isPureJjRepo", () => { + const tempDirs: string[] = []; + + afterEach(async () => { + jj.repo.clearRootCache(); + await Promise.all(tempDirs.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true }))); + }); + + async function createTempDir(prefix: string): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; + } + + async function initGit(dir: string): Promise { + const env = { ...process.env, HOME: dir, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null" }; + const exit = async (args: string[]) => { + const proc = Bun.spawn(["git", "-C", dir, ...args], { env, stdout: "ignore", stderr: "pipe" }); + const code = await proc.exited; + if (code !== 0) { + const stderr = await new Response(proc.stderr).text(); + throw new Error(`git ${args.join(" ")} failed (${code}): ${stderr}`); + } + }; + await exit(["init", "-q", "-b", "main"]); + await exit(["config", "user.email", "test@example.com"]); + await exit(["config", "user.name", "Test"]); + } + + it("flags a pure jj workspace (no colocated git)", async () => { + const dir = await createTempDir("omp-jj-pure-"); + await fs.mkdir(path.join(dir, ".jj", "repo", "store"), { recursive: true }); + expect(await jj.isPureJjRepo(dir)).toBe(true); + }); + + it("treats a colocated jj-git workspace as non-pure", async () => { + const dir = await createTempDir("omp-jj-colocated-"); + await fs.mkdir(path.join(dir, ".jj", "repo", "store"), { recursive: true }); + await initGit(dir); + expect(await jj.isPureJjRepo(dir)).toBe(false); + }); + + it("returns false for a plain git checkout (no jj metadata)", async () => { + const dir = await createTempDir("omp-jj-plaingit-"); + await initGit(dir); + expect(await jj.isPureJjRepo(dir)).toBe(false); + }); + + it("returns false when neither jj nor git metadata is present", async () => { + const dir = await createTempDir("omp-jj-empty-"); + expect(await jj.isPureJjRepo(dir)).toBe(false); + }); + + it("flags a jj workspace nested inside an unrelated git checkout as pure", async () => { + const outer = await createTempDir("omp-jj-nested-outer-"); + await initGit(outer); + const inner = path.join(outer, "nested"); + await fs.mkdir(path.join(inner, ".jj", "repo", "store"), { recursive: true }); + // The inner directory is its own jj workspace; the surrounding git + // checkout would mutate state outside jj's model. + expect(await jj.isPureJjRepo(inner)).toBe(true); + }); + + it("treats a nested git checkout under an outer jj workspace as non-pure", async () => { + // `git.repo.root(inner)` returns the inner .git, so Git automation + // targets the nested checkout safely and never touches the surrounding + // jj tree — the inner git wins. + const outer = await createTempDir("omp-jj-nested-jj-outer-"); + await fs.mkdir(path.join(outer, ".jj", "repo", "store"), { recursive: true }); + const inner = path.join(outer, "vendor"); + await fs.mkdir(inner, { recursive: true }); + await initGit(inner); + expect(await jj.isPureJjRepo(inner)).toBe(false); + }); +});