From 4d5b8ae531ce7b2e4fd87fc66fce621dc8ad75e2 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Tue, 11 Aug 2026 15:38:02 +0530 Subject: [PATCH 1/4] Add database schema design skill and rules documentation --- .agents/skills/designing-db-schemas/SKILL.md | 227 ++++++++ .../api-platform-db-schema-rules.md | 491 ++++++++++++++++++ .../scripts/generate-schema-report.js | 137 +++++ .claude/rules/db-schema-changes.md | 33 ++ 4 files changed, 888 insertions(+) create mode 100644 .agents/skills/designing-db-schemas/SKILL.md create mode 100644 .agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md create mode 100644 .agents/skills/designing-db-schemas/scripts/generate-schema-report.js create mode 100644 .claude/rules/db-schema-changes.md diff --git a/.agents/skills/designing-db-schemas/SKILL.md b/.agents/skills/designing-db-schemas/SKILL.md new file mode 100644 index 000000000..3a005d988 --- /dev/null +++ b/.agents/skills/designing-db-schemas/SKILL.md @@ -0,0 +1,227 @@ +--- +name: designing-db-schemas +description: Design, change, or review a database schema in the WSO2 API Platform. Use when adding or altering a table, column, index, or constraint in any *.sql schema file, reviewing schema changes before a PR, evaluating a migration plan, or asking whether a table is well designed and what indexes it needs. +allowed-tools: Bash, Read, Edit, Write, Glob, Grep +--- + +# WSO2 API Platform — Database Schema Design + +This skill governs **all schema work** for the WSO2 API Platform: designing new tables, adding to existing ones, and reviewing DDL changes. It is not a post-hoc review tool — it is the process to follow when writing DDL. + +The rules live in **`references/api-platform-db-schema-rules.md`** (next to this skill). That file is the source of truth for every rule; this file is the workflow. + +## Usage + +``` +/designing-db-schemas [table-name | path-to-schema-file] +``` + +- **No argument** — review all in-scope schema files. +- **Table name** — apply the relevant workflow for that table. +- **Schema file path** — review that file only. + +--- + +## Schema file scope + +Discover schema files with: + +```bash +find . -name "*.sql" -not -path "*/node_modules/*" -not -path "*/target/*" | sort +``` + +Do **not** use `find . -name "schema*.sql"` — it matches 10 of the 19 schema files and misses every gateway-controller and event-gateway file. + +| GA product | Path | Rules | +|---|---|---| +| Platform API | `platform-api/internal/database/` | R0–R10 | +| Platform API — eventgateway plugin | `platform-api/plugins/eventgateway/schema/` | R0–R10 | +| API Portal | `portals/api-portal/database/` | R0–R10 | +| AI Workspace | packages Platform API's schemas — no source of its own | — | +| Gateway Controller | `gateway/gateway-controller/pkg/storage/` **and** `gateway/gateway-controller/resources/` | R0–R2, R4–R10 (**R3 skipped**) | +| Event Gateway Controller | `event-gateway/gateway-controller/pkg/dbschema/` | R0–R2, R4–R10 (**R3 skipped**) | +| Fixtures | `platform-api/internal/database/init-platform-api-db.sql`, `tests/integration-e2e/init-db.sql` | keep in sync | + +**Type exemption** — gateway-controller and event-gateway schemas are owned by separate teams who manage their own type choices. Apply all structural, constraint, audit, index, alignment, and idempotency rules (R0–R2, R4–R10) as normal, but do **not** raise R3 findings (column types, JSONB, BOOLEAN, TIMESTAMPTZ, VARCHAR widths) against those files. + +**Two copies** — `gateway/gateway-controller` keeps the same schema under both `pkg/storage/` and `resources/`. Grep repo-wide before editing: `grep -rln "" --include="*.sql" .` + +--- + +## Workflows + +### Workflow A — Making a schema change + +#### Step A0 — Admissibility gate (R0) + +These are GA products. Decide whether the change is allowed **before** touching a file. + +| Request | Verdict | +|---|---| +| New table | Allowed — full R1–R10 apply | +| New nullable/defaulted column on a shipped table | Allowed | +| New index on a shipped table | Allowed | +| Change a shipped column's type or width | **Blocked** — migration | +| Rename a shipped column or table | **Blocked** — migration | +| Add/drop a PK, or change an FK target or `ON DELETE` | **Blocked** — migration | +| Add `NOT NULL` or `UNIQUE` to an existing column | **Blocked** — revalidates customer data | +| Drop a column | **Blocked** as one step — two-release sequence, needs approval | +| "Fix" a shipped table that violates R1–R10 | **Blocked** — record as `LEGACY-ACCEPTED` (R0-LEGACY-ACCEPTED) | + +Confirm the table has actually shipped before applying the freeze — one added earlier in the same unreleased branch is still malleable: + +```bash +git log --oneline -1 -S"CREATE TABLE IF NOT EXISTS " -- '*.sql' +``` + +When blocked, don't silently narrow the task. Say which part is blocked and why, deliver the allowed remainder, and offer the migration-plan route as separate, approved work. + +#### Step A1 — Read the schemas first + +Locate every schema file with the glob above and read each in full before drafting anything. Apply R3 type rules to all files except gateway-controller and event-gateway. + +#### Step A2 — Open the rules reference + +Read `references/api-platform-db-schema-rules.md`. The rules you need depend on the change: + +| Change type | Rules to apply | +|---|---| +| New table | R0 (admissibility), R1 (identity), R2 (org-scoping), R3 (types), R4 (constraints), R5 (audit), R6 (indexes), R8 (all dialects), R9 (idempotent DDL), R10 (naming) | +| New column | R0, R3 (type), R4 (constraints), R5 (audit), R6 (index if filterable), R7 (Go layer sync), R8, R9, R10 | +| New index | R0, R6 (correct pattern — FK, status, compound, partial), R8, R9, R10 | +| Type change on a shipped column | **R0 — blocked.** Stop; this is a migration, not a schema edit | + +#### Step A3 — Self-review checklist + +``` +[ ] R0 Change is additive — no retype/rename/PK-FK change/NOT NULL/UNIQUE on a shipped table +[ ] R0 Per-dialect ALTER TABLE written for already-provisioned databases +[ ] R1 Entity tables: uuid VARCHAR(40) PRIMARY KEY +[ ] R1 Junction/mapping tables: composite PRIMARY KEY — not UNIQUE-only, not surrogate UUID +[ ] R1 Non-leading FK columns of a composite PK have their own indexes +[ ] R1 Named resource tables carry handle + name + version (all NOT NULL) +[ ] R1 handle VARCHAR(40) slug ≠ name VARCHAR(255) display string; no UNIQUE on name +[ ] R2 organization_uuid FK present; UNIQUE constraints include it (if org-scoped) +[ ] R3 No bare TEXT in Postgres — SQLite TEXT / SQL Server NVARCHAR(MAX) are intentional (R8) +[ ] R3 Large/variable payloads use BYTEA/BLOB/VARBINARY(MAX) — not wide VARCHAR +[ ] R3 JSONB only when queried with JSON operators AND the scan target implements sql.Scanner +[ ] R3 Boolean flags: SMALLINT (Postgres/SQL Server) or INTEGER (SQLite), 0/1 — no BOOLEAN +[ ] R3 VARCHAR widths match R3-VARCHAR-SIZES; nothing above VARCHAR(1023) for plain storage +[ ] R3 Indexed/UNIQUE columns ≤ VARCHAR(255); hashes are VARCHAR(255) +[ ] R3 Timestamps: TIMESTAMPTZ (Postgres) / DATETIME (SQLite) / DATETIME2(7) (SQL Server), UTC +[ ] R4 No CHECK constraints for enum/status values — validated in the service layer instead +[ ] R4 Required columns NOT NULL, with a DEFAULT where one is sensible +[ ] R4 No plaintext credential/token column; hashed or vault-referenced, named accordingly +[ ] R4 Every FK has an explicit ON DELETE clause +[ ] R5 User-initiated table → all four audit columns; system-managed → created_by/updated_by ABSENT +[ ] R5 Every domain entity table has data_version VARCHAR(20) NOT NULL DEFAULT '1.0' +[ ] R6 FK columns, organization_uuid, and filtered status columns have indexes +[ ] R7 Go model/repository/DTO updated in the same commit; named columns, no SELECT * +[ ] R8 Change applied to every dialect file (or divergence is intentional and documented) +[ ] R9 All DDL is idempotent (IF NOT EXISTS / OBJECT_ID / sys.indexes guards) +[ ] R10 All identifiers lowercase snake_case; pure mapping tables have a _mappings suffix +``` + +#### Step A4 — Write the DDL + +Use the engine-specific guards (R9) — `CREATE TABLE IF NOT EXISTS` is not valid T-SQL: + +```sql +-- PostgreSQL / SQLite +CREATE TABLE IF NOT EXISTS
(...); +CREATE INDEX IF NOT EXISTS idx_... ON
(...); + +-- SQL Server +IF OBJECT_ID(N'dbo.
', N'U') IS NULL +CREATE TABLE dbo.
(...); + +IF NOT EXISTS (SELECT 1 FROM sys.indexes WHERE name = N'idx_...' AND object_id = OBJECT_ID(N'dbo.
')) +CREATE INDEX idx_... ON dbo.
(...); +``` + +Keep `CREATE INDEX` statements in a dedicated block after all `CREATE TABLE` statements. Start from the quick-reference templates at the end of the rules file. + +#### Step A5 — Apply to all schema files, then ship the upgrade path + +Apply to every in-scope dialect file, same column order, same position. Only the R8 divergences may differ. Then write the per-dialect `ALTER TABLE` (R0-UPGRADE-PATH) — `CREATE TABLE IF NOT EXISTS` does nothing to a database that already exists. + +#### Step A6 — Verify on more than one dialect + +```bash +cd && go build ./... && go test ./internal/repository/... ./internal/database/... +``` + +Then: start from an empty **SQLite** database; start from an empty **server dialect** (Postgres or SQL Server); and apply the `ALTER` to a **pre-existing** database from the previous release. Report which dialects were actually exercised and which were only edited. + +--- + +### Workflow B — Reviewing existing DDL (PR / audit) + +#### Step B1 — Locate and read all schema files + +Use the glob above. Note which files are gateway-controller or event-gateway — those skip R3. + +#### Step B2 — Open the rules reference + +Read `references/api-platform-db-schema-rules.md`. Evaluate every rule group (R0–R10) in order. + +#### Step B3 — Record findings + +| Field | Value | +|---|---| +| **Rule** | e.g. `R3-NO-TEXT` | +| **Table · column** | exact location | +| **Severity** | `HIGH` (data safety / correctness) · `MEDIUM` (missing guarantee or index) · `LOW` (style) · `LEGACY-ACCEPTED` (shipped table, frozen by R0) | +| **Finding** | what is wrong | +| **Fix** | the exact DDL needed — **omit for `LEGACY-ACCEPTED`** | + +Two adjustments this repo requires: + +- **A shipped table's violation is `LEGACY-ACCEPTED`, not `HIGH`.** Record it in the deviations table of `.claude/rules/db-schema-changes.md` so later reviews stop re-reporting it. Do not propose remediation DDL. +- **Blanket-missing findings collapse.** When a rule is violated uniformly across many tables, record one representative finding naming the pattern with a couple of examples — not one per table. + +Findings on new tables/columns in the diff are live and use the normal severities. + +#### Step B4 — Cross-check multi-engine alignment + +Verify all dialect files are structurally in sync (R8). Intentional type-level divergences are not findings. To compare one table across dialects: + +```bash +for f in platform-api/internal/database/schema*.sql; do + echo "--- $f" + awk '/CREATE TABLE.*
/,/\);/' "$f" | grep -oE '^\s+[a-z_]+' | tr -d ' ' +done +``` + +#### Step B5 — Write findings to JSON + +Run from this skill's directory, with an absolute `--out` so the report lands in the project: + +```bash +node scripts/generate-schema-report.js \ + --findings '' \ + --schema '' \ + --out "$(git rev-parse --show-toplevel)/schema-reports/schema-review.json" +``` + +Output shape: + +```json +{ + "meta": { "schema": "", "reviewedAt": "", "rules": ["R0","R1","R2","R3","R4","R5","R6","R7","R8","R9","R10"] }, + "summary": { "HIGH": 0, "MEDIUM": 0, "LOW": 0, "LEGACY-ACCEPTED": 0 }, + "findings": [ + { "id": "r3-001", "severity": "HIGH", "rule": "R3-NO-TEXT", "table": "
", "column": "", "finding": "...", "fix": "..." } + ] +} +``` + +#### Step B6 — Report summary + +Produce a findings table sorted by severity. Include a "No issues" row for any rule group that passed cleanly — reviewers need to know what was checked. + +--- + +## Quick-reference templates + +New entity table (Postgres and SQL Server), the standard column-type/width cheat sheet, and the junction/mapping table pattern (under **R1-COMPOSITE-PK**) all live in **`references/api-platform-db-schema-rules.md`**. Use them as the starting point in Step A4. diff --git a/.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md b/.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md new file mode 100644 index 000000000..8aaede930 --- /dev/null +++ b/.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md @@ -0,0 +1,491 @@ +# API Platform DB Schema Rules (R0–R10) + +Reference for the `designing-db-schemas` skill. These are the WSO2 API Platform conventions for relational database schemas. Step A2 and Step B2 of the skill read this file and evaluate each rule against the schema. + +**Scope:** applies to every SQL schema file in the repository — discovered with the glob in R0-SCOPE, not `schema*.sql`, which misses 9 of the 19 files. For `gateway/gateway-controller/` and `event-gateway/gateway-controller/` schemas, **skip R3** (type rules) — those teams own their type choices. All other rules (R0–R2, R4–R10) apply to every schema file. + +> **Recording blanket-missing findings.** When a rule is violated uniformly across many tables (e.g. a convention that no tables follow), record **one representative finding** that names the pattern and gives a couple of examples, rather than one finding per table. + +--- + +## R0 · Change Admissibility + +**These are GA products with no migration framework.** The following ship to customers, and their tables are already in customer databases: + +| GA product | Schema ownership | +|---|---| +| **Gateway Controller** (`gateway/gateway-controller`) | own schemas | +| **Event Gateway Controller** (`event-gateway/gateway-controller`) | own schemas | +| **Platform API** (`platform-api`) | own schemas, plus the `eventgateway` plugin schemas | +| **API Portal** (`portals/api-portal`) | own schemas | +| **AI Workspace** (`portals/ai-workspace`) | no schema of its own — **packages Platform API's** at `resources/platform-api/db-scripts/`, so a Platform API schema change ships to AI Workspace customers too | + +Each bootstraps from hand-maintained per-dialect files of `CREATE TABLE IF NOT EXISTS`, selected by driver name at `platform-api/internal/database/connection.go:192`. R1–R10 describe how to build a **new** table or column correctly. They are not a licence to rewrite a shipped one. + +**R0-FROZEN** — On a table that has shipped, the only permitted changes are additive and backward-compatible: a new nullable-or-defaulted column, a new index, a new table. Never, on a shipped table: change a column's type or width, rename a column or table, add or drop a primary key, change a foreign key's target or `ON DELETE` action, add `NOT NULL` to an existing nullable column, or add a `UNIQUE` constraint. Each rewrites or revalidates customer data on upgrade. + +**R0-LEGACY-ACCEPTED** — A shipped table that violates R1–R10 is **accepted legacy, not a finding to fix**. Report it at severity `LEGACY-ACCEPTED` and record it in the deviations table of `.claude/rules/db-schema-changes.md`; do not propose remediation DDL. Bringing it into conformance needs a versioned migration plan approved outside this skill — never a drive-by schema edit. Column removal is a two-release sequence under the same approval: stop reading the column and ship, drop it later. + +Before applying the freeze, confirm the table has actually shipped — one added earlier in the same unreleased branch is still malleable: + +```bash +git log --oneline -1 -S"CREATE TABLE IF NOT EXISTS
" -- '*.sql' +``` + +**R0-SCOPE** — Discover schema files with: + +```bash +find . -name "*.sql" -not -path "*/node_modules/*" -not -path "*/target/*" | sort +``` + +| Component | Files | Rules | +|---|---|---| +| Platform API | `platform-api/internal/database/schema{,.postgres,.sqlite,.sqlserver}.sql` | R0–R10 | +| Event-gateway plugin | `platform-api/plugins/eventgateway/schema/schema.{postgres,sqlite,sqlserver}.sql` | R0–R10 | +| API Portal | `portals/api-portal/database/schema.{postgres,sqlite,sqlserver}.sql` | R0–R10 | +| Gateway Controller | `gateway/gateway-controller/pkg/storage/gateway-controller-db{,.postgres,.sqlserver}.sql` **and** `gateway/gateway-controller/resources/gateway-controller-db.sql` | R0–R2, R4–R10 (**R3 exempt**) | +| Event Gateway Controller | `event-gateway/gateway-controller/pkg/dbschema/eventgateway-db{,.postgres,.sqlserver}.sql` | R0–R2, R4–R10 (**R3 exempt**) | +| Fixtures | `platform-api/internal/database/init-platform-api-db.sql`, `tests/integration-e2e/init-db.sql` | keep in sync | + +`gateway/gateway-controller` keeps **two copies** of the same schema. Grep repo-wide before editing so no copy is missed: + +```bash +grep -rln "" --include="*.sql" . +``` + +**R0-UPGRADE-PATH** — `CREATE TABLE IF NOT EXISTS` is a no-op against a database that already exists, so a column added to a `CREATE TABLE` body reaches fresh installs only. Every additive change ships a matching per-dialect `ALTER TABLE`, nullable or defaulted so it applies while the previous release is still running: + +```sql +ALTER TABLE ADD COLUMN NULL; -- postgres / sqlite +ALTER TABLE ADD NULL; -- sqlserver +``` + +If a component has no upgrade-script location, create one beside its schema files and say so in the PR description. Never skip it. + +--- + +## R1 · Primary Key & Identity + +Every table must have a single UUID primary key and, where it is a named resource, the standard identity triple. + +**R1-UUID** — Primary key must be `uuid VARCHAR(40) PRIMARY KEY`. Do not use `SERIAL`, `BIGINT`, or `INTEGER` as a primary key for domain entities. Junction/mapping tables must use a composite PK (see R1-COMPOSITE-PK). + +**R1-COMPOSITE-PK** — Pure junction/mapping tables (those whose only purpose is to link two or more entities) must use a composite `PRIMARY KEY` over their FK columns — not a surrogate UUID, and not a bare `UNIQUE` constraint. + +Use a composite PK when **all** of the following are true: +- Every query hits the table via the composite key (no query looks up a row by a single generated ID) +- No other table holds a FK reference to a row in this table by a surrogate ID +- The table has no independent lifecycle (rows are inserted or deleted, never updated in place by identity) + +Do **not** use a composite PK (use a UUID PK instead) when: +- Another table references individual rows by ID (e.g. an audit log or event stream that stores a FK to this table's row) +- The table is exposed as a standalone resource in an API with its own URL (e.g. `/associations/{id}`) + +**Why composite PK over UNIQUE-only** — A bare `UNIQUE` constraint without a `PRIMARY KEY` breaks Postgres logical replication for `UPDATE` and `DELETE` operations. Postgres `REPLICA IDENTITY DEFAULT` uses the PK to identify rows in the WAL stream; without a PK it falls back to `REPLICA IDENTITY FULL` (logs the entire old row on every write — high WAL volume) or replication fails entirely. CDC tools (Debezium, AWS DMS) have the same requirement. Distributed SQL engines (CockroachDB, YugabyteDB) silently add a hidden PK if you omit one, with unpredictable sharding consequences. + +**Column order** — Put the most common filter/scope column first (typically `organization_uuid`), then the remaining FKs. The leading column is covered by the PK index; add separate indexes only for the non-leading FK columns. + +```sql +-- Correct composite PK pattern +CREATE TABLE IF NOT EXISTS ( + organization_uuid VARCHAR(40) NOT NULL, + entity_a_uuid VARCHAR(40) NOT NULL, + entity_b_uuid VARCHAR(40) NOT NULL, + created_at TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (organization_uuid, entity_a_uuid, entity_b_uuid), + FOREIGN KEY (organization_uuid) REFERENCES organizations(uuid) ON DELETE CASCADE, + FOREIGN KEY (entity_a_uuid) REFERENCES entity_a(uuid) ON DELETE CASCADE, + FOREIGN KEY (entity_b_uuid) REFERENCES entity_b(uuid) ON DELETE CASCADE +); + +-- Indexes for non-leading FK columns only (leading column covered by PK) +CREATE INDEX IF NOT EXISTS idx__entity_a_uuid ON (entity_a_uuid); +CREATE INDEX IF NOT EXISTS idx__entity_b_uuid ON (entity_b_uuid); +``` + +**R1-IDENTITY** — Tables representing named resources (APIs, gateways, providers, applications, subscriptions, or any domain entity with a stable slug and a display name) must carry the full identity triple directly: + +```sql +handle VARCHAR(40) NOT NULL, -- url-safe slug, immutable once set +name VARCHAR(255) NOT NULL, -- human-readable display name +version VARCHAR(30) NOT NULL DEFAULT 'v1.0', -- semver or opaque version string + +-- handle is unique per organisation, never globally: the same handle may +-- exist in different organisations. Enforce org-scoped, not UNIQUE(handle). +UNIQUE(organization_uuid, handle), +``` + +Identity must be denormalised onto the table itself so queries against a single table are self-contained. Do not rely on a parent record for identity fields. + +**R1-HANDLE-NAME** — `handle` and `name` are distinct: +- `handle` is the URL-safe slug used in API paths (e.g. `/resources/{handle}`). It must be unique within its scope (always org-scoped) and treated as immutable after creation. +- `name` is the human-readable display string. It may change and **is not required to be unique** — not even within a project. + +Conflating them (using `name` as the slug, or allowing `handle` to contain spaces) is a finding. Adding a `UNIQUE` constraint on `name` (alone or combined with `project_uuid`) for API-type entities is also a finding — enforce only `UNIQUE(organization_uuid, handle)`. + +--- + +## R2 · Organisation Scoping + +**R2-ORG-FK** — Every domain table that belongs to an organisation must carry `organization_uuid VARCHAR(40) NOT NULL` with: + +```sql +FOREIGN KEY (organization_uuid) REFERENCES organizations(uuid) ON DELETE CASCADE +``` + +This is not decoration: `GO-AUTH-005` requires every tenant-scoped query to filter on it using a value taken from verified JWT claims, never from request input. A table without the column makes that impossible to enforce. + +**R2-ORG-UNIQUE** — The `handle` of a named resource must be unique within the organisation: + +```sql +UNIQUE(organization_uuid, handle) -- not UNIQUE(handle) alone +``` + +A `UNIQUE(handle)` without the org scope is a critical data-isolation bug. `name` does **not** need a uniqueness constraint — two resources may share a display name within the same org or project. + +--- + +## R3 · Column Types + +**R3-NO-TEXT** — This rule is **engine-scoped to PostgreSQL**. In a Postgres schema, do not use the bare `TEXT` type for any column — use a bounded `VARCHAR(N)`, a binary type (`BYTEA`), or `JSONB` when content is queried. A bare `TEXT` column in a Postgres schema is a finding at MEDIUM severity. + +Do **not** raise R3-NO-TEXT against SQLite or SQL Server schema files: per R8, `TEXT` is the intended type in SQLite (for JSON, large text, and opaque payloads) and `NVARCHAR(MAX)` / `TEXT` is the intended type in SQL Server. These are intentional type-level divergences, not findings. + +**R3-LARGE-PAYLOAD** — Any payload that can grow large or is variable-length must use the engine-appropriate large type, never a wide VARCHAR. Apply: binary payloads → `BYTEA` (Postgres) / `BLOB` (SQLite) / `VARBINARY(MAX)` (SQL Server); large text payloads → `BYTEA` (Postgres) / `TEXT` (SQLite) / `NVARCHAR(MAX)` (SQL Server), per the R8 divergence table. Columns that always need this: `openapi_spec`, `model_list`, `content`, `configuration`, `properties`, `manifest`, `policy_definition`, `metadata`, `api_key_hashes`, or any future column whose value can exceed a few hundred bytes. (The SQLite/SQL Server forms above are the intended divergences — do not flag them as R3-NO-TEXT.) + +**R3-JSONB** — In PostgreSQL, use `JSONB` only when the application queries inside the JSON using Postgres JSON operators. Evidence that a column is actively queried inside: +1. The `DEFAULT` is a JSON literal like `'{}'` +2. The column name implies structure: `settings`, `event_data` +3. The same column in a sibling table already uses `JSONB` + +SQLite and SQL Server equivalents (`TEXT` / `NVARCHAR(MAX)`) are intentional type-level divergences — not findings. + +**R3-JSONB-SCAN-COMPAT** — **PostgreSQL only** (`JSONB` does not exist in SQLite or SQL Server, so this rule never applies to those files). Do not use `JSONB` if the application layer scans it into a plain `string` variable and calls `json.Unmarshal` manually. Postgres drivers return JSONB as binary, which breaks `string` scan targets at runtime. Only use `JSONB` when the scan target implements `sql.Scanner` (e.g. `pgtype.JSONB`, `json.RawMessage`, or a custom struct). + +**R3-BOOLEAN-AS-INT** — Do not use the `BOOLEAN` type. Represent boolean flags as `0`/`1` in: +- `SMALLINT` — PostgreSQL and SQL Server +- `INTEGER` — SQLite + +e.g. `is_default SMALLINT NOT NULL DEFAULT 0` (Postgres/SQL Server), `is_default INTEGER NOT NULL DEFAULT 0` (SQLite). This matches the R8 divergence table exactly. + +**R3-TIMESTAMPTZ** — Use `TIMESTAMPTZ` for **all** timestamp columns in PostgreSQL. `TIMESTAMP` (without timezone) is a bare clock reading with no timezone attached — a finding at MEDIUM. Use `DATETIME` in SQLite and `DATETIME2(7) DEFAULT SYSUTCDATETIME()` in SQL Server. Timestamps are always written as UTC. + +**R3-VARCHAR-SIZES** — Standard widths. These are authoritative; they match the shipped schemas and the quick-reference cheat sheet at the end of this file. + +| Purpose | Width | +|---|---| +| UUID / foreign key to a UUID | `VARCHAR(40)` | +| `handle` (url-safe slug) | `VARCHAR(40)` | +| `name` / display string | `VARCHAR(255)` | +| User identity (email / sub) — `created_by`, `updated_by`, `revoked_by` | `VARCHAR(200)` | +| `version` (resource version) | `VARCHAR(30)`, default `'v1.0'` | +| `data_version` (audit) | `VARCHAR(20)`, default `'1.0'` | +| Lifecycle / status enum | `VARCHAR(20)` | +| Hash (SHA-256 hex) | `VARCHAR(255)` | +| Token (encrypted value) | `VARCHAR(512)` | +| Description / reason | `VARCHAR(1023)` | + +`handle` is **`VARCHAR(40)`**, not 255 — it is a slug, not a display string, and every shipped `handle` column is 40. `version` and `data_version` are different columns with different widths and defaults; do not conflate them. Any width above `VARCHAR(1023)` is a strong signal the column should be `BYTEA`/`BLOB` instead. + +**R3-VARCHAR-ENGINE-LIMITS** — Key engine limits for indexed/unique columns: + +| Usage | Safe max width | +|---|---| +| Plain storage, no index | `VARCHAR(1023)` | +| Appears in a UNIQUE constraint or any index | `VARCHAR(255)` (safe across all engines with utf8mb4) | +| Oracle target (any index or non-extended) | `VARCHAR(255)` | +| MySQL target (utf8mb4, default prefix limit) | `VARCHAR(191)` | + +When a column must be unique but its value can be large, store the value in `BYTEA`/`BLOB` and put a SHA-256 hash in a separate `VARCHAR(255)` column — index and unique-constrain the hash, not the value. `VARCHAR(255)` keeps the hash inside the index-safe ceiling above. + +--- + +## R4 · Constraints + +**R4-NO-ENUM-CHECK** — **Do NOT add `CHECK (col IN (...))` constraints for enum or status columns.** Enum validation belongs in application code (Go constants in `internal/constants`, service-layer validation). DB-layer enum checks require a DDL migration just to add a new valid value. + +The consequence is that the database will **not** reject a bad enum value, so the application layer must actually pick up the burden: values written to an enum/status column come from a Go constant set and are validated in the service layer before the write. Never let a handler pass a free-form string through to an `INSERT`. + +The only `CHECK` constraints that belong in the schema are structural/cross-column invariants: +```sql +-- Cross-column consistency: both NULL or both non-NULL +CONSTRAINT chk_throttle_pair CHECK ( + (throttle_limit_count IS NULL AND throttle_limit_unit IS NULL) OR + (throttle_limit_count IS NOT NULL AND throttle_limit_unit IS NOT NULL) +) +-- Temporal consistency +CHECK (revoked_at IS NULL OR status = 'revoked') +``` + +**R4-NOT-NULL** — Columns that are always required must be `NOT NULL`, paired with a `DEFAULT` where a sensible one exists. Common offenders: `organization_uuid`, `name`, `handle`, `version`, `status`. A nullable column with no default asserts that absence of a value is meaningful — make that assertion deliberately. + +**R4-NO-PLAINTEXT-SECRET** — Never store a credential, API key, or token as its raw value. Store a hash, or a reference to the vault under `internal/vault`, and name the column for what it holds (`token_hash`, not `token`). Never truncate a value to fit a column — if a value can exceed the column, the column is wrong. + +**R4-FK-BEHAVIOR** — Every foreign key must declare an explicit `ON DELETE` action: + +| Relationship | Rule | +|---|---| +| Child owned by parent (cascade delete is safe) | `ON DELETE CASCADE` | +| Reference that must not dangle but parent cannot be deleted | `ON DELETE RESTRICT` | +| Optional reference — row survives if target is deleted | `ON DELETE SET NULL` | + +Omitting `ON DELETE` means the DB default (`RESTRICT` in most engines) applies silently — flag it. + +**SQL Server cascade-path caveat** — SQL Server rejects multiple cascade paths converging on the same parent table. Where Postgres takes `ON DELETE CASCADE` on two FKs that both reach `organizations`, the SQL Server file must use `ON DELETE NO ACTION` on the secondary path and handle that cleanup in application code. Comment it inline as an intentional divergence; it is not an R8 finding. + +--- + +## R5 · Audit Columns + +**R5-AUDIT-SET** — Only tables written as a **direct consequence of a user-initiated action** (e.g. a REST API call) carry `created_by` / `updated_by`. Tables written by background sync, callbacks, or internal system processes must **not** include these columns. + +Rule of thumb: ask "does a human-initiated request cause this row to be inserted or updated?" If yes → include the full audit set. If no → omit `created_by` and `updated_by`. + +```sql +data_version VARCHAR(20) NOT NULL DEFAULT '1.0', +created_by VARCHAR(200), +created_at TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP, -- DATETIME on SQLite / DATETIME2(7) on SQL Server +updated_by VARCHAR(200), +updated_at TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP, +``` + +Where present, `created_by`/`updated_by` must actually be populated from verified JWT claims, never from request input (`GO-AUTH-005`). + +**R5-IMMUTABLE-CREATED** — `created_by` and `created_at` must never be updated after insert. + +**R5-REVOKE-PATTERN** — Tables with soft-revocation add: + +```sql +revoked_by VARCHAR(200), +revoked_at TIMESTAMPTZ, +CHECK (revoked_at IS NULL OR status = 'revoked') +``` + +This `CHECK` is a permitted temporal invariant under R4-NO-ENUM-CHECK — it constrains the relationship between two columns, not the set of valid `status` values. + +**R5-DATA-VERSION** — Every domain entity table must include `data_version VARCHAR(20) NOT NULL DEFAULT '1.0'`. Place it immediately before `created_by`. Excluded tables: junction/mapping tables and pure system/event tables (e.g. `events`, `gateway_states`, `audit`, `deployment_status`, `gateway_custom_policy_usages`, `application_api_keys`, `application_artifacts`, `gateway_association_mappings`). + +--- + +## R6 · Indexing + +Index every column (or compound) that appears in a `WHERE`, `JOIN`, or `ORDER BY` clause at production query volume. + +**R6-FK-INDEX** — Every foreign key column must have an index unless it is already the leftmost column of the PK or a covering UNIQUE constraint. + +```sql +CREATE INDEX IF NOT EXISTS idx_
_ ON
(); +``` + +**R6-ORG-INDEX** — Every org-scoped table must have an index on `organization_uuid`: + +```sql +CREATE INDEX IF NOT EXISTS idx_
_org ON
(organization_uuid); +``` + +**R6-STATUS-INDEX** — Tables with a `status` column that is filtered in list queries need a status index: + +```sql +CREATE INDEX IF NOT EXISTS idx_
_status ON
(status); +``` + +**R6-COMPOUND-INDEX** — When the common query is `WHERE a = ? AND b = ?`, a single compound index `(a, b)` outperforms two single-column indexes. Most-selective filter first. + +**R6-PARTIAL-INDEX** — Use a partial index when the filter discards most rows: + +```sql +CREATE INDEX IF NOT EXISTS idx_
_expires_at + ON
(expires_at) WHERE expires_at IS NOT NULL; +``` + +**R6-UNIQUE-PARTIAL** — For "at most one default per org" patterns: + +```sql +CREATE UNIQUE INDEX IF NOT EXISTS idx_
_default_per_org + ON
(organization_uuid) WHERE is_default = 1; +``` + +SQL Server spells a partial index `WHERE ...` as a filtered index with the same syntax; SQLite supports partial indexes from 3.8.0. Where an engine cannot express it, enforce the invariant in application code and comment the divergence. + +**R6-NO-REDUNDANT-INDEX** — Do not create an index that is a prefix of an existing UNIQUE constraint or PK. + +**R6-GIN-JSONB** — Add a GIN index only when the application concretely queries inside a JSONB column with JSONB operators — do not pre-emptively GIN-index every JSONB column. + +--- + +## R7 · Application Logic Safety + +**R7-NO-SELECT-STAR** — Schema changes break `SELECT *` callers. All queries must select named columns. + +**R7-PARAMETERIZED** — Every query built from request input uses `?`/named placeholders, never `fmt.Sprintf` of a value; dynamic identifiers (sort columns, table names) resolve through an explicit allowlist (`GO-AUTH-008`). + +**R7-JSONB-SCAN** — A `JSONB` column must be scanned into a type that implements `sql.Scanner`. Scanning into `string` bypasses Postgres validation. + +**R7-HANDLE-IMMUTABLE** — `handle` must be set on INSERT and never appear in `UPDATE` statements. + +**R7-CREATED-IMMUTABLE** — `created_at` and `created_by` must not appear in `UPDATE` statements. + +**R7-LAYER-SYNC** — A schema change must land in `internal/model` (struct + `db:` tags), `internal/repository` (every `SELECT`/`INSERT` column list), the DTO/mapping layer, and fixtures **in the same commit**. A column added to the schema alone is either silently ignored or a scan-time failure. + +**R7-SOFT-DELETE** — Prefer status transitions to terminal states (`REVOKED`, `ARCHIVED`, `RETIRED`) over soft-delete (`deleted_at`) columns. Only add `deleted_at` when the design explicitly requires row-level tombstones. + +**R7-OPTIMISTIC-LOCK** — For resources that are concurrently edited, compare `updated_at` before issuing an UPDATE: + +```sql +UPDATE
+SET ..., updated_at = CURRENT_TIMESTAMP, updated_by = $n +WHERE uuid = $1 AND updated_at = $expected_updated_at +``` + +If 0 rows are affected, the caller lost the race and must retry. + +--- + +## R8 · Multi-Engine Schema Alignment + +The project maintains schema files for multiple database engines (Postgres + SQLite + SQL Server). Every change lands in **all** of a component's dialect files in the same commit — a change to one alone is a broken deployment on the others, and nothing cross-checks them. The files must remain structurally in sync except for the intentional type-level divergences below. + +**Intentional divergences — not findings:** + +| Feature | SQLite | PostgreSQL | SQL Server | +|---|---|---|---| +| JSON-valued columns (queried with JSON operators) | `TEXT` | `JSONB` | `NVARCHAR(MAX)` | +| JSON-valued columns (opaque storage only) | `TEXT` | `VARCHAR(N)` or `BYTEA` | `VARCHAR(N)` or `NVARCHAR(MAX)` | +| Binary columns | `BLOB` | `BYTEA` | `VARBINARY(MAX)` | +| Large text payloads | `TEXT` | `BYTEA` | `NVARCHAR(MAX)` | +| All timestamps | `DATETIME` | `TIMESTAMPTZ` | `DATETIME2(7) DEFAULT SYSUTCDATETIME()` | +| Boolean flags (0/1) | `INTEGER` | `SMALLINT` | `SMALLINT` | +| JSON literal defaults | `'{}'` | `'{}'::jsonb` | `'{}'` | +| Converging cascade paths | `ON DELETE CASCADE` | `ON DELETE CASCADE` | `ON DELETE NO ACTION` + app-side cleanup (R4) | + +**R8-SYNC-STRUCTURE** — Table definitions (columns, order, constraints, CHECK values, FK targets) not in the intentional-divergence list must be identical across all schema files. Verify: +- Same columns, same order +- Same `NOT NULL` / nullable on each column +- Same `DEFAULT` values (modulo type syntax) +- Same `CHECK` constraint values +- Same index definitions + +**R8-SQLITE-NO-JSONB** — SQLite does not support `JSONB`. Any `JSONB` in a SQLite schema file is a bug — all JSON columns must be `TEXT`. + +--- + +## R9 · Idempotent DDL + +Every `CREATE TABLE` and `CREATE INDEX` statement must be safe to re-run without errors. + +**R9-TABLE** — Use the engine-specific existence guard. `CREATE TABLE IF NOT EXISTS` is **not valid T-SQL**: + +```sql +-- PostgreSQL / SQLite +CREATE TABLE IF NOT EXISTS
(...); + +-- SQL Server +IF OBJECT_ID(N'dbo.
', N'U') IS NULL +CREATE TABLE dbo.
(...); +``` + +**R9-INDEX** — Use `IF NOT EXISTS` (Postgres/SQLite) or a `sys.indexes` check (SQL Server): + +```sql +-- PostgreSQL / SQLite +CREATE INDEX IF NOT EXISTS idx_... ON
(...); + +-- SQL Server +IF NOT EXISTS (SELECT 1 FROM sys.indexes WHERE name = N'idx_...' AND object_id = OBJECT_ID(N'dbo.
')) +CREATE INDEX idx_... ON dbo.
(...); +``` + +Keep `CREATE INDEX` statements in a dedicated block after all `CREATE TABLE` statements. A `CREATE TABLE` or `CREATE INDEX` without an existence guard is always a finding at MEDIUM severity. + +--- + +## R10 · Naming Conventions + +**R10-LOWERCASE** — Every SQL identifier — table names, column names, index names, and constraint names — must be lowercase `snake_case`. PostgreSQL folds unquoted identifiers to lowercase, so a mixed-case identifier only resolves if **every** reference quotes it (`"MyTable"`), which is brittle and breaks silently across engines and ORMs. Use `organization_uuid`, not `organizationUuid` or `OrganizationUUID`; `idx_apis_org`, not `idx_APIs_Org`. An upper-case or camelCase identifier is a finding at MEDIUM severity. + +**R10-MAPPING-SUFFIX** — Pure junction/mapping tables (those defined under R1-COMPOSITE-PK) must be named with a `_mappings` suffix, e.g. `application_api_mappings`, `gateway_association_mappings`. The suffix distinguishes link tables from entity tables at a glance and keeps the schema self-documenting. A pure mapping table named without the `_mappings` suffix is a finding at LOW severity. (Tables that link two entities but also carry their own identity/lifecycle are entity tables, not mapping tables — they keep a UUID PK and an entity-style name.) + +--- + +## Quick-Reference Templates + +Copy-paste starting points for new DDL. The junction/mapping table template is shown above under **R1-COMPOSITE-PK**. + +### New entity table (Postgres) + +```sql +CREATE TABLE IF NOT EXISTS
( + uuid VARCHAR(40) PRIMARY KEY, + organization_uuid VARCHAR(40) NOT NULL, + handle VARCHAR(40) NOT NULL, + name VARCHAR(255) NOT NULL, + version VARCHAR(30) NOT NULL DEFAULT 'v1.0', + status VARCHAR(20) NOT NULL DEFAULT 'CREATED', + description VARCHAR(1023), + data_version VARCHAR(20) NOT NULL DEFAULT '1.0', + created_by VARCHAR(200), + created_at TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP, + updated_by VARCHAR(200), + updated_at TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP, + UNIQUE(organization_uuid, handle), + FOREIGN KEY (organization_uuid) REFERENCES organizations(uuid) ON DELETE CASCADE +); + +CREATE INDEX IF NOT EXISTS idx_
_org ON
(organization_uuid); +CREATE INDEX IF NOT EXISTS idx_
_status ON
(status); +``` + +### Same table, SQL Server counterpart + +```sql +IF OBJECT_ID(N'dbo.
', N'U') IS NULL +CREATE TABLE dbo.
( + uuid VARCHAR(40) PRIMARY KEY, + organization_uuid VARCHAR(40) NOT NULL, + handle VARCHAR(40) NOT NULL, + name VARCHAR(255) NOT NULL, + version VARCHAR(30) NOT NULL DEFAULT 'v1.0', + status VARCHAR(20) NOT NULL DEFAULT 'CREATED', + description VARCHAR(1023), + data_version VARCHAR(20) NOT NULL DEFAULT '1.0', + created_by VARCHAR(200), + created_at DATETIME2(7) DEFAULT SYSUTCDATETIME(), + updated_by VARCHAR(200), + updated_at DATETIME2(7) DEFAULT SYSUTCDATETIME(), + UNIQUE(organization_uuid, handle), + FOREIGN KEY (organization_uuid) REFERENCES organizations(uuid) ON DELETE CASCADE +); +``` + +### Standard column types & widths + +``` +VARCHAR(20) — status, lifecycle_status, kind, short enums + — data_version (audit column, DEFAULT '1.0') +VARCHAR(30) — version (resource version, DEFAULT 'v1.0') +VARCHAR(40) — uuid, all FK columns referencing UUIDs + — handle (url-safe slug, NOT NULL; unique via UNIQUE(organization_uuid, handle)) +VARCHAR(200) — created_by, updated_by, revoked_by (user email/subject) +VARCHAR(255) — name, display strings + — hashes (SHA-256 hex) + — SAFE upper bound for indexed/unique columns across all engines +VARCHAR(512) — tokens (encrypted values) +VARCHAR(1023) — description, reason + — UPPER BOUND for plain-storage VARCHAR (above this → BYTEA/BLOB) + +BYTEA (Postgres) / BLOB (SQLite) / VARBINARY(MAX) (SQL Server) + — openapi_spec, model_list, content, configuration, properties, + manifest, policy_definition, metadata, api_key_hashes, + and any payload that can exceed a few hundred bytes + +JSONB — Postgres only; only when queried with JSON operators + — SQLite equivalent: TEXT (intentional, not a finding) + — SQL Server equivalent: NVARCHAR(MAX) (intentional, not a finding) + +TIMESTAMPTZ — all timestamps in Postgres (created_at, updated_at, expires_at, …) +DATETIME — all timestamps in SQLite +DATETIME2(7) DEFAULT SYSUTCDATETIME() — all timestamps in SQL Server + +SMALLINT — boolean flags in Postgres and SQL Server (is_active, is_default …) — use 0/1 +INTEGER — boolean flags in SQLite — use 0/1 +``` diff --git a/.agents/skills/designing-db-schemas/scripts/generate-schema-report.js b/.agents/skills/designing-db-schemas/scripts/generate-schema-report.js new file mode 100644 index 000000000..b9c87820b --- /dev/null +++ b/.agents/skills/designing-db-schemas/scripts/generate-schema-report.js @@ -0,0 +1,137 @@ +#!/usr/bin/env node +// generate-schema-report.js +// +// Belongs to the `designing-db-schemas` skill. +// Writes a structured JSON findings report from schema review findings. +// +// Usage: +// node generate-schema-report.js \ +// --findings '' (required) \ +// --schema (required) \ +// [--out ] (default: ./schema-reports/schema-review.json) +// +// Finding shape (each element of --findings array): +// { "rule": "R3-NO-TEXT", "table": "apis", "column": "config", +// "severity": "HIGH"|"MEDIUM"|"LOW"|"LEGACY-ACCEPTED", "finding": "...", "fix": "..." } +// +// LEGACY-ACCEPTED marks a violation on a shipped (GA) table, frozen by R0. +// It is recorded, not remediated — such findings carry no `fix`. +// +// Output shape: +// { +// "meta": { "schema": "...", "reviewedAt": "...", "rules": [...] }, +// "summary": { "HIGH": N, "MEDIUM": N, "LOW": N, "LEGACY-ACCEPTED": N }, +// "findings": [ { "id": "r1-001", "severity": "...", "rule": "...", ... } ] +// } +// +// IDs are deterministic: findings are sorted on stable keys BEFORE numbering, +// so the same finding set always yields the same rN-### ids regardless of the +// order they were passed in. That keeps ids comparable across reviews. + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +// ---------- arg parsing ---------- +const args = process.argv.slice(2); +function flag(name) { + const i = args.indexOf(name); + return i !== -1 ? args[i + 1] : null; +} + +if (args.includes('--help') || args.includes('-h')) { + console.log("Usage: generate-schema-report.js --findings '[...]' --schema [--out ]"); + process.exit(0); +} + +const findingsRaw = flag('--findings'); +const schemaPath = flag('--schema'); +const outPath = flag('--out') || './schema-reports/schema-review.json'; + +if (!findingsRaw || !schemaPath) { + console.error("Usage: generate-schema-report.js --findings '[...]' --schema [--out ]"); + process.exit(1); +} + +// ---------- parse findings ---------- +let findings; +try { + findings = JSON.parse(findingsRaw); +} catch (e) { + console.error('--findings must be a valid JSON array:', e.message); + process.exit(1); +} + +if (!Array.isArray(findings)) { + console.error('--findings must be a JSON array'); + process.exit(1); +} + +// Severity ordering and the set of supported, normalised severity values. +// LEGACY-ACCEPTED sorts last: it is a record of an R0-frozen deviation, not +// actionable work. +const ORDER = { HIGH: 0, MEDIUM: 1, LOW: 2, 'LEGACY-ACCEPTED': 3 }; + +// ---------- normalise (no ids yet) ---------- +const normalised = findings.map(f => { + const rule = f.rule || 'UNKNOWN'; + const sev = String(f.severity || 'MEDIUM').toUpperCase(); + return { + severity: ORDER[sev] !== undefined ? sev : 'MEDIUM', + rule, + table: f.table || null, + column: f.column || null, + finding: f.finding || '', + fix: f.fix || '', + }; +}); + +// ---------- sort on stable keys BEFORE numbering ---------- +// Severity first (report order), then rule/table/column/finding so that two +// runs over the same findings in a different input order produce identical ids. +const cmp = (a, b) => String(a ?? '').localeCompare(String(b ?? '')); +normalised.sort((a, b) => + (ORDER[a.severity] ?? 9) - (ORDER[b.severity] ?? 9) || + cmp(a.rule, b.rule) || + cmp(a.table, b.table) || + cmp(a.column, b.column) || + cmp(a.finding, b.finding) +); + +// ---------- assign ids from the sorted order ---------- +const counters = {}; +for (const f of normalised) { + // Rule-group identifier per the report contract: R3-NO-TEXT -> r3 + const group = f.rule.split('-')[0].toLowerCase().replace(/[^a-z0-9]/g, '') || 'unknown'; + counters[group] = (counters[group] || 0) + 1; + f.id = `${group}-${String(counters[group]).padStart(3, '0')}`; +} + +// Put `id` first in each object for readability +const ordered = normalised.map(({ id, severity, rule, table, column, finding, fix }) => + ({ id, severity, rule, table, column, finding, fix })); + +// ---------- summary counts ---------- +const summary = { HIGH: 0, MEDIUM: 0, LOW: 0, 'LEGACY-ACCEPTED': 0 }; +for (const f of ordered) summary[f.severity] = (summary[f.severity] || 0) + 1; + +// ---------- build output ---------- +const report = { + meta: { + schema: schemaPath, + reviewedAt: new Date().toISOString(), + rules: ['R0','R1','R2','R3','R4','R5','R6','R7','R8','R9','R10'], + }, + summary, + findings: ordered, +}; + +// ---------- write output ---------- +const outDir = path.dirname(outPath); +if (!fs.existsSync(outDir)) fs.mkdirSync(outDir, { recursive: true }); + +fs.writeFileSync(outPath, JSON.stringify(report, null, 2) + '\n'); +console.log(`Schema review report written to: ${outPath}`); +console.log(` HIGH: ${summary.HIGH} MEDIUM: ${summary.MEDIUM} LOW: ${summary.LOW}` + + ` LEGACY-ACCEPTED: ${summary['LEGACY-ACCEPTED']} Total: ${ordered.length}`); diff --git a/.claude/rules/db-schema-changes.md b/.claude/rules/db-schema-changes.md new file mode 100644 index 000000000..41ff62fe0 --- /dev/null +++ b/.claude/rules/db-schema-changes.md @@ -0,0 +1,33 @@ +# Rule: Database Schema Changes + +## Context & Scope + +Applies whenever adding or altering a table, column, index, or constraint in any `*.sql` schema file, and whenever changing the Go `model`/`repository` code that reads or writes those columns. + +**The rules are R0–R10, defined in `.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md`.** That file is the single source of truth — column types and widths, primary-key and foreign-key shape, org-scoping, audit columns, indexing, multi-engine alignment, idempotent DDL, and naming. Invoke the `designing-db-schemas` skill to apply them; this file exists to state the two things that govern *whether* a change is allowed at all, and to hold the deviations register. + +## Directives + +1. **These are GA products — shipped tables are frozen (R0-FROZEN).** Gateway Controller, Event Gateway Controller, Platform API, API Portal, and AI Workspace (which packages Platform API's schemas) are all in customer hands. On a table that has shipped, the only permitted changes are additive: a new nullable-or-defaulted column, a new index, a new table. Never retype, re-widen, rename, add or drop a primary key, change a foreign key's target or `ON DELETE`, add `NOT NULL` to an existing nullable column, or add a `UNIQUE` constraint — each rewrites or revalidates customer data on upgrade, and this repo has no migration framework to do it safely. Column removal is a two-release sequence. Anything blocked here needs an approved migration plan, not a schema edit. + +2. **A shipped table that violates R1–R10 is accepted legacy, not a bug to fix (R0-LEGACY-ACCEPTED).** Record it in Appendix A below at severity `LEGACY-ACCEPTED` and move on. Do not write remediation DDL, and do not let a schema audit turn into unplanned migration work. + +3. **Every change ships its upgrade path (R0-UPGRADE-PATH).** `CREATE TABLE IF NOT EXISTS` is a no-op against a database that already exists, so a column added to a `CREATE TABLE` body reaches fresh installs only. Ship the matching per-dialect `ALTER TABLE`, nullable or defaulted so it applies while the previous release is still running. + +4. **No deferring a violation behind a code comment.** Never resolve a missing dialect file, a missing `ALTER` path, an absent foreign key, or a plaintext-secret column with a `-- TODO`/`FIXME` comment and merge anyway — a comment does not create a column on a customer's database. Fix it, or raise a tracked issue with an owner and a deadline in the PR description. + +> **Before outputting any schema change:** +> * Does it touch a shipped table in any way other than adding a nullable/defaulted column or an index? (If so, stop — that's a migration.) +> * Is a shipped table's non-conformance being "fixed" instead of recorded in Appendix A? +> * Is there a per-dialect `ALTER TABLE` for already-provisioned databases? +> * Have R1–R10 been applied to the new table/column via the `designing-db-schemas` skill? + +--- + +## Appendix A — Accepted legacy deviations + +Shipped tables that don't conform to R1–R10, frozen under Directive 1. **Do not "fix" these.** Reviews add a row rather than re-reporting; the `designing-db-schemas` Workflow B writes findings here at severity `LEGACY-ACCEPTED`. + +| Product | Table | Rule | Deviation | Recorded | +|---|---|---|---|---| +| _(populate from the first full audit)_ | | | | | From ce6d5ab69cc813356ac3c636fc429d409210c28a Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Tue, 11 Aug 2026 21:01:30 +0530 Subject: [PATCH 2/4] Enhance database schema rules documentation --- .../api-platform-db-schema-rules.md | 22 +++++----- .../scripts/generate-schema-report.js | 40 +++++++++++++++---- .claude/rules/db-schema-changes.md | 4 +- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md b/.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md index 8aaede930..dc314e733 100644 --- a/.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md +++ b/.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md @@ -53,7 +53,7 @@ find . -name "*.sql" -not -path "*/node_modules/*" -not -path "*/target/*" | sor grep -rln "" --include="*.sql" . ``` -**R0-UPGRADE-PATH** — `CREATE TABLE IF NOT EXISTS` is a no-op against a database that already exists, so a column added to a `CREATE TABLE` body reaches fresh installs only. Every additive change ships a matching per-dialect `ALTER TABLE`, nullable or defaulted so it applies while the previous release is still running: +**R0-UPGRADE-PATH** — `CREATE TABLE IF NOT EXISTS` is a no-op against a database that already exists, so a column added to a `CREATE TABLE` body reaches fresh installs only. A **column added to an existing table** therefore ships a matching per-dialect `ALTER TABLE`, nullable or defaulted so it applies while the previous release is still running. A **new table** or a **new index** needs no `ALTER` — its guarded `CREATE ... IF NOT EXISTS` (R9) already runs against both fresh and existing databases: ```sql ALTER TABLE ADD COLUMN NULL; -- postgres / sqlite @@ -66,7 +66,7 @@ If a component has no upgrade-script location, create one beside its schema file ## R1 · Primary Key & Identity -Every table must have a single UUID primary key and, where it is a named resource, the standard identity triple. +Every **entity** table must have a single UUID primary key and, where it is a named resource, the standard identity triple. Pure junction/mapping tables are excluded — they carry a composite PK instead (R1-COMPOSITE-PK). **R1-UUID** — Primary key must be `uuid VARCHAR(40) PRIMARY KEY`. Do not use `SERIAL`, `BIGINT`, or `INTEGER` as a primary key for domain entities. Junction/mapping tables must use a composite PK (see R1-COMPOSITE-PK). @@ -153,14 +153,16 @@ Do **not** raise R3-NO-TEXT against SQLite or SQL Server schema files: per R8, ` **R3-LARGE-PAYLOAD** — Any payload that can grow large or is variable-length must use the engine-appropriate large type, never a wide VARCHAR. Apply: binary payloads → `BYTEA` (Postgres) / `BLOB` (SQLite) / `VARBINARY(MAX)` (SQL Server); large text payloads → `BYTEA` (Postgres) / `TEXT` (SQLite) / `NVARCHAR(MAX)` (SQL Server), per the R8 divergence table. Columns that always need this: `openapi_spec`, `model_list`, `content`, `configuration`, `properties`, `manifest`, `policy_definition`, `metadata`, `api_key_hashes`, or any future column whose value can exceed a few hundred bytes. (The SQLite/SQL Server forms above are the intended divergences — do not flag them as R3-NO-TEXT.) -**R3-JSONB** — In PostgreSQL, use `JSONB` only when the application queries inside the JSON using Postgres JSON operators. Evidence that a column is actively queried inside: -1. The `DEFAULT` is a JSON literal like `'{}'` -2. The column name implies structure: `settings`, `event_data` -3. The same column in a sibling table already uses `JSONB` +**R3-JSONB** — In PostgreSQL, use `JSONB` only when the application demonstrably queries inside the JSON using Postgres JSON operators. The evidence must be a concrete query in the repository layer using `->`, `->>`, `#>`, `#>>`, `@>`, `?`, `jsonb_path_*`, or an equivalent JSONB operator/function against that column. A JSON-literal `DEFAULT`, a structured-sounding column name, or a sibling table already using `JSONB` are **not** evidence — they are how columns end up `JSONB` without anything ever querying inside them. Absent such a query, store the payload per R3-LARGE-PAYLOAD instead. Once direct query evidence exists, R3-JSONB-SCAN-COMPAT still applies to the scan target. SQLite and SQL Server equivalents (`TEXT` / `NVARCHAR(MAX)`) are intentional type-level divergences — not findings. -**R3-JSONB-SCAN-COMPAT** — **PostgreSQL only** (`JSONB` does not exist in SQLite or SQL Server, so this rule never applies to those files). Do not use `JSONB` if the application layer scans it into a plain `string` variable and calls `json.Unmarshal` manually. Postgres drivers return JSONB as binary, which breaks `string` scan targets at runtime. Only use `JSONB` when the scan target implements `sql.Scanner` (e.g. `pgtype.JSONB`, `json.RawMessage`, or a custom struct). +**R3-JSONB-SCAN-COMPAT** — **PostgreSQL only** (`JSONB` does not exist in SQLite or SQL Server, so this rule never applies to those files). Whether a `JSONB` column can be scanned into a given Go type is **driver-specific**; check the driver before raising a finding. + +- **`github.com/jackc/pgx/v5` (v5.9.2, the driver `platform-api` uses).** `JSONBCodec` scans into `*string` and `*[]byte`, into any `sql.Scanner` implementation, and — as a fallback — into any other pointer target by `json.Unmarshal`. So `var s string` plus a manual `json.Unmarshal(s)` is supported here, as is `*json.RawMessage` (which is a `[]byte` alias, not an `sql.Scanner` implementation). Do not flag either as a scan-compatibility violation under pgx v5. +- **Other PostgreSQL drivers** (e.g. `lib/pq`, or pgx used through `database/sql` with a different codec registration) do not all convert JSONB to text for a `*string` target. When a schema targets one of those, restrict the scan target to `[]byte` or a type that implements `database/sql.Scanner` (a custom struct with a `Scan(src any) error` method, or `pgtype.JSON`/`pgtype.JSONB`-style wrapper types provided by that driver). + +The rule that survives across drivers: pick the scan target deliberately and confirm it against the driver in use — never assume a `string` target works, and never assume it fails. **R3-BOOLEAN-AS-INT** — Do not use the `BOOLEAN` type. Represent boolean flags as `0`/`1` in: - `SMALLINT` — PostgreSQL and SQL Server @@ -277,7 +279,7 @@ Index every column (or compound) that appears in a `WHERE`, `JOIN`, or `ORDER BY CREATE INDEX IF NOT EXISTS idx_
_ ON
(); ``` -**R6-ORG-INDEX** — Every org-scoped table must have an index on `organization_uuid`: +**R6-ORG-INDEX** — Every org-scoped table must have an index on `organization_uuid`, unless it is already the leftmost column of the PK or of a covering UNIQUE constraint — in which case that index already serves `organization_uuid` lookups and a separate one is redundant (R6-NO-REDUNDANT-INDEX). This is the normal case for junction tables built per R1-COMPOSITE-PK, which put `organization_uuid` first. ```sql CREATE INDEX IF NOT EXISTS idx_
_org ON
(organization_uuid); @@ -319,7 +321,7 @@ SQL Server spells a partial index `WHERE ...` as a filtered index with the same **R7-PARAMETERIZED** — Every query built from request input uses `?`/named placeholders, never `fmt.Sprintf` of a value; dynamic identifiers (sort columns, table names) resolve through an explicit allowlist (`GO-AUTH-008`). -**R7-JSONB-SCAN** — A `JSONB` column must be scanned into a type that implements `sql.Scanner`. Scanning into `string` bypasses Postgres validation. +**R7-JSONB-SCAN** — A `JSONB` column's scan target must be one the configured driver actually supports; see R3-JSONB-SCAN-COMPAT for the pgx-v5 vs other-driver split before raising a finding. **R7-HANDLE-IMMUTABLE** — `handle` must be set on INSERT and never appear in `UPDATE` statements. @@ -459,7 +461,7 @@ CREATE TABLE dbo.
( ### Standard column types & widths -``` +```text VARCHAR(20) — status, lifecycle_status, kind, short enums — data_version (audit column, DEFAULT '1.0') VARCHAR(30) — version (resource version, DEFAULT 'v1.0') diff --git a/.agents/skills/designing-db-schemas/scripts/generate-schema-report.js b/.agents/skills/designing-db-schemas/scripts/generate-schema-report.js index b9c87820b..9a90da1d6 100644 --- a/.agents/skills/designing-db-schemas/scripts/generate-schema-report.js +++ b/.agents/skills/designing-db-schemas/scripts/generate-schema-report.js @@ -35,9 +35,15 @@ const path = require('path'); // ---------- arg parsing ---------- const args = process.argv.slice(2); +// Returns the token after `name`, or null when the option is absent, has no +// following token, or is followed by another option — so `--schema --out x` +// fails the usage check below instead of silently taking "--out" as a path. function flag(name) { const i = args.indexOf(name); - return i !== -1 ? args[i + 1] : null; + if (i === -1) return null; + const value = args[i + 1]; + if (value === undefined || value.startsWith('--')) return null; + return value; } if (args.includes('--help') || args.includes('-h')) { @@ -74,16 +80,33 @@ if (!Array.isArray(findings)) { const ORDER = { HIGH: 0, MEDIUM: 1, LOW: 2, 'LEGACY-ACCEPTED': 3 }; // ---------- normalise (no ids yet) ---------- -const normalised = findings.map(f => { - const rule = f.rule || 'UNKNOWN'; - const sev = String(f.severity || 'MEDIUM').toUpperCase(); +// Malformed records are rejected outright rather than coerced to a default — a +// silently-defaulted severity or an 'UNKNOWN' rule would land in the report as +// if it were a real finding. +const normalised = findings.map((f, i) => { + if (f === null || typeof f !== 'object' || Array.isArray(f)) { + console.error(`--findings[${i}] must be an object`); + process.exit(1); + } + if (typeof f.rule !== 'string' || f.rule.trim() === '') { + console.error(`--findings[${i}] must have a non-empty string "rule"`); + process.exit(1); + } + const sev = String(f.severity ?? 'MEDIUM').toUpperCase(); + if (ORDER[sev] === undefined) { + console.error(`--findings[${i}] has unsupported severity ${JSON.stringify(f.severity)}; ` + + `expected one of ${Object.keys(ORDER).join(', ')}`); + process.exit(1); + } return { - severity: ORDER[sev] !== undefined ? sev : 'MEDIUM', - rule, + severity: sev, + rule: f.rule.trim(), table: f.table || null, column: f.column || null, finding: f.finding || '', - fix: f.fix || '', + // A LEGACY-ACCEPTED finding records an R0-frozen deviation and is never + // remediated, so it carries no fix even if one was supplied. + fix: sev === 'LEGACY-ACCEPTED' ? '' : (f.fix || ''), }; }); @@ -96,7 +119,8 @@ normalised.sort((a, b) => cmp(a.rule, b.rule) || cmp(a.table, b.table) || cmp(a.column, b.column) || - cmp(a.finding, b.finding) + cmp(a.finding, b.finding) || + cmp(a.fix, b.fix) ); // ---------- assign ids from the sorted order ---------- diff --git a/.claude/rules/db-schema-changes.md b/.claude/rules/db-schema-changes.md index 41ff62fe0..4120f3c72 100644 --- a/.claude/rules/db-schema-changes.md +++ b/.claude/rules/db-schema-changes.md @@ -12,9 +12,9 @@ Applies whenever adding or altering a table, column, index, or constraint in any 2. **A shipped table that violates R1–R10 is accepted legacy, not a bug to fix (R0-LEGACY-ACCEPTED).** Record it in Appendix A below at severity `LEGACY-ACCEPTED` and move on. Do not write remediation DDL, and do not let a schema audit turn into unplanned migration work. -3. **Every change ships its upgrade path (R0-UPGRADE-PATH).** `CREATE TABLE IF NOT EXISTS` is a no-op against a database that already exists, so a column added to a `CREATE TABLE` body reaches fresh installs only. Ship the matching per-dialect `ALTER TABLE`, nullable or defaulted so it applies while the previous release is still running. +3. **Every change ships its upgrade path (R0-UPGRADE-PATH).** What that path is depends on the change: a **new table** or **new index** needs only its guarded `CREATE ... IF NOT EXISTS` (or the `OBJECT_ID`/`sys.indexes` equivalent), which runs against fresh and already-provisioned databases alike. A **column added to an existing table** needs more: `CREATE TABLE IF NOT EXISTS` is a no-op against a database that already exists, so a column added to a `CREATE TABLE` body reaches fresh installs only — ship the matching per-dialect `ALTER TABLE`, nullable or defaulted so it applies while the previous release is still running. -4. **No deferring a violation behind a code comment.** Never resolve a missing dialect file, a missing `ALTER` path, an absent foreign key, or a plaintext-secret column with a `-- TODO`/`FIXME` comment and merge anyway — a comment does not create a column on a customer's database. Fix it, or raise a tracked issue with an owner and a deadline in the PR description. +4. **No deferring a violation behind a code comment.** Never resolve a missing dialect file, a missing `ALTER` path, an absent foreign key, or a plaintext-secret column with a `-- TODO`/`FIXME` comment and merge anyway — a comment does not create a column on a customer's database. Fix it, or raise a tracked issue with an owner and a deadline in the PR description. This applies to violations **in the change under review**. A violation that already exists in a shipped table is exempt: it is `LEGACY-ACCEPTED` under Directive 2 — record it in Appendix A, do not write remediation DDL, and leave the fix to an approved migration plan. Recording it there is the documentation, not a deferral comment. > **Before outputting any schema change:** > * Does it touch a shipped table in any way other than adding a nullable/defaulted column or an index? (If so, stop — that's a migration.) From 6465d6decc10764dbc0a87a91bdbdbd69c917eb9 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Tue, 11 Aug 2026 21:49:03 +0530 Subject: [PATCH 3/4] Refine documentation for database schema design skill, updating code block formatting and clarifying self-review checklist steps. --- .agents/skills/designing-db-schemas/SKILL.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.agents/skills/designing-db-schemas/SKILL.md b/.agents/skills/designing-db-schemas/SKILL.md index 3a005d988..b05960441 100644 --- a/.agents/skills/designing-db-schemas/SKILL.md +++ b/.agents/skills/designing-db-schemas/SKILL.md @@ -12,7 +12,7 @@ The rules live in **`references/api-platform-db-schema-rules.md`** (next to this ## Usage -``` +```text /designing-db-schemas [table-name | path-to-schema-file] ``` @@ -93,9 +93,10 @@ Read `references/api-platform-db-schema-rules.md`. The rules you need depend on #### Step A3 — Self-review checklist -``` +```text [ ] R0 Change is additive — no retype/rename/PK-FK change/NOT NULL/UNIQUE on a shipped table -[ ] R0 Per-dialect ALTER TABLE written for already-provisioned databases +[ ] R0 Column added to an existing table: per-dialect ALTER TABLE written for already-provisioned + databases (new tables/indexes need none — their guarded CREATE covers both cases) [ ] R1 Entity tables: uuid VARCHAR(40) PRIMARY KEY [ ] R1 Junction/mapping tables: composite PRIMARY KEY — not UNIQUE-only, not surrogate UUID [ ] R1 Non-leading FK columns of a composite PK have their own indexes @@ -115,7 +116,8 @@ Read `references/api-platform-db-schema-rules.md`. The rules you need depend on [ ] R4 Every FK has an explicit ON DELETE clause [ ] R5 User-initiated table → all four audit columns; system-managed → created_by/updated_by ABSENT [ ] R5 Every domain entity table has data_version VARCHAR(20) NOT NULL DEFAULT '1.0' -[ ] R6 FK columns, organization_uuid, and filtered status columns have indexes +[ ] R6 FK columns, organization_uuid, and filtered status columns have indexes — except where the + column is already the leftmost part of the PK or a covering UNIQUE constraint [ ] R7 Go model/repository/DTO updated in the same commit; named columns, no SELECT * [ ] R8 Change applied to every dialect file (or divergence is intentional and documented) [ ] R9 All DDL is idempotent (IF NOT EXISTS / OBJECT_ID / sys.indexes guards) @@ -143,7 +145,9 @@ Keep `CREATE INDEX` statements in a dedicated block after all `CREATE TABLE` sta #### Step A5 — Apply to all schema files, then ship the upgrade path -Apply to every in-scope dialect file, same column order, same position. Only the R8 divergences may differ. Then write the per-dialect `ALTER TABLE` (R0-UPGRADE-PATH) — `CREATE TABLE IF NOT EXISTS` does nothing to a database that already exists. +Apply to every in-scope dialect file, same column order, same position. Only the R8 divergences may differ. + +Then, **if the change adds a column to an existing table**, write the per-dialect `ALTER TABLE` (R0-UPGRADE-PATH) — `CREATE TABLE IF NOT EXISTS` does nothing to a database that already exists, so a column added to a `CREATE TABLE` body reaches fresh installs only. A new table or a new index needs no `ALTER`: its guarded `CREATE ... IF NOT EXISTS` / `OBJECT_ID` / `sys.indexes` form (R9, Step A4) already applies to fresh and already-provisioned databases alike. #### Step A6 — Verify on more than one dialect From b2f7f0ddc964b0646388d3202a700c269a41c6e5 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Wed, 12 Aug 2026 12:52:24 +0530 Subject: [PATCH 4/4] Clarify database schema change rules and update JSONB handling guidelines in documentation. --- .agents/skills/designing-db-schemas/SKILL.md | 3 ++- .claude/rules/db-schema-changes.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.agents/skills/designing-db-schemas/SKILL.md b/.agents/skills/designing-db-schemas/SKILL.md index b05960441..10df06ad4 100644 --- a/.agents/skills/designing-db-schemas/SKILL.md +++ b/.agents/skills/designing-db-schemas/SKILL.md @@ -105,7 +105,8 @@ Read `references/api-platform-db-schema-rules.md`. The rules you need depend on [ ] R2 organization_uuid FK present; UNIQUE constraints include it (if org-scoped) [ ] R3 No bare TEXT in Postgres — SQLite TEXT / SQL Server NVARCHAR(MAX) are intentional (R8) [ ] R3 Large/variable payloads use BYTEA/BLOB/VARBINARY(MAX) — not wide VARCHAR -[ ] R3 JSONB only when queried with JSON operators AND the scan target implements sql.Scanner +[ ] R3 JSONB only when a repository query reads inside it with JSON operators; scan target checked + against the driver in use (pgx v5 also takes *string/*[]byte — see R3-JSONB-SCAN-COMPAT) [ ] R3 Boolean flags: SMALLINT (Postgres/SQL Server) or INTEGER (SQLite), 0/1 — no BOOLEAN [ ] R3 VARCHAR widths match R3-VARCHAR-SIZES; nothing above VARCHAR(1023) for plain storage [ ] R3 Indexed/UNIQUE columns ≤ VARCHAR(255); hashes are VARCHAR(255) diff --git a/.claude/rules/db-schema-changes.md b/.claude/rules/db-schema-changes.md index 4120f3c72..3537a980f 100644 --- a/.claude/rules/db-schema-changes.md +++ b/.claude/rules/db-schema-changes.md @@ -19,7 +19,7 @@ Applies whenever adding or altering a table, column, index, or constraint in any > **Before outputting any schema change:** > * Does it touch a shipped table in any way other than adding a nullable/defaulted column or an index? (If so, stop — that's a migration.) > * Is a shipped table's non-conformance being "fixed" instead of recorded in Appendix A? -> * Is there a per-dialect `ALTER TABLE` for already-provisioned databases? +> * Does a column added to an existing table ship a per-dialect `ALTER TABLE` for already-provisioned databases? (New tables and indexes need none — their guarded `CREATE` covers both cases.) > * Have R1–R10 been applied to the new table/column via the `designing-db-schemas` skill? ---