Skip to content

Commit 36b79fd

Browse files
authored
fix(pds): accept create+delete on the same rkey in applyWrites (#176)
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.
1 parent b2de77a commit 36b79fd

3 files changed

Lines changed: 59 additions & 22 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@getcirrus/pds": patch
3+
---
4+
5+
`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.

packages/pds/src/xrpc/repo.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -513,10 +513,6 @@ export async function applyWrites(
513513
value?: unknown;
514514
validationStatus?: ValidationStatus;
515515
}> = [];
516-
// Detect intra-batch rkey duplicates here (client error → 400) so they
517-
// don't surface from the DO as 409 RecordAlreadyExists, which is reserved
518-
// for collisions against existing repo state.
519-
const seenRkeys = new Set<string>();
520516

521517
for (let i = 0; i < writes.length; i++) {
522518
const write = writes[i];
@@ -567,20 +563,6 @@ export async function applyWrites(
567563
}
568564
}
569565

570-
if (typeof write.rkey === "string") {
571-
const composite = `${write.collection}/${write.rkey}`;
572-
if (seenRkeys.has(composite)) {
573-
return c.json(
574-
{
575-
error: "InvalidRequest",
576-
message: `Write ${i}: duplicate rkey in batch (${composite})`,
577-
},
578-
400,
579-
);
580-
}
581-
seenRkeys.add(composite);
582-
}
583-
584566
if (action === "delete") {
585567
preparedWrites.push({
586568
$type: write.$type,

packages/pds/test/xrpc.test.ts

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,7 +1227,58 @@ describe("XRPC Endpoints", () => {
12271227
expect(data.results[1].validationStatus).toBe("unknown");
12281228
});
12291229

1230-
it("rejects intra-batch duplicate rkey as 400 InvalidRequest (not 409)", async () => {
1230+
it("accepts create+delete on the same rkey atomically, leaving no record", async () => {
1231+
const rkey = genTid();
1232+
const response = await worker.fetch(
1233+
new Request("http://pds.test/xrpc/com.atproto.repo.applyWrites", {
1234+
method: "POST",
1235+
headers: {
1236+
"Content-Type": "application/json",
1237+
Authorization: `Bearer ${env.AUTH_TOKEN}`,
1238+
},
1239+
body: JSON.stringify({
1240+
repo: env.DID,
1241+
writes: [
1242+
{
1243+
$type: "com.atproto.repo.applyWrites#create",
1244+
collection: "app.bsky.feed.post",
1245+
rkey,
1246+
value: {
1247+
$type: "app.bsky.feed.post",
1248+
text: "ephemeral",
1249+
createdAt: new Date().toISOString(),
1250+
},
1251+
},
1252+
{
1253+
$type: "com.atproto.repo.applyWrites#delete",
1254+
collection: "app.bsky.feed.post",
1255+
rkey,
1256+
},
1257+
],
1258+
}),
1259+
}),
1260+
env,
1261+
);
1262+
expect(response.status).toBe(200);
1263+
const data = (await response.json()) as any;
1264+
expect(data.results).toHaveLength(2);
1265+
expect(data.results[0].$type).toBe(
1266+
"com.atproto.repo.applyWrites#createResult",
1267+
);
1268+
expect(data.results[1].$type).toBe(
1269+
"com.atproto.repo.applyWrites#deleteResult",
1270+
);
1271+
1272+
const getResponse = await worker.fetch(
1273+
new Request(
1274+
`http://pds.test/xrpc/com.atproto.repo.getRecord?repo=${env.DID}&collection=app.bsky.feed.post&rkey=${rkey}`,
1275+
),
1276+
env,
1277+
);
1278+
expect(getResponse.status).toBe(404);
1279+
});
1280+
1281+
it("rejects two creates for the same rkey as 409 RecordAlreadyExists", async () => {
12311282
const dupRkey = genTid();
12321283
const response = await worker.fetch(
12331284
new Request("http://pds.test/xrpc/com.atproto.repo.applyWrites", {
@@ -1264,10 +1315,9 @@ describe("XRPC Endpoints", () => {
12641315
}),
12651316
env,
12661317
);
1267-
expect(response.status).toBe(400);
1318+
expect(response.status).toBe(409);
12681319
const data = (await response.json()) as any;
1269-
expect(data.error).toBe("InvalidRequest");
1270-
expect(data.message).toMatch(/duplicate rkey in batch/i);
1320+
expect(data.error).toBe("RecordAlreadyExists");
12711321
});
12721322

12731323
it("rejects validate=true with unknown collection in applyWrites as 400", async () => {

0 commit comments

Comments
 (0)