Skip to content

feat(appkit): add the service-principal typed DatabasePlugin API - #526

Open
ditadi wants to merge 1 commit into
stack/database-mvp/01-runtimefrom
stack/database-mvp/02-typed-api
Open

feat(appkit): add the service-principal typed DatabasePlugin API#526
ditadi wants to merge 1 commit into
stack/database-mvp/01-runtimefrom
stack/database-mvp/02-typed-api

Conversation

@ditadi

@ditadiditadi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Stack

Each PR targets the one above it, so the diff shown here is only the delta on top of #525. Review in order.

What

Turns the runtime from #525 into a plugin you can actually use. database({ schema }) publishes one typed client per table, plus transactions and parameterized SQL, and the existing typegen flow learns to derive the declarations that make all of it typed from the schema file alone.

This is the first PR in the series with an exported surface. The plugin runs as the app's service principal and registers no HTTP routes — generated CRUD arrives in the next PR, so everything here is reachable only from server code you write.

import{database,defineSchema,id,text,timestamp}from"@databricks/appkit/beta";constschema=defineSchema((t)=>{constnotes=t.table("notes",{id: id(),body: text().notNull(),createdAt: timestamp().defaultNow(),});return{ notes };});constappkit=awaitcreateApp({plugins: [database({ schema })]});constrecent=awaitappkit.database.notes.order({createdAt: "desc"}).limit(10).toArray();awaitappkit.database.transaction(async(tx)=>{constnote=awaittx.notes.create({body: "hello"});awaittx.sql`insert into audit (note_id) values (${note.id})`;});

Changes

The plugin (plugins/database/)

  • database({ schema }) binds one plugin instance to one finalized schema. lifecycle.ts owns setup: it validates the schema, builds the Lakebase pool, and publishes the export surface only once it is ready. Setup is single-flight, and a failure ends the pool rather than leaving a half-open plugin.
  • Each table gets an EntityClient with a chainable read side — where, order, select, include, limit, offset, terminated by toArray, first, find, or count — and the keyed writes create, update, upsert, and delete. Every call goes through the DataPath from feat(appkit): add the database runtime and harden its schema builder #525, so the bounds, the parameterization, and the private-column projection hold here by construction.
  • transaction(cb) hands the callback a client bound to that transaction; the tagged sql template is available both at the top level and inside a transaction, and interpolates values only.
  • Reads and writes carry different interceptor policies (defaults.ts): neither retries, and mutations are never cached.

Errors (database/errors.ts)

DatabasePluginError maps a small closed set of categories onto stable status codes and client messages. A driver error never reaches the caller: it is classified, and the original is logged with its SQLSTATE before the safe error replaces it. The same is true for schema-validation and setup failures, so an operator can diagnose what the client is deliberately not shown.

Schema-derived types (type-generator/database/)

walk-schema.ts walks a finalized schema and generate.ts renders it into appkit-types/database.d.ts, which augments the DatabaseRegistry interface. That is what makes appkit.database.notes know its own columns, filters, and relations. The generator loads the schema file through jiti (pinned at 2.6.1), so a TypeScript schema needs no build step first.

It is wired into both existing entry points — the Vite plugin regenerates on change during development, and appkit generate-types emits it in CI — following the same shape the analytics and serving generators already use.

Exports

defineSchema, the column builders, and database are exported from @databricks/appkit/beta; DatabaseRegistry is exported from the root so the generated declaration file can augment it.

Verification

  • pnpm vitest run — 4057 passing, 1 skipped; the new suites cover the plugin lifecycle, the entity client, the generated types, and the schema walker
  • pnpm -r typecheck — clean across all packages
  • pnpm run generate:types, pnpm run sync:template, and pnpm run docs:build produce no drift
  • pnpm install --frozen-lockfilejiti adds 3 lines to the lockfile and nothing else

Expose the hardened runtime as one service-principal plugin with typed entity clients,
transactions, tagged SQL, and schema-derived declarations in the existing typegen flow.
Driver, setup, and unclassified failures are logged with their original cause before
the safe error replaces them, so operators can diagnose what the client never sees.
Signed-off-by: ditadi <victordperd@gmail.com>
}

/** Snapshot and compose predicates; the adapter validates columns/operators. */
where(filter: WhereClause): EntityClient {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] Private-column enforcement is absent at this layer.where/order/select/include snapshot the caller input and pass it straight to the ungated DataPath, so db.notes.where({author_email: {like: "%@x.com"}}) (a .private() column) executes as a filter oracle and .select(["author_email"]) projects it. The generated types even include private columns in the row/filters facets, so there is no type-level deterrent. #527's HTTP layer filters via selectable/queryable, but any hand-written route forwarding untrusted where/select here re-opens the exposure.

Automated review finding.

);
}

upsert(values: Row, options: { onConflict: string }): Promise<Row> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] upsert can silently rewrite a natural primary key.$insertSchema excludes only serverGenerated columns, so a natural PK (e.g. uuid().primaryKey() / text().primaryKey()) stays in the validated payload. db.users.upsert({id:"B", email:"a@x.com"}, {onConflict:"email"}) against an existing {id:"A", email:"a@x.com"} emits ON CONFLICT (email) DO UPDATE SET id="B" → the existing row's PK flips A→B (FK orphaning / identity reassignment). id()/bigid() PKs are safe. Fix: build the DO UPDATE set from $updateSchema semantics (exclude the PK).

Automated review finding.

/** AppKit-facing database failure with stable metadata and no driver details. */
export class DatabasePluginError extends AppKitError {
readonly code = "DATABASE_PLUGIN_ERROR";
readonly isRetryable = false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] Transient serialization/deadlock aborts are non-retryable and misclassified.isRetryable is hardcoded false, and classifyDriverError maps everything but 42501/23xxx to INTERNAL — so 40001 (serialization_failure) and 40P01 (deadlock) surface as opaque, non-retryable 500s. A SERIALIZABLE/high-contention workload cannot distinguish or retry the transient abort Postgres asked it to retry (and transaction() bypasses the RetryInterceptor regardless). Consider a distinguishable retryable category for 40001/40P01.

Automated review finding.

: code?.startsWith("23")
? "CONFLICT"
: "INTERNAL";
logger.error(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] %O logs the raw driver error → row/PII in server logs.classifyDriverError logs the full pg error with %O; on a routine 23505 the pg detail is e.g. Key (email)=(alice@x.com) already exists, so private/PII column values land in stdout — bypassing the AppKitError redaction and undermining .private(). Log only the SQLSTATE and category, never the raw driver object.

Automated review finding.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ditadi@MarioCadenas