Skip to content

Commit d29594d

Browse files
eatmoreduckeatmoreduck
committed
refactor(server): dedicated MessageConflictError for message-ID import conflicts
Per review: a message-ID conflict never created the destination session, so reusing ImportConflictError (with a reason discriminant) made every consumer unwrap a union that only this path produced. MessageConflictError now carries the sessionID directly and maps to the 409 'Session messages already exist: <id>' body; ImportConflictError is back to meaning 'the session row exists'. Both 409 bodies and the no-destination-session assertion are covered by the import tests.
1 parent f2e8224 commit d29594d

3 files changed

Lines changed: 24 additions & 13 deletions

File tree

‎packages/core/src/session/transfer.ts‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,13 @@ export type Data = SessionTransfer.Data
2727

2828
export class ImportConflictError extends Schema.TaggedError<ImportConflictError>()(
2929
"SessionTransfer.ImportConflictError",
30-
// "session": the session row already exists; "messages": only some message IDs exist
31-
// under another session, so the session itself was never created.
32-
{ sessionID: Session.ID, reason: Schema.Literals(["session", "messages"]) },
30+
{ sessionID: Session.ID },
31+
) {}
32+
33+
/** Some imported message IDs are already taken by another session; the destination session row was never created. */
34+
export class MessageConflictError extends Schema.TaggedError<MessageConflictError>()(
35+
"SessionTransfer.MessageConflictError",
36+
{ sessionID: Session.ID },
3337
) {}
3438

3539
export interface Interface {
@@ -40,7 +44,7 @@ export interface Interface {
4044
readonly import: (input: {
4145
data: Data
4246
location: Location.Ref
43-
}) => Effect.Effect<Session.Info, ImportConflictError | Session.NotFoundError>
47+
}) => Effect.Effect<Session.Info, ImportConflictError | MessageConflictError | Session.NotFoundError>
4448
}
4549

4650
export class Service extends Context.Service<Service, Interface>()("@opencode/SessionTransfer") {}
@@ -71,7 +75,7 @@ const layer = Layer.effect(
7175
.where(eq(SessionTable.id, sessionID))
7276
.get()
7377
.pipe(Effect.orDie)
74-
if (recorded) return yield* new ImportConflictError({ sessionID, reason: "session" })
78+
if (recorded) return yield* new ImportConflictError({ sessionID })
7579
const settled = input.data.messages.filter(isSettled)
7680
for (const batch of batches(settled.map((message) => message.id))) {
7781
const taken = yield* db
@@ -80,7 +84,7 @@ const layer = Layer.effect(
8084
.where(inArray(SessionMessageTable.id, batch))
8185
.get()
8286
.pipe(Effect.orDie)
83-
if (taken) return yield* new ImportConflictError({ sessionID, reason: "messages" })
87+
if (taken) return yield* new MessageConflictError({ sessionID })
8488
}
8589
if (input.data.info.parentID) yield* sessions.get(input.data.info.parentID)
8690
const project = yield* projects.resolve(input.location.directory)
@@ -158,7 +162,7 @@ const layer = Layer.effect(
158162
.pipe(
159163
Effect.catchDefect((defect) =>
160164
defect instanceof SessionProjector.SessionAlreadyProjected
161-
? Effect.fail(new ImportConflictError({ sessionID, reason: "session" }))
165+
? Effect.fail(new ImportConflictError({ sessionID }))
162166
: Effect.die(defect),
163167
),
164168
)

‎packages/server/src/handlers/session.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,17 @@ export const SessionHandler = HttpApiBuilder.group(Api, "server.session", (handl
124124
"SessionTransfer.ImportConflictError",
125125
(error) =>
126126
new ConflictError({
127-
// A message-ID conflict never created the destination session, so
128-
// claiming "Session already exists" would be wrong for it.
129-
message:
130-
error.reason === "messages"
131-
? `Session messages already exist: ${error.sessionID}`
132-
: `Session already exists: ${error.sessionID}`,
127+
message: `Session already exists: ${error.sessionID}`,
128+
resource: error.sessionID,
129+
}),
130+
),
131+
Effect.catchTag(
132+
"SessionTransfer.MessageConflictError",
133+
// Only the message IDs exist (under another session); the destination
134+
// session row was never created.
135+
(error) =>
136+
new ConflictError({
137+
message: `Session messages already exist: ${error.sessionID}`,
133138
resource: error.sessionID,
134139
}),
135140
),

‎packages/server/test/session-import.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ it.live("maps imported message-ID collisions to 409 without creating the session
154154
resource: second,
155155
message: `Session messages already exist: ${second}`,
156156
})
157+
// The rejected import must not have created the destination session (the
158+
// failure mode when the constraint fires mid-publication).
157159
expect(yield* request(`/api/session/${second}`, undefined, 404)).toMatchObject({
158160
_tag: "SessionNotFoundError",
159161
sessionID: second,

0 commit comments

Comments
 (0)