fix(session): tolerate pre-#24512 model JSON shape in fromRow

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.
This commit is contained in:
Developer
2026-05-09 16:07:23 -04:00
parent 5e49029e70
commit d2bf8aa630
2 changed files with 85 additions and 1 deletions

View File

@@ -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,
}

View File

@@ -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")
},
}),
)
}),
)
})