diff --git a/MANUAL_TEST_PLAN.md b/MANUAL_TEST_PLAN.md index 6ed044c..87f51cb 100644 --- a/MANUAL_TEST_PLAN.md +++ b/MANUAL_TEST_PLAN.md @@ -132,10 +132,11 @@ Run these tests for basic validation after a code change. References use test ID | 6 | Phase 5 | 5.7-5.10 (plan with deps → verify waiting_deps → cancel) | Plan basics | | 7 | Phase 9 | 9.1 (overview empty) | Dashboard baseline | | 8 | Phase 9 | 9.4 (overview with jobs) | Dashboard with data | -| 9 | Phase 9 | 9.14 (overview after cleanup) | Dashboard cleanup | -| 10 | Phase 12 | Nuclear Cleanup | Clean exit | +| 9 | Phase 5G | 5.76 (retry + relaunch mutual exclusion) | TouchSet param validation | +| 10 | Phase 9 | 9.14 (overview after cleanup) | Dashboard cleanup | +| 11 | Phase 12 | Nuclear Cleanup | Clean exit | -**Pass criteria**: All 10 steps succeed. If any fail, run the full test plan for that phase. +**Pass criteria**: All 11 steps succeed. If any fail, run the full test plan for that phase. --- @@ -159,7 +160,7 @@ All 17 tools must be exercised during this plan. Check off as tested: | `mc_plan` | 5, 6 | Create orchestrated plans | | `mc_plan_status` | 5, 6 | Plan progress | | `mc_plan_cancel` | 5, 6 | Cancel active plan | -| `mc_plan_approve` | 5 | Approve copilot/supervisor | +| `mc_plan_approve` | 5, 5G | Approve copilot/supervisor, accept/relaunch/retry touchSet violations | | `mc_report` | 8 | Agent status reporting (filesystem verification) | | `mc_overview` | 9 | Dashboard summary | @@ -175,7 +176,7 @@ From `plan-types.ts`: | `waiting_deps` | Waiting for dependencies to merge | Phase 5, 6 | | `running` | Agent is actively working | Phase 1, 3, 6 | | `completed` | Agent finished successfully | Phase 1 (if agent completes), 6 | -| `failed` | Agent crashed or exited non-zero | Phase 11 (observational) | +| `failed` | Agent crashed, exited non-zero, or touchSet violation | Phase 5G, 11 (observational) | | `ready_to_merge` | Completed and queued for merge train | Phase 6 (plan context) | | `merging` | Currently being merged into integration | Phase 6 (plan context) | | `merged` | Successfully merged into integration | Phase 6 (plan context) | @@ -480,14 +481,123 @@ This phase tests the git integration tools on a job with real commits. | 5.29 | Check for checkpoint pauses | `mc_plan_status` | May show `paused` at checkpoint or `running` | | 5.30 | Approve checkpoint (if paused) | `mc_plan_approve` checkpoint=pre_merge | Execution continues | +### 5G: TouchSet Enforcement + +This section tests the touchSet violation detection and the three resolution paths: +**accept**, **relaunch**, and **retry**. These require a plan with `touchSet` configured +and a job that deliberately modifies files outside its allowed patterns. + +> **Key Concept**: TouchSet validation runs after a job completes but before it enters the +> merge train. If violations are found, the plan pauses at an `on_error` checkpoint with +> structured `checkpointContext` containing `failureKind`, `jobName`, `touchSetViolations`, +> and `touchSetPatterns`. + +#### 5G-1: TouchSet Violation Detection + +| # | Test | Action | Expected | +|---|------|--------|----------| +| 5.35 | Cleanup from prior tests | `mc_cleanup` all=true, deleteBranch=true | Clean | +| 5.36 | Create plan with touchSet | `mc_plan` name=tmc-plan-touch, mode=supervisor, jobs=[{name: "tmc-ts1", prompt: "Create allowed.txt with 'hello' and also create rogue.txt with 'oops'", touchSet: ["allowed.txt"]}] | Plan created, job launches | +| 5.37 | **Wait 3-5 seconds** | — | — | +| 5.38 | Verify job running | `mc_plan_status` | tmc-ts1=`running` | +| 5.39 | Kill job to simulate completion | `mc_kill` name=tmc-ts1 | Stopped | +| 5.40 | Get worktree path | `mc_status` name=tmc-ts1 | Extract worktree path | +| 5.41 | Create violating files in worktree | In worktree: `echo 'hello' > allowed.txt && echo 'oops' > rogue.txt && git add . && git commit -m "add files"` | Commit with both files | +| 5.42 | **Simulate completion**: Set job status to `completed` via state file edit — update `jobs.json` entry for tmc-ts1 to `status: "completed"` | Job appears completed | +| 5.43 | **Wait 15 seconds** | Orchestrator reconciler detects completion and runs touchSet validation | — | +| 5.44 | Verify plan paused | `mc_plan_status` | Plan `paused`, checkpoint=`on_error` | +| 5.45 | Verify checkpoint context | Read `plan.json` from state dir | `checkpointContext.failureKind` = `"touchset"`, `checkpointContext.jobName` = `"tmc-ts1"`, `checkpointContext.touchSetViolations` includes `"rogue.txt"`, `checkpointContext.touchSetPatterns` = `["allowed.txt"]` | +| 5.46 | Verify job marked failed | `mc_jobs` | tmc-ts1 shows as `failed` | + +> **Note**: Steps 5.42-5.43 are synthetic — we manually set the job to `completed` to trigger +> the orchestrator's touchSet validation. In production, the monitor detects agent completion +> and transitions the job state automatically. + +#### 5G-2: Accept Path (Clear Checkpoint) + +Continue from 5G-1 state (plan paused with touchSet violation). + +| # | Test | Action | Expected | +|---|------|--------|----------| +| 5.47 | Accept violations | `mc_plan_approve` checkpoint=on_error | Checkpoint cleared, job moves to `ready_to_merge` | +| 5.48 | Verify plan resumed | `mc_plan_status` | Plan `running` (or `merging` if merge train started) | +| 5.49 | Verify job state | `mc_jobs` | tmc-ts1 = `ready_to_merge` or `merging` or `merged` | + +> **Cancel immediately after verifying** — do NOT let the plan reach `creating_pr`. + +| # | Action | Verify | +|---|--------|--------| +| 5.50 | `mc_plan_cancel` | Plan cancelled | +| 5.51 | `mc_cleanup` all=true, deleteBranch=true | Cleaned | + +#### 5G-3: Relaunch Path (Agent Correction) + +This tests spawning a new agent in the existing worktree to fix violations. + +| # | Test | Action | Expected | +|---|------|--------|----------| +| 5.52 | Create plan with touchSet | `mc_plan` name=tmc-plan-relaunch, mode=supervisor, jobs=[{name: "tmc-rl1", prompt: "Create allowed.txt with 'hello'", touchSet: ["allowed.txt"]}] | Plan created | +| 5.53 | **Wait 3-5 seconds** | — | — | +| 5.54 | Kill job, create violation, set completed | Same as steps 5.39-5.42 for tmc-rl1 | Job appears completed with rogue.txt | +| 5.55 | **Wait 15 seconds** | Orchestrator detects and validates | — | +| 5.56 | Verify plan paused | `mc_plan_status` | Paused, checkpoint=`on_error` | +| 5.57 | Relaunch agent | `mc_plan_approve` checkpoint=on_error, relaunch=tmc-rl1 | New tmux session created in existing worktree, job back to `running` | +| 5.58 | **Wait 3-5 seconds** | — | — | +| 5.59 | Verify new tmux session | `tmux list-sessions \| grep mc-tmc-rl1` | Session exists | +| 5.60 | Verify job running | `mc_jobs` | tmc-rl1 = `running` | +| 5.61 | Verify correction prompt | `mc_capture` name=tmc-rl1, lines=50 | Agent output visible — correction prompt includes violation details | + +> **Cancel immediately** — the relaunched agent may or may not fix the violations. + +| # | Action | Verify | +|---|--------|--------| +| 5.62 | `mc_plan_cancel` | Plan cancelled | +| 5.63 | `mc_cleanup` all=true, deleteBranch=true | Cleaned | + +#### 5G-4: Retry Path (Manual Fix + Re-validation) + +This tests manually fixing the branch and having MC re-validate. + +| # | Test | Action | Expected | +|---|------|--------|----------| +| 5.64 | Create plan with touchSet | `mc_plan` name=tmc-plan-retry, mode=supervisor, jobs=[{name: "tmc-rt1", prompt: "Create allowed.txt with 'hello'", touchSet: ["allowed.txt"]}] | Plan created | +| 5.65 | **Wait 3-5 seconds** | — | — | +| 5.66 | Kill job, create violation, set completed | Same as steps 5.39-5.42 for tmc-rt1 | Job appears completed with rogue.txt | +| 5.67 | **Wait 15 seconds** | Orchestrator detects and validates | — | +| 5.68 | Verify plan paused | `mc_plan_status` | Paused, checkpoint=`on_error` | +| 5.69 | Retry WITHOUT fixing (should fail) | `mc_plan_approve` checkpoint=on_error, retry=tmc-rt1 | Error: touchSet still violated (rogue.txt still present) | +| 5.70 | Verify plan still paused | `mc_plan_status` | Still paused, checkpoint=`on_error` | +| 5.71 | Fix violation manually | In worktree: `git rm rogue.txt && git commit -m "remove rogue file"` | rogue.txt removed | +| 5.72 | Retry after fix (should succeed) | `mc_plan_approve` checkpoint=on_error, retry=tmc-rt1 | TouchSet re-validated, job moves to `ready_to_merge` | +| 5.73 | Verify plan resumed | `mc_plan_status` | Plan running | + +> **Cancel immediately**. + +| # | Action | Verify | +|---|--------|--------| +| 5.74 | `mc_plan_cancel` | Plan cancelled | +| 5.75 | `mc_cleanup` all=true, deleteBranch=true | Cleaned | + +#### 5G-5: Mutual Exclusion (retry vs relaunch) + +| # | Test | Action | Expected | +|---|------|--------|----------| +| 5.76 | Both retry and relaunch | `mc_plan_approve` checkpoint=on_error, retry=tmc-x, relaunch=tmc-x | Error: cannot specify both retry and relaunch | + +#### 5G-6: Relaunch Non-TouchSet Job Rejected + +| # | Test | Action | Expected | +|---|------|--------|----------| +| 5.77 | Relaunch on non-touchset failure | (If a plan is paused with a non-touchset failure) `mc_plan_approve` checkpoint=on_error, relaunch=jobname | Error: relaunch only available for touchSet violations | + ### Phase 5 Cleanup | # | Action | Verify | |---|--------|--------| -| 5.31 | `mc_plan_cancel` (if still active) | Plan cancelled | -| 5.32 | `mc_cleanup` all=true, deleteBranch=true | All plan artifacts cleaned | -| 5.33 | `mc_jobs` — verify empty | "No jobs found." | -| 5.34 | `mc_plan_status` — verify no plan | "No active plan" | +| 5.78 | `mc_plan_cancel` (if still active) | Plan cancelled | +| 5.79 | `mc_cleanup` all=true, deleteBranch=true | All plan artifacts cleaned | +| 5.80 | `mc_jobs` — verify empty | "No jobs found." | +| 5.81 | `mc_plan_status` — verify no plan | "No active plan" | --- @@ -963,7 +1073,7 @@ rm -f "$STATE_DIR/state/jobs.json" 2>/dev/null || true | 2 | Error handling & edge cases | 28 | | | | +4 window placement, +11 post-create hooks | | 3 | Multiple jobs | 14 | | | | +3 status filter tests | | 4 | Git workflow (sync & merge) | 18 | | | | | -| 5 | Plan orchestration | 34 | | | | | +| 5 | Plan orchestration | 81 | | | | +47 touchSet enforcement (5G: detect, accept, relaunch, retry, mutual exclusion) | | 6 | Realistic multi-job (overlap/conflict) | 49 | | | | | | 7 | Model verification | 12 | | | | +3 model ID, prompt file, model match | | 8 | mc_report flow | 54 | | | | +17 deterministic injection (replaced 21 non-deterministic) | @@ -971,7 +1081,7 @@ rm -f "$STATE_DIR/state/jobs.json" 2>/dev/null || true | 10 | OMO plan mode | 10 | | | | | | 11 | Hooks (observational) | 5 | | | | | | 12 | Final verification & nuclear cleanup | 8 | | | | | -| **Total** | | **276** | | | | | +| **Total** | | **323** | | | | | --- @@ -987,6 +1097,7 @@ rm -f "$STATE_DIR/state/jobs.json" 2>/dev/null || true 8. **Report reliability**: Agents have `mc_report` available (plugin loaded via `.opencode` symlink) and are instructed to call it via `MC_REPORT_SUFFIX` prompt injection. Report files should appear reliably, but agent behavior is ultimately non-deterministic — a missing report after 15 seconds warrants investigation but is not necessarily a plugin failure. 9. **Launcher script timing**: `.mc-launch.sh` is deleted after 5 seconds. Phase 7 must read it immediately after launch. If you miss the window, the test is inconclusive, not failed. 10. **Worktree initialization race**: Some operations may fail if attempted before the worktree is fully initialized. The 3-5 second wait after every `mc_launch` mitigates this. +11. **TouchSet testing on feature branches**: When running Phase 5G on a non-main branch, job worktrees inherit the feature branch's uncommitted changes. TouchSet validation compares the job branch against the integration branch, so feature branch source files show up as spurious violations alongside the actual test violations (e.g., `rogue.txt`). This is a testing artifact — in production, both branches share the same base so only the job's own changes appear. --- @@ -1043,12 +1154,12 @@ This is mitigated by: ``` queued ──────> waiting_deps ──> running ──> completed ──> ready_to_merge ──> merging ──> merged - │ │ │ │ - │ │ ├──> failed ├──> conflict ──> ready_to_merge - │ │ │ │ - │ │ ├──> stopped └──> (canceled/stopped) - │ │ │ - │ │ └──> canceled + │ │ │ │ │ + │ │ ├──> failed └──> failed (touchSet) ├──> conflict ──> ready_to_merge + │ │ │ │ │ + │ │ ├──> stopped ├──> ready_to_merge (accept) └──> (canceled/stopped) + │ │ │ ├──> running (relaunch) + │ │ └──> canceled └──> ready_to_merge (retry) │ │ │ ├──> stopped │ └──> canceled diff --git a/README.md b/README.md index 2ae85c3..ca548bf 100644 --- a/README.md +++ b/README.md @@ -504,7 +504,7 @@ Create and start a multi-job orchestrated plan. | `name` | `string` | Yes | Unique job name | | `prompt` | `string` | Yes | Task prompt for the AI agent | | `dependsOn` | `string[]` | No | Job names this job depends on (must complete and merge first) | -| `touchSet` | `string[]` | No | File globs this job expects to modify | +| `touchSet` | `string[]` | No | File globs this job expects to modify. After completion, the orchestrator validates that only files matching these patterns were changed. Violations pause the plan — see [TouchSet Enforcement](#touchset-enforcement). | | `mode` | `"vanilla"` \| `"plan"` \| `"ralph"` \| `"ulw"` | No | Execution mode override (defaults to `omo.defaultMode` config) | | `copyFiles` | `string[]` | No | Files to copy into the worktree | | `symlinkDirs` | `string[]` | No | Directories to symlink into the worktree | @@ -563,11 +563,21 @@ Show the current state of the active plan — job statuses, progress, and any ch #### `mc_plan_approve` -Approve a copilot plan to start execution, or clear a supervisor checkpoint to continue. +Approve a pending copilot plan, clear a supervisor checkpoint, or handle a failed job. When a touchSet violation pauses the plan, three actions are available: | Parameter | Type | Required | Description | |-----------|------|----------|-------------| | `checkpoint` | `"pre_merge"` \| `"on_error"` \| `"pre_pr"` | No | Specific checkpoint to clear (supervisor mode) | +| `retry` | `string` | No | Name of a failed/conflict/needs_rebase job to retry after manual fix. Re-validates touchSet before proceeding. | +| `relaunch` | `string` | No | Name of a touchSet-failed job to relaunch. Spawns a new agent in the existing worktree with a correction prompt. | + +**TouchSet violation options:** + +| Action | Command | Behavior | +|--------|---------|----------| +| **Accept** | `mc_plan_approve(checkpoint: "on_error")` | Violations are legitimate — skip validation and proceed to merge | +| **Relaunch** | `mc_plan_approve(checkpoint: "on_error", relaunch: "job")` | Spawn agent to fix — relaunches in existing worktree with violation details | +| **Retry** | `mc_plan_approve(checkpoint: "on_error", retry: "job")` | You fixed it manually — re-validates touchSet, then proceeds if clean | #### `mc_plan_cancel` @@ -622,14 +632,38 @@ Result: > **Why not `mc_launch`?** You *could* launch these sequentially, merging each into main before starting the next. But you'd lose the automated test gating, the single integration PR, and the parallel execution of `dashboard-ui` and `docs`. The plan handles the dependency graph, merge ordering, and PR creation — you just check `mc_plan_status` or wait for the completion notification. +### TouchSet Enforcement + +When a job specifies `touchSet` patterns, the orchestrator validates the job's changes after completion — before the job enters the merge train. If the job modified files that don't match any `touchSet` glob, the plan pauses with an `on_error` checkpoint: + +``` +❌ Job "db-schema" modified files outside its touchSet: + Violations: src/types/search.ts, src/utils/format.ts + Allowed: src/db/** + +Options: + Accept violations: mc_plan_approve(checkpoint: "on_error") + Agent fixes branch: mc_plan_approve(checkpoint: "on_error", relaunch: "db-schema") + You fix, re-check: mc_plan_approve(checkpoint: "on_error", retry: "db-schema") +``` + +| Option | When to use | +|--------|-------------| +| **Accept** | You reviewed the violations and they're legitimate (e.g., the agent needed to update shared types) | +| **Relaunch** | The violations are accidental — a new agent spawns in the same worktree with a correction prompt to revert them | +| **Retry** | You manually fixed the branch — Mission Control re-validates the touchSet before proceeding | + +The **relaunch** path reuses the existing worktree and branch. The correction agent sees the full git history and receives a prompt with the original task, the specific violations, and the allowed patterns. After the agent completes, touchSet validation runs again automatically. + ### Merge Train The Merge Train is the engine behind plan integration. Each completed job's branch is merged into a dedicated **integration branch** (`mc/integration-{plan-id}`): -1. **Merge** — `git merge --no-ff {job-branch}` into the integration worktree -2. **Test** — If a `testCommand` is configured (or detected from `package.json`), it runs after each merge -3. **Rollback** — If tests fail or time out, the merge is automatically rolled back (`git merge --abort` or `git reset --hard HEAD~1`) -4. **Conflict detection** — Merge conflicts are caught, reported with file-level detail, and the merge is aborted +1. **TouchSet validation** — If `touchSet` is configured, verify the job only modified allowed files (violations pause the plan) +2. **Merge** — `git merge --no-ff {job-branch}` into the integration worktree +3. **Test** — If a `testCommand` is configured (or detected from `package.json`), it runs after each merge +4. **Rollback** — If tests fail or time out, the merge is automatically rolled back (`git merge --abort` or `git reset --hard HEAD~1`) +5. **Conflict detection** — Merge conflicts are caught, reported with file-level detail, and the merge is aborted Once all jobs are merged and tests pass, the integration branch is pushed and a PR is created. @@ -803,6 +837,12 @@ Yes. Use `mc_attach` to get the tmux command, then run it in your terminal. You' If the configured `testCommand` fails after a merge, the merge is automatically rolled back. The job is marked as failed and the plan status updates accordingly (in supervisor mode, it pauses for your review). +
+What happens if a job modifies files outside its touchSet? + +The plan pauses at an `on_error` checkpoint with three options: **accept** the violations if they're legitimate, **relaunch** the agent with a correction prompt to fix them, or **retry** after you fix the branch manually. See [TouchSet Enforcement](#touchset-enforcement) for details. +
+
Why not just tell my agents to run git worktree add themselves? diff --git a/src/lib/orchestrator.ts b/src/lib/orchestrator.ts index 62c78f9..ae1042d 100644 --- a/src/lib/orchestrator.ts +++ b/src/lib/orchestrator.ts @@ -1,6 +1,6 @@ import { randomUUID } from 'crypto'; import type { MCConfig } from './config'; -import type { PlanSpec, JobSpec, PlanStatus, CheckpointType } from './plan-types'; +import type { PlanSpec, JobSpec, PlanStatus, CheckpointType, CheckpointContext } from './plan-types'; import { loadPlan, savePlan, updatePlanJob, clearPlan, validateGhAuth } from './plan-state'; import { getDefaultBranch } from './git'; import { createIntegrationBranch, deleteIntegrationBranch } from './integration'; @@ -302,6 +302,7 @@ export class Orchestrator { plan.status = 'running'; plan.checkpoint = null; + plan.checkpointContext = null; await savePlan(plan); this.showToast('Mission Control', 'Checkpoint cleared, resuming execution.', 'info'); @@ -388,10 +389,11 @@ export class Orchestrator { this.isRunning = false; } - private async setCheckpoint(type: CheckpointType, plan: PlanSpec): Promise { + private async setCheckpoint(type: CheckpointType, plan: PlanSpec, context?: CheckpointContext): Promise { this.checkpoint = type; plan.status = 'paused'; plan.checkpoint = type; + plan.checkpointContext = context ?? null; await savePlan(plan); this.stopReconciler(); this.showToast( @@ -501,8 +503,21 @@ export class Orchestrator { job.status = 'failed'; this.showToast('Mission Control', `Job "${job.name}" touched files outside its touchSet. Plan paused.`, 'error'); - this.notify(`❌ Job "${job.name}" modified files outside its touchSet:\n Violations: ${validation.violations.join(', ')}\n Allowed: ${job.touchSet.join(', ')}\nFix the branch and retry with mc_plan_approve(checkpoint: "on_error", retry: "${job.name}").`); - await this.setCheckpoint('on_error', plan); + this.notify( + `❌ Job "${job.name}" modified files outside its touchSet:\n` + + ` Violations: ${validation.violations.join(', ')}\n` + + ` Allowed: ${job.touchSet.join(', ')}\n\n` + + `Options:\n` + + ` Accept violations: mc_plan_approve(checkpoint: "on_error")\n` + + ` Agent fixes branch: mc_plan_approve(checkpoint: "on_error", relaunch: "${job.name}")\n` + + ` You fix, re-check: mc_plan_approve(checkpoint: "on_error", retry: "${job.name}")`, + ); + await this.setCheckpoint('on_error', plan, { + jobName: job.name, + failureKind: 'touchset', + touchSetViolations: validation.violations, + touchSetPatterns: job.touchSet, + }); return; } } @@ -539,7 +554,10 @@ export class Orchestrator { this.showToast('Mission Control', `Job "${job.name}" has merge conflicts. Plan paused.`, 'error'); this.notify(`❌ Job "${job.name}" would conflict with the integration branch.\n Files: ${mergeCheck.conflicts?.join(', ') ?? 'unknown'}\nRebase the job branch and retry with mc_plan_approve(checkpoint: "on_error", retry: "${job.name}").`); - await this.setCheckpoint('on_error', plan); + await this.setCheckpoint('on_error', plan, { + jobName: job.name, + failureKind: 'merge_conflict', + }); return; } } @@ -590,7 +608,10 @@ export class Orchestrator { this.showToast('Mission Control', `Merge conflict in job "${nextJob.name}". Plan paused.`, 'error'); this.notify(`❌ Merge conflict in job "${nextJob.name}". Files: ${mergeResult.files?.join(', ') ?? 'unknown'}. Fix the branch and retry with mc_plan_approve(checkpoint: "on_error", retry: "${nextJob.name}").`); - await this.setCheckpoint('on_error', plan); + await this.setCheckpoint('on_error', plan, { + jobName: nextJob.name, + failureKind: 'merge_conflict', + }); return; } else { await updatePlanJob(plan.id, nextJob.name, { @@ -605,7 +626,10 @@ export class Orchestrator { this.showToast('Mission Control', `Job "${nextJob.name}" failed merge tests. Plan paused.`, 'error'); this.notify(`❌ Job "${nextJob.name}" failed merge tests. Fix the branch and retry with mc_plan_approve(checkpoint: "on_error", retry: "${nextJob.name}").`); - await this.setCheckpoint('on_error', plan); + await this.setCheckpoint('on_error', plan, { + jobName: nextJob.name, + failureKind: 'test_failure', + }); return; } } @@ -857,6 +881,190 @@ If your work needs human review before it can proceed: mc_report(status: "needs_ } } + async relaunchJobForCorrection( + jobName: string, + violations: string[], + touchSetPatterns: string[], + ): Promise { + const plan = await loadPlan(); + if (!plan) { + throw new Error('No active plan'); + } + + const job = plan.jobs.find((j) => j.name === jobName); + if (!job) { + throw new Error(`Job "${jobName}" not found in plan`); + } + if (!job.worktreePath || !job.branch) { + throw new Error(`Job "${jobName}" has no worktree or branch — cannot relaunch`); + } + + const placement = this.planPlacement ?? this.config.defaultPlacement ?? 'session'; + const mode = job.mode ?? this.config.omo?.defaultMode ?? 'vanilla'; + const sanitizedName = job.name.replace(/[^a-zA-Z0-9_-]/g, '-'); + const tmuxSessionName = `mc-${sanitizedName}`; + const tmuxTarget = + placement === 'session' + ? tmuxSessionName + : (() => { + const currentSession = getCurrentSession(); + if (!currentSession && !isInsideTmux()) { + throw new Error('Window placement requires running inside tmux'); + } + return `${currentSession}:${sanitizedName}`; + })(); + + // Kill old tmux session if still alive + try { + if (placement === 'session') { + await killSession(tmuxSessionName); + } else { + const [session, window] = tmuxTarget.split(':'); + if (session && window) { + await killWindow(session, window); + } + } + } catch { + // Session may already be dead — that's fine + } + + // Build correction prompt with context from the original task + const correctionPrompt = + `CORRECTION TASK — TouchSet Violation\n\n` + + `The previous task in this worktree was:\n"${job.prompt}"\n\n` + + `It completed successfully but modified files outside the allowed scope.\n\n` + + `Violations (files you must revert):\n${violations.map((v) => ` - ${v}`).join('\n')}\n\n` + + `Allowed patterns (files you may keep):\n${touchSetPatterns.map((p) => ` - ${p}`).join('\n')}\n\n` + + `Instructions:\n` + + `1. Review the changes on this branch (git log, git diff)\n` + + `2. Revert ONLY the violating files listed above — do NOT break the intended work\n` + + `3. If a violating file is genuinely required, explain why in your commit message\n` + + `4. Commit your corrections and report completion`; + + const mcReportSuffix = `\n\nCRITICAL — STATUS REPORTING REQUIRED: +You MUST call the mc_report tool at these points — this is NOT optional: + +1. IMMEDIATELY when you start: mc_report(status: "working", message: "Starting: ") +2. At each major milestone: mc_report(status: "progress", message: "", progress: <0-100>) +3. If you get stuck or need input: mc_report(status: "blocked", message: "") +4. WHEN YOU ARE COMPLETELY DONE: mc_report(status: "completed", message: "") + +The "completed" call is MANDATORY — it signals Mission Control that your job is finished. Without it, your job will appear stuck as "running" and block the pipeline. Always call mc_report(status: "completed", ...) as your FINAL action. + +If your work needs human review before it can proceed: mc_report(status: "needs_review", message: "")`; + const autoCommitSuffix = (this.config.autoCommit !== false) + ? `\n\nIMPORTANT: When you have completed ALL of your work, you MUST commit your changes before finishing. Stage all modified and new files, then create a commit with a conventional commit message (e.g. "feat: ...", "fix: ...", "docs: ...", "refactor: ...", "chore: ..."). Do NOT skip this step.` + : ''; + + let fullPrompt = correctionPrompt + mcReportSuffix + autoCommitSuffix; + if (mode === 'ralph') { + fullPrompt = `/ralph-loop ${fullPrompt}`; + } else if (mode === 'ulw') { + fullPrompt = `/ulw-loop ${fullPrompt}`; + } + + const worktreePath = job.worktreePath; + let promptFilePath: string | undefined; + + try { + promptFilePath = await writePromptFile(worktreePath, fullPrompt); + const model = this.planModelSnapshot ?? getCurrentModel(); + const launcherPath = await writeLauncherScript(worktreePath, promptFilePath, model); + + const initialCommand = `bash '${launcherPath}'`; + if (placement === 'session') { + await createSession({ + name: tmuxSessionName, + workdir: worktreePath, + command: initialCommand, + }); + } else { + await createWindow({ + session: getCurrentSession()!, + name: sanitizedName, + workdir: worktreePath, + command: initialCommand, + }); + } + + await setPaneDiedHook(tmuxTarget, `run-shell "echo '${job.id}' >> .mission-control/completed-jobs.log"`); + cleanupPromptFile(promptFilePath); + cleanupLauncherScript(worktreePath); + + if (mode !== 'vanilla') { + try { + await new Promise((resolve) => setTimeout(resolve, 2000)); + switch (mode) { + case 'plan': + await sendKeys(tmuxTarget, '/start-work'); + await sendKeys(tmuxTarget, 'Enter'); + break; + case 'ralph': + await sendKeys(tmuxTarget, '/ralph-loop'); + await sendKeys(tmuxTarget, 'Enter'); + break; + case 'ulw': + await sendKeys(tmuxTarget, '/ulw-loop'); + await sendKeys(tmuxTarget, 'Enter'); + break; + } + } catch { + // Non-fatal: OMO command delivery is best-effort + } + } + + // Clean up stale job entries and create fresh one for the relaunch + const existingState = await loadJobState(); + const staleJobs = existingState.jobs.filter( + (j) => j.name === job.name && j.status !== 'running', + ); + for (const stale of staleJobs) { + await removeReport(stale.id).catch(() => {}); + await removeJob(stale.id).catch(() => {}); + } + + const jobId = randomUUID(); + await addJob({ + id: jobId, + name: job.name, + worktreePath, + branch: job.branch, + tmuxTarget, + placement, + status: 'running', + prompt: correctionPrompt, + mode, + createdAt: new Date().toISOString(), + planId: plan.id, + }); + + await updatePlanJob(plan.id, job.name, { + status: 'running', + tmuxTarget, + error: undefined, + }); + } catch (error) { + if (promptFilePath) { + cleanupPromptFile(promptFilePath, 0); + } + + try { + if (placement === 'session') { + await killSession(tmuxSessionName); + } else { + const [session, window] = tmuxTarget.split(':'); + if (session && window) { + await killWindow(session, window); + } + } + } catch {} + + throw new Error( + `Failed to relaunch job "${jobName}" for correction: ${error instanceof Error ? error.message : String(error)}`, + ); + } + } + private handleJobComplete = (job: Job): void => { if (!job.planId || !this.activePlanId || job.planId !== this.activePlanId) { return; @@ -893,7 +1101,10 @@ If your work needs human review before it can proceed: mc_report(status: "needs_ this.showToast('Mission Control', `Job "${job.name}" failed. Plan paused.`, 'error'); this.notify(`❌ Job "${job.name}" failed. Fix and retry with mc_plan_approve(checkpoint: "on_error", retry: "${job.name}").`); - await this.setCheckpoint('on_error', plan); + await this.setCheckpoint('on_error', plan, { + jobName: job.name, + failureKind: 'job_failed', + }); }) .catch(() => {}) .finally(() => { @@ -945,6 +1156,7 @@ If your work needs human review before it can proceed: mc_report(status: "needs_ if (plan.status === 'paused') { plan.status = 'running'; plan.checkpoint = null; + plan.checkpointContext = null; await savePlan(plan); } this.checkpoint = null; diff --git a/src/lib/plan-types.ts b/src/lib/plan-types.ts index c11c5ac..bdd0499 100644 --- a/src/lib/plan-types.ts +++ b/src/lib/plan-types.ts @@ -10,6 +10,15 @@ export type PlanStatus = export type CheckpointType = 'pre_merge' | 'on_error' | 'pre_pr'; +export type FailureKind = 'touchset' | 'merge_conflict' | 'test_failure' | 'job_failed'; + +export interface CheckpointContext { + jobName: string; + failureKind: FailureKind; + touchSetViolations?: string[]; + touchSetPatterns?: string[]; +} + export type JobStatus = | 'queued' | 'waiting_deps' @@ -39,6 +48,7 @@ export interface PlanSpec { completedAt?: string; prUrl?: string; checkpoint?: CheckpointType | null; + checkpointContext?: CheckpointContext | null; } export interface JobSpec { @@ -76,8 +86,8 @@ export const VALID_JOB_TRANSITIONS: Record = { queued: ['waiting_deps', 'running', 'stopped', 'canceled'], waiting_deps: ['running', 'stopped', 'canceled'], running: ['completed', 'failed', 'stopped', 'canceled'], - completed: ['ready_to_merge', 'stopped', 'canceled'], - failed: ['ready_to_merge', 'stopped', 'canceled'], + completed: ['ready_to_merge', 'failed', 'stopped', 'canceled'], + failed: ['ready_to_merge', 'running', 'stopped', 'canceled'], ready_to_merge: ['merging', 'needs_rebase', 'stopped', 'canceled'], merging: ['merged', 'conflict', 'stopped', 'canceled'], merged: ['needs_rebase'], diff --git a/src/lib/schemas.ts b/src/lib/schemas.ts index ff0409a..b756f30 100644 --- a/src/lib/schemas.ts +++ b/src/lib/schemas.ts @@ -72,6 +72,15 @@ export const JobSpecSchema = z.object({ mode: z.enum(['vanilla', 'plan', 'ralph', 'ulw']).optional(), }); +export const FailureKindSchema = z.enum(['touchset', 'merge_conflict', 'test_failure', 'job_failed']); + +export const CheckpointContextSchema = z.object({ + jobName: z.string(), + failureKind: FailureKindSchema, + touchSetViolations: z.array(z.string()).optional(), + touchSetPatterns: z.array(z.string()).optional(), +}); + export const PlanSpecSchema = z.object({ id: z.string(), name: z.string(), @@ -86,6 +95,7 @@ export const PlanSpecSchema = z.object({ completedAt: z.string().optional(), prUrl: z.string().optional(), checkpoint: CheckpointTypeSchema.nullable().optional(), + checkpointContext: CheckpointContextSchema.nullable().optional(), ghAuthenticated: z.boolean().optional(), }); diff --git a/src/tools/plan-approve.ts b/src/tools/plan-approve.ts index bd5a1c6..acea36a 100644 --- a/src/tools/plan-approve.ts +++ b/src/tools/plan-approve.ts @@ -1,5 +1,5 @@ import { tool, type ToolDefinition } from '@opencode-ai/plugin'; -import { loadPlan, savePlan, updatePlanJob } from '../lib/plan-state'; +import { loadPlan, savePlan } from '../lib/plan-state'; import { Orchestrator } from '../lib/orchestrator'; import { getSharedMonitor, getSharedNotifyCallback, setSharedOrchestrator } from '../lib/orchestrator-singleton'; import type { CheckpointType } from '../lib/plan-types'; @@ -7,10 +7,11 @@ import { loadConfig } from '../lib/config'; import { getCurrentModel } from '../lib/model-tracker'; import { createIntegrationBranch } from '../lib/integration'; import { resolvePostCreateHook } from '../lib/worktree-setup'; +import { validateTouchSet } from '../lib/merge-train'; export const mc_plan_approve: ToolDefinition = tool({ description: - 'Approve a pending copilot plan, clear a supervisor checkpoint, or retry a failed job to continue execution', + 'Approve a pending copilot plan, clear a supervisor checkpoint, or retry/relaunch a failed job to continue execution', args: { checkpoint: tool.schema .enum(['pre_merge', 'on_error', 'pre_pr']) @@ -19,51 +20,139 @@ export const mc_plan_approve: ToolDefinition = tool({ retry: tool.schema .string() .optional() - .describe('Name of a failed, conflict, or needs_rebase job to retry'), + .describe('Name of a failed, conflict, or needs_rebase job to retry after manual fix (re-validates touchSet)'), + relaunch: tool.schema + .string() + .optional() + .describe('Name of a touchSet-failed job to relaunch — spawns agent in existing worktree with correction prompt'), }, async execute(args) { + if (args.retry && args.relaunch) { + throw new Error('Cannot specify both "retry" and "relaunch". Use "retry" for manual fixes (re-validates) or "relaunch" to spawn an agent to fix violations.'); + } + const plan = await loadPlan(); if (!plan) { throw new Error('No active plan to approve'); } - if (args.retry) { - const job = plan.jobs.find((j) => j.name === args.retry); + const targetJobName = args.retry ?? args.relaunch; + if (targetJobName) { + const job = plan.jobs.find((j) => j.name === targetJobName); if (!job) { - throw new Error(`Job "${args.retry}" not found in plan`); + throw new Error(`Job "${targetJobName}" not found in plan`); } if (job.status !== 'failed' && job.status !== 'conflict' && job.status !== 'needs_rebase') { - throw new Error(`Job "${args.retry}" is not in a retryable state (current: ${job.status}). Only failed, conflict, or needs_rebase jobs can be retried.`); + throw new Error(`Job "${targetJobName}" is not in a retryable state (current: ${job.status}). Only failed, conflict, or needs_rebase jobs can be retried.`); } } if (plan.status === 'paused' && plan.checkpoint) { const checkpoint = (args.checkpoint ?? plan.checkpoint) as CheckpointType; + const config = await loadConfig(); + + if (args.relaunch) { + const ctx = plan.checkpointContext; + if (!ctx || ctx.failureKind !== 'touchset') { + throw new Error(`Job "${args.relaunch}" was not failed due to a touchSet violation — use "retry" instead.`); + } + if (ctx.jobName !== args.relaunch) { + throw new Error(`Checkpoint was set for job "${ctx.jobName}", not "${args.relaunch}".`); + } + + plan.status = 'running'; + plan.checkpoint = null; + plan.checkpointContext = null; + await savePlan(plan); + + const orchestrator = new Orchestrator(getSharedMonitor(), config, { notify: getSharedNotifyCallback() ?? undefined }); + setSharedOrchestrator(orchestrator); + orchestrator.setPlanModelSnapshot(getCurrentModel()); + + await orchestrator.relaunchJobForCorrection( + args.relaunch, + ctx.touchSetViolations ?? [], + ctx.touchSetPatterns ?? [], + ); + await orchestrator.resumePlan(); + + return [ + `Checkpoint "${checkpoint}" cleared. Job "${args.relaunch}" relaunched with correction prompt.`, + '', + ` ID: ${plan.id}`, + ` Mode: ${plan.mode}`, + '', + 'The agent will fix touchSet violations in the existing worktree.', + 'Use mc_plan_status to monitor progress.', + ].join('\n'); + } if (args.retry) { - const job = plan.jobs.find(j => j.name === args.retry); - if (!job) { - throw new Error(`Job "${args.retry}" not found in plan`); + const job = plan.jobs.find(j => j.name === args.retry)!; + + const ctx = plan.checkpointContext; + if (ctx?.failureKind === 'touchset' && ctx.jobName === args.retry) { + if (job.touchSet && job.touchSet.length > 0 && job.branch && plan.integrationBranch) { + const validation = await validateTouchSet(job.branch, plan.integrationBranch, job.touchSet); + if (!validation.valid && validation.violations) { + throw new Error( + `Job "${args.retry}" still has touchSet violations after manual fix:\n` + + ` Violations: ${validation.violations.join(', ')}\n` + + ` Allowed: ${job.touchSet.join(', ')}\n` + + `Fix the remaining violations and retry again.`, + ); + } + } } - if (job.status !== 'failed' && job.status !== 'conflict' && job.status !== 'needs_rebase') { - throw new Error(`Job "${args.retry}" is not in a retryable state (current: ${job.status}). Only failed, conflict, or needs_rebase jobs can be retried.`); + + const retryJob = plan.jobs.find(j => j.name === args.retry)!; + retryJob.status = 'ready_to_merge'; + retryJob.error = undefined; + + plan.status = 'running'; + plan.checkpoint = null; + plan.checkpointContext = null; + await savePlan(plan); + + const orchestrator = new Orchestrator(getSharedMonitor(), config, { notify: getSharedNotifyCallback() ?? undefined }); + setSharedOrchestrator(orchestrator); + orchestrator.setPlanModelSnapshot(getCurrentModel()); + await orchestrator.resumePlan(); + + return [ + `Checkpoint "${checkpoint}" cleared. Job "${args.retry}" reset to ready_to_merge.`, + '', + ` ID: ${plan.id}`, + ` Mode: ${plan.mode}`, + '', + 'Use mc_plan_status to monitor progress.', + ].join('\n'); + } + + const ctx = plan.checkpointContext; + if (ctx?.failureKind === 'touchset') { + const job = plan.jobs.find(j => j.name === ctx.jobName); + if (job && job.status === 'failed') { + job.status = 'ready_to_merge'; + job.error = undefined; } - await updatePlanJob(plan.id, args.retry, { status: 'ready_to_merge', error: undefined }); } plan.status = 'running'; plan.checkpoint = null; + plan.checkpointContext = null; await savePlan(plan); - const config = await loadConfig(); const orchestrator = new Orchestrator(getSharedMonitor(), config, { notify: getSharedNotifyCallback() ?? undefined }); setSharedOrchestrator(orchestrator); orchestrator.setPlanModelSnapshot(getCurrentModel()); await orchestrator.resumePlan(); - const retryMsg = args.retry ? ` Job "${args.retry}" reset to ready_to_merge.` : ''; + const acceptMsg = ctx?.failureKind === 'touchset' + ? ` TouchSet violations for job "${ctx.jobName}" accepted.` + : ''; return [ - `Checkpoint "${checkpoint}" cleared.${retryMsg} Plan "${plan.name}" resuming.`, + `Checkpoint "${checkpoint}" cleared.${acceptMsg} Plan "${plan.name}" resuming.`, '', ` ID: ${plan.id}`, ` Mode: ${plan.mode}`, @@ -78,7 +167,6 @@ export const mc_plan_approve: ToolDefinition = tool({ ); } - // Create integration infrastructure that copilot mode skipped const config = await loadConfig(); const integrationPostCreate = resolvePostCreateHook(config.worktreeSetup); const integration = plan.baseBranch diff --git a/tests/lib/orchestrator.test.ts b/tests/lib/orchestrator.test.ts index daf6f35..d8c17a6 100644 --- a/tests/lib/orchestrator.test.ts +++ b/tests/lib/orchestrator.test.ts @@ -636,6 +636,14 @@ describe('orchestrator', () => { ); expect(planState?.status).toBe('paused'); expect(planState?.checkpoint).toBe('on_error'); + expect(planState?.checkpointContext).toEqual( + expect.objectContaining({ + jobName: 'touch-violator', + failureKind: 'touchset', + touchSetViolations: expect.arrayContaining(['README.md']), + touchSetPatterns: expect.arrayContaining(['src/**']), + }), + ); }); it('should allow transition to ready_to_merge when touchSet is satisfied', async () => { diff --git a/tests/tools/plan-approve.test.ts b/tests/tools/plan-approve.test.ts index f479a0f..bb6596a 100644 --- a/tests/tools/plan-approve.test.ts +++ b/tests/tools/plan-approve.test.ts @@ -3,6 +3,7 @@ import * as planState from '../../src/lib/plan-state'; import * as orchestrator from '../../src/lib/orchestrator'; import * as config from '../../src/lib/config'; import * as integration from '../../src/lib/integration'; +import * as mergeTrain from '../../src/lib/merge-train'; const { mc_plan_approve } = await import('../../src/tools/plan-approve'); @@ -65,6 +66,12 @@ describe('mc_plan_approve', () => { }); describe('retry validation', () => { + it('should reject when both retry and relaunch are provided', async () => { + expect( + mc_plan_approve.execute({ checkpoint: 'on_error', retry: 'bad-job', relaunch: 'bad-job' }, mockContext), + ).rejects.toThrow('Cannot specify both "retry" and "relaunch"'); + }); + it('should reset a failed job to ready_to_merge when retry is provided', async () => { spyOn(planState, 'loadPlan').mockResolvedValue({ id: 'plan-1', @@ -82,21 +89,224 @@ describe('mc_plan_approve', () => { }); const mockSavePlan = spyOn(planState, 'savePlan').mockResolvedValue(undefined); - const mockUpdatePlanJob = spyOn(planState, 'updatePlanJob').mockResolvedValue(undefined); const mockResumePlan = mock().mockResolvedValue(undefined); spyOn(orchestrator.Orchestrator.prototype, 'resumePlan').mockImplementation(mockResumePlan); spyOn(orchestrator.Orchestrator.prototype, 'setPlanModelSnapshot').mockImplementation(() => {}); const result = await mc_plan_approve.execute({ checkpoint: 'on_error', retry: 'bad-job' }, mockContext); - expect(mockUpdatePlanJob).toHaveBeenCalledWith('plan-1', 'bad-job', { status: 'ready_to_merge', error: undefined }); + expect(mockSavePlan).toHaveBeenCalledWith(expect.objectContaining({ + status: 'running', + jobs: expect.arrayContaining([ + expect.objectContaining({ name: 'bad-job', status: 'ready_to_merge' }), + ]), + })); expect(result).toContain('bad-job'); expect(result).toContain('ready_to_merge'); - expect(result).toContain('resuming'); - expect(mockSavePlan).toHaveBeenCalled(); expect(mockResumePlan).toHaveBeenCalled(); }); + it('should accept touchSet violations and move failed job to ready_to_merge', async () => { + spyOn(planState, 'loadPlan').mockResolvedValue({ + id: 'plan-1', + name: 'TouchSet Plan', + mode: 'supervisor', + status: 'paused', + checkpoint: 'on_error', + checkpointContext: { + jobName: 'touch-job', + failureKind: 'touchset', + touchSetViolations: ['README.md'], + touchSetPatterns: ['src/**'], + }, + jobs: [ + { id: 'j1', name: 'touch-job', prompt: 'fix files', status: 'failed', error: 'touchSet violation' }, + ], + integrationBranch: 'mc/integration/plan-1', + baseCommit: 'abc123', + createdAt: new Date().toISOString(), + }); + + const mockSavePlan = spyOn(planState, 'savePlan').mockResolvedValue(undefined); + const mockResumePlan = mock().mockResolvedValue(undefined); + spyOn(orchestrator.Orchestrator.prototype, 'resumePlan').mockImplementation(mockResumePlan); + spyOn(orchestrator.Orchestrator.prototype, 'setPlanModelSnapshot').mockImplementation(() => {}); + + const result = await mc_plan_approve.execute({ checkpoint: 'on_error' }, mockContext); + + expect(mockSavePlan).toHaveBeenCalledWith(expect.objectContaining({ + status: 'running', + checkpoint: null, + checkpointContext: null, + jobs: expect.arrayContaining([ + expect.objectContaining({ name: 'touch-job', status: 'ready_to_merge' }), + ]), + })); + expect(mockResumePlan).toHaveBeenCalled(); + expect(result).toContain('TouchSet violations for job "touch-job" accepted'); + }); + + it('should relaunch touchSet-failed job for correction', async () => { + spyOn(planState, 'loadPlan').mockResolvedValue({ + id: 'plan-1', + name: 'TouchSet Relaunch Plan', + mode: 'supervisor', + status: 'paused', + checkpoint: 'on_error', + checkpointContext: { + jobName: 'touch-job', + failureKind: 'touchset', + touchSetViolations: ['README.md'], + touchSetPatterns: ['src/**'], + }, + jobs: [ + { + id: 'j1', + name: 'touch-job', + prompt: 'fix files', + status: 'failed', + touchSet: ['src/**'], + branch: 'mc/plan/plan-1/touch-job', + }, + ], + integrationBranch: 'mc/integration/plan-1', + baseCommit: 'abc123', + createdAt: new Date().toISOString(), + }); + + const mockSavePlan = spyOn(planState, 'savePlan').mockResolvedValue(undefined); + const mockResumePlan = mock().mockResolvedValue(undefined); + const relaunchSpy = mock().mockResolvedValue(undefined); + spyOn(orchestrator.Orchestrator.prototype, 'resumePlan').mockImplementation(mockResumePlan); + spyOn(orchestrator.Orchestrator.prototype, 'relaunchJobForCorrection').mockImplementation(relaunchSpy); + spyOn(orchestrator.Orchestrator.prototype, 'setPlanModelSnapshot').mockImplementation(() => {}); + + const result = await mc_plan_approve.execute({ checkpoint: 'on_error', relaunch: 'touch-job' }, mockContext); + + expect(mockSavePlan).toHaveBeenCalledWith(expect.objectContaining({ + status: 'running', + checkpoint: null, + checkpointContext: null, + })); + expect(relaunchSpy).toHaveBeenCalledWith('touch-job', ['README.md'], ['src/**']); + expect(mockResumePlan).toHaveBeenCalled(); + expect(result).toContain('relaunched with correction prompt'); + }); + + it('should re-validate touchSet on retry before proceeding', async () => { + spyOn(planState, 'loadPlan').mockResolvedValue({ + id: 'plan-1', + name: 'TouchSet Retry Plan', + mode: 'supervisor', + status: 'paused', + checkpoint: 'on_error', + checkpointContext: { + jobName: 'touch-job', + failureKind: 'touchset', + touchSetViolations: ['README.md'], + touchSetPatterns: ['src/**'], + }, + jobs: [ + { + id: 'j1', + name: 'touch-job', + prompt: 'fix files', + status: 'failed', + touchSet: ['src/**'], + branch: 'mc/plan/plan-1/touch-job', + }, + ], + integrationBranch: 'mc/integration/plan-1', + baseCommit: 'abc123', + createdAt: new Date().toISOString(), + }); + + const validateSpy = spyOn(mergeTrain, 'validateTouchSet').mockResolvedValue({ + valid: true, + changedFiles: ['src/main.ts'], + }); + const mockSavePlan = spyOn(planState, 'savePlan').mockResolvedValue(undefined); + const mockResumePlan = mock().mockResolvedValue(undefined); + spyOn(orchestrator.Orchestrator.prototype, 'resumePlan').mockImplementation(mockResumePlan); + spyOn(orchestrator.Orchestrator.prototype, 'setPlanModelSnapshot').mockImplementation(() => {}); + + const result = await mc_plan_approve.execute({ checkpoint: 'on_error', retry: 'touch-job' }, mockContext); + + expect(validateSpy).toHaveBeenCalledWith('mc/plan/plan-1/touch-job', 'mc/integration/plan-1', ['src/**']); + expect(mockSavePlan).toHaveBeenCalledWith(expect.objectContaining({ + status: 'running', + jobs: expect.arrayContaining([ + expect.objectContaining({ name: 'touch-job', status: 'ready_to_merge' }), + ]), + })); + expect(mockResumePlan).toHaveBeenCalled(); + expect(result).toContain('touch-job'); + expect(result).toContain('ready_to_merge'); + }); + + it('should reject retry when touchSet remains violated after manual fix', async () => { + spyOn(planState, 'loadPlan').mockResolvedValue({ + id: 'plan-1', + name: 'TouchSet Retry Plan', + mode: 'supervisor', + status: 'paused', + checkpoint: 'on_error', + checkpointContext: { + jobName: 'touch-job', + failureKind: 'touchset', + touchSetViolations: ['README.md'], + touchSetPatterns: ['src/**'], + }, + jobs: [ + { + id: 'j1', + name: 'touch-job', + prompt: 'fix files', + status: 'failed', + touchSet: ['src/**'], + branch: 'mc/plan/plan-1/touch-job', + }, + ], + integrationBranch: 'mc/integration/plan-1', + baseCommit: 'abc123', + createdAt: new Date().toISOString(), + }); + + spyOn(mergeTrain, 'validateTouchSet').mockResolvedValue({ + valid: false, + violations: ['README.md'], + changedFiles: ['src/main.ts', 'README.md'], + }); + + expect( + mc_plan_approve.execute({ checkpoint: 'on_error', retry: 'touch-job' }, mockContext), + ).rejects.toThrow('still has touchSet violations after manual fix'); + }); + + it('should reject relaunch when failure is not touchSet-related', async () => { + spyOn(planState, 'loadPlan').mockResolvedValue({ + id: 'plan-1', + name: 'Non TouchSet Plan', + mode: 'supervisor', + status: 'paused', + checkpoint: 'on_error', + checkpointContext: { + jobName: 'bad-job', + failureKind: 'test_failure', + }, + jobs: [ + { id: 'j1', name: 'bad-job', prompt: 'fix', status: 'failed', error: 'tests failed' }, + ], + integrationBranch: 'mc/integration/plan-1', + baseCommit: 'abc123', + createdAt: new Date().toISOString(), + }); + + expect( + mc_plan_approve.execute({ checkpoint: 'on_error', relaunch: 'bad-job' }, mockContext), + ).rejects.toThrow('was not failed due to a touchSet violation'); + }); + it('should allow retry of needs_rebase jobs', async () => { spyOn(planState, 'loadPlan').mockResolvedValue({ id: 'plan-1', @@ -112,7 +322,6 @@ describe('mc_plan_approve', () => { createdAt: new Date().toISOString(), }); - const mockUpdatePlanJob = spyOn(planState, 'updatePlanJob').mockResolvedValue(undefined); const mockSavePlan = spyOn(planState, 'savePlan').mockResolvedValue(undefined); const mockResumePlan = mock().mockResolvedValue(undefined); spyOn(orchestrator.Orchestrator.prototype, 'resumePlan').mockImplementation(mockResumePlan); @@ -120,10 +329,14 @@ describe('mc_plan_approve', () => { const result = await mc_plan_approve.execute({ checkpoint: 'on_error', retry: 'conflicting-job' }, mockContext); - expect(mockUpdatePlanJob).toHaveBeenCalledWith('plan-1', 'conflicting-job', { status: 'ready_to_merge', error: undefined }); + expect(mockSavePlan).toHaveBeenCalledWith(expect.objectContaining({ + status: 'running', + jobs: expect.arrayContaining([ + expect.objectContaining({ name: 'conflicting-job', status: 'ready_to_merge' }), + ]), + })); expect(result).toContain('Checkpoint'); - expect(result).toContain('resuming'); - expect(mockSavePlan).toHaveBeenCalled(); + expect(result).toContain('ready_to_merge'); expect(mockResumePlan).toHaveBeenCalled(); });