fix: preserve orchestration launcher compatibility

This commit is contained in:
Affaan Mustafa
2026-03-12 15:40:25 -07:00
parent c5b8a0783e
commit 2d43541f0e
5 changed files with 37 additions and 24 deletions

View File

@@ -293,11 +293,12 @@ function readPlanString(config, key, absoluteTarget) {
return undefined; return undefined;
} }
if (typeof value !== 'string' || value.trim().length === 0) { if (typeof value !== 'string') {
throw new Error(`Invalid orchestration plan: ${key} must be a non-empty string (${absoluteTarget})`); 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()) { function resolveSnapshotTarget(targetPath, cwd = process.cwd()) {

View File

@@ -39,7 +39,7 @@ function buildTemplateVariables(values) {
const stringValue = String(value); const stringValue = String(value);
const quotedValue = shellQuote(stringValue); const quotedValue = shellQuote(stringValue);
accumulator[key] = quotedValue; accumulator[key] = stringValue;
accumulator[`${key}_raw`] = stringValue; accumulator[`${key}_raw`] = stringValue;
accumulator[`${key}_sh`] = quotedValue; accumulator[`${key}_sh`] = quotedValue;
return accumulator; return accumulator;

View File

@@ -17,11 +17,11 @@ function usage() {
' node scripts/orchestrate-worktrees.js <plan.json> [--write-only]', ' node scripts/orchestrate-worktrees.js <plan.json> [--write-only]',
'', '',
'Placeholders supported in launcherCommand:', 'Placeholders supported in launcherCommand:',
' Shell-safe defaults: {worker_name} {worker_slug} {session_name} {repo_root}', ' Raw defaults: {worker_name} {worker_slug} {session_name} {repo_root}',
' Shell-safe defaults: {worktree_path} {branch_name} {task_file} {handoff_file} {status_file}', ' Raw 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}', ' Shell-safe aliases: {worker_name_sh} {worker_slug_sh} {session_name_sh} {repo_root_sh}',
' Raw variants: {worktree_path_raw} {branch_name_raw} {task_file_raw} {handoff_file_raw} {status_file_raw}', ' Shell-safe aliases: {worktree_path_sh} {branch_name_sh} {task_file_sh} {handoff_file_sh} {status_file_sh}',
' Explicit shell-safe aliases also exist with the _sh suffix.', ' Explicit raw aliases also exist with the _raw suffix.',
'', '',
'Without flags the script prints a dry-run plan only.' 'Without flags the script prints a dry-run plan only.'
].join('\n')); ].join('\n'));

View File

@@ -262,13 +262,19 @@ test('resolveSnapshotTarget rejects malformed plan files and invalid config fiel
fs.mkdirSync(repoRoot, { recursive: true }); fs.mkdirSync(repoRoot, { recursive: true });
const invalidJsonPath = path.join(repoRoot, 'invalid-json.json'); 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 invalidSessionNamePath = path.join(repoRoot, 'invalid-session.json');
const invalidRepoRootPath = path.join(repoRoot, 'invalid-repo-root.json'); const invalidRepoRootPath = path.join(repoRoot, 'invalid-repo-root.json');
const invalidCoordinationRootPath = path.join(repoRoot, 'invalid-coordination-root.json'); const invalidCoordinationRootPath = path.join(repoRoot, 'invalid-coordination-root.json');
fs.writeFileSync(invalidJsonPath, '{not valid json'); fs.writeFileSync(invalidJsonPath, '{not valid json');
fs.writeFileSync(blankFieldsPath, JSON.stringify({
sessionName: ' ',
repoRoot: ' ',
coordinationRoot: ' '
}));
fs.writeFileSync(invalidSessionNamePath, JSON.stringify({ fs.writeFileSync(invalidSessionNamePath, JSON.stringify({
sessionName: '', sessionName: 42,
repoRoot repoRoot
})); }));
fs.writeFileSync(invalidRepoRootPath, JSON.stringify({ fs.writeFileSync(invalidRepoRootPath, JSON.stringify({
@@ -278,25 +284,30 @@ test('resolveSnapshotTarget rejects malformed plan files and invalid config fiel
fs.writeFileSync(invalidCoordinationRootPath, JSON.stringify({ fs.writeFileSync(invalidCoordinationRootPath, JSON.stringify({
sessionName: 'workflow', sessionName: 'workflow',
repoRoot, repoRoot,
coordinationRoot: ' ' coordinationRoot: false
})); }));
try { 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( assert.throws(
() => resolveSnapshotTarget(invalidJsonPath, repoRoot), () => resolveSnapshotTarget(invalidJsonPath, repoRoot),
/Invalid orchestration plan JSON/ /Invalid orchestration plan JSON/
); );
assert.throws( assert.throws(
() => resolveSnapshotTarget(invalidSessionNamePath, repoRoot), () => resolveSnapshotTarget(invalidSessionNamePath, repoRoot),
/sessionName must be a non-empty string/ /sessionName must be a string when provided/
); );
assert.throws( assert.throws(
() => resolveSnapshotTarget(invalidRepoRootPath, repoRoot), () => resolveSnapshotTarget(invalidRepoRootPath, repoRoot),
/repoRoot must be a non-empty string/ /repoRoot must be a string when provided/
); );
assert.throws( assert.throws(
() => resolveSnapshotTarget(invalidCoordinationRootPath, repoRoot), () => resolveSnapshotTarget(invalidCoordinationRootPath, repoRoot),
/coordinationRoot must be a non-empty string/ /coordinationRoot must be a string when provided/
); );
} finally { } finally {
fs.rmSync(tempRoot, { recursive: true, force: true }); fs.rmSync(tempRoot, { recursive: true, force: true });

View File

@@ -57,7 +57,7 @@ test('buildOrchestrationPlan creates worktrees, branches, and tmux commands', ()
repoRoot, repoRoot,
sessionName: 'Skill Audit', sessionName: 'Skill Audit',
baseRef: 'main', 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: [ workers: [
{ name: 'Docs A', task: 'Fix skills 1-4' }, { name: 'Docs A', task: 'Fix skills 1-4' },
{ name: 'Docs B', task: 'Fix skills 5-8' } { 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 repoRoot = path.join('/tmp', 'My Repo');
const plan = buildOrchestrationPlan({ const plan = buildOrchestrationPlan({
repoRoot, repoRoot,
sessionName: 'Spacing Audit', 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' }] workers: [{ name: 'Docs Fixer', task: 'Update docs' }]
}); });
const quote = value => `'${String(value).replace(/'/g, `'\\''`)}'`;
assert.ok( assert.ok(
plan.workerPlans[0].launchCommand.includes(`bash '${repoRoot}'/scripts/orchestrate-codex-worker.sh`), plan.workerPlans[0].launchCommand.includes(`bash ${quote(repoRoot)}/scripts/orchestrate-codex-worker.sh`),
'repo_root should be shell-safe by default' 'repo_root_sh should provide a shell-safe path'
); );
assert.ok( assert.ok(
plan.workerPlans[0].launchCommand.includes(`'${plan.workerPlans[0].taskFilePath}'`), plan.workerPlans[0].launchCommand.includes(quote(plan.workerPlans[0].taskFilePath)),
'task_file should be shell-safe by default' 'task_file_sh should provide a shell-safe path'
); );
assert.ok( assert.ok(
plan.workerPlans[0].launchCommand.includes(`'${plan.workerPlans[0].workerName}' ${plan.workerPlans[0].workerName}`), plan.workerPlans[0].launchCommand.includes(`${quote(plan.workerPlans[0].workerName)} ${plan.workerPlans[0].workerName}`),
'worker_name_raw should preserve the unquoted value when explicitly requested' 'raw defaults should remain available alongside shell-safe aliases'
); );
}); });
@@ -201,7 +202,7 @@ test('materializePlan keeps worker instructions inside the worktree boundary', (
repoRoot: tempRoot, repoRoot: tempRoot,
coordinationRoot: path.join(tempRoot, '.claude', 'orchestration'), coordinationRoot: path.join(tempRoot, '.claude', 'orchestration'),
sessionName: 'Workflow E2E', 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.' }] workers: [{ name: 'Docs', task: 'Update the workflow docs.' }]
}); });