nexus/.planning/codebase/QUALITY.md

# Code Quality — Paperclip (Nexus)

**Analysis Date:** 2026-03-30

---

## Testing Frameworks

**Unit/Integration Test Runner:**
- Vitest 3.x, configured at the monorepo root via `vitest.config.ts`
- Projects registered: `packages/db`, `packages/adapters/opencode-local`, `server`, `ui`, `cli`

**E2E Test Runner:**
- Playwright (`@playwright/test` ^1.58.2)
- E2E config: `tests/e2e/playwright.config.ts`
- Release smoke config: `tests/release-smoke/playwright.config.ts`

**Run Commands:**
```bash
pnpm test:run          # All vitest tests (CI mode)
pnpm test              # All vitest tests (watch mode)
pnpm test:e2e          # Playwright E2E suite
pnpm test:e2e:headed   # Playwright with browser visible
```

---

## Test Counts & Coverage

| Workspace | Test Files | describe/it/test calls |
|-----------|-----------|----------------------|
| `server/src/__tests__/` | 94 files (+ 1 helpers/ dir) | ~649 describe/it/test calls |
| `cli/src/__tests__/` | 17 files | ~120 calls |
| `ui/src/` (lib + adapters + hooks + context) | 18 `.test.ts` files | ~100+ calls |
| `ui/src/` (components) | 2 `.test.tsx` files | minimal |
| `packages/db/src/` | 3 test files | ~20 calls |
| `packages/adapters/opencode-local/` | 3 test files | ~10 calls |
| `packages/adapters/pi-local/` | 2 test files | ~10 calls |
| E2E (`tests/e2e/`) | 1 spec file | 1 test |
| Release smoke (`tests/release-smoke/`) | 1 spec file | 1 test |

**Coverage tooling:** No coverage thresholds or reporters are configured. None of the `vitest.config.ts` files include a `coverage` block. Coverage is not tracked.

---

## What Is Tested

**Well-covered:**
- Server route handlers (via `supertest` HTTP-level integration tests against a real Express app backed by embedded Postgres) — e.g., `server/src/__tests__/routines-e2e.test.ts`, `approval-routes-idempotency.test.ts`
- Server services (pure logic tested in isolation with vi.fn() mocks) — e.g., `issues-service.test.ts`, `approvals-service.test.ts`, `company-portability.test.ts`
- Adapter models, parse logic, skill sync for every adapter type (claude-local, codex-local, cursor-local, gemini-local, opencode-local, pi-local, openclaw-gateway)
- Database runtime config resolution across all source precedence paths — `packages/db/src/runtime-config.test.ts`
- CLI commands: worktree management, company import/export, auth flows, home path resolution
- UI lib utilities: inbox badge computation, assignee logic, routine trigger patches, onboarding routing, company portability
- Security/redaction: `log-redaction.test.ts`, `forbidden-tokens.test.ts`, `redaction.test.ts`
- Error handler middleware — `error-handler.test.ts`
- Zod validation flow: routes use `validate(schema)` middleware, covered by route-level tests

**Under-tested or not tested:**
- UI components (83 components, only 2 have test files: `MarkdownBody.test.tsx`, `RunTranscriptView.test.tsx`)
- UI pages (39 pages, zero test files)
- Real-database integration tests skip on unsupported hosts (`describe.skip` via `getEmbeddedPostgresTestSupport`) — these tests pass silently in most environments
- Storage provider implementations (`server/src/storage/`) — only referenced, not directly tested
- Plugin lifecycle/loader/worker manager have some tests but plugin tooling is lightly covered

---

## Test Patterns

**Standard unit test structure:**
```typescript
import { describe, expect, it, vi } from "vitest";

describe("feature name", () => {
  it("describes the expected behavior", () => {
    expect(result).toEqual(expected);
  });
});
```

**Integration test pattern (HTTP + real DB):**
Tests in `server/src/__tests__/routines-e2e.test.ts` and similar spin up an Express app with a real embedded Postgres instance:
```typescript
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;

describeEmbeddedPostgres("feature", () => {
  beforeAll(async () => {
    tempDb = await startEmbeddedPostgresTestDatabase("prefix-");
    db = createDb(tempDb.connectionString);
  }, 20_000);
  afterAll(async () => { await tempDb?.cleanup(); });
  afterEach(async () => { /* truncate tables */ });
});
```

**Service mock pattern (for route-level tests without DB):**
```typescript
const issueSvc = {
  list: vi.fn(),
  create: vi.fn(),
  // ...
};
vi.mock("../services/index.js", () => ({ issueService: () => issueSvc }));
```

**Test helper location:** `server/src/__tests__/helpers/embedded-postgres.ts` re-exports `@paperclipai/db` test utilities.

**Factory functions:** Tests consistently define local `make*()` helpers (e.g., `makeApproval()`, `makeRun()`, `makeIssue()`) rather than shared factories. No shared fixture library exists.

---

## Linting & Formatting

**ESLint:** Not detected. No `.eslintrc*`, `eslint.config.*`, or biome config exists at any level of the monorepo.

**Prettier:** Not detected. No `.prettierrc*` found.

**Implications:** Code style is enforced only by TypeScript's strict mode and reviewer convention. There is no automated formatting gate in CI. Formatting inconsistencies (e.g., line length, trailing commas) are present but generally consistent within files.

**Observed style (informal conventions):**
- 2-space indentation throughout
- Double quotes for imports (`"..."`)
- Trailing commas in multi-line expressions
- Named exports preferred over default exports in services and routes
- Consistent `node:` prefix on Node built-ins (`import fs from "node:fs"`)

---

## TypeScript Strictness

**Shared base config:** `tsconfig.base.json` at repo root:
- `"strict": true` — enables all strict checks (noImplicitAny, strictNullChecks, etc.)
- `"target": "ES2023"`, `"module": "NodeNext"`, `"moduleResolution": "NodeNext"`
- `"isolatedModules": true`
- `"forceConsistentCasingInFileNames": true`

**UI override:** `ui/tsconfig.json` also sets `"strict": true`, with `"module": "ESNext"`, `"moduleResolution": "bundler"`.

**All packages** extend `tsconfig.base.json` or have equivalent strictness settings.

**`as any` usage:** 131 occurrences across 35 files, mostly in test mocks and Express middleware internals (e.g., casting `res` to `any` to attach custom error context properties). Production service code rarely uses `as any` outside of plugin-related services where dynamic dispatch requires it (`plugin-host-services.ts`: 16 occurrences).

**`@ts-ignore` / `@ts-expect-error`:** Zero occurrences across the entire codebase — a strong signal of type discipline.

**Typecheck CI gate:** `pnpm -r typecheck` runs in both the `verify` job (on PRs) and `verify_canary` job (on merge). Type errors block CI.

---

## Error Handling Patterns

**Custom HTTP error classes:** `server/src/errors.ts` defines `HttpError` with factory helpers:
```typescript
export function badRequest(message, details?) { return new HttpError(400, message, details); }
export function notFound(message?) { return new HttpError(404, message ?? "Not found"); }
export function forbidden(message?) { return new HttpError(403, message ?? "Forbidden"); }
// + unauthorized, conflict, unprocessable
```

**Centralized error handler:** `server/src/middleware/error-handler.ts` handles:
- `HttpError` → structured JSON response with correct status
- `ZodError` → 400 with validation details
- Unknown errors → 500 with `"Internal server error"` (no leak)
- Attaches `res.__errorContext` for logging (consumed by `pino-http`)

**Route error propagation:** Routes use `async` handler functions with `next(err)` pass-through, relying on the global Express error handler. No try/catch noise in route files.

**Validation:** Zod schemas defined in `@paperclipai/shared` (e.g., `createIssueSchema`) are applied via the `validate(schema)` middleware in `server/src/middleware/validate.ts`. Schema parse errors are automatically caught by the error handler.

---

## Logging

**Framework:** Pino + pino-http + pino-pretty

**Configuration:** `server/src/middleware/logger.ts`
- `info` level to stdout (colorized via pino-pretty)
- `debug` level to `server.log` file (plain text)
- HTTP requests logged with custom log levels: 5xx → error, 4xx → warn, 2xx → info
- Error context (body, params, query) attached to error-level log entries
- Log directory resolved from `PAPERCLIP_LOG_DIR` env, config file, or default home path

**Production code:** Minimal `console.*` usage (9 files in server, mostly in plugin services and test `console.warn` for unsupported environments). UI has 6 console calls total, confined to plugin slots.

---

## CI/CD Quality Gates

**On every PR to `master`** (`.github/workflows/pr.yml`):

| Check | Job |
|-------|-----|
| Block manual `pnpm-lock.yaml` edits | `policy` |
| Validate Dockerfile `deps` stage completeness | `policy` |
| Validate dependency resolution on manifest changes | `policy` |
| `pnpm -r typecheck` | `verify` |
| `pnpm test:run` (all Vitest tests) | `verify` |
| `pnpm build` | `verify` |
| Canary release dry run | `verify` |
| Playwright E2E (skip-LLM mode) | `e2e` |

**On merge to `master`** (`.github/workflows/release.yml`):
- Full re-run of typecheck + tests before publishing canary

**Release smoke tests** (`.github/workflows/release-smoke.yml`): Separate workflow for post-release Docker auth/onboarding smoke.

**E2E tests** (`.github/workflows/e2e.yml`): Manual dispatch only, supports full LLM execution via `ANTHROPIC_API_KEY`.

**No coverage gate** in CI. No lint gate in CI.

---

## Code Organization & Consistency

**Service factory pattern:** All server services are instantiated as factory functions receiving a `Db` instance:
```typescript
export function issueService(db: Db) {
  return {
    list: async (params) => { ... },
    create: async (data) => { ... },
  };
}
```
This pattern is consistent across all 64 service files in `server/src/services/`.

**Route factory pattern:** Routes are factory functions receiving `db` and optional `storage`:
```typescript
export function issueRoutes(db: Db, storage: StorageService) {
  const router = Router();
  const svc = issueService(db);
  // ...
  return router;
}
```

**Monorepo workspace structure:** Clear separation between `server/`, `ui/`, `cli/`, `packages/shared/`, `packages/db/`, `packages/adapters/*`. Cross-package imports use `@paperclipai/*` namespace.

**Import organization:** Node built-ins first with `node:` prefix, then external packages, then internal workspace packages (`@paperclipai/*`), then relative imports. No enforced by tooling but consistently applied.

---

## Documentation Quality

**Inline comments:** Sparse but purposeful. Comments appear where non-obvious logic is present (e.g., SSRF protection in `plugin-host-services.ts` has a section comment block). Files generally are self-documenting through naming.

**JSDoc/TSDoc:** Not used. No `/** */` doc-comments on exported functions. Types are the documentation.

**TODO comments:** Only 3 in the entire codebase:
- `ui/src/pages/AgentDetail.tsx:771` — commented-out skills tab view
- `ui/src/adapters/runtime-json-fields.tsx:5` — disabled UI pending worktree workflow
- `cli/src/commands/client/company.ts:362` — temporary `claude_local` fallback in import TUI

**README:** `README.md` exists at root. `CONTRIBUTING.md` has clear contribution guidelines including required PR "thinking path" format.

**PR template:** `.github/PULL_REQUEST_TEMPLATE.md` enforces thinking path, what changed, verification steps, risks, and checklist including test and doc updates.

---

## Technical Debt Indicators

**No linting or formatting tooling:**
- Risk: style drift over time, no automated enforcement
- Files affected: entire codebase
- Fix approach: add Biome (preferred for TS monorepos) or ESLint + Prettier with CI gate

**No coverage measurement:**
- Risk: regressions in untested paths go undetected
- Files affected: primarily `ui/src/pages/`, `ui/src/components/`, `server/src/storage/`
- Fix approach: add `@vitest/coverage-v8`, set minimum thresholds per workspace

**UI components have near-zero unit tests:**
- 83 component files, 2 test files
- Risk: UI logic regressions caught only by E2E or manually
- Most UI lib logic (`ui/src/lib/`) is tested; the gap is components and pages
- Fix approach: add React Testing Library for component tests

**`as any` in production plugin services (`plugin-host-services.ts`):**
- 16 occurrences in a single file
- Indicates dynamic dispatch complexity in plugin host layer
- Risk: type errors in plugin boundary surface at runtime

**Embedded Postgres tests skip silently on many hosts:**
- Pattern: `const describeEmbeddedPostgres = supported ? describe : describe.skip`
- This means DB integration tests do not run in many dev environments and potentially some CI hosts
- CI does run them (`ubuntu-latest` is a supported host)

**`server/src/services/company-portability.ts` uses `as any` 12 times:**
- Most complex service file; high import count (30+ types from shared)
- Deserves a refactor pass once the portability feature stabilizes

---

*Quality audit: 2026-03-30*