diff --git a/plugin-kit/package.json b/plugin-kit/package.json index c53efb8e..c4cf6fad 100644 --- a/plugin-kit/package.json +++ b/plugin-kit/package.json @@ -1,6 +1,6 @@ { "name": "@punktfunk/plugin-kit", - "version": "0.3.0", + "version": "0.3.1", "description": "Effect-based framework for punktfunk plugins: lifecycle runtime, config/state, sync engine, UI serving, CLI scaffold, and browser helpers.", "type": "module", "license": "MIT OR Apache-2.0", diff --git a/plugin-kit/src/cli.ts b/plugin-kit/src/cli.ts index fcad0385..2535a5e6 100644 --- a/plugin-kit/src/cli.ts +++ b/plugin-kit/src/cli.ts @@ -73,7 +73,13 @@ export const runPluginCli = async (opts: { const rt = ManagedRuntime.make(Layer.provideMerge(opts.def.layer, base)); try { await rt.runPromise(Effect.scoped(command.run(rest))); - process.exitCode = 0; + // Do NOT clobber a non-zero code the command set deliberately. `parity --compare` reports a + // mismatch by setting `process.exitCode = 1` and then RETURNING normally — a red parity is a + // finished comparison, not a crashed command. Assigning 0 here unconditionally overwrote it, + // so the one verb documented as a release gate ("exits non-zero on any difference", "do not + // publish a version whose parity run is red") always exited 0, and any scripted use of it + // passed. MEASURED against a live host on 2026-08-06: `parity FAILED — 1 missing`, exit 0. + process.exitCode ??= 0; } catch (e) { const hint = e instanceof HostRequestError diff --git a/plugin-kit/src/library/parsers/sqlite.ts b/plugin-kit/src/library/parsers/sqlite.ts index 6831796b..df08298b 100644 --- a/plugin-kit/src/library/parsers/sqlite.ts +++ b/plugin-kit/src/library/parsers/sqlite.ts @@ -4,9 +4,22 @@ // opened it read-write could take a write lock, create `-wal`/`-shm` sidecars next to it, or (worst // case) be blamed for a corrupted library. `immutable=1` promises the file will not change while // open, which makes Bun skip locking entirely — the strictest possible "look, don't touch". -import { Database } from "bun:sqlite"; +import { constants, Database } from "bun:sqlite"; import { isFile } from "./fs.js"; +/** + * READONLY | URI, passed as raw open flags. + * + * The `{ readonly: true }` options object does NOT enable SQLite's URI filename parsing, so a + * `file:…?immutable=1` name is taken literally, no such file exists, and the open throws + * `SQLiteError: unable to open database file`. Every caller here degrades an open failure to + * "launcher not installed", so that turned into a silent, total "0 games" on every box — see the + * regression test in test/library-parsers.test.ts. SQLITE_OPEN_URI is what makes the query string + * mean anything. (`{ readonly: true, uri: true }` is not a thing — measured on bun 1.3.14.) + */ +const OPEN_READONLY_URI = + constants.SQLITE_OPEN_READONLY | constants.SQLITE_OPEN_URI; + export interface ReadOnlyDb { /** Run a query and return its rows. Returns `[]` rather than throwing on a bad query. */ readonly query: >( @@ -29,7 +42,9 @@ export const openReadOnly = (file: string): ReadOnlyDb | undefined => { // `readonly` alone still takes locks and can spawn WAL sidecars; `immutable=1` is what makes // this a pure read. It is safe here precisely because a scan is a point-in-time snapshot — // if the launcher writes mid-scan we simply pick it up on the next sync. - db = new Database(`file:${encodeURI(file)}?immutable=1`, { readonly: true }); + // The flags (not `{ readonly: true }`) are load-bearing: without SQLITE_OPEN_URI the name + // below is not parsed as a URI and the open always fails. See OPEN_READONLY_URI. + db = new Database(`file:${encodeURI(file)}?immutable=1`, OPEN_READONLY_URI); } catch { return undefined; } diff --git a/plugin-kit/test/library-parsers.test.ts b/plugin-kit/test/library-parsers.test.ts index 8c038cd3..ae290c7d 100644 --- a/plugin-kit/test/library-parsers.test.ts +++ b/plugin-kit/test/library-parsers.test.ts @@ -6,6 +6,7 @@ // per-plugin parity harness (design M5) then checks the whole pipeline against a live host, but // these catch a drift long before that. import { describe, expect, test } from "bun:test"; +import { Database } from "bun:sqlite"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -17,6 +18,8 @@ import { fileUrl, gridFilenames, isSteamTool, + withReadOnlyDb, + openReadOnly, parseAppManifest, parseRegQuery, parseShortcuts, @@ -287,3 +290,79 @@ describe("reg.exe output", () => { ]); }); }); + +// The read-only SQLite helper, against a REAL database file. +// +// This exists because its absence shipped a total failure. `openReadOnly` built a +// `file:…?immutable=1` URI but opened it with `{ readonly: true }`, which does not enable SQLite's +// URI filename parsing — so the name was taken literally, the open threw, and `openReadOnly` +// returned `undefined`. Every caller reads that as "this launcher isn't installed", and +// `withReadOnlyDb(...) ?? []` turns it into an empty library. The lutris plugin therefore reported +// "0 games" on every box, forever, while `detect` still said "present" (it only stats the file) — +// and the only thing that caught it was a hand-run parity gate against a live host. +// +// So: assert the helper can actually READ, not merely that it returns something. +describe("openReadOnly", () => { + const withDb = (use: (file: string) => T): T => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "pf-kit-sqlite-")); + const file = path.join(dir, "pga.db"); + const seed = new Database(file); + seed.run("CREATE TABLE games (id INTEGER PRIMARY KEY, name TEXT, installed INT)"); + seed.run("INSERT INTO games (id, name, installed) VALUES (1, 'Ubisoft Connect', 1)"); + seed.close(); + try { + return use(file); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }; + + test("opens a real database and returns its rows", () => { + withDb((file) => { + const db = openReadOnly(file); + expect(db).toBeDefined(); + expect(db?.query("SELECT id, name FROM games WHERE installed = 1")).toEqual([ + { id: 1, name: "Ubisoft Connect" }, + ]); + db?.close(); + }); + }); + + // A path with a space is the realistic URI-encoding case (Flatpak roots, "Program Files"). + test("opens a path that needs URI escaping", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "pf kit sqlite ")); + const file = path.join(dir, "pga.db"); + const seed = new Database(file); + seed.run("CREATE TABLE games (id INTEGER PRIMARY KEY)"); + seed.run("INSERT INTO games (id) VALUES (7)"); + seed.close(); + try { + expect(openReadOnly(file)?.query("SELECT id FROM games")).toEqual([{ id: 7 }]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test("withReadOnlyDb reads, then closes", () => { + withDb((file) => { + expect(withReadOnlyDb(file, (h) => h.query("SELECT name FROM games"))).toEqual([ + { name: "Ubisoft Connect" }, + ]); + }); + }); + + // The "not installed" contract — an absent file is `undefined`, never a throw. + test("absent file is undefined, not an error", () => { + expect(openReadOnly(path.join(os.tmpdir(), "pf-kit-nope", "pga.db"))).toBeUndefined(); + expect(withReadOnlyDb(path.join(os.tmpdir(), "pf-kit-nope", "pga.db"), () => 1)).toBeUndefined(); + }); + + // Schema drift degrades to no rows rather than taking the plugin down. + test("a bad query returns [] rather than throwing", () => { + withDb((file) => { + const db = openReadOnly(file); + expect(db?.query("SELECT missing_column FROM games")).toEqual([]); + db?.close(); + }); + }); +});