Merge branch 'pr-92-review' into test/pr-92

2026-07-24 10:21:29 +02:00
parent 165928a543 dd435cb1b0
commit 63b85fc246
16 changed files with 1055 additions and 73 deletions
@@ -0,0 +1,111 @@
+import { execFileSync } from "node:child_process";
+import { mkdtempSync, rmSync, writeFileSync, renameSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterAll, describe, expect, it } from "vitest";
+import { gitDiff, gitStatus } from "./gitService.js";
+
+// Isolate from any global/system git config and force a deterministic identity;
+// `protocol.file.allow` is required for `submodule add` from a local path.
+const GIT_FLAGS = ["-c", "user.name=Test", "-c", "[email protected]", "-c", "protocol.file.allow=always", "-c", "commit.gpgsign=false"];
+const GIT_ENV = { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null", GIT_TERMINAL_PROMPT: "0" };
+
+const created: string[] = [];
+afterAll(() => { for (const dir of created) rmSync(dir, { recursive: true, force: true }); });
+
+function git(cwd: string, args: string[]): string {
+  return execFileSync("git", [...GIT_FLAGS, ...args], { cwd, encoding: "utf8", env: GIT_ENV });
+}
+
+/** Superproject at `dir` with a submodule `HARL` recorded at commit `c2`; the
+ * submodule origin has two commits `c1` (a.txt=v1) then `c2` (a.txt=v2). */
+function createFixture(): { dir: string; c1: string; c2: string } {
+  const base = mkdtempSync(join(tmpdir(), "pi-web-sub-"));
+  created.push(base);
+  const origin = join(base, "origin");
+  const sup = join(base, "sup");
+
+  git(base, ["init", "-b", "main", origin]);
+  writeFileSync(join(origin, "a.txt"), "v1\n");
+  git(origin, ["add", "-A"]);
+  git(origin, ["commit", "-m", "c1"]);
+  const c1 = git(origin, ["rev-parse", "HEAD"]).trim();
+  writeFileSync(join(origin, "a.txt"), "v2\n");
+  git(origin, ["add", "-A"]);
+  git(origin, ["commit", "-m", "c2"]);
+  const c2 = git(origin, ["rev-parse", "HEAD"]).trim();
+
+  git(base, ["init", "-b", "main", sup]);
+  git(sup, ["submodule", "add", origin, "HARL"]);
+  writeFileSync(join(sup, "root.txt"), "root\n");
+  git(sup, ["add", "-A"]);
+  git(sup, ["commit", "-m", "init"]);
+  return { dir: sup, c1, c2 };
+}
+
+describe("gitStatus with submodules", () => {
+  it("surfaces a moved commit pointer with short SHAs and no inner files", async () => {
+    const { dir, c1, c2 } = createFixture();
+    git(join(dir, "HARL"), ["checkout", c1]); // move the pointer, leave the tree clean
+
+    const status = await gitStatus(dir);
+    expect(status.submodules).toContain("HARL");
+    const pointer = status.files.find((file) => file.path === "HARL");
+    expect(pointer?.submoduleFromCommit).toBe(c2.slice(0, 7));
+    expect(pointer?.submoduleToCommit).toBe(c1.slice(0, 7));
+    expect(status.files.some((file) => file.path.startsWith("HARL/"))).toBe(false);
+  });
+
+  it("lists modified and untracked inner files and omits the pointer when the commit is unchanged", async () => {
+    const { dir } = createFixture();
+    writeFileSync(join(dir, "HARL", "a.txt"), "v2\nchanged\n");
+    writeFileSync(join(dir, "HARL", "new.txt"), "brand-new\n");
+
+    const status = await gitStatus(dir);
+    expect(status.submodules).toContain("HARL");
+    expect(status.files.find((file) => file.path === "HARL")).toBeUndefined();
+    const inner = status.files.filter((file) => file.path.startsWith("HARL/")).map((file) => file.path);
+    expect(inner).toContain("HARL/a.txt");
+    expect(inner).toContain("HARL/new.txt");
+  });
+
+  it("skips inner recursion without throwing when the submodule repo is unreadable", async () => {
+    const { dir } = createFixture();
+    writeFileSync(join(dir, "HARL", "new.txt"), "brand-new\n"); // untracked → would trigger recursion
+    renameSync(join(dir, "HARL", ".git"), join(dir, "HARL", ".git.bak")); // break the inner repo
+
+    const status = await gitStatus(dir);
+    expect(status.isGitRepo).toBe(true);
+    expect(status.files.some((file) => file.path.startsWith("HARL/"))).toBe(false);
+  });
+});
+
+describe("gitDiff routing into submodules", () => {
+  it("returns real content for a tracked file inside the submodule", async () => {
+    const { dir } = createFixture();
+    writeFileSync(join(dir, "HARL", "a.txt"), "v2\nchanged\n");
+
+    const diff = await gitDiff(dir, { path: "HARL/a.txt" });
+    expect(diff.path).toBe("HARL/a.txt");
+    expect(diff.diff).toContain("@@");
+    expect(diff.diff).toContain("changed");
+  });
+
+  it("produces an untracked-file diff inside the submodule via --no-index", async () => {
+    const { dir } = createFixture();
+    writeFileSync(join(dir, "HARL", "new.txt"), "brand-new\n");
+
+    const diff = await gitDiff(dir, { path: "HARL/new.txt" });
+    expect(diff.path).toBe("HARL/new.txt");
+    expect(diff.diff).toContain("brand-new");
+  });
+
+  it("diffs the submodule path itself against the superproject pointer", async () => {
+    const { dir, c1 } = createFixture();
+    git(join(dir, "HARL"), ["checkout", c1]);
+
+    const diff = await gitDiff(dir, { path: "HARL" });
+    expect(diff.path).toBe("HARL");
+    expect(diff.diff).toContain("Subproject commit");
+  });
+});
@@ -1,15 +1,103 @@
 import { createHash } from "node:crypto";
 import { spawn } from "node:child_process";
+import { join } from "node:path";
 import type { GitDiffResponse, GitFileState, GitStatusFile, GitStatusResponse } from "../../shared/apiTypes.js";
 import { normalizeRelativePath } from "../workspaces/pathSafety.js";
 import { sanitizedGitEnv } from "./gitEnv.js";

 const MAX_OUTPUT = 2 * 1024 * 1024;

+/**
+ * A submodule row parsed from the superproject status. `git status` reports a
+ * submodule as a single path with an `S<c><m><u>` flag field (commit changed /
+ * modified tracked content / untracked content) but never lists the files that
+ * changed inside it, so we recurse in `expandSubmodules`.
+ */
+interface SubmoduleRecord {
+  path: string;
+  index: GitFileState;
+  workingTree: GitFileState;
+  commitChanged: boolean;
+  hasModifiedContent: boolean;
+  hasUntrackedContent: boolean;
+  headOid: string;
+  indexOid: string;
+}
+
+interface ParsedStatus {
+  isGitRepo: true;
+  branch?: string;
+  upstream?: string;
+  ahead?: number;
+  behind?: number;
+  files: GitStatusFile[];
+  submodules: SubmoduleRecord[];
+}
+
 export async function gitStatus(cwd: string): Promise<GitStatusResponse> {
  const result = await runGit(cwd, ["status", "--porcelain=v2", "--branch", "--untracked-files=all", "-z"]);
-  if (result.code !== 0) return { isGitRepo: false, hash: hash(result.stdout + result.stderr), files: [] };
-  return parseStatus(result.stdout);
+  if (result.code !== 0) return { isGitRepo: false, hash: hash(result.stdout + result.stderr), files: [], submodules: [] };
+  const parsed = parseStatus(result.stdout, { deferSubmodules: true });
+  return expandSubmodules(cwd, parsed, result.stdout);
+}
+
+/**
+ * Merge each dirty submodule's own changes into the flat file list. A moved
+ * commit pointer becomes a single entry keyed by the submodule path (carrying
+ * the short SHAs for display); modified/untracked content is listed as regular
+ * entries under `<submodule>/<inner path>`. A plain `-dirty` pointer (commit
+ * unchanged) is intentionally not surfaced as a pointer entry.
+ */
+async function expandSubmodules(cwd: string, parsed: ParsedStatus, topRaw: string): Promise<GitStatusResponse> {
+  const files: GitStatusFile[] = [...parsed.files];
+  const submodulePaths: string[] = [];
+  let extraForHash = "";
+
+  for (const sub of parsed.submodules) {
+    submodulePaths.push(sub.path);
+    if (sub.commitChanged) {
+      files.push({
+        path: sub.path,
+        index: sub.index,
+        workingTree: sub.workingTree,
+        submoduleFromCommit: short(sub.headOid),
+        submoduleToCommit: short(await resolveSubmoduleToCommit(cwd, sub)),
+      });
+    }
+    if (sub.hasModifiedContent || sub.hasUntrackedContent) {
+      const inner = await runGit(join(cwd, sub.path), ["status", "--porcelain=v2", "--untracked-files=all", "-z"]);
+      if (inner.code !== 0) continue; // uninitialized / unreadable submodule: skip silently
+      extraForHash += `\0${sub.path}\0${inner.stdout}`;
+      const innerFiles = parseStatus(inner.stdout, { deferSubmodules: false }).files;
+      for (const file of innerFiles) {
+        files.push({
+          ...file,
+          path: `${sub.path}/${file.path}`,
+          ...(file.oldPath === undefined ? {} : { oldPath: `${sub.path}/${file.oldPath}` }),
+        });
+      }
+    }
+  }
+
+  return {
+    isGitRepo: true,
+    hash: hash(topRaw + extraForHash),
+    ...(parsed.branch === undefined ? {} : { branch: parsed.branch }),
+    ...(parsed.upstream === undefined ? {} : { upstream: parsed.upstream }),
+    ...(parsed.ahead === undefined ? {} : { ahead: parsed.ahead }),
+    ...(parsed.behind === undefined ? {} : { behind: parsed.behind }),
+    files,
+    submodules: submodulePaths,
+  };
+}
+
+async function resolveSubmoduleToCommit(cwd: string, sub: SubmoduleRecord): Promise<string> {
+  // Staged pointer moves already expose the new commit as the index OID; an
+  // unstaged move only records the old OID, so read the submodule's HEAD.
+  if (sub.indexOid !== sub.headOid) return sub.indexOid;
+  const head = await runGit(join(cwd, sub.path), ["rev-parse", "HEAD"]);
+  const resolved = head.stdout.trim();
+  return head.code === 0 && resolved !== "" ? resolved : sub.indexOid;
 }

 export async function gitDiff(cwd: string, options: { path?: string; staged?: boolean }): Promise<GitDiffResponse> {
@@ -17,6 +105,11 @@ export async function gitDiff(cwd: string, options: { path?: string; staged?: bo
  let path: string | undefined;
  if (options.path !== undefined && options.path !== "") path = normalizeRelativePath(options.path);

+  if (path !== undefined) {
+    const owner = await submoduleForPath(cwd, path);
+    if (owner !== undefined) return submoduleDiff(cwd, owner, path, staged);
+  }
+
  const args = ["diff", "--no-ext-diff", "--color=never"];
  if (staged) args.push("--cached");
  if (path !== undefined) args.push("--", path);
@@ -31,14 +124,64 @@ export async function gitDiff(cwd: string, options: { path?: string; staged?: bo
  return { ...(path === undefined ? {} : { path }), staged, hash: hash(result.stdout), diff: result.stdout, truncated: result.truncated };
 }

+/**
+ * Run the diff inside the owning submodule's working tree, since `git diff` at
+ * the superproject root never shows content changes below a submodule boundary.
+ * The response path stays the full superproject-relative path so the viewer and
+ * the selected row line up.
+ */
+async function submoduleDiff(cwd: string, owner: string, path: string, staged: boolean): Promise<GitDiffResponse> {
+  const subCwd = join(cwd, owner);
+  const rel = normalizeRelativePath(path.slice(owner.length + 1));
+
+  const args = ["diff", "--no-ext-diff", "--color=never"];
+  if (staged) args.push("--cached");
+  args.push("--", rel);
+
+  const result = await runGit(subCwd, args);
+  if (result.code !== 0) throw new Error(result.stderr.trim() || "git diff failed");
+  if (!staged && result.stdout === "" && await isUntracked(subCwd, rel)) {
+    const untracked = await runGit(subCwd, ["diff", "--no-ext-diff", "--color=never", "--no-index", "/dev/null", "--", rel]);
+    if (untracked.code !== 0 && untracked.code !== 1) throw new Error(untracked.stderr.trim() || "git diff failed");
+    return { path, staged, hash: hash(untracked.stdout), diff: untracked.stdout, truncated: untracked.truncated };
+  }
+  return { path, staged, hash: hash(result.stdout), diff: result.stdout, truncated: result.truncated };
+}
+
 async function isUntracked(cwd: string, path: string): Promise<boolean> {
  const result = await runGit(cwd, ["ls-files", "--others", "--exclude-standard", "-z", "--", path]);
  return result.code === 0 && result.stdout.split("\0").includes(path);
 }

-function parseStatus(raw: string): GitStatusResponse {
+/** Configured direct-submodule paths (depth 1), read from `.gitmodules`. */
+async function submodulePaths(cwd: string): Promise<string[]> {
+  const result = await runGit(cwd, ["config", "--file", ".gitmodules", "--get-regexp", "^submodule\\..+\\.path$"]);
+  if (result.code !== 0) return [];
+  const paths: string[] = [];
+  for (const line of result.stdout.split("\n")) {
+    const trimmed = line.trim();
+    if (trimmed === "") continue;
+    const spaceAt = trimmed.indexOf(" ");
+    if (spaceAt === -1) continue;
+    paths.push(trimmed.slice(spaceAt + 1));
+  }
+  return paths;
+}
+
+/** The submodule that strictly contains `path`, if any (longest match wins). */
+async function submoduleForPath(cwd: string, path: string): Promise<string | undefined> {
+  const subs = await submodulePaths(cwd);
+  let best: string | undefined;
+  for (const sub of subs) {
+    if (sub !== "" && path.startsWith(`${sub}/`) && (best === undefined || sub.length > best.length)) best = sub;
+  }
+  return best;
+}
+
+function parseStatus(raw: string, options: { deferSubmodules: boolean }): ParsedStatus {
  const records = raw.split("\0").filter((record) => record !== "");
  const files: GitStatusFile[] = [];
+  const submodules: SubmoduleRecord[] = [];
  let branch: string | undefined;
  let upstream: string | undefined;
  let ahead: number | undefined;
@@ -56,7 +199,22 @@ function parseStatus(raw: string): GitStatusResponse {
    else if (record.startsWith("! ")) files.push({ path: record.slice(2), index: "ignored", workingTree: "ignored" });
    else if (record.startsWith("1 ")) {
      const parts = record.split(" ");
-      files.push({ path: parts.slice(8).join(" "), index: stateFor(parts[1]?.[0]), workingTree: stateFor(parts[1]?.[1]) });
+      const sub = parts[2];
+      const path = parts.slice(8).join(" ");
+      if (options.deferSubmodules && sub?.startsWith("S") === true) {
+        submodules.push({
+          path,
+          index: stateFor(parts[1]?.[0]),
+          workingTree: stateFor(parts[1]?.[1]),
+          commitChanged: sub[1] === "C",
+          hasModifiedContent: sub[2] === "M",
+          hasUntrackedContent: sub[3] === "U",
+          headOid: parts[6] ?? "",
+          indexOid: parts[7] ?? "",
+        });
+      } else {
+        files.push({ path, index: stateFor(parts[1]?.[0]), workingTree: stateFor(parts[1]?.[1]) });
+      }
    } else if (record.startsWith("2 ")) {
      const parts = record.split(" ");
      const path = parts.slice(9).join(" ");
@@ -69,7 +227,7 @@ function parseStatus(raw: string): GitStatusResponse {
    }
  }

-  return { isGitRepo: true, hash: hash(raw), ...(branch === undefined ? {} : { branch }), ...(upstream === undefined ? {} : { upstream }), ...(ahead === undefined ? {} : { ahead }), ...(behind === undefined ? {} : { behind }), files };
+  return { isGitRepo: true, ...(branch === undefined ? {} : { branch }), ...(upstream === undefined ? {} : { upstream }), ...(ahead === undefined ? {} : { ahead }), ...(behind === undefined ? {} : { behind }), files, submodules };
 }

 function stateFor(code: string | undefined): GitFileState {
@@ -90,6 +248,10 @@ function normalizeBranch(value: string): string | undefined {
  return value === "(detached)" ? undefined : value;
 }

+function short(oid: string): string {
+  return oid.slice(0, 7);
+}
+
 function hash(value: string): string {
  return createHash("sha1").update(value).digest("hex");
 }