fix(core): migrate V1 messages that predate required metadata - #53996

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

1. Open an issue describing the bug/feature (if one doesn't exist)
2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

The following comment was made by an LLM, it may be inaccurate:

Noting the automated pr-standards:issue result: the PR body already says Closes #53971, but GitHub only resolves closing keywords into the linked-issue field when the base is the repository's default branch. This change cannot target that branch — it touches packages/core/src/database/v1-migration.bun.ts, which only exists on v2, and the issue reproduces there — so the linked-issue field stays empty and this check cannot pass by design, same as on my other v2-based PRs (e.g. #53983, #53945). The reference is in the body; no further action from me here.

I'm still reviewing this pull request; I'll answer here when that's done.

Thanks for explaining. My review didn't consider that check, and I didn't change it. The body links #53971, and the review was against v2, where I confirmed the fix. I have no further requests on the code.

Cross-posting the point @opencode-agent asked me to bring here (from #55991, which is being closed as a duplicate of #53971).

Independent confirmation from a second database of the row shape this PR targets: 9,330 assistant rows across 127 sessions have agent IS NULL, and all 9,330 carry mode (build 8121 / plan 665 / explore 514 / general 30). No assistant row is missing parentID, and no user row is missing agent/model — so the parentID and user-model derivations are never exercised on this install. The skip log and the DB-derived count also matched exactly (9,330 / 9,330), i.e. the log was complete here unlike in #52226.

The gap this PR leaves open, and the reason I'm posting here: the migration returns early whenever kv['migration.v1-v2'] is {"phase":"completed"}, and this PR only changes how rows are converted. On our install, updating 2.0.29 → 2.0.30 re-projected nothing, so the fix alone will not repair a database that has already migrated — those users still need the manual reset, with the warning that a re-run wipes V2 history appended after the first run (#52226).

Would it be feasible to also cover already-migrated installs — for example, re-run when the marker is completed and rows with a missing agent are still present, or repair only the affected sessions — or at minimum state the required reset in the release notes / migration docs? A one-line note would save every affected user this same research.

I can test a build against this database (318 V1-origin sessions, 9,330 affected rows, re-run currently lossless here: 257 V2-only rows in V1-origin sessions, none of them real conversation content) and report the before/after numbers back here.

Thanks for the cross-post, and for the numbers. The independent count is useful, and the mode breakdown matters specifically for this fix.

On the gap you found: confirmed, and it cannot be closed by a re-run

The migration short-circuits on the completion marker:

• packages/core/src/database/v1-migration.bun.ts:540 — run() returns { status: "completed" } when kv['migration.v1-v2'].phase === "completed", before any row is read. So none of this PR executes on an install that has already migrated, exactly as you describe.

The obvious trigger you suggest — re-run when the marker is completed and rows with a missing agent are still present — would be worse than the bug, because of what the per-session import does:

• v1-migration.bun.ts:641 — DELETE FROM session_message WHERE session_id = ?, and then the whole transformed V1 projection is re-inserted for that session.

So a re-run deletes every already-projected message for the session, including anything V2 appended after the first run, and puts back the same partial set: the V1 source rows have not changed, so the same rows drop again — which is what #53971 already reports. That is the mechanism behind the warning in #52226, and it is why I am not making the marker conditional in this PR. The failure mode is data loss and the blast radius is every affected install on the next boot.

What a safe repair would have to look like

It cannot reuse the migration's per-session replace. It has to be additive per message: transform the session as today, then insert only the projected messages whose ids are absent from session_message, and never delete. That is a second code path with its own watermark and event_sequence bookkeeping, and it changes what the completed marker promises — the migration would have to look at the data on every start. That is a design call about data-migration semantics rather than a fix, so I would rather do it as its own PR than grow this one's blast radius. If you or a maintainer want it, say so and I will open it; otherwise I am happy to open an issue so the direction gets set before any code is written.

What I will not do

Add "delete kv.migration.v1-v2" to the migration docs as a remedy. It is the recovery that #52226 needed, but at line 641 it also discards V2-appended history, and documenting it without that caveat costs affected users more than it saves them.

What would help this PR right now

Your offer to test a build is the most useful thing available. This PR resolves a missing agent from the row's own value, then mode, then the reply that handled the turn, then the session row, then the built-in default — and your database is the case the mode step exists for: all 9,330 of its rows carry mode (build 8121 / plan 665 / explore 514 / general 30).

A before/after count from a scratch copy of that database would be worth more than anything I can run here, and it would pin both halves at once:

1. Fresh marker — copy the database, delete kv.migration.v1-v2, start a build with this PR, and report how many of the 9,330 rows appear in session_message. I expect all 9,330, with the agent values following the mode breakdown above.
2. Existing marker — the same copy without touching the marker, to show that nothing changes. That is the gap in this PR, measured rather than reasoned, and it is the number that would justify the additive repair above.

Both runs also answer the question in your last paragraph: whether the repair would need to be scoped per session, or whether re-projecting every legacy session on first start after the upgrade is cheap enough at your size (336 sessions / 9,330 rows).

Closing this to stay within GitHub's 3-open-pull-request limit for contributors without write access, which currently blocks every new pull request here. The branch remains on my fork, so I can reopen it on request.

Independent before/after run on a real affected database (Windows, opencode 2.1.2, opencode.db ~1.6 GB, 318 V1 sessions). It confirms both halves: the mode derivation is exactly what this database needed, and the marker short-circuit is the remaining blocker.

State before

• message (V1): 32,573 rows across 318 sessions
• session_message (V2): 31,395 rows
• assistant rows with agent IS NULL: 9,330 — all 9,330 carry mode (build 8121 / plan 665 / explore 514 / general 30)
• user rows without model: 0; assistant rows without parentID: 0 — so the parentID and user-model steps are never exercised on this install
• kv['migration.v1-v2'] = {"phase":"completed"}

Method (the manual path, because the PR alone cannot re-run)

1. backup of opencode.db + -wal + -shm
2. set agent from mode on the 9,330 rows
3. DELETE FROM kv WHERE key='migration.v1-v2'
4. restart opencode 2.1.2

State after

• session_message: 31,395 → 40,847 (+9,452)
• agent IS NULL: 9,330 → 0
• marker back to completed
• type breakdown: 36,040 assistant / 3,680 user / 502 system / 370 idle / 205 synthetic / 37 compaction / 12 agent-switched / 3 location-switched
• sessions with no assistant row at all: 127 → 1

Sample session ses_29790b6c9ffeu67nLM0EU5zTdT — the one I used to report the bug, 55 entries at the time with only prompts and synthetic notes. After the re-run: 747 assistant + 50 user + 5 compaction + 5 synthetic, agent: "build", model deepseek/deepseek-chat.

What the destructive re-run cost, measured

• 257 V2-only rows in 185 V1-origin sessions, all of them synthetic "The available tools have changed…" notices; no real conversation content. They are regenerated automatically — the newest row of the sample session is exactly such a notice again.
• 0 V1-origin sessions had been active since the first migration (max(time_updated) on session equals the migration date), so no real V2-appended history was at risk here. That is the check anyone has to run before using this path.

So for already-migrated installs the gap you described is the whole problem: without the reset nothing runs, and with the reset the per-session DELETE is what forces the "is anything V2-only in here?" audit. An additive repair (insert projected messages whose ids are absent, never delete) would remove that audit entirely.

I kept the pre-fix backup and can run a build against it if that PR appears. Acceptance numbers on that copy: agent IS NULL → 0, session_message → ~40,800, and zero deleted rows.

Happy to see this reopened if the PR limit allows.