Repository navigation
feat: 冲突任务在合并结果之上重做; 修复成功运行多挂 15 分钟 #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<string, number>; | ||
| /** | ||
| * 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<string, Array<{ task: string; files: string[] }>>; | ||
| /** 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<SchedulerReport> { | ||
| 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<string, number> = {}; | ||
| 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; | ||
|
Comment on lines
+230
to
+234
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If a retry exits nonzero or times out after changing files, Useful? React with 👍 / 👎. |
||
| } 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<string, ExecutionResult>(); | ||
| 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<string, Array<{ task: string; files: string[] }>> = {}; | ||
| 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, | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When Salacia is enabled, retries are built without their scope contract and execute using whichever plugin remains in
this.execution.salaciaPluginfrom the initial batch. For example, with concurrency 1, if task b conflicts and task c executes afterward, b's retry uses c's plugin;SalaciaPlugin.checkDriftfinds no contract for b and returnsblocked: false. Consequently, a retry that commits changes to protected paths can bypass the protection enforced on its original attempt. Preserve or recreate the task-specific plugin and use it both when building and executing the retry.Useful? React with 👍 / 👎.