Skip to content

fix(service-package): unwrap the mysql2 [rows, fields] tuple so a populated result stops reading as "not installed" - #11207

Merged
os-sam merged 1 commit into
mainfrom
claude/issue-11062-service-package-mysql2-tuple
Aug 23, 2026
Merged

fix(service-package): unwrap the mysql2 [rows, fields] tuple so a populated result stops reading as "not installed"#11207
os-sam merged 1 commit into
mainfrom
claude/issue-11062-service-package-mysql2-tuple

Conversation

@os-sam

@os-sam os-sam commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Fixes #11062

The defect, and that it is LIVE rather than latent

The card traced this from source and explicitly did not reproduce it, leaving two things unmeasured. Both are answered below; the first changes the card's severity.

packages/services/service-package/src/index.ts's local normalizeRows had two accepting branches where its own docblock claimed three-dialect coverage:

if (Array.isArray(result)) return result;          // shape 1 — and, wrongly, shape 3
if (result && Array.isArray(result.rows)) return result.rows;
return [];

mysql2's [rows, fields] tuple is an array, so it satisfied the first branch and was returned whole. get() then read rows[0] — the row array, not a row — so row.manifest was undefined, JSON.parse(undefined) threw into get()'s own catch, and that catch answers null: "this package is not installed", over a driver that had just returned the row. list() mapped over the tuple and failed identically into []: "no packages are installed". Boot-time hydration reads list(), so it silently installed nothing.

This is not #10965 re-opened: that guard correctly treats a tuple as an answer, so no false 503 was ever introduced. The tuple simply was never unwrapped.

Q1 — can a mysql2 seam actually reach this service? Yes. Verified by reading the chain end to end.

Every hop passes the driver's shape through untouched:

hop evidence
ObjectQLEngine.execute() engine.ts ends return driver.execute(rawCommand, params, options) — verbatim. Its docblock names PackageService as a caller it exists for.
SqlDriver.execute() sql-driver.ts ends const result = await builder; … return result where builder = this.knex.raw(...) — verbatim.
the client's shape sql-driver.ts states it in its own words where it flattens a raw SELECT internally: // mysql2 returns [rows, fields].
a supported composition standalone-stack.ts dispatches OS_DATABASE_URL=mysql://… to kind === 'mysql' → a SqlDriver on the mysql2 client, as the default driver — which is the driver objectql.execute() selects for this service's raw SELECTs. serve.ts loads PackageServicePlugin for the marketplace feature over that same engine.

So the composition exists and is supported. Live, not latent.

Q2 — does the CLI's third copy have a matching gap? No. Nothing to file.

packages/cli/src/commands/migrate/duplicates.ts keeps a local isResultSet (a predicate — a tuple is an array, so it correctly answers true), but for flattening it imports normalizeRows from @objectstack/metadata-protocol — the three-branch copy that already unwraps the tuple. There are therefore only two flatteners in the repo, not three, and the CLI consumes the correct one.

The fix

The Array.isArray branch now tests for the nested row array first, matching the metadata-protocol sibling:

if (Array.isArray(result)) {
  // mysql2's `[rows, fields]`: the first element is itself the row array.
  if (result.length > 0 && Array.isArray(result[0])) return result[0];
  return result;
}

The tuple test cannot misfire on shape 1, because a bare row array holds row objects. Measured on the installed @libsql/client 0.17.4 (Turso's remote transport returns result.rows):

Array.isArray(rows)    : true
Array.isArray(rows[0]) : false     ← plain Object

knex over better-sqlite3 likewise maps rows to objects. Only mysql2 nests an array at index 0.

Local copy, not unification — and what holds them together

Kept local, deliberately, which is the call this file's isResultSet already documents: "Unifying the three is its own decision, not a rider on this fix." Reversing that here would make a dependency-graph change to a published package a rider on a 3-line bug fix — @objectstack/metadata-protocol is not a dependency of @objectstack/service-package at all, and it resolves through exports to dist/, so value-importing it would make this package's unit pins a verdict about a build artifact (check:test-source-alias). The card raises unification as an open question; it stays open and is left to PM/maintainer.

What keeps the copies from drifting is a pin rather than prose: mysql2-tuple.test.ts asserts all three dialect shapes recover the same row, so the next divergence is a red test.

Tests — pinned in both directions

packages/services/service-package/src/mysql2-tuple.test.ts (7 cases):

  • all three dialects, one row, one answer — bare array, { rows, rowCount }, and [rows, fields] each recover the identical package from get() and list(), plus an explicit cross-dialect agreement case. The two shapes that already worked are half the contract, not regression ballast.
  • no swallowed throw — the populated cases assert errorLogs is empty. The defect's signature was a caught exception, so a fix returning the right rows while still throwing-and-recovering would pass a rows-only assertion.
  • the tuple test cannot eat a bare array — multi-row and single-row bare arrays keep every row.
  • empty stays an answer[], { rows: [] }, and [[], fields] all answer "no rows" rather than raising the seam refusal (service-package answers "no such package" / "no packages installed" over a driver it never queried — its own normalizeRows maps a non-answering seam onto zero rows #10965's leg, re-asserted for the added shape).

null-seam.test.ts's "deliberately NOT asserted" section described this gap as standing; updated to point at the new pin. Its existing tuple case is unchanged and still green.

Reverse verification

Fix reverted on top of the committed change, mutation confirmed on disk in both directions (new branch absent, old branch present) before measuring:

Test Files  1 failed | 4 passed (5)
     Tests  4 failed | 71 passed (75)
  × get() returns the package from `[rows, fields]` tuple (mysql2)
  × list() returns exactly one package from `[rows, fields]` tuple (mysql2)
  × all three dialects agree — same row in, same answer out
  × `[[], fields]` (mysql2, zero rows) answers no-rows without throwing

Exactly the mysql2 legs fail; the bare-array and pg legs stay green, so the pin is specific rather than brittle. Restored and re-confirmed clean afterwards. No rebuild was needed for this ablation: the suite imports ./index.js, a same-package relative specifier vitest resolves to source, so no dist/ is involved.

Gates

Union derived on the final commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, no hand-supplied paths — provenance line confirmed the answer came from this repo's tree.

All 12 path-matched families green, plus the convention-triggered set for "adds or edits a test file": check:query-options-erasure, check:type-check-coverage, check:engine-double-contract, check:cross-package-test-inputs, check:where-matcher, and check:nul-bytes. check:type-check-debt --re-measure initially refused with PREREQUISITE NOT MET (unbuilt closure — recorded as NOT MEASURED, not as a pass); the full workspace closure was built (turbo run build, 70/70) and it was re-run to a real verdict.

pnpm --filter @objectstack/service-package test75 passed (5 files); typecheck → clean.


⚠️ The issue body contains an unreplaced PR #REPLACE_PR placeholder — a filing defect in the card, not addressed here and not investigated.

Generated by Claude Code


Generated by Claude Code

…lizeRows

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".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 21756b3254ed61a6c38baa46642bd003952194b4packageMentionDocs.

Which tree this was computed on

This run read content/docs from ad96c87179a4a13e040e615158e93101a87859d8 — the merge of head 769ff7b83ce0d0d61722216292e726c61d85a679 into base 21756b3254ed61a6c38baa46642bd003952194b4, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ad96c87179a4a13e040e615158e93101a87859d8 && git checkout ad96c87179a4a13e040e615158e93101a87859d8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 21756b3254ed61a6c38baa46642bd003952194b4 769ff7b83ce0d0d61722216292e726c61d85a679 && git checkout -B drift-repro 21756b3254ed61a6c38baa46642bd003952194b4 && git merge --no-ff 769ff7b83ce0d0d61722216292e726c61d85a679

node scripts/docs-audit/affected-docs.mjs --json 21756b3254ed61a6c38baa46642bd003952194b4

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

service-package's local normalizeRows never unwraps the mysql2 [rows, fields] tuple — a populated result reads as "not installed"

2 participants