From d801b91b863c5fdfd857da059e64cf5dc531d5e3 Mon Sep 17 00:00:00 2001 From: Pablo P Varela Date: Wed, 17 Jun 2026 15:29:17 +0200 Subject: [PATCH] feat: establish verification baseline (#62) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add vitest.config.ts scoping tests to src/**/*.test.ts - Add typecheck, test, check scripts to package.json - Fix all TypeScript errors (chat.ts null checks, exactOptionalPropertyTypes, missing job types, store handler types) - Convert hand-rolled test files to vitest (job-state, composeFailure) - Fix 3 sanitize test failures (min-length regex match, undefined error handling) - Gate CI: check job with biome+typecheck+test+build before Docker publish on push, also runs on PRs - Fix AGENTS.md: Astro v5 → v6, add verification commands table - Align queue job type allowlists across status.ts and jobs-stream.ts --- .github/workflows/docker-build.yml | 25 ++++ AGENTS.md | 15 ++- package.json | 3 + src/actions/chat.ts | 27 ++++- src/lib/chat/attachmentPromptText.ts | 2 +- src/pages/api/queue/jobs-stream.ts | 2 + src/server/docker/composeFailure.test.ts | 82 ++++--------- src/server/opencode/normalize.ts | 2 +- src/server/queue/job-state.test.ts | 112 +++++++----------- src/server/settings/status.ts | 1 + src/server/utils/sanitize.test.ts | 4 +- src/server/utils/sanitize.ts | 2 +- .../useChatStore.interactionHandlers.ts | 44 +++---- vitest.config.ts | 13 ++ 14 files changed, 176 insertions(+), 158 deletions(-) create mode 100644 vitest.config.ts diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 36d3f52f..9267e60b 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -3,9 +3,34 @@ name: Build and Push Docker Image on: push: branches: [main] + pull_request: + branches: [main] jobs: + check: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup pnpm + uses: pnpm/action-setup@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "22" + cache: "pnpm" + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Run checks + run: pnpm check + build-and-push: + needs: check + if: github.event_name == 'push' runs-on: ubuntu-latest permissions: contents: read diff --git a/AGENTS.md b/AGENTS.md index 88e393a6..92a3e45b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -42,6 +42,19 @@ An open-source, self-hostable web UI for building and deploying websites with AI * **API calls** should be abstracted into service functions rather than scattered directly in components. * **State management** should use `zustand` to create domain-specific stores for complex state on components, or custom hooks if a store is not needed. +## Verification Commands + +Run these commands to verify your changes before pushing: + +| Command | Purpose | +|---|---| +| `pnpm dev` | Start the dev server | +| `pnpm test` | Run vitest test suites (`src/**/*.test.ts`) | +| `pnpm typecheck` | Run TypeScript type checking | +| `pnpm check` | Full verification (biome lint + typecheck + tests + build) | +| `pnpm format` | Auto-format and fix lint issues with Biome | +| `pnpm build` | Production build | + ## Tech Stack ### Package Manager @@ -51,7 +64,7 @@ An open-source, self-hostable web UI for building and deploying websites with AI **TypeScript** - Used throughout for type safety. Strict mode enabled. ### Framework -**Astro v5** - Full-stack framework providing: +**Astro v6** - Full-stack framework providing: - Astro Pages for file-based routing - Astro Actions for type-safe server-side operations like CRUD - React integration for interactive components diff --git a/package.json b/package.json index a923b629..2136772f 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,9 @@ "shadcn:update": "bash scripts/shadcn-update.sh", "icons:generate": "node scripts/generate-icons.js", "format": "pnpm exec biome check --write src/", + "typecheck": "astro sync && tsc --noEmit", + "test": "vitest run", + "check": "pnpm exec biome check src/ && pnpm typecheck && pnpm test && pnpm build", "drizzle:push": "drizzle-kit push", "drizzle:generate": "drizzle-kit generate", "drizzle:migrate": "drizzle-kit migrate", diff --git a/src/actions/chat.ts b/src/actions/chat.ts index 8b16448f..40f5ded5 100644 --- a/src/actions/chat.ts +++ b/src/actions/chat.ts @@ -17,7 +17,13 @@ async function authorizeSession(projectId: string, userId: string) { }); } const project = await getProjectById(projectId); - if (!project?.bootstrapSessionId) { + if (!project) { + throw new ActionError({ + code: "NOT_FOUND", + message: "Project not found", + }); + } + if (!project.bootstrapSessionId) { throw new ActionError({ code: "NOT_FOUND", message: "No active session for this project", @@ -28,6 +34,13 @@ async function authorizeSession(projectId: string, userId: string) { async function assertRestoreAllowed(projectId: string, userId: string) { const project = await authorizeSession(projectId, userId); + const sessionId = project.bootstrapSessionId; + if (!sessionId) { + throw new ActionError({ + code: "NOT_FOUND", + message: "No active session for this project", + }); + } const restoreSafety = await getRestoreSafetyStatus(project); if (!restoreSafety.canRestore) { throw new ActionError({ @@ -39,7 +52,7 @@ async function assertRestoreAllowed(projectId: string, userId: string) { } return { project, - sessionId: project.bootstrapSessionId, + sessionId, restoreSafety, }; } @@ -98,24 +111,26 @@ export const chat = { messageID: messageId, }); - let serverRevertMessageId: string | null = messageId; + let revertMessageId = messageId; try { const info = await client.session.get({ sessionID: sessionId }); const data = (info as { data?: { revert?: { messageID?: string } } }) .data; - serverRevertMessageId = data?.revert?.messageID ?? messageId; + if (data?.revert?.messageID) { + revertMessageId = data.revert.messageID; + } } catch (error) { logger.debug({ error }, "Failed to refetch session after revert"); } logger.debug( - { projectId, sessionId, messageId, serverRevertMessageId }, + { projectId, sessionId, messageId, revertMessageId }, "Reverted session to message", ); return { success: true as const, - revertMessageId: serverRevertMessageId, + revertMessageId, }; }, }), diff --git a/src/lib/chat/attachmentPromptText.ts b/src/lib/chat/attachmentPromptText.ts index 0feb1f37..4c24ba00 100644 --- a/src/lib/chat/attachmentPromptText.ts +++ b/src/lib/chat/attachmentPromptText.ts @@ -3,7 +3,7 @@ import type { PromptAttachmentPart } from "@/types/message"; interface TextAttachmentInput { filename: string; - textContent?: string; + textContent?: string | undefined; } export function formatTextAttachmentForPrompt({ diff --git a/src/pages/api/queue/jobs-stream.ts b/src/pages/api/queue/jobs-stream.ts index 3396fcd8..a294cbce 100644 --- a/src/pages/api/queue/jobs-stream.ts +++ b/src/pages/api/queue/jobs-stream.ts @@ -19,6 +19,8 @@ const PAGE_SIZE = 25; function validateJobType(typeParam: string): QueueJobType | undefined { const allowedTypes = [ "project.create", + "project.identityGenerate", + "project.descriptionSync", "project.delete", "projects.deleteAllForUser", "docker.composeUp", diff --git a/src/server/docker/composeFailure.test.ts b/src/server/docker/composeFailure.test.ts index c56e7dec..b260be6c 100644 --- a/src/server/docker/composeFailure.test.ts +++ b/src/server/docker/composeFailure.test.ts @@ -1,59 +1,25 @@ -import { - type ComposeFailureKind, - classifyComposeFailure, -} from "./composeFailure"; - -interface TestResult { - test: string; - passed: boolean; - error?: string; -} - -const results: TestResult[] = []; - -function test(name: string, fn: () => void): void { - try { - fn(); - results.push({ test: name, passed: true }); - console.log(`PASS ${name}`); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - results.push({ test: name, passed: false, error: message }); - console.log(`FAIL ${name}: ${message}`); - } -} - -function assertEqual(actual: unknown, expected: unknown): void { - if (actual !== expected) { - throw new Error(`Expected ${String(expected)}, got ${String(actual)}`); - } -} - -function assertKind(rawOutput: string, expected: ComposeFailureKind): void { - const diagnostic = classifyComposeFailure(rawOutput); - assertEqual(diagnostic.kind, expected); -} - -test("classifies cloudflare docker blob timeout as registry timeout", () => { - assertKind( - "failed to do request: Get https://docker-images-prod.abc.r2.cloudflarestorage.com/blob: dial tcp 172.64.66.1:443: i/o timeout", - "registry_timeout", - ); +import { describe, expect, it } from "vitest"; +import { classifyComposeFailure } from "./composeFailure"; + +describe("classifyComposeFailure", () => { + it("classifies cloudflare docker blob timeout as registry_timeout", () => { + const diagnostic = classifyComposeFailure( + "failed to do request: Get https://docker-images-prod.abc.r2.cloudflarestorage.com/blob: dial tcp 172.64.66.1:443: i/o timeout", + ); + expect(diagnostic.kind).toBe("registry_timeout"); + }); + + it("classifies missing container errors as missing_container", () => { + const diagnostic = classifyComposeFailure( + "Error: No such container: doce_preview_1", + ); + expect(diagnostic.kind).toBe("missing_container"); + }); + + it("falls back to generic for unknown failures", () => { + const diagnostic = classifyComposeFailure( + "Service 'preview' failed to build: Build failed", + ); + expect(diagnostic.kind).toBe("generic"); + }); }); - -test("classifies missing container errors", () => { - assertKind("Error: No such container: doce_preview_1", "missing_container"); -}); - -test("falls back to generic for unknown failures", () => { - assertKind("Service 'preview' failed to build: Build failed", "generic"); -}); - -const passed = results.filter((result) => result.passed).length; -const failed = results.length - passed; - -console.log(`\n${passed}/${results.length} tests passed`); - -if (failed > 0) { - process.exitCode = 1; -} diff --git a/src/server/opencode/normalize.ts b/src/server/opencode/normalize.ts index 6530e01a..6484556e 100644 --- a/src/server/opencode/normalize.ts +++ b/src/server/opencode/normalize.ts @@ -52,7 +52,7 @@ export type NormalizedEventType = export interface NormalizedEventEnvelope { type: NormalizedEventType; projectId: string; - sessionId?: string; + sessionId?: string | undefined; time: string; payload: unknown; } diff --git a/src/server/queue/job-state.test.ts b/src/server/queue/job-state.test.ts index 3a89e3f4..0ddc7415 100644 --- a/src/server/queue/job-state.test.ts +++ b/src/server/queue/job-state.test.ts @@ -1,3 +1,4 @@ +import { describe, expect, it } from "vitest"; import type { QueueJob } from "@/server/db/schema"; import { getQueueJobDerivedError, @@ -5,32 +6,6 @@ import { isQueueJobExhausted, } from "./job-state"; -interface TestResult { - test: string; - passed: boolean; - error?: string; -} - -const results: TestResult[] = []; - -function test(name: string, fn: () => void): void { - try { - fn(); - results.push({ test: name, passed: true }); - console.log(`PASS ${name}`); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - results.push({ test: name, passed: false, error: message }); - console.log(`FAIL ${name}: ${message}`); - } -} - -function assertEqual(actual: unknown, expected: unknown): void { - if (actual !== expected) { - throw new Error(`Expected ${String(expected)}, got ${String(actual)}`); - } -} - function buildJob(overrides: Partial): QueueJob { const now = new Date(); @@ -60,51 +35,52 @@ function buildJob(overrides: Partial): QueueJob { }; } -test("queued jobs at max attempts are exhausted", () => { - const job = buildJob({ attempts: 3, maxAttempts: 3, state: "queued" }); - assertEqual(isQueueJobExhausted(job), true); - assertEqual(getQueueJobDerivedState(job), "exhausted"); -}); +describe("job-state", () => { + describe("isQueueJobExhausted", () => { + it("returns true for queued jobs at max attempts", () => { + const job = buildJob({ attempts: 3, maxAttempts: 3, state: "queued" }); + expect(isQueueJobExhausted(job)).toBe(true); + expect(getQueueJobDerivedState(job)).toBe("exhausted"); + }); -test("running jobs are not exhausted", () => { - const job = buildJob({ attempts: 3, maxAttempts: 3, state: "running" }); - assertEqual(isQueueJobExhausted(job), false); - assertEqual(getQueueJobDerivedState(job), "running"); -}); + it("returns false for running jobs even at max attempts", () => { + const job = buildJob({ attempts: 3, maxAttempts: 3, state: "running" }); + expect(isQueueJobExhausted(job)).toBe(false); + expect(getQueueJobDerivedState(job)).toBe("running"); + }); -test("queued jobs with lock owner are not exhausted", () => { - const job = buildJob({ - attempts: 3, - maxAttempts: 3, - state: "queued", - lockedBy: "worker_1", + it("returns false for queued jobs with a lock owner", () => { + const job = buildJob({ + attempts: 3, + maxAttempts: 3, + state: "queued", + lockedBy: "worker_1", + }); + expect(isQueueJobExhausted(job)).toBe(false); + expect(getQueueJobDerivedState(job)).toBe("queued"); + }); }); - assertEqual(isQueueJobExhausted(job), false); - assertEqual(getQueueJobDerivedState(job), "queued"); -}); - -test("derived error uses lastError when present", () => { - const job = buildJob({ - attempts: 3, - maxAttempts: 3, - lastError: "compose failed", - }); - assertEqual(getQueueJobDerivedError(job), "compose failed"); -}); -test("derived error provides fallback for exhausted jobs", () => { - const job = buildJob({ attempts: 3, maxAttempts: 3, state: "queued" }); - assertEqual( - getQueueJobDerivedError(job), - "Job exhausted all retry attempts before it could be marked failed.", - ); -}); - -const passed = results.filter((result) => result.passed).length; -const failed = results.length - passed; + describe("getQueueJobDerivedError", () => { + it("uses lastError when present", () => { + const job = buildJob({ + attempts: 3, + maxAttempts: 3, + lastError: "compose failed", + }); + expect(getQueueJobDerivedError(job)).toBe("compose failed"); + }); -console.log(`\n${passed}/${results.length} tests passed`); + it("provides fallback for exhausted jobs without lastError", () => { + const job = buildJob({ attempts: 3, maxAttempts: 3, state: "queued" }); + expect(getQueueJobDerivedError(job)).toBe( + "Job exhausted all retry attempts before it could be marked failed.", + ); + }); -if (failed > 0) { - process.exitCode = 1; -} + it("returns null for non-exhausted jobs without lastError", () => { + const job = buildJob({ attempts: 1, maxAttempts: 3, state: "queued" }); + expect(getQueueJobDerivedError(job)).toBeNull(); + }); + }); +}); diff --git a/src/server/settings/status.ts b/src/server/settings/status.ts index abf9ea69..2c6fb817 100644 --- a/src/server/settings/status.ts +++ b/src/server/settings/status.ts @@ -87,6 +87,7 @@ function validateJobType(typeParam: string): QueueJobType | undefined { const allowedTypes = [ "project.create", "project.identityGenerate", + "project.descriptionSync", "project.delete", "projects.deleteAllForUser", "docker.composeUp", diff --git a/src/server/utils/sanitize.test.ts b/src/server/utils/sanitize.test.ts index f4f5c1df..fe193fcf 100644 --- a/src/server/utils/sanitize.test.ts +++ b/src/server/utils/sanitize.test.ts @@ -27,7 +27,7 @@ describe("sanitizeMessage", () => { it("should redact multiple sensitive values in one message", () => { const message = - "api_key:sk-1234567890 and password: secret123 and token: abc1234567890123456"; + "api_key:sk-1234567890abcdefghij and password: secret123 and token: abc1234567890123456"; const result = sanitizeMessage(message); expect(result).toContain("api_key:[REDACTED]"); expect(result).toContain("password:[REDACTED]"); @@ -46,7 +46,7 @@ describe("sanitizeMessage", () => { describe("errorToSanitizedMessage", () => { it("should sanitize error messages", () => { - const error = new Error("Failed with api_key:sk-secret123456"); + const error = new Error("Failed with api_key:sk-secret1234567890ab"); expect(errorToSanitizedMessage(error)).toBe( "Failed with api_key:[REDACTED]", ); diff --git a/src/server/utils/sanitize.ts b/src/server/utils/sanitize.ts index 8c6609af..ee71136f 100644 --- a/src/server/utils/sanitize.ts +++ b/src/server/utils/sanitize.ts @@ -174,7 +174,7 @@ export function errorToSanitizedMessage(error: unknown): string { // For other types, convert to string and sanitize try { const str = JSON.stringify(error); - return sanitizeMessage(str); + return sanitizeMessage(str ?? String(error)); } catch { return "[Unable to serialize error]"; } diff --git a/src/stores/useChatStore.interactionHandlers.ts b/src/stores/useChatStore.interactionHandlers.ts index 6d370f98..ed1c08c5 100644 --- a/src/stores/useChatStore.interactionHandlers.ts +++ b/src/stores/useChatStore.interactionHandlers.ts @@ -1,39 +1,43 @@ import type { - ChatStore, - PendingPermissionRequest, - PendingQuestionRequest, - TodoItem, -} from "./useChatStore"; + PermissionRequestPayload, + QuestionRequestPayload, + TodoUpdatedPayload, +} from "@/server/opencode/normalize"; +import type { ChatStore } from "./useChatStore"; type ChatStoreSet = ( partial: Partial | ((state: ChatStore) => Partial), ) => void; -export function handlePermissionRequested( - set: ChatStoreSet, - request: PendingPermissionRequest, -) { - set({ pendingPermission: request }); +/** + * Casts payload to the permission-request shape from normalize. + * Safe because this handler only runs when the event type is + * "chat.permission.requested", which guarantees the shape. + */ +export function handlePermissionRequested(set: ChatStoreSet, payload: unknown) { + const p = payload as PermissionRequestPayload; + set({ pendingPermission: p }); } export function handlePermissionResolved(set: ChatStoreSet) { set({ pendingPermission: null }); } -export function handleQuestionRequested( - set: ChatStoreSet, - request: PendingQuestionRequest, -) { - set({ pendingQuestion: request }); +/** + * Casts payload to the question-request shape from normalize. + * Safe because this handler only runs when the event type is + * "chat.question.requested", which guarantees the shape. + */ +export function handleQuestionRequested(set: ChatStoreSet, payload: unknown) { + const p = payload as QuestionRequestPayload; + set({ pendingQuestion: p }); } export function handleQuestionResolved(set: ChatStoreSet) { set({ pendingQuestion: null }); } -export function handleTodoUpdated( - set: ChatStoreSet, - payload: { todos: TodoItem[] }, -) { - set({ todos: payload.todos ?? [] }); +export function handleTodoUpdated(set: ChatStoreSet, payload: unknown) { + const p = payload as TodoUpdatedPayload; + set({ todos: p.todos ?? [] }); } diff --git a/vitest.config.ts b/vitest.config.ts new file mode 100644 index 00000000..41e4e7c5 --- /dev/null +++ b/vitest.config.ts @@ -0,0 +1,13 @@ +import { defineConfig } from "vitest/config"; +import path from "node:path"; + +export default defineConfig({ + test: { + include: ["src/**/*.test.ts"], + }, + resolve: { + alias: { + "@": path.resolve(__dirname, "src"), + }, + }, +});