From d2bf8aa6307266f0faea7bfdce1398cc157b42be Mon Sep 17 00:00:00 2001 From: Developer Date: Sat, 9 May 2026 16:07:23 -0400 Subject: [PATCH] fix(session): tolerate pre-#24512 model JSON shape in fromRow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #26435. Sessions created before #24512 (commit a3bc5d35b, 2026-05-02) wrote the `model` column as { providerID, modelID }. That PR renamed the shape to { id, providerID, variant } and updated `fromRow` to read row.model.id, but only updated the Drizzle \$type<>() annotation — the on-disk JSON of existing rows was not migrated. fromRow now crashes on those legacy rows with 'Expected string, got undefined', killing the whole session list (no per-row containment). Fall back to row.model.modelID when row.model.id is absent. Newly written rows still use the new shape; this only affects reads of legacy data. --- packages/opencode/src/session/session.ts | 4 +- .../session-legacy-model-shape.test.ts | 82 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 packages/opencode/test/session/session-legacy-model-shape.test.ts diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index 5c938ff693..bab1fb94d6 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -81,7 +81,9 @@ export function fromRow(row: SessionRow): Info { agent: row.agent ?? undefined, model: row.model ? { - id: ModelID.make(row.model.id), + // Pre-#24512 rows store `modelID` instead of `id`; fall back so + // sessions created before 2026-05-02 don't crash `fromRow`. + id: ModelID.make(row.model.id ?? (row.model as { modelID?: string }).modelID ?? ""), providerID: ProviderID.make(row.model.providerID), variant: row.model.variant, } diff --git a/packages/opencode/test/session/session-legacy-model-shape.test.ts b/packages/opencode/test/session/session-legacy-model-shape.test.ts new file mode 100644 index 0000000000..86a932f8ac --- /dev/null +++ b/packages/opencode/test/session/session-legacy-model-shape.test.ts @@ -0,0 +1,82 @@ +/** + * Reproducer for #26435 — `session list` crashes when the DB contains + * a legacy-shape `model` column. + * + * On 2026-05-02 #24512 renamed the JSON shape stored in `session.model` + * from `{ providerID, modelID }` to `{ id, providerID, variant }`. + * The Drizzle column type was updated but no data migration rewrote + * existing rows. `fromRow` now reads `row.model.id`; on legacy rows + * that field is undefined and `ModelID.make(undefined)` throws, + * killing the entire list (no per-row containment). + */ +import { afterEach, describe, expect } from "bun:test" +import { Effect, Layer } from "effect" +import { eq } from "drizzle-orm" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { Database } from "@/storage/db" +import { Session as SessionNs } from "@/session/session" +import { SessionTable } from "@/session/session.sql" +import { WithInstance } from "@/project/with-instance" +import * as Log from "@opencode-ai/core/util/log" +import { disposeAllInstances, tmpdir } from "../fixture/fixture" +import { testEffect } from "../lib/effect" + +void Log.init({ print: false }) + +const it = testEffect(Layer.mergeAll(SessionNs.defaultLayer, CrossSpawnSpawner.defaultLayer)) + +afterEach(async () => { + await disposeAllInstances() +}) + +describe("session list with legacy model shape (#26435)", () => { + it.live("does not crash when a row has the pre-#24512 {providerID, modelID} shape", () => + Effect.gen(function* () { + const tmp = yield* Effect.acquireRelease( + Effect.promise(() => tmpdir({ git: true, config: { formatter: false, lsp: false } })), + (t) => Effect.promise(() => t[Symbol.asyncDispose]()), + ) + yield* Effect.promise(() => + WithInstance.provide({ + directory: tmp.path, + fn: async () => { + const svc = SessionNs.Service + // Create two valid sessions so we can prove the list still + // returns the good ones after the legacy row is patched. + const legacy = await Effect.runPromise( + Effect.provide(svc.use((s) => s.create({ title: "legacy" })), SessionNs.defaultLayer), + ) + const ok = await Effect.runPromise( + Effect.provide(svc.use((s) => s.create({ title: "ok" })), SessionNs.defaultLayer), + ) + + // Replace the legacy row's model JSON with the pre-#24512 + // shape that's still on disk for users who upgraded. + Database.use((db) => + db + .update(SessionTable) + .set({ model: { providerID: "opencode", modelID: "big-pickle" } as any }) + .where(eq(SessionTable.id, legacy.id)) + .run(), + ) + + // Pre-fix this throws inside `fromRow` and the whole list + // call rejects, hiding `ok` along with `legacy`. + const list = await Effect.runPromise( + Effect.provide(svc.use((s) => s.list()), SessionNs.defaultLayer), + ) + const ids = list.map((s) => s.id) + expect(ids).toContain(ok.id) + expect(ids).toContain(legacy.id) + + const legacyEntry = list.find((s) => s.id === legacy.id)! + // Post-fix the legacy row resolves cleanly with the + // recovered modelID surfaced as `id`. + expect(legacyEntry.model?.id).toBe("big-pickle") + expect(legacyEntry.model?.providerID).toBe("opencode") + }, + }), + ) + }), + ) +})