From 2d43541f0e6d7fc98fd63b96143180b353adae05 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Thu, 12 Mar 2026 15:40:25 -0700 Subject: [PATCH] fix: preserve orchestration launcher compatibility --- scripts/lib/orchestration-session.js | 7 ++++--- scripts/lib/tmux-worktree-orchestrator.js | 2 +- scripts/orchestrate-worktrees.js | 10 +++++----- tests/lib/orchestration-session.test.js | 21 +++++++++++++++----- tests/lib/tmux-worktree-orchestrator.test.js | 21 ++++++++++---------- 5 files changed, 37 insertions(+), 24 deletions(-) diff --git a/scripts/lib/orchestration-session.js b/scripts/lib/orchestration-session.js index 292c7010..27a29555 100644 --- a/scripts/lib/orchestration-session.js +++ b/scripts/lib/orchestration-session.js @@ -293,11 +293,12 @@ function readPlanString(config, key, absoluteTarget) { return undefined; } - if (typeof value !== 'string' || value.trim().length === 0) { - throw new Error(`Invalid orchestration plan: ${key} must be a non-empty string (${absoluteTarget})`); + if (typeof value !== 'string') { + throw new Error(`Invalid orchestration plan: ${key} must be a string when provided (${absoluteTarget})`); } - return value.trim(); + const normalized = value.trim(); + return normalized.length > 0 ? normalized : undefined; } function resolveSnapshotTarget(targetPath, cwd = process.cwd()) { diff --git a/scripts/lib/tmux-worktree-orchestrator.js b/scripts/lib/tmux-worktree-orchestrator.js index 0aa89c97..f0c2620f 100644 --- a/scripts/lib/tmux-worktree-orchestrator.js +++ b/scripts/lib/tmux-worktree-orchestrator.js @@ -39,7 +39,7 @@ function buildTemplateVariables(values) { const stringValue = String(value); const quotedValue = shellQuote(stringValue); - accumulator[key] = quotedValue; + accumulator[key] = stringValue; accumulator[`${key}_raw`] = stringValue; accumulator[`${key}_sh`] = quotedValue; return accumulator; diff --git a/scripts/orchestrate-worktrees.js b/scripts/orchestrate-worktrees.js index c6af18d0..fc61b022 100644 --- a/scripts/orchestrate-worktrees.js +++ b/scripts/orchestrate-worktrees.js @@ -17,11 +17,11 @@ function usage() { ' node scripts/orchestrate-worktrees.js [--write-only]', '', 'Placeholders supported in launcherCommand:', - ' Shell-safe defaults: {worker_name} {worker_slug} {session_name} {repo_root}', - ' Shell-safe defaults: {worktree_path} {branch_name} {task_file} {handoff_file} {status_file}', - ' Raw variants: {worker_name_raw} {worker_slug_raw} {session_name_raw} {repo_root_raw}', - ' Raw variants: {worktree_path_raw} {branch_name_raw} {task_file_raw} {handoff_file_raw} {status_file_raw}', - ' Explicit shell-safe aliases also exist with the _sh suffix.', + ' Raw defaults: {worker_name} {worker_slug} {session_name} {repo_root}', + ' Raw defaults: {worktree_path} {branch_name} {task_file} {handoff_file} {status_file}', + ' Shell-safe aliases: {worker_name_sh} {worker_slug_sh} {session_name_sh} {repo_root_sh}', + ' Shell-safe aliases: {worktree_path_sh} {branch_name_sh} {task_file_sh} {handoff_file_sh} {status_file_sh}', + ' Explicit raw aliases also exist with the _raw suffix.', '', 'Without flags the script prints a dry-run plan only.' ].join('\n')); diff --git a/tests/lib/orchestration-session.test.js b/tests/lib/orchestration-session.test.js index be196b7b..ebcbf587 100644 --- a/tests/lib/orchestration-session.test.js +++ b/tests/lib/orchestration-session.test.js @@ -262,13 +262,19 @@ test('resolveSnapshotTarget rejects malformed plan files and invalid config fiel fs.mkdirSync(repoRoot, { recursive: true }); const invalidJsonPath = path.join(repoRoot, 'invalid-json.json'); + const blankFieldsPath = path.join(repoRoot, 'blank-fields.json'); const invalidSessionNamePath = path.join(repoRoot, 'invalid-session.json'); const invalidRepoRootPath = path.join(repoRoot, 'invalid-repo-root.json'); const invalidCoordinationRootPath = path.join(repoRoot, 'invalid-coordination-root.json'); fs.writeFileSync(invalidJsonPath, '{not valid json'); + fs.writeFileSync(blankFieldsPath, JSON.stringify({ + sessionName: ' ', + repoRoot: ' ', + coordinationRoot: ' ' + })); fs.writeFileSync(invalidSessionNamePath, JSON.stringify({ - sessionName: '', + sessionName: 42, repoRoot })); fs.writeFileSync(invalidRepoRootPath, JSON.stringify({ @@ -278,25 +284,30 @@ test('resolveSnapshotTarget rejects malformed plan files and invalid config fiel fs.writeFileSync(invalidCoordinationRootPath, JSON.stringify({ sessionName: 'workflow', repoRoot, - coordinationRoot: ' ' + coordinationRoot: false })); try { + const blankFields = resolveSnapshotTarget(blankFieldsPath, repoRoot); + assert.strictEqual(blankFields.sessionName, 'repo'); + assert.strictEqual(blankFields.repoRoot, repoRoot); + assert.ok(blankFields.coordinationDir.endsWith(path.join('.orchestration', 'repo'))); + assert.throws( () => resolveSnapshotTarget(invalidJsonPath, repoRoot), /Invalid orchestration plan JSON/ ); assert.throws( () => resolveSnapshotTarget(invalidSessionNamePath, repoRoot), - /sessionName must be a non-empty string/ + /sessionName must be a string when provided/ ); assert.throws( () => resolveSnapshotTarget(invalidRepoRootPath, repoRoot), - /repoRoot must be a non-empty string/ + /repoRoot must be a string when provided/ ); assert.throws( () => resolveSnapshotTarget(invalidCoordinationRootPath, repoRoot), - /coordinationRoot must be a non-empty string/ + /coordinationRoot must be a string when provided/ ); } finally { fs.rmSync(tempRoot, { recursive: true, force: true }); diff --git a/tests/lib/tmux-worktree-orchestrator.test.js b/tests/lib/tmux-worktree-orchestrator.test.js index cedaf085..e4ee3204 100644 --- a/tests/lib/tmux-worktree-orchestrator.test.js +++ b/tests/lib/tmux-worktree-orchestrator.test.js @@ -57,7 +57,7 @@ test('buildOrchestrationPlan creates worktrees, branches, and tmux commands', () repoRoot, sessionName: 'Skill Audit', baseRef: 'main', - launcherCommand: 'codex exec --cwd {worktree_path} --task-file {task_file}', + launcherCommand: 'codex exec --cwd {worktree_path_sh} --task-file {task_file_sh}', workers: [ { name: 'Docs A', task: 'Fix skills 1-4' }, { name: 'Docs B', task: 'Fix skills 5-8' } @@ -152,26 +152,27 @@ test('buildOrchestrationPlan rejects worker names that collapse to the same slug ); }); -test('buildOrchestrationPlan exposes shell-safe launcher placeholders with raw aliases', () => { +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}/scripts/orchestrate-codex-worker.sh {task_file} {handoff_file} {status_file} {worker_name} {worker_name_raw}', + 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, `'\\''`)}'`; assert.ok( - plan.workerPlans[0].launchCommand.includes(`bash '${repoRoot}'/scripts/orchestrate-codex-worker.sh`), - 'repo_root should be shell-safe by default' + plan.workerPlans[0].launchCommand.includes(`bash ${quote(repoRoot)}/scripts/orchestrate-codex-worker.sh`), + 'repo_root_sh should provide a shell-safe path' ); assert.ok( - plan.workerPlans[0].launchCommand.includes(`'${plan.workerPlans[0].taskFilePath}'`), - 'task_file should be shell-safe by default' + 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(`'${plan.workerPlans[0].workerName}' ${plan.workerPlans[0].workerName}`), - 'worker_name_raw should preserve the unquoted value when explicitly requested' + plan.workerPlans[0].launchCommand.includes(`${quote(plan.workerPlans[0].workerName)} ${plan.workerPlans[0].workerName}`), + 'raw defaults should remain available alongside shell-safe aliases' ); }); @@ -201,7 +202,7 @@ test('materializePlan keeps worker instructions inside the worktree boundary', ( repoRoot: tempRoot, coordinationRoot: path.join(tempRoot, '.claude', 'orchestration'), sessionName: 'Workflow E2E', - launcherCommand: 'bash {repo_root}/scripts/orchestrate-codex-worker.sh {task_file} {handoff_file} {status_file}', + launcherCommand: 'bash {repo_root_sh}/scripts/orchestrate-codex-worker.sh {task_file_sh} {handoff_file_sh} {status_file_sh}', workers: [{ name: 'Docs', task: 'Update the workflow docs.' }] });