From 9c81c4bc7750091e578480f96a79f3d79d7f66e3 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Sun, 24 May 2026 20:43:53 +0100 Subject: [PATCH] fix(pds): accept create+delete on the same rkey in applyWrites Drop the pre-flight intra-batch rkey-collision check so create+delete on the same rkey atomically resolves to a no-op, matching the reference PDS. Two creates for the same rkey now surface as 409 RecordAlreadyExists from the repo layer instead of a 400. --- .changeset/applywrites-same-rkey-batch.md | 5 ++ packages/pds/src/xrpc/repo.ts | 18 ------- packages/pds/test/xrpc.test.ts | 58 +++++++++++++++++++++-- 3 files changed, 59 insertions(+), 22 deletions(-) create mode 100644 .changeset/applywrites-same-rkey-batch.md diff --git a/.changeset/applywrites-same-rkey-batch.md b/.changeset/applywrites-same-rkey-batch.md new file mode 100644 index 00000000..c07dc0b2 --- /dev/null +++ b/.changeset/applywrites-same-rkey-batch.md @@ -0,0 +1,5 @@ +--- +"@getcirrus/pds": patch +--- + +`com.atproto.repo.applyWrites` now accepts batches that touch the same rkey more than once, matching the reference PDS. The common case is a create followed by a delete on the same rkey within one batch (an atomic no-op pattern several clients rely on); previously Cirrus rejected this with `400 InvalidRequest: duplicate rkey in batch`. Two creates on the same rkey still fail, but now as `409 RecordAlreadyExists` from the repo layer rather than a pre-flight 400. diff --git a/packages/pds/src/xrpc/repo.ts b/packages/pds/src/xrpc/repo.ts index f1c77479..abd239ae 100644 --- a/packages/pds/src/xrpc/repo.ts +++ b/packages/pds/src/xrpc/repo.ts @@ -513,10 +513,6 @@ export async function applyWrites( value?: unknown; validationStatus?: ValidationStatus; }> = []; - // Detect intra-batch rkey duplicates here (client error → 400) so they - // don't surface from the DO as 409 RecordAlreadyExists, which is reserved - // for collisions against existing repo state. - const seenRkeys = new Set(); for (let i = 0; i < writes.length; i++) { const write = writes[i]; @@ -567,20 +563,6 @@ export async function applyWrites( } } - if (typeof write.rkey === "string") { - const composite = `${write.collection}/${write.rkey}`; - if (seenRkeys.has(composite)) { - return c.json( - { - error: "InvalidRequest", - message: `Write ${i}: duplicate rkey in batch (${composite})`, - }, - 400, - ); - } - seenRkeys.add(composite); - } - if (action === "delete") { preparedWrites.push({ $type: write.$type, diff --git a/packages/pds/test/xrpc.test.ts b/packages/pds/test/xrpc.test.ts index 6b5f1ccb..4403a69f 100644 --- a/packages/pds/test/xrpc.test.ts +++ b/packages/pds/test/xrpc.test.ts @@ -1227,7 +1227,58 @@ describe("XRPC Endpoints", () => { expect(data.results[1].validationStatus).toBe("unknown"); }); - it("rejects intra-batch duplicate rkey as 400 InvalidRequest (not 409)", async () => { + it("accepts create+delete on the same rkey atomically, leaving no record", async () => { + const rkey = genTid(); + const response = await worker.fetch( + new Request("http://pds.test/xrpc/com.atproto.repo.applyWrites", { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${env.AUTH_TOKEN}`, + }, + body: JSON.stringify({ + repo: env.DID, + writes: [ + { + $type: "com.atproto.repo.applyWrites#create", + collection: "app.bsky.feed.post", + rkey, + value: { + $type: "app.bsky.feed.post", + text: "ephemeral", + createdAt: new Date().toISOString(), + }, + }, + { + $type: "com.atproto.repo.applyWrites#delete", + collection: "app.bsky.feed.post", + rkey, + }, + ], + }), + }), + env, + ); + expect(response.status).toBe(200); + const data = (await response.json()) as any; + expect(data.results).toHaveLength(2); + expect(data.results[0].$type).toBe( + "com.atproto.repo.applyWrites#createResult", + ); + expect(data.results[1].$type).toBe( + "com.atproto.repo.applyWrites#deleteResult", + ); + + const getResponse = await worker.fetch( + new Request( + `http://pds.test/xrpc/com.atproto.repo.getRecord?repo=${env.DID}&collection=app.bsky.feed.post&rkey=${rkey}`, + ), + env, + ); + expect(getResponse.status).toBe(404); + }); + + it("rejects two creates for the same rkey as 409 RecordAlreadyExists", async () => { const dupRkey = genTid(); const response = await worker.fetch( new Request("http://pds.test/xrpc/com.atproto.repo.applyWrites", { @@ -1264,10 +1315,9 @@ describe("XRPC Endpoints", () => { }), env, ); - expect(response.status).toBe(400); + expect(response.status).toBe(409); const data = (await response.json()) as any; - expect(data.error).toBe("InvalidRequest"); - expect(data.message).toMatch(/duplicate rkey in batch/i); + expect(data.error).toBe("RecordAlreadyExists"); }); it("rejects validate=true with unknown collection in applyWrites as 400", async () => {