Skip to content

Commit fb360c9

Browse files
christsoclaude
christso
and
claude
authored
fix(validate): catch workspace setup_error causes at validate-time (#1087)
* fix(validate): catch workspace setup_error causes at validate-time Adds validateWorkspacePaths() to detect two classes of errors that surface as setup_error at runtime but are statically checkable: 1. workspace.hooks.*.command — script arguments that look like relative file paths (./foo, ../bar/setup.mjs) are resolved using the same cwd precedence as the runtime (hook.cwd ?? workspaceFileDir ?? evalDir) and checked for existence. 2. workspace.template — the template path is resolved and checked. Both checks apply to inline workspace configs and to paths inside external workspace files (workspace: "path.yaml"). The external file existence case is already caught by eval-validator; the new validator focuses on the internal hook script paths and template that were previously only caught at runtime. Closes #1078 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: fix biome formatting in workspace-path-validator test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address code review findings - Combine duplicate node:fs/promises imports into one - Fix JSDoc: "two classes" now correctly lists two, clarify external file note - Remove redundant workspace string existence check (eval-validator owns it) - Add command arg index to error location: command[1] instead of command - Add .bash and .zsh to script extension heuristic - Add tests for: after_each/after_all hooks, script alias, external workspace template Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: fix biome line-length for scriptExtensions array Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 88586cc commit fb360c9

4 files changed

Lines changed: 411 additions & 5 deletions

File tree

‎apps/cli/src/commands/validate/validate-files.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
validateEvalFile,
1111
validateFileReferences,
1212
validateTargetsFile,
13+
validateWorkspacePaths,
1314
} from '@agentv/core/evaluation/validation';
1415
import fg from 'fast-glob';
1516

@@ -43,14 +44,18 @@ async function validateSingleFile(filePath: string): Promise<ValidationResult> {
4344
if (fileType === 'eval') {
4445
result = await validateEvalFile(absolutePath);
4546

46-
// Also validate file references for eval files
47+
// Also validate file references and workspace paths for eval files
4748
if (result.valid || result.errors.filter((e) => e.severity === 'error').length === 0) {
48-
const fileRefErrors = await validateFileReferences(absolutePath);
49-
if (fileRefErrors.length > 0) {
49+
const [fileRefErrors, workspaceErrors] = await Promise.all([
50+
validateFileReferences(absolutePath),
51+
validateWorkspacePaths(absolutePath),
52+
]);
53+
const extraErrors = [...fileRefErrors, ...workspaceErrors];
54+
if (extraErrors.length > 0) {
5055
result = {
5156
...result,
52-
errors: [...result.errors, ...fileRefErrors],
53-
valid: result.valid && fileRefErrors.filter((e) => e.severity === 'error').length === 0,
57+
errors: [...result.errors, ...extraErrors],
58+
valid: result.valid && extraErrors.filter((e) => e.severity === 'error').length === 0,
5459
};
5560
}
5661
}

‎packages/core/src/evaluation/validation/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export { validateCasesFile } from './cases-validator.js';
88
export { validateTargetsFile } from './targets-validator.js';
99
export { validateConfigFile } from './config-validator.js';
1010
export { validateFileReferences } from './file-reference-validator.js';
11+
export { validateWorkspacePaths } from './workspace-path-validator.js';
1112
export type {
1213
FileType,
1314
ValidationSeverity,

‎packages/core/src/evaluation/validation/workspace-path-validator.ts‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
import { access, readFile } from 'node:fs/promises';
2+
import path from 'node:path';
3+
import { parse } from 'yaml';
4+
5+
import type { ValidationError } from './types.js';
6+
7+
type JsonValue = string | number | boolean | null | JsonObject | JsonArray;
8+
type JsonObject = { readonly [key: string]: JsonValue };
9+
type JsonArray = readonly JsonValue[];
10+
11+
function isObject(value: unknown): value is JsonObject {
12+
return typeof value === 'object' && value !== null && !Array.isArray(value);
13+
}
14+
15+
/**
16+
* Validate that workspace template and hook script paths in eval files exist.
17+
*
18+
* Catches two classes of errors that surface as `setup_error` at runtime but are
19+
* detectable statically:
20+
*
21+
* 1. `workspace.template` — the template path must exist.
22+
* 2. `workspace.hooks.*.command` — script arguments that look like relative file
23+
* paths (start with `./`/`../` or carry a script extension) must resolve to
24+
* existing files using the same cwd precedence the runtime uses:
25+
* `hook.cwd ?? workspaceFileDir ?? evalDir`
26+
*
27+
* When `workspace` is a string path to an external file, that file is also read
28+
* and its template/hook paths are checked relative to the workspace file's directory.
29+
* (The workspace file's existence itself is already checked by eval-validator.)
30+
*/
31+
export async function validateWorkspacePaths(
32+
evalFilePath: string,
33+
): Promise<readonly ValidationError[]> {
34+
const errors: ValidationError[] = [];
35+
const absolutePath = path.resolve(evalFilePath);
36+
const evalDir = path.dirname(absolutePath);
37+
38+
let parsed: unknown;
39+
try {
40+
const content = await readFile(absolutePath, 'utf8');
41+
parsed = parse(content);
42+
} catch {
43+
// Parse errors are already caught by eval-validator
44+
return errors;
45+
}
46+
47+
if (!isObject(parsed)) return errors;
48+
49+
const workspaceRaw = parsed.workspace;
50+
if (workspaceRaw === undefined || workspaceRaw === null) return errors;
51+
52+
if (typeof workspaceRaw === 'string') {
53+
// External workspace file — existence is already checked by eval-validator.
54+
// Read and check template/hook paths inside the external file.
55+
const workspaceFilePath = path.resolve(evalDir, workspaceRaw);
56+
try {
57+
const wsContent = await readFile(workspaceFilePath, 'utf8');
58+
const wsParsed = parse(wsContent);
59+
if (isObject(wsParsed)) {
60+
const wsDir = path.dirname(workspaceFilePath);
61+
await validateWorkspaceObject(wsParsed, wsDir, absolutePath, 'workspace', errors);
62+
}
63+
} catch {
64+
// File missing or invalid YAML — eval-validator already reports this
65+
}
66+
} else if (isObject(workspaceRaw)) {
67+
await validateWorkspaceObject(workspaceRaw, evalDir, absolutePath, 'workspace', errors);
68+
}
69+
70+
return errors;
71+
}
72+
73+
async function validateWorkspaceObject(
74+
obj: JsonObject,
75+
baseDir: string,
76+
evalFilePath: string,
77+
location: string,
78+
errors: ValidationError[],
79+
): Promise<void> {
80+
// Check template path
81+
const template = obj.template;
82+
if (typeof template === 'string') {
83+
const templatePath = path.isAbsolute(template) ? template : path.resolve(baseDir, template);
84+
if (!(await fileExists(templatePath))) {
85+
errors.push({
86+
severity: 'error',
87+
filePath: evalFilePath,
88+
location: `${location}.template`,
89+
message: `Template path not found: ${template} (resolved to ${templatePath})`,
90+
});
91+
}
92+
}
93+
94+
// Check hook script paths
95+
const hooks = obj.hooks;
96+
if (!isObject(hooks)) return;
97+
98+
for (const hookName of ['before_all', 'before_each', 'after_each', 'after_all'] as const) {
99+
const hook = hooks[hookName];
100+
if (!isObject(hook)) continue;
101+
102+
// Resolve hook cwd the same way yaml-parser does at parse time:
103+
// config.cwd (resolved against baseDir) ?? baseDir
104+
// baseDir = workspaceFileDir for external workspace files, evalDir for inline.
105+
const hookCwdRaw = typeof hook.cwd === 'string' ? hook.cwd : undefined;
106+
const hookCwd = hookCwdRaw
107+
? path.isAbsolute(hookCwdRaw)
108+
? hookCwdRaw
109+
: path.resolve(baseDir, hookCwdRaw)
110+
: baseDir;
111+
112+
// Support both `command` (canonical) and `script` (deprecated alias)
113+
const command = hook.command ?? hook.script;
114+
if (!Array.isArray(command)) continue;
115+
116+
for (let i = 0; i < command.length; i++) {
117+
const arg = command[i];
118+
if (typeof arg !== 'string') continue;
119+
if (!looksLikeFilePath(arg)) continue;
120+
121+
const resolved = path.isAbsolute(arg) ? arg : path.resolve(hookCwd, arg);
122+
if (!(await fileExists(resolved))) {
123+
errors.push({
124+
severity: 'error',
125+
filePath: evalFilePath,
126+
location: `${location}.hooks.${hookName}.command[${i}]`,
127+
message: `Hook script not found: ${arg} (resolved to ${resolved})`,
128+
});
129+
}
130+
}
131+
}
132+
}
133+
134+
/**
135+
* Heuristic: does this command argument look like a file path rather than a
136+
* system binary name?
137+
*
138+
* Detects:
139+
* - Explicit relative paths: `./foo`, `../bar/baz`
140+
* - Script-extension arguments: `setup.mjs`, `scripts/init.sh`
141+
*/
142+
function looksLikeFilePath(arg: string): boolean {
143+
if (arg.startsWith('./') || arg.startsWith('../')) return true;
144+
const scriptExtensions = [
145+
'.mjs',
146+
'.cjs',
147+
'.js',
148+
'.ts',
149+
'.sh',
150+
'.bash',
151+
'.zsh',
152+
'.py',
153+
'.rb',
154+
'.pl',
155+
];
156+
return scriptExtensions.some((ext) => arg.endsWith(ext));
157+
}
158+
159+
async function fileExists(filePath: string): Promise<boolean> {
160+
try {
161+
await access(filePath);
162+
return true;
163+
} catch {
164+
return false;
165+
}
166+
}

0 commit comments

Comments
 (0)