diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index f6efac5963..3109087089 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/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, diff --git a/packages/gittensory-engine/src/miner/lint-guard.ts b/packages/gittensory-engine/src/miner/lint-guard.ts new file mode 100644 index 0000000000..5be9275f9e --- /dev/null +++ b/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, 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 { + 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 { + const cwd = options.cwd ?? process.cwd(); + const byPackage = new Map(); + 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 { + 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 }; +} diff --git a/test/unit/coding-agent-miner.test.ts b/test/unit/coding-agent-miner.test.ts index 37f51b3bfd..418ec23f70 100644 --- a/test/unit/coding-agent-miner.test.ts +++ b/test/unit/coding-agent-miner.test.ts @@ -3,6 +3,7 @@ import { ATTEMPT_LOG_EVENT_TYPES, CODING_AGENT_DRIVER_CONFIG_ENV, CODING_AGENT_DRIVER_NAMES, + classifyLintGuardPackage, codingAgentModeExecutes, createAttemptLogBuffer, createCodingAgentDriver, @@ -10,6 +11,8 @@ import { 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): { + 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 { + 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); + }); +});