fix: refresh orchestration follow-up after #414 (#430)

commands/orchestrate.md

@@ -1,3 +1,7 @@
+---
+description: Sequential and tmux/worktree orchestration guidance for multi-agent workflows.
+---
+
 # Orchestrate Command
 
 Sequential agent workflow for complex tasks.

commands/sessions.md

@@ -1,3 +1,7 @@
+---
+description: Manage Claude Code session history, aliases, and session metadata.
+---
+
 # Sessions Command
 
 Manage Claude Code session history - list, load, alias, and edit sessions stored in `~/.claude/sessions/`.
@@ -255,11 +259,6 @@ Show all session aliases.
 /sessions aliases                      # List all aliases
 ```
 
-## Operator Notes
-
-- Session files persist `Project`, `Branch`, and `Worktree` in the header so `/sessions info` can disambiguate parallel tmux/worktree runs.
-- For command-center style monitoring, combine `/sessions info`, `git diff --stat`, and the cost metrics emitted by `scripts/hooks/cost-tracker.js`.
-
 **Script:**
 ```bash
 node -e "
@@ -284,6 +283,11 @@ if (aliases.length === 0) {
 "
 ```
 
+## Operator Notes
+
+- Session files persist `Project`, `Branch`, and `Worktree` in the header so `/sessions info` can disambiguate parallel tmux/worktree runs.
+- For command-center style monitoring, combine `/sessions info`, `git diff --stat`, and the cost metrics emitted by `scripts/hooks/cost-tracker.js`.
+
 ## Arguments
 
 $ARGUMENTS:

scripts/lib/tmux-worktree-orchestrator.js

@@ -34,6 +34,22 @@ function formatCommand(program, args) {
   return [program, ...args.map(shellQuote)].join(' ');
 }
 
+function buildTemplateVariables(values) {
+  return Object.entries(values).reduce((accumulator, [key, value]) => {
+    const stringValue = String(value);
+    const quotedValue = shellQuote(stringValue);
+
+    accumulator[key] = stringValue;
+    accumulator[`${key}_raw`] = stringValue;
+    accumulator[`${key}_sh`] = quotedValue;
+    return accumulator;
+  }, {});
+}
+
+function buildSessionBannerCommand(sessionName, coordinationDir) {
+  return `printf '%s\\n' ${shellQuote(`Session: ${sessionName}`)} ${shellQuote(`Coordination: ${coordinationDir}`)}`;
+}
+
 function normalizeSeedPaths(seedPaths, repoRoot) {
   const resolvedRepoRoot = path.resolve(repoRoot);
   const entries = Array.isArray(seedPaths) ? seedPaths : [];
@@ -239,7 +255,7 @@ function buildOrchestrationPlan(config = {}) {
         'send-keys',
         '-t',
         sessionName,
-        `printf '%s\\n' 'Session: ${sessionName}' 'Coordination: ${coordinationDir}'`,
+        buildSessionBannerCommand(sessionName, coordinationDir),
         'C-m'
       ],
       description: 'Print orchestrator session details'
@@ -400,14 +416,82 @@ function cleanupExisting(plan) {
   }
 }
 
-function executePlan(plan) {
+function rollbackCreatedResources(plan, createdState, runtime = {}) {
-  runCommand('git', ['rev-parse', '--is-inside-work-tree'], { cwd: plan.repoRoot });
+  const runCommandImpl = runtime.runCommand || runCommand;
-  runCommand('tmux', ['-V']);
+  const listWorktreesImpl = runtime.listWorktrees || listWorktrees;
+  const branchExistsImpl = runtime.branchExists || branchExists;
+  const errors = [];
+
+  if (createdState.sessionCreated) {
+    try {
+      runCommandImpl('tmux', ['kill-session', '-t', plan.sessionName], { cwd: plan.repoRoot });
+    } catch (error) {
+      errors.push(error.message);
+    }
+  }
+
+  for (const workerPlan of [...createdState.workerPlans].reverse()) {
+    const expectedWorktreePath = canonicalizePath(workerPlan.worktreePath);
+    const existingWorktree = listWorktreesImpl(plan.repoRoot).find(
+      worktree => worktree.canonicalPath === expectedWorktreePath
+    );
+
+    if (existingWorktree) {
+      try {
+        runCommandImpl('git', ['worktree', 'remove', '--force', existingWorktree.listedPath], {
+          cwd: plan.repoRoot
+        });
+      } catch (error) {
+        errors.push(error.message);
+      }
+    } else if (fs.existsSync(workerPlan.worktreePath)) {
+      fs.rmSync(workerPlan.worktreePath, { force: true, recursive: true });
+    }
+
+    try {
+      runCommandImpl('git', ['worktree', 'prune', '--expire', 'now'], { cwd: plan.repoRoot });
+    } catch (error) {
+      errors.push(error.message);
+    }
+
+    if (branchExistsImpl(plan.repoRoot, workerPlan.branchName)) {
+      try {
+        runCommandImpl('git', ['branch', '-D', workerPlan.branchName], { cwd: plan.repoRoot });
+      } catch (error) {
+        errors.push(error.message);
+      }
+    }
+  }
+
+  if (createdState.removeCoordinationDir && fs.existsSync(plan.coordinationDir)) {
+    fs.rmSync(plan.coordinationDir, { force: true, recursive: true });
+  }
+
+  if (errors.length > 0) {
+    throw new Error(`rollback failed: ${errors.join('; ')}`);
+  }
+}
+
+function executePlan(plan, runtime = {}) {
+  const spawnSyncImpl = runtime.spawnSync || spawnSync;
+  const runCommandImpl = runtime.runCommand || runCommand;
+  const materializePlanImpl = runtime.materializePlan || materializePlan;
+  const overlaySeedPathsImpl = runtime.overlaySeedPaths || overlaySeedPaths;
+  const cleanupExistingImpl = runtime.cleanupExisting || cleanupExisting;
+  const rollbackCreatedResourcesImpl = runtime.rollbackCreatedResources || rollbackCreatedResources;
+  const createdState = {
+    workerPlans: [],
+    sessionCreated: false,
+    removeCoordinationDir: !fs.existsSync(plan.coordinationDir)
+  };
+
+  runCommandImpl('git', ['rev-parse', '--is-inside-work-tree'], { cwd: plan.repoRoot });
+  runCommandImpl('tmux', ['-V']);
 
   if (plan.replaceExisting) {
-    cleanupExisting(plan);
+    cleanupExistingImpl(plan);
   } else {
-    const hasSession = spawnSync('tmux', ['has-session', '-t', plan.sessionName], {
+    const hasSession = spawnSyncImpl('tmux', ['has-session', '-t', plan.sessionName], {
       encoding: 'utf8',
       stdio: ['ignore', 'pipe', 'pipe']
     });
@@ -416,36 +500,39 @@ function executePlan(plan) {
     }
   }
 
-  materializePlan(plan);
+  try {
+    materializePlanImpl(plan);
 
     for (const workerPlan of plan.workerPlans) {
-    runCommand('git', workerPlan.gitArgs, { cwd: plan.repoRoot });
+      runCommandImpl('git', workerPlan.gitArgs, { cwd: plan.repoRoot });
-    overlaySeedPaths({
+      createdState.workerPlans.push(workerPlan);
+      overlaySeedPathsImpl({
         repoRoot: plan.repoRoot,
         seedPaths: workerPlan.seedPaths,
         worktreePath: workerPlan.worktreePath
       });
     }
 
-  runCommand(
+    runCommandImpl(
       'tmux',
       ['new-session', '-d', '-s', plan.sessionName, '-n', 'orchestrator', '-c', plan.repoRoot],
       { cwd: plan.repoRoot }
     );
-  runCommand(
+    createdState.sessionCreated = true;
+    runCommandImpl(
       'tmux',
       [
         'send-keys',
         '-t',
         plan.sessionName,
-      `printf '%s\\n' 'Session: ${plan.sessionName}' 'Coordination: ${plan.coordinationDir}'`,
+        buildSessionBannerCommand(plan.sessionName, plan.coordinationDir),
         'C-m'
       ],
       { cwd: plan.repoRoot }
     );
 
     for (const workerPlan of plan.workerPlans) {
-    const splitResult = runCommand(
+      const splitResult = runCommandImpl(
         'tmux',
         ['split-window', '-d', '-P', '-F', '#{pane_id}', '-t', plan.sessionName, '-c', workerPlan.worktreePath],
         { cwd: plan.repoRoot }
@@ -456,11 +543,11 @@ function executePlan(plan) {
         throw new Error(`tmux split-window did not return a pane id for ${workerPlan.workerName}`);
       }
 
-    runCommand('tmux', ['select-layout', '-t', plan.sessionName, 'tiled'], { cwd: plan.repoRoot });
+      runCommandImpl('tmux', ['select-layout', '-t', plan.sessionName, 'tiled'], { cwd: plan.repoRoot });
-    runCommand('tmux', ['select-pane', '-t', paneId, '-T', workerPlan.workerSlug], {
+      runCommandImpl('tmux', ['select-pane', '-t', paneId, '-T', workerPlan.workerSlug], {
         cwd: plan.repoRoot
       });
-    runCommand(
+      runCommandImpl(
         'tmux',
         [
           'send-keys',
@@ -472,6 +559,18 @@ function executePlan(plan) {
         { cwd: plan.repoRoot }
       );
     }
+  } catch (error) {
+    try {
+      rollbackCreatedResourcesImpl(plan, createdState, {
+        branchExists: runtime.branchExists,
+        listWorktrees: runtime.listWorktrees,
+        runCommand: runCommandImpl
+      });
+    } catch (cleanupError) {
+      error.message = `${error.message}; cleanup failed: ${cleanupError.message}`;
+    }
+    throw error;
+  }
 
   return {
     coordinationDir: plan.coordinationDir,
@@ -486,6 +585,7 @@ module.exports = {
   materializePlan,
   normalizeSeedPaths,
   overlaySeedPaths,
+  rollbackCreatedResources,
   renderTemplate,
   slugify
 };

tests/lib/tmux-worktree-orchestrator.test.js

@@ -9,6 +9,7 @@ const {
   slugify,
   renderTemplate,
   buildOrchestrationPlan,
+  executePlan,
   materializePlan,
   normalizeSeedPaths,
   overlaySeedPaths
@@ -137,6 +138,64 @@ test('buildOrchestrationPlan normalizes global and worker seed paths', () => {
   ]);
 });
 
+test('buildOrchestrationPlan rejects worker names that collapse to the same slug', () => {
+  assert.throws(
+    () => buildOrchestrationPlan({
+      repoRoot: '/tmp/ecc',
+      sessionName: 'duplicates',
+      launcherCommand: 'echo run',
+      workers: [
+        { name: 'Docs A', task: 'Fix skill docs' },
+        { name: 'Docs/A', task: 'Fix tests' }
+      ]
+    }),
+    /unique slugs/
+  );
+});
+
+test('buildOrchestrationPlan exposes shell-safe launcher aliases alongside raw defaults', () => {
+  const repoRoot = path.join('/tmp', 'My Repo');
+  const plan = buildOrchestrationPlan({
+    repoRoot,
+    sessionName: 'Spacing Audit',
+    launcherCommand: 'bash {repo_root_sh}/scripts/orchestrate-codex-worker.sh {task_file_sh} {handoff_file_sh} {status_file_sh} {worker_name_sh} {worker_name}',
+    workers: [{ name: 'Docs Fixer', task: 'Update docs' }]
+  });
+  const quote = value => `'${String(value).replace(/'/g, `'\\''`)}'`;
+  const resolvedRepoRoot = plan.workerPlans[0].repoRoot;
+
+  assert.ok(
+    plan.workerPlans[0].launchCommand.includes(`bash ${quote(resolvedRepoRoot)}/scripts/orchestrate-codex-worker.sh`),
+    'repo_root_sh should provide a shell-safe path'
+  );
+  assert.ok(
+    plan.workerPlans[0].launchCommand.includes(quote(plan.workerPlans[0].taskFilePath)),
+    'task_file_sh should provide a shell-safe path'
+  );
+  assert.ok(
+    plan.workerPlans[0].launchCommand.includes(`${quote(plan.workerPlans[0].workerName)} ${plan.workerPlans[0].workerName}`),
+    'raw defaults should remain available alongside shell-safe aliases'
+  );
+});
+
+test('buildOrchestrationPlan shell-quotes the orchestration banner command', () => {
+  const repoRoot = path.join('/tmp', "O'Hare Repo");
+  const plan = buildOrchestrationPlan({
+    repoRoot,
+    sessionName: 'Quote Audit',
+    launcherCommand: 'echo run',
+    workers: [{ name: 'Docs', task: 'Update docs' }]
+  });
+  const quote = value => `'${String(value).replace(/'/g, `'\\''`)}'`;
+  const bannerCommand = plan.tmuxCommands[1].args[3];
+
+  assert.strictEqual(
+    bannerCommand,
+    `printf '%s\\n' ${quote(`Session: ${plan.sessionName}`)} ${quote(`Coordination: ${plan.coordinationDir}`)}`,
+    'Banner command should quote coordination paths safely for tmux send-keys'
+  );
+});
+
 test('normalizeSeedPaths rejects paths outside the repo root', () => {
   assert.throws(
     () => normalizeSeedPaths(['../outside.txt'], '/tmp/ecc'),
@@ -229,5 +288,136 @@ test('overlaySeedPaths copies local overlays into the worker worktree', () => {
   }
 });
 
+test('executePlan rolls back partial setup when orchestration fails mid-run', () => {
+  const plan = {
+    repoRoot: '/tmp/ecc',
+    sessionName: 'rollback-test',
+    coordinationDir: '/tmp/ecc/.orchestration/rollback-test',
+    replaceExisting: false,
+    workerPlans: [
+      {
+        workerName: 'Docs',
+        workerSlug: 'docs',
+        worktreePath: '/tmp/ecc-rollback-docs',
+        seedPaths: ['commands/orchestrate.md'],
+        gitArgs: ['worktree', 'add', '-b', 'orchestrator-rollback-test-docs', '/tmp/ecc-rollback-docs', 'HEAD'],
+        launchCommand: 'echo run'
+      }
+    ]
+  };
+  const calls = [];
+  const rollbackCalls = [];
+
+  assert.throws(
+    () => executePlan(plan, {
+      spawnSync(program, args) {
+        calls.push({ type: 'spawnSync', program, args });
+        if (program === 'tmux' && args[0] === 'has-session') {
+          return { status: 1, stdout: '', stderr: '' };
+        }
+        throw new Error(`Unexpected spawnSync call: ${program} ${args.join(' ')}`);
+      },
+      runCommand(program, args) {
+        calls.push({ type: 'runCommand', program, args });
+        if (program === 'git' && args[0] === 'rev-parse') {
+          return { status: 0, stdout: 'true\n', stderr: '' };
+        }
+        if (program === 'tmux' && args[0] === '-V') {
+          return { status: 0, stdout: 'tmux 3.4\n', stderr: '' };
+        }
+        if (program === 'git' && args[0] === 'worktree') {
+          return { status: 0, stdout: '', stderr: '' };
+        }
+        throw new Error(`Unexpected runCommand call: ${program} ${args.join(' ')}`);
+      },
+      materializePlan(receivedPlan) {
+        calls.push({ type: 'materializePlan', receivedPlan });
+      },
+      overlaySeedPaths() {
+        throw new Error('overlay failed');
+      },
+      rollbackCreatedResources(receivedPlan, createdState) {
+        rollbackCalls.push({ receivedPlan, createdState });
+      }
+    }),
+    /overlay failed/
+  );
+
+  assert.deepStrictEqual(
+    rollbackCalls.map(call => call.receivedPlan),
+    [plan],
+    'executePlan should invoke rollback on failure'
+  );
+  assert.deepStrictEqual(
+    rollbackCalls[0].createdState.workerPlans,
+    plan.workerPlans,
+    'executePlan should only roll back resources created before the failure'
+  );
+  assert.ok(
+    calls.some(call => call.type === 'runCommand' && call.program === 'git' && call.args[0] === 'worktree'),
+    'executePlan should attempt setup before rolling back'
+  );
+});
+
+test('executePlan does not mark pre-existing resources for rollback when worktree creation fails', () => {
+  const plan = {
+    repoRoot: '/tmp/ecc',
+    sessionName: 'rollback-existing',
+    coordinationDir: '/tmp/ecc/.orchestration/rollback-existing',
+    replaceExisting: false,
+    workerPlans: [
+      {
+        workerName: 'Docs',
+        workerSlug: 'docs',
+        worktreePath: '/tmp/ecc-existing-docs',
+        seedPaths: [],
+        gitArgs: ['worktree', 'add', '-b', 'orchestrator-rollback-existing-docs', '/tmp/ecc-existing-docs', 'HEAD'],
+        launchCommand: 'echo run',
+        branchName: 'orchestrator-rollback-existing-docs'
+      }
+    ]
+  };
+  const rollbackCalls = [];
+
+  assert.throws(
+    () => executePlan(plan, {
+      spawnSync(program, args) {
+        if (program === 'tmux' && args[0] === 'has-session') {
+          return { status: 1, stdout: '', stderr: '' };
+        }
+        throw new Error(`Unexpected spawnSync call: ${program} ${args.join(' ')}`);
+      },
+      runCommand(program, args) {
+        if (program === 'git' && args[0] === 'rev-parse') {
+          return { status: 0, stdout: 'true\n', stderr: '' };
+        }
+        if (program === 'tmux' && args[0] === '-V') {
+          return { status: 0, stdout: 'tmux 3.4\n', stderr: '' };
+        }
+        if (program === 'git' && args[0] === 'worktree') {
+          throw new Error('branch already exists');
+        }
+        throw new Error(`Unexpected runCommand call: ${program} ${args.join(' ')}`);
+      },
+      materializePlan() {},
+      rollbackCreatedResources(receivedPlan, createdState) {
+        rollbackCalls.push({ receivedPlan, createdState });
+      }
+    }),
+    /branch already exists/
+  );
+
+  assert.deepStrictEqual(
+    rollbackCalls[0].createdState.workerPlans,
+    [],
+    'Failures before creation should not schedule any worker resources for rollback'
+  );
+  assert.strictEqual(
+    rollbackCalls[0].createdState.sessionCreated,
+    false,
+    'Failures before tmux session creation should not mark a session for rollback'
+  );
+});
+
 console.log(`\n=== Results: ${passed} passed, ${failed} failed ===`);
 if (failed > 0) process.exit(1);

tests/scripts/orchestrate-codex-worker.test.js

@@ -0,0 +1,63 @@
+'use strict';
+
+const assert = require('assert');
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const { spawnSync } = require('child_process');
+
+const SCRIPT = path.join(__dirname, '..', '..', 'scripts', 'orchestrate-codex-worker.sh');
+
+console.log('=== Testing orchestrate-codex-worker.sh ===\n');
+
+let passed = 0;
+let failed = 0;
+
+function test(desc, fn) {
+  try {
+    fn();
+    console.log(`  ✓ ${desc}`);
+    passed++;
+  } catch (error) {
+    console.log(`  ✗ ${desc}: ${error.message}`);
+    failed++;
+  }
+}
+
+test('fails fast for an unreadable task file and records failure artifacts', () => {
+  const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-orch-worker-'));
+  const handoffFile = path.join(tempRoot, '.orchestration', 'docs', 'handoff.md');
+  const statusFile = path.join(tempRoot, '.orchestration', 'docs', 'status.md');
+  const missingTaskFile = path.join(tempRoot, '.orchestration', 'docs', 'task.md');
+
+  try {
+    spawnSync('git', ['init'], { cwd: tempRoot, stdio: 'ignore' });
+
+    const result = spawnSync('bash', [SCRIPT, missingTaskFile, handoffFile, statusFile], {
+      cwd: tempRoot,
+      encoding: 'utf8'
+    });
+
+    assert.notStrictEqual(result.status, 0, 'Script should fail when task file is unreadable');
+    assert.ok(fs.existsSync(statusFile), 'Script should still write a status file');
+    assert.ok(fs.existsSync(handoffFile), 'Script should still write a handoff file');
+
+    const statusContent = fs.readFileSync(statusFile, 'utf8');
+    const handoffContent = fs.readFileSync(handoffFile, 'utf8');
+
+    assert.ok(statusContent.includes('- State: failed'), 'Status file should record the failure state');
+    assert.ok(
+      statusContent.includes('task file is missing or unreadable'),
+      'Status file should explain the task-file failure'
+    );
+    assert.ok(
+      handoffContent.includes('Task file is missing or unreadable'),
+      'Handoff file should explain the task-file failure'
+    );
+  } finally {
+    fs.rmSync(tempRoot, { recursive: true, force: true });
+  }
+});
+
+console.log(`\n=== Results: ${passed} passed, ${failed} failed ===`);
+if (failed > 0) process.exit(1);