this repo has no description
Require parent directory URI on document delete
The old signature took Option<&str> and silently skipped the parent's
entries-array cleanup when None was passed. The web store was passing
undefined while viewing the cabinet root (the currentDirectoryUri was
losing the root URI at load time), which left dangling document URIs
in the directory record on the PDS. Any consumer mirroring that record
— the indexer included — saw a ghost document that kept showing up in
tree views even after the document itself was deleted.
Two stacked fixes:
- Make parent_directory_uri required all the way up the stack (core →
wasm → sdk → react hook). Callers can no longer silently corrupt
tree state.
- Track resolvedUri (not the caller's input) on loadDirectory in the
web store, and resolve the parent via findParentUri in deleteDocument
so the mutation is robust against viewing context.
Adds an integration test in manager_tests.rs that asserts the
applyWrites batch carries both the document delete op and the directory
update with the URI pruned and siblings preserved.
+126
-25
+14
-3
apps/web/src/stores/documents/store.ts
+14
-3
apps/web/src/stores/documents/store.ts
···
482
482
set((draft) => {
483
483
draft.items = fileItems as typeof draft.items;
484
484
draft.treeSnapshot = snapshot as typeof draft.treeSnapshot;
485
-
draft.currentDirectoryUri = directoryUri;
485
+
// Track the resolved URI — root fallback applied — so callers that
486
+
// read currentDirectoryUri (upload default, breadcrumb, watcher key)
487
+
// see the actual loaded node rather than the caller's `undefined`.
488
+
draft.currentDirectoryUri = resolvedUri;
486
489
draft.loaded = true;
487
490
draft.error = null;
488
491
});
···
518
521
519
522
async deleteDocument(documentUri) {
520
523
await runMutation("documents-delete", "File deleted", "Delete failed", async (fm) => {
521
-
const dir = get().currentDirectoryUri;
522
-
await fm.delete(documentUri, dir ?? undefined);
524
+
const snapshot = get().treeSnapshot;
525
+
const parent = snapshot ? findParentUri(snapshot, documentUri) : null;
526
+
if (!parent) {
527
+
// The tree must know where the doc lives — otherwise the atomic
528
+
// delete+unlink can't run and we'd orphan the entry in PDS state.
529
+
throw new Error(
530
+
`Cannot delete ${documentUri}: parent directory not found in tree snapshot`,
531
+
);
532
+
}
533
+
await fm.delete(documentUri, parent);
523
534
});
524
535
},
525
536
+13
-13
crates/opake-core/src/manager/delete.rs
+13
-13
crates/opake-core/src/manager/delete.rs
···
12
12
impl<T: Transport, R: CryptoRng + RngCore, S: Storage> FileManager<'_, T, R, S> {
13
13
/// Delete a document and clean up its directory entry — atomically.
14
14
///
15
-
/// When a parent directory is provided, the document deletion and directory
16
-
/// entry removal happen in a single `applyWrites` call. No dangling
17
-
/// references on partial failure.
15
+
/// The caller MUST pass the parent directory URI. The document deletion
16
+
/// and directory entry removal happen in a single `applyWrites` call, so
17
+
/// there's no dangling reference on partial failure.
18
+
///
19
+
/// Callers that genuinely don't know the parent (shouldn't happen via the
20
+
/// UI) must resolve it from the tree first. Silently accepting an unknown
21
+
/// parent would leave the entry behind in the parent's `entries` array
22
+
/// and desync every consumer that mirrors that list.
18
23
#[::opake_derive::signoff]
19
24
pub async fn delete(
20
25
&mut self,
21
26
document_uri: &str,
22
-
parent_directory_uri: Option<&str>,
27
+
parent_directory_uri: &str,
23
28
) -> Result<MutationOutcome, Error> {
24
29
let doc_at = atproto::parse_at_uri(document_uri)?;
25
30
let delete_op = ApplyWriteOp::Delete {
···
27
32
rkey: doc_at.rkey.clone(),
28
33
};
29
34
30
-
let Some(parent) = parent_directory_uri else {
31
-
self.opake.client.apply_writes(&[delete_op]).await?;
32
-
return Ok(MutationOutcome::Applied);
33
-
};
34
-
35
35
let now = self.opake.now();
36
36
37
37
match &self.context {
38
38
FileContext::Cabinet(_) => {
39
39
let dir_op = directories::prepare_remove_entry(
40
40
&mut self.opake.client,
41
-
parent,
41
+
parent_directory_uri,
42
42
document_uri,
43
43
&now,
44
44
)
···
49
49
Ok(MutationOutcome::Applied)
50
50
}
51
51
FileContext::Workspace(ws) => {
52
-
let dir_owner = atproto::parse_at_uri(parent)?.authority;
52
+
let dir_owner = atproto::parse_at_uri(parent_directory_uri)?.authority;
53
53
if dir_owner == self.opake.did {
54
54
let dir_op = directories::prepare_remove_entry(
55
55
&mut self.opake.client,
56
-
parent,
56
+
parent_directory_uri,
57
57
document_uri,
58
58
&now,
59
59
)
···
65
65
} else {
66
66
let update = DirectoryUpdateRecord::remove_entry(
67
67
ws.uri.clone(),
68
-
parent.to_string(),
68
+
parent_directory_uri.to_string(),
69
69
document_uri.to_string(),
70
70
now,
71
71
);
+86
crates/opake-core/src/manager/manager_tests.rs
+86
crates/opake-core/src/manager/manager_tests.rs
···
1
+
use crate::client::RequestBody;
1
2
use crate::manager::types::{FileContext, MutationOutcome};
3
+
use crate::records::Directory;
2
4
3
5
#[test]
4
6
fn file_context_owner_did_cabinet() {
···
34
36
assert_eq!(ctx.owner_did(), "did:plc:bob");
35
37
assert!(ctx.is_workspace());
36
38
assert!(!ctx.is_cabinet());
39
+
}
40
+
41
+
/// Cabinet delete must atomically (a) delete the document record AND
42
+
/// (b) remove the document URI from the parent directory's entries.
43
+
/// Regression: the old code had an escape hatch that skipped (b) when the
44
+
/// caller didn't pass a parent URI, leaving dangling entries in the PDS
45
+
/// directory record (and therefore in any consumer that mirrored it).
46
+
#[tokio::test]
47
+
async fn cabinet_delete_removes_doc_and_unlinks_parent_entry() {
48
+
use crate::client::{Session, LegacySession, XrpcClient};
49
+
use crate::crypto::OsRng;
50
+
use crate::directories::tests::{
51
+
dummy_directory_with_entries, get_record_response, put_record_response,
52
+
};
53
+
use crate::opake::Opake;
54
+
use crate::storage::{Identity, NoopStorage};
55
+
use crate::test_utils::MockTransport;
56
+
57
+
const DID: &str = "did:plc:test";
58
+
const PARENT_URI: &str = "at://did:plc:test/app.opake.directory/self";
59
+
const DOC_URI: &str = "at://did:plc:test/app.opake.document/doc1";
60
+
const OTHER_DOC_URI: &str = "at://did:plc:test/app.opake.document/keep";
61
+
62
+
// Seed the mock: getRecord for the parent directory (loaded by
63
+
// prepare_remove_entry), then applyWrites (the atomic delete + update).
64
+
let mock = MockTransport::new();
65
+
mock.enqueue(get_record_response(
66
+
PARENT_URI,
67
+
&dummy_directory_with_entries("root", vec![DOC_URI.into(), OTHER_DOC_URI.into()]),
68
+
));
69
+
mock.enqueue(put_record_response(PARENT_URI));
70
+
71
+
let session = Session::Legacy(LegacySession {
72
+
did: DID.into(),
73
+
handle: "test.handle".into(),
74
+
access_jwt: "test-jwt".into(),
75
+
refresh_jwt: "test-refresh".into(),
76
+
});
77
+
let client = XrpcClient::with_session(mock.clone(), "https://pds.test".into(), session);
78
+
let identity = Identity::generate(DID, &mut OsRng);
79
+
let mut opake = Opake::new(
80
+
client,
81
+
DID.into(),
82
+
Some(identity),
83
+
OsRng,
84
+
NoopStorage,
85
+
|| "2026-01-01T00:00:00Z".into(),
86
+
|| 1_700_000_000_000_000,
87
+
);
88
+
89
+
let ctx = opake.cabinet_context().unwrap();
90
+
let mut mgr = opake.file_manager(&ctx);
91
+
let outcome = mgr.delete(DOC_URI, PARENT_URI).await.unwrap();
92
+
93
+
assert!(matches!(outcome, MutationOutcome::Applied));
94
+
95
+
let reqs = mock.requests();
96
+
assert_eq!(reqs.len(), 2, "expected getRecord + applyWrites");
97
+
assert!(reqs[0].url.contains("getRecord"), "first call is fetch");
98
+
assert!(reqs[1].url.contains("applyWrites"), "second call is atomic write");
99
+
100
+
// Assert the applyWrites body carries BOTH ops in one batch, and the
101
+
// directory update actually prunes the deleted doc (but keeps siblings).
102
+
let Some(RequestBody::Json(body)) = &reqs[1].body else {
103
+
panic!("applyWrites body should be JSON");
104
+
};
105
+
let writes = body["writes"].as_array().expect("writes array");
106
+
assert_eq!(writes.len(), 2, "atomic batch: [delete doc, update dir]");
107
+
108
+
let delete_op = &writes[0];
109
+
assert_eq!(delete_op["$type"], "com.atproto.repo.applyWrites#delete");
110
+
assert_eq!(delete_op["collection"], "app.opake.document");
111
+
assert_eq!(delete_op["rkey"], "doc1");
112
+
113
+
let update_op = &writes[1];
114
+
assert_eq!(update_op["$type"], "com.atproto.repo.applyWrites#update");
115
+
assert_eq!(update_op["collection"], "app.opake.directory");
116
+
let updated: Directory = serde_json::from_value(update_op["value"].clone()).unwrap();
117
+
assert_eq!(
118
+
updated.entries,
119
+
vec![OTHER_DOC_URI.to_string()],
120
+
"deleted doc URI must be pruned from parent.entries; siblings preserved",
121
+
);
122
+
assert_eq!(updated.modified_at.as_deref(), Some("2026-01-01T00:00:00Z"));
37
123
}
38
124
39
125
#[test]
+2
-2
crates/opake-wasm/src/file_manager_wasm.rs
+2
-2
crates/opake-wasm/src/file_manager_wasm.rs
···
87
87
pub async fn delete(
88
88
&self,
89
89
document_uri: &str,
90
-
parent_directory_uri: Option<String>,
90
+
parent_directory_uri: &str,
91
91
) -> Result<JsValue, JsError> {
92
92
let (mut opake, ctx) = self.parts().await?;
93
93
let mut mgr = opake.file_manager(ctx);
94
94
let result = mgr
95
-
.delete(document_uri, parent_directory_uri.as_deref())
95
+
.delete(document_uri, parent_directory_uri)
96
96
.await
97
97
.map_err(wasm_err)?;
98
98
to_js(&MutationResultDto {
+1
-2
packages/opake-react/src/hooks/use-delete.ts
+1
-2
packages/opake-react/src/hooks/use-delete.ts
···
3
3
4
4
interface DeleteInput {
5
5
readonly documentUri: string;
6
-
readonly parentDirectoryUri?: string;
6
+
readonly parentDirectoryUri: string;
7
7
}
8
8
9
9
/**
···
18
18
keyringUri,
19
19
mutationFn: (fm, input) => fm.delete(input.documentUri, input.parentDirectoryUri),
20
20
optimisticUpdate: (snapshot, input) => {
21
-
if (!input.parentDirectoryUri) return snapshot;
22
21
const dir = snapshot.directories[input.parentDirectoryUri];
23
22
if (!dir) return snapshot;
24
23
+10
-5
packages/opake-sdk/src/file-manager.ts
+10
-5
packages/opake-sdk/src/file-manager.ts
···
35
35
directoryUri: string | null,
36
36
): Promise<unknown>;
37
37
download(documentUri: string): Promise<unknown>;
38
-
delete(documentUri: string, parentDirectoryUri: string | null): Promise<unknown>;
38
+
delete(documentUri: string, parentDirectoryUri: string): Promise<unknown>;
39
39
moveEntry(entryUri: string, sourceDir: string, targetDir: string): Promise<unknown>;
40
40
createDirectory(name: string, parentUri: string | null): Promise<unknown>;
41
41
ensureRoot(): Promise<string>;
···
213
213
* directory URI is provided, also removes the entry from that directory.
214
214
*
215
215
* @param documentUri - AT URI of the document to delete.
216
-
* @param parentDirectoryUri - Directory containing the document (for entry cleanup).
216
+
* @param parentDirectoryUri - Directory containing the document. Required:
217
+
* the delete is atomic with removing this entry from the parent. Callers
218
+
* must resolve the parent before calling; passing the wrong one is caught
219
+
* by `prepare_remove_entry` (NotFound error), passing a valid-but-stale
220
+
* one is not.
217
221
*
218
-
* @throws {OpakeError} kind "NotFound" if the document doesn't exist.
222
+
* @throws {OpakeError} kind "NotFound" if the document doesn't exist in the
223
+
* parent directory.
219
224
*/
220
225
@wrapWasmErrors
221
-
delete(documentUri: string, parentDirectoryUri?: string): Promise<MutationResult> {
226
+
delete(documentUri: string, parentDirectoryUri: string): Promise<MutationResult> {
222
227
return this.requireHandle().delete(
223
228
documentUri,
224
-
parentDirectoryUri ?? null,
229
+
parentDirectoryUri,
225
230
) as Promise<MutationResult>;
226
231
}
227
232