Improve smoke test error reporting and stability

richiemcilroy · richiemcilroy · commit ebf0aee1fc96 · 2026-03-08T14:02:49.000Z
diff --git a/apps/admin/src/app/api/smoke/route.ts b/apps/admin/src/app/api/smoke/route.ts
@@ -26,10 +26,7 @@ function getSmokeConfig() {
 
   return {
     apiKey,
-    baseUrl:
-      process.env['SANDCHEST_SMOKE_BASE_URL']?.trim() ||
-      process.env['SANDCHEST_BASE_URL']?.trim() ||
-      DEFAULT_BASE_URL,
+    baseUrl: DEFAULT_BASE_URL,
     image: process.env['SANDCHEST_SMOKE_IMAGE']?.trim() || undefined,
     profile: (process.env['SANDCHEST_SMOKE_PROFILE']?.trim() || undefined) as
       | SmokeProfile
@@ -38,12 +35,33 @@ function getSmokeConfig() {
   }
 }
 
+function errorMessages(error: unknown): string[] {
+  if (error instanceof AggregateError) {
+    return error.errors.flatMap((entry) => errorMessages(entry))
+  }
+  if (error instanceof Error) {
+    const messages = [error.message]
+    if (error.cause instanceof Error && error.cause.message !== error.message) {
+      messages.push(error.cause.message)
+    }
+    return messages
+  }
+  return [String(error)]
+}
+
 export async function POST() {
   try {
     const result = await runSandboxSmokeTest(getSmokeConfig())
     return NextResponse.json(result)
   } catch (error) {
-    const message = error instanceof Error ? error.message : 'Unknown error'
-    return NextResponse.json({ error: message }, { status: 500 })
+    console.error('Admin smoke failed', error)
+    const details = errorMessages(error)
+    return NextResponse.json(
+      {
+        error: details[0] ?? 'Unknown error',
+        details: details.length > 1 ? details.slice(1) : [],
+      },
+      { status: 500 },
+    )
   }
 }
diff --git a/apps/admin/src/app/smoke/page.tsx b/apps/admin/src/app/smoke/page.tsx
@@ -15,24 +15,35 @@ interface SmokeResult {
   checks: SmokeCheck[]
 }
 
+interface SmokeErrorPayload {
+  error: string
+  details?: string[] | undefined
+}
+
 export default function SmokePage() {
   const [isRunning, setIsRunning] = useState(false)
   const [error, setError] = useState<string | null>(null)
+  const [errorDetails, setErrorDetails] = useState<string[]>([])
   const [result, setResult] = useState<SmokeResult | null>(null)
 
   async function handleRun() {
     setIsRunning(true)
     setError(null)
+    setErrorDetails([])
     setResult(null)
 
     try {
       const response = await fetch('/api/smoke', { method: 'POST' })
       const body = (await response.json().catch(() => ({ error: 'Request failed' }))) as
         | SmokeResult
-        | { error: string }
+        | SmokeErrorPayload
 
       if (!response.ok || 'error' in body) {
-        throw new Error('error' in body ? body.error : `Request failed (${response.status})`)
+        if ('error' in body) {
+          setErrorDetails(body.details ?? [])
+          throw new Error(body.error)
+        }
+        throw new Error(`Request failed (${response.status})`)
       }
 
       setResult(body)
@@ -53,9 +64,9 @@ export default function SmokePage() {
         <div style={{ display: 'flex', flexDirection: 'column', gap: '0.375rem' }}>
           <div className="card-title">Production Lifecycle Test</div>
           <p className="text-weak" style={{ fontSize: '0.8125rem', lineHeight: 1.6 }}>
-            Runs the full sandbox lifecycle from the admin server using configured environment
-            credentials. The flow creates live sandboxes, exercises exec/session/file/fork paths,
-            and attempts cleanup in all cases.
+            Runs the full sandbox lifecycle against <code>https://api.sandchest.com</code> from
+            the admin server using configured credentials. The flow creates live sandboxes,
+            exercises exec/session/file/fork paths, and attempts cleanup in all cases.
           </p>
         </div>
 
@@ -72,6 +83,15 @@ export default function SmokePage() {
       {error ? (
         <div className="card feedback-card feedback-danger">
           Smoke test failed: {error}
+          {errorDetails.length > 0 ? (
+            <div style={{ marginTop: '0.75rem', display: 'flex', flexDirection: 'column', gap: '0.375rem' }}>
+              {errorDetails.map((detail) => (
+                <div key={detail} style={{ fontSize: '0.75rem', lineHeight: 1.5 }}>
+                  {detail}
+                </div>
+              ))}
+            </div>
+          ) : null}
         </div>
       ) : null}
 
diff --git a/packages/admin-cli/src/sandbox-smoke.live.test.ts b/packages/admin-cli/src/sandbox-smoke.live.test.ts
@@ -13,7 +13,7 @@ describe.skipIf(!RUN_ADMIN_SANDBOX_SMOKE_TESTS)('sandbox smoke (live)', () => {
       baseUrl: process.env['SANDCHEST_BASE_URL'] ?? config.api?.baseUrl,
     })
 
-    expect(result.checks.length).toBeGreaterThanOrEqual(8)
+    expect(result.checks.length).toBeGreaterThanOrEqual(9)
     expect(result.rootSandboxId.startsWith('sb_')).toBe(true)
     expect(result.forkSandboxId.startsWith('sb_')).toBe(true)
   })
diff --git a/packages/admin-cli/src/sandbox-smoke.ts b/packages/admin-cli/src/sandbox-smoke.ts
@@ -61,6 +61,11 @@ type CleanupTask = {
   destroy: () => Promise<void>
 }
 
+type SandboxStateSnapshot = {
+  status: string
+  failureReason: string | null
+}
+
 function toError(error: unknown): Error {
   if (error instanceof Error) {
     return error
@@ -93,14 +98,65 @@ function makeRunId(): string {
   return `admin-smoke-${Date.now()}-${crypto.randomUUID().slice(0, 8)}`
 }
 
+async function fetchSandboxState(
+  baseUrl: string,
+  apiKey: string,
+  sandboxId: string,
+): Promise<SandboxStateSnapshot | null> {
+  try {
+    const response = await fetch(`${baseUrl}/v1/sandboxes/${sandboxId}`, {
+      headers: {
+        Authorization: `Bearer ${apiKey}`,
+      },
+    })
+
+    if (!response.ok) {
+      return null
+    }
+
+    const body = await response.json() as {
+      status?: unknown
+      failure_reason?: unknown
+    }
+
+    if (typeof body.status !== 'string') {
+      return null
+    }
+
+    return {
+      status: body.status,
+      failureReason: typeof body.failure_reason === 'string' ? body.failure_reason : null,
+    }
+  } catch {
+    return null
+  }
+}
+
+function formatSandboxState(snapshot: SandboxStateSnapshot | null): string {
+  if (!snapshot) {
+    return 'sandbox state unavailable'
+  }
+  if (snapshot.failureReason) {
+    return `sandbox state ${snapshot.status} (${snapshot.failureReason})`
+  }
+  return `sandbox state ${snapshot.status}`
+}
+
 async function measureCheck(
   name: string,
   logger: SandboxSmokeLogger | undefined,
   fn: () => Promise<void>,
 ): Promise<SandboxSmokeCheckResult> {
   logger?.step?.('check', name)
   const startedAt = Date.now()
-  await fn()
+  try {
+    await fn()
+  } catch (error) {
+    const normalized = toError(error)
+    throw new Error(`Smoke step "${name}" failed: ${normalized.message}`, {
+      cause: normalized,
+    })
+  }
   const durationMs = Date.now() - startedAt
   logger?.info?.(`${name} (${durationMs}ms)`)
   return { name, durationMs }
@@ -216,8 +272,8 @@ export async function runSandboxSmokeTest(
   const tracker = new SandboxSmokeTracker()
   const checks: SandboxSmokeCheckResult[] = []
   const runId = makeRunId()
-  const sharedPath = `/work/${runId}.txt`
-  const sessionPath = `/work/${runId}.session.txt`
+  const sharedPath = `/tmp/${runId}.txt`
+  const sessionPath = `/tmp/${runId}.session.txt`
   const fileContents = `smoke:${runId}`
 
   let rootSandboxId = ''
@@ -232,9 +288,6 @@ export async function runSandboxSmokeTest(
           image: resolved.image,
           profile: resolved.profile,
           ttlSeconds: resolved.ttlSeconds,
-          env: {
-            SANDCHEST_SMOKE_RUN: runId,
-          },
         })
         tracker.trackSandbox('root', rootSandbox)
         rootSandboxId = rootSandbox.id
@@ -245,37 +298,33 @@ export async function runSandboxSmokeTest(
     assert(rootSandbox, 'Sandbox was not created')
     const sandbox = rootSandbox
 
-    checks.push(
-      await measureCheck('lookup sandbox', logger, async () => {
-        const fetched = await client.get(sandbox.id)
-        assert(fetched.id === sandbox.id, 'client.get returned the wrong sandbox id')
-
-        const runningSandboxes = await client.list({ status: 'running' })
-        assert(
-          runningSandboxes.some((listedSandbox: { id: string }) => listedSandbox.id === sandbox.id),
-          `client.list did not include sandbox ${sandbox.id}`,
-        )
-      }),
-    )
-
     checks.push(
       await measureCheck('exec command', logger, async () => {
         const execResult = await sandbox.exec(
-          ['sh', '-lc', 'test "$SANDCHEST_SMOKE_RUN" = "$EXPECTED" && printf smoke-ok'],
-          { env: { EXPECTED: runId }, timeout: 60 },
+          ['sh', '-lc', 'test "$SMOKE_EXEC" = "$EXPECTED" && printf smoke-ok'],
+          { env: { SMOKE_EXEC: runId, EXPECTED: runId }, timeout: 60 },
         )
         assertExecSuccess(execResult, 'sandbox exec')
         assert(execResult.stdout === 'smoke-ok', `Unexpected exec stdout: ${JSON.stringify(execResult.stdout)}`)
       }),
     )
 
     checks.push(
-      await measureCheck('file operations', logger, async () => {
+      await measureCheck('upload file', logger, async () => {
         await sandbox.fs.write(sharedPath, fileContents)
+      }),
+    )
+
+    checks.push(
+      await measureCheck('read file', logger, async () => {
         const fileValue = await sandbox.fs.read(sharedPath)
         assert(fileValue === fileContents, 'sandbox.fs.read did not match written content')
+      }),
+    )
 
-        const files = await sandbox.fs.ls('/work')
+    checks.push(
+      await measureCheck('list files', logger, async () => {
+        const files = await sandbox.fs.ls('/tmp')
         assert(
           files.some((entry: { path: string }) => entry.path === sharedPath),
           `sandbox.fs.ls did not include ${sharedPath}`,
@@ -287,45 +336,47 @@ export async function runSandboxSmokeTest(
       await measureCheck('artifact registration', logger, async () => {
         const registered = await sandbox.artifacts.register([sharedPath])
         assert(registered.registered >= 1, 'artifact registration returned zero registered artifacts')
-
-        const artifacts = await sandbox.artifacts.list()
-        assert(
-          artifacts.some((artifact: { name: string }) => artifact.name === sharedPath),
-          `artifact list did not include ${sharedPath}`,
-        )
+        assert(registered.total >= 1, 'artifact registration did not increase total registered paths')
       }),
     )
 
     checks.push(
       await measureCheck('session lifecycle', logger, async () => {
-        const session = await sandbox.session.create({ shell: '/bin/bash' })
-        tracker.trackSession('root-shell', session)
-
-        const primeResult = await session.exec(
-          `cd /work && printf '%s' "session:${runId}" > "${sessionPath}"`,
-          { timeout: 60 },
-        )
-        assertExecSuccess(primeResult, 'session prime exec')
-
-        const persistedResult = await session.exec(`pwd && cat "${sessionPath}"`, { timeout: 60 })
-        assertExecSuccess(persistedResult, 'session persisted exec')
-
-        const output = persistedResult.stdout.trimEnd()
-        assert(
-          output === `/work\nsession:${runId}`,
-          `Session state did not persist as expected: ${JSON.stringify(output)}`,
-        )
-
-        await session.destroy()
-        tracker.releaseSession(session.id)
+        try {
+          const session = await sandbox.session.create({ shell: '/bin/sh' })
+          tracker.trackSession('root-shell', session)
+
+          const primeResult = await session.exec(
+            `cd /tmp && printf '%s' "session:${runId}" > "${sessionPath}"`,
+            { timeout: 60 },
+          )
+          assertExecSuccess(primeResult, 'session prime exec')
+
+          const persistedResult = await session.exec(`pwd && cat "${sessionPath}"`, { timeout: 60 })
+          assertExecSuccess(persistedResult, 'session persisted exec')
+
+          const output = persistedResult.stdout.trimEnd()
+          assert(
+            output === `/tmp\nsession:${runId}`,
+            `Session state did not persist as expected: ${JSON.stringify(output)}`,
+          )
+
+          await session.destroy()
+          tracker.releaseSession(session.id)
+        } catch (error) {
+          const snapshot = await fetchSandboxState(resolved.baseUrl, resolved.apiKey, sandbox.id)
+          const normalized = toError(error)
+          throw new Error(`${normalized.message}; ${formatSandboxState(snapshot)}`, {
+            cause: normalized,
+          })
+        }
       }),
     )
 
     let forkSandbox: Sandbox | undefined
     checks.push(
       await measureCheck('fork sandbox', logger, async () => {
         forkSandbox = await sandbox.fork({
-          env: { SANDCHEST_SMOKE_FORK: runId },
           ttlSeconds: resolved.ttlSeconds,
         })
         tracker.trackSandbox('fork', forkSandbox)
@@ -335,8 +386,8 @@ export async function runSandboxSmokeTest(
         assert(forkReadback === fileContents, 'Fork did not inherit parent filesystem state')
 
         const forkExec = await forkSandbox.exec(
-          ['sh', '-lc', 'test "$SANDCHEST_SMOKE_FORK" = "$EXPECTED" && printf fork-ok'],
-          { env: { EXPECTED: runId }, timeout: 60 },
+          ['sh', '-lc', 'test "$SMOKE_FORK" = "$EXPECTED" && printf fork-ok'],
+          { env: { SMOKE_FORK: runId, EXPECTED: runId }, timeout: 60 },
         )
         assertExecSuccess(forkExec, 'fork exec')
         assert(forkExec.stdout === 'fork-ok', `Unexpected fork stdout: ${JSON.stringify(forkExec.stdout)}`)
@@ -369,6 +420,28 @@ export async function runSandboxSmokeTest(
         )
       }),
     )
+
+    checks.push(
+      await measureCheck('collect artifacts on stop', logger, async () => {
+        await sandbox.stop()
+        assert(
+          sandbox.status === 'stopping' || sandbox.status === 'stopped',
+          `Expected root sandbox to be stopping or stopped, got ${sandbox.status}`,
+        )
+
+        const fetched = await client.get(sandbox.id)
+        assert(
+          fetched.status === 'stopping' || fetched.status === 'stopped',
+          `Expected fetched root sandbox to be stopping or stopped, got ${fetched.status}`,
+        )
+
+        const artifacts = await sandbox.artifacts.list()
+        assert(
+          artifacts.some((artifact: { name: string }) => artifact.name === sharedPath),
+          `artifact list did not include ${sharedPath}`,
+        )
+      }),
+    )
   } catch (error) {
     primaryError = toError(error)
   }
