Skip to content

Commit dbd7a2a

Browse files
committed
Bugfix: error dialogs render OS strings verbatim
- Raw OS messages like `STATUS_DELETE_PENDING during Create` used to render in error dialogs as `STATUS<em>DELETE</em>PENDING` because `format!()` baked the underscores straight into snarkdown's input. - New `Markdown` newtype + `md!` macro in `friendly_error/markdown.rs`: templates are trusted markdown; positional `{}` args go through the `MarkdownArg` trait which escapes plain strings, paths, and numbers, and passes `Markdown` values through unescaped. - HTML-entity escape strategy (`&#95;` etc.), not CommonMark `\_`. snarkdown doesn't honor backslash escapes — `\_` would render literally — so we encode and let the browser decode at `{@html}` time. - `FriendlyError.{explanation,suggestion}` re-typed `String -> Markdown`; `#[serde(transparent)]` keeps the wire format unchanged. - FE: bindings.ts post-processed in the regen test to emit `Markdown = string & { readonly __markdown: unique symbol }`. The sole `renderErrorMarkdown` call now rejects plain strings at compile time. - ~30 call sites migrated across `friendly_error/`, `provider.rs`, `git/friendly.rs`. Captured-identifier `{name}` syntax replaced with positional `{}` (the macro can't escape captured idents). - Conservative escape set: `.`, `-`, `+`, `#`, `|` left alone so paths and provider names like `Sync.com` render naturally. - Regression test `io_serious_escapes_message_markdown_specials` pins the bug from the screenshot.
1 parent 0ae2ee9 commit dbd7a2a

19 files changed

Lines changed: 808 additions & 453 deletions

apps/desktop/src-tauri/src/file_system/git/friendly.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,10 @@ impl FriendlyGitError {
234234
FriendlyError {
235235
category: self.kind.category(),
236236
title: self.kind.title().to_string(),
237-
explanation: self.kind.explanation().to_string(),
238-
suggestion: self.kind.suggestion().to_string(),
237+
// Git friendly-error copy is hand-authored markdown (sometimes
238+
// contains `code` spans and **bold**); wrap as literal.
239+
explanation: crate::file_system::volume::friendly_error::Markdown::literal(self.kind.explanation()),
240+
suggestion: crate::file_system::volume::friendly_error::Markdown::literal(self.kind.suggestion()),
239241
raw_detail,
240242
retry_hint: matches!(self.kind.category(), ErrorCategory::Transient),
241243
action_kind: None,
@@ -351,8 +353,8 @@ mod tests {
351353
] {
352354
let f = FriendlyGitError::new(kind, "/some/path").to_friendly_error();
353355
never_says_error_or_failed(&f.title);
354-
never_says_error_or_failed(&f.explanation);
355-
never_says_error_or_failed(&f.suggestion);
356+
never_says_error_or_failed(f.explanation.as_str());
357+
never_says_error_or_failed(f.suggestion.as_str());
356358
}
357359
}
358360
}

apps/desktop/src-tauri/src/file_system/volume/CLAUDE.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,12 @@ When you need to handle a new errno or `VolumeError` variant:
264264
2. Pick the right `ErrorCategory`: **Transient** (retry might work), **NeedsAction** (user must do something),
265265
**Serious** (something is genuinely broken)
266266
3. Write the message following the rules below
267-
4. Add a unit test asserting the category and that the text follows the style rules
268-
5. Run the existing `error_messages_never_contain_error_or_failed` test to catch violations
267+
4. Build `explanation` / `suggestion` with the `md!(...)` macro (see `friendly_error/markdown.rs`). Templates are
268+
trusted markdown; positional `{}` args route through `MarkdownArg::render_arg` which escapes plain strings
269+
(paths, OS messages, names) and passes a `Markdown` value through unescaped. **Use positional `{}` only**
270+
captured-identifier syntax (`md!("foo {bar}")`) bypasses escaping and renders the literal `{bar}` in the UI.
271+
5. Add a unit test asserting the category and that the text follows the style rules
272+
6. Run the existing `error_messages_never_contain_error_or_failed` test to catch violations
269273

270274
### Adding a new provider
271275

@@ -362,6 +366,9 @@ The hook order is fixed: `resolve()` first (normalizes the path), then `try_rout
362366
**Decision**: Friendly error mapping lives in Rust, not the frontend
363367
**Why**: The mapping needs access to the full path (for provider detection) and platform-specific errno codes. Doing it in Rust keeps the frontend thin (principle: smart backend, thin frontend) and avoids duplicating errno knowledge in TypeScript. The frontend receives a ready-to-render `FriendlyError` struct with markdown strings.
364368

369+
**Decision**: `FriendlyError.explanation` / `.suggestion` are typed `Markdown`, not `String`; built via the `md!` macro
370+
**Why**: Raw OS messages and provider names contain markdown specials (the bug was `STATUS_DELETE_PENDING` rendering as italics because `format!()` baked the underscores straight into the explanation). The `Markdown` newtype + `md!` macro escape every interpolated runtime value via the `MarkdownArg` trait while leaving the trusted template literal alone. `#[serde(transparent)]` keeps the wire format identical to the old `String`, and the FE bindings.ts post-processing brands the type as `string & { readonly __markdown: unique symbol }` so the single `renderErrorMarkdown` call site only accepts wire-supplied markdown values. See `friendly_error/markdown.rs` for the macro, the trait, the HTML-entity escape strategy (snarkdown doesn't honor CommonMark `\` escapes, so `\_` would render literally — we emit `&#95;` instead, which snarkdown ignores and the browser decodes), the conservative escape set (line-start chars like `.` / `-` / `#` left alone so paths render naturally), and the captured-identifier footgun warning.
371+
365372
**Decision**: `LocalPosixVolume` uses `symlink_metadata` for `exists()` instead of `Path::exists()`
366373
**Why**: `Path::exists()` follows symlinks. A dangling symlink returns `false`, which would make the volume claim a file doesn't exist when it visibly does in a directory listing. `symlink_metadata` detects the symlink itself, matching what the user sees.
367374

apps/desktop/src-tauri/src/file_system/volume/friendly_error/empty_root.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use std::path::Path;
1010

1111
use super::{ErrorCategory, FriendlyError};
12+
use crate::md;
1213

1314
/// Returns a friendly hint when a directory at a TCC-sensitive volume root listed
1415
/// successfully but came back empty.
@@ -25,16 +26,16 @@ pub fn friendly_error_for_restricted_empty_root(volume_id: &str, path: &Path) ->
2526
Some(FriendlyError {
2627
category: ErrorCategory::NeedsAction,
2728
title: "iCloud Drive looks empty".into(),
28-
explanation: "Cmdr opened iCloud Drive but it came back with no files. macOS hides iCloud Drive contents \
29+
explanation: md!(
30+
"Cmdr opened iCloud Drive but it came back with no files. macOS hides iCloud Drive contents \
2931
from apps that don't have **Full Disk Access**, so granting Cmdr that permission is the most \
3032
likely fix.\n\nIf your iCloud Drive really is empty, you can ignore this hint."
31-
.into(),
32-
suggestion: "Here's what to try:\n\
33+
),
34+
suggestion: md!("Here's what to try:\n\
3335
- Open [**System Settings > Privacy & Security**](x-apple.systempreferences:com.apple.preference.security?Privacy) and pick **Full Disk Access**\n\
3436
- Add Cmdr (use the **+** button) and toggle it on\n\
3537
- Quit and reopen Cmdr\n\
36-
- Come back here to retry"
37-
.into(),
38+
- Come back here to retry"),
3839
raw_detail: format!("volume={volume_id}, path={}, entries=0", path.display()),
3940
retry_hint: true,
4041
action_kind: Some(super::ErrorActionKind::OpenPrivacySettings),

0 commit comments

Comments
 (0)