From 681d054d8c3691d354707f7ba54b665983681c2f Mon Sep 17 00:00:00 2001 From: River Bailey Date: Tue, 22 Sep 2026 14:20:30 -0600 Subject: [PATCH 1/7] fix(data): return best_train_accuracy as a number for whole-number SCIL/ACIL scores When every iteration's trainAccuracy is a whole number (e.g. a run that scores a perfect 1 on every iteration), DuckDB infers the column as BIGINT and MAX() returns a JS BigInt, which Hono's c.json cannot serialize. The SCIL and ACIL history pages then fail with a 500. Cast the aggregate to DOUBLE. Co-Authored-By: Claude --- .../data/src/analytics.integration.test.ts | 30 +++++++++++++++++++ packages/data/src/run-status.ts | 4 +-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/data/src/analytics.integration.test.ts b/packages/data/src/analytics.integration.test.ts index 2383ceb..3c9d0c5 100644 --- a/packages/data/src/analytics.integration.test.ts +++ b/packages/data/src/analytics.integration.test.ts @@ -806,6 +806,21 @@ describe('queryScilHistory', () => { best_train_accuracy: 1.0, }) }) + + it('returns best_train_accuracy as a number when every iteration scored a whole number', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await writeScilRunFixture({ + outputDir, + runId: '20260101T200001', + iterations: [makeScilIterationRecord({ trainAccuracy: 1 })], + }) + await updateAllParquet({ outputDir, dataDir }) + + const rows = await queryScilHistory(dataDir) + + expect(rows[0].best_train_accuracy).toBe(1) + }) }) // ─── SCIL: queryScilRunDetails ─────────────────────────────────────────────── @@ -1488,6 +1503,21 @@ describe('queryAcilHistory', () => { }) }) + it('returns best_train_accuracy as a number when every iteration scored a whole number', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await writeAcilRunFixture({ + outputDir, + runId: '20260101T300001', + iterations: [makeAcilIterationRecord({ trainAccuracy: 1 })], + }) + await updateAllParquet({ outputDir, dataDir }) + + const rows = await queryAcilHistory(dataDir) + + expect(rows[0].best_train_accuracy).toBe(1) + }) + it('returns multiple runs ordered by test_run_id DESC', async () => { const outputDir = path.join(tmpDir, 'output') const dataDir = path.join(tmpDir, 'analytics') diff --git a/packages/data/src/run-status.ts b/packages/data/src/run-status.ts index 5e71001..fdccedc 100644 --- a/packages/data/src/run-status.ts +++ b/packages/data/src/run-status.ts @@ -44,7 +44,7 @@ export async function queryScilHistory(dataDir: string): Promise Date: Tue, 22 Sep 2026 14:21:19 -0600 Subject: [PATCH 2/7] fix(data): return total_cost_usd as a number when every run cost a whole number ROUND() of a BIGINT stays BIGINT. When every total_cost_usd in the data set is a whole number (e.g. early-failing runs that report 0), DuckDB infers BIGINT and queryPerTest / queryTestRunDetails return a JS BigInt, which breaks JSON serialization on the per-test analytics and run detail pages. Cast to DOUBLE. Co-Authored-By: Claude --- .../data/src/analytics.integration.test.ts | 22 +++++++++++++++++++ packages/data/src/analytics.ts | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/data/src/analytics.integration.test.ts b/packages/data/src/analytics.integration.test.ts index 3c9d0c5..177dc86 100644 --- a/packages/data/src/analytics.integration.test.ts +++ b/packages/data/src/analytics.integration.test.ts @@ -356,6 +356,17 @@ describe('queryPerTest', () => { const rows = await queryPerTest(dataDir) expect(rows[0].total_cost_usd).toBe(0.12) }) + + it('returns total_cost_usd as a number when every run cost a whole number', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await writeRunFixture({ outputDir, testRunId: '20260101T100001', eval: 's', testName: 'test one', totalCostUsd: 0 }) + await updateAllParquet({ outputDir, dataDir }) + + const rows = await queryPerTest(dataDir) + + expect(rows[0].total_cost_usd).toBe(0) + }) }) // ─── queryTestRunSummaries ──────────────────────────────────────────────────── @@ -482,6 +493,17 @@ describe('queryTestRunDetails', () => { const details = await queryTestRunDetails(dataDir, '20260101T000008') expect(details.summary[0].total_cost_usd).toBe(0.1235) }) + + it('returns total_cost_usd as a number in details when every run cost a whole number', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await writeRunFixture({ outputDir, testRunId: '20260101T100001', eval: 's', testName: 'test one', totalCostUsd: 0 }) + await updateAllParquet({ outputDir, dataDir }) + + const details = await queryTestRunDetails(dataDir, '20260101T100001') + + expect(details.summary[0].total_cost_usd).toBe(0) + }) }) // ─── queryPerTest — JOIN edge cases ────────────────────────────────────────── diff --git a/packages/data/src/analytics.ts b/packages/data/src/analytics.ts index 3f426d3..a93ea81 100644 --- a/packages/data/src/analytics.ts +++ b/packages/data/src/analytics.ts @@ -345,7 +345,7 @@ export async function queryPerTest(dataDir: string): Promise { c.test.name AS test_name, c.eval, e.all_expectations_passed, - ROUND(r.total_cost_usd, 2) AS total_cost_usd, + CAST(ROUND(r.total_cost_usd, 2) AS DOUBLE) AS total_cost_usd, CAST(r.num_turns AS INTEGER) AS num_turns, CAST(r.usage.input_tokens AS INTEGER) AS input_tokens, CAST(r.usage.output_tokens AS INTEGER) AS output_tokens @@ -464,7 +464,7 @@ export async function queryTestRunDetails(dataDir: string, testRunId: string): P r.is_error, e.all_expectations_passed, r.result, - ROUND(r.total_cost_usd, 4) AS total_cost_usd, + CAST(ROUND(r.total_cost_usd, 4) AS DOUBLE) AS total_cost_usd, CAST(r.num_turns AS INTEGER) AS num_turns, CAST(r.usage.input_tokens AS INTEGER) AS input_tokens, CAST(r.usage.output_tokens AS INTEGER) AS output_tokens From ae51d2150f8b2f3c5b12b1ebd60cc26f65ba551d Mon Sep 17 00:00:00 2001 From: River Bailey Date: Tue, 22 Sep 2026 14:21:19 -0600 Subject: [PATCH 3/7] test(web): exercise every API route against real DuckDB data Route unit tests mock the data layer and fake c.json, so a BigInt or a missing parquet file never reached real JSON serialization and both crashes shipped. Extract the API wiring into createApp(dataDir) so an integration test can send real requests through the real Hono app, backed by parquet built from JSONL with whole-number scores, accuracies, and costs, plus an empty data directory. Co-Authored-By: Claude --- .../web/src/server/app.integration.test.ts | 116 ++++++++++++++++++ packages/web/src/server/app.ts | 24 ++++ packages/web/src/server/index.ts | 20 +-- 3 files changed, 142 insertions(+), 18 deletions(-) create mode 100644 packages/web/src/server/app.integration.test.ts create mode 100644 packages/web/src/server/app.ts diff --git a/packages/web/src/server/app.integration.test.ts b/packages/web/src/server/app.integration.test.ts new file mode 100644 index 0000000..cc21af3 --- /dev/null +++ b/packages/web/src/server/app.integration.test.ts @@ -0,0 +1,116 @@ +import { rm } from 'node:fs/promises' +import path from 'node:path' +import { updateAllParquet } from '@testdouble/skillwalker-data' +import { + makeAcilIterationRecord, + makeConfigRecord, + makeResultRecord, + makeRunResultRecord, + makeScilIterationRecord, + makeTmpDir, + writeAcilRunFixture, + writeJsonl, + writeScilRunFixture, +} from '@testdouble/skillwalker-data/src/analytics-test-helpers.js' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { createApp } from './app.js' + +// ─── fixtures ───────────────────────────────────────────────────────────────── + +const TEST_RUN_ID = '20260101T100001' +const SCIL_RUN_ID = '20260101T200001' +const ACIL_RUN_ID = '20260101T300001' + +// Whole-number values are written to JSONL without a decimal point, so DuckDB infers +// BIGINT columns. These are the shapes that crashed the dashboard with BigInt values. +async function writeWholeNumberRunData(outputDir: string): Promise { + const runDir = path.join(outputDir, TEST_RUN_ID) + const evalName = 'my-eval' + const testName = 'judged test' + + await writeJsonl(path.join(runDir, 'test-config.jsonl'), [ + makeConfigRecord({ testRunId: TEST_RUN_ID, eval: evalName, testName }), + ]) + await writeJsonl(path.join(runDir, 'test-run.jsonl'), [ + makeRunResultRecord({ testRunId: TEST_RUN_ID, eval: evalName, testName, totalCostUsd: 0 }), + ]) + await writeJsonl(path.join(runDir, 'test-results.jsonl'), [ + makeResultRecord({ + testRunId: TEST_RUN_ID, + eval: evalName, + testName, + expectType: 'llm-judge', + judgeModel: 'opus', + judgeThreshold: 1, + judgeScore: 1, + rubricFile: 'rubric.md', + }), + ]) + + await writeScilRunFixture({ + outputDir, + runId: SCIL_RUN_ID, + iterations: [makeScilIterationRecord({ test_run_id: SCIL_RUN_ID, trainAccuracy: 1 })], + }) + await writeAcilRunFixture({ + outputDir, + runId: ACIL_RUN_ID, + iterations: [makeAcilIterationRecord({ test_run_id: ACIL_RUN_ID, trainAccuracy: 1 })], + }) +} + +// ─── test lifecycle ─────────────────────────────────────────────────────────── + +let tmpDir: string +let outputDir: string +let dataDir: string + +beforeEach(async () => { + tmpDir = await makeTmpDir() + outputDir = path.join(tmpDir, 'output') + dataDir = path.join(tmpDir, 'analytics') +}) + +afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }) +}) + +// ─── API against real stored data ───────────────────────────────────────────── + +describe('API routes with real analytics data', () => { + beforeEach(async () => { + await writeWholeNumberRunData(outputDir) + await updateAllParquet({ outputDir, dataDir }) + }) + + it.each([ + ['/api/test-runs', (body: Record) => expect(body.runs).toHaveLength(1)], + [`/api/test-runs/${TEST_RUN_ID}`, (body: Record) => expect(body.summary).toBeDefined()], + ['/api/analytics/per-test', (body: Record) => expect(body.rows).toHaveLength(1)], + ['/api/scil', (body: Record) => expect(body.runs).toHaveLength(1)], + [`/api/scil/${SCIL_RUN_ID}`, (body: Record) => expect(body.iterations).toHaveLength(1)], + ['/api/acil', (body: Record) => expect(body.runs).toHaveLength(1)], + [`/api/acil/${ACIL_RUN_ID}`, (body: Record) => expect(body.iterations).toHaveLength(1)], + ])('GET %s returns 200 with a JSON body', async (url, assertBody) => { + const res = await createApp(dataDir).request(url) + + expect(res.status).toBe(200) + assertBody(await res.json()) + }) +}) + +// ─── API with no data ───────────────────────────────────────────────────────── + +describe('API routes with no analytics data', () => { + it.each([ + ['/api/test-runs', 'runs'], + ['/api/analytics/per-test', 'rows'], + ['/api/scil', 'runs'], + ['/api/acil', 'runs'], + ])('GET %s returns 200 with an empty list', async (url, key) => { + const res = await createApp(dataDir).request(url) + + expect(res.status).toBe(200) + expect((await res.json())[key]).toEqual([]) + }) +}) diff --git a/packages/web/src/server/app.ts b/packages/web/src/server/app.ts new file mode 100644 index 0000000..bbc7a35 --- /dev/null +++ b/packages/web/src/server/app.ts @@ -0,0 +1,24 @@ +import { Hono } from 'hono' +import { getAcilHistory, getAcilRunById } from './routes/acil.js' +import { getPerTestAnalytics } from './routes/analytics.js' +import { jsonErrorHandler } from './routes/error-handler.js' +import { getScilHistory, getScilRunById } from './routes/scil.js' +import { getTestRunById, getTestRuns } from './routes/test-runs.js' + +// API routes only — static client assets are added by the entry point, which embeds them at compile time. +export function createApp(dataDir: string): Hono { + const app = new Hono() + + app.onError(jsonErrorHandler) + + app.get('/api/health', (c) => c.json({ status: 'ok' })) + app.get('/api/test-runs', (c) => getTestRuns(c, dataDir)) + app.get('/api/test-runs/:runId', (c) => getTestRunById(c, dataDir)) + app.get('/api/analytics/per-test', (c) => getPerTestAnalytics(c, dataDir)) + app.get('/api/scil', (c) => getScilHistory(c, dataDir)) + app.get('/api/scil/:runId', (c) => getScilRunById(c, dataDir)) + app.get('/api/acil', (c) => getAcilHistory(c, dataDir)) + app.get('/api/acil/:runId', (c) => getAcilRunById(c, dataDir)) + + return app +} diff --git a/packages/web/src/server/index.ts b/packages/web/src/server/index.ts index 22cbe1e..9d866d6 100644 --- a/packages/web/src/server/index.ts +++ b/packages/web/src/server/index.ts @@ -1,16 +1,11 @@ import path from 'node:path' -import { Hono } from 'hono' import yargs from 'yargs' import { hideBin } from 'yargs/helpers' import indexCss from '../../dist/client/index.css' with { type: 'file' } // Embedded client files — resolved to $bunfs paths when compiled as a standalone executable import _indexHtml from '../../dist/client/index.html' with { type: 'file' } import indexJs from '../../dist/client/index.js' with { type: 'file' } -import { getAcilHistory, getAcilRunById } from './routes/acil' -import { getPerTestAnalytics } from './routes/analytics' -import { jsonErrorHandler } from './routes/error-handler' -import { getScilHistory, getScilRunById } from './routes/scil' -import { getTestRunById, getTestRuns } from './routes/test-runs' +import { createApp } from './app' // default port const DEFAULT_PORT = 3099 @@ -37,18 +32,7 @@ const argv = await yargs(hideBin(Bun.argv)) const port = argv.port const dataDir = argv['data-dir'] -const app = new Hono() - -app.onError(jsonErrorHandler) - -app.get('/api/health', (c) => c.json({ status: 'ok' })) -app.get('/api/test-runs', (c) => getTestRuns(c, dataDir)) -app.get('/api/test-runs/:runId', (c) => getTestRunById(c, dataDir)) -app.get('/api/analytics/per-test', (c) => getPerTestAnalytics(c, dataDir)) -app.get('/api/scil', (c) => getScilHistory(c, dataDir)) -app.get('/api/scil/:runId', (c) => getScilRunById(c, dataDir)) -app.get('/api/acil', (c) => getAcilHistory(c, dataDir)) -app.get('/api/acil/:runId', (c) => getAcilRunById(c, dataDir)) +const app = createApp(dataDir) // Serve embedded static assets app.get('/index.js', () => new Response(Bun.file(indexJs))) From 077e7d024a77a1f94abdae39da7b79fc2706a8e4 Mon Sep 17 00:00:00 2001 From: River Bailey Date: Tue, 22 Sep 2026 14:22:01 -0600 Subject: [PATCH 4/7] fix(sandbox): fail sandbox create when sbx run exits non-zero createSandbox never checked the exit code of the interactive sbx run, so a failed login or template download still printed "Sandbox is ready" and the next eval run failed far from the real cause. Throw a SandboxError instead. The test stubs for sbx run now report exitCode 0, as a real finished process does. Co-Authored-By: Claude --- .../sandbox-integration/src/lifecycle.test.ts | 29 +++++++++++++++++-- packages/sandbox-integration/src/lifecycle.ts | 7 +++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/sandbox-integration/src/lifecycle.test.ts b/packages/sandbox-integration/src/lifecycle.test.ts index b0e4e6c..fbb805c 100644 --- a/packages/sandbox-integration/src/lifecycle.test.ts +++ b/packages/sandbox-integration/src/lifecycle.test.ts @@ -91,6 +91,7 @@ describe('createSandbox', () => { }) .mockReturnValueOnce({ exited: Promise.resolve(), + exitCode: 0, }) const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) @@ -104,6 +105,27 @@ describe('createSandbox', () => { stderrSpy.mockRestore() }) + + it('throws SandboxError and does not report the sandbox ready when sbx run fails', async () => { + ;(globalThis as any).Bun.spawn + .mockReturnValueOnce({ + stdout: makeStream('other-sandbox\n'), + stderr: makeStream(''), + exited: Promise.resolve(), + exitCode: 0, + }) + .mockReturnValueOnce({ + exited: Promise.resolve(), + exitCode: 1, + }) + const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + + const { createSandbox } = await import('./lifecycle.js') + + await expect(createSandbox('/repo/root')).rejects.toBeInstanceOf(SandboxError) + expect(stderrSpy).not.toHaveBeenCalledWith(expect.stringContaining('is ready')) + }) + function stubNoSandboxThenRun() { ;(globalThis as any).Bun.spawn .mockReturnValueOnce({ @@ -114,6 +136,7 @@ describe('createSandbox', () => { }) .mockReturnValueOnce({ exited: Promise.resolve(), + exitCode: 0, }) } @@ -182,7 +205,7 @@ describe('updateSandbox', () => { .mockReturnValueOnce(makeCapturedProc('')) .mockReturnValueOnce(makeCapturedProc('')) .mockReturnValueOnce(makeCapturedProc('')) - .mockReturnValueOnce({ exited: Promise.resolve() }) + .mockReturnValueOnce({ exited: Promise.resolve(), exitCode: 0 }) const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) @@ -207,7 +230,7 @@ describe('updateSandbox', () => { .mockReturnValueOnce(makeCapturedProc('other-sandbox\n')) .mockReturnValueOnce(makeCapturedProc(templateList.split('\n')[0])) .mockReturnValueOnce(makeCapturedProc('other-sandbox\n')) - .mockReturnValueOnce({ exited: Promise.resolve() }) + .mockReturnValueOnce({ exited: Promise.resolve(), exitCode: 0 }) const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) @@ -231,7 +254,7 @@ describe('updateSandbox', () => { .mockReturnValueOnce(makeCapturedProc('')) .mockReturnValueOnce(makeCapturedProc("ERROR: sandboxd error: status 404: no template image '94670d5b2a24'", 1)) .mockReturnValueOnce(makeCapturedProc('other-sandbox\n')) - .mockReturnValueOnce({ exited: Promise.resolve() }) + .mockReturnValueOnce({ exited: Promise.resolve(), exitCode: 0 }) const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) diff --git a/packages/sandbox-integration/src/lifecycle.ts b/packages/sandbox-integration/src/lifecycle.ts index ca96c6c..a978b45 100644 --- a/packages/sandbox-integration/src/lifecycle.ts +++ b/packages/sandbox-integration/src/lifecycle.ts @@ -107,6 +107,13 @@ export async function createSandbox(repoRoot: string, extraWorkspaces: string[] }) await runProc.exited + if (runProc.exitCode !== 0) { + throw new SandboxError( + `sbx run failed (exit code ${runProc.exitCode ?? 1}). The sandbox was not created.\nRetry with \`./build/skillwalker sandbox create\`.`, + runProc.exitCode, + ) + } + process.stderr.write(`\nSandbox "${SANDBOX_NAME}" is ready. You can now run tests.\n`) } From 4c7b1c14b402f87ef3d81d5dca3b80a786fc7912 Mon Sep 17 00:00:00 2001 From: River Bailey Date: Tue, 22 Sep 2026 14:22:32 -0600 Subject: [PATCH 5/7] test(cli): print sandbox errors as a single Error line The CLI catches SandboxError separately from SkillwalkerError, and only the SkillwalkerError path was tested. Run a sandbox command against a fake sbx on PATH that fails, and check the output is one Error line with no help text or stack trace. Confirmed the test fails when the SandboxError catch is removed. Co-Authored-By: Claude --- .../command-registration.integration.test.ts | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/command-registration.integration.test.ts b/packages/cli/src/command-registration.integration.test.ts index 4fc89a6..e0213a4 100644 --- a/packages/cli/src/command-registration.integration.test.ts +++ b/packages/cli/src/command-registration.integration.test.ts @@ -1,11 +1,18 @@ import { spawnSync } from 'node:child_process' +import { chmod, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' import { fileURLToPath } from 'node:url' -import { describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' const entryPoint = fileURLToPath(new URL('../index.ts', import.meta.url)) function runCli(...args: string[]): { status: number; output: string } { - const result = spawnSync('bun', [entryPoint, ...args], { encoding: 'utf8' }) + return runCliWithEnv(process.env, ...args) +} + +function runCliWithEnv(env: NodeJS.ProcessEnv, ...args: string[]): { status: number; output: string } { + const result = spawnSync('bun', [entryPoint, ...args], { encoding: 'utf8', env }) return { status: result.status ?? 1, output: `${result.stdout}${result.stderr}` } } @@ -66,4 +73,30 @@ describe('command handler errors', () => { expect(output).not.toContain('Options:') expect(output).not.toContain('RunNotFoundError') }) + + describe('when sbx fails', () => { + let fakeBinDir: string + + beforeEach(async () => { + fakeBinDir = await mkdtemp(path.join(tmpdir(), 'skillwalker-fake-sbx-')) + const fakeSbx = path.join(fakeBinDir, 'sbx') + await writeFile(fakeSbx, '#!/bin/sh\necho "You are not logged in." >&2\nexit 1\n', 'utf8') + await chmod(fakeSbx, 0o755) + }) + + afterEach(async () => { + await rm(fakeBinDir, { recursive: true, force: true }) + }) + + it('prints a sandbox error as a single Error line without help text or a stack trace', () => { + const env = { ...process.env, PATH: `${fakeBinDir}${path.delimiter}${process.env.PATH}` } + + const { status, output } = runCliWithEnv(env, 'sandbox', 'shell') + + expect(status).toBe(1) + expect(output).toMatch(/^Error: Unable to list sandboxes with sbx/m) + expect(output).not.toContain('Options:') + expect(output).not.toMatch(/^\s+at /m) + }) + }) }) From 6f92ed116dfc1d3dfe01e229a2e852f810e03f73 Mon Sep 17 00:00:00 2001 From: River Bailey Date: Tue, 22 Sep 2026 14:23:54 -0600 Subject: [PATCH 6/7] test: smoke test the compiled binaries in CI CI never built or ran ./build/skillwalker, and the staging code's unit tests stub the Bun runtime, so nothing could catch the compiled binary failing to load the DuckDB native addon. Add a smoke test that runs update-analytics-data with the compiled CLI and queries /api/test-runs from the compiled web server, and a CI job that runs it after make build. Smoke tests are excluded from the unit and make test runs because they need build output. Confirmed both tests fail when build/duckdb.node is missing. Co-Authored-By: Claude --- .github/workflows/ci.yml | 10 +++ package.json | 1 + .../cli/src/compiled-binary.smoke.test.ts | 79 +++++++++++++++++++ vitest.all.config.ts | 2 + vitest.config.ts | 2 +- vitest.smoke.config.ts | 11 +++ 6 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/compiled-binary.smoke.test.ts create mode 100644 vitest.smoke.config.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a2db3b7..cd167d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,6 +52,16 @@ jobs: - run: bun install --frozen-lockfile - run: bun run test:integration + test-compiled-binaries: + name: Compiled Binary Smoke Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: oven-sh/setup-bun@v2 + - run: bun install --frozen-lockfile + - run: make build + - run: bun run test:smoke + security: name: Security Audit runs-on: ubuntu-latest diff --git a/package.json b/package.json index a3478b2..4247ea9 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "test:watch": "vitest", "test:integration": "vitest run --config vitest.integration.config.ts", "test:all": "vitest run --config vitest.all.config.ts", + "test:smoke": "vitest run --config vitest.smoke.config.ts", "lint": "biome lint", "format:check": "biome format", "check": "biome check", diff --git a/packages/cli/src/compiled-binary.smoke.test.ts b/packages/cli/src/compiled-binary.smoke.test.ts new file mode 100644 index 0000000..eb36b07 --- /dev/null +++ b/packages/cli/src/compiled-binary.smoke.test.ts @@ -0,0 +1,79 @@ +import { type ChildProcess, spawn, spawnSync } from 'node:child_process' +import { existsSync } from 'node:fs' +import { rm } from 'node:fs/promises' +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import { makeTmpDir, writeRunFixture } from '@testdouble/skillwalker-data/src/analytics-test-helpers.js' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +// Runs the binaries `make build` produces. Unit tests stub the Bun runtime, so only +// a compiled binary can show the DuckDB native addon actually loads. +const buildDir = fileURLToPath(new URL('../../../build/', import.meta.url)) +const cliBinary = path.join(buildDir, 'skillwalker') +const webBinary = path.join(buildDir, 'skillwalker-web') + +const TEST_RUN_ID = '20260101T100001' +const WEB_PORT = 39099 + +async function waitForServer(url: string, proc: ChildProcess, timeoutMs = 10000): Promise { + const deadline = Date.now() + timeoutMs + while (Date.now() < deadline) { + if (proc.exitCode !== null) throw new Error(`skillwalker-web exited early with code ${proc.exitCode}`) + try { + return await fetch(url) + } catch { + await new Promise((resolve) => setTimeout(resolve, 100)) + } + } + throw new Error(`skillwalker-web did not answer ${url} within ${timeoutMs}ms`) +} + +// ─── test lifecycle ─────────────────────────────────────────────────────────── + +let tmpDir: string +let outputDir: string +let dataDir: string + +beforeEach(async () => { + tmpDir = await makeTmpDir() + outputDir = path.join(tmpDir, 'output') + dataDir = path.join(tmpDir, 'analytics') + await writeRunFixture({ outputDir, testRunId: TEST_RUN_ID, eval: 'smoke', testName: 'smoke test' }) +}) + +afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }) +}) + +// ─── compiled binaries ──────────────────────────────────────────────────────── + +describe('compiled skillwalker binary', () => { + it('imports run output into parquet with the bundled DuckDB addon', () => { + const result = spawnSync(cliBinary, ['update-analytics-data', '--output-dir', outputDir, '--data-dir', dataDir], { + encoding: 'utf8', + }) + const output = `${result.stdout}${result.stderr}` + + expect(output).not.toContain('Cannot find module') + expect(result.status).toBe(0) + expect(existsSync(path.join(dataDir, 'test-run.parquet'))).toBe(true) + }) +}) + +describe('compiled skillwalker-web binary', () => { + let server: ChildProcess | undefined + + afterEach(() => { + server?.kill() + }) + + it('serves test runs queried through the bundled DuckDB addon', async () => { + spawnSync(cliBinary, ['update-analytics-data', '--output-dir', outputDir, '--data-dir', dataDir]) + server = spawn(webBinary, ['--port', String(WEB_PORT), '--data-dir', dataDir]) + + const res = await waitForServer(`http://localhost:${WEB_PORT}/api/test-runs`, server) + + expect(res.status).toBe(200) + expect((await res.json()).runs).toHaveLength(1) + }) +}) diff --git a/vitest.all.config.ts b/vitest.all.config.ts index 45a2e1d..1abe69d 100644 --- a/vitest.all.config.ts +++ b/vitest.all.config.ts @@ -5,6 +5,8 @@ export default defineConfig({ globals: true, environment: 'node', include: ['packages/*/src/**/*.test.ts'], + // Smoke tests need `make build` output; run them with `bun run test:smoke`. + exclude: ['packages/*/src/**/*.smoke.test.ts', '**/node_modules/**'], testTimeout: 30000, }, }) diff --git a/vitest.config.ts b/vitest.config.ts index 9f128c5..a5f5102 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -5,6 +5,6 @@ export default defineConfig({ globals: true, environment: 'node', include: ['packages/*/src/**/*.test.ts'], - exclude: ['packages/*/src/**/*.integration.test.ts', '**/node_modules/**'], + exclude: ['packages/*/src/**/*.integration.test.ts', 'packages/*/src/**/*.smoke.test.ts', '**/node_modules/**'], }, }) diff --git a/vitest.smoke.config.ts b/vitest.smoke.config.ts new file mode 100644 index 0000000..0130c4e --- /dev/null +++ b/vitest.smoke.config.ts @@ -0,0 +1,11 @@ +import { defineConfig } from 'vitest/config' + +// Runs against the binaries in ./build — run `make build` first. +export default defineConfig({ + test: { + globals: true, + environment: 'node', + include: ['packages/*/src/**/*.smoke.test.ts'], + testTimeout: 30000, + }, +}) From 32ef30708abf97aa3b720af11034f9d01584d55b Mon Sep 17 00:00:00 2001 From: River Bailey Date: Tue, 22 Sep 2026 14:26:26 -0600 Subject: [PATCH 7/7] docs: describe createApp, the smoke tests, and the sbx run exit check Co-Authored-By: Claude --- docs/project-discovery.md | 1 + docs/sandbox-integration-package.md | 2 +- docs/sandbox-integration.md | 2 +- docs/web.md | 9 ++++++--- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/docs/project-discovery.md b/docs/project-discovery.md index a55c84f..88654da 100644 --- a/docs/project-discovery.md +++ b/docs/project-discovery.md @@ -30,6 +30,7 @@ - Test (unit): `bun run vitest run` - Test (integration): `bun run vitest run --config vitest.integration.config.ts` - Test (all): `make test` +- Test (compiled binary smoke): `make build && bun run test:smoke` - Build: `make build` - Dev server: `make dev` - Test file pattern: `*.test.ts`, `*.integration.test.ts`, `*.unit.test.ts` diff --git a/docs/sandbox-integration-package.md b/docs/sandbox-integration-package.md index 712054e..4ce17b3 100644 --- a/docs/sandbox-integration-package.md +++ b/docs/sandbox-integration-package.md @@ -110,7 +110,7 @@ Primary execution function. Builds and spawns the command `sbx exec claude-skill async function createSandbox(repoRoot: string, extraWorkspaces: string[] = []): Promise ``` -Checks whether the sandbox already exists via an internal `sandboxExists()` helper (runs `sbx ls --quiet`). If found, prints a help message to stderr explaining how to recreate it, and returns early. Otherwise, spawns `sbx run --name claude-skills-skillwalker claude [:ro ...]` with inherited stdio for interactive OAuth login. Prints progress messages to stderr. +Checks whether the sandbox already exists via an internal `sandboxExists()` helper (runs `sbx ls --quiet`). If found, prints a help message to stderr explaining how to recreate it, and returns early. Otherwise, spawns `sbx run --name claude-skills-skillwalker claude [:ro ...]` with inherited stdio for interactive OAuth login. Prints progress messages to stderr. Throws `SandboxError` if `sbx run` exits non-zero. `extraWorkspaces` are mounted read-only after `repoRoot` (`:ro`); any already inside `repoRoot` are skipped. The CLI passes the directory holding `sandbox-run.sh` and `sandbox-extract.sh` (`sandboxScriptsDir` from `@testdouble/claude-integration`). `execInSandbox` runs those scripts by their host path, and the sandbox only sees host paths under a mounted workspace, so without this mount every test run fails whenever the target repo is not the skillwalker repo. diff --git a/docs/sandbox-integration.md b/docs/sandbox-integration.md index 0d6ad6c..2525204 100644 --- a/docs/sandbox-integration.md +++ b/docs/sandbox-integration.md @@ -171,7 +171,7 @@ When a scaffold path is provided, it copies the scaffold into a fresh temp direc export async function createSandbox(repoRoot: string, extraWorkspaces: string[] = []): Promise ``` -Checks if the sandbox already exists via an internal `sandboxExists()` helper. If it does, prints a help message to stderr and returns. Otherwise, spawns `sbx run --name claude-skills-skillwalker claude [:ro ...]` with inherited stdio for interactive OAuth login. +Checks if the sandbox already exists via an internal `sandboxExists()` helper. If it does, prints a help message to stderr and returns. Otherwise, spawns `sbx run --name claude-skills-skillwalker claude [:ro ...]` with inherited stdio for interactive OAuth login. If `sbx run` exits non-zero, it throws `SandboxError` instead of reporting the sandbox as ready. `extraWorkspaces` are mounted read-only after `repoRoot` (`:ro`); any already inside `repoRoot` are skipped. The CLI passes the directory holding `sandbox-run.sh` and `sandbox-extract.sh` (`sandboxScriptsDir` from `@testdouble/claude-integration`). `execInSandbox` runs those scripts by their host path, and the sandbox only sees host paths under a mounted workspace, so without this mount every test run fails whenever the target repo is not the skillwalker repo. diff --git a/docs/web.md b/docs/web.md index 587a2c2..2934454 100644 --- a/docs/web.md +++ b/docs/web.md @@ -16,7 +16,8 @@ Change this package when you need to touch the dashboard's Hono API server, the - Compiled as a standalone Bun executable (`skillwalker-web`) with embedded client assets via Bun's `{ type: 'file' }` imports Key files: -- `packages/web/src/server/index.ts` — Server entry point, CLI arg parsing, route registration, embedded asset serving +- `packages/web/src/server/index.ts` — Server entry point, CLI arg parsing, embedded asset serving +- `packages/web/src/server/app.ts` — `createApp(dataDir)`: the Hono app with the API routes and `jsonErrorHandler` - `packages/web/src/client/index.tsx` — Client entry point, router and page registration - `packages/web/src/server/routes/test-runs.ts` — Test run list and detail API endpoints - `packages/web/src/server/routes/scil.ts` — SCIL history and detail API endpoints @@ -68,7 +69,8 @@ flowchart TB ### Backend | File | Purpose | |------|---------| -| `packages/web/src/server/index.ts` | Server entry point — Yargs CLI, Hono app, route registration, embedded static asset serving, SPA fallback | +| `packages/web/src/server/index.ts` | Server entry point — Yargs CLI, embedded static asset serving, SPA fallback | +| `packages/web/src/server/app.ts` | `createApp(dataDir)` — Hono app with the API routes and `jsonErrorHandler` | | `packages/web/src/server/routes/test-runs.ts` | `getTestRuns` and `getTestRunById` handlers delegating to `queryTestRunSummaries` / `queryTestRunDetails` | | `packages/web/src/server/routes/scil.ts` | `getScilHistory` and `getScilRunById` handlers delegating to `queryScilHistory` / `queryScilRunDetails` | | `packages/web/src/server/routes/analytics.ts` | `getPerTestAnalytics` handler with optional `?eval=` query param filter | @@ -209,7 +211,7 @@ interface ScilSummaryRow { #### Server Startup and Asset Embedding -The server entry (`packages/web/src/server/index.ts`) uses Yargs to parse `--port` and `--data-dir` CLI arguments. It registers the API routes, a `jsonErrorHandler` via `app.onError` so unexpected errors reach the client as JSON, and the static asset routes. The client build output (`dist/client/`) is embedded using Bun's `import ... with { type: 'file' }` syntax, which resolves to `$bunfs` paths in compiled standalone executables. A SPA fallback (`/*`) serves `index.html` for all unmatched paths, enabling client-side routing. +The server entry (`packages/web/src/server/index.ts`) uses Yargs to parse `--port` and `--data-dir` CLI arguments. It builds the app with `createApp(dataDir)` from `app.ts`, which registers the API routes and a `jsonErrorHandler` via `app.onError` so unexpected errors reach the client as JSON. The entry then adds the static asset routes. The client build output (`dist/client/`) is embedded using Bun's `import ... with { type: 'file' }` syntax, which resolves to `$bunfs` paths in compiled standalone executables. A SPA fallback (`/*`) serves `index.html` for all unmatched paths, enabling client-side routing. #### Route Handler Pattern @@ -429,6 +431,7 @@ flowchart TB - `packages/web/src/server/routes/scil.test.ts` / `acil.test.ts` — Test the SCIL and ACIL history and detail handlers, including 404 for malformed run IDs - `packages/web/src/server/routes/error-handler.test.ts` — Tests that `jsonErrorHandler` logs the error and returns a JSON 500 - `packages/web/src/server/routes/analytics.test.ts` — Tests `getPerTestAnalytics` including eval filter behavior +- `packages/web/src/server/app.integration.test.ts` — Sends real requests through `createApp` against Parquet built from real JSONL, with nothing mocked. Covers whole-number scores, accuracies, and costs, which DuckDB returns as `BigInt` unless the query casts them, and an empty data directory ### Frontend - `packages/web/src/client/lib/fetch-json.test.ts` — Tests `fetchJson` with OK, non-OK JSON, and non-OK plain-text responses, using `vi.stubGlobal('fetch', ...)`