Skip to content

Commit 3e2382a

Browse files
christsoclaude
andauthored
fix(core): accept string commands in workspace hook config (#781)
* fix(core): accept string commands in workspace hook config Workspace hooks silently ignored string commands (e.g., `command: node scripts/setup.mjs`) because parseWorkspaceScriptConfig only accepted arrays. Now auto-splits string commands on whitespace to match user expectations. Closes #778 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix lint errors in setup.mjs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add JSDoc note about naive whitespace splitting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: normalize input early instead of branching to shared helper Extract parseCommandArray to handle string/array normalization upfront, keeping parseWorkspaceScriptConfig as a single linear flow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: replace deprecated script field with command in workspace tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3a876cc commit 3e2382a

File tree

3 files changed

+57
-17
lines changed

3 files changed

+57
-17
lines changed

evals/agentic-engineering/workspace-template/scripts/setup.mjs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
* Runs with cwd = eval file directory (which is inside the repo).
66
*/
77

8-
import { cpSync, mkdirSync, readdirSync, readFileSync } from 'node:fs';
9-
import { join } from 'node:path';
108
import { execSync } from 'node:child_process';
9+
import { cpSync, mkdirSync, readFileSync, readdirSync } from 'node:fs';
10+
import { join } from 'node:path';
1111

1212
// Read workspace_path from stdin (provided by AgentV orchestrator)
1313
let workspacePath;
@@ -32,10 +32,7 @@ console.log(`Workspace: ${workspacePath}`);
3232
console.log(`Repo root: ${repoRoot}`);
3333

3434
// Copy to skill discovery directories in the workspace
35-
const skillDirs = [
36-
join(workspacePath, '.agents', 'skills'),
37-
join(workspacePath, '.pi', 'skills'),
38-
];
35+
const skillDirs = [join(workspacePath, '.agents', 'skills'), join(workspacePath, '.pi', 'skills')];
3936
for (const dir of skillDirs) {
4037
mkdirSync(dir, { recursive: true });
4138
}

packages/core/src/evaluation/yaml-parser.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -513,9 +513,28 @@ export async function loadTestById(
513513
/** @deprecated Use `loadTestById` instead */
514514
export const loadEvalCaseById = loadTestById;
515515

516+
/**
517+
* Normalize a command value from YAML into a string array.
518+
* Accepts a string (split on whitespace) or an array of strings.
519+
*/
520+
function parseCommandArray(source: unknown): string[] | undefined {
521+
if (typeof source === 'string') {
522+
const parts = source.trim().split(/\s+/);
523+
return parts.length > 0 && parts[0] !== '' ? parts : undefined;
524+
}
525+
if (Array.isArray(source)) {
526+
const arr = source.filter((s): s is string => typeof s === 'string');
527+
return arr.length > 0 ? arr : undefined;
528+
}
529+
return undefined;
530+
}
531+
516532
/**
517533
* Parse a WorkspaceScriptConfig from raw YAML value.
518534
* Accepts both `command` (preferred) and `script` (deprecated alias).
535+
* Command can be an array of strings or a single string (auto-split on whitespace).
536+
* Note: string commands are split naively on whitespace. For arguments containing
537+
* spaces, use the array form: command: ["node", "path with spaces/setup.mjs"]
519538
*/
520539
function parseWorkspaceScriptConfig(
521540
raw: unknown,
@@ -527,10 +546,9 @@ function parseWorkspaceScriptConfig(
527546
if (obj.script !== undefined && obj.command === undefined) {
528547
logWarning("'script' is deprecated. Use 'command' instead.");
529548
}
530-
const commandSource = obj.command ?? obj.script;
531-
if (!Array.isArray(commandSource) || commandSource.length === 0) return undefined;
532-
const commandArr = commandSource.filter((s): s is string => typeof s === 'string');
533-
if (commandArr.length === 0) return undefined;
549+
550+
const command = parseCommandArray(obj.command ?? obj.script);
551+
if (!command) return undefined;
534552

535553
const timeoutMs = typeof obj.timeout_ms === 'number' ? obj.timeout_ms : undefined;
536554
let cwd = typeof obj.cwd === 'string' ? obj.cwd : undefined;
@@ -540,7 +558,7 @@ function parseWorkspaceScriptConfig(
540558
cwd = path.resolve(evalFileDir, cwd);
541559
}
542560

543-
const config: WorkspaceScriptConfig = { command: commandArr };
561+
const config: WorkspaceScriptConfig = { command };
544562
if (timeoutMs !== undefined) {
545563
return { ...config, timeout_ms: timeoutMs, ...(cwd !== undefined && { cwd }) };
546564
}

packages/core/test/evaluation/workspace-config-parsing.test.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ tests:
2929
workspace:
3030
hooks:
3131
before_all:
32-
script: ["bun", "run", "setup.ts"]
32+
command: ["bun", "run", "setup.ts"]
3333
timeout_ms: 120000
3434
after_each:
35-
script: ["bun", "run", "teardown.ts"]
35+
command: ["bun", "run", "teardown.ts"]
3636
timeout_ms: 30000
3737
`,
3838
);
@@ -81,7 +81,7 @@ tests:
8181
workspace:
8282
hooks:
8383
before_all:
84-
script: ["bun", "run", "default-setup.ts"]
84+
command: ["bun", "run", "default-setup.ts"]
8585
8686
tests:
8787
- id: case-1
@@ -112,7 +112,7 @@ tests:
112112
workspace:
113113
hooks:
114114
before_all:
115-
script: ["bun", "run", "default-setup.ts"]
115+
command: ["bun", "run", "default-setup.ts"]
116116
117117
tests:
118118
- id: case-override
@@ -121,7 +121,7 @@ tests:
121121
workspace:
122122
hooks:
123123
before_all:
124-
script: ["bun", "run", "custom-setup.ts"]
124+
command: ["bun", "run", "custom-setup.ts"]
125125
- id: case-default
126126
input: "Do something else"
127127
criteria: "Should work"
@@ -158,7 +158,7 @@ tests:
158158
workspace:
159159
hooks:
160160
before_all:
161-
script: ["bun", "run", "setup.ts"]
161+
command: ["bun", "run", "setup.ts"]
162162
cwd: ./scripts
163163
`,
164164
);
@@ -337,6 +337,31 @@ tests:
337337
await expect(loadTests(evalFile, testDir)).rejects.toThrow(/workspace\.pool has been removed/i);
338338
});
339339

340+
it('should accept string command and auto-split on whitespace', async () => {
341+
const evalFile = path.join(testDir, 'workspace-string-cmd.yaml');
342+
await writeFile(
343+
evalFile,
344+
`
345+
tests:
346+
- id: test-string-cmd
347+
input: "Do something"
348+
criteria: "Should work"
349+
workspace:
350+
hooks:
351+
before_all:
352+
command: node scripts/setup.mjs
353+
timeout_ms: 60000
354+
`,
355+
);
356+
357+
const cases = await loadTests(evalFile, testDir);
358+
expect(cases).toHaveLength(1);
359+
expect(cases[0].workspace?.hooks?.before_all).toEqual({
360+
command: ['node', 'scripts/setup.mjs'],
361+
timeout_ms: 60000,
362+
});
363+
});
364+
340365
it('should handle case with no workspace config', async () => {
341366
const evalFile = path.join(testDir, 'no-workspace.yaml');
342367
await writeFile(

0 commit comments

Comments
 (0)