Skip to content

Commit d25f700

Browse files
os-samclaude
andauthored
fix(service-package): unwrap the mysql2 [rows, fields] tuple in normalizeRows (#11207)
The local flattener had two accepting branches where its docblock claimed three-dialect coverage. mysql2's `[rows, fields]` tuple is an array, so it satisfied the bare-array branch and was returned whole: `get()` read index 0 — the row array, not a row — so `row.manifest` was undefined, `JSON.parse` threw into the method's own catch, and the caller was told the package was not installed over a driver that had just returned it. `list()` failed the same way into `[]`. Reachable in a supported composition: `OS_DATABASE_URL=mysql://…` dispatches to a SqlDriver on the mysql2 client as the DEFAULT driver, and both `ObjectQLEngine.execute()` and `SqlDriver.execute()` return the client's shape verbatim. Kept a local copy rather than depending on `@objectstack/metadata-protocol`, matching the call this file's `isResultSet` already documents. Drift is pinned instead: `mysql2-tuple.test.ts` asserts all three dialect shapes recover the same row, that the tuple test cannot misfire on a bare row array, and that an empty result in any dialect still answers "no rows". Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9883f58 commit d25f700

4 files changed

Lines changed: 291 additions & 18 deletions

File tree

.changeset/eighty-donkeys-shave.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/service-package": patch
3+
---
4+
5+
Unwrap the mysql2 `[rows, fields]` tuple in the package service's row flattener
6+
7+
On a MySQL/MariaDB-backed deployment (`OS_DATABASE_URL=mysql://…`, which builds a
8+
`SqlDriver` on the `mysql2` client as the default driver), `PackageService.get()` and
9+
`PackageService.list()` reported **"this package is not installed"** and **"no packages
10+
are installed"** over a database that had just returned the rows.
11+
12+
`ObjectQLEngine.execute()` and `SqlDriver.execute()` both pass the underlying client's
13+
result through verbatim, so mysql2's `[rows, fields]` tuple reached the service's local
14+
`normalizeRows` unflattened. The tuple is an array, so it was returned whole; `get()`
15+
then read index 0 — the row *array* rather than a row — and `JSON.parse(undefined)` threw
16+
into the method's own catch, which answers `null`. `list()` failed the same way into `[]`.
17+
Boot-time package hydration read the same empty answer and silently installed nothing.
18+
19+
The flattener now unwraps the tuple, matching the three-dialect coverage its own docblock
20+
already claimed and the `metadata-protocol` sibling already implemented. The bare row
21+
array (better-sqlite3 through knex, Turso) and `{ rows, rowCount }` (pg) shapes are
22+
unchanged, and an empty result in any of the three still answers "no rows" rather than
23+
raising the seam refusal.

packages/services/service-package/src/index.ts

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -166,20 +166,55 @@ function declaresHttpAnswer(error: unknown): boolean {
166166
/**
167167
* Normalize the result of `objectql.execute()` into a row array.
168168
*
169-
* Different drivers return different shapes for raw SELECT statements:
170-
* - SQL driver (knex/SQLite) and Turso remote transport return rows
171-
* directly as an array.
172-
* - PostgreSQL (knex/pg) returns `{ rows, rowCount, ... }`.
173-
* - Some drivers may return `{ rows: [...] }` wrappers in other contexts.
169+
* `ObjectQLEngine.execute()` returns what the driver returned, VERBATIM, and
170+
* `SqlDriver.execute()` in turn returns `knex.raw()` verbatim — so the shape
171+
* that arrives here is the underlying client's own, and there are THREE:
174172
*
175-
* This helper accepts any of those shapes and always returns an array.
173+
* 1. **bare row array** — better-sqlite3 through knex, and Turso's remote
174+
* transport (which returns `@libsql/client`'s `result.rows`).
175+
* 2. **`{ rows, rowCount, … }`** — PostgreSQL (knex/pg).
176+
* 3. **`[rows, fields]` tuple** — mysql2. The first element is itself the
177+
* row array; the second is column metadata.
178+
*
179+
* ⚠️ [#11062] Shape 3 used to fall through the `Array.isArray` branch
180+
* UNFLATTENED, and that was not a shape this file merely failed to support — it
181+
* was a shape it MISREAD. The tuple is an array, so it satisfied both the
182+
* branch below and {@link isResultSet}; `get()` then read `rows[0]`, which is
183+
* the row ARRAY rather than a row, so `row.manifest` was `undefined` and
184+
* `JSON.parse(undefined)` threw into `get()`'s own catch — which answers
185+
* `null`, i.e. **"this package is not installed"** over a driver that had just
186+
* returned the row. `list()` failed the same way into `[]`. The defect is
187+
* reachable in a supported composition: `OS_DATABASE_URL=mysql://…` builds a
188+
* `SqlDriver` on the `mysql2` client as the DEFAULT driver, which is the one
189+
* this service's raw SELECTs land on.
190+
*
191+
* The tuple test is `Array.isArray(result[0])`, and it cannot misfire on shape
192+
* 1: a bare row array holds row OBJECTS. Measured on `@libsql/client` 0.17.4,
193+
* `result.rows` is a real array whose elements are plain objects
194+
* (`Array.isArray(rows[0]) === false`), and knex/better-sqlite3 likewise maps
195+
* rows to objects. Only mysql2 nests an array at index 0.
196+
*
197+
* ⛔ A LOCAL copy, deliberately — the same call this file's {@link isResultSet}
198+
* already makes and for the same reason: `@objectstack/metadata-protocol`
199+
* (whose exported `normalizeRows` is the sibling this arm is aligned with) is
200+
* not a dependency of this package at all, and it resolves through `exports` to
201+
* `dist/`, so value-importing it would make this package's unit pins a verdict
202+
* about a build artifact (`check:test-source-alias`). Unifying the copies is
203+
* its own decision, not a rider on this fix. What holds them together
204+
* meanwhile is a pin: `mysql2-tuple.test.ts` asserts all three shapes recover
205+
* the SAME row, so the next divergence is a red test rather than prose someone
206+
* has to re-read.
176207
*
177208
* ⚠️ [#10965] It returns `[]` for EVERYTHING else too, and that is the whole
178209
* defect this file's seam guard exists for — see {@link isResultSet}. Flatten
179210
* with this only AFTER the result has been established as an answer.
180211
*/
181212
function normalizeRows(result: any): any[] {
182-
if (Array.isArray(result)) return result;
213+
if (Array.isArray(result)) {
214+
// mysql2's `[rows, fields]`: the first element is itself the row array.
215+
if (result.length > 0 && Array.isArray(result[0])) return result[0];
216+
return result;
217+
}
183218
if (result && Array.isArray(result.rows)) return result.rows;
184219
return [];
185220
}
Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #11062 — the mysql2 `[rows, fields]` tuple was never unwrapped, so a
5+
* POPULATED result answered "not installed".
6+
*
7+
* ## The shape, and why it reaches this file
8+
*
9+
* `ObjectQLEngine.execute()` returns what the driver returned verbatim, and
10+
* `SqlDriver.execute()` returns `knex.raw()` verbatim — so the raw client's own
11+
* per-dialect shape arrives at this service's `normalizeRows` untouched.
12+
* `sql-driver.ts` says so in its own words at the one place it flattens a raw
13+
* SELECT internally: *"mysql2 returns [rows, fields]"*.
14+
*
15+
* This is a SUPPORTED composition, not a hypothetical one:
16+
* `OS_DATABASE_URL=mysql://…` is dispatched by `standalone-stack.ts` to
17+
* `kind === 'mysql'` → a `SqlDriver` on the `mysql2` client, as the DEFAULT
18+
* driver — and the default driver is exactly the one `objectql.execute()`
19+
* selects for this service's raw SELECTs (its own docblock names
20+
* `PackageService` as the caller it exists for). `os serve` loads
21+
* `PackageServicePlugin` for the `marketplace` feature over that same engine.
22+
*
23+
* ## What the defect did
24+
*
25+
* The tuple is an ARRAY, so it satisfied both the old `Array.isArray(result)`
26+
* branch and `isResultSet` — no false 503, and #10965's guard was never at
27+
* fault. `normalizeRows` simply returned the 2-element tuple, so:
28+
*
29+
* - `get()` read `rows[0]` — the row ARRAY, not a row. `row.manifest` was
30+
* `undefined`, `JSON.parse(undefined)` threw into `get()`'s catch, and the
31+
* catch answers `null` ⇒ **"this package is not installed"**.
32+
* - `list()` mapped over `[rows, fields]` and threw in the same place, into a
33+
* catch that answers `[]` ⇒ **"no packages are installed"**.
34+
*
35+
* Both over a driver that had just returned the row. The swallowed throw is the
36+
* signature, which is why the populated cases below also assert that NOTHING
37+
* was logged to `error` — a fix that returned the right rows while still
38+
* throwing and recovering somewhere would pass a rows-only assertion.
39+
*
40+
* ## What is pinned — all three dialects, one row, one answer
41+
*
42+
* The parity case is the point: the SAME logical row, spelled three ways,
43+
* must produce the SAME answer. A fix that taught the flattener the tuple but
44+
* broke the bare array or `{ rows }` would fail here, and so would a "fix" that
45+
* special-cased mysql2 into a different result. Shapes 1 and 2 are not
46+
* regression ballast — they are half the contract.
47+
*/
48+
49+
import { describe, it, expect } from 'vitest';
50+
import {
51+
PackageServicePlugin,
52+
type PackageService,
53+
} from './index.js';
54+
55+
const MANIFEST = { id: 'com.acme.crm', name: 'CRM', version: '1.0.0', type: 'application' };
56+
const METADATA = { author: 'ACME', installedBy: 'os-dev' };
57+
58+
/** One stored row, as every dialect hands back the columns of `sys_packages`. */
59+
const ROW = {
60+
id: 'com.acme.crm',
61+
version: '1.0.0',
62+
manifest: JSON.stringify(MANIFEST),
63+
metadata: JSON.stringify(METADATA),
64+
hash: 'd3adb33f',
65+
created_at: '2026-08-23T00:00:00.000Z',
66+
updated_at: '2026-08-23T00:00:00.000Z',
67+
};
68+
69+
/** The answer both read doors must produce from that row, in every dialect. */
70+
const EXPECTED = {
71+
id: 'com.acme.crm',
72+
version: '1.0.0',
73+
manifest: MANIFEST,
74+
metadata: METADATA,
75+
hash: 'd3adb33f',
76+
created_at: '2026-08-23T00:00:00.000Z',
77+
updated_at: '2026-08-23T00:00:00.000Z',
78+
};
79+
80+
/**
81+
* mysql2's second tuple element — column metadata, never rows.
82+
*
83+
* Spelled out rather than left `[]` so the tuple case cannot pass by accident:
84+
* with a non-empty second element, returning the whole tuple yields a
85+
* 2-element `list()`, which is visibly wrong rather than coincidentally equal.
86+
*/
87+
const FIELDS = [
88+
{ name: 'id', type: 253 },
89+
{ name: 'version', type: 253 },
90+
{ name: 'manifest', type: 252 },
91+
];
92+
93+
interface Booted {
94+
svc: PackageService;
95+
errorLogs: string[];
96+
}
97+
98+
/** Boot the real plugin over a seam that returns `result` for every SELECT. */
99+
async function bootReturning(result: unknown): Promise<Booted> {
100+
const errorLogs: string[] = [];
101+
const engine: any = {
102+
async execute({ sql }: { sql: string; args?: unknown[] }) {
103+
// DDL from `ensureTable` answers like a real driver; only SELECTs carry
104+
// the dialect shape under test.
105+
return /^\s*select/i.test(sql) ? result : undefined;
106+
},
107+
};
108+
109+
let registered: PackageService | undefined;
110+
const ctx: any = {
111+
logger: {
112+
debug: () => {},
113+
info: () => {},
114+
warn: () => {},
115+
error: (msg: string) => errorLogs.push(String(msg)),
116+
},
117+
getService: (n: string) => (n === 'objectql' ? engine : undefined),
118+
registerService: (_n: string, s: PackageService) => { registered = s; },
119+
};
120+
121+
const plugin = new PackageServicePlugin();
122+
await plugin.init(ctx);
123+
await plugin.start(ctx);
124+
return { svc: registered!, errorLogs };
125+
}
126+
127+
// ───────────────────────────────────────────────────────────────────────────
128+
// 1. The three dialect shapes recover the SAME row
129+
// ───────────────────────────────────────────────────────────────────────────
130+
131+
describe('#11062 normalizeRows — every supported dialect answers with the row', () => {
132+
const dialects: Array<[string, unknown]> = [
133+
['bare row array (better-sqlite3 through knex, Turso remote)', [ROW]],
134+
['`{ rows, rowCount }` (pg)', { rows: [ROW], rowCount: 1 }],
135+
['`[rows, fields]` tuple (mysql2)', [[ROW], FIELDS]],
136+
];
137+
138+
for (const [label, shape] of dialects) {
139+
it(`get() returns the package from ${label}`, async () => {
140+
const { svc, errorLogs } = await bootReturning(shape);
141+
await expect(svc.get('com.acme.crm', 'latest')).resolves.toEqual(EXPECTED);
142+
// The defect's signature was a SWALLOWED throw, not a wrong return.
143+
expect(errorLogs).toEqual([]);
144+
});
145+
146+
it(`list() returns exactly one package from ${label}`, async () => {
147+
const { svc, errorLogs } = await bootReturning(shape);
148+
await expect(svc.list()).resolves.toEqual([EXPECTED]);
149+
expect(errorLogs).toEqual([]);
150+
});
151+
}
152+
153+
it('all three dialects agree — same row in, same answer out', async () => {
154+
const answers = [];
155+
for (const [, shape] of dialects) {
156+
const { svc } = await bootReturning(shape);
157+
answers.push(await svc.list());
158+
}
159+
expect(answers[0]).toEqual(answers[1]);
160+
expect(answers[1]).toEqual(answers[2]);
161+
});
162+
});
163+
164+
// ───────────────────────────────────────────────────────────────────────────
165+
// 2. The tuple branch must not misfire on the shapes that already worked
166+
// ───────────────────────────────────────────────────────────────────────────
167+
168+
describe('#11062 the tuple test cannot swallow a bare row array', () => {
169+
/**
170+
* The unwrap keys on `Array.isArray(result[0])`, so it fires only where a
171+
* dialect NESTS an array at index 0. A bare row array holds row OBJECTS —
172+
* measured on `@libsql/client` 0.17.4, `result.rows` is a real array whose
173+
* elements are plain objects (`Array.isArray(rows[0]) === false`), and knex
174+
* over better-sqlite3 likewise maps rows to objects.
175+
*/
176+
it('a multi-row bare array keeps every row', async () => {
177+
const second = { ...ROW, id: 'com.acme.hr', version: '2.0.0' };
178+
const { svc } = await bootReturning([ROW, second]);
179+
const listed = await svc.list();
180+
expect(listed).toHaveLength(2);
181+
expect(listed.map((p: any) => p.id)).toEqual(['com.acme.crm', 'com.acme.hr']);
182+
});
183+
184+
it('a single-row bare array is not mistaken for a tuple', async () => {
185+
const { svc } = await bootReturning([ROW]);
186+
await expect(svc.list()).resolves.toEqual([EXPECTED]);
187+
});
188+
});
189+
190+
// ───────────────────────────────────────────────────────────────────────────
191+
// 3. An EMPTY answer stays an answer — in every dialect, including the tuple
192+
// ───────────────────────────────────────────────────────────────────────────
193+
194+
describe('#11062 empty results remain "no rows", never a refusal', () => {
195+
/**
196+
* The half that stops this being a rename (#10965's leg, re-asserted for the
197+
* shape this card adds): an empty result set in ANY spelling is still a
198+
* result set, so it answers "not installed" / "nothing installed" rather than
199+
* raising the seam refusal.
200+
*/
201+
const empties: Array<[string, unknown]> = [
202+
['bare `[]`', []],
203+
['`{ rows: [], rowCount: 0 }` (pg)', { rows: [], rowCount: 0 }],
204+
['`[[], fields]` (mysql2, zero rows)', [[], FIELDS]],
205+
];
206+
207+
for (const [label, shape] of empties) {
208+
it(`${label} answers no-rows without throwing`, async () => {
209+
const { svc, errorLogs } = await bootReturning(shape);
210+
await expect(svc.get('com.acme.crm', 'latest')).resolves.toBeNull();
211+
await expect(svc.list()).resolves.toEqual([]);
212+
expect(errorLogs).toEqual([]);
213+
});
214+
}
215+
});

packages/services/service-package/src/null-seam.test.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,14 @@
3939
*
4040
* ## What is deliberately NOT asserted here
4141
*
42-
* This service's LOCAL `normalizeRows` implements TWO of the three dialect
43-
* shapes — a bare row array and `{ rows }`. It does not unwrap the mysql2
44-
* `[rows, fields]` tuple the way `metadata-protocol`'s copy does (that one
45-
* tests `Array.isArray(result[0])`). So no populated-tuple result is asserted
46-
* here: it would be a pin on behaviour this file does not have. What IS pinned
47-
* is that a tuple-shaped result is still treated as an ANSWER, so the guard
48-
* cannot misfire on a dialect it does not fully flatten. The gap itself is
49-
* filed separately rather than fixed as a rider.
42+
* The ROWS a dialect yields are not this file's subject — only the
43+
* answered/unanswered separation is. When these tests were written the local
44+
* `normalizeRows` implemented two of the three dialect shapes and did not
45+
* unwrap the mysql2 `[rows, fields]` tuple; that gap was filed rather than
46+
* fixed as a rider, and closed in #11062. The populated-tuple assertions live
47+
* with the rest of the dialect-shape contract in `mysql2-tuple.test.ts`. What
48+
* stays pinned HERE is the part this card owns: a tuple-shaped result is an
49+
* ANSWER, so the guard cannot misfire on it.
5050
*/
5151

5252
import { describe, it, expect } from 'vitest';
@@ -267,9 +267,9 @@ describe('#10965 the guard never turns a result set into a refusal', () => {
267267
});
268268

269269
it('an `[rows, fields]`-shaped result is an ANSWER — the guard does not misfire', async () => {
270-
// This local flattener does not UNWRAP the tuple (filed separately), so
271-
// nothing is asserted about the rows it yields. What is asserted is the
272-
// only thing this card owns: it is not mistaken for a seam that failed to
270+
// Nothing is asserted here about the rows the tuple yields — that is
271+
// `mysql2-tuple.test.ts`'s contract (#11062). What is asserted is the only
272+
// thing this card owns: it is not mistaken for a seam that failed to
273273
// answer, so no dialect gets a false 503.
274274
const { svc } = await bootWith(seamReturning([[], []]));
275275
await expect(svc.list()).resolves.toEqual([]);

0 commit comments

Comments
 (0)