Repository navigation
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works. I ran the #61947 reproduction script from source on the base and on this branch. On the base, run -s <id> creates a session_v2 row that reuses the ID of the session that exists only in V1, in the other project's directory, and session export returns that empty session. With this change, run -s <id> fails, no V2 row is written and the V1 row is untouched. The new test fails on the base and passes here. session-create, v1-migration, session-list and session-store tests pass. The package's typecheck script fails before checking anything (Script not found "check"), so I ran its typecheck:tsc script instead, which is clean.
Public API: no fields or routes change. POST /api/session with an explicit ID still held by a V1 session now returns 404 SessionNotFoundError instead of 200. This is backwards compatible for callers that already handle a 404 from create.
The check is in the right layer (server-side create, so every client is covered). My one comment is about what users see when this happens (inline). It's worth fixing but doesn't block a merge.
Related: #50130
| .pipe(Effect.orDie) | ||
| if (legacyTable) { | ||
| const legacy = yield* db.get(sql`SELECT id FROM session WHERE id = ${sessionID}`).pipe(Effect.orDie) | ||
| if (legacy) return yield* new NotFoundError({ sessionID }) |
There was a problem hiding this comment.
Returning a plain NotFoundError here means a user running opencode -s <id> only sees Session not found: ses_…, which is the same message as for an ID that doesn't exist. Their session does exist, but only in V1 data that was never migrated, so the message points them the wrong way. Could this case get its own error, or at least a message such as "session exists in V1 data but was not migrated", so the CLI and other clients can tell users what actually happened?
There was a problem hiding this comment.
The earlier point about the error message is addressed. run -s <id> on an ID held only by an unmigrated V1 session now fails with Session exists in V1 data but was not migrated: ses_…, and still writes no V2 row (re-checked with the #61947 reproduction script from source). session-create and handler-policy tests pass. The packages' typecheck scripts don't run in this checkout (Script not found "check"), so I ran their typecheck:tsc scripts for core and server instead; both are clean.
Public API: POST /api/session keeps the 404 SessionNotFoundError shape; only the message differs for this case. The plugin API's session.create can now also fail with the new Session.UnmigratedError, but only for explicit IDs still held by a V1 session, which previously created a duplicate session without any error. Backwards compatible otherwise.
Nothing new to raise.
Issue for this PR
Closes #61947
Type of change
What does this PR do?
After the V1 to V2 update, some V1 sessions were never migrated (#50130).
Continuing one of those by ID missed the V2 lookup, and session creation
then reused the requested ID for a new empty session in the current
project. From then on the ID resolved to that empty session and the
original, history and all, was unreachable.
Session.createnow checks the legacy V1sessiontable when it isasked to create with an explicit ID that has no V2 row. If a V1 session
still holds that ID, create fails with
SessionNotFoundErrorinstead ofinserting the shadow row. Nothing is imported here; importing
unmigrated sessions is #54272's scope. Databases without a legacy
sessiontable skip the check.How did you verify your code works?
In packages/core (
bun run test):test/session-create.test.ts: creating with an ID heldby a V1-only row fails with NotFoundError and inserts no V2 row (this
test fails on unpatched code); creating with a fresh explicit ID and
with no ID still works when a V1 table is present.
suites: 59 pass.
bun typecheckclean.End to end with a seeded database (V1 row present, migration marked
completed):
POST /api/sessionwith that ID returned 200 and createdthe shadow session before this change; with it, the same request
returns 404 SessionNotFoundError, no
session_v2row is written, andthe V1 row is untouched. Fresh-ID and no-ID creates still return 200.
Checklist
Added by the opencode bot (AI-generated).
Related: #50130
Public API: plugin session.create can fail with new Session.UnmigratedError; POST /api/session returns 404 with a distinct message for unmigrated V1 IDs (backwards compatible)