Merge pull request #1 from OpenKnots/okcode/codebase-improvements

Add CI coverage and route error boundaries

Author: BunsDev

.github/workflows/ci.yml

@@ -0,0 +1,27 @@
+name: CI
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+jobs:
+  check:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: pnpm/action-setup@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 18
+          cache: pnpm
+
+      - run: pnpm install --frozen-lockfile
+
+      - run: pnpm exec secretlint "**/*"
+
+      - run: pnpm test -- --run

__tests__/api-contract.test.ts

@@ -0,0 +1,67 @@
+import { describe, it, expect } from "vitest";
+import { ok, fail, ApiValidationError } from "@/lib/opentrust/api-contract";
+
+describe("ok()", () => {
+  it("wraps data in success envelope", () => {
+    const result = ok({ foo: "bar" });
+    expect(result.ok).toBe(true);
+    expect(result.data).toEqual({ foo: "bar" });
+  });
+
+  it("works with null data", () => {
+    const result = ok(null);
+    expect(result.ok).toBe(true);
+    expect(result.data).toBeNull();
+  });
+});
+
+describe("fail()", () => {
+  it("formats ApiValidationError with code and message", () => {
+    const error = new ApiValidationError("Bad input", { code: "invalid_input", status: 400 });
+    const result = fail(error);
+
+    expect(result.ok).toBe(false);
+    expect(result.error.code).toBe("invalid_input");
+    expect(result.error.message).toBe("Bad input");
+    expect(result.status).toBe(400);
+  });
+
+  it("includes details when provided", () => {
+    const error = new ApiValidationError("Missing field", {
+      code: "missing_field",
+      details: { field: "title" },
+    });
+    const result = fail(error);
+    expect(result.error.details).toEqual({ field: "title" });
+  });
+
+  it("returns internal_error for unknown errors", () => {
+    const result = fail(new Error("something broke"));
+    expect(result.ok).toBe(false);
+    expect(result.error.code).toBe("internal_error");
+    expect(result.status).toBe(500);
+    // Should NOT leak the original error message
+    expect(result.error.message).not.toContain("something broke");
+  });
+
+  it("handles non-Error thrown values", () => {
+    const result = fail("string error");
+    expect(result.ok).toBe(false);
+    expect(result.error.code).toBe("internal_error");
+  });
+});
+
+describe("ApiValidationError", () => {
+  it("defaults to status 400 and code invalid_request", () => {
+    const error = new ApiValidationError("test");
+    expect(error.status).toBe(400);
+    expect(error.code).toBe("invalid_request");
+    expect(error.name).toBe("ApiValidationError");
+  });
+
+  it("accepts custom status and code", () => {
+    const error = new ApiValidationError("forbidden", { status: 403, code: "forbidden" });
+    expect(error.status).toBe(403);
+    expect(error.code).toBe("forbidden");
+  });
+});

__tests__/artifact-extract.test.ts

@@ -0,0 +1,50 @@
+import { describe, it, expect } from "vitest";
+import { extractArtifactsFromText } from "@/lib/opentrust/artifact-extract";
+
+describe("extractArtifactsFromText", () => {
+  it("extracts URLs", () => {
+    const text = "Check out https://example.com/docs and http://localhost:3000";
+    const artifacts = extractArtifactsFromText(text);
+
+    const urls = artifacts.filter((a) => a.kind === "url");
+    expect(urls.length).toBeGreaterThanOrEqual(2);
+    expect(urls.some((a) => a.uri.includes("example.com"))).toBe(true);
+  });
+
+  it("extracts GitHub-style repo references", () => {
+    const text = "See OpenKnots/OpenTrust for details";
+    const artifacts = extractArtifactsFromText(text);
+
+    const repos = artifacts.filter((a) => a.kind === "repo");
+    expect(repos.some((a) => a.uri === "OpenKnots/OpenTrust")).toBe(true);
+  });
+
+  it("extracts doc file references", () => {
+    const text = "Updated docs/ARCHITECTURE.md and lib/opentrust/db.ts";
+    const artifacts = extractArtifactsFromText(text);
+
+    const docs = artifacts.filter((a) => a.kind === "doc");
+    expect(docs.some((a) => a.uri.includes("ARCHITECTURE.md"))).toBe(true);
+    expect(docs.some((a) => a.uri.includes("db.ts"))).toBe(true);
+  });
+
+  it("deduplicates artifacts by kind+uri", () => {
+    const text = "See https://example.com and also https://example.com again";
+    const artifacts = extractArtifactsFromText(text);
+
+    const urls = artifacts.filter((a) => a.kind === "url" && a.uri.includes("example.com"));
+    expect(urls).toHaveLength(1);
+  });
+
+  it("limits to 24 artifacts", () => {
+    const urls = Array.from({ length: 30 }, (_, i) => `https://example.com/page${i}`).join(" ");
+    const artifacts = extractArtifactsFromText(urls);
+    expect(artifacts.length).toBeLessThanOrEqual(24);
+  });
+
+  it("returns empty for text with no artifacts", () => {
+    const artifacts = extractArtifactsFromText("Just plain text with no links or references");
+    // May or may not have matches depending on regex — at minimum should not throw
+    expect(Array.isArray(artifacts)).toBe(true);
+  });
+});

__tests__/auth-rate-limit.test.ts

@@ -0,0 +1,80 @@
+import { describe, it, expect, beforeEach } from "vitest";
+import {
+  getLoginRateLimit,
+  recordLoginFailure,
+  clearLoginFailures,
+  getRateLimitKey,
+} from "@/lib/opentrust/auth-rate-limit";
+
+describe("getRateLimitKey", () => {
+  it("returns the IP when provided", () => {
+    expect(getRateLimitKey("192.168.1.1")).toBe("192.168.1.1");
+  });
+
+  it("trims whitespace", () => {
+    expect(getRateLimitKey("  10.0.0.1  ")).toBe("10.0.0.1");
+  });
+
+  it('returns "unknown" for null/undefined/empty', () => {
+    expect(getRateLimitKey(null)).toBe("unknown");
+    expect(getRateLimitKey(undefined)).toBe("unknown");
+    expect(getRateLimitKey("")).toBe("unknown");
+  });
+});
+
+describe("getLoginRateLimit", () => {
+  beforeEach(() => {
+    clearLoginFailures("test-ip-limit");
+  });
+
+  it("starts with 5 remaining attempts", () => {
+    const state = getLoginRateLimit("test-ip-limit");
+    expect(state.remaining).toBe(5);
+    expect(state.count).toBe(0);
+  });
+
+  it("returns a future resetAt timestamp", () => {
+    const state = getLoginRateLimit("test-ip-limit");
+    expect(state.resetAt).toBeGreaterThan(Date.now());
+  });
+});
+
+describe("recordLoginFailure", () => {
+  beforeEach(() => {
+    clearLoginFailures("test-ip-fail");
+  });
+
+  it("increments the failure count", () => {
+    recordLoginFailure("test-ip-fail");
+    const state = getLoginRateLimit("test-ip-fail");
+    expect(state.remaining).toBe(4);
+  });
+
+  it("marks as limited after 5 failures", () => {
+    for (let i = 0; i < 4; i++) {
+      const result = recordLoginFailure("test-ip-fail");
+      expect(result.limited).toBe(false);
+    }
+    const fifth = recordLoginFailure("test-ip-fail");
+    expect(fifth.limited).toBe(true);
+  });
+
+  it("returns 0 remaining after limit reached", () => {
+    for (let i = 0; i < 5; i++) {
+      recordLoginFailure("test-ip-fail");
+    }
+    const state = getLoginRateLimit("test-ip-fail");
+    expect(state.remaining).toBe(0);
+  });
+});
+
+describe("clearLoginFailures", () => {
+  it("resets the failure count", () => {
+    recordLoginFailure("test-ip-clear");
+    recordLoginFailure("test-ip-clear");
+    clearLoginFailures("test-ip-clear");
+
+    const state = getLoginRateLimit("test-ip-clear");
+    expect(state.remaining).toBe(5);
+  });
+});

__tests__/db.test.ts

@@ -0,0 +1,123 @@
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import Database from "better-sqlite3";
+import { existsSync, mkdirSync, readFileSync, rmSync } from "node:fs";
+import path from "node:path";
+
+/**
+ * These tests exercise the database layer against a real SQLite instance
+ * using a temporary directory, bypassing the singleton to avoid polluting
+ * the development database.
+ */
+
+const TEST_DIR = path.join(process.cwd(), "storage", "__test__");
+const TEST_DB_PATH = path.join(TEST_DIR, "test.sqlite");
+const MIGRATION_PATH = path.join(process.cwd(), "db", "0001_init.sql");
+
+function freshDb() {
+  if (!existsSync(TEST_DIR)) mkdirSync(TEST_DIR, { recursive: true });
+  if (existsSync(TEST_DB_PATH)) rmSync(TEST_DB_PATH);
+
+  const db = new Database(TEST_DB_PATH);
+  db.exec("PRAGMA journal_mode=WAL;");
+  db.exec("PRAGMA foreign_keys=ON;");
+  return db;
+}
+
+describe("database initialization", () => {
+  let db: Database.Database;
+
+  beforeEach(() => {
+    db = freshDb();
+  });
+
+  afterEach(() => {
+    db.close();
+    if (existsSync(TEST_DB_PATH)) rmSync(TEST_DB_PATH);
+  });
+
+  it("enables WAL journal mode", () => {
+    const row = db.prepare("PRAGMA journal_mode;").get() as { journal_mode: string };
+    expect(row.journal_mode).toBe("wal");
+  });
+
+  it("enables foreign keys", () => {
+    const row = db.prepare("PRAGMA foreign_keys;").get() as { foreign_keys: number };
+    expect(row.foreign_keys).toBe(1);
+  });
+
+  it("applies the init migration without errors", () => {
+    const migrationSql = readFileSync(MIGRATION_PATH, "utf8");
+    expect(() => db.exec(migrationSql)).not.toThrow();
+  });
+
+  it("creates expected core tables after migration", () => {
+    const migrationSql = readFileSync(MIGRATION_PATH, "utf8");
+    db.exec(migrationSql);
+
+    const tables = db
+      .prepare("SELECT name FROM sqlite_master WHERE type = 'table' ORDER BY name;")
+      .all() as { name: string }[];
+
+    const tableNames = tables.map((t) => t.name);
+
+    expect(tableNames).toContain("sessions");
+    expect(tableNames).toContain("traces");
+    expect(tableNames).toContain("events");
+    expect(tableNames).toContain("tool_calls");
+    expect(tableNames).toContain("workflow_runs");
+    expect(tableNames).toContain("workflow_steps");
+    expect(tableNames).toContain("artifacts");
+    expect(tableNames).toContain("capabilities");
+  });
+
+  it("migration is idempotent — running twice does not error", () => {
+    const migrationSql = readFileSync(MIGRATION_PATH, "utf8");
+    db.exec(migrationSql);
+    expect(() => db.exec(migrationSql)).not.toThrow();
+  });
+});
+
+describe("transaction support", () => {
+  let db: Database.Database;
+
+  beforeEach(() => {
+    db = freshDb();
+    const migrationSql = readFileSync(MIGRATION_PATH, "utf8");
+    db.exec(migrationSql);
+  });
+
+  afterEach(() => {
+    db.close();
+    if (existsSync(TEST_DB_PATH)) rmSync(TEST_DB_PATH);
+  });
+
+  it("commits all writes on success", () => {
+    const run = db.transaction(() => {
+      db.prepare(
+        "INSERT INTO capabilities (id, kind, name, metadata_json) VALUES (?, ?, ?, ?)",
+      ).run("test:a", "skill", "a", "{}");
+      db.prepare(
+        "INSERT INTO capabilities (id, kind, name, metadata_json) VALUES (?, ?, ?, ?)",
+      ).run("test:b", "skill", "b", "{}");
+    });
+
+    run();
+
+    const rows = db.prepare("SELECT id FROM capabilities WHERE id LIKE 'test:%'").all() as { id: string }[];
+    expect(rows).toHaveLength(2);
+  });
+
+  it("rolls back all writes on failure", () => {
+    const run = db.transaction(() => {
+      db.prepare(
+        "INSERT INTO capabilities (id, kind, name, metadata_json) VALUES (?, ?, ?, ?)",
+      ).run("test:c", "skill", "c", "{}");
+      throw new Error("deliberate failure");
+    });
+
+    expect(() => run()).toThrow("deliberate failure");
+
+    const rows = db.prepare("SELECT id FROM capabilities WHERE id = 'test:c'").all();
+    expect(rows).toHaveLength(0);
+  });
+});