diff --git a/src/vcs-provider.ts b/src/vcs-provider.ts index 51a6e71..bcdd73d 100644 --- a/src/vcs-provider.ts +++ b/src/vcs-provider.ts @@ -303,50 +303,14 @@ export class SvnProvider implements VcsProvider { combinedDiff += addDiffRes.stdout.trim() + "\n"; } } else { - // Unversioned file: generate diff manually - if (fs.existsSync(fullPath)) { - const stats = fs.statSync(fullPath); - if (stats.isDirectory()) { - // Skip directories - continue; - } - if (stats.isFile()) { - try { - // Try to read as text, but handle binary - const content = fs.readFileSync(fullPath); - // Check if binary - const isBinary = content.includes(0); - - if (isBinary) { - combinedDiff += `Index: ${entryPath}\n`; - combinedDiff += `===================================================================\n`; - combinedDiff += `Cannot display: file marked as a binary type.\n`; - combinedDiff += `svn:mime-type = application/octet-stream\n`; - } else { - const textContent = content.toString("utf8"); - const lines = textContent.split("\n"); - const hasTrailingNewline = textContent.endsWith("\n"); - const lineCount = hasTrailingNewline ? lines.length - 1 : lines.length; - - combinedDiff += `Index: ${entryPath}\n`; - combinedDiff += `===================================================================\n`; - combinedDiff += `--- ${entryPath}\t(nonexistent)\n`; - combinedDiff += `+++ ${entryPath}\t(working copy)\n`; - combinedDiff += `@@ -0,0 +1,${lineCount} @@\n`; - for (let i = 0; i < lines.length; i++) { - if (i === lines.length - 1 && hasTrailingNewline && lines[i] === "") { - continue; - } - combinedDiff += `+${lines[i]}\n`; - } - if (!hasTrailingNewline && lineCount > 0) { - combinedDiff += "\\ No newline at end of file\n"; - } - } - } catch (err) { - throw new VcsError(`Failed to read unversioned file ${entryPath}: ${(err as Error).message}`); - } - } + // Unversioned node: generate diff manually + // Recursively collect regular (non-symlink) files; skip symlinks and nodes outside wc + const unversionedFiles = this.collectUnversionedFiles(fullPath, repoRoot); + // Sort stably so diff output is deterministic + unversionedFiles.sort(); + for (const absFile of unversionedFiles) { + const relFile = path.relative(repoRoot, absFile); + combinedDiff += this.generateUnversionedFileDiff(absFile, relFile); } } } else if (itemStatus === "missing" || itemStatus === "deleted") { @@ -496,6 +460,131 @@ export class SvnProvider implements VcsProvider { } } + /** + * Recursively collect regular (non-symlink) files under an unversioned directory. + * Uses lstat so directory symlinks are never followed. Only returns nodes that are + * within repoRoot and that are regular files (not symlinks). + */ + private collectUnversionedFiles(rootPath: string, repoRoot: string): string[] { + const result: string[] = []; + + // Skip .svn administration directory immediately + if (path.basename(rootPath) === ".svn") { + return result; + } + + let st: fs.Stats; + try { + st = fs.lstatSync(rootPath); + } catch (err) { + throw new VcsError(`Failed to lstat ${rootPath}: ${(err as Error).message}`); + } + + if (st.isSymbolicLink()) { + // Do not follow symlinks, do not traverse, do not include + return result; + } + + if (st.isFile()) { + const rel = path.relative(repoRoot, rootPath); + if (!rel.startsWith("..") && !path.isAbsolute(rel)) { + result.push(rootPath); + } + return result; + } + + if (st.isDirectory()) { + let entries: string[]; + try { + entries = fs.readdirSync(rootPath); + } catch (err) { + throw new VcsError(`Failed to readdir ${rootPath}: ${(err as Error).message}`); + } + + for (const entry of entries) { + const full = path.join(rootPath, entry); + const rel = path.relative(repoRoot, full); + if (rel.startsWith("..") || path.isAbsolute(rel)) continue; + + const nested = this.collectUnversionedFiles(full, repoRoot); + result.push(...nested); + } + } + + return result; + } + + /** + * Generate a review-evidence diff block for a single unversioned regular file. + * Always emits Index + header lines even for empty files. + */ + private generateUnversionedFileDiff(absFile: string, relPath: string): string { + let block = ""; + let fd: number; + try { + // open with O_NOFOLLOW to ensure we do not follow symlinks at read time + fd = fs.openSync(absFile, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW); + } catch (err) { + throw new VcsError(`Failed to open unversioned file ${relPath}: ${(err as Error).message}`); + } + + try { + const stats = fs.fstatSync(fd); + if (!stats.isFile()) { + throw new VcsError(`Failed to read unversioned file ${relPath}: not a regular file`); + } + const buffer = Buffer.alloc(stats.size); + let bytesRead = 0; + while (bytesRead < stats.size) { + const read = fs.readSync(fd, buffer, bytesRead, stats.size - bytesRead, null); + if (read === 0) break; + bytesRead += read; + } + const content = buffer.subarray(0, bytesRead); + const isBinary = content.includes(0); + + block += `Index: ${relPath}\n`; + block += `===================================================================\n`; + + if (isBinary) { + block += `Cannot display: file marked as a binary type.\n`; + block += `svn:mime-type = application/octet-stream\n`; + } else { + const textContent = content.toString("utf8"); + block += `--- ${relPath}\t(nonexistent)\n`; + block += `+++ ${relPath}\t(working copy)\n`; + + if (textContent.length === 0) { + // Empty file: hunk with zero lines + block += `@@ -0,0 +0,0 @@\n`; + } else { + const lines = textContent.split("\n"); + const hasTrailingNewline = textContent.endsWith("\n"); + const lineCount = hasTrailingNewline ? lines.length - 1 : lines.length; + block += `@@ -0,0 +1,${lineCount} @@\n`; + for (let i = 0; i < lines.length; i++) { + if (i === lines.length - 1 && hasTrailingNewline && lines[i] === "") { + continue; + } + block += `+${lines[i]}\n`; + } + if (!hasTrailingNewline && lineCount > 0) { + block += "\\ No newline at end of file\n"; + } + } + } + } catch (err) { + throw new VcsError(`Failed to read unversioned file ${relPath}: ${(err as Error).message}`); + } finally { + try { + fs.closeSync(fd); + } catch { + // ignore close error + } + } + return block; + } + /** Strict validation: reject mixed revisions, switched paths, externals, conflicts, etc. */ private validateStrictWorkingCopy(repoRoot: string): void { try { diff --git a/src/workflow-engine.ts b/src/workflow-engine.ts index 488ed75..d2b9dcb 100644 --- a/src/workflow-engine.ts +++ b/src/workflow-engine.ts @@ -278,7 +278,7 @@ export async function resolveVcsSelection( // Load config from every distinct candidate root const configs = new Map(); - for (const [vcsType, root] of Object.entries(candidates) as Array<[string, string | undefined]>) { + for (const [, root] of Object.entries(candidates) as Array<[string, string | undefined]>) { if (!root) continue; // Skip if we already loaded config from this root @@ -288,15 +288,23 @@ export async function resolveVcsSelection( const config = resolveWorkflowConfig(root, {}); configs.set(root, { vcs: config.vcs, root }); } catch (err) { - // Re-throw validation errors, tolerate missing config + // Re-throw validation/parse errors (WorkflowConfigError) — these are real problems if (err instanceof WorkflowConfigError) { throw new WorkflowEngineError(`Invalid agent-workflow.json at ${root}: ${(err as WorkflowConfigError).message}`); } - // Config doesn't exist - that's ok - configs.set(root, { root }); + // Re-throw file access errors unless the file simply doesn't exist (ENOENT). + // Permission denied, I/O errors, generic Error/TypeError, etc. must surface rather than be silently ignored. + const errCode = (err as any)?.code; + if (errCode === "ENOENT") { + // Config file doesn't exist — that's ok, treat as no project config + configs.set(root, { root }); + } else { + throw err; + } } } + // Extract project defaultVcs settings const projectDefaults = Array.from(configs.values()) .map(c => c.vcs) diff --git a/src/workflow-svn-extend.test.ts b/src/workflow-svn-extend.test.ts index e4cb8a0..d185982 100644 --- a/src/workflow-svn-extend.test.ts +++ b/src/workflow-svn-extend.test.ts @@ -3,11 +3,13 @@ * * Uses real svnadmin repositories and fake executors to test: * - VCS detection and ambiguity with real repos - * - SVN strict validation (mixed revisions, externals, conflicts, etc.) + * - VCS selection determinism: CLI override > project defaultVcs > single candidate + * - SVN strict validation (mixed revisions, switched paths, externals, conflicts, etc.) * - SVN diff generation (modifications, additions, properties, missing files) - * - Extension validation and immediate resume with session preservation - * - Extension lock/baseline/scope gates - * - Convergence check during extension + * - SVN diff: recursive unversioned directory, scheduled add/delete, ignored file + * - Extension audit: full argv recording, log file content, budget_extended event, lock ownership + * - Convergence rejection: state and history unchanged, lock released + * - Scope rejection rollback: state and history unchanged, lock released * - Backward compatibility with old Git states */ @@ -22,11 +24,22 @@ import { createVcsProvider, SvnProvider, GitProvider, + findVcsCandidates, } from "./vcs-provider.js"; import { resolveWorkflowConfig } from "./workflow-config.js"; -import { extendWorkflow, startWorkflow } from "./workflow-engine.js"; -import { readState, runDir, writeState, initWorkflowState, gitHead, releaseLock } from "./workflow-state.js"; -import type { WorkflowPlanV1, HostDecisionV1, ExtensionRecord } from "./workflow-types.js"; +import { extendWorkflow, resolveVcsSelection, startWorkflow } from "./workflow-engine.js"; +import { + readState, + runDir, + writeState, + initWorkflowState, + gitHead, + releaseLock, + acquireLock, + readLock, + eventsFilePath, +} from "./workflow-state.js"; +import type { WorkflowPlanV1, HostDecisionV1, ExtensionRecord, WorkflowEvent } from "./workflow-types.js"; function makeTmpDir(): string { return fs.mkdtempSync(path.join(os.tmpdir(), "agent-workflow-svn-test-")); @@ -40,12 +53,12 @@ function cleanupDir(dir: string): void { } } -function makeValidPlan(): WorkflowPlanV1 { +function makeValidPlan(scope?: string[]): WorkflowPlanV1 { return { version: "1", title: "Test plan", planMarkdown: "Do something.", - scope: ["src/"], + scope: scope ?? ["src/"], acceptanceCriteria: ["Tests pass"], verificationCommands: [["echo", "test"]], }; @@ -53,24 +66,28 @@ function makeValidPlan(): WorkflowPlanV1 { // Create a real SVN repository with svnadmin function createSvnRepo(repoDir: string, wcDir: string): void { - // Create repository execFileSync("svnadmin", ["create", repoDir], { stdio: "ignore" }); - - // Checkout working copy execFileSync("svn", ["checkout", `file://${repoDir}`, wcDir], { stdio: "ignore" }); - // Create initial structure fs.mkdirSync(path.join(wcDir, "src"), { recursive: true }); fs.writeFileSync(path.join(wcDir, "src", "test.txt"), "initial content\n"); - // Add and commit execFileSync("svn", ["add", "src"], { cwd: wcDir, stdio: "ignore" }); execFileSync("svn", ["commit", "-m", "Initial commit"], { cwd: wcDir, stdio: "ignore" }); - - // Update working copy to ensure it's at the latest revision execFileSync("svn", ["update"], { cwd: wcDir, stdio: "ignore" }); } +/** Read all events from events.jsonl */ +function readEvents(dir: string): WorkflowEvent[] { + const filePath = eventsFilePath(dir); + if (!fs.existsSync(filePath)) return []; + return fs + .readFileSync(filePath, "utf8") + .split("\n") + .filter(Boolean) + .map((line) => JSON.parse(line) as WorkflowEvent); +} + // --------------------------------------------------------------------------- // VCS Detection and Ambiguity // --------------------------------------------------------------------------- @@ -104,7 +121,6 @@ describe("VCS detection with real repositories", () => { const wcDir = path.join(tmpDir, "wc"); createSvnRepo(repoDir, wcDir); - // Create .git directory inside SVN working copy execFileSync("git", ["init"], { cwd: wcDir, stdio: "ignore" }); expect(() => detectVcs(wcDir)).toThrow(/Ambiguous VCS.*both Git and SVN/); @@ -154,6 +170,276 @@ describe("VCS precedence from nested directories", () => { }); }); +// --------------------------------------------------------------------------- +// VCS Selection: resolveVcsSelection determinism with dual-VCS fixture +// --------------------------------------------------------------------------- + +describe("resolveVcsSelection with real dual-VCS fixture", () => { + let tmpDir: string; + let gitRoot: string; + let svnRepoDir: string; + let svnWcDir: string; + let originalWorkflowDir: string | undefined; + + beforeEach(() => { + tmpDir = makeTmpDir(); + gitRoot = path.join(tmpDir, "git-repo"); + svnRepoDir = path.join(tmpDir, "svn-repo"); + svnWcDir = path.join(tmpDir, "svn-wc"); + + // Create independent Git repo + fs.mkdirSync(gitRoot, { recursive: true }); + execFileSync("git", ["init"], { cwd: gitRoot, stdio: "ignore" }); + execFileSync("git", ["config", "user.email", "test@test.com"], { cwd: gitRoot }); + execFileSync("git", ["config", "user.name", "Test"], { cwd: gitRoot }); + fs.writeFileSync(path.join(gitRoot, "README.md"), "git repo"); + execFileSync("git", ["add", "."], { cwd: gitRoot }); + execFileSync("git", ["commit", "-m", "init"], { cwd: gitRoot }); + + // Create independent SVN repo + WC + createSvnRepo(svnRepoDir, svnWcDir); + + originalWorkflowDir = process.env.AGENT_WORKFLOW_DIR; + process.env.AGENT_WORKFLOW_DIR = path.join(tmpDir, "workflows"); + }); + + afterEach(() => { + if (originalWorkflowDir === undefined) { + delete process.env.AGENT_WORKFLOW_DIR; + } else { + process.env.AGENT_WORKFLOW_DIR = originalWorkflowDir; + } + cleanupDir(tmpDir); + }); + + it("selects git when --vcs git is specified (CLI override)", async () => { + const result = await resolveVcsSelection(gitRoot, "git"); + expect(result.vcsKind).toBe("git"); + expect(result.repoRoot).toBe(gitRoot); + }); + + it("selects svn when --vcs svn is specified (CLI override)", async () => { + const result = await resolveVcsSelection(svnWcDir, "svn"); + expect(result.vcsKind).toBe("svn"); + expect(result.repoRoot).toBe(svnWcDir); + }); + + it("resolves from project defaultVcs when only one VCS is present (git)", async () => { + fs.writeFileSync( + path.join(gitRoot, "agent-workflow.json"), + JSON.stringify({ version: "1", defaultVcs: "git" }) + ); + const result = await resolveVcsSelection(gitRoot); + expect(result.vcsKind).toBe("git"); + expect(result.repoRoot).toBe(gitRoot); + }); + + it("resolves from project defaultVcs when only one VCS is present (svn)", async () => { + fs.writeFileSync( + path.join(svnWcDir, "agent-workflow.json"), + JSON.stringify({ version: "1", defaultVcs: "svn" }) + ); + const result = await resolveVcsSelection(svnWcDir); + expect(result.vcsKind).toBe("svn"); + }); + + it("auto-selects unique git candidate without config", async () => { + const result = await resolveVcsSelection(gitRoot); + expect(result.vcsKind).toBe("git"); + }); + + it("auto-selects unique svn candidate without config", async () => { + const result = await resolveVcsSelection(svnWcDir); + expect(result.vcsKind).toBe("svn"); + }); + + it("propagates non-ENOENT exceptions from resolveWorkflowConfig (including plain errors without a code)", async () => { + const originalExists = fs.existsSync; + (fs as any).existsSync = (p: any) => { + if (typeof p === "string" && p.includes("agent-workflow.json")) { + throw new Error("Generic read error without code"); + } + return originalExists(p); + }; + try { + await expect(resolveVcsSelection(gitRoot)).rejects.toThrow("Generic read error without code"); + } finally { + (fs as any).existsSync = originalExists; + } + }); + + it("propagates TypeError from config resolution", async () => { + const originalExists = fs.existsSync; + (fs as any).existsSync = (p: any) => { + if (typeof p === "string" && p.includes("agent-workflow.json")) { + throw new TypeError("Type error during read"); + } + return originalExists(p); + }; + try { + await expect(resolveVcsSelection(gitRoot)).rejects.toThrow(TypeError); + } finally { + (fs as any).existsSync = originalExists; + } + }); +}); + +// --------------------------------------------------------------------------- +// startWorkflow integration: nested dual-VCS fixture (git over svn wc) +// --------------------------------------------------------------------------- + +describe("startWorkflow integration with nested dual-VCS fixture", () => { + let tmpDir: string; + let gitRoot: string; + let svnRepoDir: string; + let svnWcDir: string; + let fakeBinDir: string; + let originalWorkflowDir: string | undefined; + let stateRoot: string; + + beforeEach(() => { + tmpDir = makeTmpDir(); + gitRoot = path.join(tmpDir, "git-repo"); + svnRepoDir = path.join(tmpDir, "svn-repo"); + svnWcDir = path.join(gitRoot, "svn-wc"); + fakeBinDir = path.join(tmpDir, "bin"); + stateRoot = path.join(tmpDir, "workflows"); + + // 1. Create and initialize Git repository (outer) + fs.mkdirSync(gitRoot, { recursive: true }); + execFileSync("git", ["init"], { cwd: gitRoot, stdio: "ignore" }); + execFileSync("git", ["config", "user.email", "test@test.com"], { cwd: gitRoot }); + execFileSync("git", ["config", "user.name", "Test"], { cwd: gitRoot }); + fs.writeFileSync(path.join(gitRoot, "README.md"), "git repo"); + fs.writeFileSync(path.join(gitRoot, ".gitignore"), "svn-wc/\n"); + execFileSync("git", ["add", "."], { cwd: gitRoot }); + execFileSync("git", ["commit", "-m", "init git"], { cwd: gitRoot }); + + // 2. Create SVN repo and checkout nested inside gitRoot (at git-repo/svn-wc) + createSvnRepo(svnRepoDir, svnWcDir); + + fs.mkdirSync(fakeBinDir, { recursive: true }); + + const fakeBin = path.join(fakeBinDir, "reasonix"); + fs.writeFileSync( + fakeBin, + [`#!/bin/bash`, `echo '{"session_id":"test-session"}'`, `exit 0`].join("\n"), + { mode: 0o755 } + ); + + originalWorkflowDir = process.env.AGENT_WORKFLOW_DIR; + process.env.AGENT_WORKFLOW_DIR = stateRoot; + }); + + afterEach(() => { + if (originalWorkflowDir === undefined) { + delete process.env.AGENT_WORKFLOW_DIR; + } else { + process.env.AGENT_WORKFLOW_DIR = originalWorkflowDir; + } + cleanupDir(tmpDir); + }); + + it("selects svn when project defaultVcs=svn and starts workflow: persists repoRoot and vcsKind in state", async () => { + // Assert findVcsCandidates(cwd) returns both expected candidate roots before startWorkflow + const candidates = findVcsCandidates(svnWcDir); + expect(candidates.git).toBe(gitRoot); + expect(candidates.svn).toBe(svnWcDir); + + // Commit agent-workflow.json to SVN (clean WC) including the fake executor binary path + const fakeBin = path.join(fakeBinDir, "reasonix"); + const configJson = JSON.stringify({ + version: "1", + defaultVcs: "svn", + executors: { reasonix: { binary: fakeBin } }, + }); + fs.writeFileSync(path.join(svnWcDir, "agent-workflow.json"), configJson); + execFileSync("svn", ["add", "agent-workflow.json"], { cwd: svnWcDir, stdio: "ignore" }); + execFileSync("svn", ["commit", "-m", "add workflow config"], { cwd: svnWcDir, stdio: "ignore" }); + execFileSync("svn", ["update"], { cwd: svnWcDir, stdio: "ignore" }); + + const planFile = path.join(tmpDir, "plan.json"); + const plan = makeValidPlan(["src/"]); + fs.writeFileSync(planFile, JSON.stringify(plan)); + + const { runId } = await startWorkflow({ + planInput: planFile, + executor: "reasonix", + cwd: svnWcDir, // both candidate roots are discoverable from here (SVN at svnWcDir, Git at gitRoot) + vcs: undefined, + }); + + const { findRunDir } = await import("./workflow-state.js"); + const dir = findRunDir(runId); + expect(dir).toBeDefined(); + const state = readState(dir!); + expect(state).toBeDefined(); + + // repoRoot must be the SVN working copy root (svnWcDir) + expect(state!.vcsKind).toBe("svn"); + expect(state!.repoRoot).toBe(svnWcDir); + expect(state!.vcsBaseline?.kind).toBe("svn"); + + // Release lock left by startWorkflow + releaseLock(state!.repoRoot, runId); + }, 30000); + + it("CLI --vcs git override wins over project defaultVcs=svn: persists git repoRoot and vcsKind", async () => { + // Assert findVcsCandidates(cwd) returns both expected candidate roots before startWorkflow + const candidates = findVcsCandidates(svnWcDir); + expect(candidates.git).toBe(gitRoot); + expect(candidates.svn).toBe(svnWcDir); + + const fakeBin = path.join(fakeBinDir, "reasonix"); + + // Commit agent-workflow.json (declaring SVN) in SVN WC + const configSvnJson = JSON.stringify({ + version: "1", + defaultVcs: "svn", + executors: { reasonix: { binary: fakeBin } }, + }); + fs.writeFileSync(path.join(svnWcDir, "agent-workflow.json"), configSvnJson); + execFileSync("svn", ["add", "agent-workflow.json"], { cwd: svnWcDir, stdio: "ignore" }); + execFileSync("svn", ["commit", "-m", "add svn workflow config"], { cwd: svnWcDir, stdio: "ignore" }); + execFileSync("svn", ["update"], { cwd: svnWcDir, stdio: "ignore" }); + + // Also commit agent-workflow.json in outer Git repo (using defaultVcs: svn contrary config) + const configGitJson = JSON.stringify({ + version: "1", + defaultVcs: "svn", + executors: { reasonix: { binary: fakeBin } }, + }); + fs.writeFileSync(path.join(gitRoot, "agent-workflow.json"), configGitJson); + execFileSync("git", ["add", "."], { cwd: gitRoot }); + execFileSync("git", ["commit", "-m", "add git workflow config"], { cwd: gitRoot }); + + const planFile = path.join(tmpDir, "plan.json"); + const plan = makeValidPlan(["README.md"]); + fs.writeFileSync(planFile, JSON.stringify(plan)); + + const { runId } = await startWorkflow({ + planInput: planFile, + executor: "reasonix", + cwd: svnWcDir, // both candidate roots are discoverable from here + vcs: "git", // CLI override wins + }); + + const { findRunDir } = await import("./workflow-state.js"); + const dir = findRunDir(runId); + expect(dir).toBeDefined(); + const state = readState(dir!); + expect(state).toBeDefined(); + + // vcsKind must be "git" because CLI wins + expect(state!.vcsKind).toBe("git"); + expect(state!.vcsBaseline?.kind).toBe("git"); + // repoRoot should be the git root (gitRoot) + expect(state!.repoRoot).toBe(gitRoot); + + releaseLock(state!.repoRoot, runId); + }, 30000); +}); + // --------------------------------------------------------------------------- // SVN Mixed Revisions Detection // --------------------------------------------------------------------------- @@ -178,27 +464,21 @@ describe("SVN mixed revisions with real repository", () => { it("accepts working copy at single revision with different last-changed revisions", () => { createSvnRepo(repoDir, wcDir); - // Make another commit that modifies existing file fs.appendFileSync(path.join(wcDir, "src", "test.txt"), "second line\n"); execFileSync("svn", ["commit", "-m", "Modify test.txt"], { cwd: wcDir, stdio: "ignore" }); - - // Now update to latest - all files at same working revision but different last-changed execFileSync("svn", ["update"], { cwd: wcDir, stdio: "ignore" }); - // Should not throw - all at same working revision expect(() => svnProvider.captureBaseline(wcDir)).not.toThrow(); }); it("rejects actual mixed working revisions", () => { createSvnRepo(repoDir, wcDir); - // Make another commit fs.writeFileSync(path.join(wcDir, "src", "test2.txt"), "second file\n"); execFileSync("svn", ["add", "src/test2.txt"], { cwd: wcDir, stdio: "ignore" }); execFileSync("svn", ["commit", "-m", "Add second file"], { cwd: wcDir, stdio: "ignore" }); execFileSync("svn", ["update"], { cwd: wcDir, stdio: "ignore" }); - // Update only one file to create mixed revisions execFileSync("svn", ["update", "-r", "1", "src/test.txt"], { cwd: wcDir, stdio: "ignore" }); expect(() => svnProvider.captureBaseline(wcDir)).toThrow(/mixed revisions/i); @@ -226,33 +506,33 @@ describe("SVN strict validation with real states", () => { cleanupDir(tmpDir); }); - it("detects conflicted files", () => { + it("detects conflicted files using deterministic dual-working-copy scenario", () => { + // Create two working copies from the same SVN repo createSvnRepo(repoDir, wcDir); + const wc2Dir = path.join(tmpDir, "wc2"); + execFileSync("svn", ["checkout", `file://${repoDir}`, wc2Dir], { stdio: "ignore" }); - // Create a conflict by modifying the same file in different ways - // Make a change and commit - fs.writeFileSync(path.join(wcDir, "src", "test.txt"), "version A\n"); - execFileSync("svn", ["commit", "-m", "Version A"], { cwd: wcDir, stdio: "ignore" }); - - // Go back to r1 and make a different change - execFileSync("svn", ["update", "-r", "1"], { cwd: wcDir, stdio: "ignore" }); + // WC1 (wcDir) keeps local modification to test.txt (version B) + // but does NOT commit — this is the working copy under test fs.writeFileSync(path.join(wcDir, "src", "test.txt"), "version B\n"); - // Try to update - this creates a conflict + // WC2 commits a conflicting change first (version C) + fs.writeFileSync(path.join(wc2Dir, "src", "test.txt"), "version C\n"); + execFileSync("svn", ["commit", "-m", "Version C from wc2"], { cwd: wc2Dir, stdio: "ignore" }); + + // Now update WC1 against the new revision — this creates a conflict spawnSync("svn", ["update"], { cwd: wcDir }); - // Check if conflict exists by looking at status + // Assert that svn status --xml actually reports item=conflicted before checking validator const statusXml = execFileSync("svn", ["status", "--xml"], { cwd: wcDir, encoding: "utf8", }); - // Verify conflict was actually created - if (!statusXml.includes('item="conflicted"')) { - // Skip test if conflict wasn't created - return; - } + // This fixture MUST produce a conflict, or the test setup is wrong + expect(statusXml).toContain('item="conflicted"'); + // Now the validator must reject it expect(() => svnProvider.validateSvnStatusXml(statusXml)).toThrow(/conflict/i); }); @@ -325,6 +605,71 @@ describe("SVN strict validation with real states", () => { expect(() => svnProvider.validateSvnStatusXml(statusXml)).toThrow(/incomplete/i); }); + + it("validates switched path via XML", () => { + const statusXml = ` + + + + + + + +`; + + expect(() => svnProvider.validateSvnStatusXml(statusXml)).toThrow(/switched/i); + }); + + it("rejects real switched path created via svn copy + svn switch", () => { + createSvnRepo(repoDir, wcDir); + + // Create the branches parent directory in the SVN repo + execFileSync( + "svn", + ["mkdir", `file://${repoDir}/branches`, "-m", "create branches dir"], + { stdio: "ignore" } + ); + + // Create a branch of src/ using svn copy (URL-to-URL), the branch has common ancestry + execFileSync( + "svn", + ["copy", `file://${repoDir}/src`, `file://${repoDir}/branches/src-branch`, "-m", "create branch"], + { stdio: "ignore" } + ); + + // Update the working copy to pick up the new repo revisions + execFileSync("svn", ["update"], { cwd: wcDir, stdio: "ignore" }); + + // Switch the versioned src/ subdirectory to the branch — this makes it a switched path + execFileSync("svn", ["switch", `file://${repoDir}/branches/src-branch`, "src"], { + cwd: wcDir, + stdio: "ignore", + }); + + // Verify that svn status --xml reports switched=true BEFORE testing the validator + const statusXml = execFileSync("svn", ["status", "--xml"], { + cwd: wcDir, + encoding: "utf8", + }); + expect(statusXml).toContain('switched="true"'); + + // The strict validator must reject this working copy + expect(() => svnProvider.validateSvnStatusXml(statusXml)).toThrow(/switched/i); + }); + + it("validates tree-conflict via XML", () => { + const statusXml = ` + + + + + + + +`; + + expect(() => svnProvider.validateSvnStatusXml(statusXml)).toThrow(/external/i); + }); }); // --------------------------------------------------------------------------- @@ -359,7 +704,7 @@ describe("SVN diff generation", () => { expect(diff).toContain("+modified content"); }); - it("generates diff for unversioned additions", () => { + it("generates diff for unversioned file additions", () => { const baseline = svnProvider.captureBaseline(wcDir); fs.writeFileSync(path.join(wcDir, "src", "new.txt"), "new file\n"); @@ -369,6 +714,27 @@ describe("SVN diff generation", () => { expect(diff).toContain("+new file"); }); + it("generates diff for scheduled addition (svn add)", () => { + const baseline = svnProvider.captureBaseline(wcDir); + + fs.writeFileSync(path.join(wcDir, "src", "added.txt"), "added content\n"); + execFileSync("svn", ["add", "src/added.txt"], { cwd: wcDir, stdio: "ignore" }); + + const diff = svnProvider.diffFromBaseline(wcDir, baseline); + expect(diff).toContain("src/added.txt"); + expect(diff).toContain("+added content"); + }); + + it("generates diff for scheduled deletion (svn delete)", () => { + const baseline = svnProvider.captureBaseline(wcDir); + + execFileSync("svn", ["delete", "src/test.txt"], { cwd: wcDir, stdio: "ignore" }); + + const diff = svnProvider.diffFromBaseline(wcDir, baseline); + expect(diff).toContain("src/test.txt"); + expect(diff).toContain("-initial content"); + }); + it("generates diff for directly missing tracked files", () => { const baseline = svnProvider.captureBaseline(wcDir); @@ -390,13 +756,19 @@ describe("SVN diff generation", () => { expect(diff).toContain("\\ No newline at end of file"); }); - it("handles empty unversioned files", () => { + it("handles empty unversioned files — produces Index header and empty evidence", () => { const baseline = svnProvider.captureBaseline(wcDir); fs.writeFileSync(path.join(wcDir, "src", "empty.txt"), ""); const diff = svnProvider.diffFromBaseline(wcDir, baseline); + // Must contain the Index header for the file even though it's empty expect(diff).toContain("src/empty.txt"); + expect(diff).toContain("Index: src/empty.txt"); + expect(diff).toContain("==================================================================="); + expect(diff).toContain("--- src/empty.txt\t(nonexistent)"); + expect(diff).toContain("+++ src/empty.txt\t(working copy)"); + expect(diff).toContain("@@ -0,0 +0,0 @@"); }); it("handles paths with spaces and special characters", () => { @@ -415,8 +787,7 @@ describe("SVN diff generation", () => { it("handles binary unversioned files", () => { const baseline = svnProvider.captureBaseline(wcDir); - // Create a binary file with null bytes - const binaryData = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x00, 0x01, 0x02]); + const binaryData = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0x01, 0x02]); fs.writeFileSync(path.join(wcDir, "src", "binary.dat"), binaryData); const diff = svnProvider.diffFromBaseline(wcDir, baseline); @@ -427,7 +798,6 @@ describe("SVN diff generation", () => { it("handles property-only changes", () => { const baseline = svnProvider.captureBaseline(wcDir); - // Set a property on existing file execFileSync("svn", ["propset", "svn:eol-style", "native", "src/test.txt"], { cwd: wcDir, stdio: "ignore", @@ -438,23 +808,169 @@ describe("SVN diff generation", () => { expect(diff).toContain("svn:eol-style"); }); - it("skips directories in unversioned additions", () => { + it("recursively enumerates files inside an unversioned directory with stable sort", () => { const baseline = svnProvider.captureBaseline(wcDir); - // Create unversioned directory with a file - fs.mkdirSync(path.join(wcDir, "src", "newdir")); - fs.writeFileSync(path.join(wcDir, "src", "newdir", "file.txt"), "content\n"); + // Create unversioned directory with nested files (SVN status reports only the dir as unversioned) + fs.mkdirSync(path.join(wcDir, "src", "newdir"), { recursive: true }); + fs.mkdirSync(path.join(wcDir, "src", "newdir", "sub"), { recursive: true }); + fs.writeFileSync(path.join(wcDir, "src", "newdir", "file-a.txt"), "file-a content\n"); + fs.writeFileSync(path.join(wcDir, "src", "newdir", "file-z.txt"), "file-z content\n"); + fs.writeFileSync(path.join(wcDir, "src", "newdir", "sub", "deep.txt"), "deep content\n"); const diff = svnProvider.diffFromBaseline(wcDir, baseline); - // SVN status only reports the parent directory as unversioned, not contents - // If directory is reported, it's skipped; if only file, it's included - // This test verifies directories themselves are skipped in diff generation - expect(diff).not.toContain("Cannot display"); + + // All nested regular files must appear + expect(diff).toContain("src/newdir/file-a.txt"); + expect(diff).toContain("src/newdir/file-z.txt"); + expect(diff).toContain("src/newdir/sub/deep.txt"); + + // Content must be correct + expect(diff).toContain("+file-a content"); + expect(diff).toContain("+file-z content"); + expect(diff).toContain("+deep content"); + + // Paths must appear in sorted (stable) order + const idxA = diff.indexOf("src/newdir/file-a.txt"); + const idxSub = diff.indexOf("src/newdir/sub/deep.txt"); + const idxZ = diff.indexOf("src/newdir/file-z.txt"); + expect(idxA).toBeGreaterThanOrEqual(0); + expect(idxZ).toBeGreaterThanOrEqual(0); + expect(idxSub).toBeGreaterThanOrEqual(0); + // After sorting: file-a, file-z, sub/deep + expect(idxA).toBeLessThan(idxZ); + expect(idxZ).toBeLessThan(idxSub); + }); + + it("does not include svn:ignore-matched files in diff", () => { + const baseline = svnProvider.captureBaseline(wcDir); + + // Set svn:ignore on src/ to ignore *.log files + execFileSync("svn", ["propset", "svn:ignore", "*.log", "src"], { + cwd: wcDir, + stdio: "ignore", + }); + + // Create an ignored file and a non-ignored unversioned file + fs.writeFileSync(path.join(wcDir, "src", "debug.log"), "ignored content\n"); + fs.writeFileSync(path.join(wcDir, "src", "visible.txt"), "visible content\n"); + + // SVN status should not list debug.log (it's ignored) + const diff = svnProvider.diffFromBaseline(wcDir, baseline); + + // The ignored log file must NOT appear + expect(diff).not.toContain("debug.log"); + // The visible unversioned file MUST appear + expect(diff).toContain("src/visible.txt"); + // The property change on src itself should appear too + expect(diff).toContain("svn:ignore"); + }); + + it("does not follow symlinks when enumerating unversioned directory", () => { + const baseline = svnProvider.captureBaseline(wcDir); + + // Create an unversioned directory with a real file and a symlink pointing outside the WC + const outsideFile = path.join(tmpDir, "outside.txt"); + fs.writeFileSync(outsideFile, "outside content\n"); + + fs.mkdirSync(path.join(wcDir, "src", "withlink"), { recursive: true }); + fs.writeFileSync(path.join(wcDir, "src", "withlink", "real.txt"), "real content\n"); + + // Create a file symlink pointing outside the working copy + try { + fs.symlinkSync(outsideFile, path.join(wcDir, "src", "withlink", "link-to-outside.txt")); + } catch { + // If symlinks aren't supported, skip the rest + return; + } + + const diff = svnProvider.diffFromBaseline(wcDir, baseline); + + // The real file must appear + expect(diff).toContain("src/withlink/real.txt"); + // The symlink must NOT be followed / must NOT appear + expect(diff).not.toContain("outside content"); + }); + + it("surfaces lstat failures as VcsError", () => { + const baseline = svnProvider.captureBaseline(wcDir); + fs.mkdirSync(path.join(wcDir, "src", "errdir"), { recursive: true }); + fs.writeFileSync(path.join(wcDir, "src", "errdir", "file.txt"), "content"); + + const originalLstat = fs.lstatSync; + (fs as any).lstatSync = (p: any, opts: any) => { + if (typeof p === "string" && p.includes("file.txt")) { + throw new Error("lstat mock error"); + } + return originalLstat(p, opts); + }; + + try { + expect(() => svnProvider.diffFromBaseline(wcDir, baseline)).toThrow(VcsError); + } finally { + (fs as any).lstatSync = originalLstat; + } + }); + + it("surfaces readdir failures as VcsError", () => { + const baseline = svnProvider.captureBaseline(wcDir); + fs.mkdirSync(path.join(wcDir, "src", "errdir"), { recursive: true }); + fs.writeFileSync(path.join(wcDir, "src", "errdir", "file.txt"), "content"); + + const originalReaddir = fs.readdirSync; + (fs as any).readdirSync = (p: any, opts: any) => { + if (typeof p === "string" && p.includes("errdir")) { + throw new Error("readdir mock error"); + } + return originalReaddir(p, opts); + }; + + try { + expect(() => svnProvider.diffFromBaseline(wcDir, baseline)).toThrow(VcsError); + } finally { + (fs as any).readdirSync = originalReaddir; + } + }); + + it("surfaces read failures (openSync) as VcsError", () => { + const baseline = svnProvider.captureBaseline(wcDir); + fs.writeFileSync(path.join(wcDir, "src", "errfile.txt"), "content"); + + const originalOpen = fs.openSync; + (fs as any).openSync = (p: any, flags: any, mode: any) => { + if (typeof p === "string" && p.includes("errfile.txt")) { + throw new Error("open mock error"); + } + return originalOpen(p, flags, mode); + }; + + try { + expect(() => svnProvider.diffFromBaseline(wcDir, baseline)).toThrow(VcsError); + } finally { + (fs as any).openSync = originalOpen; + } + }); + + it("does not traverse .svn administrative directories", () => { + const baseline = svnProvider.captureBaseline(wcDir); + + // Create an unversioned folder + const newDir = path.join(wcDir, "src", "newdir"); + fs.mkdirSync(newDir, { recursive: true }); + + // Create a mock .svn inside it and put a file in it + const mockSvn = path.join(newDir, ".svn"); + fs.mkdirSync(mockSvn, { recursive: true }); + fs.writeFileSync(path.join(mockSvn, "entries"), "some entry contents"); + + const diff = svnProvider.diffFromBaseline(wcDir, baseline); + // The entries file inside .svn must NOT be in the diff + expect(diff).not.toContain("entries"); }); }); // --------------------------------------------------------------------------- -// Extension Feature with Fake Executor +// Extension Feature with Fake Executor (recording full argv) // --------------------------------------------------------------------------- describe("Budget extension with fake executor", () => { @@ -463,12 +979,14 @@ describe("Budget extension with fake executor", () => { let stateRoot: string; let originalWorkflowDir: string | undefined; let fakeBinDir: string; + let argLogFile: string; beforeEach(() => { tmpDir = makeTmpDir(); repoRoot = path.join(tmpDir, "repo"); stateRoot = path.join(tmpDir, "workflows"); fakeBinDir = path.join(tmpDir, "bin"); + argLogFile = path.join(tmpDir, "reasonix-argv.log"); fs.mkdirSync(repoRoot, { recursive: true }); fs.mkdirSync(fakeBinDir, { recursive: true }); @@ -483,9 +1001,19 @@ describe("Budget extension with fake executor", () => { originalWorkflowDir = process.env.AGENT_WORKFLOW_DIR; process.env.AGENT_WORKFLOW_DIR = stateRoot; - // Create fake reasonix that returns immediately + // Create fake reasonix that records ALL argv (one arg per line) then exits 0 const fakeBin = path.join(fakeBinDir, "reasonix"); - fs.writeFileSync(fakeBin, `#!/bin/bash\necho '{"session_id":"test-session"}'\nexit 0\n`, { mode: 0o755 }); + fs.writeFileSync( + fakeBin, + [ + "#!/bin/bash", + // Write each arg on its own line for reliable contains-check + `printf '%s\\n' "$@" >> ${JSON.stringify(argLogFile)}`, + `echo '{"session_id":"test-session"}'`, + "exit 0", + ].join("\n"), + { mode: 0o755 } + ); }); afterEach(() => { @@ -502,6 +1030,9 @@ describe("Budget extension with fake executor", () => { const dir = runDir(repoRoot, runId); fs.mkdirSync(dir, { recursive: true }); + const sessionFile = path.join(tmpDir, "session.jsonl"); + fs.writeFileSync(sessionFile, ""); + const state = initWorkflowState({ runId, repoRoot, @@ -513,8 +1044,7 @@ describe("Budget extension with fake executor", () => { }); state.status = "budget_exhausted"; state.stopReason = "budget_exhausted"; - state.sessionHandle = { kind: "reasonix", sessionFilePath: path.join(tmpDir, "session.jsonl") }; - fs.writeFileSync(path.join(tmpDir, "session.jsonl"), ""); + state.sessionHandle = { kind: "reasonix", sessionFilePath: sessionFile }; state.executorConfig = { binary: path.join(fakeBinDir, "reasonix") }; const decision: HostDecisionV1 = { @@ -525,32 +1055,34 @@ describe("Budget extension with fake executor", () => { decidedAt: new Date().toISOString(), }; - state.cycles = [{ - cycleIndex: 2, - startedAt: new Date().toISOString(), - decisionFile: "cycle-2-decision.json", - decisionOutcome: "fix", - }]; + state.cycles = [ + { + cycleIndex: 2, + startedAt: new Date().toISOString(), + decisionFile: "cycle-2-decision.json", + decisionOutcome: "fix", + }, + ]; state.currentCycle = 2; fs.writeFileSync(path.join(dir, "cycle-2-decision.json"), JSON.stringify(decision)); writeState(dir, state); - // Extension should succeed and resume executor await extendWorkflow({ runId, additionalCycles: 3 }); - // Check state after successful extension const updatedState = readState(dir); expect(updatedState).toBeDefined(); - // State should be awaiting_review after fake executor completes + // Status should be awaiting_review after fake executor completes expect(updatedState!.status).toBe("awaiting_review"); expect(updatedState!.maxCycles).toBe(5); expect(updatedState!.currentCycle).toBe(3); - // Session handle unchanged + // Session handle preserved with original sessionFilePath expect(updatedState!.sessionHandle?.kind).toBe("reasonix"); - expect(updatedState!.sessionHandle?.sessionFilePath).toBe(path.join(tmpDir, "session.jsonl")); + if (updatedState!.sessionHandle?.kind === "reasonix") { + expect(updatedState!.sessionHandle.sessionFilePath).toBe(sessionFile); + } // Extension history recorded expect(updatedState!.extensionHistory).toBeDefined(); @@ -560,10 +1092,50 @@ describe("Budget extension with fake executor", () => { expect(updatedState!.extensionHistory![0].additionalCycles).toBe(3); expect(updatedState!.extensionHistory![0].resumedCycle).toBe(3); - // Check cycle 3 was created + // Cycle 3 was created expect(updatedState!.cycles).toHaveLength(2); expect(updatedState!.cycles[1].cycleIndex).toBe(3); - }, 10000); + + // Executor log file for cycle 3 must exist and be non-empty or at least present + const cycle3 = updatedState!.cycles.find((c) => c.cycleIndex === 3); + expect(cycle3?.executorLogFile).toBeDefined(); + const logAbsPath = path.join(dir, cycle3!.executorLogFile!); + expect(fs.existsSync(logAbsPath)).toBe(true); + const executorLogContent = fs.readFileSync(logAbsPath, "utf8"); + expect(executorLogContent.trim()).not.toBe(""); + expect(executorLogContent).toContain('{"session_id":"test-session"}'); + + // The resume call must have used --resume + // With printf '%s\n' "$@", each arg is on its own line + expect(fs.existsSync(argLogFile)).toBe(true); + const argLines = fs.readFileSync(argLogFile, "utf8").split("\n").map(line => line.trim()).filter(Boolean); + const resumeIndex = argLines.indexOf("--resume"); + expect(resumeIndex).toBeGreaterThan(-1); + expect(argLines[resumeIndex + 1]).toBe(sessionFile); + + + // events.jsonl must contain a budget_extended event with correct fields + const events = readEvents(dir); + const extEvent = events.find((e) => e.kind === "budget_extended"); + expect(extEvent).toBeDefined(); + expect(extEvent!.runId).toBe(runId); + expect(extEvent!.data?.additionalCycles).toBe(3); + expect(extEvent!.data?.oldMaxCycles).toBe(2); + expect(extEvent!.data?.newMaxCycles).toBe(5); + expect(extEvent!.data?.nextCycle).toBe(3); + + // After successful extension (awaiting_review), the lock must still be held by this runId + // (extendWorkflow does NOT release the lock — it transitions to awaiting_review with the lock held + // until the workflow fully completes or is explicitly aborted) + const activeLock = readLock(repoRoot); + // The lock should be absent after resumeExecutorCycle completes (transitionToTerminal or writeStateSafety) + // Actually: after awaiting_review, the lock stays until decide/review/abort releases it + // But startWorkflow acquires lock and leaves it held when status is awaiting_review + // Similarly extendWorkflow: the lock is held while executing and NOT released on awaiting_review + // So the lock should still be held by runId + expect(activeLock).toBe(runId); + releaseLock(repoRoot, runId); + }, 15000); it("rejects extension with lock conflict", async () => { const runId = "test-extend-lock"; @@ -591,28 +1163,27 @@ describe("Budget extension with fake executor", () => { decidedAt: new Date().toISOString(), }; - state.cycles = [{ - cycleIndex: 2, - startedAt: new Date().toISOString(), - decisionFile: "cycle-2-decision.json", - decisionOutcome: "fix", - }]; + state.cycles = [ + { + cycleIndex: 2, + startedAt: new Date().toISOString(), + decisionFile: "cycle-2-decision.json", + decisionOutcome: "fix", + }, + ]; state.currentCycle = 2; fs.writeFileSync(path.join(dir, "cycle-2-decision.json"), JSON.stringify(decision)); writeState(dir, state); - // Acquire lock with different runId to simulate conflict - const { acquireLock } = await import("./workflow-state.js"); acquireLock(repoRoot, "other-run-id"); await expect(extendWorkflow({ runId, additionalCycles: 2 })).rejects.toThrow(/workflow.*active/i); - // Clean up releaseLock(repoRoot, "other-run-id"); }); - it("rejects extension with out-of-scope changes", async () => { + it("rejects extension with out-of-scope changes — state, history, and events unchanged; lock released", async () => { const runId = "test-extend-scope"; const dir = runDir(repoRoot, runId); fs.mkdirSync(dir, { recursive: true }); @@ -638,24 +1209,43 @@ describe("Budget extension with fake executor", () => { decidedAt: new Date().toISOString(), }; - state.cycles = [{ - cycleIndex: 2, - startedAt: new Date().toISOString(), - decisionFile: "cycle-2-decision.json", - decisionOutcome: "fix", - }]; + state.cycles = [ + { + cycleIndex: 2, + startedAt: new Date().toISOString(), + decisionFile: "cycle-2-decision.json", + decisionOutcome: "fix", + }, + ]; state.currentCycle = 2; fs.writeFileSync(path.join(dir, "cycle-2-decision.json"), JSON.stringify(decision)); writeState(dir, state); + // Snapshot state and events before rejection + const snapshotState = readState(dir); + const snapshotEvents = readEvents(dir); + // Create out-of-scope file - fs.writeFileSync(path.join(repoRoot, "out-of-scope.txt"), "bad"); + const outOfScopeFile = path.join(repoRoot, "out-of-scope.txt"); + fs.writeFileSync(outOfScopeFile, "bad"); await expect(extendWorkflow({ runId, additionalCycles: 2 })).rejects.toThrow(/out-of-scope/i); - // Clean up - fs.unlinkSync(path.join(repoRoot, "out-of-scope.txt")); + // State must be completely unchanged + const afterState = readState(dir); + expect(afterState).toEqual(snapshotState); + + // Events must be unchanged + const afterEvents = readEvents(dir); + expect(afterEvents).toEqual(snapshotEvents); + + // Lock must be released after the rejection + expect(() => acquireLock(repoRoot, runId)).not.toThrow(); + releaseLock(repoRoot, runId); + + // Cleanup + fs.unlinkSync(outOfScopeFile); }); it("rolls back on corrupt decision with lock released", async () => { @@ -676,15 +1266,16 @@ describe("Budget extension with fake executor", () => { state.stopReason = "budget_exhausted"; state.sessionHandle = { kind: "reasonix", sessionFilePath: path.join(tmpDir, "session.jsonl") }; - state.cycles = [{ - cycleIndex: 2, - startedAt: new Date().toISOString(), - decisionFile: "cycle-2-decision.json", - decisionOutcome: "fix", - }]; + state.cycles = [ + { + cycleIndex: 2, + startedAt: new Date().toISOString(), + decisionFile: "cycle-2-decision.json", + decisionOutcome: "fix", + }, + ]; state.currentCycle = 2; - // Write corrupt decision JSON fs.writeFileSync(path.join(dir, "cycle-2-decision.json"), "{ invalid json"); writeState(dir, state); @@ -694,7 +1285,6 @@ describe("Budget extension with fake executor", () => { await expect(extendWorkflow({ runId, additionalCycles: 2 })).rejects.toThrow(/decision/i); - // State should be unchanged (rollback) const unchangedState = readState(dir); expect(unchangedState!.maxCycles).toBe(originalMaxCycles); expect(unchangedState!.status).toBe(originalStatus); @@ -702,7 +1292,6 @@ describe("Budget extension with fake executor", () => { expect(unchangedState!.extensionHistory).toBeUndefined(); // Lock should be released - const { acquireLock } = await import("./workflow-state.js"); expect(() => acquireLock(repoRoot, runId)).not.toThrow(); releaseLock(repoRoot, runId); }); @@ -735,12 +1324,14 @@ describe("Budget extension with fake executor", () => { decidedAt: new Date().toISOString(), }; - state.cycles = [{ - cycleIndex: 2, - startedAt: new Date().toISOString(), - decisionFile: "cycle-2-decision.json", - decisionOutcome: "fix", - }]; + state.cycles = [ + { + cycleIndex: 2, + startedAt: new Date().toISOString(), + decisionFile: "cycle-2-decision.json", + decisionOutcome: "fix", + }, + ]; state.currentCycle = 2; fs.writeFileSync(path.join(dir, "cycle-2-decision.json"), JSON.stringify(decision)); @@ -753,9 +1344,90 @@ describe("Budget extension with fake executor", () => { await expect(extendWorkflow({ runId, additionalCycles: 2 })).rejects.toThrow(/baseline/i); - // The extension should have acquired and then released the lock on failure - // Try to acquire it ourselves to confirm it was released - const { acquireLock } = await import("./workflow-state.js"); + // Lock must be released + expect(() => acquireLock(repoRoot, runId)).not.toThrow(); + releaseLock(repoRoot, runId); + }); + + it("rejects extension when convergence detected: state, history, events unchanged; lock released", async () => { + const runId = "test-extend-convergence"; + const dir = runDir(repoRoot, runId); + fs.mkdirSync(dir, { recursive: true }); + + const sessionFile = path.join(tmpDir, "session.jsonl"); + fs.writeFileSync(sessionFile, ""); + + const state = initWorkflowState({ + runId, + repoRoot, + vcsBaseline: { kind: "git", head: gitHead(repoRoot) }, + vcsKind: "git", + executor: "reasonix", + plan: makeValidPlan(), + maxCycles: 3, + }); + state.status = "budget_exhausted"; + state.stopReason = "budget_exhausted"; + state.sessionHandle = { kind: "reasonix", sessionFilePath: sessionFile }; + state.executorConfig = { binary: path.join(fakeBinDir, "reasonix") }; + + // Build two identical fix decisions with the same accepted finding signature (same findingId+summary) + // This makes checkConvergence detect non-convergence (persistent finding) + const repeatedFinding = { findingId: "dup-1", disposition: "accept" as const, summary: "Same issue persists", rationale: "unchanged" }; + const decision1: HostDecisionV1 = { + version: "1", + outcome: "fix", + findingDecisions: [repeatedFinding], + verificationEvidence: [], + decidedAt: new Date().toISOString(), + }; + const decision2: HostDecisionV1 = { + version: "1", + outcome: "fix", + findingDecisions: [repeatedFinding], // identical signature + verificationEvidence: [], + decidedAt: new Date().toISOString(), + }; + + // Two consecutive cycles, both with fix decisions having the same finding + state.cycles = [ + { + cycleIndex: 1, + startedAt: new Date().toISOString(), + decisionFile: "cycle-1-decision.json", + decisionOutcome: "fix", + completedAt: new Date().toISOString(), + }, + { + cycleIndex: 2, + startedAt: new Date().toISOString(), + decisionFile: "cycle-2-decision.json", + decisionOutcome: "fix", + completedAt: new Date().toISOString(), + }, + ]; + state.currentCycle = 2; + + fs.writeFileSync(path.join(dir, "cycle-1-decision.json"), JSON.stringify(decision1)); + fs.writeFileSync(path.join(dir, "cycle-2-decision.json"), JSON.stringify(decision2)); + writeState(dir, state); + + // Snapshot before rejection + const snapshotState = readState(dir); + const snapshotEvents = readEvents(dir); + + // Extend must be rejected due to convergence + await expect(extendWorkflow({ runId, additionalCycles: 2 })).rejects.toThrow(/convergence|converging/i); + + // State must be completely unchanged + const afterState = readState(dir); + expect(afterState).toEqual(snapshotState); + + // Events must be unchanged + const afterEvents = readEvents(dir); + expect(afterEvents).toEqual(snapshotEvents); + + // Lock must be released expect(() => acquireLock(repoRoot, runId)).not.toThrow(); releaseLock(repoRoot, runId); }); @@ -796,10 +1468,7 @@ describe("Backward compatibility", () => { updatedAt: new Date().toISOString(), }; - fs.writeFileSync( - path.join(dir, "state.json"), - JSON.stringify(oldState, null, 2) - ); + fs.writeFileSync(path.join(dir, "state.json"), JSON.stringify(oldState, null, 2)); const state = readState(dir); expect(state).toBeDefined();