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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/daemon/src/chimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ mod tests {
error: None,
}],
mutation_id: None,
undo_unavailable: false,
}
}
}
5 changes: 5 additions & 0 deletions crates/daemon/src/commands/mutations/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,11 @@ fn print_mutation_result_output(
println!("{}", counts.changed_line(result.succeeded as usize));
if let Some(mutation_id) = result.mutation_id.as_deref() {
println!("Undo with: mxr undo {mutation_id}");
} else if result.undo_unavailable {
eprintln!(
"Note: undo is unavailable for this mutation — recording the undo entry failed, \
so it can't be reversed with `mxr undo`."
);
}
}
OutputFormat::Json => println!(
Expand Down
18 changes: 12 additions & 6 deletions crates/daemon/src/handler/mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ pub(super) async fn mutation(
failed: 0,
accounts,
mutation_id: None,
undo_unavailable: false,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: merge_mutation_result does not propagate undo_unavailable from chunk to aggregate, so the flag is silently lost in the background job (run_mutation_job) path. A chunk that returns undo_unavailable: true will have that bit dropped, and the final job.result will always show false — defeating the feature for any multi-chunk or even single-chunk job.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/daemon/src/handler/mutations.rs, line 549:

<comment>`merge_mutation_result` does not propagate `undo_unavailable` from chunk to aggregate, so the flag is silently lost in the background job (`run_mutation_job`) path. A chunk that returns `undo_unavailable: true` will have that bit dropped, and the final `job.result` will always show `false` — defeating the feature for any multi-chunk or even single-chunk job.</comment>

<file context>
@@ -546,6 +546,7 @@ pub(super) async fn mutation(
                 failed: 0,
                 accounts,
                 mutation_id: None,
+                undo_unavailable: false,
             },
         });
</file context>

},
});
}
Expand Down Expand Up @@ -637,7 +638,7 @@ pub(super) async fn mutation(
// Persist an undo entry only for undoable kinds and only if at
// least one envelope succeeded. The mutation_id is the same one
// used for dedup above; undo + dedup share a key by design.
let mutation_id = match (undoable_kind, succeeded_snapshots.is_empty()) {
let (mutation_id, undo_unavailable) = match (undoable_kind, succeeded_snapshots.is_empty()) {
(Some(kind), false) => {
let now = chrono::Utc::now().timestamp();
let entry = UndoEntry {
Expand All @@ -648,15 +649,16 @@ pub(super) async fn mutation(
expires_at: now + UNDO_WINDOW_SECS,
};
if let Err(error) = state.store.write_undo_entry(&entry).await {
// Non-fatal: the mutation already succeeded; the user
// just loses the undo affordance.
// Non-fatal: the mutation already succeeded. The user
// loses the undo affordance, but flag it so clients can
// say so rather than silently dropping undo.
tracing::warn!(%error, "failed to write undo entry");
None
(None, true)
} else {
Some(mutation_id)
(Some(mutation_id), false)
}
}
_ => None,
_ => (None, false),
};

let result = MutationResultData {
Expand All @@ -666,6 +668,7 @@ pub(super) async fn mutation(
failed,
accounts,
mutation_id,
undo_unavailable,
};

emit_mutation_reconciliation_failed_if_needed(state, client_correlation_id, &result);
Expand Down Expand Up @@ -888,6 +891,7 @@ fn empty_mutation_result(requested: u32) -> MutationResultData {
failed: 0,
accounts: Vec::new(),
mutation_id: None,
undo_unavailable: false,
}
}

Expand Down Expand Up @@ -2710,6 +2714,7 @@ mod reconciliation_failed_emit_tests {
failed: 0,
accounts: vec![],
mutation_id: None,
undo_unavailable: false,
}
}

Expand Down Expand Up @@ -2750,6 +2755,7 @@ mod reconciliation_failed_emit_tests {
failed: 0,
accounts: vec![],
mutation_id: None,
undo_unavailable: false,
};
emit_mutation_reconciliation_failed_if_needed(&state, Some("1"), &result);
assert!(
Expand Down
30 changes: 30 additions & 0 deletions crates/protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,35 @@ mod tests {

use super::*;
use bytes::BytesMut;

#[test]
fn mutation_result_undo_unavailable_serializes_only_when_set() {
let mut result = MutationResultData {
requested: 1,
succeeded: 1,
skipped: 0,
failed: 0,
accounts: vec![],
mutation_id: None,
undo_unavailable: false,
};
// Default (false) is omitted from the wire, so existing
// consumers see no new field.
let json = serde_json::to_value(&result).unwrap();
assert!(json.get("undo_unavailable").is_none());

// Set to true, it appears so clients can warn the user.
result.undo_unavailable = true;
let json = serde_json::to_value(&result).unwrap();
assert_eq!(json.get("undo_unavailable"), Some(&serde_json::json!(true)));

// And it round-trips back through a missing field as false.
let parsed: MutationResultData =
serde_json::from_value(serde_json::json!({"requested":1,"succeeded":1,"skipped":0,"failed":0,"accounts":[]}))
.unwrap();
assert!(!parsed.undo_unavailable);
}

use mxr_core::id::*;
use mxr_core::{
Address, Draft, DraftIntent, ExportFormat, SavedSearch, SearchMode, SemanticProfile,
Expand Down Expand Up @@ -774,6 +803,7 @@ mod tests {
error: None,
}],
mutation_id: None,
undo_unavailable: false,
},
},
IpcCategory::CoreMail,
Expand Down
6 changes: 6 additions & 0 deletions crates/protocol/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,12 @@ pub struct MutationResultData {
/// `None` for non-undoable mutations (Star, ModifyLabels, Move).
#[serde(default, skip_serializing_if = "Option::is_none")]
pub mutation_id: Option<String>,
/// True when an undoable mutation succeeded but persisting its undo
/// entry failed, so `mutation_id` is `None` even though undo was
/// expected. Lets clients distinguish "no undo by design" from "undo
/// was lost" and warn the user instead of silently dropping it.
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
pub undo_unavailable: bool,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
Expand Down
1 change: 1 addition & 0 deletions crates/tui/src/runner/tests/mailbox_views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,7 @@ fn format_mutation_failure_joins_per_account_errors() {
error: None,
}],
mutation_id: None,
undo_unavailable: false,
};
assert_eq!(
format_mutation_failure(&bare),
Expand Down
1 change: 1 addition & 0 deletions crates/web/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2721,6 +2721,7 @@ async fn read_mutation_endpoint_reports_skipped_result_as_not_ok() {
error: Some("account unavailable".into()),
}],
mutation_id: None,
undo_unavailable: false,
},
},
})
Expand Down
1 change: 1 addition & 0 deletions site/src/content/docs/guides/automation-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ mxr archive --search 'from:noreply older_than:30d' --account work --yes
- `mxr send DRAFT_ID` is **not idempotent** — calling twice will send twice (the daemon schedules the second send). Always check `mxr drafts list` first.
- `mxr unsubscribe` may hit a provider URL once; re-running on an already-unsubscribed message is harmless but emits no useful new state.
- `mxr undo MUTATION_ID` works within a 60-second window; after that it returns an error.
- An undoable mutation normally returns a `mutation_id`. If that field is absent **and** the result has `"undo_unavailable": true`, the mutation succeeded but its undo entry could not be recorded — it can't be reversed with `mxr undo`. A plain absent `mutation_id` (no `undo_unavailable`) just means the mutation isn't undoable by design (e.g. star, label, move).

## See also

Expand Down
Loading