Repository navigation
fix(core): surface message ID conflicts on session import - #54543
reynold-hu wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
|
Thanks for picking this up, @reynold-hu, and for the clear write-up and test! I think this duplicates #54353, which is still open. It fixes #54336 the same way: it checks the imported message IDs against If you think the more specific 409 message is worth having, a review or comment on #54353 would be the best place to suggest it, and testing it there would help too. |
|
Agreed — closing in favor of #54353. I left the conflict-message note and an extra assertion suggestion there. |
Issue for this PR
Closes #54336
Type of change
What does this PR do?
Importing a session whose message IDs already exist died on the
session_messageUNIQUE constraint inside the import publication and surfaced as HTTP 500.SessionTransfer.importnow checks the imported message IDs before publishing and fails withMessageConflictError, which the import handler maps to a 409ConflictErrorwithSession messages already exist: <sessionID>.I used a distinct error instead of reusing
ImportConflictErrorso the 409 message stays accurate: the destination session does not exist here, only its messages do. Session-ID conflicts keep working exactly as before.How did you verify your code works?
packages/server/test/session-import.test.ts: new case imports a second session with a colliding message ID, expects a 409 and asserts the destination session is not created.bun test test/session-import.test.ts→ 4 pass.bun test test/session-create.test.ts(existing SessionTransfer behavior) → 44 pass.bun typecheckinpackages/coreandpackages/serverboth pass.Screenshots / recordings
Not a UI change.
Checklist