fix(test): validate failure get --out before API fetch (#26)

runFailureGet now resolves and validates --out via resolveBundleDir and assertOutDirParentExists before calling GET /tests/{id}/failure, matching runArtifactGet and runCodeGet fast-fail behavior.

Adds regression tests asserting zero fetch calls on empty --out and missing parent dir paths.

Author: SahilRakhaiya05

src/commands/test.test.ts

@@ -3267,6 +3267,52 @@ function makeFailureContext(overrides: Partial<CliFailureContext> = {}): CliFail
 }
 
 describe('runFailureGet', () => {
+  it('--out rejects an empty path with VALIDATION_ERROR (exit 5) before any network I/O', async () => {
+    const { credentialsPath } = makeCreds();
+    let fetchCalls = 0;
+    const fetchImpl = makeFetch(() => {
+      fetchCalls += 1;
+      return { body: makeFailureContext() };
+    });
+    await expect(
+      runFailureGet(
+        {
+          profile: 'default',
+          output: 'text',
+          debug: false,
+          testId: 'test_failed',
+          failedOnly: false,
+          out: '',
+        },
+        { credentialsPath, fetchImpl },
+      ),
+    ).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
+    expect(fetchCalls).toBe(0);
+  });
+
+  it('--out rejects a path under a missing parent dir with VALIDATION_ERROR (exit 5) before any network I/O', async () => {
+    const { credentialsPath } = makeCreds();
+    let fetchCalls = 0;
+    const fetchImpl = makeFetch(() => {
+      fetchCalls += 1;
+      return { body: makeFailureContext() };
+    });
+    await expect(
+      runFailureGet(
+        {
+          profile: 'default',
+          output: 'text',
+          debug: false,
+          testId: 'test_failed',
+          failedOnly: false,
+          out: `/tmp/_p5_no_such_dir_${process.pid}_${Date.now()}/bundle`,
+        },
+        { credentialsPath, fetchImpl },
+      ),
+    ).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
+    expect(fetchCalls).toBe(0);
+  });
+
   it('JSON mode (no --out) prints the wire envelope verbatim to stdout', async () => {
     const { credentialsPath } = makeCreds();
     const ctx = makeFailureContext();

src/commands/test.ts

@@ -4063,11 +4063,16 @@ export async function runFailureGet(
 ): Promise<FailureGetResult> {
   const out = makeOutput(opts.output, deps);
   const client = makeClient(opts, deps);
-  // We resolve the output dir BEFORE the network call so a missing /
+
+  // Resolve and validate --out BEFORE the network call so a missing /
   // empty path surfaces as VALIDATION_ERROR (exit 5) without spending
-  // an API call. `writeBundle` re-validates internally; this is the
-  // fast-fail.
-  const requestedDir = opts.out;
+  // an API call. Mirrors `runArtifactGet`; `writeBundle` re-validates
+  // internally as defense-in-depth.
+  let resolvedDir: string | undefined;
+  if (opts.out !== undefined) {
+    resolvedDir = resolveBundleDir(opts.out);
+    await assertOutDirParentExists(resolvedDir);
+  }
 
   const context = await client.get<CliFailureContext>(
     `/tests/${encodeURIComponent(opts.testId)}/failure`,
@@ -4080,7 +4085,7 @@ export async function runFailureGet(
   // internally; this call is the cheap upfront trap.
   assertContextIntegrity(context, 'local');
 
-  if (requestedDir !== undefined) {
+  if (resolvedDir !== undefined) {
     // Dry-run: do NOT call writeBundle (which would mkdir, fetch
     // presigned URLs, and write files). Print the would-be bundle layout
     // to stderr and emit the wire envelope to stdout so the agent sees
@@ -4089,18 +4094,18 @@ export async function runFailureGet(
       const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`));
       const fileNames = plannedBundleFiles(context, opts.failedOnly);
       stderr(
-        `[dry-run] would write bundle to ${requestedDir} (${fileNames.length} files; meta.json renames last)`,
+        `[dry-run] would write bundle to ${resolvedDir} (${fileNames.length} files; meta.json renames last)`,
       );
       for (const f of fileNames) stderr(`[dry-run]   ${f}`);
       if (opts.output === 'json') {
-        out.print({ ok: true, dir: requestedDir, dryRun: true, context });
+        out.print({ ok: true, dir: resolvedDir, dryRun: true, context });
       } else {
         // Use a dry-run-specific renderer: the real success renderer
         // says "Bundle written to ..." which would be a lie here. Stdout
         // is the success contract automation may parse, so it must not
         // imply the bundle was created.
         out.print(
-          { dir: requestedDir, files: fileNames.length, snapshotId: context.snapshotId },
+          { dir: resolvedDir, files: fileNames.length, snapshotId: context.snapshotId },
           data =>
             renderBundleDryRunText(data as { dir: string; files: number; snapshotId: string }),
         );
@@ -4109,7 +4114,7 @@ export async function runFailureGet(
     }
 
     const bundle = await writeBundle(context, {
-      dir: requestedDir,
+      dir: resolvedDir,
       failedOnly: opts.failedOnly,
       fetchImpl: deps.fetchImpl,
     });