Skip to content

feat: tell users when a mutation's undo could not be recorded#77

Merged
bhekanik merged 1 commit into
mainfrom
feat/undo-unavailable-hint
Jun 11, 2026
Merged

feat: tell users when a mutation's undo could not be recorded#77
bhekanik merged 1 commit into
mainfrom
feat/undo-unavailable-hint

Conversation

@bhekanik

@bhekanik bhekanik commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What

Surfaces "undo unavailable" when an undoable mutation succeeds but its undo entry can't be persisted. Audit backlog item P2 #19. Off main.

Why

On the undo-write failure path the handler returned mutation_id: None and logged a warning — but None is also what non-undoable mutations (star, label, move) return. So a lost undo affordance looked identical to "no undo by design," and the user was never told.

How

  • New undo_unavailable: bool on MutationResultData, #[serde(default, skip_serializing_if = "std::ops::Not::not")] so it's omitted from the wire unless true — existing consumers are unaffected.
  • The handler sets it true only when an undoable mutation succeeded but write_undo_entry failed.
  • CLI prints a note ("undo is unavailable for this mutation — recording the undo entry failed"); the JSON output already embeds the full result, so agents get the flag automatically.

Verification

  • New protocol test: field omitted when false, present when true, and a missing field deserializes to false.
  • mxr-protocol (33+1), mxr mutation/chimes (100), mxr-tui (3), mxr-web (54) tests pass; clippy + fmt clean across all four crates.
  • Docs: guides/automation-contract.md documents how agents should read the flag.

Note

openapi.json / web TS types are generated artifacts and regenerate on the web build; the field is additive/optional, and the web UI doesn't consume it yet, so no generated files are committed here.

Generated with Claude Code


Summary by cubic

Tell users when an undoable mutation succeeded but its undo entry couldn't be saved, so a lost undo is not mistaken for a non-undoable action. Addresses P2 #19.

  • New Features
    • Added undo_unavailable: bool to MutationResultData (omitted unless true).
    • Set when an undoable mutation commits but writing the undo entry fails; mutation_id remains None.
    • CLI prints a note when undo_unavailable is true; JSON includes the flag for agents.

Written for commit be880f5. Summary will update on new commits.

Review in cubic

When an undoable mutation (archive/trash/spam/...) committed but writing
its undo entry failed, the response's `mutation_id` came back `None` —
indistinguishable from a mutation that simply isn't undoable. The undo
affordance vanished silently.

Add an `undo_unavailable` flag to `MutationResultData` (serialized only
when true, so existing consumers are unaffected). The handler sets it on
the undo-write failure path; the CLI prints a note that undo is
unavailable, and the JSON contract carries the flag so agents can tell
"undo failed" apart from "not undoable by design".
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@bhekanik, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 9 minutes and 8 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5bbfbd7a-115a-4111-841e-ac49be78bb76

📥 Commits

Reviewing files that changed from the base of the PR and between 7bb7a09 and be880f5.

📒 Files selected for processing (8)
  • crates/daemon/src/chimes.rs
  • crates/daemon/src/commands/mutations/helpers.rs
  • crates/daemon/src/handler/mutations.rs
  • crates/protocol/src/lib.rs
  • crates/protocol/src/types.rs
  • crates/tui/src/runner/tests/mailbox_views.rs
  • crates/web/src/tests.rs
  • site/src/content/docs/guides/automation-contract.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/undo-unavailable-hint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mxr-mail Ready Ready Preview, Comment Jun 10, 2026 8:06pm

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/daemon/src/handler/mutations.rs">

<violation number="1" location="crates/daemon/src/handler/mutations.rs:549">
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.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

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>

@bhekanik bhekanik merged commit b138fcc into main Jun 11, 2026
26 of 29 checks passed
@bhekanik bhekanik deleted the feat/undo-unavailable-hint branch June 11, 2026 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant