feat(miner-hands): lint-guarded edit wrapper for coding-agent drivers (#4276)

packages/gittensory-engine/src/index.ts

@@ -189,6 +189,17 @@ export {
   invokeCodingAgentDriver,
   type AttemptLogSink,
 } from "./miner/coding-agent-invoke.js";
+export {
+  classifyLintGuardPackage,
+  guardChangedFiles,
+  guardCodingAgentDriverResult,
+  type LintGuardCheckResult,
+  type LintGuardedDriverResult,
+  type LintGuardOptions,
+  type LintGuardPackage,
+  type LintGuardResult,
+  type LintGuardSpawnFn,
+} from "./miner/lint-guard.js";
 export {
   CODING_AGENT_DRIVER_CONFIG_ENV,
   CODING_AGENT_DRIVER_NAMES,

packages/gittensory-engine/src/miner/lint-guard.ts

@@ -0,0 +1,133 @@
+// Lint-guarded edit wrapper for coding-agent drivers (#4276). This repo has no repo-wide ESLint (or other
+// linter) at the src/packages/* level -- the only ESLint config is apps/gittensory-ui/eslint.config.js,
+// wired up solely through `ui:lint`. The gate everything else runs is `typecheck` (`tsc --noEmit`) plus each
+// package/* under `packages/*` having its OWN build-time check: `gittensory-engine` runs its own `tsc -p
+// tsconfig.json`; `gittensory-miner`/`gittensory-mcp` ship plain JS with `node --check` per shipped `.js`
+// file (their non-`.js` files, like hand-written `.d.ts` declarations, are covered by the root typecheck
+// instead, same as `src/`). "Lint-guarded" therefore means: after a coding-agent driver edits files, run the
+// EXISTING check appropriate to each changed file's package -- never introduce a new linter.
+//
+// Implementations here MAY perform real IO (spawn `tsc`/`node`/`npm`), same allowance as `CodingAgentDriver`
+// itself (coding-agent-driver.ts) -- the spawn function is injected (mirrors `SpawnFn` in
+// `src/selfhost/ai.ts`), so this module stays synchronous-IO-free in tests.
+import type { CodingAgentDriverResult } from "./coding-agent-driver.js";
+
+/** Which existing check governs a changed file. `root` covers `src/**` and any non-`.js` file under
+ *  `packages/gittensory-miner`/`packages/gittensory-mcp` (e.g. a hand-written `.d.ts`), since those are
+ *  type-checked by the root `tsc --noEmit`, not `node --check`. */
+export type LintGuardPackage = "ui" | "engine" | "miner-js" | "mcp-js" | "root";
+
+const MINER_JS_EXTENSION = /\.(js|mjs|cjs)$/;
+
+/** Classify a changed file path (POSIX or Windows separators) into the package whose existing check governs
+ *  it. Pure path matching -- no filesystem access. */
+export function classifyLintGuardPackage(path: string): LintGuardPackage {
+  const normalized = path.replace(/\\/g, "/").replace(/^\.\//, "");
+  if (normalized.startsWith("apps/gittensory-ui/")) return "ui";
+  if (normalized.startsWith("packages/gittensory-engine/")) return "engine";
+  if (normalized.startsWith("packages/gittensory-miner/") && MINER_JS_EXTENSION.test(normalized)) return "miner-js";
+  if (normalized.startsWith("packages/gittensory-mcp/") && MINER_JS_EXTENSION.test(normalized)) return "mcp-js";
+  return "root";
+}
+
+export type LintGuardSpawnResult = { ok: boolean; output: string };
+
+/** Injected process runner -- real IO lives here, not in `guardChangedFiles`, so tests never spawn a real
+ *  subprocess. `ok` is derived from the exit code by the caller of this function, not by this type. */
+export type LintGuardSpawnFn = (
+  cmd: string,
+  args: readonly string[],
+  opts: { cwd: string },
+) => Promise<{ code: number | null; output: string }>;
+
+export type LintGuardCheckResult = {
+  package: LintGuardPackage;
+  file: string;
+  command: string;
+  ok: boolean;
+  output: string;
+};
+
+/** Structured result -- never a thrown exception -- so a caller (the self-review loop, #2333) can
+ *  distinguish "the edit doesn't typecheck" (a `checks` entry with `ok: false`) from "the coding agent
+ *  itself failed" (a separate concern entirely, see {@link guardCodingAgentDriverResult}). */
+export type LintGuardResult = {
+  ok: boolean;
+  checks: readonly LintGuardCheckResult[];
+};
+
+export type LintGuardOptions = {
+  spawn: LintGuardSpawnFn;
+  /** Repo root the checks run from. Default: `process.cwd()`. */
+  cwd?: string | undefined;
+};
+
+const PACKAGE_COMMAND: Readonly<Record<Exclude<LintGuardPackage, "miner-js" | "mcp-js">, readonly string[]>> = Object.freeze({
+  root: Object.freeze(["npm", "run", "typecheck"]),
+  engine: Object.freeze(["npm", "run", "build", "--workspace", "@jsonbored/gittensory-engine"]),
+  ui: Object.freeze(["npm", "run", "ui:typecheck"]),
+});
+
+async function runPackageCheck(
+  pkg: LintGuardPackage,
+  files: readonly string[],
+  spawn: LintGuardSpawnFn,
+  cwd: string,
+): Promise<LintGuardCheckResult[]> {
+  if (pkg === "miner-js" || pkg === "mcp-js") {
+    // node --check is inherently per-file, unlike the whole-package tsc/ui:lint commands below.
+    const results: LintGuardCheckResult[] = [];
+    for (const file of files) {
+      const { code, output } = await spawn("node", ["--check", file], { cwd });
+      results.push({ package: pkg, file, command: `node --check ${file}`, ok: code === 0, output });
+    }
+    return results;
+  }
+  const command = PACKAGE_COMMAND[pkg];
+  const { code, output } = await spawn(command[0]!, command.slice(1), { cwd });
+  return [{ package: pkg, file: files.join(", "), command: command.join(" "), ok: code === 0, output }];
+}
+
+/**
+ * Run the existing check for every package a changed file belongs to. One check per package group, not one
+ * per file (except `node --check`, which is inherently per-file) -- `tsc`/`ui:typecheck` validate a whole
+ * package at once, so re-running them per file would be redundant work, not extra coverage.
+ */
+export async function guardChangedFiles(
+  changedFiles: readonly string[],
+  options: LintGuardOptions,
+): Promise<LintGuardResult> {
+  const cwd = options.cwd ?? process.cwd();
+  const byPackage = new Map<LintGuardPackage, string[]>();
+  for (const file of changedFiles) {
+    const pkg = classifyLintGuardPackage(file);
+    const list = byPackage.get(pkg);
+    if (list) list.push(file);
+    else byPackage.set(pkg, [file]);
+  }
+
+  const checks: LintGuardCheckResult[] = [];
+  for (const [pkg, files] of byPackage) {
+    checks.push(...(await runPackageCheck(pkg, files, options.spawn, cwd)));
+  }
+
+  return { ok: checks.every((check) => check.ok), checks };
+}
+
+export type LintGuardedDriverResult = CodingAgentDriverResult & { lintGuard: LintGuardResult };
+
+/**
+ * Decorate a `CodingAgentDriver` result with its lint-guard verdict. Skips the guard entirely (an empty,
+ * passing `lintGuard`) when the driver itself failed or reported no changed files -- there is nothing to
+ * check, and running checks against an untouched tree would only produce a misleading unrelated result.
+ */
+export async function guardCodingAgentDriverResult(
+  result: CodingAgentDriverResult,
+  options: LintGuardOptions,
+): Promise<LintGuardedDriverResult> {
+  if (!result.ok || result.changedFiles.length === 0) {
+    return { ...result, lintGuard: { ok: true, checks: [] } };
+  }
+  const lintGuard = await guardChangedFiles(result.changedFiles, options);
+  return { ...result, ok: result.ok && lintGuard.ok, lintGuard };
+}

test/unit/coding-agent-miner.test.ts

@@ -3,13 +3,16 @@ import {
   ATTEMPT_LOG_EVENT_TYPES,
   CODING_AGENT_DRIVER_CONFIG_ENV,
   CODING_AGENT_DRIVER_NAMES,
+  classifyLintGuardPackage,
   codingAgentModeExecutes,
   createAttemptLogBuffer,
   createCodingAgentDriver,
   createFakeCodingAgentDriver,
   createFakeCodingAgentDriverForFactory,
   createNoopCodingAgentDriver,
   formatAttemptLogJsonl,
+  guardChangedFiles,
+  guardCodingAgentDriverResult,
   invokeCodingAgentDriver,
   isConfiguredCodingAgentDriver,
   isGlobalMinerCodingAgentPause,
@@ -18,7 +21,9 @@ import {
   resolveCodingAgentModeFromConfig,
   resolveConfiguredCodingAgentDriverNames,
   runCodingAgentAttempt,
+  type CodingAgentDriverResult,
   type CodingAgentDriverTask,
+  type LintGuardSpawnFn,
 } from "../../packages/gittensory-engine/src/index";
 
 const task: CodingAgentDriverTask = {
@@ -359,3 +364,173 @@ describe("coding-agent driver factory (#4289)", () => {
     expect(fake.lastTask).toBeNull();
   });
 });
+
+describe("lint-guarded edit wrapper (#4276)", () => {
+  it("classifyLintGuardPackage routes each file to the check that actually governs it", () => {
+    expect(classifyLintGuardPackage("apps/gittensory-ui/src/App.tsx")).toBe("ui");
+    expect(classifyLintGuardPackage("packages/gittensory-engine/src/miner/lint-guard.ts")).toBe("engine");
+    expect(classifyLintGuardPackage("packages/gittensory-miner/lib/cli.js")).toBe("miner-js");
+    expect(classifyLintGuardPackage("packages/gittensory-mcp/bin/gittensory-mcp.js")).toBe("mcp-js");
+    expect(classifyLintGuardPackage("src/review/ops-wire.ts")).toBe("root");
+    // A hand-written .d.ts under miner/mcp is type-checked by the root tsc, not node --check.
+    expect(classifyLintGuardPackage("packages/gittensory-miner/lib/cli.d.ts")).toBe("root");
+    expect(classifyLintGuardPackage("packages/gittensory-mcp/lib/local-branch.d.ts")).toBe("root");
+  });
+
+  it("classifyLintGuardPackage normalizes Windows-style backslash paths and a leading ./", () => {
+    expect(classifyLintGuardPackage("packages\\gittensory-miner\\lib\\cli.js")).toBe("miner-js");
+    expect(classifyLintGuardPackage("./src/review/ops-wire.ts")).toBe("root");
+  });
+
+  function recordingSpawn(outcomes: Record<string, { code: number; output: string }>): {
+    spawn: LintGuardSpawnFn;
+    calls: Array<{ cmd: string; args: readonly string[] }>;
+  } {
+    const calls: Array<{ cmd: string; args: readonly string[] }> = [];
+    const spawn: LintGuardSpawnFn = async (cmd, args) => {
+      calls.push({ cmd, args });
+      const key = [cmd, ...args].join(" ");
+      const outcome = outcomes[key] ?? { code: 0, output: "" };
+      return outcome;
+    };
+    return { spawn, calls };
+  }
+
+  it("guardChangedFiles reports a root typecheck failure as a structured (not thrown) result", async () => {
+    const { spawn } = recordingSpawn({
+      "npm run typecheck": { code: 2, output: "src/review/ops-wire.ts(10,3): error TS2322" },
+    });
+    const result = await guardChangedFiles(["src/review/ops-wire.ts"], { spawn, cwd: "/repo" });
+    expect(result.ok).toBe(false);
+    expect(result.checks).toEqual([
+      { package: "root", file: "src/review/ops-wire.ts", command: "npm run typecheck", ok: false, output: "src/review/ops-wire.ts(10,3): error TS2322" },
+    ]);
+  });
+
+  it("guardChangedFiles reports a node --check syntax error in a gittensory-miner JS file", async () => {
+    const { spawn } = recordingSpawn({
+      "node --check packages/gittensory-miner/lib/cli.js": {
+        code: 1,
+        output: "SyntaxError: Unexpected token '}'",
+      },
+    });
+    const result = await guardChangedFiles(["packages/gittensory-miner/lib/cli.js"], { spawn, cwd: "/repo" });
+    expect(result.ok).toBe(false);
+    expect(result.checks).toEqual([
+      {
+        package: "miner-js",
+        file: "packages/gittensory-miner/lib/cli.js",
+        command: "node --check packages/gittensory-miner/lib/cli.js",
+        ok: false,
+        output: "SyntaxError: Unexpected token '}'",
+      },
+    ]);
+  });
+
+  it("guardChangedFiles reports ok: true for a fully clean change", async () => {
+    const { spawn } = recordingSpawn({});
+    const result = await guardChangedFiles(["src/review/ops-wire.ts"], { spawn, cwd: "/repo" });
+    expect(result.ok).toBe(true);
+    expect(result.checks).toEqual([
+      { package: "root", file: "src/review/ops-wire.ts", command: "npm run typecheck", ok: true, output: "" },
+    ]);
+  });
+
+  it("guardChangedFiles checks a changeset spanning multiple packages against each file's OWN rule, once per package", async () => {
+    const { spawn, calls } = recordingSpawn({
+      "npm run typecheck": { code: 0, output: "" },
+      "npm run build --workspace @jsonbored/gittensory-engine": { code: 1, output: "engine build failed" },
+      "node --check packages/gittensory-miner/lib/cli.js": { code: 0, output: "" },
+      "npm run ui:typecheck": { code: 0, output: "" },
+    });
+    const result = await guardChangedFiles(
+      [
+        "src/review/ops-wire.ts",
+        "packages/gittensory-engine/src/miner/lint-guard.ts",
+        "packages/gittensory-miner/lib/cli.js",
+        "apps/gittensory-ui/src/App.tsx",
+      ],
+      { spawn, cwd: "/repo" },
+    );
+    expect(result.ok).toBe(false);
+    // Exactly one check per package group -- the root/ui/engine commands are NOT re-run per file.
+    expect(result.checks).toHaveLength(4);
+    expect(result.checks.find((check) => check.package === "root")?.ok).toBe(true);
+    expect(result.checks.find((check) => check.package === "engine")?.ok).toBe(false);
+    expect(result.checks.find((check) => check.package === "miner-js")?.ok).toBe(true);
+    expect(result.checks.find((check) => check.package === "ui")?.ok).toBe(true);
+    expect(calls.filter((call) => call.cmd === "npm" && call.args.join(" ") === "run typecheck")).toHaveLength(1);
+  });
+
+  it("guardChangedFiles groups multiple files in the same package into a single check", async () => {
+    const { spawn } = recordingSpawn({
+      "node --check packages/gittensory-miner/lib/a.js": { code: 0, output: "" },
+      "node --check packages/gittensory-miner/lib/b.js": { code: 0, output: "" },
+    });
+    const result = await guardChangedFiles(
+      ["packages/gittensory-miner/lib/a.js", "packages/gittensory-miner/lib/b.js"],
+      { spawn, cwd: "/repo" },
+    );
+    expect(result.ok).toBe(true);
+    expect(result.checks).toHaveLength(2);
+    expect(result.checks.map((check) => check.file)).toEqual([
+      "packages/gittensory-miner/lib/a.js",
+      "packages/gittensory-miner/lib/b.js",
+    ]);
+  });
+
+  it("guardChangedFiles defaults cwd to process.cwd() when omitted", async () => {
+    const seenCwds: Array<{ cwd: string }> = [];
+    const spawn: LintGuardSpawnFn = async (_cmd, _args, opts) => {
+      seenCwds.push(opts);
+      return { code: 0, output: "" };
+    };
+    await guardChangedFiles(["src/review/ops-wire.ts"], { spawn });
+    expect(seenCwds).toEqual([{ cwd: process.cwd() }]);
+  });
+
+  function driverResult(overrides: Partial<CodingAgentDriverResult> = {}): CodingAgentDriverResult {
+    return { ok: true, changedFiles: [], summary: "ok", turnsUsed: 1, ...overrides };
+  }
+
+  it("guardCodingAgentDriverResult skips the guard when the driver itself failed", async () => {
+    const { spawn, calls } = recordingSpawn({});
+    const decorated = await guardCodingAgentDriverResult(driverResult({ ok: false, error: "boom" }), { spawn });
+    expect(decorated.ok).toBe(false);
+    expect(decorated.lintGuard).toEqual({ ok: true, checks: [] });
+    expect(calls).toHaveLength(0);
+  });
+
+  it("guardCodingAgentDriverResult skips the guard when no files changed", async () => {
+    const { spawn, calls } = recordingSpawn({});
+    const decorated = await guardCodingAgentDriverResult(driverResult({ changedFiles: [] }), { spawn });
+    expect(decorated.ok).toBe(true);
+    expect(decorated.lintGuard).toEqual({ ok: true, checks: [] });
+    expect(calls).toHaveLength(0);
+  });
+
+  it("guardCodingAgentDriverResult runs the guard and downgrades ok when a check fails", async () => {
+    const { spawn } = recordingSpawn({
+      "npm run typecheck": { code: 2, output: "type error" },
+    });
+    const decorated = await guardCodingAgentDriverResult(
+      driverResult({ changedFiles: ["src/review/ops-wire.ts"] }),
+      { spawn },
+    );
+    expect(decorated.ok).toBe(false);
+    expect(decorated.lintGuard.ok).toBe(false);
+    expect(decorated.lintGuard.checks).toHaveLength(1);
+  });
+
+  it("guardCodingAgentDriverResult keeps ok: true and preserves driver fields when every check passes", async () => {
+    const { spawn } = recordingSpawn({});
+    const decorated = await guardCodingAgentDriverResult(
+      driverResult({ changedFiles: ["src/review/ops-wire.ts"], summary: "did the thing", turnsUsed: 3 }),
+      { spawn },
+    );
+    expect(decorated.ok).toBe(true);
+    expect(decorated.summary).toBe("did the thing");
+    expect(decorated.turnsUsed).toBe(3);
+    expect(decorated.lintGuard.ok).toBe(true);
+  });
+});