From 58ffa68f4c13d96289e1f7de84c14979d62ec330 Mon Sep 17 00:00:00 2001 From: igm Date: Mon, 12 Jan 2026 15:35:02 +0800 Subject: [PATCH 1/2] Add tests for @reviq/db package - token.test.ts: Unit tests for generateToken, parseToken, hashToken - client.test.ts: Tests for createDb validation and e2e connectivity - execute-bootstrap.test.ts: Comprehensive e2e tests for bootstrap operation including overwrite mode and related record cleanup Coverage: client.ts 100%, token.ts 100%, execute-bootstrap.ts 98.69% Co-Authored-By: Claude Opus 4.5 --- packages/db/src/client.test.ts | 56 ++ .../db/src/helpers/execute-bootstrap.test.ts | 697 ++++++++++++++++++ packages/db/src/helpers/token.test.ts | 136 ++++ 3 files changed, 889 insertions(+) create mode 100644 packages/db/src/client.test.ts create mode 100644 packages/db/src/helpers/execute-bootstrap.test.ts create mode 100644 packages/db/src/helpers/token.test.ts diff --git a/packages/db/src/client.test.ts b/packages/db/src/client.test.ts new file mode 100644 index 0000000..ce46301 --- /dev/null +++ b/packages/db/src/client.test.ts @@ -0,0 +1,56 @@ +/** + * Tests for the Kysely database client + */ + +import { describe, expect, test } from "bun:test"; +import { createDb } from "./client.js"; + +/** + * Skip flag for database-dependent tests. + * Tests are skipped when TEST_DATABASE_URL is not configured. + */ +const SKIP_DB_TESTS = !process.env.TEST_DATABASE_URL; + +const describeE2E = describe.skipIf(SKIP_DB_TESTS); + +describe("createDb", () => { + test("throws error for empty connection string", () => { + expect(() => createDb("")).toThrow("Database connection string is required"); + }); + + test("throws error for null-ish connection string", () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument -- testing edge case + expect(() => createDb(null as any)).toThrow( + "Database connection string is required", + ); + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument -- testing edge case + expect(() => createDb(undefined as any)).toThrow( + "Database connection string is required", + ); + }); +}); + +describeE2E("[e2e] createDb with real database", () => { + test("creates working database connection", async () => { + const testUrl = process.env.TEST_DATABASE_URL; + if (!testUrl) { + throw new Error("TEST_DATABASE_URL not set"); + } + + const db = createDb(testUrl); + + try { + // Verify the connection works by executing a simple query + const result = await db + .selectFrom("users") + .select(["id"]) + .limit(1) + .execute(); + + // Should return an array (may be empty) + expect(Array.isArray(result)).toBe(true); + } finally { + await db.destroy(); + } + }); +}); diff --git a/packages/db/src/helpers/execute-bootstrap.test.ts b/packages/db/src/helpers/execute-bootstrap.test.ts new file mode 100644 index 0000000..f354bce --- /dev/null +++ b/packages/db/src/helpers/execute-bootstrap.test.ts @@ -0,0 +1,697 @@ +/** + * Tests for the bootstrap operation + * + * These tests use a real PostgreSQL database to test the executeBootstrap function. + */ + +import type { Database } from "@reviq/db-schema"; +import type { Kysely } from "kysely"; +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { sql } from "kysely"; +import { createDb } from "../client.js"; +import { executeBootstrap } from "./execute-bootstrap.js"; +import { hashToken, parseToken, TOKEN_PREFIX } from "./token.js"; + +/** + * Skip flag for database-dependent tests. + * Tests are skipped when TEST_DATABASE_URL is not configured. + */ +const SKIP_DB_TESTS = !process.env.TEST_DATABASE_URL; + +const describeE2E = describe.skipIf(SKIP_DB_TESTS); + +/** Tables to truncate between tests */ +const TABLES_TO_TRUNCATE = [ + "sessions", + "api_tokens", + "login_requests", + "passkeys", + "user_devices", + "webauthn_challenges", + "email_verifications", + "password_resets", + "org_invites", + "org_sites", + "org_members", + "orgs", + "users", +] as const; + +/** Generate unique test ID */ +let testCounter = 0; +const uniqueTestId = (): string => { + const timestamp = Date.now(); + testCounter++; + return `${timestamp}-${testCounter.toString()}`; +}; + +/** Truncate all tables */ +async function truncateAllTables(db: Kysely): Promise { + const tableList = TABLES_TO_TRUNCATE.join(", "); + await sql`TRUNCATE ${sql.raw(tableList)} RESTART IDENTITY CASCADE`.execute( + db, + ); +} + +/** Signal for transaction rollback */ +class RollbackSignal extends Error { + constructor() { + super("RollbackSignal"); + this.name = "RollbackSignal"; + } +} + +/** Run test in transaction that auto-rollbacks */ +async function withTestTransaction( + db: Kysely, + testFn: (trx: Kysely) => Promise, +): Promise { + let result: T | undefined; + + try { + await db.transaction().execute(async (trx) => { + result = await testFn(trx); + throw new RollbackSignal(); + }); + } catch (e) { + if (!(e instanceof RollbackSignal)) { + throw e; + } + } + + return result; +} + +describeE2E("[e2e] executeBootstrap", () => { + let db: Kysely; + + beforeAll(async () => { + const testUrl = process.env.TEST_DATABASE_URL; + if (!testUrl) { + throw new Error("TEST_DATABASE_URL not set"); + } + db = createDb(testUrl); + await truncateAllTables(db); + }); + + afterAll(async () => { + if (db) { + await truncateAllTables(db); + await db.destroy(); + } + }); + + test("creates superuser with correct email and password", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + }); + + expect(result.user.email).toBe("admin@example.com"); + + // Verify user in database + const user = await trx + .selectFrom("users") + .where("id", "=", result.user.id) + .selectAll() + .executeTakeFirstOrThrow(); + + expect(user.email).toBe("admin@example.com"); + expect(user.is_superuser).toBe(true); + expect(user.email_verified_at).not.toBeNull(); + expect(user.password_hash).not.toBeNull(); + }); + }); + + test("normalizes email to lowercase", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "ADMIN@EXAMPLE.COM", + password: "password123", + }); + + expect(result.user.email).toBe("admin@example.com"); + }); + }); + + test("creates organization with default slug and name", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + }); + + expect(result.org.slug).toBe("reviq"); + + // Verify org in database + const org = await trx + .selectFrom("orgs") + .where("id", "=", result.org.id) + .selectAll() + .executeTakeFirstOrThrow(); + + expect(org.slug).toBe("reviq"); + expect(org.display_name).toBe("RevIQ"); + }); + }); + + test("creates organization with custom slug and name", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + orgSlug: "custom-org", + orgDisplayName: "Custom Organization", + }); + + expect(result.org.slug).toBe("custom-org"); + + const org = await trx + .selectFrom("orgs") + .where("id", "=", result.org.id) + .selectAll() + .executeTakeFirstOrThrow(); + + expect(org.slug).toBe("custom-org"); + expect(org.display_name).toBe("Custom Organization"); + }); + }); + + test("adds user as owner of organization", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + }); + + const membership = await trx + .selectFrom("org_members") + .where("user_id", "=", result.user.id) + .where("org_id", "=", result.org.id) + .selectAll() + .executeTakeFirstOrThrow(); + + expect(membership.role).toBe("owner"); + }); + }); + + test("creates API token with correct properties", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + }); + + // Token should be parseable + expect(result.token.startsWith(TOKEN_PREFIX)).toBe(true); + expect(parseToken(result.token)).not.toBeNull(); + + // Token should be stored as hash in database + const tokenRecord = await trx + .selectFrom("api_tokens") + .where("user_id", "=", result.user.id) + .selectAll() + .executeTakeFirstOrThrow(); + + expect(tokenRecord.token_hash).toBe(hashToken(result.token)); + expect(tokenRecord.name).toBe("CLI bootstrap token"); + }); + }); + + test("creates API token with custom name", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + tokenName: "Custom Token Name", + }); + + const tokenRecord = await trx + .selectFrom("api_tokens") + .where("user_id", "=", result.user.id) + .selectAll() + .executeTakeFirstOrThrow(); + + expect(tokenRecord.name).toBe("Custom Token Name"); + }); + }); + + test("creates API token with custom expiration", async () => { + await withTestTransaction(db, async (trx) => { + const beforeCreate = Date.now(); + + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + tokenExpirationDays: 30, + }); + + const tokenRecord = await trx + .selectFrom("api_tokens") + .where("user_id", "=", result.user.id) + .selectAll() + .executeTakeFirstOrThrow(); + + const expectedMin = beforeCreate + 30 * 24 * 60 * 60 * 1000 - 1000; + const expectedMax = beforeCreate + 30 * 24 * 60 * 60 * 1000 + 5000; + + expect(tokenRecord.expires_at.getTime()).toBeGreaterThan(expectedMin); + expect(tokenRecord.expires_at.getTime()).toBeLessThan(expectedMax); + }); + }); + + test("throws error for password less than 8 characters", async () => { + await withTestTransaction(db, async (trx) => { + await expect( + executeBootstrap(trx, { + email: "admin@example.com", + password: "short", + }), + ).rejects.toThrow("Password must be at least 8 characters"); + }); + }); + + test("throws error for password exactly 7 characters", async () => { + await withTestTransaction(db, async (trx) => { + await expect( + executeBootstrap(trx, { + email: "admin@example.com", + password: "1234567", + }), + ).rejects.toThrow("Password must be at least 8 characters"); + }); + }); + + test("accepts password exactly 8 characters", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "12345678", + }); + + expect(result.user.email).toBe("admin@example.com"); + }); + }); + + test("throws error for invalid email without @", async () => { + await withTestTransaction(db, async (trx) => { + await expect( + executeBootstrap(trx, { + email: "invalidemail", + password: "password123", + }), + ).rejects.toThrow("Invalid email address"); + }); + }); + + test("accepts email with @ symbol", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "valid@email", + password: "password123", + }); + + expect(result.user.email).toBe("valid@email"); + }); + }); + + test("throws error if user already exists (normal mode)", async () => { + await withTestTransaction(db, async (trx) => { + // Create the first user + await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + orgSlug: "org1", + }); + + // Attempt to create the same user again + await expect( + executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + orgSlug: "org2", + }), + ).rejects.toThrow("User with email admin@example.com already exists"); + }); + }); + + test("overwrites existing user in dangerouslyOverwriteExisting mode", async () => { + await withTestTransaction(db, async (trx) => { + // Create the first user + const result1 = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + orgSlug: "original-org", + }); + + const originalUserId = result1.user.id; + + // Overwrite the user + const result2 = await executeBootstrap(trx, { + email: "admin@example.com", + password: "newpassword123", + orgSlug: "new-org", + dangerouslyOverwriteExisting: true, + }); + + // Should be a different user ID + expect(result2.user.id).not.toBe(originalUserId); + + // Original user should be deleted + const originalUser = await trx + .selectFrom("users") + .where("id", "=", originalUserId) + .selectAll() + .executeTakeFirst(); + + expect(originalUser).toBeUndefined(); + + // New user should exist + const newUser = await trx + .selectFrom("users") + .where("id", "=", result2.user.id) + .selectAll() + .executeTakeFirst(); + + expect(newUser).toBeDefined(); + }); + }); + + test("deletes existing org in dangerouslyOverwriteExisting mode", async () => { + await withTestTransaction(db, async (trx) => { + // Create the first bootstrap + const result1 = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + orgSlug: "test-org", + }); + + const originalOrgId = result1.org.id; + + // Overwrite with a different email but same org slug + const result2 = await executeBootstrap(trx, { + email: "newadmin@example.com", + password: "password123", + orgSlug: "test-org", + dangerouslyOverwriteExisting: true, + }); + + // Should be a different org ID + expect(result2.org.id).not.toBe(originalOrgId); + + // Original org should be deleted + const originalOrg = await trx + .selectFrom("orgs") + .where("id", "=", originalOrgId) + .selectAll() + .executeTakeFirst(); + + expect(originalOrg).toBeUndefined(); + }); + }); + + test("deletes related user records in overwrite mode", async () => { + const uniqueId = uniqueTestId(); + await withTestTransaction(db, async (trx) => { + // Create the first bootstrap + const result1 = await executeBootstrap(trx, { + email: `admin-${uniqueId}@example.com`, + password: "password123", + orgSlug: `org-${uniqueId}`, + }); + + // Manually add some related records + await trx + .insertInto("sessions") + .values({ + user_id: result1.user.id, + token_hash: "test-hash", + ip_address: "127.0.0.1", + user_agent: "test", + expires_at: new Date(Date.now() + 86400000), + trusted_mode: false, + }) + .execute(); + + await trx + .insertInto("email_verifications") + .values({ + user_id: result1.user.id, + token: "test-token", + expires_at: new Date(Date.now() + 86400000), + }) + .execute(); + + await trx + .insertInto("login_requests") + .values({ + user_id: result1.user.id, + email: `admin-${uniqueId}@example.com`, + token: "login-token", + device_fingerprint: "fingerprint", + expires_at: new Date(Date.now() + 86400000), + }) + .execute(); + + await trx + .insertInto("passkeys") + .values({ + user_id: result1.user.id, + credential_id: Buffer.from("credential"), + public_key: Buffer.from("publickey"), + counter: 0, + backup_eligible: false, + backup_status: false, + device_type: "singleDevice", + name: "Test Passkey", + rpid: "localhost", + webauthn_user_id: "test-user-id", + }) + .execute(); + + await trx + .insertInto("password_resets") + .values({ + user_id: result1.user.id, + token: "reset-token", + expires_at: new Date(Date.now() + 86400000), + }) + .execute(); + + await trx + .insertInto("user_devices") + .values({ + user_id: result1.user.id, + device_fingerprint: "device-fingerprint", + user_agent: "test-agent", + }) + .execute(); + + // Overwrite the user + await executeBootstrap(trx, { + email: `admin-${uniqueId}@example.com`, + password: "newpassword123", + orgSlug: `org-${uniqueId}`, + dangerouslyOverwriteExisting: true, + }); + + // All related records should be deleted + const sessions = await trx + .selectFrom("sessions") + .where("user_id", "=", result1.user.id) + .selectAll() + .execute(); + expect(sessions).toHaveLength(0); + + const emailVerifications = await trx + .selectFrom("email_verifications") + .where("user_id", "=", result1.user.id) + .selectAll() + .execute(); + expect(emailVerifications).toHaveLength(0); + + const loginRequests = await trx + .selectFrom("login_requests") + .where("user_id", "=", result1.user.id) + .selectAll() + .execute(); + expect(loginRequests).toHaveLength(0); + + const passkeys = await trx + .selectFrom("passkeys") + .where("user_id", "=", result1.user.id) + .selectAll() + .execute(); + expect(passkeys).toHaveLength(0); + + const passwordResets = await trx + .selectFrom("password_resets") + .where("user_id", "=", result1.user.id) + .selectAll() + .execute(); + expect(passwordResets).toHaveLength(0); + + const userDevices = await trx + .selectFrom("user_devices") + .where("user_id", "=", result1.user.id) + .selectAll() + .execute(); + expect(userDevices).toHaveLength(0); + }); + }); + + test("deletes org invites created by user in overwrite mode", async () => { + const uniqueId = uniqueTestId(); + await withTestTransaction(db, async (trx) => { + // Create the first bootstrap + const result1 = await executeBootstrap(trx, { + email: `admin-${uniqueId}@example.com`, + password: "password123", + orgSlug: `org-${uniqueId}`, + }); + + // Create another org and invite + const [otherOrg] = await trx + .insertInto("orgs") + .values({ + slug: `other-org-${uniqueId}`, + display_name: "Other Org", + }) + .returning(["id"]) + .execute(); + + await trx + .insertInto("org_invites") + .values({ + org_id: otherOrg!.id, + email: "invitee@example.com", + role: "member", + invited_by: result1.user.id, + token: "invite-token", + expires_at: new Date(Date.now() + 86400000), + }) + .execute(); + + // Overwrite the user + await executeBootstrap(trx, { + email: `admin-${uniqueId}@example.com`, + password: "newpassword123", + orgSlug: `new-org-${uniqueId}`, + dangerouslyOverwriteExisting: true, + }); + + // Invite created by the user should be deleted + const invites = await trx + .selectFrom("org_invites") + .where("invited_by", "=", result1.user.id) + .selectAll() + .execute(); + expect(invites).toHaveLength(0); + }); + }); + + test("deletes org related records in overwrite mode", async () => { + const uniqueId = uniqueTestId(); + await withTestTransaction(db, async (trx) => { + // Create the first bootstrap + const result1 = await executeBootstrap(trx, { + email: `admin-${uniqueId}@example.com`, + password: "password123", + orgSlug: `org-${uniqueId}`, + }); + + // Add org sites + await trx + .insertInto("org_sites") + .values({ + org_id: result1.org.id, + domain: "example.com", + }) + .execute(); + + // Add org invites (to the org, not by the user) + const [anotherUser] = await trx + .insertInto("users") + .values({ + email: `other-${uniqueId}@example.com`, + display_name: "Other User", + }) + .returning(["id"]) + .execute(); + + await trx + .insertInto("org_invites") + .values({ + org_id: result1.org.id, + email: "invitee@example.com", + role: "member", + invited_by: anotherUser!.id, + token: "invite-token-2", + expires_at: new Date(Date.now() + 86400000), + }) + .execute(); + + // Overwrite with the same org slug + await executeBootstrap(trx, { + email: `newadmin-${uniqueId}@example.com`, + password: "password123", + orgSlug: `org-${uniqueId}`, + dangerouslyOverwriteExisting: true, + }); + + // Org sites should be deleted + const sites = await trx + .selectFrom("org_sites") + .where("org_id", "=", result1.org.id) + .selectAll() + .execute(); + expect(sites).toHaveLength(0); + + // Org invites should be deleted + const invites = await trx + .selectFrom("org_invites") + .where("org_id", "=", result1.org.id) + .selectAll() + .execute(); + expect(invites).toHaveLength(0); + }); + }); + + test("succeeds when no existing user/org in overwrite mode", async () => { + const uniqueId = uniqueTestId(); + await withTestTransaction(db, async (trx) => { + // Should not throw even when nothing exists to overwrite + const result = await executeBootstrap(trx, { + email: `fresh-${uniqueId}@example.com`, + password: "password123", + orgSlug: `fresh-org-${uniqueId}`, + dangerouslyOverwriteExisting: true, + }); + + expect(result.user.email).toBe(`fresh-${uniqueId}@example.com`); + expect(result.org.slug).toBe(`fresh-org-${uniqueId}`); + }); + }); + + test("returns all expected fields", async () => { + await withTestTransaction(db, async (trx) => { + const result = await executeBootstrap(trx, { + email: "admin@example.com", + password: "password123", + }); + + // Check user fields + expect(typeof result.user.id).toBe("number"); + expect(typeof result.user.email).toBe("string"); + + // Check org fields + expect(typeof result.org.id).toBe("number"); + expect(typeof result.org.slug).toBe("string"); + + // Check token + expect(typeof result.token).toBe("string"); + expect(result.token.startsWith(TOKEN_PREFIX)).toBe(true); + }); + }); +}); diff --git a/packages/db/src/helpers/token.test.ts b/packages/db/src/helpers/token.test.ts new file mode 100644 index 0000000..c09df4e --- /dev/null +++ b/packages/db/src/helpers/token.test.ts @@ -0,0 +1,136 @@ +/** + * Tests for token generation and hashing utilities + */ + +import { describe, expect, test } from "bun:test"; +import { generateToken, hashToken, parseToken, TOKEN_PREFIX } from "./token.js"; + +describe("token utilities", () => { + describe("TOKEN_PREFIX", () => { + test("has expected value", () => { + expect(TOKEN_PREFIX).toBe("reviq_"); + }); + }); + + describe("generateToken", () => { + test("generates token with correct prefix", () => { + const token = generateToken(); + expect(token.startsWith(TOKEN_PREFIX)).toBe(true); + }); + + test("generates unique tokens", () => { + const tokens = new Set(); + for (let i = 0; i < 100; i++) { + tokens.add(generateToken()); + } + expect(tokens.size).toBe(100); + }); + + test("generates token of expected length", () => { + const token = generateToken(); + // reviq_ (6 chars) + base58 encoded 32 bytes (~44 chars) + expect(token.length).toBeGreaterThan(40); + expect(token.length).toBeLessThan(60); + }); + }); + + describe("parseToken", () => { + test("parses valid token and returns bytes", () => { + const token = generateToken(); + const bytes = parseToken(token); + + expect(bytes).not.toBeNull(); + expect(bytes).toBeInstanceOf(Uint8Array); + expect(bytes?.length).toBe(32); + }); + + test("returns null for token without prefix", () => { + const result = parseToken("invalid_token_without_prefix"); + expect(result).toBeNull(); + }); + + test("returns null for empty string", () => { + const result = parseToken(""); + expect(result).toBeNull(); + }); + + test("returns null for token with wrong prefix", () => { + const result = parseToken("wrong_prefix_abc123"); + expect(result).toBeNull(); + }); + + test("returns null for token with invalid base58", () => { + // Include invalid base58 characters (0, O, I, l) + const result = parseToken(`${TOKEN_PREFIX}invalid0OIl`); + expect(result).toBeNull(); + }); + + test("returns null for token with wrong byte length", () => { + // Create a valid base58 string but with fewer bytes + // base58 encode a 16-byte value (too short) + const result = parseToken(`${TOKEN_PREFIX}2VQr`); + expect(result).toBeNull(); + }); + + test("returns same bytes for same token", () => { + const token = generateToken(); + const bytes1 = parseToken(token); + const bytes2 = parseToken(token); + + expect(bytes1).toEqual(bytes2); + }); + }); + + describe("hashToken", () => { + test("returns hex string", () => { + const token = generateToken(); + const hash = hashToken(token); + + // SHA-256 produces 32 bytes = 64 hex chars + expect(hash.length).toBe(64); + expect(/^[0-9a-f]+$/.test(hash)).toBe(true); + }); + + test("produces deterministic hash", () => { + const token = generateToken(); + const hash1 = hashToken(token); + const hash2 = hashToken(token); + + expect(hash1).toBe(hash2); + }); + + test("produces different hashes for different tokens", () => { + const token1 = generateToken(); + const token2 = generateToken(); + + expect(hashToken(token1)).not.toBe(hashToken(token2)); + }); + + test("hashes any string input", () => { + const hash = hashToken("arbitrary string input"); + expect(hash.length).toBe(64); + }); + + test("hashes empty string", () => { + const hash = hashToken(""); + // SHA-256 of empty string is a known value + expect(hash).toBe( + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + ); + }); + }); + + describe("round-trip", () => { + test("generated tokens can be parsed and hashed", () => { + for (let i = 0; i < 10; i++) { + const token = generateToken(); + const bytes = parseToken(token); + const hash = hashToken(token); + + expect(bytes).not.toBeNull(); + expect(bytes?.length).toBe(32); + expect(hash.length).toBe(64); + } + }); + }); +}); From 67930d90d57e9cd6015efd0c846f638be5868347 Mon Sep 17 00:00:00 2001 From: igm Date: Mon, 12 Jan 2026 15:42:39 +0800 Subject: [PATCH 2/2] Simplify apps/cli/ code - config.ts: Convert arrow functions to function declarations - api-client.ts: Extract duplicated RPCLink logic into buildClient helper - format-error.ts: Add centralized ORPCError handling - complete-login.ts: Remove redundant error handling (now in formatError) - status.ts: Simplify formatRelativeTime, improve whitespace - create.ts: Rename validRoles to VALID_ROLES, add as const, early return - completions.ts: Derive Shell type from SUPPORTED_SHELLS array Co-Authored-By: Claude Opus 4.5 --- apps/cli/src/routes/admin/complete-login.ts | 8 +----- apps/cli/src/routes/auth/status.ts | 8 +++--- apps/cli/src/routes/completions.ts | 5 ++-- apps/cli/src/routes/user/create.ts | 14 ++++++----- apps/cli/src/utils/api-client.ts | 28 ++++++++------------- apps/cli/src/utils/config.ts | 16 ++++++------ apps/cli/src/utils/format-error.ts | 8 +++++- 7 files changed, 41 insertions(+), 46 deletions(-) diff --git a/apps/cli/src/routes/admin/complete-login.ts b/apps/cli/src/routes/admin/complete-login.ts index 19f3833..8f2c3e1 100644 --- a/apps/cli/src/routes/admin/complete-login.ts +++ b/apps/cli/src/routes/admin/complete-login.ts @@ -1,5 +1,4 @@ import type { LocalContext } from "../../context.js"; -import { ORPCError } from "@orpc/client"; import { buildCommand } from "@stricli/core"; import { createApiClient } from "../../utils/api-client.js"; import { formatError } from "../../utils/format-error.js"; @@ -21,12 +20,7 @@ async function completeLogin( console.log(`Completed login request for: ${flags.email}`); } catch (error) { - if (error instanceof ORPCError) { - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions -- ORPCError.code is typed as any - console.error(`Error [${error.code}]:`, error.message); - } else { - console.error("Error:", formatError(error)); - } + console.error("Error:", formatError(error)); this.process.exit(1); } } diff --git a/apps/cli/src/routes/auth/status.ts b/apps/cli/src/routes/auth/status.ts index 2bd0233..a18211d 100644 --- a/apps/cli/src/routes/auth/status.ts +++ b/apps/cli/src/routes/auth/status.ts @@ -17,16 +17,16 @@ function formatRelativeTime(date: Date): string { if (diffDays < 0) { return `${Math.abs(diffDays).toLocaleString()} days ago`; } + if (diffDays === 0) { const diffHours = Math.floor(diffMs / (1000 * 60 * 60)); - if (diffHours <= 0) { - return "expired"; - } - return `in ${diffHours.toLocaleString()} hours`; + return diffHours <= 0 ? "expired" : `in ${diffHours.toLocaleString()} hours`; } + if (diffDays === 1) { return "tomorrow"; } + return `in ${diffDays.toLocaleString()} days`; } diff --git a/apps/cli/src/routes/completions.ts b/apps/cli/src/routes/completions.ts index 06280f5..3b4d189 100644 --- a/apps/cli/src/routes/completions.ts +++ b/apps/cli/src/routes/completions.ts @@ -1,9 +1,8 @@ import type { LocalContext } from "../context.js"; import { buildCommand } from "@stricli/core"; -type Shell = "bash" | "zsh" | "fish"; - -const SUPPORTED_SHELLS: readonly Shell[] = ["bash", "zsh", "fish"] as const; +const SUPPORTED_SHELLS = ["bash", "zsh", "fish"] as const; +type Shell = (typeof SUPPORTED_SHELLS)[number]; function parseShell(value: string): Shell { const shell = value.toLowerCase(); diff --git a/apps/cli/src/routes/user/create.ts b/apps/cli/src/routes/user/create.ts index 249ea49..e53b400 100644 --- a/apps/cli/src/routes/user/create.ts +++ b/apps/cli/src/routes/user/create.ts @@ -5,18 +5,20 @@ import { formatError } from "../../utils/format-error.js"; type OrgRole = "owner" | "admin" | "member"; -const validRoles: OrgRole[] = ["owner", "admin", "member"]; +const VALID_ROLES: readonly OrgRole[] = ["owner", "admin", "member"] as const; function parseRole(role: string | undefined): OrgRole | undefined { if (!role) { return undefined; } - if (validRoles.includes(role as OrgRole)) { - return role as OrgRole; + + if (!VALID_ROLES.includes(role as OrgRole)) { + throw new Error( + `Invalid role: ${role}. Must be one of: ${VALID_ROLES.join(", ")}`, + ); } - throw new Error( - `Invalid role: ${role}. Must be one of: ${validRoles.join(", ")}`, - ); + + return role as OrgRole; } interface CreateUserFlags { diff --git a/apps/cli/src/utils/api-client.ts b/apps/cli/src/utils/api-client.ts index 00cec6c..41e6a9e 100644 --- a/apps/cli/src/utils/api-client.ts +++ b/apps/cli/src/utils/api-client.ts @@ -10,6 +10,14 @@ import { readConfig } from "./config.js"; export type ApiClient = ContractRouterClient; +function buildClient(apiUrl: string, token: string): ApiClient { + const link = new RPCLink({ + url: `${apiUrl}/api/v1/rpc`, + headers: { "X-API-Key": token }, + }); + return createORPCClient(link) as unknown as ApiClient; +} + /** * Create an oRPC API client with provided credentials */ @@ -25,18 +33,10 @@ export function createApiClient( apiUrl?: string, token?: string, ): ApiClient | Promise { - // If both arguments are provided, create client directly if (apiUrl !== undefined && token !== undefined) { - const link = new RPCLink({ - url: `${apiUrl}/api/v1/rpc`, - headers: { - "X-API-Key": token, - }, - }); - return createORPCClient(link) as unknown as ApiClient; + return buildClient(apiUrl, token); } - // Otherwise, read from config asynchronously return (async (): Promise => { const config = await readConfig(); if (!config) { @@ -44,14 +44,6 @@ export function createApiClient( "Not logged in. Run 'reviq bootstrap' or 'reviq auth login' first.", ); } - - const link = new RPCLink({ - url: `${config.apiUrl}/api/v1/rpc`, - headers: { - "X-API-Key": config.token, - }, - }); - - return createORPCClient(link) as unknown as ApiClient; + return buildClient(config.apiUrl, config.token); })(); } diff --git a/apps/cli/src/utils/config.ts b/apps/cli/src/utils/config.ts index 9b47e42..374e7fa 100644 --- a/apps/cli/src/utils/config.ts +++ b/apps/cli/src/utils/config.ts @@ -19,40 +19,42 @@ const CONFIG_FILE = join(CONFIG_DIR, "credentials.json"); /** * Get the path to the config file */ -export const getConfigPath = (): string => CONFIG_FILE; +export function getConfigPath(): string { + return CONFIG_FILE; +} /** * Read the config file * Returns null if the file doesn't exist or is invalid */ -export const readConfig = async (): Promise => { +export async function readConfig(): Promise { try { const data = await readFile(CONFIG_FILE, "utf-8"); return JSON.parse(data) as Config; } catch { return null; } -}; +} /** * Write the config file * Creates the config directory if it doesn't exist */ -export const writeConfig = async (config: Config): Promise => { +export async function writeConfig(config: Config): Promise { await mkdir(CONFIG_DIR, { recursive: true, mode: 0o700 }); await writeFile(CONFIG_FILE, JSON.stringify(config, null, 2), { mode: 0o600, }); -}; +} /** * Delete the config file * Ignores errors if the file doesn't exist */ -export const deleteConfig = async (): Promise => { +export async function deleteConfig(): Promise { try { await unlink(CONFIG_FILE); } catch { // Ignore if doesn't exist } -}; +} diff --git a/apps/cli/src/utils/format-error.ts b/apps/cli/src/utils/format-error.ts index a033552..2b5ced5 100644 --- a/apps/cli/src/utils/format-error.ts +++ b/apps/cli/src/utils/format-error.ts @@ -1,8 +1,14 @@ +import { ORPCError } from "@orpc/client"; + /** * Format an unknown error value into a string message. - * Handles Error instances, strings, and other types safely. + * Handles ORPCError, Error instances, strings, and other types safely. */ export function formatError(error: unknown): string { + if (error instanceof ORPCError) { + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions -- ORPCError.code is typed as any + return `[${error.code}] ${error.message}`; + } if (error instanceof Error) { return error.message; }