Skip to content

Commit 99608d0

Browse files
authored
Fix explain artifact path resolution (#38)
1 parent 9facf01 commit 99608d0

4 files changed

Lines changed: 285 additions & 3 deletions

File tree

src/cli/explain.ts

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { readFile } from "node:fs/promises";
1+
import { readFile, readdir } from "node:fs/promises";
22
import path from "node:path";
33
import process from "node:process";
44
import cliSpinners from "cli-spinners";
@@ -32,7 +32,7 @@ export async function explainCommandWithWriter(
3232
const colors = pc.createColors(Boolean(stdout.isTTY));
3333
const accent = (value: string): string =>
3434
colors.isColorSupported ? `${ACCENT_OPEN}${value}${ACCENT_CLOSE}` : value;
35-
const artifactDir = path.resolve(options.artifactDir);
35+
const artifactDir = await resolveExplainArtifactDirectory(path.resolve(options.artifactDir));
3636
const explainPath = path.join(artifactDir, "explain.json");
3737
const explanationsPath = path.join(artifactDir, "explanations.json");
3838
const reportPath = path.join(artifactDir, "report.json");
@@ -195,6 +195,72 @@ async function readJsonIfPresent<T>(filePath: string): Promise<T | undefined> {
195195
}
196196
}
197197

198+
async function resolveExplainArtifactDirectory(artifactDir: string): Promise<string> {
199+
if (await hasExplainArtifacts(artifactDir)) {
200+
return artifactDir;
201+
}
202+
203+
const nestedArtifactDirs = await collectExplainArtifactDirectories(artifactDir, 2);
204+
205+
if (nestedArtifactDirs.length === 1) {
206+
return nestedArtifactDirs[0]!;
207+
}
208+
209+
if (nestedArtifactDirs.length > 1) {
210+
throw new Error(
211+
"Multiple explain artifacts were found below the provided artifact directory. Pass the exact failed artifact directory instead.",
212+
);
213+
}
214+
215+
return artifactDir;
216+
}
217+
218+
async function collectExplainArtifactDirectories(
219+
artifactDir: string,
220+
depth: number,
221+
): Promise<string[]> {
222+
if (depth <= 0) {
223+
return [];
224+
}
225+
226+
let entries;
227+
try {
228+
entries = await readdir(artifactDir, { withFileTypes: true });
229+
} catch (error) {
230+
if (isMissingFileError(error)) {
231+
return [];
232+
}
233+
234+
throw error;
235+
}
236+
237+
const nestedDirs = entries
238+
.filter((entry) => entry.isDirectory())
239+
.map((entry) => path.join(artifactDir, entry.name))
240+
.sort();
241+
const matches: string[] = [];
242+
243+
for (const nestedDir of nestedDirs) {
244+
if (await hasExplainArtifacts(nestedDir)) {
245+
matches.push(nestedDir);
246+
continue;
247+
}
248+
249+
matches.push(...(await collectExplainArtifactDirectories(nestedDir, depth - 1)));
250+
}
251+
252+
return matches;
253+
}
254+
255+
async function hasExplainArtifacts(artifactDir: string): Promise<boolean> {
256+
const [explainArtifact, explanationsArtifact] = await Promise.all([
257+
readJsonIfPresent<ExplainArtifact>(path.join(artifactDir, "explain.json")),
258+
readJsonIfPresent<ExplanationsArtifact>(path.join(artifactDir, "explanations.json")),
259+
]);
260+
261+
return explainArtifact !== undefined || explanationsArtifact !== undefined;
262+
}
263+
198264
function isMissingFileError(error: unknown): error is NodeJS.ErrnoException {
199265
return typeof error === "object" && error !== null && "code" in error && error.code === "ENOENT";
200266
}

src/reporters/standard.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ export function createStandardReporter(options: StandardReporterOptions = {}): B
290290
}
291291

292292
function hasExplainArtifact(failure: FailureEntry): boolean {
293-
return existsSync(path.join(failure.executionArtifactDir, "explain.json"));
293+
return existsSync(path.join(failure.artifactDir, "explain.json"));
294294
}
295295

296296
function formatCaseRow(result: CaseResult, symbols: ReturnType<typeof getSymbols>): string {

test/cli.test.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,121 @@ test("explainCommand writes explanations.json from persisted explain questions",
246246
]);
247247
});
248248

249+
test("explainCommand resolves a nested failed artifact directory from the execution root", async () => {
250+
const tempDir = await mkdtemp(path.join(os.tmpdir(), "skillgym-cli-"));
251+
tempDirs.push(tempDir);
252+
const executionArtifactDir = path.join(tempDir, "alpha", "open-main");
253+
const artifactDir = path.join(executionArtifactDir, "repeat-1", "session-2");
254+
await mkdir(artifactDir, { recursive: true });
255+
await writeFile(
256+
path.join(artifactDir, "report.json"),
257+
`${JSON.stringify({
258+
runner: {
259+
id: "open-main",
260+
pathKey: "open-main",
261+
agent: { type: "opencode", model: "openai/gpt-5" },
262+
},
263+
prompt: "prompt",
264+
usage: {
265+
inputChars: 0,
266+
outputChars: 0,
267+
reasoningChars: 0,
268+
source: { input: "chars", output: "chars", reasoning: "chars" },
269+
},
270+
files: { observedReads: [], observedSkillReads: [] },
271+
detectedSkills: [],
272+
events: [],
273+
finalOutput: "",
274+
rawArtifacts: {},
275+
})}\n`,
276+
"utf8",
277+
);
278+
await writeFile(
279+
path.join(artifactDir, "explain.json"),
280+
`${JSON.stringify({
281+
suitePath: path.join(tempDir, "suite.ts"),
282+
caseId: "alpha",
283+
runnerId: "open-main",
284+
cwd: tempDir,
285+
sessionId: "ses_123",
286+
questions: [
287+
{
288+
question: "Why did you skip SKILL.md?",
289+
source: { filePath: path.join(tempDir, "suite.ts"), line: "12", column: "5" },
290+
},
291+
],
292+
})}\n`,
293+
"utf8",
294+
);
295+
296+
const explain = vi.fn(async () => ({
297+
answers: [
298+
{
299+
question: {
300+
question: "Why did you skip SKILL.md?",
301+
source: { filePath: path.join(tempDir, "suite.ts"), line: "12", column: "5" },
302+
},
303+
answer: "Because the prompt looked sufficient.",
304+
sessionId: "ses_123",
305+
startedAt: "2026-05-07T10:00:00.000Z",
306+
endedAt: "2026-05-07T10:00:01.000Z",
307+
durationMs: 1_000,
308+
rawArtifacts: {},
309+
},
310+
],
311+
}));
312+
313+
vi.resetModules();
314+
vi.doMock("../src/config.js", () => ({
315+
loadConfig: vi.fn(async () => ({
316+
config: {
317+
runners: {
318+
"open-main": { agent: { type: "opencode", model: "openai/gpt-5" } },
319+
},
320+
},
321+
})),
322+
}));
323+
vi.doMock("../src/adapters/index.js", () => ({
324+
getAdapter: vi.fn(() => ({
325+
run: vi.fn(),
326+
collect: vi.fn(),
327+
normalize: vi.fn(),
328+
explain,
329+
})),
330+
}));
331+
332+
const output: string[] = [];
333+
const stdout = {
334+
isTTY: false,
335+
write(value: string) {
336+
output.push(value);
337+
return true;
338+
},
339+
};
340+
try {
341+
const { explainCommandWithWriter } = await import("../src/cli/explain.js");
342+
await explainCommandWithWriter({ artifactDir: executionArtifactDir }, stdout);
343+
} finally {
344+
vi.doUnmock("../src/config.js");
345+
vi.doUnmock("../src/adapters/index.js");
346+
vi.resetModules();
347+
}
348+
349+
expect(explain).toHaveBeenCalledWith(
350+
expect.objectContaining({
351+
artifactDir,
352+
cwd: tempDir,
353+
sessionId: "ses_123",
354+
}),
355+
);
356+
expect(output.join("")).toContain(artifactDir);
357+
expect(
358+
JSON.parse(await readFile(path.join(artifactDir, "explanations.json"), "utf8")) as {
359+
sessionId: string;
360+
},
361+
).toMatchObject({ sessionId: "ses_123" });
362+
});
363+
249364
test("explainCommand reuses existing explanations.json by default", async () => {
250365
const tempDir = await mkdtemp(path.join(os.tmpdir(), "skillgym-cli-"));
251366
tempDirs.push(tempDir);

test/reporters/standard.test.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,107 @@ test("standard reporter shows explain hint when a failed execution has explain.j
279279
expect(output).toContain("Explain failed executions with `skillgym explain <artifactDir>`.");
280280
});
281281

282+
test("standard reporter finds explain.json in the failed artifactDir for retried executions", async () => {
283+
const writes: string[] = [];
284+
const reporter = createStandardReporter({
285+
stdout: {
286+
isTTY: false,
287+
columns: 120,
288+
write(chunk: string) {
289+
writes.push(chunk);
290+
return true;
291+
},
292+
},
293+
isInteractive: false,
294+
isUnicode: true,
295+
});
296+
297+
const executionArtifactDir = await fs.mkdtemp(
298+
path.join(os.tmpdir(), "skillgym-standard-reporter-"),
299+
);
300+
const artifactDir = path.join(executionArtifactDir, "session-2");
301+
await fs.mkdir(artifactDir, { recursive: true });
302+
await fs.writeFile(
303+
path.join(artifactDir, "explain.json"),
304+
JSON.stringify({ questions: ["why"] }),
305+
);
306+
307+
const runner = createRunnerInfo("code-main", { type: "codex", model: "gpt-5.4" });
308+
const context = {
309+
isInteractive: false,
310+
cwd: "/workspace",
311+
workspaceMode: "shared" as const,
312+
suitePath: "examples/basic-suite.ts",
313+
suiteRunArtifactDir: ".skillgym-results/run-1",
314+
selectedCaseCount: 1,
315+
selectedRunnerCount: 1,
316+
selectedExecutionCount: 1,
317+
scheduleMode: "serial" as const,
318+
maxParallel: 1,
319+
declaredTags: [],
320+
};
321+
const suiteResult: SuiteRunResult = {
322+
suitePath: context.suitePath,
323+
startedAt: "2026-04-02T12:00:00.000Z",
324+
endedAt: "2026-04-02T12:01:42.000Z",
325+
durationMs: 102_000,
326+
suiteRunArtifactDir: context.suiteRunArtifactDir,
327+
declaredTags: [],
328+
selectedTags: [],
329+
cases: [
330+
createCaseResult({
331+
caseId: "case-a",
332+
runnerResults: [
333+
createRunnerResult({
334+
runner,
335+
passed: false,
336+
executionArtifactDir,
337+
artifactDir,
338+
totalTokens: 12_000,
339+
sessions: 2,
340+
}),
341+
],
342+
}),
343+
],
344+
runners: [
345+
createRunnerSummary({
346+
runner,
347+
passedCases: 0,
348+
totalCases: 1,
349+
averageDurationMs: 19_300,
350+
averageTotalTokens: 13_500,
351+
}),
352+
],
353+
};
354+
355+
await reporter.onSuiteStart?.({
356+
context,
357+
cases: [],
358+
runners: [runner],
359+
startedAt: suiteResult.startedAt,
360+
});
361+
await reporter.onRunnerFinish?.({
362+
context,
363+
case: { id: "case-a", prompt: "", assert() {} },
364+
runner,
365+
result: suiteResult.cases[0]!.runnerResults[0]!,
366+
caseIndex: 1,
367+
totalCases: 1,
368+
});
369+
await reporter.onCaseFinish?.({
370+
context,
371+
case: { id: "case-a", prompt: "", assert() {} },
372+
result: suiteResult.cases[0]!,
373+
caseIndex: 1,
374+
totalCases: 1,
375+
});
376+
await reporter.onSuiteFinish?.({ context, result: suiteResult });
377+
378+
const output = writes.join("");
379+
380+
expect(output).toContain("Explain failed executions with `skillgym explain <artifactDir>`.");
381+
});
382+
282383
test("standard reporter interactive mode renders queued, running, and finished executions", async () => {
283384
vi.useFakeTimers();
284385
const writes: string[] = [];

0 commit comments

Comments
 (0)