From b24c47486992e86b5767fcac0c0c04e6ece7a79a Mon Sep 17 00:00:00 2001 From: k-atana-agent Date: Mon, 26 Oct 2026 02:54:04 +0800 Subject: [PATCH 1/2] feat: retry conflicted tasks on top of the merge; fix finished runs hanging for 15 minutes - scheduler: a task whose commit conflicts is run again in a fresh worktree from the current base branch with a note naming the conflicting files (conflictRetries, default 1, 0 = off). The conflicting attempt is kept as fsc/-attemptN until a retry merges. Retried tasks that touch files another merged task also touched are listed in report.retryOverlaps and printed as a WARNING: a clean merge does not prove the retry kept the other task's change. - local execution: clear the agent timeout timer when the agent exits. It was never cleared, so a successful run kept the process alive until the timer fired (15 min); failing runs exited via process.exit(1) and hid it. Measured: 2-task run 15m28s -> 1m18s. Regression test included. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016FSVfwwP6TVxGLUWatWM53 --- packages/cli/src/index.ts | 8 +- packages/core/src/execution/local.ts | 6 +- packages/core/src/execution/scheduler.ts | 105 +++++++++++++-- .../core/test/execution/fixtures/run-once.ts | 21 +++ packages/core/test/execution/local.test.ts | 30 +++++ .../test/execution/scheduler.retry.test.ts | 120 ++++++++++++++++++ 6 files changed, 280 insertions(+), 10 deletions(-) create mode 100644 packages/core/test/execution/fixtures/run-once.ts create mode 100644 packages/core/test/execution/scheduler.retry.test.ts diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 63dc5a3..9d9b661 100755 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -112,9 +112,15 @@ export async function runFullAnalysis(): Promise { console.log(''); for (const t of report.tasks) { const merged = report.mergeResults.find((m) => m.taskId === t.taskId); - const mergeNote = merged ? (merged.success ? ` (merged${merged.resolvedTier ? `, tier ${merged.resolvedTier}` : ''})` : ` (NOT merged: ${merged.error})`) : ''; + const retried = report.retries?.[t.taskId]; + const mergeNote = merged ? (merged.success ? ` (merged${retried ? ` after ${retried} retr${retried === 1 ? 'y' : 'ies'}` : ''}${merged.resolvedTier ? `, tier ${merged.resolvedTier}` : ''})` : ` (NOT merged: ${merged.error})`) : ''; console.log(` ${t.status === 'completed' && merged?.success ? 'ok ' : 'FAIL'} ${t.taskId}${mergeNote}${t.error ? ` - ${t.error}` : ''}`); } + for (const [id, list] of Object.entries(report.retryOverlaps ?? {})) { + for (const o of list) { + console.log(` WARNING ${id} merged after a retry and changed files that ${o.task} also changed (${o.files.join(', ')}). Check that it kept ${o.task}'s change.`); + } + } const merged = report.mergeResults.filter((m) => m.success && m.resolvedTier).length; console.log(` ${report.completed}/${report.total} task(s) completed, ${merged} merged, ${report.failed} failed (${Math.round(report.durationMs / 1000)}s)`); diff --git a/packages/core/src/execution/local.ts b/packages/core/src/execution/local.ts index 8cf44c4..ed3d1a5 100644 --- a/packages/core/src/execution/local.ts +++ b/packages/core/src/execution/local.ts @@ -49,6 +49,7 @@ export class LocalExecution implements ExecutionStrategy { // 4. Spawn agent process let timedOut = false; let spawnError: string | null = null; + let timeoutTimer: ReturnType | undefined; let exitCode: number | null = null; let stdoutChunks: string[] = []; let stderrChunks: string[] = []; @@ -73,7 +74,7 @@ export class LocalExecution implements ExecutionStrategy { // Race process completion against timeout const timeoutPromise = new Promise(resolve => { - setTimeout(() => { + timeoutTimer = setTimeout(() => { timedOut = true; try { proc.kill(); } catch {} resolve(); @@ -104,6 +105,9 @@ export class LocalExecution implements ExecutionStrategy { stderrChunks.push(spawnError!); } } finally { + // An uncleared timer keeps the whole process alive until it fires: a finished run used to + // hang for the full agent timeout (15 min) before the CLI could exit. + clearTimeout(timeoutTimer); this.running.delete(unit.id); } diff --git a/packages/core/src/execution/scheduler.ts b/packages/core/src/execution/scheduler.ts index ba77f71..0120e19 100644 --- a/packages/core/src/execution/scheduler.ts +++ b/packages/core/src/execution/scheduler.ts @@ -20,6 +20,12 @@ export interface SchedulerConfig { agentType?: string; /** Build the work unit yourself (benchmarks and tests inject a mock agent here). */ unitFactory?: (task: Task, repoPath: string) => WorkUnit; + /** + * When a task's commit conflicts with what other tasks already merged, run the agent again on + * top of the merged result (new worktree from the current base branch, told which files + * conflicted). Each retry costs another agent run. Default 1; 0 disables it. + */ + conflictRetries?: number; freeclawBase?: string; salacia?: SalaciaConfig & { experiment?: { enabled: boolean; splitRatio: number }; @@ -39,6 +45,16 @@ export interface SchedulerReport { execMs?: number; /** Time spent merging the finished commits onto the base branch. */ mergeMs?: number; + /** Time spent re-running conflicted tasks and merging them again. */ + retryMs?: number; + /** taskId -> number of retry rounds it needed (only tasks that were retried). */ + retries?: Record; + /** + * For tasks that merged only after a retry: files they changed that another merged task also + * changed. The merge was clean, but a clean merge does not prove the retry kept the other + * task's change (two contradictory tasks both "succeed", the later one wins), so look at these. + */ + retryOverlaps?: Record>; /** Time spent removing task worktrees and branches. */ cleanupMs?: number; zeroRegressionRate?: number; // SWE-CI inspired: fraction of merges where ALL pre-existing tests still pass @@ -75,6 +91,20 @@ export class UnifiedScheduler { this.execution = new LocalExecution(this.worktrees); } + private buildUnit(task: Task, repoPath: string, plugin?: SalaciaPlugin, retryNote?: string): WorkUnit { + const unit = this.config.unitFactory + ? this.config.unitFactory(task, repoPath) + : this.config.agentType + ? toWorkUnitForAgent(task, this.config.agentType, repoPath, plugin) + : toWorkUnitByTier(task, repoPath, this.config.costStrategy, this.config.freeclawBase, plugin); + // LocalExecution creates the worktree under repoPath + unit.repoPath = repoPath; + if (retryNote) unit.taskPrompt += ` + +${retryNote}`; + return unit; + } + async execute(tasks: Task[], repoPath: string): Promise { const startTime = Date.now(); this.repoPath = repoPath; @@ -114,13 +144,7 @@ export class UnifiedScheduler { this.stateStore.transition(task.ID, "dispatched"); // Build work unit (with optional salacia enrichment) - const unit = this.config.unitFactory - ? this.config.unitFactory(task, repoPath) - : this.config.agentType - ? toWorkUnitForAgent(task, this.config.agentType, repoPath, plugin) - : toWorkUnitByTier(task, repoPath, this.config.costStrategy, this.config.freeclawBase, plugin); - // LocalExecution creates the worktree under repoPath - unit.repoPath = repoPath; + const unit = this.buildUnit(task, repoPath, plugin); // Running — inject salacia plugin into execution this.stateStore.transition(task.ID, "running", { agentType: unit.agentConfig.type }); @@ -176,9 +200,63 @@ export class UnifiedScheduler { // 4. Merge via sequencer const sequencer = new CommitSequencer(repoPath, this.baseBranch); const mergeStart = Date.now(); - const mergeResults = await sequencer.applyAll(executionResults); + let mergeResults = await sequencer.applyAll(executionResults); const mergeMs = Date.now() - mergeStart; + // 4b. Conflicts: run those tasks again on top of what was merged (see conflictRetries) + const retries: Record = {}; + const retryStart = Date.now(); + const taskById = new Map(tasks.map(t => [t.ID, t])); + for (let round = 1; round <= (this.config.conflictRetries ?? 1); round++) { + const conflicted = mergeResults.filter(m => !m.success && (m.error ?? "").includes("MERGE_CONFLICT")); + if (conflicted.length === 0) break; + + const retryResults: ExecutionResult[] = []; + await Promise.allSettled(conflicted.map(c => this.concurrency.run(async () => { + const task = taskById.get(c.taskId); + if (!task) return; + try { + // Keep the conflicting attempt for inspection; the retry needs the branch name. + gitExec(["branch", "-m", `fsc/${c.taskId}`, `fsc/${c.taskId}-attempt${round}`], repoPath); + const files = (c.error ?? "").split("): ").slice(1).join("): "); + const note = + `## Retry note +An earlier attempt at this task could not be merged because other tasks ` + + `already changed the same code (${files || "unknown files"}). Those changes are in your ` + + `working tree now. Read the current code and do this task again on top of it. If another ` + + `task already made a conflicting decision, keep it unless this task explicitly requires ` + + `otherwise, and say what you did in your final message.`; + const unit = this.buildUnit(task, repoPath, undefined, note); + const r = await this.execution.execute({ ...unit, id: task.ID, repoPath }); + await this.worktrees.removeWorktree(r.worktreePath, repoPath); + retryResults.push(r); + executionResults.push(r); + retries[task.ID] = round; + } catch (e: any) { + console.error(`[Retry] ${c.taskId}: ${e?.message ?? e}`); + } + }))); + + const retryMerge = await sequencer.applyAll(retryResults); + const byTask = new Map(retryMerge.map(m => [m.taskId, m])); + mergeResults = mergeResults.map(m => byTask.get(m.taskId) ?? m); + } + const retryMs = Date.now() - retryStart; + + const latest = new Map(); + for (const r of executionResults) latest.set(r.workUnitId, r); + const mergedIds = new Set(mergeResults.filter(m => m.success).map(m => m.taskId)); + const retryOverlaps: Record> = {}; + for (const taskId of Object.keys(retries)) { + if (!mergedIds.has(taskId)) continue; + const mine = new Set(latest.get(taskId)?.filesChanged ?? []); + for (const [other, r] of latest) { + if (other === taskId || !mergedIds.has(other)) continue; + const files = r.filesChanged.filter(f => mine.has(f)); + if (files.length > 0) (retryOverlaps[taskId] ??= []).push({ task: other, files }); + } + } + // Post-merge regression test (SWE-CI Zero-Regression Rate) // MEMO-inspired: failed merges are logged to an inverse-frequency replay buffer // so retryFailed() can prioritize rare/hard failures. @@ -223,6 +301,14 @@ export class UnifiedScheduler { gitExec(["branch", "-D", er.branch], repoPath); } } + // A retry that merged supersedes the conflicting attempts that were kept for inspection. + for (const taskId of Object.keys(retries)) { + if (!mergeResults.some(m => m.taskId === taskId && m.success)) continue; + const old = gitExec(["branch", "--list", `fsc/${taskId}-attempt*`, "--format=%(refname:short)"], repoPath); + for (const b of old.stdout.split("\n").map(s => s.trim()).filter(Boolean)) { + gitExec(["branch", "-D", b], repoPath); + } + } const cleanupMs = Date.now() - cleanupStart; const durationMs = Date.now() - startTime; @@ -238,6 +324,9 @@ export class UnifiedScheduler { durationMs, execMs, mergeMs, + retryMs, + retries, + retryOverlaps, cleanupMs, zeroRegressionRate: mergedCount > 0 ? zeroRegressionCount / mergedCount : undefined, }; diff --git a/packages/core/test/execution/fixtures/run-once.ts b/packages/core/test/execution/fixtures/run-once.ts new file mode 100644 index 0000000..1e6e71f --- /dev/null +++ b/packages/core/test/execution/fixtures/run-once.ts @@ -0,0 +1,21 @@ +// Used by local.test.ts: run one mock agent through LocalExecution, print the status, return. +import { LocalExecution } from "../../../src/execution/local"; +import { WorktreeManager } from "../../../src/execution/worktree"; + +const repo = process.argv[2]; +const execution = new LocalExecution(new WorktreeManager()); +const result = await execution.execute({ + id: "timer-test", + repoPath: repo, + baseBranch: "main", + taskPrompt: "x", + agentConfig: { + type: "custom", + command: process.platform === "win32" ? "bash" : "/bin/sh", + buildArgs: () => ["-c", "echo hi > timer.txt"], + costTier: "free", + memoryMB: 50, + }, + constraints: { timeoutMs: 600_000 }, +}); +console.log(result.status); diff --git a/packages/core/test/execution/local.test.ts b/packages/core/test/execution/local.test.ts index 594261b..5b92dc0 100644 --- a/packages/core/test/execution/local.test.ts +++ b/packages/core/test/execution/local.test.ts @@ -157,3 +157,33 @@ describe("LocalExecution", () => { expect(result.commitHash).toBeNull(); }); }); + +describe("LocalExecution timers", () => { + test("a finished run leaves no pending timeout timer behind", () => { + // run-once.ts executes a 1 s agent with a 10-minute timeout, then returns. If the timeout + // timer is not cleared the process stays alive for the full 10 minutes. + const repo = path.resolve(os.tmpdir(), `fsc-timer-${Date.now()}`); + fs.mkdirSync(repo, { recursive: true }); + const g = (...a: string[]) => gitExec(a, repo); + g("init"); g("checkout", "-b", "main"); g("config", "user.name", "t"); g("config", "user.email", "t@t"); + fs.writeFileSync(path.join(repo, "a.txt"), "a"); + g("add", "-A"); g("commit", "-m", "init"); + + const started = Date.now(); + const proc = Bun.spawnSync({ + cmd: ["bun", path.resolve(import.meta.dir, "fixtures", "run-once.ts"), repo], + timeout: 60_000, + }); + const elapsed = Date.now() - started; + + expect(new TextDecoder().decode(proc.stdout)).toContain("success"); + expect(elapsed).toBeLessThan(30_000); // about 600 s without clearTimeout + + new WorktreeManager().cleanup(repo); + fs.rmSync(repo, { recursive: true, force: true }); + const parent = path.dirname(repo); + for (const e of fs.readdirSync(parent)) { + if (e.startsWith(`${path.basename(repo)}-fsc-`)) fs.rmSync(path.join(parent, e), { recursive: true, force: true }); + } + }, 90_000); +}); diff --git a/packages/core/test/execution/scheduler.retry.test.ts b/packages/core/test/execution/scheduler.retry.test.ts new file mode 100644 index 0000000..aa5796a --- /dev/null +++ b/packages/core/test/execution/scheduler.retry.test.ts @@ -0,0 +1,120 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import { UnifiedScheduler } from "../../src/execution/scheduler"; +import { gitExec } from "../../src/execution/gitExec"; +import type { WorkUnit } from "../../src/execution/types"; +import type { Task } from "../../src/task"; +import fs from "fs"; +import path from "path"; +import os from "os"; + +// Two tasks rewrite the same line, so the second merge conflicts. The mock agent behaves like a +// sensible one: when its prompt carries the retry note it adds its own line instead of +// overwriting the line that is already there. +const SCRIPT = [ + 'if printf %s "$PROMPT" | grep -q "Retry note"; then', + ' printf "LINE=%s\\n" "$FSC_TASK_ID" >> shared.txt;', + "else", + ' printf "LINE=%s\\n" "$FSC_TASK_ID" > shared.txt;', + "fi", +].join(" "); + +function makeUnit(task: Task, repoPath: string): WorkUnit { + return { + id: task.ID, + repoPath, + baseBranch: "main", + taskPrompt: task.title, + agentConfig: { + type: "custom", + command: "bash", + buildArgs: (prompt: string) => ["-c", `PROMPT=${JSON.stringify(prompt)}; ${SCRIPT}`], + costTier: "free", + memoryMB: 50, + }, + constraints: { timeoutMs: 30_000 }, + }; +} + +const tasks: Task[] = [ + { ID: "a", title: "task a", description: "", priority: 1 }, + { ID: "b", title: "task b", description: "", priority: 1 }, +]; + +describe("UnifiedScheduler conflict retry", () => { + let repo: string; + + beforeEach(() => { + repo = path.resolve(os.tmpdir(), `fsc-retry-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`); + fs.mkdirSync(repo, { recursive: true }); + const g = (...a: string[]) => gitExec(a, repo); + g("init"); + g("checkout", "-b", "main"); + g("config", "user.name", "t"); + g("config", "user.email", "t@t"); + fs.writeFileSync(path.join(repo, "shared.txt"), "LINE=base\n"); + g("add", "-A"); + g("commit", "-m", "base"); + }); + + afterEach(() => { + fs.rmSync(repo, { recursive: true, force: true }); + const parent = path.dirname(repo); + for (const e of fs.readdirSync(parent)) { + if (e.startsWith(`${path.basename(repo)}-fsc-`)) { + fs.rmSync(path.join(parent, e), { recursive: true, force: true }); + } + } + }); + + test("a conflicting task is re-run on top of the merge and then merges", async () => { + const scheduler = new UnifiedScheduler({ maxConcurrency: 2, unitFactory: makeUnit }); + const report = await scheduler.execute(tasks, repo); + + expect(report.mergeResults.every(m => m.success)).toBe(true); + expect(report.retries).toEqual({ b: 1 }); + // clean merge, but both tasks touched shared.txt: flagged so a human checks the outcome + expect(report.retryOverlaps).toEqual({ b: [{ task: "a", files: ["shared.txt"] }] }); + const shared = fs.readFileSync(path.join(repo, "shared.txt"), "utf-8"); + expect(shared).toContain("LINE=a"); + expect(shared).toContain("LINE=b"); + // the superseded attempt branch is gone, nothing is left behind + const branches = gitExec(["branch", "--list", "fsc/*"], repo).stdout.trim(); + expect(branches).toBe(""); + expect(gitExec(["status", "--porcelain", "--untracked-files=no"], repo).stdout.trim()).toBe(""); + }, 60_000); + + test("conflictRetries: 0 keeps the old behaviour (conflict reported, branch kept)", async () => { + const scheduler = new UnifiedScheduler({ maxConcurrency: 2, conflictRetries: 0, unitFactory: makeUnit }); + const report = await scheduler.execute(tasks, repo); + + const failed = report.mergeResults.filter(m => !m.success); + expect(failed).toHaveLength(1); + expect(failed[0].error).toContain("MERGE_CONFLICT"); + expect(report.retries).toEqual({}); + expect(gitExec(["branch", "--list", `fsc/${failed[0].taskId}`], repo).stdout.trim()).not.toBe(""); + }, 60_000); + + test("a retry that loses the race again is reported, with both attempts kept", async () => { + // Three agents that always rewrite the same line. Round 0: a merges, b and c conflict. + // Retry round: b and c both rewrite the new line; b merges first, c conflicts again. + const three: Task[] = [...tasks, { ID: "c", title: "task c", description: "", priority: 1 }]; + const stubborn = (task: Task, repoPath: string): WorkUnit => { + const u = makeUnit(task, repoPath); + u.agentConfig.buildArgs = () => ["-c", 'printf "LINE=%s\n" "$FSC_TASK_ID" > shared.txt']; + return u; + }; + const scheduler = new UnifiedScheduler({ maxConcurrency: 3, conflictRetries: 1, unitFactory: stubborn }); + const report = await scheduler.execute(three, repo); + + const failed = report.mergeResults.filter(m => !m.success); + expect(failed).toHaveLength(1); + expect(failed[0].error).toContain("MERGE_CONFLICT"); + expect(report.mergeResults.filter(m => m.success)).toHaveLength(2); + expect(Object.values(report.retries ?? {})).toEqual([1, 1]); + + const id = failed[0].taskId; + const kept = gitExec(["branch", "--list", `fsc/${id}*`], repo).stdout; + expect(kept).toContain(`fsc/${id}-attempt1`); + expect(kept).toContain(`fsc/${id} `.trimEnd()); + }, 90_000); +}); From fa8e2bc8dbd1489e6bcdf82ea46c62f0493ba62f Mon Sep 17 00:00:00 2001 From: k-atana-agent Date: Mon, 26 Oct 2026 02:54:34 +0800 Subject: [PATCH 2/2] docs(report): conflict retry run and the 15-minute hang Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016FSVfwwP6TVxGLUWatWM53 --- docs/reports/e2e-smoke-2026-10-25.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/reports/e2e-smoke-2026-10-25.md b/docs/reports/e2e-smoke-2026-10-25.md index 8e31dff..c8ef9ce 100644 --- a/docs/reports/e2e-smoke-2026-10-25.md +++ b/docs/reports/e2e-smoke-2026-10-25.md @@ -85,3 +85,18 @@ Change: `claude` is started with `--setting-sources project,local` (opt back in - Trivial prompt, default vs isolated: 22 s vs 9 s. Default: the agent could recite the operator's tracker rules and the Stop hook; isolated: it knew neither. - Same two-task repo as the first run, real `claude` agents: exec phase **20 s** (first run: 77 s). Both tasks merged at tier 1; `main` has the two implementations; agent logs contain no tracker or hook talk. - **Correction:** this run took 15 min 28 s wall clock, not 20 s: analysis and execution finished within about a minute (file timestamps: 20 s + 27 s) and the process then idled for ~14 minutes. So the slow runs are not explained by a user-level hook (this run had them isolated). The cause is a different defect, found right after this report and fixed in a follow-up PR: the agent timeout timer was never cleared, so a successful run kept the process alive until the 15-minute timeout fired. Failing runs exited through `process.exit(1)` and never showed it. That also explains why only successful runs were slow. + +## Fourth run: conflict retry, and the real cause of the 15-minute runs (2026-10-26) + +### Conflict retry (`conflictRetries`, default 1) + +Same three tasks as the second run, real `claude` agents, retry on: + +- `double-c` and `greet-a` merged at tier 1. `greet-b` conflicted, was set aside as `fsc/greet-b-attempt1`, and was run again in a fresh worktree from the new `main` with a note naming `config.py`. The retry merged cleanly: **3/3 merged, 69 s**. +- **The outcome is not "both greetings applied".** `main` ended with `GREETING = "good day"`: the retry agent read its task as explicitly requiring that value and overwrote `greet-a`'s `"hi there"`. The two tasks contradict each other, and a clean merge does not mean both survived. The scheduler now lists such cases in `report.retryOverlaps` (retried tasks that touched a file another merged task also touched), and the CLI prints a WARNING for each. FSC cannot decide which task should win. +- Unit tests (mock agents): retry merges and cleans up the superseded branch; `conflictRetries: 0` keeps the old behavior; a retry that loses the race again is reported with both attempts kept. +- Cost: every retry is another agent run. + +### The 15-minute runs + +Two successful real runs had taken 16 min 10 s and 15 min 28 s although the work was done in about a minute. Cause: the per-agent timeout timer (15 min by default) was never cleared, so Bun kept the process alive until it fired. Runs that failed exited through `process.exit(1)` and never showed it. After clearing the timer, the same two-task run takes **1 min 18 s** total (analysis about 37 s, execution 40 s) and exits right after the report. A regression test starts a 1 s agent with a 10-minute timeout in a child process and requires it to exit within 30 s; it fails (60 s, killed) without the fix.