From f8f3fd469c7d4ecfd587e394d88ee2b19f6ef3ff Mon Sep 17 00:00:00 2001 From: Kieran Klukas Date: Tue, 18 Aug 2026 15:40:46 -0400 Subject: [PATCH] fix: migrate the picker's new columns onto an existing database CREATE TABLE IF NOT EXISTS leaves a table exactly as it found it, so display_name and sort_order never reached a database that already had user_models: every request 500'd on the first read. Migrated, with a test that opens a database built to the old shape. --- src/store.ts | 10 ++++++++++ tests/picker.test.ts | 31 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/store.ts b/src/store.ts index 1707bb8..20ad963 100644 --- a/src/store.ts +++ b/src/store.ts @@ -627,6 +627,16 @@ export class Store { } catch { // column already exists } + // Migration: a picker began as a bare list of refs and grew a name and an + // order per person. CREATE TABLE IF NOT EXISTS leaves an existing table + // exactly as it found it, so the columns have to be added by hand. + for (const column of ["display_name TEXT", "sort_order INTEGER NOT NULL DEFAULT 0"]) { + try { + this.db.exec(`ALTER TABLE user_models ADD COLUMN ${column}`); + } catch { + // column already exists + } + } // Migration: room for the non-secret bits a provider needs alongside the // token (a ChatGPT account id, so far). try { diff --git a/tests/picker.test.ts b/tests/picker.test.ts index 99b91ff..93bf19e 100644 --- a/tests/picker.test.ts +++ b/tests/picker.test.ts @@ -1,3 +1,4 @@ +import { Database } from "bun:sqlite"; import { afterAll, beforeEach, expect, test } from "bun:test"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -150,3 +151,33 @@ test("your picker survives a restart", async () => { { ref: "acme/acme-1", displayName: "Persisted", sortOrder: 0 }, ]); }); + +test("a database from before the picker grew columns still opens", () => { + const tmp = mkdtempSync(join(tmpdir(), "kloe-migrate-")); + tmpDirs.push(tmp); + const dbPath = join(tmp, "old.db"); + + // The shape user_models had when it was a bare list of refs. CREATE TABLE IF + // NOT EXISTS leaves an existing table exactly as it found it, so a schema + // that grew columns only reaches an old database through a migration — and + // forgetting one is a 500 on the first request, not a failing test. + const old = new Database(dbPath); + old.exec( + `CREATE TABLE user_models ( + sub TEXT NOT NULL, model_ref TEXT NOT NULL, updated_at INTEGER NOT NULL, + PRIMARY KEY (sub, model_ref) + )`, + ); + old.exec( + "INSERT INTO user_models (sub, model_ref, updated_at) VALUES ('local', 'acme/acme-1', 1)", + ); + old.close(); + + const store = new Store(dbPath); + expect(store.listUserModels("local")).toEqual([ + { ref: "acme/acme-1", displayName: null, sortOrder: 0 }, + ]); + // …and it can be written to afterwards. + store.setUserModel("local", "acme/acme-1", { enabled: true, displayName: "Renamed" }); + expect(store.listUserModels("local")[0]?.displayName).toBe("Renamed"); +}); -- 2.51.2