Skip to content

Commit 0dd925f

Browse files
dmealingclaude
andcommitted
fix(runtime-ts): make the Hono CRUD helpers work on Postgres (#286)
Hono's mountCrudRoutes / mountReadOnlyRoutes called Drizzle's .all() and .get(). Those are libsql/better-sqlite3-only; the node-postgres builder is thenable but has neither, so every GET failed with `TypeError: q.all is not a function`. Shipped broken on Postgres through 0.21.2 and 0.21.3, reported from a real adoption. The Fastify adapter was already correct and carries a comment explaining exactly this — the fix just never got ported. All six Hono call sites now use the portable form: `await q` for lists, `.limit(1)` + `[0]` for single-row reads. The one-liner is not the interesting part. Every adapter test in this package used libsql, where .all()/.get() exist and work, so NO test could have caught the incompatibility for either adapter — Fastify's fix was reasoned, not gated. A per-adapter suite with a single dialect structurally cannot detect a dialect divergence. So this adds a cross-adapter dialect MATRIX against a real Postgres: the same read paths (list, get-by-id, 404, withCount envelope, filter+sort) asserted across Fastify AND Hono, plus a payload-equality check between them. Adding a third adapter means adding a row, not a new file. Verified to reproduce the reported `q.all is not a function` on four tests with the fix reverted. runtime-ts's suite runs in the ts-unit lane, which has no database, so the matrix is wired into ts-slow — the lane with the Postgres sidecar — next to the migrate-ts real-PG gate, with the same loud-skip sentinel: a lane that declares it intends Postgres and then does not supply it now fails rather than skipping in silence. runtime-ts 389 pass against real PG, typecheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KTGT5ksntpcJDZVJ5VyXHS
1 parent ee3b08a commit 0dd925f

8 files changed

Lines changed: 262 additions & 8 deletions

File tree

‎.github/workflows/local-ci.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,12 @@ jobs:
150150
# Makes the in-suite sentinel FAIL if the URL above ever rots away (renamed
151151
# variable, dropped sidecar) rather than describe.skip-ing in silence.
152152
MIGRATE_TS_PG_EXPECT: '1'
153+
# Same, for runtime-ts's cross-adapter dialect matrix (#286). runtime-ts's own
154+
# suite runs in ts-unit, which has no database, and every adapter test there uses
155+
# libsql — where Drizzle's libsql-only .all()/.get() work. That is exactly how
156+
# Hono shipped 500ing on Postgres while Fastify, fixed for the same thing,
157+
# stayed green. METAOBJECTS_TEST_PG_URL above is the matrix's connection.
158+
RUNTIME_TS_PG_EXPECT: '1'
153159
run: scripts/ci-local.sh --only ts-slow --strict-toolchains
154160

155161
# Java FAST lane — java + kotlin conformance only (the quick correctness signal).

‎CHANGELOG.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,36 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
### Fixed — [#286](https://github.com/metaobjectsdev/metaobjects/issues/286): the Hono CRUD helpers 500'd on Postgres (npm)
11+
12+
`runtime-ts`'s Hono `mountCrudRoutes` / `mountReadOnlyRoutes` called Drizzle's `.all()`
13+
and `.get()`. Those are **libsql / better-sqlite3-only**; the node-postgres builder is
14+
thenable but has neither, so every `GET` failed with
15+
`TypeError: q.all is not a function`. Shipped broken on Postgres through `0.21.2` and
16+
`0.21.3`. Reported from a real adoption.
17+
18+
The Fastify adapter was already correct — it awaits the builder directly and carries a
19+
comment explaining exactly this. The fix simply never got ported to Hono. All six call
20+
sites now use the portable form (`await q` for lists, `.limit(1)` + `[0]` for single-row
21+
reads).
22+
23+
**Why it survived, which matters more than the one-liner:** every adapter test in the
24+
package used libsql, where `.all()`/`.get()` exist and work — so no test could have caught
25+
the incompatibility for *either* adapter, and Fastify's fix was reasoned rather than
26+
gated. A per-adapter suite with a single dialect structurally cannot detect a dialect
27+
divergence.
28+
29+
Closed with a **cross-adapter dialect matrix** against a real Postgres: the same read paths
30+
(list, get-by-id, 404, `withCount` envelope, filter+sort) asserted across Fastify *and*
31+
Hono, plus a payload-equality check between them. Adding a third adapter means adding a row,
32+
not a new file. Verified to reproduce the reported `q.all is not a function` on four tests
33+
with the fix reverted.
34+
35+
`runtime-ts`'s own suite runs in CI's `ts-unit` lane, which has no database, so the matrix
36+
is wired into `ts-slow` (the lane with the Postgres sidecar) alongside the migrate-ts
37+
real-PG gate, with the same loud-skip sentinel — a lane that declares it intends Postgres
38+
and then fails to supply it now fails instead of silently skipping.
39+
1040
### Fixed — [#285](https://github.com/metaobjectsdev/metaobjects/issues/285): `meta migrate` emitted an un-appliable `DROP INDEX` for a constraint-backed index (npm)
1141

1242
Postgres creates an index to enforce `UNIQUE` / `PRIMARY KEY` / `EXCLUDE`, then refuses to

‎bun.lock‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎scripts/ci-local.sh‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,17 @@ gate_ts_build() { bun_install && bun run --filter '*' build; }
154154
# vars (a local run with no Postgres) the PG describes self-skip exactly as before.
155155
gate_migrate_ts_pg() { bun_install && ( cd server/typescript/packages/migrate-ts && bun test ); }
156156

157+
# runtime-ts real-Postgres dialect matrix — the generated CRUD helpers exercised through
158+
# BOTH the Fastify and Hono adapters against a real engine. runtime-ts's own suite runs in
159+
# the ts-unit lane, which has no database, and every adapter test there uses libsql — where
160+
# Drizzle's libsql-only `.all()`/`.get()` work. That is why Hono shipped 500ing on Postgres
161+
# while Fastify (fixed for the same thing) stayed green (#286). Runs here because ts-slow is
162+
# the lane with the sidecar; METAOBJECTS_TEST_PG_URL + RUNTIME_TS_PG_EXPECT=1 come from the
163+
# CI job, and without them the suite self-skips exactly as before.
164+
gate_runtime_ts_pg() {
165+
bun_install && ( cd server/typescript/packages/runtime-ts && bun test test/dialect-matrix-pg.test.ts )
166+
}
167+
157168
# ── conformance.yml — per-port conformance corpora (exact CI commands) ────────
158169
gate_conf_ts() {
159170
bun_install || return 1
@@ -356,6 +367,7 @@ else
356367
# Ordered BEFORE the docker integration step so a container-readiness flake there
357368
# can never prevent the migrate verdict from being produced.
358369
want_any ts ts-slow && step_if bun "migrate-ts real-PG suite" gate_migrate_ts_pg
370+
want_any ts ts-slow && step_if bun "runtime-ts real-PG dialect matrix" gate_runtime_ts_pg
359371
want_any ts ts-slow && run_integration_for ts ts
360372
want_any java java-slow && run_integration_for java java kotlin
361373
want python && run_integration_for python python

‎server/typescript/packages/runtime-ts/package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,15 @@
9595
"devDependencies": {
9696
"@libsql/client": "^0.15.0",
9797
"@libsql/kysely-libsql": "^0.4.1",
98+
"@types/pg": "^8.20.0",
9899
"@types/qs": "^6.9.0",
99100
"bun-types": "latest",
100101
"drizzle-orm": "^0.45.1",
101102
"fastify": "^5.6.2",
102103
"hono": "^4.6.0",
103104
"jsdom": "^29.1.1",
104105
"kysely": "^0.27.0",
106+
"pg": "^8.20.0",
105107
"pg-mem": "^3.0.4",
106108
"typescript": "^5.6.0",
107109
"zod": "^4.4.3"

‎server/typescript/packages/runtime-ts/src/hono/index.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,19 @@ export function mountListRoute(opts: VerbOptions): void {
137137
if (flat.offset !== undefined) q = q.offset(Number(flat.offset));
138138
}
139139

140-
const rows = await q.all();
140+
// Await the query directly rather than calling `.all()`: the drizzle-orm
141+
// node-postgres builder is thenable but has no `.all()` (a libsql /
142+
// better-sqlite3-only API). Awaiting works on BOTH dialects — this is what
143+
// makes the Hono helpers genuinely Postgres-capable, matching the Fastify
144+
// adapter, which carried this fix while Hono did not (#286).
145+
const rows = await q;
141146

142147
if (!withCount) return c.json(rows);
143148

144149
// Count query: same WHERE, no limit/offset/orderBy.
145150
let cq = opts.db.select({ c: count() }).from(opts.table).$dynamic();
146151
if (where) cq = cq.where(where);
147-
const countRow = (await cq.all())[0] as { c: number } | undefined;
152+
const countRow = (await cq)[0] as { c: number } | undefined;
148153
const total = countRow?.c ?? 0;
149154
return c.json({ rows, total });
150155
} catch (err) {
@@ -163,11 +168,14 @@ export function mountGetRoute(opts: VerbOptions): void {
163168
// must stay a string ('0123' ≠ '123'), or affinity matches the WRONG row.
164169
const idValue = coerceIdForColumn(opts.table.id, id);
165170
if (idValue === undefined) return c.json({ error: "invalid_id" }, 400);
166-
const row = await opts.db
171+
// `.get()` is likewise libsql/better-sqlite3-only; `.limit(1)` + await + [0]
172+
// is the portable single-row read (#286).
173+
const rows = await opts.db
167174
.select()
168175
.from(opts.table)
169176
.where(eq(opts.table.id, idValue))
170-
.get();
177+
.limit(1);
178+
const row = (rows as unknown[])[0];
171179
return row ? c.json(row) : c.json({ error: "not_found" }, 404);
172180
});
173181
}

‎server/typescript/packages/runtime-ts/src/hono/mount-read-only.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,18 @@ export function mountReadOnlyCrudRoutes(opts: MountReadOnlyOptions): void {
134134
if (result.orderBy) q = q.orderBy(...result.orderBy);
135135
if (result.limit !== undefined) q = q.limit(result.limit);
136136
if (result.offset !== undefined) q = q.offset(result.offset);
137-
const rows = await q.all();
137+
// Await the query directly rather than calling `.all()`: the drizzle-orm
138+
// node-postgres builder is thenable but has no `.all()` (a libsql /
139+
// better-sqlite3-only API). Awaiting works on BOTH dialects — this is what
140+
// makes the Hono helpers genuinely Postgres-capable, matching the Fastify
141+
// adapter, which carried this fix while Hono did not (#286).
142+
const rows = await q;
138143

139144
if (!withCount) return c.json(rows);
140145

141146
let cq = db.select({ c: count() }).from(view);
142147
if (combinedWhere) cq = cq.where(combinedWhere);
143-
const countRow = (await cq.all())[0] as { c: number } | undefined;
148+
const countRow = (await cq)[0] as { c: number } | undefined;
144149
const total = countRow?.c ?? 0;
145150
return c.json({ rows, total });
146151
} catch (err) {
@@ -167,11 +172,14 @@ export function mountReadOnlyCrudRoutes(opts: MountReadOnlyOptions): void {
167172
if (idValue === undefined) {
168173
return c.json({ error: "invalid_id" }, 400);
169174
}
170-
const row = await db
175+
// `.get()` is likewise libsql/better-sqlite3-only; `.limit(1)` + await + [0]
176+
// is the portable single-row read (#286).
177+
const rows = await db
171178
.select()
172179
.from(view)
173180
.where(colRef !== undefined ? eq(colRef, idValue) : undefined)
174-
.get();
181+
.limit(1);
182+
const row = (rows as unknown[])[0];
175183
return row ? c.json(row) : c.json({ error: "not_found" }, 404);
176184
});
177185

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
/**
2+
* #286 — the CRUD helpers must work on Postgres, in BOTH adapters.
3+
*
4+
* Drizzle's `.all()` and `.get()` are libsql/better-sqlite3-only. The node-postgres
5+
* builder is thenable but has neither, so calling them 500s every GET with
6+
* `TypeError: q.all is not a function`. The Fastify adapter was fixed for this and
7+
* carries a comment explaining it; **Hono was not**, and shipped broken on Postgres
8+
* through 0.21.2 and 0.21.3. Reported from a real adoption.
9+
*
10+
* The reason it survived is the interesting part, and it is what this file exists to
11+
* close: EVERY adapter test in this package used libsql (`drizzle-orm/libsql`,
12+
* `:memory:`), where `.all()`/`.get()` exist and work. No test could have caught the
13+
* incompatibility for either adapter — Fastify's fix was reasoned, not gated. A
14+
* per-adapter test suite with a single dialect cannot detect a dialect divergence.
15+
*
16+
* So this is a MATRIX: the same read paths, asserted across both adapters, against a
17+
* REAL Postgres. Adding a third adapter should mean adding a row here, not a new file.
18+
*
19+
* Gated on METAOBJECTS_TEST_PG_URL (CI's ts-slow lane supplies a Postgres sidecar);
20+
* skips loudly-but-harmlessly when absent, and the sentinel below fails if a lane
21+
* claims it intends Postgres and then does not provide it.
22+
*/
23+
24+
import { describe, test, expect, beforeAll, afterAll } from "bun:test";
25+
import { Hono } from "hono";
26+
import Fastify, { type FastifyInstance } from "fastify";
27+
import { Pool } from "pg";
28+
import { drizzle } from "drizzle-orm/node-postgres";
29+
import { pgTable, serial, text, boolean } from "drizzle-orm/pg-core";
30+
import { z } from "zod";
31+
import { mountCrudRoutes as mountHono } from "../src/hono/index.js";
32+
import { mountCrudRoutes as mountFastify } from "../src/drizzle-fastify/index.js";
33+
import type { FilterAllowlist, SortAllowlist } from "../src/drizzle-fastify/filter-allowlist.js";
34+
35+
const PG_URL = process.env["METAOBJECTS_TEST_PG_URL"];
36+
37+
const subscribers = pgTable("mo286_subscribers", {
38+
id: serial("id").primaryKey(),
39+
email: text("email").notNull(),
40+
firstName: text("first_name").notNull(),
41+
subscribed: boolean("subscribed").notNull().default(true),
42+
});
43+
44+
const InsertSchema = z.object({
45+
email: z.string(),
46+
firstName: z.string(),
47+
subscribed: z.boolean().optional(),
48+
});
49+
const UpdateSchema = InsertSchema.partial();
50+
51+
const filterAllowlist: FilterAllowlist = {
52+
email: { ops: ["eq", "ne", "in", "like", "isNull"], subType: "string", leadingWildcard: true },
53+
firstName: { ops: ["eq", "ne", "in", "like", "isNull"], subType: "string", leadingWildcard: false },
54+
subscribed: { ops: ["eq", "isNull"], subType: "boolean", leadingWildcard: false },
55+
};
56+
const sortAllowlist: SortAllowlist = { email: {}, firstName: {} };
57+
58+
describe("real-PG dialect matrix sentinel", () => {
59+
test("METAOBJECTS_TEST_PG_URL is set when the lane declares RUNTIME_TS_PG_EXPECT=1", () => {
60+
if (process.env["RUNTIME_TS_PG_EXPECT"] === "1") {
61+
expect(PG_URL).toBeTruthy();
62+
}
63+
});
64+
});
65+
66+
describe("#286 — CRUD helpers on real Postgres, both adapters", () => {
67+
if (!PG_URL) {
68+
test.skip("skipped — METAOBJECTS_TEST_PG_URL not set", () => {});
69+
return;
70+
}
71+
72+
let pool: Pool;
73+
// biome-ignore lint/suspicious/noExplicitAny: drizzle db handle, structurally typed by the mounts
74+
let db: any;
75+
let hono: Hono;
76+
let fastify: FastifyInstance;
77+
78+
beforeAll(async () => {
79+
pool = new Pool({ connectionString: PG_URL });
80+
db = drizzle(pool);
81+
await pool.query(`DROP TABLE IF EXISTS mo286_subscribers`);
82+
await pool.query(
83+
`CREATE TABLE mo286_subscribers (
84+
id serial PRIMARY KEY,
85+
email text NOT NULL,
86+
first_name text NOT NULL,
87+
subscribed boolean NOT NULL DEFAULT true
88+
)`,
89+
);
90+
await pool.query(
91+
`INSERT INTO mo286_subscribers (email, first_name, subscribed)
92+
VALUES ('alice@x.com','Alice',true), ('bob@x.com','Bob',false), ('carol@y.com','Carol',true)`,
93+
);
94+
95+
hono = new Hono();
96+
mountHono({
97+
app: hono, path: "/subscribers", db, table: subscribers,
98+
insertSchema: InsertSchema, updateSchema: UpdateSchema,
99+
filterAllowlist, sortAllowlist, dialect: "postgres",
100+
});
101+
102+
fastify = Fastify();
103+
mountFastify({
104+
fastify, path: "/subscribers", db, table: subscribers,
105+
insertSchema: InsertSchema, updateSchema: UpdateSchema,
106+
filterAllowlist, sortAllowlist, dialect: "postgres",
107+
});
108+
await fastify.ready();
109+
});
110+
111+
afterAll(async () => {
112+
await fastify?.close();
113+
await pool.query(`DROP TABLE IF EXISTS mo286_subscribers`);
114+
await pool.end();
115+
});
116+
117+
/** Uniform request surface so the same assertions run against both adapters. */
118+
const adapters: Array<{ name: string; get: (url: string) => Promise<{ status: number; body: unknown }> }> = [
119+
{
120+
name: "hono",
121+
get: async (url) => {
122+
const res = await hono.request(url);
123+
return { status: res.status, body: res.status === 204 ? null : await res.json() };
124+
},
125+
},
126+
{
127+
name: "fastify",
128+
get: async (url) => {
129+
const res = await fastify.inject({ method: "GET", url });
130+
return { status: res.statusCode, body: res.statusCode === 204 ? null : res.json() };
131+
},
132+
},
133+
];
134+
135+
for (const a of adapters) {
136+
describe(a.name, () => {
137+
test("list returns rows (the exact call that threw `q.all is not a function`)", async () => {
138+
const { status, body } = await a.get("/subscribers");
139+
expect(status).toBe(200);
140+
expect(Array.isArray(body)).toBe(true);
141+
expect((body as unknown[]).length).toBe(3);
142+
});
143+
144+
test("get-by-id returns one row (the `.get()` path)", async () => {
145+
const list = (await a.get("/subscribers")).body as Array<{ id: number; email: string }>;
146+
const target = list.find((r) => r.email === "bob@x.com");
147+
expect(target).toBeDefined();
148+
const { status, body } = await a.get(`/subscribers/${target?.id}`);
149+
expect(status).toBe(200);
150+
expect((body as { email: string }).email).toBe("bob@x.com");
151+
});
152+
153+
test("get-by-id 404s for a missing row rather than throwing", async () => {
154+
const { status } = await a.get("/subscribers/99999");
155+
expect(status).toBe(404);
156+
});
157+
158+
test("withCount returns the {rows,total} envelope (the count-query `.all()` path)", async () => {
159+
const { status, body } = await a.get("/subscribers?withCount=1");
160+
expect(status).toBe(200);
161+
const env = body as { rows: unknown[]; total: number };
162+
expect(Array.isArray(env.rows)).toBe(true);
163+
expect(env.total).toBe(3);
164+
});
165+
166+
test("filter + sort execute on Postgres", async () => {
167+
const { status, body } = await a.get("/subscribers?filter[subscribed]=true&sort=email:desc");
168+
expect(status).toBe(200);
169+
const rows = body as Array<{ email: string }>;
170+
expect(rows.map((r) => r.email)).toEqual(["carol@y.com", "alice@x.com"]);
171+
});
172+
});
173+
}
174+
175+
test("both adapters return the SAME list payload — the matrix's whole point", async () => {
176+
const [h, f] = await Promise.all([
177+
a0().get("/subscribers?sort=email:asc"),
178+
a1().get("/subscribers?sort=email:asc"),
179+
]);
180+
expect(h.status).toBe(f.status);
181+
expect(h.body).toEqual(f.body);
182+
});
183+
184+
function a0() { return adapters[0]!; }
185+
function a1() { return adapters[1]!; }
186+
});

0 commit comments

Comments
 (0)