fix: preserve OPFS recovery authority during cleanup
Repair the compensating half of the OPFS put saga. The coordinator now owns a single abortPreparedPut() driven by a composition-owned bounded signal instead of the caller's already aborted one, and the worker client no longer issues a duplicate fire-and-forget abort. Journal rows and budget reservations are released only after the physical effect is confirmed CLEANED or ALREADY_CLEAN; a timeout, malformed response or EFFECT_UNKNOWN keeps PREPARING/FILES_READY and returns OBJECT_RECONCILE. New writes carry a transaction-unique physicalGenerationId through the staging receipt, manifest path and prepared object, so a late compensation deletes only its own transaction's directory even when a newer transaction legitimately reuses the same logical generation. v1 paths, receipts and prepared objects stay readable through the rollback window. Abort and cleanup hold the origin mutation lease through physical deletion and staging removal. A transaction that never reached staging returns ALREADY_CLEAN without waiting for the lease, which would otherwise deadlock against the BEGIN it is cancelling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
6d1e44f206
commit
618da9abf5
@@ -290,7 +290,10 @@ function createWorker(
|
||||
return browserDataSuccess(undefined);
|
||||
},
|
||||
async cleanupTransaction() {
|
||||
return browserDataSuccess(undefined);
|
||||
return browserDataSuccess({ kind: "CLEANED" as const });
|
||||
},
|
||||
async abortPreparedPut() {
|
||||
return browserDataSuccess({ kind: "CLEANED" as const });
|
||||
},
|
||||
async finalizePut() {
|
||||
return browserDataSuccess(undefined);
|
||||
@@ -501,6 +504,176 @@ describe("OPFS byte-store coordinator", () => {
|
||||
expect(replacementWorkerMethod).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps PREPARING journal when compensating cleanup is aborted or unavailable", async () => {
|
||||
for (const failing of [
|
||||
{
|
||||
async abortPreparedPut() {
|
||||
return browserDataFailure("ABORTED", "OBJECT_RECONCILE");
|
||||
},
|
||||
},
|
||||
{
|
||||
async abortPreparedPut() {
|
||||
return browserDataSuccess({ kind: "EFFECT_UNKNOWN" as const });
|
||||
},
|
||||
},
|
||||
]) {
|
||||
const journal = createJournal();
|
||||
const worker = createWorker({
|
||||
async preparePut() {
|
||||
return browserDataFailure("QUOTA_EXCEEDED", "OBJECT_WRITE");
|
||||
},
|
||||
...failing,
|
||||
});
|
||||
const adapter = createOpfsByteStoreAdapter({
|
||||
journal,
|
||||
worker,
|
||||
scope,
|
||||
storagePolicy,
|
||||
createTransactionId: () => "transaction_12345678",
|
||||
now: () => 100,
|
||||
});
|
||||
|
||||
const result = await adapter.objects.put({
|
||||
objectId: "object_12345678",
|
||||
expectedGeneration: null,
|
||||
mediaType: "application/octet-stream",
|
||||
source: sourceFrom(new Uint8Array([1])),
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
// The journal row is the only durable evidence that staging bytes may
|
||||
// still exist, so it survives an unconfirmed compensation.
|
||||
expect(
|
||||
journal.transactions.get("transaction_12345678")?.phase,
|
||||
).toBe("PREPARING");
|
||||
}
|
||||
});
|
||||
|
||||
it("does not roll back journal after an unknown worker mutation effect", async () => {
|
||||
const journal = createJournal();
|
||||
const worker = createWorker({
|
||||
async abortPreparedPut() {
|
||||
return browserDataSuccess({ kind: "EFFECT_UNKNOWN" as const });
|
||||
},
|
||||
});
|
||||
const rollback = vi.spyOn(journal, "rollback");
|
||||
vi.spyOn(journal, "markFilesReady").mockResolvedValueOnce(
|
||||
browserDataFailure("UNAVAILABLE", "OBJECT_WRITE"),
|
||||
);
|
||||
const adapter = createOpfsByteStoreAdapter({
|
||||
journal,
|
||||
worker,
|
||||
scope,
|
||||
storagePolicy,
|
||||
createTransactionId: () => "transaction_12345678",
|
||||
now: () => 100,
|
||||
});
|
||||
|
||||
const result = await adapter.objects.put({
|
||||
objectId: "object_12345678",
|
||||
expectedGeneration: null,
|
||||
mediaType: "application/octet-stream",
|
||||
source: sourceFrom(new Uint8Array([1])),
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
expect(rollback).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("holds the OPFS mutation lease until exact physical cleanup completes", async () => {
|
||||
const journal = createJournal();
|
||||
const observed: Array<Readonly<Record<string, unknown>>> = [];
|
||||
const worker = createWorker({
|
||||
async preparePut() {
|
||||
return browserDataFailure("QUOTA_EXCEEDED", "OBJECT_WRITE");
|
||||
},
|
||||
async abortPreparedPut(request) {
|
||||
observed.push({ ...request });
|
||||
return browserDataSuccess({ kind: "CLEANED" as const });
|
||||
},
|
||||
});
|
||||
const compensation = new AbortController();
|
||||
const caller = new AbortController();
|
||||
caller.abort();
|
||||
const adapter = createOpfsByteStoreAdapter({
|
||||
journal,
|
||||
worker,
|
||||
scope,
|
||||
storagePolicy,
|
||||
createTransactionId: () => "transaction_12345678",
|
||||
createPhysicalGenerationId: () => "f".repeat(32) as never,
|
||||
compensationSignal: compensation.signal,
|
||||
now: () => 100,
|
||||
});
|
||||
|
||||
await adapter.objects.put({
|
||||
objectId: "object_12345678",
|
||||
expectedGeneration: null,
|
||||
mediaType: "application/octet-stream",
|
||||
source: sourceFrom(new Uint8Array([1])),
|
||||
});
|
||||
|
||||
expect(observed).toHaveLength(1);
|
||||
expect(observed[0]).toMatchObject({
|
||||
transactionId: "transaction_12345678",
|
||||
physicalGenerationId: "f".repeat(32),
|
||||
});
|
||||
// Compensation never inherits the caller signal.
|
||||
expect(observed[0]?.signal).toBe(compensation.signal);
|
||||
expect(
|
||||
journal.transactions.has("transaction_12345678"),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("delayed stale cleanup cannot delete a reused logical generation", async () => {
|
||||
const journal = createJournal();
|
||||
const issued: string[] = [];
|
||||
let nextToken = 0;
|
||||
const worker = createWorker({
|
||||
async abortPreparedPut(request) {
|
||||
issued.push(request.physicalGenerationId);
|
||||
return browserDataSuccess({ kind: "CLEANED" as const });
|
||||
},
|
||||
});
|
||||
let transaction = 0;
|
||||
// T1 abandons its prepared put at the same logical generation 1.
|
||||
vi.spyOn(journal, "markFilesReady").mockResolvedValueOnce(
|
||||
browserDataFailure("UNAVAILABLE", "OBJECT_WRITE"),
|
||||
);
|
||||
const adapter = createOpfsByteStoreAdapter({
|
||||
journal,
|
||||
worker,
|
||||
scope,
|
||||
storagePolicy,
|
||||
createTransactionId: () => `transaction_1234567${(transaction += 1)}`,
|
||||
createPhysicalGenerationId: () =>
|
||||
String(nextToken += 1).padStart(32, "0") as never,
|
||||
now: () => 100,
|
||||
});
|
||||
const first = await adapter.objects.put({
|
||||
objectId: "object_12345678",
|
||||
expectedGeneration: null,
|
||||
mediaType: "application/octet-stream",
|
||||
source: sourceFrom(new Uint8Array([1])),
|
||||
});
|
||||
expect(first.ok).toBe(false);
|
||||
|
||||
// T2 legitimately reuses logical generation 1 with a different token.
|
||||
const second = await adapter.objects.put({
|
||||
objectId: "object_12345678",
|
||||
expectedGeneration: null,
|
||||
mediaType: "application/octet-stream",
|
||||
source: sourceFrom(new Uint8Array([1])),
|
||||
});
|
||||
expect(second.ok).toBe(true);
|
||||
if (second.ok) expect(second.value.generation).toBe(1);
|
||||
|
||||
// The stale compensation targeted only T1's physical token.
|
||||
expect(issued).toEqual([String(1).padStart(32, "0")]);
|
||||
const stored = journal.objects.get("object_12345678");
|
||||
expect(stored?.physicalSchemaVersion).toBe(1);
|
||||
});
|
||||
|
||||
it("keeps a committed journal row for reconciliation when cleanup fails", async () => {
|
||||
const journal = createJournal();
|
||||
const worker = createWorker({
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
|
||||
|
||||
import type {
|
||||
OpfsPreparedObject,
|
||||
OpfsPhysicalGenerationId,
|
||||
OpfsStorageScope,
|
||||
} from "../../src/application/ports/browser-file-storage/opfs-ports.ts";
|
||||
import type {
|
||||
@@ -170,10 +171,13 @@ function notFound(): DOMException {
|
||||
return new DOMException("Entry was not found.", "NotFoundError");
|
||||
}
|
||||
|
||||
const PHYSICAL_GENERATION_A = "a".repeat(32) as OpfsPhysicalGenerationId;
|
||||
|
||||
function beginRequest(
|
||||
requestId: string,
|
||||
transactionId: string,
|
||||
scope: OpfsStorageScope,
|
||||
physicalGenerationId: OpfsPhysicalGenerationId = PHYSICAL_GENERATION_A,
|
||||
): OpfsWorkerRequest {
|
||||
return {
|
||||
requestId,
|
||||
@@ -182,6 +186,7 @@ function beginRequest(
|
||||
scope,
|
||||
objectId: "object_12345678",
|
||||
generation: 1,
|
||||
physicalGenerationId,
|
||||
declaredByteLength: 1,
|
||||
mediaType: "application/octet-stream",
|
||||
createdAtEpochMs: 100,
|
||||
|
||||
Reference in New Issue
Block a user