Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/applywrites-same-rkey-batch.md
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 0 additions & 18 deletions packages/pds/src/xrpc/repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>();

for (let i = 0; i < writes.length; i++) {
const write = writes[i];
Expand Down Expand Up @@ -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,
Expand Down
58 changes: 54 additions & 4 deletions packages/pds/test/xrpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down Expand Up @@ -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 () => {
Expand Down
Loading