From 1c3045e81047e83d3563c74913fd2df76d5a6850 Mon Sep 17 00:00:00 2001 From: Nico Wiedemann Date: Sun, 27 Sep 2026 01:07:43 +0200 Subject: [PATCH 1/3] Delete a context's stashes with it, instead of stranding them Deleting a context - locally, from another device, or over MCP - only marked the context deleted. Its stashes kept pointing at the dead id, so no view showed them (every filter compares against contextId || "default"), yet they were still stored, counted, exported and synced. The cloud had the same gap: it never deleted a context's stashes either, and a stash with no context at all - an unknown id on create, an explicit null on move, a device pushing a context this server had not seen - was stored with context_id NULL, which the app renders as Default but the server never queries as "default". Both sides now apply the same two rules. No context, or one that does not exist, means the default context: every installation creates it and nothing can delete it. A stash whose context is deleted is deleted with it. Deleting a context now deletes the stashes in it, and the confirmation dialog says how many. A database trigger refuses a NULL context_id on both sides rather than a NOT NULL column, since the column has been nullable since the first migration and adding the constraint would mean rebuilding the table under attachments' ON DELETE CASCADE. Every stash write now runs enforce_context_rules(): on startup, after importing contexts or stashes from a sync pull, and after a local context deletion. It moves stashes with no valid context to Default and deletes any still sitting under an already-deleted one, returning what it deleted so the caller can remove their cached attachment files. Matches the migration and rules just committed to cloud/, so a stash ends up in the same place whichever side writes it. Ran the Rust suite (159 tests, 6 new) and the frontend suite (219 tests, 2 new), plus `tsc` and `svelte-check` with zero errors. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KkYvnuq8Y4nYD1jSCi9GCR --- CHANGELOG.md | 16 ++ TESTING.md | 2 +- src-tauri/src/account_export.rs | 14 +- src-tauri/src/contexts.rs | 18 +- src-tauri/src/db.rs | 227 +++++++++++++++++- src-tauri/src/models.rs | 26 +- src-tauri/src/stashes.rs | 12 +- src-tauri/src/transfer.rs | 13 +- src/lib/components/ContextManager.svelte | 16 +- src/lib/i18n/locales/de.json | 3 + src/lib/i18n/locales/en.json | 3 + src/lib/services/__tests__/cloud-sync.test.ts | 21 ++ .../__tests__/desktop-adapter.test.ts | 4 + src/lib/services/cloud-sync.ts | 6 +- src/lib/types.ts | 3 +- 15 files changed, 341 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a8d16e..44d354e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,22 @@ popover, not for the person who wrote the commit. ## [Unreleased] +### Changed +- **Deleting a context now deletes the stashes in it, and the confirmation says how many.** + They used to stay behind, filed under a context that no longer existed: no view showed + them, yet they were still stored, counted and synced. Deleted stashes stay recoverable on + the cloud for about 30 days. The Default context still cannot be deleted + +### Fixed +- **Stashes left behind by an earlier context deletion are cleaned up.** After this update, + any stash still filed under a deleted context - on this device or in the cloud - is deleted + like its context was. You could not see these stashes anyway +- **Every stash now belongs to a context, the same way here and in the cloud.** A stash + saved by an older version, imported from an older export, or created by an AI agent could + end up with no context at all. The queue showed it under Default, but clearing completed + stashes there, exporting, and the cloud all missed it. Such a stash is now filed under + Default everywhere + ## [1.8.9] - 2026-09-26 ### Security diff --git a/TESTING.md b/TESTING.md index c40a282..9cfb84c 100644 --- a/TESTING.md +++ b/TESTING.md @@ -200,7 +200,7 @@ mod tests { attachments: vec![], files: vec![], created_at: chrono::Utc::now().to_rfc3339(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), completed: false, completed_at: None, }; diff --git a/src-tauri/src/account_export.rs b/src-tauri/src/account_export.rs index bbb253c..35fc7c7 100644 --- a/src-tauri/src/account_export.rs +++ b/src-tauri/src/account_export.rs @@ -188,7 +188,13 @@ async fn parse_export( stashes.push(StashItem { id, - context_id: raw["context_id"].as_str().map(str::to_string), + // An export from before every stash had a context carries null here, which + // lands in the default context - where the app showed it all along. + context_id: raw["context_id"] + .as_str() + .filter(|id| !id.is_empty()) + .unwrap_or(crate::models::DEFAULT_CONTEXT_ID) + .to_string(), content, files: Vec::new(), created_at: raw["created_at"].as_str().unwrap_or_default().to_string(), @@ -303,11 +309,7 @@ pub async fn export_whole_account( let mut by_context: HashMap> = HashMap::new(); for stash in stashes { - let owner = stash - .context_id - .clone() - .unwrap_or_else(|| "default".to_string()); - by_context.entry(owner).or_default().push(stash.id); + by_context.entry(stash.context_id).or_default().push(stash.id); } (contexts, by_context) }; diff --git a/src-tauri/src/contexts.rs b/src-tauri/src/contexts.rs index 17464b0..151c3bd 100644 --- a/src-tauri/src/contexts.rs +++ b/src-tauri/src/contexts.rs @@ -86,18 +86,22 @@ pub async fn save_context(state: State<'_, Arc>, context: Context) -> R /// edited and push it straight back on the next sync. #[tauri::command] pub async fn import_contexts(state: State<'_, Arc>, contexts: Vec) -> Result<(), UiError> { - state - .lock_db() - .import_contexts(&contexts) - .map_err(|e| e.to_string()) - .map_err(UiError::from) + let mut db = state.lock_db(); + // A context deleted on another device takes its stashes with it here too. + let deleted = db.import_contexts(&contexts).map_err(|e| e.to_string())?; + crate::stashes::purge_deleted_stash_files(&db, &deleted); + Ok(()) } +/// Delete a context and every stash filed under it - the same rule the cloud applies, so +/// the result is the same whichever side the deletion starts on. #[tauri::command] pub async fn delete_context(state: State<'_, Arc>, id: String) -> Result<(), UiError> { println!("Deleting context: {}", id); - if let Err(e) = state.lock_db().delete_context(&id) { - println!("Failed to delete context: {}", e); + let mut db = state.lock_db(); + match db.delete_context(&id) { + Ok(deleted) => crate::stashes::purge_deleted_stash_files(&db, &deleted), + Err(e) => println!("Failed to delete context: {}", e), } Ok(()) } diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index acd4965..f398edd 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -292,12 +292,74 @@ impl DbManager { // Ensure default context exists self.ensure_default_context()?; + // Every stash belongs to a context. Bring old rows into line, then refuse new ones + // that are not: the column has been nullable since it was created, and adding NOT + // NULL would mean rebuilding the table under attachments' ON DELETE CASCADE. + self.enforce_context_rules()?; + self.conn.execute_batch( + "CREATE TRIGGER IF NOT EXISTS stashes_context_required_on_insert + BEFORE INSERT ON stashes WHEN NEW.context_id IS NULL + BEGIN SELECT RAISE(ABORT, 'every stash belongs to a context'); END; + CREATE TRIGGER IF NOT EXISTS stashes_context_required_on_update + BEFORE UPDATE OF context_id ON stashes WHEN NEW.context_id IS NULL + BEGIN SELECT RAISE(ABORT, 'every stash belongs to a context'); END;", + )?; + // Repair rows left behind by the name-collision bug. self.reconcile_colliding_attachment_sizes(); Ok(()) } + /// Apply the rules that decide where a stash lives, and return the stashes it deleted. + /// + /// The cloud applies the same two rules to everything it stores, so a stash ends up in + /// the same place on every side: + /// + /// * **No context, or one this device does not have, means the default context.** Older + /// builds, older exports and an older server all wrote NULL, and the queue already + /// showed those stashes under Default - but deleting completed stashes there, exporting + /// the account, or filing attachments all looked for `'default'` and missed them. + /// * **A deleted context means a deleted stash.** Deleting a context used to leave its + /// stashes pointing at it, where no view shows them, while they were still counted, + /// synced and kept. The same holds when the deletion arrives from another device. + /// + /// Safe to run at any time and as often as needed: it only touches rows that break a + /// rule. The rows it deletes are stamped and queued like any other local deletion, and + /// the caller removes their files. + pub fn enforce_context_rules(&self) -> Result> { + self.conn.execute( + "UPDATE stashes SET context_id = 'default' \ + WHERE context_id IS NULL OR context_id = '' \ + OR context_id NOT IN (SELECT id FROM contexts)", + [], + )?; + + const IN_A_DELETED_CONTEXT: &str = "deleted = 0 AND context_id IN \ + (SELECT id FROM contexts WHERE deleted = 1 AND id != 'default')"; + + let doomed: Vec<(String, String)> = { + let mut stmt = self.conn.prepare(&format!( + "SELECT id, context_id FROM stashes WHERE {}", + IN_A_DELETED_CONTEXT + ))?; + let rows = stmt.query_map([], |row| Ok((row.get(0)?, row.get(1)?)))?; + rows.collect::>()? + }; + + if !doomed.is_empty() { + self.conn.execute( + &format!( + "UPDATE stashes SET deleted = 1, updated_at = ?1, pending_sync = 1 WHERE {}", + IN_A_DELETED_CONTEXT + ), + params![now_ts()], + )?; + } + + Ok(doomed) + } + /// Correct the recorded size of attachments that share a file with another row. /// /// Attachments used to be written straight to `cache///`, so two @@ -587,9 +649,18 @@ impl DbManager { /// Apply contexts received from the server in one transaction, preserving their /// timestamps so last-write-wins stays stable across devices. - pub fn import_contexts(&mut self, contexts: &[Context]) -> Result<()> { + /// + /// Returns the stashes a deletion took with it - see + /// [`enforce_context_rules`](Self::enforce_context_rules) - so the caller can remove + /// their files. + pub fn import_contexts(&mut self, contexts: &[Context]) -> Result> { let tx = self.conn.transaction()?; for ctx in contexts { + // Nothing deletes the default context, so a tombstone for it is dropped rather + // than allowed to take every unfiled stash with it. + if ctx.id == "default" && ctx.deleted { + continue; + } // The default context keeps its name and rules on every device. let (name, rules_json) = if ctx.id == "default" { ("Default".to_string(), "[]".to_string()) @@ -614,20 +685,22 @@ impl DbManager { )?; } tx.commit()?; - Ok(()) + self.enforce_context_rules() } - pub fn delete_context(&mut self, id: &str) -> Result<()> { + /// Delete a context and every stash in it, returning those stashes so the caller can + /// remove their files. + pub fn delete_context(&mut self, id: &str) -> Result> { // Protect default context from being deleted if id == "default" { - return Ok(()); // Silently ignore deletion attempts + return Ok(Vec::new()); // Silently ignore deletion attempts } self.conn.execute( "UPDATE contexts SET deleted = 1, updated_at = ?2, pending_sync = 1 WHERE id = ?1", params![id, now_ts()], )?; - Ok(()) + self.enforce_context_rules() } // --- Stash CRUD --- @@ -1091,6 +1164,8 @@ impl DbManager { } } tx.commit()?; + // A pulled stash can name a context this device has deleted, or never had. + self.enforce_context_rules()?; Ok(()) } @@ -1400,6 +1475,132 @@ mod tests { assert!(default_ctx.is_some(), "Default context should exist"); assert_eq!(default_ctx.unwrap().name, "Default"); } + + fn plain_context(id: &str, deleted: bool) -> Context { + Context { + id: id.to_string(), + name: id.to_string(), + description: None, + rules: vec![], + last_used: None, + updated_at: Some(1), + deleted, + } + } + + fn insert_raw_stash(db: &DbManager, id: &str, context_id: Option<&str>) { + db.conn + .execute( + "INSERT INTO stashes (id, context_id, content, files, created_at, completed, deleted) \ + VALUES (?1, ?2, 'x', '[]', '2026-09-27T00:00:00Z', 0, 0)", + params![id, context_id], + ) + .unwrap(); + } + + /// (context_id, deleted) as stored. + fn stored(db: &DbManager, id: &str) -> (String, bool) { + db.conn + .query_row( + "SELECT context_id, deleted FROM stashes WHERE id = ?1", + params![id], + |row| Ok((row.get(0)?, row.get::<_, i32>(1)? != 0)), + ) + .unwrap() + } + + #[test] + fn a_stash_without_a_known_context_lands_in_the_default_one() { + let db = create_test_db(); + // Planted the way older builds wrote them, with the guard lifted. + db.conn + .execute_batch( + "DROP TRIGGER stashes_context_required_on_insert; \ + DROP TRIGGER stashes_context_required_on_update;", + ) + .unwrap(); + insert_raw_stash(&db, "s-null", None); + insert_raw_stash(&db, "s-empty", Some("")); + insert_raw_stash(&db, "s-unknown", Some("never-synced")); + + let deleted = db.enforce_context_rules().unwrap(); + + assert!(deleted.is_empty(), "nothing here is in a deleted context"); + for id in ["s-null", "s-empty", "s-unknown"] { + assert_eq!(stored(&db, id), ("default".to_string(), false), "{}", id); + } + } + + #[test] + fn deleting_a_context_deletes_its_stashes() { + let mut db = create_test_db(); + db.save_context(&plain_context("work", false), WriteOrigin::LocalEdit).unwrap(); + insert_raw_stash(&db, "s-work", Some("work")); + insert_raw_stash(&db, "s-default", Some("default")); + + let deleted = db.delete_context("work").unwrap(); + + assert_eq!(deleted, vec![("s-work".to_string(), "work".to_string())]); + assert_eq!(stored(&db, "s-work"), ("work".to_string(), true)); + assert_eq!(stored(&db, "s-default"), ("default".to_string(), false)); + let pending: i32 = db + .conn + .query_row("SELECT pending_sync FROM stashes WHERE id = 's-work'", [], |row| row.get(0)) + .unwrap(); + assert_eq!(pending, 1, "the deletion has to reach the cloud"); + } + + #[test] + fn a_context_deleted_on_another_device_takes_its_stashes_here_too() { + let mut db = create_test_db(); + db.import_contexts(&[plain_context("work", false)]).unwrap(); + insert_raw_stash(&db, "s-work", Some("work")); + + let deleted = db.import_contexts(&[plain_context("work", true)]).unwrap(); + + assert_eq!(deleted.len(), 1); + assert!(stored(&db, "s-work").1, "a stash in a deleted context is deleted"); + } + + #[test] + fn the_default_context_survives_a_tombstone_from_the_server() { + let mut db = create_test_db(); + insert_raw_stash(&db, "s-default", Some("default")); + + db.import_contexts(&[plain_context("default", true)]).unwrap(); + + assert!(db.get_contexts().unwrap().iter().any(|c| c.id == "default" && !c.deleted)); + assert!(!stored(&db, "s-default").1); + } + + #[test] + fn the_database_refuses_a_stash_without_a_context() { + let db = create_test_db(); + let insert = db.conn.execute( + "INSERT INTO stashes (id, context_id, content, files, created_at) \ + VALUES ('s-raw', NULL, 'x', '[]', '2026-09-27T00:00:00Z')", + [], + ); + assert!(insert.is_err()); + + insert_raw_stash(&db, "s-raw", Some("default")); + let update = db + .conn + .execute("UPDATE stashes SET context_id = NULL WHERE id = 's-raw'", []); + assert!(update.is_err()); + } + + #[test] + fn a_stash_from_ipc_without_a_context_reads_as_the_default_one() { + for payload in [ + r#"{"id":"a","content":"x","createdAt":"t"}"#, + r#"{"id":"a","content":"x","createdAt":"t","contextId":null}"#, + r#"{"id":"a","content":"x","createdAt":"t","contextId":""}"#, + ] { + let stash: StashItem = serde_json::from_str(payload).unwrap(); + assert_eq!(stash.context_id, "default", "{}", payload); + } + } #[test] fn test_save_and_get_context() { @@ -1468,7 +1669,7 @@ mod tests { let with_file = StashItem { id: "s-with".to_string(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), content: "has an attachment".to_string(), enhanced_content: None, files: vec![], @@ -1535,7 +1736,7 @@ mod tests { let stash = StashItem { id: "test-stash-1".to_string(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), content: "Test stash content".to_string(), enhanced_content: None, files: vec![], @@ -1563,7 +1764,7 @@ mod tests { let stash = StashItem { id: "stash-to-delete".to_string(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), content: "Will be deleted".to_string(), enhanced_content: None, files: vec![], @@ -1591,7 +1792,7 @@ mod tests { // Create completed and active stashes let completed = StashItem { id: "completed-1".to_string(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), content: "Completed task".to_string(), enhanced_content: None, files: vec![], @@ -1605,7 +1806,7 @@ mod tests { let active = StashItem { id: "active-1".to_string(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), content: "Active task".to_string(), enhanced_content: None, files: vec![], @@ -1634,7 +1835,7 @@ mod tests { let stash1 = StashItem { id: "pos-1".to_string(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), content: "First".to_string(), enhanced_content: None, files: vec![], @@ -1648,7 +1849,7 @@ mod tests { let stash2 = StashItem { id: "pos-2".to_string(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), content: "Second".to_string(), enhanced_content: None, files: vec![], @@ -2054,7 +2255,7 @@ mod tests { fn stash_with_updated_at(id: &str, content: &str, updated_at: Option) -> StashItem { StashItem { id: id.to_string(), - context_id: Some("default".to_string()), + context_id: "default".to_string(), content: content.to_string(), enhanced_content: None, files: vec![], diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 0b21792..ac87c3f 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -11,6 +11,24 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. // See the GNU Affero General Public License for more details. +/// The context every installation has and cannot delete, and the home of any stash that +/// has no other. The cloud uses the same id and the same rules. +pub const DEFAULT_CONTEXT_ID: &str = "default"; + +fn default_context_id() -> String { + DEFAULT_CONTEXT_ID.to_string() +} + +fn context_or_default<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let value: Option = serde::Deserialize::deserialize(deserializer)?; + Ok(value + .filter(|id| !id.is_empty()) + .unwrap_or_else(default_context_id)) +} + #[derive(serde::Serialize, serde::Deserialize, Debug, Clone)] #[serde(rename_all = "camelCase")] pub struct Attachment { @@ -51,8 +69,12 @@ pub struct StashItem { #[serde(default)] pub attachments: Vec, pub created_at: String, - #[serde(default)] - pub context_id: Option, + /// Never absent: every stash belongs to a context, and [`DEFAULT_CONTEXT_ID`] is where + /// it goes when there is no other. A missing, `null` or empty value - from an older + /// build, an older export, or a server that still stored NULL - reads as that one, + /// which is also where the queue always showed such a stash. + #[serde(default = "default_context_id", deserialize_with = "context_or_default")] + pub context_id: String, #[serde(default)] pub completed: bool, #[serde(default)] diff --git a/src-tauri/src/stashes.rs b/src-tauri/src/stashes.rs index fa100b6..9da6432 100644 --- a/src-tauri/src/stashes.rs +++ b/src-tauri/src/stashes.rs @@ -103,7 +103,7 @@ pub async fn save_stash( // Minimal struct for check Ok(StashItem { id: row.get(0)?, - context_id: None, + context_id: crate::models::DEFAULT_CONTEXT_ID.to_string(), content: "".into(), enhanced_content: None, files: vec![], @@ -274,6 +274,16 @@ pub async fn delete_completed_stashes(state: State<'_, Arc>, context_id Ok(()) } +/// Remove the files of stashes deleted because their context was, as +/// [`DbManager::enforce_context_rules`] reports them. +pub(crate) fn purge_deleted_stash_files(db: &DbManager, stashes: &[(String, String)]) { + let with_context: Vec<(String, Option)> = stashes + .iter() + .map(|(id, context_id)| (id.clone(), Some(context_id.clone()))) + .collect(); + purge_stash_files(db, &with_context); +} + /// Remove the cache folders of the given stashes and clear the paths that pointed into /// them, so no attachment row is left claiming to hold a file that was just deleted. fn purge_stash_files(db: &DbManager, stashes: &[(String, Option)]) { diff --git a/src-tauri/src/transfer.rs b/src-tauri/src/transfer.rs index 19049f9..c5a288d 100644 --- a/src-tauri/src/transfer.rs +++ b/src-tauri/src/transfer.rs @@ -343,7 +343,7 @@ fn parse_markdown(content: &str, context_id: &str) -> ParsedDocument { files: Vec::new(), attachments: Vec::new(), created_at, - context_id: Some(context_id.to_string()), + context_id: context_id.to_string(), completed: section_completed, completed_at: if section_completed { Some(Utc::now().to_rfc3339()) @@ -517,8 +517,7 @@ pub async fn export_context_archive( let selected: Vec = all .into_iter() .filter(|s| { - let owner = s.context_id.clone().unwrap_or_else(|| "default".to_string()); - owner == context_id && wanted.contains(&s.id) + s.context_id == context_id && wanted.contains(&s.id) }) .collect(); @@ -684,9 +683,7 @@ pub async fn read_import_archive( db.get_stashes() .map_err(|e| e.to_string())? .into_iter() - .filter(|s| { - s.context_id.clone().unwrap_or_else(|| "default".to_string()) == context_id - }) + .filter(|s| s.context_id == context_id) .collect::>() }; @@ -763,7 +760,7 @@ pub async fn commit_import( let mut prepared: Vec = Vec::with_capacity(stashes.len()); for mut stash in stashes { - stash.context_id = Some(copy_context.clone()); + stash.context_id = copy_context.clone(); // Copy each referenced file out of the extraction directory and into the // stash's own cache folder, building the attachment rows as we go. @@ -865,7 +862,7 @@ mod tests { files: Vec::new(), attachments: Vec::new(), created_at: created_at.to_string(), - context_id: Some("ctx".to_string()), + context_id: "ctx".to_string(), completed, completed_at: None, updated_at: None, diff --git a/src/lib/components/ContextManager.svelte b/src/lib/components/ContextManager.svelte index dd9e8f4..e83ab48 100644 --- a/src/lib/components/ContextManager.svelte +++ b/src/lib/components/ContextManager.svelte @@ -148,12 +148,24 @@ await saveContext(newContext); } + /** What the confirmation says goes with the context - its stashes, done ones included. */ + function deleteDescription(count: number): string { + if (count === 0) return $_("contexts.deleteConfirmEmpty"); + if (count === 1) return $_("contexts.deleteConfirmOne"); + return $_("contexts.deleteConfirmMany", { values: { count } }); + } + async function removeContext(id: string) { - // Delete from database (marks as deleted = 1) + // Deletes the context and every stash in it - the backend applies the same rule + // the cloud does, so the stashes go on every device. try { await adapter.deleteContext(id); // Remove from local array after successful DB deletion contexts = contexts.filter((c) => c.id !== id); + allStashes = allStashes.filter((s) => s.contextId !== id); + delete stashCounts[id]; + delete stashTotals[id]; + delete contextSizes[id]; } catch (e) { console.error("Failed to delete context:", e); } @@ -333,7 +345,7 @@ { diff --git a/src/lib/i18n/locales/de.json b/src/lib/i18n/locales/de.json index 77e04cf..4430acf 100644 --- a/src/lib/i18n/locales/de.json +++ b/src/lib/i18n/locales/de.json @@ -323,6 +323,9 @@ "removeContext": "Entfernen", "shiftClickToSkip": "Umschalt+Klick zum Überspringen der Bestätigung", "deleteConfirm": "Diesen Kontext löschen?", + "deleteConfirmEmpty": "Er enthält keine Stashes.", + "deleteConfirmOne": "Sein Stash wird mit ihm gelöscht.", + "deleteConfirmMany": "Seine {count} Stashes werden mit ihm gelöscht.", "autoSwitchRules": "Auto-Wechsel-Regeln", "noRules": "Keine Regeln definiert. Dieser Kontext wird nur bei manueller Auswahl aktiv.", "noContexts": "Keine benutzerdefinierten Kontexte definiert. Stashes gehen zu \"Standard\".", diff --git a/src/lib/i18n/locales/en.json b/src/lib/i18n/locales/en.json index 0a1b2de..8d248e8 100644 --- a/src/lib/i18n/locales/en.json +++ b/src/lib/i18n/locales/en.json @@ -323,6 +323,9 @@ "removeContext": "Remove", "shiftClickToSkip": "Shift+Click to skip confirmation", "deleteConfirm": "Delete this context?", + "deleteConfirmEmpty": "It holds no stashes.", + "deleteConfirmOne": "Its stash is deleted with it.", + "deleteConfirmMany": "Its {count} stashes are deleted with it.", "autoSwitchRules": "Auto-Switch Rules", "noRules": "No rules defined. This context will only be active if selected manually.", "noContexts": "No custom contexts defined. Stashes will go to \"Default\".", diff --git a/src/lib/services/__tests__/cloud-sync.test.ts b/src/lib/services/__tests__/cloud-sync.test.ts index 7290f53..870ed13 100644 --- a/src/lib/services/__tests__/cloud-sync.test.ts +++ b/src/lib/services/__tests__/cloud-sync.test.ts @@ -225,6 +225,27 @@ describe('CloudSyncService', () => { expect(payload.stashes[0].updatedAt).toBe(new Date(1755512000 * 1000).toISOString()); }); + it('never sends a stash without a context', async () => { + // It used to send null, and the server stored the stash with no context at all - + // where the queue here showed it under Default and the server filed it nowhere. + const stash = { + id: 's1', + content: 'from an older row', + createdAt: '2026-08-18T10:00:00Z', + attachments: [], + } as unknown as StashItem; + + const adapter = createAdapter({ + loadStashesForSync: vi.fn().mockResolvedValue([stash]), + }); + const service = new CloudSyncService(adapter); + await service.initialize(settingsWith(cloudConfig())); + await flushPromises(); + + const payload = (adapter.syncStashesApi as any).mock.calls[0][0]; + expect(payload.stashes[0].contextId).toBe('default'); + }); + it('includes the context description so the server can store it', async () => { // Without this the server had nothing to return and every pull blanked the // local description via INSERT OR REPLACE. diff --git a/src/lib/services/__tests__/desktop-adapter.test.ts b/src/lib/services/__tests__/desktop-adapter.test.ts index 9bea76e..94512d0 100644 --- a/src/lib/services/__tests__/desktop-adapter.test.ts +++ b/src/lib/services/__tests__/desktop-adapter.test.ts @@ -39,6 +39,7 @@ describe('DesktopStorageAdapter', () => { content: 'test content', attachments: [], createdAt: '2026-01-08T00:00:00Z', + contextId: 'default', }; mockInvoke.mockResolvedValue(undefined); @@ -55,6 +56,7 @@ describe('DesktopStorageAdapter', () => { content: 'test content', attachments: [], createdAt: '2026-01-08T00:00:00Z', + contextId: 'default', }; mockInvoke.mockResolvedValue(undefined); @@ -74,12 +76,14 @@ describe('DesktopStorageAdapter', () => { content: 'stash 1', attachments: [], createdAt: '2026-01-08T00:00:00Z', + contextId: 'default', }, { id: '2', content: 'stash 2', attachments: [], createdAt: '2026-01-08T01:00:00Z', + contextId: 'default', }, ]; diff --git a/src/lib/services/cloud-sync.ts b/src/lib/services/cloud-sync.ts index c25e34c..376b015 100644 --- a/src/lib/services/cloud-sync.ts +++ b/src/lib/services/cloud-sync.ts @@ -111,7 +111,7 @@ interface SyncAttachmentInput { /** Stash format expected by the cloud API */ interface SyncStashInput { id: string; - contextId: string | null; + contextId: string; content: string; enhancedContent: string | null; completed: boolean; @@ -634,7 +634,9 @@ export class CloudSyncService { lastSyncAt: config.lastSyncAt || null, stashes: pushStashes.map(stash => ({ id: stash.id, - contextId: stash.contextId || null, + // Never null: every stash belongs to a context, and the cloud files a + // missing one under the default context just as the backend here does. + contextId: stash.contextId || 'default', content: stash.content, enhancedContent: stash.enhancedContent || null, completed: !!stash.completed, diff --git a/src/lib/types.ts b/src/lib/types.ts index bd9f24f..91f67f9 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -30,7 +30,8 @@ export interface StashItem { attachments: Attachment[]; files?: string[]; // Deprecated, kept for backward compatibility during migration createdAt: string; - contextId?: string; + /** Every stash belongs to a context; "default" holds the ones with no other place. */ + contextId: string; completed?: boolean; completedAt?: string; // ISO Date string updatedAt?: string | number; // ISO Date string (string) or Unix timestamp (number) From cea2490aea6f9121504b7a1d7c07f31ea4edf68b Mon Sep 17 00:00:00 2001 From: Nico Wiedemann Date: Sun, 27 Sep 2026 02:16:34 +0200 Subject: [PATCH 2/3] Stop context export losing files to a name clash, and imports finding none Exporting a context wrote each attachment as <8 chars of stash id>_. Two pasted screenshots in one stash, both image.png, collided on that name; the zip writer refused the second write and the export died part way with the first file written and the dialog left open with nothing said. Files are now keyed by attachment id, which cannot repeat, with the original name kept after it so a particular file can still be found by eye. An export also used to drop any attachment not yet downloaded to this device (synced from elsewhere, background download still pending) while the document went on linking to it - an archive that looked complete and was not. It now refuses instead, names what is missing, and the dialog fetches those files and tries again before giving up with a real error. Separately, importing never found a single attachment: the importer looked for each file under the *new* id an imported stash gets, which is never the id the archive was written with. The markdown now carries the archive path itself, which the importer reads back directly. Both the new attachment-id naming and every older stash-id-prefixed archive are recognised. A copy that fails now rolls back what it had already placed, and both dialogs show an error instead of failing to the console with nothing on screen. Added i18n for the save/pick file-type labels, which were hardcoded English. Rust: 164 tests (new: the name clash, refusing an incomplete archive, older archives still resolving, a copy failure rolling back). Frontend: 219 tests, tsc and svelte-check clean. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 14 + src-tauri/src/transfer.rs | 603 ++++++++++++++++++------- src/lib/components/ExportDialog.svelte | 72 ++- src/lib/components/ImportDialog.svelte | 36 +- src/lib/i18n/locales/de.json | 15 +- src/lib/i18n/locales/en.json | 15 +- src/lib/types.ts | 2 + 7 files changed, 576 insertions(+), 181 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44d354e..371d91c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,20 @@ popover, not for the person who wrote the commit. end up with no context at all. The queue showed it under Default, but clearing completed stashes there, exporting, and the cloud all missed it. Such a stash is now filed under Default everywhere +- **Exporting a context with two attachments of the same name no longer fails silently.** + Two pasted screenshots, both called `image.png`, made the export archive stop after the + first one - and the dialog gave no sign anything had gone wrong. Files are now named by + attachment id inside the archive, so a repeated name no longer collides +- **An export refuses to leave a file out rather than write an incomplete archive.** An + attachment synced from another device but not yet downloaded used to be dropped from the + archive while the document still linked to it. The export now fetches anything missing + and tries again, or says exactly which file it could not include +- **Importing a `.zip` export now actually finds its attachments.** The importer looked for + each file under the *new* id an imported stash gets, which is never the id the archive was + written with, so no attachment from an export ever survived an import. Older exports still + import correctly +- **A failed export or import now shows why, instead of leaving the dialog open with nothing + said.** Both used to fail silently to the console ## [1.8.9] - 2026-09-26 diff --git a/src-tauri/src/transfer.rs b/src-tauri/src/transfer.rs index c5a288d..55a7f08 100644 --- a/src-tauri/src/transfer.rs +++ b/src-tauri/src/transfer.rs @@ -22,7 +22,7 @@ //! Doing it here means the bytes never leave the Rust side, compression runs off the UI //! thread, and an import is a single transaction instead of `2N + M` round trips. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fs; use std::io::{Read, Write}; use std::path::{Path, PathBuf}; @@ -80,6 +80,12 @@ pub struct ImportPreview { /// bad archive quietly lost every creation date. Reporting the count lets the UI say /// so instead. pub unreadable_dates: u32, + /// Attachments the document links to that the archive does not contain. + /// + /// They are skipped on import. Said up front because the old importer skipped them + /// with nothing but a log line, and a stash quietly arriving without its screenshot + /// is found out much later, if at all. + pub missing_attachments: Vec, } #[derive(Debug, Serialize)] @@ -153,14 +159,70 @@ fn parse_heading_date(raw: &str) -> Option> { // Markdown generation // --------------------------------------------------------------------------- -/// Short form of a stash id used to keep attachment names unique inside an archive. -fn stash_prefix(id: &str) -> String { - id.chars().take(8).collect() +/// One attachment as it goes into an archive. +struct ArchivedFile { + /// The attachment's id, or empty for a legacy `files` path, which never had one. + id: String, + /// The name the stash shows it under. + name: String, + /// Where its bytes are on this device. Empty when they never arrived here. + source: String, + /// Its name inside `attachments/`. + entry: String, +} + +/// Name an attachment gets inside an archive: `_`. +/// +/// The id is what keeps it unique. The previous `<8 chars of stash id>_` gave +/// two attachments of one name in one stash - two pasted `image.png`s, the ordinary case - +/// the same entry, the zip writer refused the second, and the export died part way with +/// one file written. The name is kept after the id so someone looking for a particular +/// file in the archive can still find it by name. +fn archive_entry_name(key: &str, file_name: &str) -> String { + let name: String = file_name + .chars() + .map(|c| { + // Separators would make a directory of it, and the rest are refused by + // Windows when someone extracts the archive there. + if matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') || c.is_control() { + '_' + } else { + c + } + }) + .collect(); + let name = if name.trim().is_empty() { + "attachment".to_string() + } else { + name + }; + format!("{}_{}", key, name) } -/// Name an attachment gets inside the archive: `<8 chars of stash id>_`. -fn archive_file_name(stash_id: &str, file_name: &str) -> String { - format!("{}_{}", stash_prefix(stash_id), file_name) +/// Every file a stash refers to, as it will be placed in an archive. +fn archived_files(stash: &StashItem) -> Vec { + let legacy = stash.files.iter().map(|path| { + let name = Path::new(path) + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| path.clone()); + // No id to key on, so a fresh one: it has the same shape, which is what the + // importer recognises and strips. + let entry = archive_entry_name(&Uuid::new_v4().to_string(), &name); + ArchivedFile { + id: String::new(), + name, + source: path.clone(), + entry, + } + }); + let current = stash.attachments.iter().map(|a| ArchivedFile { + id: a.id.clone(), + name: a.file_name.clone(), + source: a.file_path.clone(), + entry: archive_entry_name(&a.id, &a.file_name), + }); + legacy.chain(current).collect() } /// Every attachment name a stash refers to, legacy `files` paths included. @@ -184,11 +246,15 @@ fn stash_file_names(stash: &StashItem) -> Vec { /// Build the markdown document for a set of stashes. /// /// Mirrors the layout the webview produced, so archives stay mutually readable. -pub fn build_markdown( +/// +/// `archived` is what goes into the archive, per stash id, and `None` for a markdown-only +/// export. The links are written from it rather than worked out again here, so a link and +/// the entry it points at cannot disagree. +fn build_markdown( context_name: &str, metadata: &ArchiveMetadata, stashes: &[StashItem], - include_attachments: bool, + archived: Option<&HashMap>>, exported_at: DateTime, ) -> String { let mut out = String::new(); @@ -227,20 +293,26 @@ pub fn build_markdown( out.push_str("\n\n"); } - let names = stash_file_names(stash); - if !names.is_empty() { + let lines: Vec = match archived { + Some(archived) => archived + .get(&stash.id) + .map(|files| { + files + .iter() + .map(|f| format!("- [{}]({}/{})", f.name, ATTACHMENTS_DIR, f.entry)) + .collect() + }) + .unwrap_or_default(), + None => stash_file_names(stash) + .iter() + .map(|name| format!("- {}", name)) + .collect(), + }; + if !lines.is_empty() { out.push_str("**Attachments:**\n"); - for name in &names { - if include_attachments { - out.push_str(&format!( - "- [{}]({}/{})\n", - name, - ATTACHMENTS_DIR, - archive_file_name(&stash.id, name) - )); - } else { - out.push_str(&format!("- {}\n", name)); - } + for line in &lines { + out.push_str(line); + out.push('\n'); } out.push('\n'); } @@ -262,9 +334,24 @@ struct ParsedDocument { unreadable_dates: u32, } -/// Strip the `<8 hex chars>_` prefix the exporter adds, recovering the original name. +/// True for the 36 bytes of a hyphenated UUID. +fn is_uuid(bytes: &[u8]) -> bool { + bytes.len() == 36 + && bytes.iter().enumerate().all(|(i, b)| match i { + 8 | 13 | 18 | 23 => *b == b'-', + _ => b.is_ascii_hexdigit(), + }) +} + +/// Strip the prefix the exporter adds, recovering the original name. +/// +/// Two shapes exist: `_` from now on, and the `<8 hex chars of stash id>_` +/// every earlier archive carries. fn strip_archive_prefix(name: &str) -> String { let bytes = name.as_bytes(); + if bytes.len() > 37 && bytes[36] == b'_' && is_uuid(&bytes[..36]) { + return name[37..].to_string(); + } if bytes.len() > 9 && bytes[8] == b'_' // Compared on the raw bytes. The previous `name[..8].chars()` was safe - the @@ -412,23 +499,38 @@ fn parse_section_header(line: &str) -> Option { Some(kind) } -/// `- [name](attachments/prefixed)` or the plain `- name` form. +/// `- [name](attachments/entry)` or the plain `- name` form. +/// +/// The linked form comes back as its target, `attachments/`, because that is the +/// only thing that finds the file again. Reducing it to the bare name, which this used to +/// do, left the importer rebuilding the entry from the stash's id - and an imported stash +/// has a new id, so not one file of an exported archive was ever found. fn parse_attachment_line(line: &str) -> Option { let rest = line.strip_prefix("- ")?; if let Some(open) = rest.find("](") { if rest.starts_with('[') && rest.ends_with(')') { let target = &rest[open + 2..rest.len() - 1]; - let name = target - .strip_prefix(&format!("{}/", ATTACHMENTS_DIR)) - .unwrap_or(target); - return Some(strip_archive_prefix(name)); + if target.starts_with(&format!("{}/", ATTACHMENTS_DIR)) { + return Some(target.to_string()); + } + return Some(strip_archive_prefix(target)); } } Some(strip_archive_prefix(rest)) } +/// Where a parsed reference's bytes should be in an extracted archive, and its real name. +/// +/// `None` for the plain `- name` form: an archive exported without its attachments lists +/// them by name only, and there is nothing to look for. +fn archived_reference(extract_dir: &Path, reference: &str) -> Option<(PathBuf, String)> { + let entry = reference.strip_prefix(&format!("{}/", ATTACHMENTS_DIR))?; + let source = safe_entry_path(&extract_dir.join(ATTACHMENTS_DIR), entry)?; + Some((source, strip_archive_prefix(entry))) +} + // --------------------------------------------------------------------------- // Duplicate detection // --------------------------------------------------------------------------- @@ -546,49 +648,63 @@ pub async fn export_context_archive( .unwrap_or_default(), }; - // Only real files count: a row whose path is empty has no bytes on this device. - let attachment_sources: Vec<(String, String)> = if include_attachments { + let archived: HashMap> = if include_attachments { stashes .iter() - .flat_map(|s| { - let stash_id = s.id.clone(); - let legacy = s.files.iter().cloned().map(move |p| { - let name = Path::new(&p) - .file_name() - .map(|n| n.to_string_lossy().into_owned()) - .unwrap_or_else(|| p.clone()); - (p, name) - }); - let stash_id2 = stash_id.clone(); - let current = s - .attachments - .iter() - .filter(|a| !a.file_path.trim().is_empty()) - .map(move |a| (a.file_path.clone(), a.file_name.clone())); - - legacy - .map(move |(p, n)| (stash_id.clone(), p, n)) - .chain(current.map(move |(p, n)| (stash_id2.clone(), p, n))) - }) - .filter(|(_, path, _)| Path::new(path).exists()) - .map(|(stash_id, path, name)| (path, archive_file_name(&stash_id, &name))) + .map(|s| (s.id.clone(), archived_files(s))) + .filter(|(_, files)| !files.is_empty()) .collect() } else { - Vec::new() + HashMap::new() }; + // An archive that says it holds a context's attachments has to hold all of them. This + // used to drop whatever was not on disk and still link it from the document, so the + // archive looked complete and was not. Refused instead, naming the files, with the ids + // the interface needs to fetch them from the cloud and try again. + let in_order = || stashes.iter().filter_map(|s| archived.get(&s.id)).flatten(); + let missing: Vec<&ArchivedFile> = in_order() + .filter(|f| f.source.trim().is_empty() || !Path::new(&f.source).exists()) + .collect(); + if !missing.is_empty() { + let names = missing + .iter() + .map(|f| f.name.as_str()) + .collect::>() + .join(", "); + let ids = missing + .iter() + .filter(|f| !f.id.is_empty()) + .map(|f| f.id.as_str()) + .collect::>() + .join(","); + return Err(UiError::with_values( + "transfer.attachments_missing", + format!("These attachments are not on this device: {}", names), + [ + ("count", missing.len().to_string()), + ("names", names), + ("ids", ids), + ], + )); + } + let markdown = build_markdown( &context_name, &metadata, &stashes, - include_attachments && !attachment_sources.is_empty(), + (!archived.is_empty()).then_some(&archived), Utc::now(), ); let dest = PathBuf::from(&dest_path); - let attachment_count = attachment_sources.len() as u32; + let attachment_count = archived.values().map(Vec::len).sum::() as u32; let stash_count = stashes.len() as u32; + let files: Vec<(String, String)> = in_order() + .map(|f| (f.source.clone(), format!("{}/{}", ATTACHMENTS_DIR, f.entry))) + .collect(); + // Deflating every attachment in a context is the single heaviest thing this app // does, so it belongs on the blocking pool rather than an async worker. Tokio does // not migrate a task that blocks its worker, so exporting a large context inline @@ -599,38 +715,26 @@ pub async fn export_context_archive( fs::create_dir_all(parent).map_err(|e| format!("Failed to create folder: {}", e))?; } - if attachment_sources.is_empty() { + if files.is_empty() { fs::write(&write_dest, markdown) .map_err(|e| format!("Failed to write export: {}", e))?; return Ok(()); } - let file = - fs::File::create(&write_dest).map_err(|e| format!("Failed to write export: {}", e))?; - let mut zip = zip::ZipWriter::new(file); - let options: zip::write::FileOptions<'_, ()> = - zip::write::FileOptions::default().compression_method(zip::CompressionMethod::Deflated); - - zip.start_file(MARKDOWN_ENTRY, options) - .map_err(|e| e.to_string())?; - zip.write_all(markdown.as_bytes()) - .map_err(|e| e.to_string())?; + // Built beside the destination and renamed into place, so a failure part way + // leaves no half an archive under the name the user chose. + let mut partial = write_dest.clone().into_os_string(); + partial.push(".partial"); + let partial = PathBuf::from(partial); - for (source, name) in &attachment_sources { - let bytes = match fs::read(source) { - Ok(b) => b, - Err(e) => { - log::warn!("[Export] skipping {}: {}", source, e); - continue; - } - }; - zip.start_file(format!("{}/{}", ATTACHMENTS_DIR, name), options) - .map_err(|e| e.to_string())?; - zip.write_all(&bytes).map_err(|e| e.to_string())?; + let written = write_zip(&partial, &markdown, &files).and_then(|()| { + fs::rename(&partial, &write_dest) + .map_err(|e| format!("Failed to write export: {}", e).into()) + }); + if written.is_err() { + let _ = fs::remove_file(&partial); } - - zip.finish().map_err(|e| e.to_string())?; - Ok(()) + written }) .await .map_err(|e| format!("Export task failed: {}", e))??; @@ -642,6 +746,33 @@ pub async fn export_context_archive( }) } +/// Write an export archive to `path`. +/// +/// Every file it is given has to make it in, so a read that fails ends the export rather +/// than leaving a gap the document still links to. +fn write_zip(path: &Path, markdown: &str, files: &[(String, String)]) -> Result<(), UiError> { + let file = fs::File::create(path).map_err(|e| format!("Failed to write export: {}", e))?; + let mut zip = zip::ZipWriter::new(file); + let options: zip::write::FileOptions<'_, ()> = + zip::write::FileOptions::default().compression_method(zip::CompressionMethod::Deflated); + + zip.start_file(MARKDOWN_ENTRY, options) + .map_err(|e| e.to_string())?; + zip.write_all(markdown.as_bytes()) + .map_err(|e| e.to_string())?; + + for (source, entry) in files { + let mut src = + fs::File::open(source).map_err(|e| format!("Failed to read {}: {}", source, e))?; + zip.start_file(entry.as_str(), options) + .map_err(|e| e.to_string())?; + std::io::copy(&mut src, &mut zip).map_err(|e| format!("Failed to read {}: {}", source, e))?; + } + + zip.finish().map_err(|e| e.to_string())?; + Ok(()) +} + /// Read an archive and report what importing it would bring in. #[tauri::command] pub async fn read_import_archive( @@ -689,12 +820,22 @@ pub async fn read_import_archive( let duplicate_ids = find_duplicates(&parsed.stashes, &existing); + let missing_attachments = parsed + .stashes + .iter() + .flat_map(|s| s.files.iter()) + .filter_map(|reference| archived_reference(&temp_dir, reference)) + .filter(|(source, _)| !source.is_file()) + .map(|(_, name)| name) + .collect(); + Ok(ImportPreview { stashes: parsed.stashes, metadata: parsed.metadata, duplicate_ids, token, unreadable_dates: parsed.unreadable_dates, + missing_attachments, }) } @@ -755,71 +896,18 @@ pub async fn commit_import( // attachments, and every byte of that was previously copied on an async worker. let copy_context = context_id.clone(); let copy_temp = temp_dir.clone(); - let prepared: Vec = tauri::async_runtime::spawn_blocking( - move || -> Result, UiError> { - let mut prepared: Vec = Vec::with_capacity(stashes.len()); - - for mut stash in stashes { - stash.context_id = copy_context.clone(); - - // Copy each referenced file out of the extraction directory and into the - // stash's own cache folder, building the attachment rows as we go. - let mut attachments: Vec = Vec::new(); - if !stash.files.is_empty() { - let target_dir = get_stash_cache_path(&stash.id, Some(©_context)); - fs::create_dir_all(&target_dir) - .map_err(|e| format!("Failed to create attachment folder: {}", e))?; - - for name in &stash.files { - let archived = archive_file_name(&stash.id, name); - let candidates = [ - copy_temp.join(ATTACHMENTS_DIR).join(&archived), - copy_temp.join(ATTACHMENTS_DIR).join(name), - ]; - - let Some(source) = candidates.iter().find(|p| p.exists()) else { - log::warn!("[Import] {} is referenced but not in the archive", name); - continue; - }; - - // Reserved rather than joined: two attachments of one name in - // the same stash used to land on the same path, so the second - // copy replaced the first one's bytes and both rows pointed at - // the survivor. - let dest = match crate::utils::reserve_unique_path(&target_dir, name) { - Ok(dest) => dest, - Err(e) => { - log::warn!("[Import] could not place {}: {}", name, e); - continue; - } - }; - if let Err(e) = fs::copy(source, &dest) { - let _ = fs::remove_file(&dest); - log::warn!("[Import] could not place {}: {}", name, e); - continue; - } - - let size = fs::metadata(&dest).map(|m| m.len()).unwrap_or(0) as i64; - attachments.push(Attachment { - id: Uuid::new_v4().to_string(), - stash_id: stash.id.clone(), - file_path: dest.to_string_lossy().into_owned(), - file_name: name.clone(), - file_size: size, - mime_type: mime_guess::from_path(&dest).first().map(|m| m.to_string()), - syntax: None, - created_at: Utc::now().to_rfc3339(), - }); - } + let (prepared, placed) = tauri::async_runtime::spawn_blocking( + move || -> Result<(Vec, Vec), UiError> { + // Every file placed so far, so a failure can take them back out. Otherwise a + // half-finished import leaves files in the cache that no row points at. + let mut placed: Vec = Vec::new(); + match place_import_files(stashes, ©_context, ©_temp, &mut placed) { + Ok(prepared) => Ok((prepared, placed)), + Err(e) => { + remove_all(&placed); + Err(e) } - - // The legacy `files` column is not carried forward; attachments replace it. - stash.files = Vec::new(); - stash.attachments = attachments; - prepared.push(stash); } - - Ok(prepared) }, ) .await @@ -828,10 +916,11 @@ pub async fn commit_import( // One transaction for the lot, rather than the two-commands-per-stash-plus-one-per-file // the webview used to issue. insert_local_stashes, not import_stashes: these records // are new on this device and have to reach the cloud, so they stay pending. - state - .lock_db() - .insert_local_stashes(&prepared) - .map_err(|e| e.to_string())?; + let inserted = state.lock_db().insert_local_stashes(&prepared); + if let Err(e) = inserted { + remove_all(&placed); + return Err(e.to_string().into()); + } let cleanup_dir = temp_dir.clone(); let _ = tauri::async_runtime::spawn_blocking(move || fs::remove_dir_all(&cleanup_dir)).await; @@ -839,6 +928,79 @@ pub async fn commit_import( Ok(prepared.len() as u32) } +/// Take back files an import placed before it failed. +fn remove_all(paths: &[PathBuf]) { + for path in paths { + let _ = fs::remove_file(path); + } +} + +/// Copy each stash's files out of the extraction directory into its own cache folder, +/// building the attachment rows as it goes. +/// +/// A file the archive does not contain is skipped - the preview already named it. A file +/// that is there and cannot be copied ends the import instead: it is an error on this +/// machine, and importing the rest without it would lose it without a word. +fn place_import_files( + stashes: Vec, + context_id: &str, + extract_dir: &Path, + placed: &mut Vec, +) -> Result, UiError> { + let mut prepared: Vec = Vec::with_capacity(stashes.len()); + + for mut stash in stashes { + stash.context_id = context_id.to_string(); + + let mut attachments: Vec = Vec::new(); + let references: Vec<(PathBuf, String)> = stash + .files + .iter() + .filter_map(|reference| archived_reference(extract_dir, reference)) + .collect(); + + if !references.is_empty() { + let target_dir = get_stash_cache_path(&stash.id, Some(context_id)); + fs::create_dir_all(&target_dir) + .map_err(|e| format!("Failed to create attachment folder: {}", e))?; + + for (source, name) in &references { + if !source.is_file() { + log::warn!("[Import] {} is referenced but not in the archive", name); + continue; + } + + // Reserved rather than joined: two attachments of one name in the same + // stash used to land on the same path, so the second copy replaced the + // first one's bytes and both rows pointed at the survivor. + let dest = crate::utils::reserve_unique_path(&target_dir, name) + .map_err(|e| format!("Could not place {}: {}", name, e))?; + placed.push(dest.clone()); + fs::copy(source, &dest).map_err(|e| format!("Could not place {}: {}", name, e))?; + + let size = fs::metadata(&dest).map(|m| m.len()).unwrap_or(0) as i64; + attachments.push(Attachment { + id: Uuid::new_v4().to_string(), + stash_id: stash.id.clone(), + file_path: dest.to_string_lossy().into_owned(), + file_name: name.clone(), + file_size: size, + mime_type: mime_guess::from_path(&dest).first().map(|m| m.to_string()), + syntax: None, + created_at: Utc::now().to_rfc3339(), + }); + } + } + + // The legacy `files` column is not carried forward; attachments replace it. + stash.files = Vec::new(); + stash.attachments = attachments; + prepared.push(stash); + } + + Ok(prepared) +} + /// Drop the files an aborted import had extracted. #[tauri::command] pub async fn discard_import(token: String) -> Result<(), UiError> { @@ -885,7 +1047,7 @@ mod tests { stash("b", "second entry", "2026-08-17T09:30:00Z", true), ]; - let md = build_markdown("Work", &metadata(), &stashes, false, Utc::now()); + let md = build_markdown("Work", &metadata(), &stashes, None, Utc::now()); let parsed = parse_markdown(&md, "ctx"); assert_eq!(parsed.unreadable_dates, 0); @@ -901,7 +1063,6 @@ mod tests { assert!(done.created_at.starts_with("2026-08-17T09:30:00")); } - #[test] /// Non-ASCII names are left intact rather than mistaken for a prefixed one. #[test] fn a_non_ascii_attachment_name_keeps_its_leading_characters() { @@ -913,27 +1074,147 @@ mod tests { assert_eq!(strip_archive_prefix("abcdef12_shot.png"), "shot.png"); // Eight characters that are not hex are left alone. assert_eq!(strip_archive_prefix("zzzzzzzz_shot.png"), "zzzzzzzz_shot.png"); + // The attachment-id prefix archives are written with now. + assert_eq!( + strip_archive_prefix("0cdf01cd-f5be-49a2-840b-cd1d12f43a42_shot.png"), + "shot.png" + ); + // Something UUID-length that is not one keeps its name. + assert_eq!( + strip_archive_prefix("zzzzzzzz-f5be-49a2-840b-cd1d12f43a42_shot.png"), + "zzzzzzzz-f5be-49a2-840b-cd1d12f43a42_shot.png" + ); } - #[test] - fn attachment_names_round_trip_without_their_archive_prefix() { - let mut item = stash("abcdef12", "has a file", "2026-08-18T10:00:00Z", false); - item.attachments.push(Attachment { - id: "att".into(), - stash_id: "abcdef12".into(), - file_path: "/cache/ctx/abcdef12/shot.png".into(), - file_name: "shot.png".into(), + fn attachment(id: &str, stash_id: &str, name: &str) -> Attachment { + Attachment { + id: id.into(), + stash_id: stash_id.into(), + file_path: format!("/cache/ctx/{}/{}", stash_id, name), + file_name: name.into(), file_size: 1, mime_type: None, syntax: None, created_at: "2026-08-18T10:00:00Z".into(), - }); + } + } + + const ATT_A: &str = "11111111-2222-4333-8444-555555555555"; + const ATT_B: &str = "66666666-7777-4888-8999-aaaaaaaaaaaa"; - let md = build_markdown("Work", &metadata(), &[item], true, Utc::now()); - assert!(md.contains("attachments/abcdef12_shot.png")); + fn archived_for(stashes: &[StashItem]) -> HashMap> { + stashes + .iter() + .map(|s| (s.id.clone(), archived_files(s))) + .collect() + } + #[test] + fn attachment_names_round_trip_without_their_archive_prefix() { + let mut item = stash("abcdef12", "has a file", "2026-08-18T10:00:00Z", false); + item.attachments.push(attachment(ATT_A, "abcdef12", "shot.png")); + + let items = [item]; + let md = build_markdown("Work", &metadata(), &items, Some(&archived_for(&items)), Utc::now()); + let entry = format!("attachments/{}_shot.png", ATT_A); + assert!(md.contains(&format!("- [shot.png]({})", entry)), "{}", md); + + // The importer keeps the link target, because the imported stash has a new id and + // the target is the only thing that finds the file. let parsed = parse_markdown(&md, "ctx"); - assert_eq!(parsed.stashes[0].files, vec!["shot.png".to_string()]); + assert_eq!(parsed.stashes[0].files, vec![entry.clone()]); + + let (source, name) = archived_reference(Path::new("/tmp/import"), &entry).unwrap(); + assert_eq!(name, "shot.png"); + assert_eq!(source, Path::new("/tmp/import/attachments").join(format!("{}_shot.png", ATT_A))); + } + + /// Two pasted `image.png`s in one stash used to share an entry, and the zip writer + /// refused the second one - ending the export with one file written. + #[test] + fn two_attachments_of_one_name_get_their_own_entries() { + let mut item = stash("b53d26f6", "two images", "2026-09-25T12:49:10Z", false); + item.attachments.push(attachment(ATT_A, "b53d26f6", "image.png")); + item.attachments.push(attachment(ATT_B, "b53d26f6", "image.png")); + + let files = archived_files(&item); + assert_eq!(files.len(), 2); + assert_ne!(files[0].entry, files[1].entry); + assert!(files.iter().all(|f| f.entry.ends_with("_image.png"))); + } + + #[test] + fn an_entry_name_cannot_make_a_directory() { + assert_eq!(archive_entry_name(ATT_A, "../a/b\\c.png"), format!("{}_.._a_b_c.png", ATT_A)); + assert_eq!(archive_entry_name(ATT_A, " "), format!("{}_attachment", ATT_A)); + } + + /// Archives written before this change link `<8 chars of stash id>_`. + #[test] + fn attachments_in_older_archives_are_still_found() { + let md = concat!( + "## Active Stashes (1) + +### 2026-09-25 12:49:10 + +old + +", + "**Attachments:** +- [image.png](attachments/b53d26f6_image.png) + +--- +", + ); + let parsed = parse_markdown(md, "ctx"); + let reference = &parsed.stashes[0].files[0]; + + let (source, name) = archived_reference(Path::new("/x"), reference).unwrap(); + assert_eq!(name, "image.png"); + assert!(source.ends_with("b53d26f6_image.png")); + } + + /// The name-only list an export without attachments writes has nothing to look for. + #[test] + fn a_name_only_reference_is_not_expected_in_the_archive() { + assert!(archived_reference(Path::new("/x"), "shot.png").is_none()); + } + + #[test] + fn the_archive_holds_every_file_it_links() { + let dir = std::env::temp_dir().join(format!("stashpad-export-{}", Uuid::new_v4())); + fs::create_dir_all(&dir).unwrap(); + let first = dir.join("one.png"); + let second = dir.join("two.png"); + fs::write(&first, b"first").unwrap(); + fs::write(&second, b"second").unwrap(); + + let files = vec![ + (first.to_string_lossy().into_owned(), format!("attachments/{}_image.png", ATT_A)), + (second.to_string_lossy().into_owned(), format!("attachments/{}_image.png", ATT_B)), + ]; + let out = dir.join("export.zip"); + write_zip(&out, "# doc", &files).unwrap(); + + let mut zip = zip::ZipArchive::new(fs::File::open(&out).unwrap()).unwrap(); + assert_eq!(zip.len(), 3); + let mut body = String::new(); + zip.by_name(&files[1].1).unwrap().read_to_string(&mut body).unwrap(); + assert_eq!(body, "second"); + + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn a_file_that_cannot_be_read_fails_the_archive() { + let dir = std::env::temp_dir().join(format!("stashpad-export-{}", Uuid::new_v4())); + fs::create_dir_all(&dir).unwrap(); + let files = vec![( + dir.join("gone.png").to_string_lossy().into_owned(), + "attachments/x_gone.png".to_string(), + )]; + assert!(write_zip(&dir.join("export.zip"), "# doc", &files).is_err()); + let _ = fs::remove_dir_all(&dir); } #[test] diff --git a/src/lib/components/ExportDialog.svelte b/src/lib/components/ExportDialog.svelte index 32b0b8b..cd5b0e1 100644 --- a/src/lib/components/ExportDialog.svelte +++ b/src/lib/components/ExportDialog.svelte @@ -10,6 +10,8 @@ import { save } from "@tauri-apps/plugin-dialog"; import { stat } from "@tauri-apps/plugin-fs"; import { DesktopStorageAdapter } from "$lib/services/desktop-adapter"; + import { attachmentSync } from "$lib/stores/attachment-sync.svelte"; + import { errorCode, errorText } from "$lib/errors"; import type { Context, StashItem } from "$lib/types"; import { getRelativeTime } from "$lib/utils/date"; import { formatBytes } from "$lib/utils/format"; @@ -33,6 +35,10 @@ let selectedIds = $state>(new Set()); let includeAttachments = $state(false); let isExporting = $state(false); + /** Fetching attachments that are not on this device yet, before exporting again. */ + let isDownloading = $state(false); + /** Why the export failed. It used to reach only the console, with the dialog left open. */ + let errorMessage = $state(""); let totalAttachmentSize = $state(0); let isCalculatingSize = $state(false); @@ -47,6 +53,8 @@ ); includeAttachments = false; isExporting = false; + isDownloading = false; + errorMessage = ""; } else if (hasOpened) { // Ensure parent state is synchronized when dialog is closed/dismissed // via internal mechanisms (Esc key, backdrop click) @@ -193,28 +201,49 @@ title: $_("contexts.exportTitle"), defaultPath: defaultFileName, filters: asZip - ? [{ name: "ZIP Archive", extensions: ["zip"] }] - : [{ name: "Markdown", extensions: ["md"] }], + ? [{ name: $_("contexts.exportDialog.zipFilter"), extensions: ["zip"] }] + : [{ name: $_("contexts.exportDialog.markdownFilter"), extensions: ["md"] }], }); if (!filePath) return; isExporting = true; + errorMessage = ""; + const run = () => + adapter.exportContextArchive(context.id, [...selectedIds], asZip, filePath); try { - await adapter.exportContextArchive( - context.id, - [...selectedIds], - asZip, - filePath, - ); + try { + await run(); + } catch (e) { + // Attachments synced from another device arrive as details only, and their + // bytes download in the background. The archive has to hold every file, so + // Rust refuses while any is missing and names them; fetch those now and try + // once more. Legacy files carry no id and cannot be fetched, so an error + // without ids is shown as it is. + const ids = missingAttachmentIds(e); + if (ids.length === 0) throw e; + isDownloading = true; + await Promise.all(ids.map((id) => attachmentSync.request(id, ""))); + isDownloading = false; + await run(); + } handleClose(); } catch (e) { console.error("Export failed:", e); + errorMessage = errorText(e, $_("contexts.exportDialog.failed")); } finally { isExporting = false; + isDownloading = false; } } + /** The attachments an export was refused for, when that is why it was refused. */ + function missingAttachmentIds(error: unknown): string[] { + if (errorCode(error) !== "transfer.attachments_missing") return []; + const ids = (error as { values?: Record }).values?.ids ?? ""; + return ids.split(",").filter((id) => id.length > 0); + } + const uid = $props.id(); const titleId = `${uid}-title`; @@ -234,6 +263,11 @@ } } + /** Files a stash carries, legacy paths included. Counting `files` alone missed every current one. */ + function attachmentCount(stash: StashItem): number { + return (stash.files?.length || 0) + (stash.attachments?.length || 0); + } + /** * Get preview text for a stash (truncated) */ @@ -339,11 +373,11 @@ $_, )} - {#if stash.files && stash.files.length > 0} + {#if attachmentCount(stash) > 0} - 📎{stash.files.length} + 📎{attachmentCount(stash)} {/if} @@ -394,11 +428,11 @@ $_, )} - {#if stash.files && stash.files.length > 0} + {#if attachmentCount(stash) > 0} - 📎{stash.files.length} + 📎{attachmentCount(stash)} {/if} @@ -484,6 +518,12 @@ {/if} + {#if errorMessage} + + {/if} +
{#if isCalculatingSize} @@ -516,9 +556,11 @@ disabled={selectedIds.size === 0 || isExporting} > - {isExporting - ? $_("common.loading") - : $_("contexts.exportDialog.export")} + {isDownloading + ? $_("contexts.exportDialog.downloading") + : isExporting + ? $_("common.loading") + : $_("contexts.exportDialog.export")}
diff --git a/src/lib/components/ImportDialog.svelte b/src/lib/components/ImportDialog.svelte index d35c1f3..f33e500 100644 --- a/src/lib/components/ImportDialog.svelte +++ b/src/lib/components/ImportDialog.svelte @@ -18,6 +18,7 @@ } from "$lib/types"; import { DesktopStorageAdapter } from "$lib/services/desktop-adapter"; import { getRelativeTime } from "$lib/utils/date"; + import { errorText } from "$lib/errors"; import { Upload, FileText, @@ -58,6 +59,10 @@ let importToken = $state(null); /** Headings whose date could not be read; those stashes fall back to now. */ let unreadableDates = $state(0); + /** Attachments the document links to that the archive does not hold. */ + let missingAttachments = $state([]); + /** Why reading or importing failed. Both used to reach only the console. */ + let errorMessage = $state(""); let isImporting = $state(false); let isParsing = $state(false); let importedFileName = $state(""); @@ -91,6 +96,8 @@ duplicateIds = new Set(); importToken = null; unreadableDates = 0; + missingAttachments = []; + errorMessage = ""; isImporting = false; isParsing = false; importedFileName = ""; @@ -194,7 +201,7 @@ title: $_("contexts.importDialog.selectFile"), filters: [ { - name: "Stashpad Export", + name: $_("contexts.importDialog.fileFilter"), extensions: ["md", "zip"], }, ], @@ -210,6 +217,7 @@ */ async function loadFromPath(filePath: string) { isParsing = true; + errorMessage = ""; try { const fileName = filePath.split(/[\\/]/).pop() || ""; importedFileName = fileName; @@ -223,6 +231,7 @@ duplicateIds = new Set(preview.duplicateIds); importToken = preview.token; unreadableDates = preview.unreadableDates; + missingAttachments = preview.missingAttachments ?? []; const metadata: Metadata | undefined = preview.metadata ? { @@ -264,6 +273,7 @@ step = "preview"; } catch (e) { console.error("Failed to parse file:", e); + errorMessage = errorText(e, $_("contexts.importDialog.readFailed")); } finally { isParsing = false; } @@ -369,6 +379,7 @@ if (selectedIds.size === 0 || !importToken) return; isImporting = true; + errorMessage = ""; try { // The conflict dialog may have edited the context in memory. if (importedMetadata) { @@ -382,6 +393,7 @@ handleClose(); } catch (e) { console.error("Import failed:", e); + errorMessage = errorText(e, $_("contexts.importDialog.importFailed")); } finally { isImporting = false; } @@ -523,6 +535,11 @@

{$_("contexts.importDialog.dropToImport")}

+ {#if errorMessage} + + {/if} {:else} @@ -738,6 +755,23 @@ {/if} + {#if missingAttachments.length > 0} +
+ {$_("contexts.importDialog.missingAttachments", { + values: { + count: missingAttachments.length, + names: missingAttachments.join(", "), + }, + })} +
+ {/if} + + {#if errorMessage} + + {/if} +