diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e6db8a0..49933556 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,16 @@ All notable changes to this project will be documented in this file. `navigateToPageCitation` expose materialization and preview navigation. The MCP inline preview remains explicitly continuous pending #434. See [`docs/architecture/page_map.md`](docs/architecture/page_map.md). +- **Atomic multi-step mutation batches** (issue #445). `DocxSession.ExecuteBatch` + and the reusable nested-safe `BeginTransaction` primitive checkpoint the complete + OPC package, relationship topology, anchor/revision generators, mutable session + configuration, version, and both undo/redo cursors. Atomic mode is the default: + all available preflights run before step zero; success advances the version once + and creates one undo unit; any failed or thrown step restores the exact package + and history state and returns its index/tool/action/error with `rolledBack: true`. + Explicit `best_effort` retains sequential partial-success behavior. The contract + is available through .NET/Ops/JSON, WASM/npm, stdio/Python, and MCP; + MCP's legacy `apply` spelling is now a deprecated alias for `best_effort`. - **Optimistic mutation preconditions and a monotonic document version** (issue #447). Every `DocxSession` starts at version `0` and advances exactly once for each committed mutation, undo, or redo; failures and successful no-ops leave it diff --git a/Docxodus.Tests/DocxSessionAtomicBatchTests.cs b/Docxodus.Tests/DocxSessionAtomicBatchTests.cs new file mode 100644 index 00000000..eb7b8889 --- /dev/null +++ b/Docxodus.Tests/DocxSessionAtomicBatchTests.cs @@ -0,0 +1,608 @@ +#nullable enable + +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using System.Xml.Linq; +using DocumentFormat.OpenXml.Packaging; +using Xunit; + +namespace Docxodus.Tests; + +/// Complete-package atomic mutation batch regression coverage (issue #445). +public class DocxSessionAtomicBatchTests +{ + [Fact] + public void DS454_AtomicSuccess_IsOneVersionAndOneUndoRedoUnit() + { + using var session = Open(); + var paragraphs = BodyParagraphs(session); + var before = session.Save(persistAnchorIds: false); + + var result = session.ExecuteBatch(new[] + { + new MutationBatchStep("docx_edit", "replace_text", + s => s.ExecuteMutation( + new MutationPreconditions { ExpectedVersion = 0 }, + x => x.ReplaceText(paragraphs[0], "Atomic first."))), + new MutationBatchStep("docx_edit", "replace_text", + s => s.ExecuteMutation( + new MutationPreconditions { ExpectedVersion = 0 }, + x => x.ReplaceText(paragraphs[1], "Atomic second."))), + }); + + Assert.True(result.Success); + Assert.False(result.RolledBack); + Assert.Equal(1, session.Version); + Assert.Equal(1, session.UndoCount); + Assert.Equal(0, session.RedoCount); + var edited = session.Save(persistAnchorIds: false); + Assert.Contains("Atomic first.", session.Project().Markdown); + Assert.Contains("Atomic second.", session.Project().Markdown); + + Assert.True(session.Undo()); + Assert.Equal(2, session.Version); + AssertSamePackage(before, session.Save(persistAnchorIds: false)); + Assert.True(session.Redo()); + Assert.Equal(3, session.Version); + AssertSamePackage(edited, session.Save(persistAnchorIds: false)); + } + + [Theory] + [InlineData("body")] + [InlineData("header")] + [InlineData("footer")] + [InlineData("note")] + [InlineData("comment")] + [InlineData("table")] + [InlineData("relationship")] + [InlineData("annotation")] + public void DS455_AtomicFailure_RestoresEveryStoryAndRelationship(string mutationKind) + { + using var session = Open(); + var body = BodyParagraphs(session)[0]; + var normalBefore = session.Save(persistAnchorIds: false); + var persistedBefore = session.Save(persistAnchorIds: true); + var anchorsBefore = session.Project().AnchorIndex.Keys.OrderBy(x => x).ToArray(); + var hyperlinkCountBefore = session.LiveDocument.MainDocumentPart!.HyperlinkRelationships.Count(); + var hyperlinkCountDuring = hyperlinkCountBefore; + + EditResult Mutate(DocxSession s) => mutationKind switch + { + "body" => s.ReplaceText(body, "Changed body."), + "header" => s.SetHeaderText(body, HeaderFooterKind.Default, "Atomic header."), + "footer" => s.SetFooterText(body, HeaderFooterKind.Default, "Atomic footer."), + "note" => s.InsertFootnote(body, 3, "Atomic note."), + "comment" => s.AddComment(body, null, "Alice", "Atomic comment."), + "table" => s.InsertTable(body, Position.After, 2, 2), + "relationship" => AddHyperlink(s), + "annotation" => s.AddAnnotation(body, new CharSpan(0, 5), new DocumentAnnotation + { + Id = "atomic-annotation", + LabelId = "RISK", + Label = "Risk", + Color = "#FFCC00", + }), + _ => throw new ArgumentOutOfRangeException(nameof(mutationKind)), + }; + + EditResult AddHyperlink(DocxSession s) + { + var edit = s.ReplaceText(body, "Changed [link](https://example.test/atomic-batch)"); + hyperlinkCountDuring = s.LiveDocument.MainDocumentPart!.HyperlinkRelationships.Count(); + return edit; + } + + var result = session.ExecuteBatch(new[] + { + new MutationBatchStep("docx_edit", mutationKind, Mutate), + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText("p:body:missing", "must fail")), + }); + + Assert.False(result.Success); + Assert.True(result.RolledBack); + Assert.Equal(1, result.Failure?.Index); + Assert.Equal("docx_edit", result.Failure?.Tool); + Assert.Equal("replace_text", result.Failure?.Action); + Assert.Equal(EditErrorCode.AnchorNotFound, result.Failure?.Error.Code); + Assert.True(result.Failure?.RolledBack); + Assert.All(result.Steps, step => Assert.True(step.RolledBack)); + Assert.Equal(0, session.Version); + Assert.Equal(0, session.UndoCount); + Assert.Equal(0, session.RedoCount); + AssertSamePackage(normalBefore, session.Save(persistAnchorIds: false)); + AssertSamePackage(persistedBefore, session.Save(persistAnchorIds: true)); + Assert.Equal(anchorsBefore, session.Project().AnchorIndex.Keys.OrderBy(x => x).ToArray()); + Assert.Equal(hyperlinkCountBefore, session.LiveDocument.MainDocumentPart!.HyperlinkRelationships.Count()); + Assert.Empty(session.ListAnnotations()); + if (mutationKind == "relationship") + Assert.True(hyperlinkCountDuring > hyperlinkCountBefore); + } + + [Fact] + public void DS455B_ThrowingStep_AfterCrossPartMutationIsStructuredAndRolledBack() + { + using var session = Open(); + var body = BodyParagraphs(session)[0]; + var before = session.Save(persistAnchorIds: true); + + var result = session.ExecuteBatch(new MutationBatchStep[] + { + new("docx_create", "set_header_text", + s => s.SetHeaderText(body, HeaderFooterKind.Default, "Speculative header.")), + new("docx_edit", "throw", + (Func)(_ => throw new InvalidOperationException("deliberate batch fault"))), + }); + + Assert.False(result.Success); + Assert.True(result.RolledBack); + Assert.Equal(1, result.Failure?.Index); + Assert.Equal("docx_edit", result.Failure?.Tool); + Assert.Equal("throw", result.Failure?.Action); + Assert.Equal(EditErrorCode.InternalError, result.Failure?.Error.Code); + Assert.Contains("deliberate batch fault", result.Failure?.Error.Message); + Assert.Equal(0, session.Version); + Assert.Equal(0, session.UndoCount); + AssertSamePackage(before, session.Save(persistAnchorIds: true)); + Assert.DoesNotContain("Speculative header.", session.Project().Markdown); + } + + [Fact] + public void DS455C_AtomicFailure_PreservesOpaqueCustomXmlPayloadAndRelationship() + { + var payload = Encoding.UTF8.GetBytes( + "\n untouched \n"); + var seeded = DocxSessionTests.BuildDS001_SimpleTwoParagraphs(); + using var source = new MemoryStream(); + source.Write(seeded); + source.Position = 0; + string partUri; + string relationshipId; + using (var package = WordprocessingDocument.Open(source, isEditable: true)) + { + var custom = package.MainDocumentPart!.AddCustomXmlPart(CustomXmlPartType.CustomXml); + custom.FeedData(new MemoryStream(payload)); + partUri = custom.Uri.ToString(); + relationshipId = package.MainDocumentPart.GetIdOfPart(custom); + package.Save(); + } + + using var session = new DocxSession(source.ToArray(), new DocxSessionSettings + { + CaptureInitialProjection = false, + PersistAnchorIds = false, + }); + var anchor = BodyParagraphs(session)[0]; + var result = session.ExecuteBatch(new[] + { + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText(anchor, "Speculative body edit.")), + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText("p:body:missing", "failure")), + }); + + Assert.False(result.Success); + Assert.True(result.RolledBack); + var restored = session.LiveDocument.MainDocumentPart!.CustomXmlParts + .Single(part => part.Uri.ToString() == partUri); + Assert.Equal(relationshipId, session.LiveDocument.MainDocumentPart.GetIdOfPart(restored)); + Assert.Equal(payload, ReadPartBytes(restored)); + + // A normal output save may inspect custom XML while finding Docxodus annotations, but + // opaque application payloads must remain byte-for-byte untouched. + var saved = session.Save(persistAnchorIds: false); + using var reopened = WordprocessingDocument.Open(new MemoryStream(saved), isEditable: false); + var savedPart = reopened.MainDocumentPart!.CustomXmlParts + .Single(part => part.Uri.ToString() == partUri); + Assert.Equal(relationshipId, reopened.MainDocumentPart.GetIdOfPart(savedPart)); + Assert.Equal(payload, ReadPartBytes(savedPart)); + } + + [Fact] + public void DS456_AtomicFailure_RestoresRedoCursorAndHistoryDiagnostics() + { + using var session = Open(new DocxSessionSettings + { + PersistAnchorIds = true, + UndoDepth = 1, + }); + var body = BodyParagraphs(session)[0]; + Assert.True(session.ReplaceText(body, "History edit.").Success); + Assert.True(session.Undo()); + Assert.Equal(0, session.UndoCount); + Assert.Equal(1, session.RedoCount); + var trimmedBefore = session.UndoHistoryTrimmedForMemory; + var memoryBefore = session.UndoMemoryBytes; + var versionBefore = session.Version; + + var result = session.ExecuteBatch(new[] + { + new MutationBatchStep("docx_edit", "replace_text", s => s.ReplaceText(body, "Speculative.")), + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText("p:body:missing", "failure")), + }); + + Assert.False(result.Success); + Assert.Equal(versionBefore, session.Version); + Assert.Equal(0, session.UndoCount); + Assert.Equal(1, session.RedoCount); + Assert.Equal(trimmedBefore, session.UndoHistoryTrimmedForMemory); + Assert.Equal(memoryBefore, session.UndoMemoryBytes); + Assert.False(session.Undo()); + Assert.True(session.Redo()); + Assert.Contains("History edit.", session.Project().Markdown); + } + + [Fact] + public void DS457_Transactions_AreNestedLifoAndOuterCommitSquashesInnerWork() + { + using var session = Open(); + var paragraphs = BodyParagraphs(session); + var before = session.Save(persistAnchorIds: false); + + using (var outer = session.BeginTransaction()) + { + Assert.True(session.ReplaceText(paragraphs[0], "Outer edit.").Success); + using (var inner = session.BeginTransaction()) + { + Assert.True(session.SetHeaderText( + paragraphs[0], HeaderFooterKind.Default, "Rolled-back header.").Success); + inner.Rollback(); + } + + Assert.DoesNotContain("Rolled-back header.", session.Project().Markdown); + using (var inner = session.BeginTransaction()) + { + Assert.True(session.ReplaceText(paragraphs[1], "Inner committed edit.").Success); + inner.Commit(); + } + + Assert.Equal(0, session.Version); + Assert.False(session.Undo()); + outer.Commit(); + } + + Assert.Equal(1, session.Version); + Assert.Equal(1, session.UndoCount); + Assert.Contains("Outer edit.", session.Project().Markdown); + Assert.Contains("Inner committed edit.", session.Project().Markdown); + Assert.True(session.Undo()); + AssertSamePackage(before, session.Save(persistAnchorIds: false)); + } + + [Fact] + public void DS457B_OutOfOrderCompletionLeavesBothScopesRecoverable() + { + using var session = Open(); + var anchor = BodyParagraphs(session)[0]; + var before = session.Save(persistAnchorIds: true); + var outer = session.BeginTransaction(); + Assert.True(session.ReplaceText(anchor, "Outer speculative edit.").Success); + var inner = session.BeginTransaction(); + Assert.True(session.SetHeaderText( + anchor, HeaderFooterKind.Default, "Inner speculative header.").Success); + + Assert.Throws(() => outer.Commit()); + Assert.False(outer.IsCompleted); + Assert.False(inner.IsCompleted); + + inner.Commit(); + Assert.True(inner.IsCompleted); + Assert.False(outer.IsCompleted); + outer.Rollback(); + + Assert.True(outer.IsCompleted); + Assert.Equal(0, session.Version); + Assert.Equal(0, session.UndoCount); + AssertSamePackage(before, session.Save(persistAnchorIds: true)); + } + + [Fact] + public void DS457C_WrongThreadCompletionLeavesScopeRecoverableByOwner() + { + using var session = Open(); + var before = session.Save(persistAnchorIds: false); + var transaction = session.BeginTransaction(); + Assert.True(session.ReplaceText( + BodyParagraphs(session)[0], "Wrong-thread speculative edit.").Success); + + Exception? failure = null; + var wrongThread = new Thread(() => failure = Record.Exception(transaction.Rollback)); + wrongThread.Start(); + wrongThread.Join(); + Assert.IsType(failure); + Assert.False(transaction.IsCompleted); + + transaction.Rollback(); + Assert.True(transaction.IsCompleted); + AssertSamePackage(before, session.Save(persistAnchorIds: false)); + Assert.Equal(0, session.Version); + Assert.Equal(0, session.UndoCount); + } + + [Fact] + public void DS457D_DisposingSessionAbandonsScopesWithoutHoldingGateOrResurrectingPackage() + { + var session = Open(); + var mutationGate = PrivateField(session, "_mutationGate"); + var transaction = session.BeginTransaction(); + Assert.True(session.ReplaceText( + BodyParagraphs(session)[0], "Abandoned speculative edit.").Success); + + session.Dispose(); + + Assert.True(transaction.IsCompleted); + transaction.Dispose(); + Assert.Throws(() => session.Project()); + Assert.True(Task.Run(() => + { + if (!Monitor.TryEnter(mutationGate, TimeSpan.FromSeconds(1))) return false; + Monitor.Exit(mutationGate); + return true; + }).GetAwaiter().GetResult()); + } + + [Fact] + public void DS457E_WrongThreadSessionDisposeLeavesOwnerAbleToRollback() + { + var session = Open(); + var transaction = session.BeginTransaction(); + + Exception? failure = null; + var wrongThread = new Thread(() => failure = Record.Exception(session.Dispose)); + wrongThread.Start(); + wrongThread.Join(); + Assert.IsType(failure); + Assert.False(transaction.IsCompleted); + Assert.NotEmpty(session.Project().Markdown); + + transaction.Rollback(); + session.Dispose(); + } + + [Fact] + public void DS458_NoOpAndPreflightFailure_AreBytePureAndDoNotReassignAnchors() + { + using var session = Open(new DocxSessionSettings + { + CaptureInitialProjection = false, + PersistAnchorIds = false, + }); + var anchors = session.Project().AnchorIndex.Keys.OrderBy(x => x).ToArray(); + var normalBefore = session.Save(persistAnchorIds: false); + + using (var transaction = session.BeginTransaction()) + transaction.Commit(); + + AssertSamePackage(normalBefore, session.Save(persistAnchorIds: false)); + Assert.Equal(anchors, session.Project().AnchorIndex.Keys.OrderBy(x => x).ToArray()); + Assert.Equal(0, session.Version); + Assert.Equal(0, session.UndoCount); + + var persistedBefore = session.Save(persistAnchorIds: true); + var invoked = false; + var result = session.ExecuteBatch(new[] + { + new MutationBatchStep( + "docx_edit", "replace_text", + s => { invoked = true; return s.ReplaceText(anchors[0], "not run"); }, + _ => new EditError(EditErrorCode.PreconditionFailed, "preflight rejected")), + }); + + Assert.False(result.Success); + Assert.True(result.RolledBack); + Assert.False(invoked); + Assert.Equal(0, session.Version); + Assert.Equal(0, session.UndoCount); + AssertSamePackage(normalBefore, session.Save(persistAnchorIds: false)); + AssertSamePackage(persistedBefore, session.Save(persistAnchorIds: true)); + Assert.Equal(anchors, session.Project().AnchorIndex.Keys.OrderBy(x => x).ToArray()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void DS458B_DisposedNoOpTransaction_WithWarmedReadCachesIsStrictlyBytePure( + bool persistAnchorIds) + { + using var session = Open(new DocxSessionSettings + { + CaptureInitialProjection = false, + PersistAnchorIds = persistAnchorIds, + }); + + // Warm the XDocument-backed projection, story, relationship and custom-XML reads before + // checkpointing. A no-op rollback must not serialize or otherwise perturb those caches. + _ = session.Project(); + _ = session.ListBlocks(); + _ = session.ListNotes(); + _ = session.ListComments(); + _ = session.ListRevisions(); + _ = session.ListAnnotations(); + var before = session.Save(persistAnchorIds); + + using (session.BeginTransaction()) + { + } + + var after = session.Save(persistAnchorIds); + Assert.Equal(before, after); + Assert.Equal(0, session.Version); + Assert.Equal(0, session.UndoCount); + Assert.Equal(0, session.RedoCount); + } + + [Fact] + public void DS458C_SaveFalse_PreservesPtDeclarationNamedByMcIgnorable() + { + XNamespace mc = "http://schemas.openxmlformats.org/markup-compatibility/2006"; + var seeded = DocxSessionTests.BuildDS001_SimpleTwoParagraphs(); + using var source = new MemoryStream(); + source.Write(seeded); + source.Position = 0; + using (var package = WordprocessingDocument.Open(source, isEditable: true)) + { + var root = package.MainDocumentPart!.GetXDocument().Root!; + root.SetAttributeValue(XNamespace.Xmlns + "mc", mc.NamespaceName); + root.SetAttributeValue(XNamespace.Xmlns + "pt", PtOpenXml.pt.NamespaceName); + root.SetAttributeValue(mc + "Ignorable", "pt"); + root.Descendants().First().SetAttributeValue(PtOpenXml.Unid, "save-false-fixture"); + package.MainDocumentPart.PutXDocument(); + package.Save(); + } + + using var session = new DocxSession(source.ToArray(), new DocxSessionSettings + { + CaptureInitialProjection = false, + PersistAnchorIds = false, + }); + var saved = session.Save(persistAnchorIds: false); + + using var reopened = WordprocessingDocument.Open(new MemoryStream(saved), isEditable: false); + // Access the typed DOM to force the SDK's markup-compatibility prefix resolution. + Assert.NotNull(reopened.MainDocumentPart!.Document.Body); + var savedRoot = reopened.MainDocumentPart.GetXDocument().Root!; + Assert.Equal(PtOpenXml.pt.NamespaceName, + (string?)savedRoot.Attribute(XNamespace.Xmlns + "pt")); + Assert.Contains("pt", ((string?)savedRoot.Attribute(mc + "Ignorable") ?? string.Empty) + .Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries)); + Assert.DoesNotContain(savedRoot.DescendantsAndSelf().Attributes(), + attribute => attribute.Name == PtOpenXml.Unid); + } + + [Fact] + public void DS459_Rollback_RestoresTrackingConfigurationAndRevisionGenerators() + { + using var seed = Open(); + var baseBytes = seed.Save(persistAnchorIds: true); + var settings = new DocxSessionSettings + { + PersistAnchorIds = true, + TrackedChanges = TrackedChangeMode.RenderInline, + RevisionAuthor = "Alice", + }; + using var tested = new DocxSession(baseBytes, settings); + using var control = new DocxSession(baseBytes, settings); + var anchor = BodyParagraphs(tested)[0]; + var controlAnchor = BodyParagraphs(control)[0]; + var revisionCounter = PrivateField(tested, "_revisionCounter"); + var formatTicks = PrivateField(tested, "_lastFormatRevisionTicks"); + + var result = tested.ExecuteBatch(new[] + { + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText(anchor, "Failed tracked edit.")), + new MutationBatchStep("docx_edit", "apply_format", + s => s.ApplyFormat(anchor, new CharSpan(0, 5), new FormatOp { Bold = true })), + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText("p:body:missing", "failure")), + }); + + Assert.False(result.Success); + Assert.Equal(revisionCounter, PrivateField(tested, "_revisionCounter")); + Assert.Equal(formatTicks, PrivateField(tested, "_lastFormatRevisionTicks")); + Assert.Equal(TrackedChangeMode.RenderInline, tested.TrackedChanges); + Assert.Equal("Alice", tested.RevisionAuthor); + Assert.Empty(tested.ListRevisions()); + + Assert.True(tested.ReplaceText(anchor, "Committed tracked edit.").Success); + Assert.True(control.ReplaceText(controlAnchor, "Committed tracked edit.").Success); + Assert.Equal( + control.ListRevisions().Select(r => (r.Id, r.Type, r.Author, r.Text)), + tested.ListRevisions().Select(r => (r.Id, r.Type, r.Author, r.Text))); + + using (var transaction = tested.BeginTransaction()) + { + tested.SetTrackedChanges(TrackedChangeMode.Accept); + tested.SetRevisionAuthor("Speculative"); + transaction.Rollback(); + } + Assert.Equal(TrackedChangeMode.RenderInline, tested.TrackedChanges); + Assert.Equal("Alice", tested.RevisionAuthor); + } + + [Fact] + public void DS460_BestEffort_IsExplicitAndRetainsPartialSuccess() + { + using var session = Open(); + var anchor = BodyParagraphs(session)[0]; + + var result = session.ExecuteBatch(new[] + { + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText(anchor, "Retained partial edit.")), + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText("p:body:missing", "failure")), + }, MutationBatchMode.BestEffort); + + Assert.False(result.Success); + Assert.False(result.RolledBack); + Assert.False(result.Failure?.RolledBack); + Assert.Contains("Retained partial edit.", session.Project().Markdown); + Assert.Equal(1, session.Version); + Assert.Equal(1, session.UndoCount); + } + + [Fact] + public void DS460B_BestEffortPreflightsEachStepAgainstSequentialState() + { + using var session = Open(); + var paragraphs = BodyParagraphs(session); + + var result = session.ExecuteBatch(new[] + { + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText(paragraphs[0], "State created by step zero.")), + new MutationBatchStep( + "docx_edit", "replace_text", + s => s.ReplaceText(paragraphs[1], "Preflight observed the new state."), + s => s.Project().Markdown.Contains("State created by step zero.", StringComparison.Ordinal) + ? null + : new EditError(EditErrorCode.PreconditionFailed, + "step zero's state was not visible")), + }, MutationBatchMode.BestEffort); + + Assert.True(result.Success); + Assert.Equal(2, result.Steps.Count); + Assert.All(result.Steps, step => Assert.True(step.Success)); + Assert.Contains("State created by step zero.", session.Project().Markdown); + Assert.Contains("Preflight observed the new state.", session.Project().Markdown); + Assert.Equal(2, session.Version); + Assert.Equal(2, session.UndoCount); + } + + private static T PrivateField(DocxSession session, string name) => + (T)typeof(DocxSession).GetField(name, BindingFlags.Instance | BindingFlags.NonPublic)! + .GetValue(session)!; + + private static void AssertSamePackage(byte[] expected, byte[] actual) => + PackageEquivalence.AssertSamePackage( + new WmlDocument("expected.docx", expected), + new WmlDocument("actual.docx", actual)); + + private static byte[] ReadPartBytes(OpenXmlPart part) + { + using var source = part.GetStream(FileMode.Open, FileAccess.Read); + using var copy = new MemoryStream(); + source.CopyTo(copy); + return copy.ToArray(); + } + + private static DocxSession Open(DocxSessionSettings? settings = null) => + new(DocxSessionTests.BuildDS001_SimpleTwoParagraphs(), + settings ?? new DocxSessionSettings { PersistAnchorIds = true }); + + private static string[] BodyParagraphs(DocxSession session) => + session.Project().AnchorIndex.Keys + .Where(id => id.StartsWith("p:body:", StringComparison.Ordinal)) + .ToArray(); +} diff --git a/Docxodus.Tests/McpServerDispatcherTests.cs b/Docxodus.Tests/McpServerDispatcherTests.cs index 916f9726..fbf612d8 100644 --- a/Docxodus.Tests/McpServerDispatcherTests.cs +++ b/Docxodus.Tests/McpServerDispatcherTests.cs @@ -1029,6 +1029,166 @@ public void MCP092_Mutations_RejectsUndoRedoAsSteps() Assert.Contains("undo", ex.Message, StringComparison.OrdinalIgnoreCase); } + [Fact] + public void MCP094_Mutations_AtomicFailureIsStructuredAndLeavesNoVersionOrHistory() + { + var sessionId = OpenSession(); + var sessionArg = JsonSerializer.Serialize(sessionId); + var anchor = FirstBodyAnchorId(sessionId, _store); + var before = Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))) + .GetProperty("markdown").GetString(); + + var batch = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "atomic", + "steps": [ + { "tool": "docxodus_edit", "args": { "action": "replace_text", "anchorId": "{{anchor}}", "markdown": "speculative" } }, + { "tool": "docxodus_edit", "args": { "action": "replace_text", "anchorId": "p:body:missing", "markdown": "failure" } } + ] + } + """))); + + Assert.Equal("failed", batch.GetProperty("status").GetString()); + Assert.False(batch.GetProperty("success").GetBoolean()); + Assert.True(batch.GetProperty("rolledBack").GetBoolean()); + var failure = batch.GetProperty("failure"); + Assert.Equal(1, failure.GetProperty("index").GetInt32()); + Assert.Equal("docxodus_edit", failure.GetProperty("tool").GetString()); + Assert.Equal("replace_text", failure.GetProperty("action").GetString()); + Assert.Equal("anchor_not_found", failure.GetProperty("error").GetProperty("code").GetString()); + Assert.True(failure.GetProperty("rolledBack").GetBoolean()); + + var after = Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))) + .GetProperty("markdown").GetString(); + Assert.Equal(before, after); + var version = Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"version"}"""))) + .GetProperty("version").GetInt64(); + Assert.Equal(0, version); + var undo = Parse(Dispatcher.Call(_store, "docxodus_edit", J( + $$"""{"sessionId":{{sessionArg}},"action":"undo"}"""))); + Assert.False(undo.GetProperty("success").GetBoolean()); + } + + [Fact] + public void MCP095_Mutations_AtomicSuccessIsOneUndoAndInvalidStepHasCallerErrorCode() + { + var sessionId = OpenSession(); + var sessionArg = JsonSerializer.Serialize(sessionId); + var anchor = FirstBodyAnchorId(sessionId, _store); + + var invalid = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "atomic", + "steps": [ + { "tool": "docxodus_comment", "args": { "action": "list" } } + ] + } + """))); + Assert.Equal("invalid_batch_step", + invalid.GetProperty("failure").GetProperty("error").GetProperty("code").GetString()); + + var batch = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "atomic", + "steps": [ + { "tool": "docxodus_edit", "args": { "action": "replace_text", "anchorId": "{{anchor}}", "markdown": "atomic MCP" } }, + { "tool": "docxodus_format", "args": { "action": "apply_format", "anchorId": "{{anchor}}", "format": { "bold": true } } } + ] + } + """))); + Assert.Equal("ok", batch.GetProperty("status").GetString()); + Assert.Equal(1, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(sessionId).Handle)); + + var undo = Parse(Dispatcher.Call(_store, "docxodus_edit", J( + $$"""{"sessionId":{{sessionArg}},"action":"undo"}"""))); + Assert.True(undo.GetProperty("success").GetBoolean()); + var markdown = Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))) + .GetProperty("markdown").GetString(); + Assert.DoesNotContain("atomic MCP", markdown); + } + + [Fact] + public void MCP096_AtomicPreflightsLaterArgumentErrorsBeforeStepZeroMutates() + { + var sessionId = OpenSession(); + var sessionArg = JsonSerializer.Serialize(sessionId); + var anchor = FirstBodyAnchorId(sessionId, _store); + var before = Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))) + .GetProperty("markdown").GetString(); + + var batch = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "mode": "atomic", + "steps": [ + { "tool": "docxodus_edit", "args": { "action": "replace_text", "anchorId": "{{anchor}}", "markdown": "must never run" } }, + { "tool": "docxodus_create", "args": { "action": "set_header_text", "bodyAnchorId": "{{anchor}}", "kind": "sideways", "markdown": "invalid header" } } + ] + } + """))); + + Assert.False(batch.GetProperty("success").GetBoolean()); + Assert.True(batch.GetProperty("rolledBack").GetBoolean()); + var failure = batch.GetProperty("failure"); + Assert.Equal(1, failure.GetProperty("index").GetInt32()); + Assert.Equal("docxodus_create", failure.GetProperty("tool").GetString()); + Assert.Equal("set_header_text", failure.GetProperty("action").GetString()); + Assert.Equal("invalid_batch_step", failure.GetProperty("error").GetProperty("code").GetString()); + Assert.Contains("kind", failure.GetProperty("error").GetProperty("message").GetString()); + + var after = Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))) + .GetProperty("markdown").GetString(); + Assert.Equal(before, after); + Assert.Equal(0, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(sessionId).Handle)); + var undo = Parse(Dispatcher.Call(_store, "docxodus_edit", J( + $$"""{"sessionId":{{sessionArg}},"action":"undo"}"""))); + Assert.False(undo.GetProperty("success").GetBoolean()); + } + + [Fact] + public void MCP097_AtomicStepPreconditionsUseBatchStartState() + { + var sessionId = OpenSession(); + var sessionArg = JsonSerializer.Serialize(sessionId); + var anchor = FirstBodyAnchorId(sessionId, _store); + var info = Parse(Docxodus.Internal.DocxSessionOps.GetAnchorInfo( + _store.Get(sessionId).Handle, anchor)); + var originalText = JsonSerializer.Serialize(info.GetProperty("visibleText").GetString()); + + var batch = Parse(Dispatcher.Call(_store, "docxodus_mutations", J( + $$""" + { + "sessionId": {{sessionArg}}, + "steps": [ + { "tool": "docxodus_edit", "args": { "action": "replace_text", "anchorId": "{{anchor}}", "markdown": "first atomic state" } }, + { "tool": "docxodus_edit", "args": { "action": "replace_text", "anchorId": "{{anchor}}", "markdown": "second atomic state", "preconditions": { "expectedText": {{originalText}} } } } + ] + } + """))); + + Assert.True(batch.GetProperty("success").GetBoolean()); + Assert.Equal("atomic", batch.GetProperty("mode").GetString()); + Assert.Equal(1, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(sessionId).Handle)); + var markdown = Parse(Dispatcher.Call(_store, "docxodus_get_content", J( + $$"""{"sessionId":{{sessionArg}},"format":"markdown"}"""))) + .GetProperty("markdown").GetString(); + Assert.Contains("second atomic state", markdown); + Assert.DoesNotContain("first atomic state", markdown); + } + // ─── Tool catalog ─────────────────────────────────────────────────── [Fact] diff --git a/Docxodus/DocxSession.cs b/Docxodus/DocxSession.cs index 6a618795..0198b481 100644 --- a/Docxodus/DocxSession.cs +++ b/Docxodus/DocxSession.cs @@ -1167,6 +1167,141 @@ public sealed record MutationPreconditions /// An exact substring assertion within an anchor's visible text. public sealed record TextRangePrecondition(int Start, int Length, string Text); +/// Execution policy for a group of synchronous document mutations. +public enum MutationBatchMode +{ + /// All steps commit as one undo/version unit, or every step is rolled back. + Atomic, + + /// Run every step independently and retain successful steps after failures. + BestEffort, +} + +/// +/// One core batch step. performs any read-only validation that can be +/// decided before mutation begins (all steps up front for atomic mode, or immediately before each +/// step for best-effort mode); returns one or more edit envelopes so +/// multi-match replacement can remain one step without losing its individual results. +/// +public sealed class MutationBatchStep +{ + public MutationBatchStep( + string tool, + string action, + Func> mutation, + Func? preflight = null) + { + Tool = tool ?? throw new ArgumentNullException(nameof(tool)); + Action = action ?? throw new ArgumentNullException(nameof(action)); + Mutation = mutation ?? throw new ArgumentNullException(nameof(mutation)); + Preflight = preflight; + } + + public MutationBatchStep( + string tool, + string action, + Func mutation, + Func? preflight = null) + : this(tool, action, s => new[] { mutation(s) }, preflight) + { + } + + public string Tool { get; } + public string Action { get; } + public Func> Mutation { get; } + public Func? Preflight { get; } +} + +/// Result of one batch step, including whether its effects were rolled back. +public sealed record MutationBatchStepResult( + int Index, + string Tool, + string Action, + IReadOnlyList Results, + bool RolledBack) +{ + public bool Success => Results.All(r => r.Success); +} + +/// The first failed step in a batch. +public sealed record MutationBatchFailure( + int Index, + string Tool, + string Action, + EditError Error, + bool RolledBack); + +/// Structured result of an atomic or explicit best-effort mutation batch. +public sealed record MutationBatchResult +{ + public MutationBatchMode Mode { get; init; } + public bool Success { get; init; } + public bool RolledBack { get; init; } + public IReadOnlyList Steps { get; init; } = + Array.Empty(); + public MutationBatchFailure? Failure { get; init; } +} + +/// +/// A synchronous, nested-safe document transaction. Dispose without to +/// restore the complete package and all session/history state captured at begin. The scope owns +/// the session-wide mutation gate and must be completed on the thread that created it. +/// +public sealed class DocxSessionTransaction : IDisposable +{ + private DocxSession? _session; + private readonly long _id; + + internal DocxSessionTransaction(DocxSession session, long id) + { + _session = session; + _id = id; + } + + public bool IsCompleted => _session is null || _session.IsDisposed; + + public void Commit() + { + var session = _session ?? throw new InvalidOperationException("transaction already completed"); + if (session.IsDisposed) + { + _session = null; + throw new ObjectDisposedException(nameof(DocxSession)); + } + session.CompleteTransaction(_id, commit: true); + // Keep the scope recoverable if owner-thread/LIFO validation (or completion itself) + // throws. CompleteTransaction only removes the state after a valid completion. + _session = null; + } + + public void Rollback() + { + var session = _session ?? throw new InvalidOperationException("transaction already completed"); + if (session.IsDisposed) + { + _session = null; + throw new ObjectDisposedException(nameof(DocxSession)); + } + session.CompleteTransaction(_id, commit: false); + _session = null; + } + + public void Dispose() + { + if (_session is null) return; + var session = _session; + // Disposing the owning session explicitly abandons and invalidates its active scopes. + // A later using-scope unwind must be inert rather than reopening the disposed package. + if (session.IsDisposed) + { + _session = null; + return; + } + session.CompleteTransaction(_id, commit: false); + _session = null; + } +} + public sealed record EditError(EditErrorCode Code, string Message, string? AnchorId = null) { public PreconditionFailure? Precondition { get; init; } @@ -1254,6 +1389,9 @@ public enum EditErrorCode /// An optimistic mutation guard did not match the current session or target state. PreconditionFailed, + /// A mutation batch step names an unsupported operation or a read-only action. + InvalidBatchStep, + InternalError, } @@ -1380,16 +1518,32 @@ public sealed class DocxSession : IDisposable private long _version; private PageMap? _registeredPageMap; private readonly object _mutationGate = new(); + private readonly Stack _transactions = new(); + private long _nextTransactionId; + private int _transactionPendingMutations; private int _revisionCounter = 1000; private long _lastFormatRevisionTicks; private RawDocxOps? _raw; + internal bool IsDisposed => _disposed; + // Mutable session configuration (issue #304): seeded from _settings at construction, // switchable mid-session via SetTrackedChanges/SetRevisionAuthor. Session config, not // document state — never captured in undo snapshots. private TrackedChangeMode _trackedChanges; private string? _revisionAuthor; + private sealed record TransactionState( + long Id, + int OwnerThreadId, + DocumentSnapshot PackageSnapshot, + Internal.UndoRing.State History, + int PendingMutations, + TrackedChangeMode TrackedChanges, + string? RevisionAuthor, + Exception? LastInternalError, + Exception? LastRollbackError); + public DocxSession(byte[] docxBytes, DocxSessionSettings? settings = null) { ArgumentNullException.ThrowIfNull(docxBytes); @@ -1400,8 +1554,8 @@ public DocxSession(byte[] docxBytes, DocxSessionSettings? settings = null) _settings.UndoDepth, _settings.UndoMemoryBudgetBytes, static snapshot => snapshot.ApproximateBytes, - onRecordPreOp: _ => AdvanceVersion(), - onPopUndo: snapshot => _version = snapshot.Version); + onRecordPreOp: _ => OnHistoryRecordPreOp(), + onPopUndo: snapshot => OnHistoryPopUndo(snapshot)); _stream = new MemoryStream(); _stream.Write(docxBytes, 0, docxBytes.Length); _stream.Position = 0; @@ -2564,6 +2718,254 @@ public EditResult ExecuteMutation( } } + /// + /// Begin a complete-package transaction under the session-wide mutation gate. Transactions + /// nest in strict LIFO order: an inner commit remains speculative until its outer transaction + /// commits, while an inner rollback restores the state visible at the inner begin boundary. + /// + public DocxSessionTransaction BeginTransaction() + { + ThrowIfDisposed(); + System.Threading.Monitor.Enter(_mutationGate); + try + { + var id = checked(++_nextTransactionId); + var state = new TransactionState( + id, + Environment.CurrentManagedThreadId, + TakePackageSnapshot(), + _history.CaptureState(), + _transactionPendingMutations, + _trackedChanges, + _revisionAuthor, + LastInternalError, + LastRollbackError); + _transactions.Push(state); + return new DocxSessionTransaction(this, id); + } + catch + { + System.Threading.Monitor.Exit(_mutationGate); + throw; + } + } + + internal void CompleteTransaction(long id, bool commit) + { + if (_transactions.Count == 0 || _transactions.Peek().Id != id) + throw new InvalidOperationException("transactions must complete in strict LIFO order"); + var state = _transactions.Peek(); + if (state.OwnerThreadId != Environment.CurrentManagedThreadId) + throw new InvalidOperationException("a transaction must complete on the thread that began it"); + + bool completed = false; + try + { + if (!commit) + { + RestoreTransactionState(state); + } + else if (_transactions.Count == 1) + { + // An inner commit stays represented by its ordinary speculative history entries. + // The outermost commit is the only boundary that squashes them into one + // full-package pre-batch snapshot and advances caller-visible version once. + bool mutated = _transactionPendingMutations > state.PendingMutations; + _history.RestoreState(state.History); + _transactionPendingMutations = state.PendingMutations; + _version = state.PackageSnapshot.Version; + if (mutated) + { + _history.RecordPreOp(state.PackageSnapshot); + // The transaction remains on the stack until every completion operation has + // succeeded, so the history callback records this speculatively. Convert that + // callback effect into the single caller-visible root commit advancement. + _transactionPendingMutations = state.PendingMutations; + _version = checked(state.PackageSnapshot.Version + 1); + } + } + + // Do not orphan the transaction if validation or restoration failed. The caller may + // retry in the correct LIFO/thread context, or roll the still-active scope back. + _transactions.Pop(); + completed = true; + } + finally + { + if (completed) + System.Threading.Monitor.Exit(_mutationGate); + } + } + + private void RestoreTransactionState(TransactionState state) + { + try + { + // Every session mutation records before touching package state. If the pending count + // is unchanged, the scope performed only reads/configuration work (or an operation + // already restored its own failed-op snapshot). Reopening the checkpoint in that case + // is observably worse: OPC rewrites ZIP timestamps even for a no-op rollback. Keep the + // live package byte-pure while still restoring history/configuration below. + if (_transactionPendingMutations > state.PendingMutations) + RestoreSnapshot(state.PackageSnapshot); + _history.RestoreState(state.History); + _transactionPendingMutations = state.PendingMutations; + _trackedChanges = state.TrackedChanges; + _revisionAuthor = state.RevisionAuthor; + LastInternalError = state.LastInternalError; + LastRollbackError = state.LastRollbackError; + } + catch (Exception rollbackEx) + { + LastRollbackError = rollbackEx; + throw; + } + } + + /// + /// Execute a core mutation batch. Atomic is the default; callers must choose + /// explicitly to retain partial successes. + /// + public MutationBatchResult ExecuteBatch( + IEnumerable steps, + MutationBatchMode mode = MutationBatchMode.Atomic) + { + ArgumentNullException.ThrowIfNull(steps); + if (!Enum.IsDefined(mode)) + throw new ArgumentOutOfRangeException(nameof(mode), mode, "unknown mutation batch mode"); + var materialized = steps.ToArray(); + if (materialized.Any(s => s is null)) + throw new ArgumentException("batch steps cannot contain null", nameof(steps)); + + return mode == MutationBatchMode.Atomic + ? ExecuteAtomicBatch(materialized) + : ExecuteBestEffortBatch(materialized); + } + + private MutationBatchResult ExecuteAtomicBatch(IReadOnlyList steps) + { + using var transaction = BeginTransaction(); + + // Run every available read-only preflight before the first mutation. + for (int i = 0; i < steps.Count; i++) + { + var error = RunBatchPreflight(steps[i]); + if (error is null) continue; + transaction.Rollback(); + var failed = new MutationBatchStepResult( + i, steps[i].Tool, steps[i].Action, + new[] { new EditResult { Success = false, Error = error } }, true); + return FailedAtomicBatch(new[] { failed }, failed); + } + + var results = new List(steps.Count); + for (int i = 0; i < steps.Count; i++) + { + var stepResults = RunBatchMutation(steps[i]); + var step = new MutationBatchStepResult( + i, steps[i].Tool, steps[i].Action, stepResults, false); + results.Add(step); + if (step.Success) continue; + + transaction.Rollback(); + var rolledBack = results.Select(r => r with { RolledBack = true }).ToArray(); + return FailedAtomicBatch(rolledBack, rolledBack[^1]); + } + + transaction.Commit(); + return new MutationBatchResult + { + Mode = MutationBatchMode.Atomic, + Success = true, + RolledBack = false, + Steps = results, + }; + } + + private MutationBatchResult ExecuteBestEffortBatch(IReadOnlyList steps) + { + var results = new List(steps.Count); + MutationBatchStepResult? firstFailure = null; + + for (int i = 0; i < steps.Count; i++) + { + // Best-effort preserves sequential semantics: a later preflight may intentionally + // inspect state produced by an earlier successful step. Only atomic mode preflights + // the complete batch before its first mutation. + var stepResults = RunBatchPreflight(steps[i]) is { } error + ? new[] { new EditResult { Success = false, Error = error } } + : RunBatchMutation(steps[i]); + var step = new MutationBatchStepResult( + i, steps[i].Tool, steps[i].Action, stepResults, false); + results.Add(step); + if (!step.Success && firstFailure is null) firstFailure = step; + } + + return new MutationBatchResult + { + Mode = MutationBatchMode.BestEffort, + Success = firstFailure is null, + RolledBack = false, + Steps = results, + Failure = firstFailure is null ? null : BatchFailure(firstFailure, rolledBack: false), + }; + } + + private EditError? RunBatchPreflight(MutationBatchStep step) + { + if (step.Preflight is null) return null; + try + { + return step.Preflight(this); + } + catch (Exception ex) + { + LastInternalError = ex; + return new EditError(EditErrorCode.InternalError, ex.Message); + } + } + + private IReadOnlyList RunBatchMutation(MutationBatchStep step) + { + try + { + var results = step.Mutation(this); + if (results is not { Count: > 0 } || results.Any(result => result is null)) + return new[] + { + EditResult.Fail(EditErrorCode.InternalError, + "batch mutation returned no valid edit results"), + }; + return results; + } + catch (Exception ex) + { + LastInternalError = ex; + return new[] { EditResult.Fail(EditErrorCode.InternalError, ex.Message) }; + } + } + + private static MutationBatchResult FailedAtomicBatch( + IReadOnlyList results, + MutationBatchStepResult failed) => new() + { + Mode = MutationBatchMode.Atomic, + Success = false, + RolledBack = true, + Steps = results, + Failure = BatchFailure(failed, rolledBack: true), + }; + + private static MutationBatchFailure BatchFailure( + MutationBatchStepResult failed, + bool rolledBack) => new( + failed.Index, + failed.Tool, + failed.Action, + failed.Results.FirstOrDefault(r => !r.Success)?.Error + ?? new EditError(EditErrorCode.InternalError, "batch step failed without an error"), + rolledBack); + /// /// Resolves block-level metadata (style id + name, outline level, list /// membership, formatting probe) for . Returns @@ -4665,13 +5067,50 @@ public byte[] Save(bool persistAnchorIds) { var xdoc = part.GetXDocument(); if (xdoc.Root is null) continue; - bool any = false; + // Other Custom XML parts are opaque application data (SharePoint metadata, + // SDT bindings, ink, and future extensions). They never contain projector Unids, + // and merely reading one must not cause Save(false) to reserialize its payload. + if (part is CustomXmlPart + && (xdoc.Root.Name.NamespaceName != Internal.AnnotationsCustomXml.Namespace + || xdoc.Root.Name.LocalName != "annotations")) + continue; foreach (var el in xdoc.Root.DescendantsAndSelf()) { var attr = el.Attribute(PtOpenXml.Unid); - if (attr is not null) { attr.Remove(); any = true; } + attr?.Remove(); } - if (any) part.PutXDocument(); + // A persisted-anchor checkpoint is reopened during transaction rollback/undo. + // Its pt namespace declaration is then an explicit LINQ-to-XML attribute, unlike + // the serializer-generated declaration on an in-memory document. Once every Unid + // is stripped, remove that now-unused declaration too so normal Save output is + // identical before and after a transaction boundary. + bool ptNamespaceInUse = xdoc.Root.DescendantsAndSelf().Any(el => + el.Name.Namespace == PtOpenXml.pt + || el.Attributes().Any(a => !a.IsNamespaceDeclaration + && a.Name.Namespace == PtOpenXml.pt)); + if (!ptNamespaceInUse) + { + var ignorablePrefixes = xdoc.Root.DescendantsAndSelf() + .Attributes(MC.Ignorable) + .SelectMany(a => a.Value.Split( + (char[]?)null, StringSplitOptions.RemoveEmptyEntries)) + .ToHashSet(StringComparer.Ordinal); + var declarations = xdoc.Root.DescendantsAndSelf() + .Attributes() + .Where(a => a.IsNamespaceDeclaration + && a.Value == PtOpenXml.pt.NamespaceName + // mc:Ignorable contains QNames-as-prefix-tokens. Removing a namespace + // declaration that one of those tokens names produces XML that is + // well-formed but rejected by the Open XML markup-compatibility reader. + && !ignorablePrefixes.Contains(a.Name.LocalName)) + .ToList(); + if (declarations.Count > 0) + declarations.Remove(); + } + // Serialize every projected part, even one with no Unid. This makes normal saves + // deterministic across a package checkpoint reopen (and also guarantees cached + // settings/story edits are never skipped merely because that part has no anchor). + part.PutXDocument(); } _doc!.Save(); _stream!.Flush(); @@ -4705,6 +5144,10 @@ private IEnumerable EnumerateProjectedParts() if (main.FootnotesPart is not null) yield return main.FootnotesPart; if (main.EndnotesPart is not null) yield return main.EndnotesPart; if (main.WordprocessingCommentsPart is not null) yield return main.WordprocessingCommentsPart; + if (main.WordprocessingCommentsExPart is not null) yield return main.WordprocessingCommentsExPart; + if (main.WordprocessingCommentsIdsPart is not null) yield return main.WordprocessingCommentsIdsPart; + if (main.DocumentSettingsPart is not null) yield return main.DocumentSettingsPart; + if (main.StyleDefinitionsPart is not null) yield return main.StyleDefinitionsPart; // Custom XML parts hold annotation metadata; include them so callers that // need to look up parts by URI (e.g. ResolvePart) can find them. foreach (var cx in main.CustomXmlParts) yield return cx; @@ -10012,10 +10455,11 @@ private void RollbackFailedOp() public bool Undo() { if (_disposed) return false; + if (_transactions.Count > 0) return false; var nextVersion = NextVersion(); var (preOp, ok) = _history.PopForUndo(); if (!ok) return false; - _history.RecordForRedo(TakeSnapshot()); + _history.RecordForRedo(preOp.PackageBytes is null ? TakeSnapshot() : TakePackageSnapshot()); RestoreSnapshot(preOp); _version = nextVersion; return true; @@ -10024,10 +10468,11 @@ public bool Undo() public bool Redo() { if (_disposed) return false; + if (_transactions.Count > 0) return false; var nextVersion = NextVersion(); var (postOp, ok) = _history.PopForRedo(); if (!ok) return false; - _history.PushBackForUndo(TakeSnapshot()); + _history.PushBackForUndo(postOp.PackageBytes is null ? TakeSnapshot() : TakePackageSnapshot()); RestoreSnapshot(postOp); _version = nextVersion; return true; @@ -10037,19 +10482,62 @@ public bool Redo() private void AdvanceVersion() => _version = NextVersion(); + private void OnHistoryRecordPreOp() + { + if (_transactions.Count > 0) + _transactionPendingMutations = checked(_transactionPendingMutations + 1); + else + AdvanceVersion(); + } + + private void OnHistoryPopUndo(DocumentSnapshot snapshot) + { + if (_transactions.Count > 0) + { + _transactionPendingMutations = Math.Max(0, _transactionPendingMutations - 1); + return; + } + _version = snapshot.Version; + } + /// Restore the caller-visible version after rolling back speculative preview work. /// Internal by design: committed undo/redo must remain monotonic. internal void RestorePreviewVersion(long version) => _version = version; + /// + /// Dispose the session and abandon any active transactions. Active scopes may only be + /// abandoned by their owner thread; otherwise this throws and leaves the session usable so + /// that thread can complete them. Successfully disposing invalidates every scope, releases + /// every recursive mutation-gate entry, and makes later scope disposal a no-op. + /// public void Dispose() { if (_disposed) return; + int activeTransactions = _transactions.Count; + if (activeTransactions > 0 + && _transactions.Any(t => t.OwnerThreadId != Environment.CurrentManagedThreadId)) + throw new InvalidOperationException( + "a session with active transactions must be disposed by their owner thread"); + _disposed = true; - DisposeRenderShell(); - _doc?.Dispose(); - _stream?.Dispose(); - _doc = null; - _stream = null; + try + { + DisposeRenderShell(); + _doc?.Dispose(); + _stream?.Dispose(); + _doc = null; + _stream = null; + _raw = null; + _transactions.Clear(); + _transactionPendingMutations = 0; + } + finally + { + // BeginTransaction enters once for each nested scope. Release every recursion count + // even if package disposal itself reports an error. + for (int i = 0; i < activeTransactions; i++) + System.Threading.Monitor.Exit(_mutationGate); + } } // ─── Internal mutation helpers (used by tier methods landing in later phases) ─── @@ -10086,13 +10574,26 @@ internal sealed record DocumentSnapshot( System.Collections.Generic.IReadOnlyList<(string RelId, string PartUri)> CommentParts, System.Collections.Generic.IReadOnlyList<(string RelId, bool IsCommentsEx, string PartUri)> CommentThreadingParts) { + /// + /// Optional exact package checkpoint used by transaction boundaries. Unlike the selective + /// XML snapshot, this includes every part payload and relationship (external hyperlinks, + /// media, custom XML, and future package topology) and can therefore back an atomic + /// batch's undo/redo entry without teaching rollback about each relationship type. + /// + internal byte[]? PackageBytes { get; init; } + + internal int? RevisionCounter { get; init; } + + internal long? LastFormatRevisionTicks { get; init; } + /// /// Approximate retained heap of this snapshot's cloned part trees, for the undo ring's /// memory budget. Computed lazily and cached: the ring asks for it at most once per /// snapshot, and a session with the budget disabled never asks at all. /// internal long ApproximateBytes => - _approximateBytes ??= Parts.Sum(p => Internal.XmlMemoryEstimator.Estimate(p.Xml)); + _approximateBytes ??= PackageBytes?.LongLength + ?? Parts.Sum(p => Internal.XmlMemoryEstimator.Estimate(p.Xml)); private long? _approximateBytes; } @@ -10128,8 +10629,91 @@ internal DocumentSnapshot TakeSnapshot() return new DocumentSnapshot(_version, parts, hfParts, noteParts, commentParts, commentThreadingParts); } + /// + /// Capture the complete OPC package for an atomic transaction boundary. The ordinary per-op + /// snapshots remain selective and DOM-based for speed; only a transaction/its undo counterpart + /// pays the package serialization cost. + /// + internal DocumentSnapshot TakePackageSnapshot() + { + var bytes = SerializePackageCheckpoint(); + return new DocumentSnapshot( + _version, + Array.Empty<(string PartUri, XDocument Xml)>(), + Array.Empty<(string RelId, bool IsHeader, string PartUri)>(), + Array.Empty<(string RelId, bool IsFootnote, string PartUri)>(), + Array.Empty<(string RelId, string PartUri)>(), + Array.Empty<(string RelId, bool IsCommentsEx, string PartUri)>()) + { + PackageBytes = bytes, + RevisionCounter = _revisionCounter, + LastFormatRevisionTicks = _lastFormatRevisionTicks, + }; + } + + /// + /// Serialize a complete transaction checkpoint without flushing cached XML into the live + /// package. intentionally writes those caches to the owning stream; + /// using it at transaction begin made a no-op batch observable by adding XML declarations, + /// namespace declarations, or anchor attributes to later output. Cloning first preserves the + /// live package stream/cache exactly while still carrying all current part and relationship + /// topology. Every cached XDocument is then overlaid on its clone counterpart so edits that + /// have not yet reached a part stream are represented in the checkpoint as well. + /// + private byte[] SerializePackageCheckpoint() + { + using var stream = new MemoryStream(); + using (var clone = _doc!.Clone(stream, isEditable: true)) + { + var cloneParts = EnumeratePackageParts(clone) + .ToDictionary(part => part.Uri.ToString(), StringComparer.Ordinal); + foreach (var sourcePart in EnumeratePackageParts(_doc!)) + { + var cached = sourcePart.Annotation(); + if (cached is null) continue; + if (!cloneParts.TryGetValue(sourcePart.Uri.ToString(), out var clonePart)) + throw new InvalidOperationException( + $"package clone omitted part {sourcePart.Uri}"); + // Avoid reserializing an unchanged cached tree: XML declarations, BOMs, and + // prefix placement are package payload too. A semantic comparison lets the clone + // preserve the original part bytes when the cache is merely a read-through, while + // still overlaying every genuinely dirty cached document. + var clonedXml = clonePart.GetXDocument(); + if (XNode.DeepEquals(cached.Root, clonedXml.Root)) continue; + clonePart.PutXDocument(new XDocument(cached)); + } + clone.Save(); + } + return ZipPackageOutputNormalizer.Normalize(stream.ToArray()); + } + + private static IEnumerable EnumeratePackageParts(OpenXmlPackage package) + { + var pending = new Stack(package.Parts.Select(pair => pair.OpenXmlPart)); + var seen = new HashSet(StringComparer.Ordinal); + while (pending.Count > 0) + { + var part = pending.Pop(); + if (!seen.Add(part.Uri.ToString())) continue; + yield return part; + foreach (var child in part.Parts) + pending.Push(child.OpenXmlPart); + } + } + internal void RestoreSnapshot(DocumentSnapshot snapshot) { + if (snapshot.PackageBytes is { } packageBytes) + { + RestorePackage(packageBytes); + _version = snapshot.Version; + if (snapshot.RevisionCounter is { } revisionCounter) + _revisionCounter = revisionCounter; + if (snapshot.LastFormatRevisionTicks is { } formatTicks) + _lastFormatRevisionTicks = formatTicks; + return; + } + var byUri = snapshot.Parts.ToDictionary(p => p.PartUri, p => p.Xml); // Restore content for all parts that exist in both snapshot and document. @@ -10209,6 +10793,20 @@ internal void RestoreSnapshot(DocumentSnapshot snapshot) InvalidateProjectionCache(); } + private void RestorePackage(byte[] packageBytes) + { + DisposeRenderShell(); + _doc?.Dispose(); + _stream?.Dispose(); + + _stream = new MemoryStream(packageBytes.Length); + _stream.Write(packageBytes, 0, packageBytes.Length); + _stream.Position = 0; + _doc = WordprocessingDocument.Open(_stream, isEditable: true); + _raw = null; + InvalidateProjectionCache(); + } + /// /// Reconcile the live document's header/footer parts against : /// delete parts created since the snapshot (relationship id present live, absent in snapshot) diff --git a/Docxodus/Internal/DocxSessionJson.cs b/Docxodus/Internal/DocxSessionJson.cs index 89ca521b..1964af50 100644 --- a/Docxodus/Internal/DocxSessionJson.cs +++ b/Docxodus/Internal/DocxSessionJson.cs @@ -917,6 +917,159 @@ public static string SerializeEditResults(IReadOnlyList results) return sb.ToString(); } + /// Parse one standard EditResult envelope or an array of them for batch adapters. + public static IReadOnlyList DeserializeEditResults(string json) + { + using var doc = JsonDocument.Parse(json); + return doc.RootElement.ValueKind == JsonValueKind.Array + ? doc.RootElement.EnumerateArray().Select(ParseEditResult).ToArray() + : new[] { ParseEditResult(doc.RootElement) }; + } + + private static EditResult ParseEditResult(JsonElement root) + { + if (root.ValueKind != JsonValueKind.Object + || !root.TryGetProperty("success", out var success) + || success.ValueKind is not (JsonValueKind.True or JsonValueKind.False)) + return EditResult.Fail(EditErrorCode.InternalError, "batch step returned a non-EditResult payload"); + + EditError? error = null; + if (root.TryGetProperty("error", out var e) && e.ValueKind == JsonValueKind.Object) + { + var codeText = TryGetString(e, "code", "internal_error") ?? "internal_error"; + var code = Enum.GetValues() + .Where(c => string.Equals(EnumToSnake(c), codeText, StringComparison.Ordinal)) + .Cast() + .FirstOrDefault() ?? EditErrorCode.InternalError; + error = new EditError( + code, + TryGetString(e, "message", "batch step failed") ?? "batch step failed", + TryGetString(e, "anchorId", null)); + if (e.TryGetProperty("precondition", out var p) && p.ValueKind == JsonValueKind.Object) + { + PreconditionTarget? target = null; + if (p.TryGetProperty("currentTarget", out var t) && t.ValueKind == JsonValueKind.Object) + { + target = new PreconditionTarget + { + Exists = t.TryGetProperty("exists", out var exists) && exists.ValueKind == JsonValueKind.True, + AnchorId = TryGetString(t, "anchorId", null), + Kind = TryGetString(t, "kind", null), + Scope = TryGetString(t, "scope", null), + ContentHash = TryGetString(t, "contentHash", null), + VisibleText = TryGetString(t, "visibleText", null), + }; + } + error = error with + { + Precondition = new PreconditionFailure( + TryGetString(p, "condition", "unknown") ?? "unknown", + p.TryGetProperty("expected", out var expected) ? expected.Clone() : null, + p.TryGetProperty("actual", out var actual) ? actual.Clone() : null, + p.TryGetProperty("currentVersion", out var version) && version.ValueKind == JsonValueKind.Number + ? version.GetInt64() : 0, + target), + }; + } + } + + static IReadOnlyList Anchors(JsonElement root, string name) + { + if (!root.TryGetProperty(name, out var a) || a.ValueKind != JsonValueKind.Array) + return Array.Empty(); + return a.EnumerateArray() + .Where(x => x.ValueKind == JsonValueKind.Object) + .Select(x => new Anchor( + TryGetString(x, "id", "") ?? "", + TryGetString(x, "kind", "") ?? "", + TryGetString(x, "scope", "") ?? "", + TryGetString(x, "unid", "") ?? "")) + .ToArray(); + } + + MarkdownPatch? patch = null; + if (root.TryGetProperty("patch", out var pch) && pch.ValueKind == JsonValueKind.Object) + patch = new MarkdownPatch( + TryGetString(pch, "scopeAnchorId", "") ?? "", + TryGetString(pch, "markdown", "") ?? ""); + + return new EditResult + { + Success = success.GetBoolean(), + Error = error, + Created = Anchors(root, "created"), + Removed = Anchors(root, "removed"), + Modified = Anchors(root, "modified"), + AnnotationId = TryGetString(root, "annotationId", null), + Patch = patch, + }; + } + + /// Common structured wire shape for core and transport mutation batches. + public static string SerializeMutationBatchResult(MutationBatchResult result) + { + var sb = new StringBuilder(512); + var mode = result.Mode == MutationBatchMode.Atomic ? "atomic" : "best_effort"; + var status = result.Success ? "ok" + : result.Mode == MutationBatchMode.BestEffort && result.Steps.Any(s => s.Success) + ? "partial" : "failed"; + sb.Append("{\"mode\":").Append(JsonString(mode)) + .Append(",\"status\":").Append(JsonString(status)) + .Append(",\"success\":").Append(result.Success ? "true" : "false") + .Append(",\"rolledBack\":").Append(result.RolledBack ? "true" : "false") + .Append(",\"steps\":["); + for (int i = 0; i < result.Steps.Count; i++) + { + if (i > 0) sb.Append(','); + var step = result.Steps[i]; + sb.Append("{\"index\":").Append(step.Index) + .Append(",\"tool\":").Append(JsonString(step.Tool)) + .Append(",\"action\":").Append(JsonString(step.Action)) + .Append(",\"success\":").Append(step.Success ? "true" : "false") + .Append(",\"rolledBack\":").Append(step.RolledBack ? "true" : "false") + .Append(",\"results\":").Append(SerializeEditResults(step.Results)) + .Append('}'); + } + sb.Append(']') + .Append(",\"editsApplied\":").Append( + result.RolledBack ? 0 : result.Steps.Count(s => s.Success)) + .Append(",\"results\":["); + for (int i = 0; i < result.Steps.Count; i++) + { + if (i > 0) sb.Append(','); + var stepResults = result.Steps[i].Results; + sb.Append(stepResults.Count == 1 + ? Serialize(stepResults[0]) + : SerializeEditResults(stepResults)); + } + sb.Append("],\"errors\":["); + bool firstError = true; + foreach (var step in result.Steps.Where(s => !s.Success)) + { + var failedError = step.Results.FirstOrDefault(r => !r.Success)?.Error; + if (failedError is null) continue; + if (!firstError) sb.Append(','); + firstError = false; + var failedJson = Serialize(new EditResult { Success = false, Error = failedError }); + using var failedDoc = JsonDocument.Parse(failedJson); + sb.Append(failedDoc.RootElement.GetProperty("error").GetRawText()); + } + sb.Append(']'); + if (result.Failure is { } failure) + { + var errorJson = Serialize(new EditResult { Success = false, Error = failure.Error }); + using var errorDoc = JsonDocument.Parse(errorJson); + sb.Append(",\"failure\":{\"index\":").Append(failure.Index) + .Append(",\"tool\":").Append(JsonString(failure.Tool)) + .Append(",\"action\":").Append(JsonString(failure.Action)) + .Append(",\"error\":").Append(errorDoc.RootElement.GetProperty("error").GetRawText()) + .Append(",\"rolledBack\":").Append(failure.RolledBack ? "true" : "false") + .Append('}'); + } + sb.Append('}'); + return sb.ToString(); + } + public static void AppendAnchorArray(StringBuilder sb, IReadOnlyList anchors) { sb.Append('['); diff --git a/Docxodus/Internal/DocxSessionOps.cs b/Docxodus/Internal/DocxSessionOps.cs index 328298e6..551bca1d 100644 --- a/Docxodus/Internal/DocxSessionOps.cs +++ b/Docxodus/Internal/DocxSessionOps.cs @@ -87,6 +87,26 @@ public static string CheckPreconditions(int handle, MutationPreconditions? preco internal static void RestorePreviewVersion(int handle, long version) => SessionRegistry.Get(handle).RestorePreviewVersion(version); + public static string ExecuteBatch( + int handle, + MutationBatchMode mode, + System.Collections.Generic.IEnumerable steps) => + DocxSessionJson.SerializeMutationBatchResult( + SessionRegistry.Get(handle).ExecuteBatch(steps, mode)); + + public static DocxSessionTransaction BeginTransaction(int handle) => + SessionRegistry.Get(handle).BeginTransaction(); + + internal static MutationBatchStep SerializedBatchStep( + string tool, + string action, + System.Func mutation, + System.Func? preflight = null) => new( + tool, + action, + _ => DocxSessionJson.DeserializeEditResults(mutation()), + preflight is null ? null : _ => preflight()); + // ─── Projection + discovery ───────────────────────────────────────── public static string Project(int handle) => diff --git a/Docxodus/Internal/UndoRing.cs b/Docxodus/Internal/UndoRing.cs index dd4dafb0..0d6a783b 100644 --- a/Docxodus/Internal/UndoRing.cs +++ b/Docxodus/Internal/UndoRing.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Linq; namespace Docxodus.Internal; @@ -44,6 +45,18 @@ internal sealed class UndoRing private readonly record struct Entry(T Snapshot, long CostBytes); + /// + /// Opaque copy of both history stacks. Snapshot payloads are intentionally shared rather + /// than cloned: they are immutable after insertion, and a transaction checkpoint only needs + /// to retain entries that trimming or redo invalidation might otherwise discard. + /// + internal sealed record State( + IReadOnlyList<(T Snapshot, long CostBytes)> Undo, + IReadOnlyList<(T Snapshot, long CostBytes)> Redo, + long UndoBytes, + long RedoBytes, + bool EvictedForMemory); + /// Maximum number of undo entries. Values <= 0 clamp to 1. /// Approximate byte budget for retained snapshots, counting the /// undo and redo sides together. Values <= 0 disable the budget bound (depth only). @@ -135,6 +148,33 @@ public void Clear() _undoBytes = 0; } + /// Capture the exact undo/redo topology for an enclosing transaction. + public State CaptureState() => new( + _undo.Select(e => (e.Snapshot, e.CostBytes)).ToArray(), + _redo.Select(e => (e.Snapshot, e.CostBytes)).ToArray(), + _undoBytes, + _redoBytes, + EvictedForMemory); + + /// + /// Restore a transaction checkpoint without invoking mutation/version callbacks. This also + /// resurrects entries trimmed while speculative steps were running and restores the sticky + /// memory-eviction flag, so a rolled-back batch is invisible to history diagnostics. + /// + public void RestoreState(State state) + { + ArgumentNullException.ThrowIfNull(state); + _undo.Clear(); + _redo.Clear(); + foreach (var (snapshot, costBytes) in state.Undo) + _undo.AddLast(new Entry(snapshot, costBytes)); + foreach (var (snapshot, costBytes) in state.Redo) + _redo.AddLast(new Entry(snapshot, costBytes)); + _undoBytes = state.UndoBytes; + _redoBytes = state.RedoBytes; + EvictedForMemory = state.EvictedForMemory; + } + private void ClearRedo() { _redo.Clear(); diff --git a/docs/architecture/docx_agent_server.md b/docs/architecture/docx_agent_server.md index 2f276b5e..5e863ad6 100644 --- a/docs/architecture/docx_agent_server.md +++ b/docs/architecture/docx_agent_server.md @@ -411,22 +411,34 @@ whole-document resolution: they transform via `RevisionProcessor` and swap the s underlying handle in place (`SessionStore.Rebind`), which also covers the exotic families the per-revision listing does not enumerate (see Known gaps). -### `docxodus_mutations` — batch apply or dry-run preview +### `docxodus_mutations` — atomic batches, explicit partial apply, or legacy preview `steps: [{ tool, args }]` where `tool` is one of `docxodus_edit`/`docxodus_format`/ `docxodus_create`/`docxodus_table`/`docxodus_list`/`docxodus_comment` (their `undo`/`redo` and read-only actions — e.g. `get_membership`, comment `list` — are rejected as steps; a batch is a -sequence of *mutations*). `mode: -apply` runs every step and returns a `{ status, editsApplied, results, errors }` receipt (`status` -is `ok`/`partial`/`failed`). `mode: preview` runs every step exactly the same way, then calls +sequence of *mutations*). + +`mode: atomic` is the recommended default. The server preflights every supported +action, required argument/enum, and step precondition against the batch-start state +before step zero. Success creates one undo entry and advances the version once. Any +failed or thrown step restores the complete DOCX package, relationships, session +state, version, and undo/redo cursors; the receipt identifies the failing +`index`/`tool`/`action`/`error` and reports `rolledBack: true`. + +`mode: best_effort` is the explicit partial-success mode. It runs every step in +order and evaluates a step preflight immediately before that step, returning a +`{ status, editsApplied, results, errors }`-compatible receipt (`status` is +`ok`/`partial`/`failed`). `mode: apply` is a deprecated compatibility alias for +`best_effort`; new clients should use the risk-signaling spelling. `mode: preview` runs every step exactly the same way, then calls `DocxSessionOps.Undo` once per step that actually mutated before returning — see Known gaps for why this is "apply-then-undo" rather than a true no-op dry run. The batch itself and each step's `args` may carry `preconditions`, using the same camel-case guard object as the core API (`expectedVersion`, `anchorId`, `expectedContentHash`, exact text/range/kind/scope, and `expectedMatchCount`). A -failure is the standard structured `precondition_failed` result and does not -mutate that step. `docxodus_get_content` with `format: "version"` reads the current +failure is the standard structured `precondition_failed` result. Atomic mode +evaluates all step guards at the common batch-start boundary; best-effort mode +evaluates them sequentially. `docxodus_get_content` with `format: "version"` reads the current monotonic document version; `format: "check_preconditions"` evaluates guards without mutating. Preview restores its starting version after undoing speculative steps, so a dry-run does not make an otherwise-current plan stale. diff --git a/docs/architecture/docx_mutation_api.md b/docs/architecture/docx_mutation_api.md index b9f132c5..30f7ea80 100644 --- a/docs/architecture/docx_mutation_api.md +++ b/docs/architecture/docx_mutation_api.md @@ -14,7 +14,7 @@ Three design forces, in order of weight: **The agent must not learn OOXML.** Every public method takes an anchor id (a string) and either a markdown payload (a string) or a small typed value (a `FormatOp`, a `CharSpan`). The agent never sees an `XElement`, never picks an SDK type, never has to know that bold is `w:b` inside `w:rPr`. The Raw escape hatch exists for the cases the markdown subset can't reach, but it's a separate namespace (`session.Raw.*`) so it's syntactically obvious when you've left the safe zone. -**Edits must be reversible.** Agents make mistakes. The session keeps a bounded ring of pre-op snapshots (default 50 deep) so `Undo()` and `Redo()` work without the caller orchestrating anything. Snapshots are per-part XML clones, not full package round-trips, so the cost is proportional to the size of the part the op touched — usually just the body. +**Edits must be reversible.** Agents make mistakes. The session keeps a bounded ring of pre-op snapshots (default 20 deep) so `Undo()` and `Redo()` work without the caller orchestrating anything. Ordinary single-op snapshots are per-part XML clones; an explicit transaction uses a complete package checkpoint because a batch can change arbitrary parts and relationships. **Errors must be pattern-matchable, not stringly-typed.** Every mutation returns an `EditResult` envelope; failure carries a typed `EditErrorCode` with a remediation message. The same enum is exposed as a snake-case string union in TypeScript, so JS agents pattern-match the same way C# callers do. No method on the session throws across the boundary (the constructor and `Save()` are the only places that can — and only for fatal conditions like an invalid DOCX or IO failure). @@ -79,6 +79,46 @@ that attaches the guard to each mutation request; stdio accepts top-level property. MCP batches may additionally carry a batch-start guard. Preview mode restores the starting version after it undoes its speculative edits. +## Atomic batches and transactions + +`ExecuteBatch(steps, mode)` is atomic by default. Each `MutationBatchStep` names a +tool/action for diagnostics, supplies a synchronous mutation callback, and may +provide a read-only preflight callback: + +```csharp +var result = session.ExecuteBatch(new[] +{ + new MutationBatchStep("docx_edit", "replace_text", + s => s.ReplaceText(firstAnchor, "First replacement")), + new MutationBatchStep("docx_create", "set_header_text", + s => s.SetHeaderText(firstAnchor, HeaderFooterKind.Default, "Confidential")), +}); +``` + +Atomic mode evaluates every available preflight against the batch-start state +before step zero. A successful batch is one caller-visible version advancement and +one undo/redo unit, regardless of its step count. A failed result or thrown step +restores document content, all part/relationship topology, annotations/custom XML, +anchor and revision generators, mutable tracking configuration, version, and both +history cursors. The structured failure identifies `index`, `tool`, `action`, +`error`, and `rolledBack`; failure consumes no history and advances no version. + +`MutationBatchMode.BestEffort` must be selected explicitly. It preserves partial +successes and evaluates each step's preflight immediately before that step, so a +later preflight can observe state made by an earlier successful mutation. + +`BeginTransaction()` exposes the same full-package checkpoint for façade code that +needs callback composition. Transactions are synchronous, same-thread, and strict +LIFO, but nested scopes are supported: inner commits remain speculative, and the +outer commit squashes all nested work into one history/version unit. Dispose without +`Commit()` rolls back. A validation failure from wrong-thread or out-of-order +completion leaves the scope active and recoverable. Disposing the owning session on +the owner thread abandons all scopes and releases their mutation-gate entries. + +The same semantics reach `DocxSessionOps`/JSON, WASM and npm +(`session.executeBatch`), stdio and Python (`session.execute_batch`), and MCP +(`docxodus_mutations`). Preview isolation is intentionally separate work in #446. + ## Architecture ``` diff --git a/npm/README.md b/npm/README.md index 9cee148f..4df6ccd5 100644 --- a/npm/README.md +++ b/npm/README.md @@ -99,6 +99,23 @@ Bundler users get the same API from `docxodus/embed`; classic-script pages can l `convertWmlToMarkdown()` renders the document as markdown where **every block carries a stable id**, so an agent can point at a clause and edit it — and `openDocxSession()` writes back to that same id. +Multi-step plans are atomic by default. Each callback may call any synchronous +session mutation; success is one version/undo unit, while a failure or throw +restores the complete package and history checkpoint: + +```ts +const result = session.executeBatch([ + { tool: 'docx_edit', action: 'replace_text', + mutation: () => session.replaceText(firstAnchor, 'Replacement text') }, + { tool: 'docx_create', action: 'set_header_text', + mutation: () => session.setHeaderText(firstAnchor, 'default', 'Confidential') }, +]); + +if (!result.success) console.error(result.failure); +``` + +Pass `'best_effort'` explicitly only when partial successes should be retained. + ![Markdown projection beside the rendered document](https://raw.githubusercontent.com/JSv4/Docxodus/main/docs/images/projection.png) --- diff --git a/npm/src/session.ts b/npm/src/session.ts index 9bbc6cc7..f55e7e54 100644 --- a/npm/src/session.ts +++ b/npm/src/session.ts @@ -42,6 +42,11 @@ import type { ListFormat, GrepOptions, ListMembership, + MutationBatchFailure, + MutationBatchMode, + MutationBatchResult, + MutationBatchStep, + MutationBatchStepResult, MutationPreconditions, ReplaceOptions, RevisionListEntry, @@ -126,6 +131,114 @@ export class DocxSession { return checked.success ? mutation() : checked; } + /** + * Execute synchronous mutations atomically by default. Atomic success is one undo/version + * unit; any failed or thrown step restores the exact package and history checkpoint. + */ + executeBatch( + steps: readonly MutationBatchStep[], + mode: MutationBatchMode = "atomic", + ): MutationBatchResult { + if (mode !== "atomic" && mode !== "best_effort") { + throw new RangeError(`unknown mutation batch mode: ${String(mode)}`); + } + const internalFailure = (value: unknown): EditResult => ({ + success: false, + error: { code: "internal_error", message: value instanceof Error ? value.message : String(value) }, + created: [], removed: [], modified: [], + }); + const run = (step: MutationBatchStep): readonly EditResult[] => { + try { + const value = step.mutation(); + const results = Array.isArray(value) ? value : [value]; + if (results.length === 0 || results.some(result => + result === null || typeof result !== "object" || typeof result.success !== "boolean")) { + return [internalFailure("batch mutation returned no valid edit results")]; + } + return results; + } catch (error) { + return [internalFailure(error)]; + } + }; + const failureOf = ( + step: MutationBatchStepResult, + rolledBack: boolean, + ): MutationBatchFailure => ({ + index: step.index, + tool: step.tool, + action: step.action, + error: step.results.find(result => !result.success)?.error + ?? { code: "internal_error", message: "batch step failed without an error" }, + rolledBack, + }); + + const preflightOne = (step: MutationBatchStep): EditError | undefined => { + try { return step.preflight?.(); } catch (error) { return internalFailure(error).error; } + }; + if (mode === "atomic") { + const preflight = steps.map(preflightOne); + const failedPreflight = preflight.findIndex(error => error !== undefined); + if (failedPreflight >= 0) { + const source = steps[failedPreflight]!; + const failed: MutationBatchStepResult = { + index: failedPreflight, tool: source.tool, action: source.action, + success: false, rolledBack: true, + results: [{ success: false, error: preflight[failedPreflight]!, created: [], removed: [], modified: [] }], + }; + return { mode, status: "failed", success: false, rolledBack: true, + steps: [failed], failure: failureOf(failed, true) }; + } + + const transaction = this.wasm.BeginTransaction(this.handle); + const completed: MutationBatchStepResult[] = []; + try { + for (let index = 0; index < steps.length; index++) { + const source = steps[index]!; + const results = run(source); + const step: MutationBatchStepResult = { + index, tool: source.tool, action: source.action, + success: results.every(result => result.success), rolledBack: false, results, + }; + completed.push(step); + if (!step.success) { + this.wasm.RollbackTransaction(transaction); + const rolledBack = completed.map(value => ({ ...value, rolledBack: true })); + const failed = rolledBack[rolledBack.length - 1]!; + return { mode, status: "failed", success: false, rolledBack: true, + steps: rolledBack, failure: failureOf(failed, true) }; + } + } + this.wasm.CommitTransaction(transaction); + return { mode, status: "ok", success: true, rolledBack: false, steps: completed }; + } catch (error) { + try { this.wasm.RollbackTransaction(transaction); } catch { /* preserve the original */ } + throw error; + } + } + + // Preserve sequential best-effort semantics: a later preflight can observe state created by + // an earlier successful step, so run it immediately before that step rather than up front. + const completed: MutationBatchStepResult[] = steps.map((source, index) => { + const preflight = preflightOne(source); + const results = preflight + ? [{ success: false, error: preflight, created: [], removed: [], modified: [] }] + : run(source); + return { + index, tool: source.tool, action: source.action, + success: results.every(result => result.success), rolledBack: false, results, + }; + }); + const failed = completed.find(step => !step.success); + return { + mode, + status: failed ? (completed.some(step => step.success) ? "partial" : "failed") : "ok", + success: failed === undefined, + rolledBack: false, + steps: completed, + failure: failed ? failureOf(failed, false) : undefined, + }; + } + /** * Project a slice of the document keyed off an anchor — useful for showing * one section to an LLM at a time without paying the cost of projecting the @@ -1333,5 +1446,5 @@ export function openDocxSession( return new DocxSession(handle, bridge); } -export type { AnchorInfo, AnchorRef, AnchorTargetRef, BlockSlice, CharSpan, CommentListEntry, CrossBlockMatch, DocumentAnnotation, DocxSessionProjection, DocxSessionSettings, EditError, EditErrorCode, EditResult, FindOptions, FormatOp, GrepOptions, MarkdownPatch, MutationPreconditions, PageCitation, PageCitationRequest, PageMapRegistrationResult, PageMapStatus, PlaceholderKind, PreconditionFailure, PreconditionTarget, ReplaceOptions, RunFormatting, RunFragment, TemplatePlaceholder, TextMatch, TextRangePrecondition } from "./types.js"; +export type { AnchorInfo, AnchorRef, AnchorTargetRef, BlockSlice, CharSpan, CommentListEntry, CrossBlockMatch, DocumentAnnotation, DocxSessionProjection, DocxSessionSettings, EditError, EditErrorCode, EditResult, FindOptions, FormatOp, GrepOptions, MarkdownPatch, MutationBatchFailure, MutationBatchMode, MutationBatchResult, MutationBatchStep, MutationBatchStepResult, MutationPreconditions, PageCitation, PageCitationRequest, PageMapRegistrationResult, PageMapStatus, PlaceholderKind, PreconditionFailure, PreconditionTarget, ReplaceOptions, RunFormatting, RunFragment, TemplatePlaceholder, TextMatch, TextRangePrecondition } from "./types.js"; export { ContextBoundary, PlaceholderKinds } from "./types.js"; diff --git a/npm/src/types.ts b/npm/src/types.ts index f7ec62e4..0d4e06ea 100644 --- a/npm/src/types.ts +++ b/npm/src/types.ts @@ -1054,6 +1054,9 @@ export interface DocxodusWasmExports { GetPageMapStatus: (handle: number, requestJson: string) => string; GetPageCitation: (handle: number, anchorId: string, requestJson: string) => string; CheckPreconditions: (handle: number, preconditionsJson: string) => string; + BeginTransaction: (handle: number) => number; + CommitTransaction: (transactionHandle: number) => void; + RollbackTransaction: (transactionHandle: number) => void; ProjectAnchor: (handle: number, anchorId: string, depth: number) => string; ProjectAnchorWithCitations: ( handle: number, @@ -1317,6 +1320,7 @@ export type EditErrorCode = | "empty_comment_span" | "revision_not_found" | "precondition_failed" + | "invalid_batch_step" | "internal_error"; export interface AnchorRef { @@ -1369,6 +1373,43 @@ export interface MutationPreconditions { expectedMatchCount?: number; } +export type MutationBatchMode = "atomic" | "best_effort"; + +/** One synchronous npm batch step. Atomic is the default execution mode. */ +export interface MutationBatchStep { + tool: string; + action: string; + mutation: () => EditResult | readonly EditResult[]; + /** Optional read-only validation: all run up front in atomic mode, per-step in best-effort. */ + preflight?: () => EditError | undefined; +} + +export interface MutationBatchStepResult { + index: number; + tool: string; + action: string; + success: boolean; + rolledBack: boolean; + results: readonly EditResult[]; +} + +export interface MutationBatchFailure { + index: number; + tool: string; + action: string; + error: EditError; + rolledBack: boolean; +} + +export interface MutationBatchResult { + mode: MutationBatchMode; + status: "ok" | "failed" | "partial"; + success: boolean; + rolledBack: boolean; + steps: readonly MutationBatchStepResult[]; + failure?: MutationBatchFailure; +} + export interface MarkdownPatch { scopeAnchorId: string; markdown: string; diff --git a/npm/tests/atomic-batch.spec.ts b/npm/tests/atomic-batch.spec.ts new file mode 100644 index 00000000..ad7daae5 --- /dev/null +++ b/npm/tests/atomic-batch.spec.ts @@ -0,0 +1,113 @@ +import { test, expect, Page } from '@playwright/test'; +import * as fs from 'fs'; +import * as path from 'path'; +import { fileURLToPath } from 'url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); +const fixture = new Uint8Array(fs.readFileSync( + path.join(__dirname, '../../TestFiles/HC001-5DayTourPlanTemplate.docx'), +)); + +async function waitForDocxodus(page: Page) { + await page.waitForFunction(() => (window as any).DocxodusReady === true, { timeout: 30000 }); +} + +test.describe('DocxSession atomic batches (#445)', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/test-harness.html'); + await waitForDocxodus(page); + }); + + test('rollback is exact and success is one version/undo unit', async ({ page }) => { + const result = await page.evaluate((bytes: number[]) => { + const session = (window as any).Docxodus.openTypedSession(new Uint8Array(bytes)); + try { + const projection = session.project(); + const anchors = (Object.entries(projection.anchorIndex) as [string, any][]) + .filter(([id, value]) => value.scope === 'body' + && ['p', 'h', 'li'].includes(value.kind) + && projection.markdown.includes(`{#${id}}`)) + .map(([id]) => id); + const before = projection.markdown; + + const failed = session.executeBatch([ + { tool: 'docx_edit', action: 'replace_text', + mutation: () => session.replaceText(anchors[0], 'Speculative npm edit.') }, + { tool: 'docx_edit', action: 'replace_text', + mutation: () => session.replaceText('p:body:missing', 'failure') }, + ]); + const afterFailure = session.project().markdown; + const versionAfterFailure = session.getVersion(); + const undoAfterFailure = session.undo(); + + const committed = session.executeBatch([ + { tool: 'docx_edit', action: 'replace_text', + mutation: () => session.replaceText(anchors[0], 'Committed npm first.') }, + { tool: 'docx_edit', action: 'replace_text', + mutation: () => session.replaceText(anchors[1], 'Committed npm second.') }, + ]); + const committedMarkdown = session.project().markdown; + const committedVersion = session.getVersion(); + const undoCommitted = session.undo(); + + return { + failed, + restored: afterFailure === before, + versionAfterFailure, + undoAfterFailure, + committed, + committedMarkdown, + committedVersion, + undoCommitted, + restoredAfterUndo: session.project().markdown === before, + }; + } finally { + session.close(); + } + }, Array.from(fixture)); + + expect(result.failed.success).toBe(false); + expect(result.failed.rolledBack).toBe(true); + expect(result.failed.failure.index).toBe(1); + expect(result.failed.failure.error.code).toBe('anchor_not_found'); + expect(result.restored).toBe(true); + expect(result.versionAfterFailure).toBe(0); + expect(result.undoAfterFailure).toBe(false); + + expect(result.committed.success).toBe(true); + expect(result.committedVersion).toBe(1); + expect(result.committedMarkdown).toContain('Committed npm first.'); + expect(result.committedMarkdown).toContain('Committed npm second.'); + expect(result.undoCommitted).toBe(true); + expect(result.restoredAfterUndo).toBe(true); + }); + + test('best-effort preflight observes earlier sequential state', async ({ page }) => { + const result = await page.evaluate((bytes: number[]) => { + const session = (window as any).Docxodus.openTypedSession(new Uint8Array(bytes)); + try { + const projection = session.project(); + const anchors = (Object.entries(projection.anchorIndex) as [string, any][]) + .filter(([id, value]) => value.scope === 'body' + && ['p', 'h', 'li'].includes(value.kind) + && projection.markdown.includes(`{#${id}}`)) + .map(([id]) => id); + return session.executeBatch([ + { tool: 'docx_edit', action: 'replace_text', + mutation: () => session.replaceText(anchors[0], 'Sequential npm state.') }, + { tool: 'docx_edit', action: 'replace_text', + preflight: () => session.project().markdown.includes('Sequential npm state.') + ? undefined + : { code: 'precondition_failed', message: 'prior state missing' }, + mutation: () => session.replaceText(anchors[1], 'Observed npm state.') }, + ], 'best_effort'); + } finally { + session.close(); + } + }, Array.from(fixture)); + + expect(result.success).toBe(true); + expect(result.steps).toHaveLength(2); + }); +}); diff --git a/python/README.md b/python/README.md index 3b94683f..5afa7924 100644 --- a/python/README.md +++ b/python/README.md @@ -49,6 +49,34 @@ with open("filled.docx", "wb") as f: f.write(new_bytes) ``` +### Atomic mutation batches + +Use `execute_batch` when a plan spans several edits that must either all land or +all disappear. Atomic is the default; success is one version/undo unit and failure +returns the indexed operation error after restoring the complete package and +history state: + +```python +from docx_scalpel import MutationBatchStep + +result = session.execute_batch([ + MutationBatchStep("replace_text", { + "anchorId": first_p.id, + "markdown": "Replacement text", + }), + MutationBatchStep("set_header_text", { + "anchorId": first_p.id, + "kind": "default", + "markdown": "Confidential", + }), +]) +if not result.success: + print(result.failure.index, result.failure.action, result.failure.error) +``` + +Select `MutationBatchMode.BEST_EFFORT` explicitly only when retaining successful +steps after another step fails is intended. + The `with` block is the documented lifecycle path — it calls `session.close()` on the way out, which releases the session from the host's `SessionRegistry`. A `__del__` finalizer is a fallback for forgotten sessions but should not be relied on; interpreter shutdown may skip it. ## Why a subprocess? @@ -116,7 +144,7 @@ The `DocxSession` class exposes every op in `Docxodus.Internal.DocxSessionOps` a | Tier | Methods | |---|---| -| **Lifecycle** | `save`, `close`, `undo`, `redo`, `get_version`, `to_html`, `register_page_map`, `get_page_map_status`, `get_page_citation` | +| **Lifecycle** | `save`, `close`, `undo`, `redo`, `get_version`, `execute_batch`, `to_html`, `register_page_map`, `get_page_map_status`, `get_page_citation` | | **Projection** | `project`, `project_anchor` | | **Discovery** | `grep`, `grep_cross_block`, `find_placeholders`, `find_by_text`, `find_all_by_text`, `find_by_regex`, `find_by_kind`, `find_by_annotation`, `find_by_label`, `find_by_bookmark`, `list_annotations`, `exists`, `get_anchor_info`, `get_anchor_infos`, `get_edit_summary`, `remaining_placeholders`, `get_diff` | | **Inspection** | `get_block_metadata`, `get_block_metadatas`, `get_list_membership`, `get_section_info` | diff --git a/python/src/docx_scalpel/__init__.py b/python/src/docx_scalpel/__init__.py index b3f46b15..695b8798 100644 --- a/python/src/docx_scalpel/__init__.py +++ b/python/src/docx_scalpel/__init__.py @@ -45,6 +45,7 @@ HeaderFooterKind, LineSpacingRule, ListFormat, + MutationBatchMode, PageNumberField, ParagraphAlignment, PlaceholderKind, @@ -108,6 +109,10 @@ ListMembership, MarkdownPatch, MarkdownProjection, + MutationBatchFailure, + MutationBatchResult, + MutationBatchStep, + MutationBatchStepResult, MutationPreconditions, NumberFormat, ParagraphBorderEdge, @@ -195,6 +200,10 @@ "ListMembership", "MarkdownPatch", "MarkdownProjection", + "MutationBatchFailure", + "MutationBatchResult", + "MutationBatchStep", + "MutationBatchStepResult", "MutationPreconditions", "NumberFormat", "ParagraphBorderEdge", @@ -251,6 +260,7 @@ "EmptyParagraphMode", "HeaderFooterKind", "LineSpacingRule", + "MutationBatchMode", "ListFormat", "PageNumberField", "ParagraphAlignment", diff --git a/python/src/docx_scalpel/enums.py b/python/src/docx_scalpel/enums.py index 8785184f..e75018a6 100644 --- a/python/src/docx_scalpel/enums.py +++ b/python/src/docx_scalpel/enums.py @@ -17,6 +17,7 @@ "ParagraphAlignment", "ListFormat", "EditErrorCode", + "MutationBatchMode", "PlaceholderKind", "PlaceholderKinds", "ProjectionScopes", @@ -176,6 +177,7 @@ class EditErrorCode(str, Enum): EMPTY_ANNOTATION_SPAN = "empty_annotation_span" EMPTY_COMMENT_SPAN = "empty_comment_span" REVISION_NOT_FOUND = "revision_not_found" + INVALID_BATCH_STEP = "invalid_batch_step" INTERNAL_ERROR = "internal_error" @classmethod @@ -186,6 +188,13 @@ def _missing_(cls, value: object) -> "EditErrorCode": # type: ignore[override] return cls.INTERNAL_ERROR +class MutationBatchMode(str, Enum): + """Atomic is the safe default; best-effort explicitly retains partial successes.""" + + ATOMIC = "atomic" + BEST_EFFORT = "best_effort" + + class PlaceholderKind(str, Enum): """Discriminator for a single ``TemplatePlaceholder``.""" diff --git a/python/src/docx_scalpel/session.py b/python/src/docx_scalpel/session.py index c5a791a5..a7f94e7e 100644 --- a/python/src/docx_scalpel/session.py +++ b/python/src/docx_scalpel/session.py @@ -33,6 +33,7 @@ DiffFormat, HeaderFooterKind, ListFormat, + MutationBatchMode, PageNumberField, PlaceholderKinds, Position, @@ -68,6 +69,8 @@ HtmlOptions, ListMembership, MarkdownProjection, + MutationBatchResult, + MutationBatchStep, MutationPreconditions, NumberFormat, ParagraphFormatOp, @@ -506,6 +509,20 @@ def get_page_citation( ) ) + def execute_batch( + self, + steps: Iterable[MutationBatchStep], + mode: MutationBatchMode = MutationBatchMode.ATOMIC, + ) -> MutationBatchResult: + """Execute mutations atomically by default, or explicitly retain partial successes.""" + result = self._call( + "execute_batch", + {"mode": mode.value, "steps": [step.to_wire() for step in steps]}, + ) + if not isinstance(result, Mapping): + raise TypeError(f"execute_batch: expected object, got {result!r}") + return MutationBatchResult._from_wire(result) + def check_preconditions(self, preconditions: MutationPreconditions) -> EditResult: """Evaluate guards without mutating the document or advancing its version.""" return EditResult._from_wire( diff --git a/python/src/docx_scalpel/types.py b/python/src/docx_scalpel/types.py index f0a71fa3..899d2cd6 100644 --- a/python/src/docx_scalpel/types.py +++ b/python/src/docx_scalpel/types.py @@ -30,6 +30,7 @@ EmptyParagraphMode, HeaderFooterKind, LineSpacingRule, + MutationBatchMode, ParagraphAlignment, PlaceholderKind, PlaceholderKinds, @@ -57,6 +58,10 @@ "PreconditionFailure", "TextRangePrecondition", "MutationPreconditions", + "MutationBatchStep", + "MutationBatchStepResult", + "MutationBatchFailure", + "MutationBatchResult", "BlockMetadata", "BulkEditResult", "FillOptions", @@ -1123,6 +1128,79 @@ def _from_wire(cls, d: Mapping[str, Any]) -> "EditResult": ) +@dataclass(frozen=True, slots=True) +class MutationBatchStep: + """One standardized stdio batch operation and its normal method arguments.""" + + operation: str + args: Mapping[str, Any] = field(default_factory=dict) + + def to_wire(self) -> dict[str, Any]: + return {"operation": self.operation, "args": dict(self.args)} + + +@dataclass(frozen=True, slots=True) +class MutationBatchStepResult: + index: int + tool: str + action: str + success: bool + rolled_back: bool + results: tuple[EditResult, ...] + + @classmethod + def _from_wire(cls, d: Mapping[str, Any]) -> "MutationBatchStepResult": + return cls( + index=int(d["index"]), + tool=str(d.get("tool", "")), + action=str(d.get("action", "")), + success=bool(d.get("success", False)), + rolled_back=bool(d.get("rolledBack", False)), + results=tuple(EditResult._from_wire(r) for r in d.get("results", ())), + ) + + +@dataclass(frozen=True, slots=True) +class MutationBatchFailure: + index: int + tool: str + action: str + error: EditError + rolled_back: bool + + @classmethod + def _from_wire(cls, d: Mapping[str, Any]) -> "MutationBatchFailure": + return cls( + index=int(d["index"]), + tool=str(d.get("tool", "")), + action=str(d.get("action", "")), + error=EditError._from_wire(d["error"]), + rolled_back=bool(d.get("rolledBack", False)), + ) + + +@dataclass(frozen=True, slots=True) +class MutationBatchResult: + mode: MutationBatchMode + status: str + success: bool + rolled_back: bool + steps: tuple[MutationBatchStepResult, ...] + failure: MutationBatchFailure | None = None + + @classmethod + def _from_wire(cls, d: Mapping[str, Any]) -> "MutationBatchResult": + failure = d.get("failure") + return cls( + mode=MutationBatchMode(d.get("mode", "atomic")), + status=str(d.get("status", "failed")), + success=bool(d.get("success", False)), + rolled_back=bool(d.get("rolledBack", False)), + steps=tuple(MutationBatchStepResult._from_wire(s) for s in d.get("steps", ())), + failure=MutationBatchFailure._from_wire(failure) if failure else None, + ) + + @dataclass(frozen=True, slots=True) class CommentListEntry: """One native Word comment, in comments-part order — see ``Session.list_comments``. diff --git a/python/tests/test_atomic_batches.py b/python/tests/test_atomic_batches.py new file mode 100644 index 00000000..c5dcd9c1 --- /dev/null +++ b/python/tests/test_atomic_batches.py @@ -0,0 +1,114 @@ +"""Atomic and explicit best-effort mutation batches (issue #445).""" + +from __future__ import annotations + +from docx_scalpel import ( + EditErrorCode, + DocxSession, + MutationBatchMode, + MutationBatchStep, + open_session, +) + + +def _body_paragraphs(session: DocxSession) -> list[str]: + projection = session.project() + return [ + anchor.id + for anchor in projection.anchor_index.values() + if anchor.scope == "body" and anchor.kind in ("p", "h", "li") + ] + + +def test_atomic_batch_rolls_back_and_preserves_history(tour_plan_bytes: bytes) -> None: + with open_session(tour_plan_bytes) as session: + target = _body_paragraphs(session)[0] + before = session.project().markdown + + result = session.execute_batch( + [ + MutationBatchStep( + "replace_text", + {"anchorId": target, "markdown": "Speculative Python edit."}, + ), + MutationBatchStep( + "replace_text", + {"anchorId": "p:body:missing", "markdown": "must fail"}, + ), + ] + ) + + assert not result.success + assert result.rolled_back + assert result.mode is MutationBatchMode.ATOMIC + assert result.failure is not None + assert result.failure.index == 1 + assert result.failure.tool == "docx_scalpel" + assert result.failure.action == "replace_text" + assert result.failure.error.code is EditErrorCode.ANCHOR_NOT_FOUND + assert result.failure.rolled_back + assert all(step.rolled_back for step in result.steps) + assert session.project().markdown == before + assert session.get_version() == 0 + assert not session.undo() + + +def test_atomic_success_is_one_version_and_undo_unit(tour_plan_bytes: bytes) -> None: + with open_session(tour_plan_bytes) as session: + targets = _body_paragraphs(session)[:2] + + result = session.execute_batch( + [ + MutationBatchStep( + "replace_text", + {"anchorId": targets[0], "markdown": "Python batch first."}, + ), + MutationBatchStep( + "replace_text", + {"anchorId": targets[1], "markdown": "Python batch second."}, + ), + ] + ) + + assert result.success + assert result.status == "ok" + assert session.get_version() == 1 + assert session.undo() + assert "Python batch first." not in session.project().markdown + assert not session.undo() + + +def test_best_effort_is_explicit_and_invalid_steps_are_structured( + tour_plan_bytes: bytes, +) -> None: + with open_session(tour_plan_bytes) as session: + target = _body_paragraphs(session)[0] + result = session.execute_batch( + [ + MutationBatchStep( + "replace_text", + {"anchorId": target, "markdown": "Retained Python edit."}, + ), + MutationBatchStep( + "replace_text", + {"anchorId": "p:body:missing", "markdown": "failure"}, + ), + ], + MutationBatchMode.BEST_EFFORT, + ) + + assert not result.success + assert not result.rolled_back + assert result.status == "partial" + assert result.failure is not None + assert not result.failure.rolled_back + assert "Retained Python edit." in session.project().markdown + assert session.get_version() == 1 + + with open_session(tour_plan_bytes) as session: + invalid = session.execute_batch([MutationBatchStep("get_version")]) + assert not invalid.success + assert invalid.rolled_back + assert invalid.failure is not None + assert invalid.failure.error.code is EditErrorCode.INVALID_BATCH_STEP + assert session.get_version() == 0 diff --git a/tools/mcp-server/Dispatcher.cs b/tools/mcp-server/Dispatcher.cs index a77dbd79..d315f905 100644 --- a/tools/mcp-server/Dispatcher.cs +++ b/tools/mcp-server/Dispatcher.cs @@ -673,8 +673,10 @@ private static string FilterRevisions(string revisionsJson, string? author, stri private static string Mutations(SessionStore store, JsonElement args) { var session = Session(store, args); - var mode = Str(args, "mode"); - if (mode is not ("apply" or "preview")) + var mode = args.TryGetProperty("mode", out _) + ? Str(args, "mode") + : "atomic"; + if (mode is not ("atomic" or "best_effort" or "apply" or "preview")) throw new McpToolException($"unknown docxodus_mutations mode: {mode}"); if (!args.TryGetProperty("steps", out var stepsEl) || stepsEl.ValueKind != JsonValueKind.Array) throw new McpToolException("docxodus_mutations requires an array \"steps\""); @@ -682,6 +684,19 @@ private static string Mutations(SessionStore store, JsonElement args) var batchCheck = Check(session, ParsePreconditions(args, MutationTarget(args))); if (batchCheck is not null) return batchCheck; + // #445: committed batch modes run through the core transaction primitive. `apply` is the + // backward-compatible alias for the old partial executor and is reported as best_effort; + // new callers should spell that risk explicitly. Preview remains the pre-existing + // apply-then-undo path until isolated previews land separately in #446. + if (mode != "preview") + { + var steps = BuildMutationBatchSteps(session, stepsEl, legacyApply: mode == "apply"); + var coreMode = mode == "atomic" + ? MutationBatchMode.Atomic + : MutationBatchMode.BestEffort; + return DocxSessionOps.ExecuteBatch(session.Handle, coreMode, steps); + } + var results = new List(); var errors = new List(); var startingVersion = DocxSessionOps.GetVersion(session.Handle); @@ -768,6 +783,424 @@ private static string Mutations(SessionStore store, JsonElement args) + ",\"errors\":[" + string.Join(",", errors) + "]}"; } + private static IReadOnlyList BuildMutationBatchSteps( + DocSession session, + JsonElement stepsEl, + bool legacyApply) + { + var result = new List(); + foreach (var step in stepsEl.EnumerateArray()) + { + var stepTool = step.TryGetProperty("tool", out var toolEl) && toolEl.ValueKind == JsonValueKind.String + ? toolEl.GetString()! : throw new McpToolException("mutation step missing string \"tool\""); + var stepArgs = step.TryGetProperty("args", out var a) && a.ValueKind == JsonValueKind.Object + ? a : throw new McpToolException("mutation step missing object \"args\""); + var action = stepArgs.TryGetProperty("action", out var actEl) && actEl.ValueKind == JsonValueKind.String + ? actEl.GetString()! : throw new McpToolException("mutation step args missing string \"action\""); + + var actionError = ValidateMutationBatchAction(stepTool, action); + if (legacyApply && actionError is not null) + throw new McpToolException(actionError.Message); + // Step preconditions are evaluated by the core batch preflight: all against the + // batch-start state for atomic mode, immediately before each step for best-effort. + // Remove them from the actual dispatch so a valid atomic preflight is not evaluated + // a second time against state changed by an earlier step in the same batch. + var mutationArgs = WithoutProperty(stepArgs, "preconditions"); + result.Add(DocxSessionOps.SerializedBatchStep( + stepTool, + action, + () => stepTool switch + { + "docxodus_edit" => RunEditAction(session, action, mutationArgs), + "docxodus_format" => RunFormatAction(session, action, mutationArgs), + "docxodus_create" => RunCreateAction(session, action, mutationArgs), + "docxodus_table" => RunTableAction(session, action, mutationArgs), + "docxodus_list" => RunListAction(session, action, mutationArgs), + "docxodus_comment" => RunCommentAction(session, action, mutationArgs), + _ => throw new McpToolException($"docxodus_mutations does not accept \"{stepTool}\" as a step"), + }, + () => ValidateMutationBatchStep(session, stepTool, action, stepArgs))); + } + return result; + } + + private static EditError? ValidateMutationBatchAction(string tool, string action) + { + bool known = tool switch + { + "docxodus_edit" => action is "insert_paragraph" or "replace_text" or "replace_text_range" + or "delete_block" or "move_block" or "delete_range" or "delete_section" + or "split_paragraph" or "merge_paragraphs", + "docxodus_format" => action is "apply_format" or "apply_format_by_substring" + or "set_paragraph_style" or "set_paragraph_format" or "set_list_level" + or "remove_list_membership" or "apply_list_format", + "docxodus_create" => action is "insert_paragraph" or "insert_heading" or "insert_table" + or "insert_horizontal_rule" or "insert_footnote" or "insert_endnote" + or "insert_page_number_field" or "set_header_text" or "set_footer_text" + or "ensure_header_footer_visible", + "docxodus_table" => action is "insert" or "insert_row" or "insert_column" + or "delete_row" or "delete_column" or "replace_cell_content" or "merge_cells" + or "unmerge_cells" or "set_column_widths" or "set_borders" or "set_shading" + or "set_repeat_header_row" or "set_row_options", + "docxodus_list" => action is "apply_format" or "apply_format_range" or "set_level" + or "set_start" or "clear_start" or "remove", + "docxodus_comment" => action is "add" or "reply" or "resolve" or "update" or "remove", + _ => false, + }; + return known ? null : new EditError( + EditErrorCode.InvalidBatchStep, + $"unsupported or read-only batch action: {tool}/{action}"); + } + + private static EditError? ValidateMutationBatchStep( + DocSession session, + string tool, + string action, + JsonElement args) + { + var actionError = ValidateMutationBatchAction(tool, action); + if (actionError is not null) return actionError; + + try + { + ValidateMutationBatchArguments(tool, action, args); + var failure = Check(session, ParsePreconditions(args, MutationTarget(args))); + if (failure is null) return null; + return DocxSessionJson.DeserializeEditResults(failure).FirstOrDefault()?.Error + ?? new EditError(EditErrorCode.PreconditionFailed, + "batch step precondition failed"); + } + catch (Exception ex) when (ex is McpToolException + or ArgumentException or FormatException or JsonException or OverflowException) + { + return new EditError(EditErrorCode.InvalidBatchStep, ex.Message); + } + } + + /// + /// Parse every syntactic input that a mutation action will consume, without invoking the + /// mutation. This keeps caller-attributable schema/enum errors out of InternalError and lets + /// atomic mode reject the complete batch before step zero changes the package. + /// + private static void ValidateMutationBatchArguments(string tool, string action, JsonElement args) + { + switch ((tool, action)) + { + case ("docxodus_edit", "insert_paragraph"): + RequireStrings(args, "anchorId", "markdown"); + ValidateOptionalEnum(args, "position", "before", "after"); + break; + case ("docxodus_edit", "replace_text"): + RequireStrings(args, "anchorId", "markdown"); + break; + case ("docxodus_edit", "replace_text_range"): + RequireStrings(args, "anchorId", "find", "replace"); + ValidateOptionalBool(args, "caseSensitive"); + break; + case ("docxodus_edit", "delete_block"): + RequireStrings(args, "anchorId"); + break; + case ("docxodus_edit", "move_block"): + RequireStrings(args, "sourceAnchorId", "targetAnchorId"); + ValidateOptionalEnum(args, "position", "before", "after"); + break; + case ("docxodus_edit", "delete_range"): + RequireStrings(args, "fromAnchorId", "toAnchorIdExclusive"); + break; + case ("docxodus_edit", "delete_section"): + RequireStrings(args, "headingAnchorId"); + break; + case ("docxodus_edit", "split_paragraph"): + RequireStrings(args, "anchorId"); + RequireNumbers(args, "characterOffset"); + break; + case ("docxodus_edit", "merge_paragraphs"): + RequireStrings(args, "anchorId", "secondAnchorId"); + break; + + case ("docxodus_format", "apply_format"): + RequireStrings(args, "anchorId"); + ValidateOptionalObject(args, "format"); + ValidateOptionalSpan(args, "span"); + _ = ParseFormatOp(args); + break; + case ("docxodus_format", "apply_format_by_substring"): + RequireStrings(args, "anchorId", "substring"); + ValidateOptionalObject(args, "format"); + _ = ParseFormatOp(args); + break; + case ("docxodus_format", "set_paragraph_style"): + RequireStrings(args, "anchorId", "styleId"); + break; + case ("docxodus_format", "set_paragraph_format"): + RequireStrings(args, "anchorId"); + ValidateOptionalObject(args, "paragraphFormat"); + _ = ParseParagraphFormatOp(args); + break; + case ("docxodus_format", "set_list_level"): + RequireStrings(args, "anchorId"); + RequireNumbers(args, "levelDelta"); + break; + case ("docxodus_format", "remove_list_membership"): + RequireStrings(args, "anchorId"); + break; + case ("docxodus_format", "apply_list_format"): + RequireStrings(args, "anchorId"); + ValidateOptionalListFormat(args); + break; + + case ("docxodus_create", "insert_paragraph"): + RequireStrings(args, "anchorId", "markdown"); + ValidateOptionalEnum(args, "position", "before", "after"); + break; + case ("docxodus_create", "insert_heading"): + RequireStrings(args, "anchorId", "text"); + RequireNumbers(args, "level"); + ValidateOptionalEnum(args, "position", "before", "after"); + break; + case ("docxodus_create", "insert_table"): + RequireStrings(args, "anchorId"); + RequireNumbers(args, "rows", "columns"); + ValidateOptionalEnum(args, "position", "before", "after"); + ValidateOptionalArray(args, "cellContents"); + ValidateOptionalArray(args, "columnWidths"); + ValidateOptionalEnum(args, "cellAlignment", "left", "center", "right", "justify"); + ValidateOptionalBool(args, "borderless"); + break; + case ("docxodus_create", "insert_horizontal_rule"): + RequireStrings(args, "anchorId"); + ValidateOptionalEnum(args, "position", "before", "after"); + ValidateOptionalEnum(args, "ruleStyle", "single", "double", "thick"); + break; + case ("docxodus_create", "insert_footnote"): + case ("docxodus_create", "insert_endnote"): + RequireStrings(args, "anchorId", "markdown"); + RequireNumbers(args, "characterOffset"); + break; + case ("docxodus_create", "insert_page_number_field"): + RequireStrings(args, "anchorId"); + ValidateOptionalEnum(args, "field", "current_page", "total_pages"); + ValidateOptionalEnum(args, "numberFormat", "decimal", "upperLetter", + "lowerLetter", "upperRoman", "lowerRoman"); + break; + case ("docxodus_create", "set_header_text"): + case ("docxodus_create", "set_footer_text"): + RequireStrings(args, "bodyAnchorId", "kind", "markdown"); + ValidateRequiredEnum(args, "kind", "default", "first", "even"); + break; + case ("docxodus_create", "ensure_header_footer_visible"): + RequireStrings(args, "bodyAnchorId", "kind"); + ValidateRequiredEnum(args, "kind", "default", "first", "even"); + break; + + case ("docxodus_table", "insert"): + RequireStrings(args, "anchorId"); + RequireNumbers(args, "rows", "columns"); + ValidateOptionalEnum(args, "position", "before", "after"); + ValidateOptionalArray(args, "cellContents"); + ValidateOptionalArray(args, "columnWidths"); + ValidateOptionalEnum(args, "cellAlignment", "left", "center", "right", "justify"); + ValidateOptionalBool(args, "borderless"); + break; + case ("docxodus_table", "insert_row"): + case ("docxodus_table", "insert_column"): + RequireStrings(args, "cellAnchorId"); + ValidateOptionalEnum(args, "position", "before", "after"); + break; + case ("docxodus_table", "delete_row"): + case ("docxodus_table", "delete_column"): + case ("docxodus_table", "unmerge_cells"): + RequireStrings(args, "cellAnchorId"); + break; + case ("docxodus_table", "replace_cell_content"): + RequireStrings(args, "cellAnchorId", "markdown"); + break; + case ("docxodus_table", "merge_cells"): + RequireStrings(args, "cellAnchorId"); + ValidateOptionalNumber(args, "rowSpan"); + ValidateOptionalNumber(args, "colSpan"); + ValidateOptionalEnum(args, "mergeContent", "append", "discard", "reject"); + break; + case ("docxodus_table", "set_column_widths"): + RequireStrings(args, "cellAnchorId"); + _ = RawArray(args, "widths"); + break; + case ("docxodus_table", "set_borders"): + RequireStrings(args, "cellAnchorId"); + ValidateOptionalEnum(args, "borderScope", "all", "outside", "inside"); + ValidateOptionalString(args, "borderStyle"); + ValidateOptionalNumber(args, "borderSize"); + ValidateOptionalString(args, "borderColor"); + break; + case ("docxodus_table", "set_shading"): + RequireStrings(args, "cellAnchorId"); + ValidateOptionalString(args, "fill"); + ValidateOptionalEnum(args, "shadingScope", "cell", "row"); + break; + case ("docxodus_table", "set_repeat_header_row"): + RequireStrings(args, "cellAnchorId"); + ValidateOptionalBool(args, "repeat"); + break; + case ("docxodus_table", "set_row_options"): + RequireStrings(args, "cellAnchorId"); + ValidateOptionalBool(args, "repeat"); + ValidateOptionalBool(args, "allowBreakAcrossPages"); + ValidateOptionalNumber(args, "heightTwips"); + ValidateOptionalEnum(args, "heightRule", "auto", "atLeast", "exact"); + break; + + case ("docxodus_list", "apply_format"): + RequireStrings(args, "anchorId"); + ValidateOptionalListFormat(args); + break; + case ("docxodus_list", "apply_format_range"): + RequireStrings(args, "firstAnchorId", "lastAnchorId"); + ValidateOptionalListFormat(args); + break; + case ("docxodus_list", "set_level"): + RequireStrings(args, "anchorId"); + RequireNumbers(args, "levelDelta"); + break; + case ("docxodus_list", "set_start"): + RequireStrings(args, "anchorId"); + RequireNumbers(args, "startValue"); + break; + case ("docxodus_list", "clear_start"): + case ("docxodus_list", "remove"): + RequireStrings(args, "anchorId"); + break; + + case ("docxodus_comment", "add"): + ValidateCommentAddArguments(args); + break; + case ("docxodus_comment", "reply"): + RequireStrings(args, "commentAnchorId", "author"); + ValidateOptionalString(args, "initials"); + ValidateOptionalString(args, "date"); + ValidateOptionalString(args, "markdown"); + break; + case ("docxodus_comment", "update"): + RequireStrings(args, "commentAnchorId", "markdown"); + break; + case ("docxodus_comment", "resolve"): + RequireStrings(args, "commentAnchorId"); + ValidateOptionalBool(args, "resolved"); + break; + case ("docxodus_comment", "remove"): + RequireStrings(args, "commentAnchorId"); + break; + } + } + + private static void ValidateCommentAddArguments(JsonElement args) + { + var anchorId = OptionalStringValue(args, "anchorId"); + var revisionId = OptionalStringValue(args, "revisionId"); + if ((anchorId is null) == (revisionId is null)) + throw new McpToolException( + "docxodus_comment add requires exactly one target: anchorId or revisionId"); + RequireStrings(args, "author"); + ValidateOptionalString(args, "initials"); + ValidateOptionalString(args, "date"); + ValidateOptionalString(args, "markdown"); + if (revisionId is not null && args.TryGetProperty("span", out _)) + throw new McpToolException("revisionId comment targets cannot include span"); + ValidateOptionalSpan(args, "span"); + } + + private static void RequireStrings(JsonElement args, params string[] names) + { + foreach (var name in names) _ = Str(args, name); + } + + private static void RequireNumbers(JsonElement args, params string[] names) + { + foreach (var name in names) _ = Int(args, name); + } + + private static string? OptionalStringValue(JsonElement args, string name) + { + if (!args.TryGetProperty(name, out var value)) return null; + if (value.ValueKind != JsonValueKind.String) + throw new McpToolException($"argument \"{name}\" must be a string"); + return value.GetString(); + } + + private static void ValidateOptionalString(JsonElement args, string name) => + _ = OptionalStringValue(args, name); + + private static void ValidateOptionalBool(JsonElement args, string name) + { + if (args.TryGetProperty(name, out var value) + && value.ValueKind is not (JsonValueKind.True or JsonValueKind.False)) + throw new McpToolException($"argument \"{name}\" must be a boolean"); + } + + private static void ValidateOptionalNumber(JsonElement args, string name) + { + if (args.TryGetProperty(name, out var value) && value.ValueKind != JsonValueKind.Number) + throw new McpToolException($"argument \"{name}\" must be a number"); + if (args.TryGetProperty(name, out value)) _ = value.GetInt32(); + } + + private static void ValidateOptionalObject(JsonElement args, string name) + { + if (args.TryGetProperty(name, out var value) && value.ValueKind != JsonValueKind.Object) + throw new McpToolException($"argument \"{name}\" must be an object"); + } + + private static void ValidateOptionalArray(JsonElement args, string name) + { + if (args.TryGetProperty(name, out var value) && value.ValueKind != JsonValueKind.Array) + throw new McpToolException($"argument \"{name}\" must be an array"); + } + + private static void ValidateOptionalSpan(JsonElement args, string name) + { + if (!args.TryGetProperty(name, out var span)) return; + if (span.ValueKind != JsonValueKind.Object) + throw new McpToolException($"argument \"{name}\" must be an object"); + if (span.TryGetProperty("start", out var start)) + { + if (start.ValueKind != JsonValueKind.Number) + throw new McpToolException($"argument \"{name}.start\" must be a number"); + _ = start.GetInt32(); + } + if (span.TryGetProperty("length", out var length)) + { + if (length.ValueKind != JsonValueKind.Number) + throw new McpToolException($"argument \"{name}.length\" must be a number"); + _ = length.GetInt32(); + } + } + + private static void ValidateOptionalListFormat(JsonElement args) => + ValidateOptionalEnum(args, "listFormat", "bullet", "decimal", "lowerLetter", + "upperLetter", "lowerRoman", "upperRoman", "decimalParenthesis", + "lowerLetterParenthesis", "upperLetterParenthesis", "lowerRomanParenthesis", + "upperRomanParenthesis", "none"); + + private static void ValidateRequiredEnum(JsonElement args, string name, params string[] values) + { + _ = Str(args, name); + ValidateOptionalEnum(args, name, values); + } + + private static void ValidateOptionalEnum(JsonElement args, string name, params string[] values) + { + var value = OptionalStringValue(args, name); + if (value is not null && !values.Contains(value, StringComparer.Ordinal)) + throw new McpToolException( + $"unknown {name}: {value}; expected one of {string.Join(", ", values)}"); + } + + private static JsonElement WithoutProperty(JsonElement source, string propertyName) + { + var values = JsonSerializer.Deserialize>(source.GetRawText())!; + values.Remove(propertyName); + return JsonSerializer.SerializeToElement(values); + } + // ─── Table ────────────────────────────────────────────────────────── private static string Table(SessionStore store, JsonElement args) diff --git a/tools/mcp-server/README.md b/tools/mcp-server/README.md index 1718f535..47d4e4fe 100644 --- a/tools/mcp-server/README.md +++ b/tools/mcp-server/README.md @@ -94,7 +94,7 @@ markdown projection and search tools return: | `docxodus_comment` | Native Word review comments (real `w:comment` markup): add on an anchor/span or tracked revision id, reply in-thread, resolve/reopen, update, remove, list | | `docxodus_annotate` | Anchor-addressed highlight/label annotations (a custom-XML overlay for external tools, distinct from comments) | | `docxodus_track_changes` | List tracked changes; accept/reject one by id, or all | -| `docxodus_mutations` | Apply or dry-run-preview a batch of the above as one call | +| `docxodus_mutations` | Apply a batch atomically by default; opt into best-effort; legacy apply/preview remain available | | `docxodus_table` | Create/read tables; resolve canonical cell anchors ↔ grid coordinates; edit rows/columns/cell content/style | ## Known gaps diff --git a/tools/mcp-server/ToolCatalog.cs b/tools/mcp-server/ToolCatalog.cs index 734dc2ff..652efcb2 100644 --- a/tools/mcp-server/ToolCatalog.cs +++ b/tools/mcp-server/ToolCatalog.cs @@ -425,14 +425,14 @@ internal static class ToolCatalog """), new ToolDefinition( "docxodus_mutations", - "Apply (or preview) a batch of docxodus_edit/docxodus_format/docxodus_create/docxodus_table/docxodus_list/docxodus_comment actions as one atomic-feeling sequence, with a single aggregate result.", + "Apply a batch of docxodus_edit/docxodus_format/docxodus_create/docxodus_table/docxodus_list/docxodus_comment actions atomically by default, with explicit best-effort and legacy preview modes.", """ { "type": "object", "properties": { "sessionId": { "type": "string" }, "preconditions": { "type": "object", "description": "Optional batch-start guards. Each step args object may also carry its own preconditions." }, - "mode": { "type": "string", "enum": ["apply", "preview"], "description": "preview applies every step, records the result, then undoes them all before returning — nothing is left changed." }, + "mode": { "type": "string", "enum": ["atomic", "best_effort", "apply", "preview"], "default": "atomic", "description": "atomic (default): all steps commit as one undo/version unit or fully roll back. best_effort: explicitly retain successful steps after failures. apply: deprecated alias for best_effort. preview: legacy apply-then-undo behavior; isolated previews are tracked separately in #446." }, "steps": { "type": "array", "items": { @@ -445,7 +445,7 @@ internal static class ToolCatalog } } }, - "required": ["sessionId", "mode", "steps"] + "required": ["sessionId", "steps"] } """), new ToolDefinition( diff --git a/tools/python-host/Dispatcher.cs b/tools/python-host/Dispatcher.cs index a1cb1840..0cb4af0a 100644 --- a/tools/python-host/Dispatcher.cs +++ b/tools/python-host/Dispatcher.cs @@ -75,6 +75,7 @@ public static string Dispatch(string op, JsonElement args) DocxSessionJson.ParsePageCitationRequest(args) ?? throw new FormatException("args missing object \"citation\"")), "check_preconditions" => DocxSessionOps.CheckPreconditions(Handle(args), ParsePreconditions(args)), + "execute_batch" => ExecuteBatch(args), "replace_text" => DocxSessionOps.ReplaceText(Handle(args), Str(args, "anchorId"), Str(args, "markdown")), "delete_block" => DocxSessionOps.DeleteBlock(Handle(args), Str(args, "anchorId")), @@ -614,6 +615,51 @@ private static string[] ParseAnchorIdArray(JsonElement args) }; } + private static string ExecuteBatch(JsonElement args) + { + var handle = Handle(args); + var mode = args.TryGetProperty("mode", out var m) && m.ValueKind == JsonValueKind.String + ? m.GetString() : "atomic"; + var batchMode = mode switch + { + "atomic" => MutationBatchMode.Atomic, + "best_effort" => MutationBatchMode.BestEffort, + _ => throw new ArgumentException($"unknown batch mode: {mode}"), + }; + if (!args.TryGetProperty("steps", out var steps) || steps.ValueKind != JsonValueKind.Array) + throw new ArgumentException("execute_batch requires an array 'steps'"); + + var parsed = new List(); + foreach (var step in steps.EnumerateArray()) + { + if (step.ValueKind != JsonValueKind.Object) + throw new ArgumentException("each batch step must be an object"); + var operation = step.TryGetProperty("operation", out var op) && op.ValueKind == JsonValueKind.String + ? op.GetString()! : throw new ArgumentException("batch step missing string 'operation'"); + var stepArgs = step.TryGetProperty("args", out var a) && a.ValueKind == JsonValueKind.Object + ? WithHandle(a, handle) : WithHandle(default, handle); + EditError? preflight = !IsMutation(operation) || operation is "undo" or "redo" + ? new EditError(EditErrorCode.InvalidBatchStep, + $"unsupported or non-mutation batch operation: {operation}") + : null; + parsed.Add(DocxSessionOps.SerializedBatchStep( + "docx_scalpel", + operation, + () => Dispatch(operation, stepArgs), + preflight is null ? null : () => preflight)); + } + return DocxSessionOps.ExecuteBatch(handle, batchMode, parsed); + } + + private static JsonElement WithHandle(JsonElement args, int handle) + { + var values = args.ValueKind == JsonValueKind.Object + ? JsonSerializer.Deserialize>(args.GetRawText())! + : new Dictionary(); + values["handle"] = JsonSerializer.SerializeToElement(handle); + return JsonSerializer.SerializeToElement(values); + } + private static MutationPreconditions? ParsePreconditions(JsonElement args) { if (args.ValueKind != JsonValueKind.Object diff --git a/wasm/DocxodusWasm/DocxSessionBridge.cs b/wasm/DocxodusWasm/DocxSessionBridge.cs index 00934599..373317c1 100644 --- a/wasm/DocxodusWasm/DocxSessionBridge.cs +++ b/wasm/DocxodusWasm/DocxSessionBridge.cs @@ -23,12 +23,28 @@ namespace DocxodusWasm; [SupportedOSPlatform("browser")] public static partial class DocxSessionBridge { + private static readonly Dictionary Transactions = new(); + private static int _nextTransactionHandle; + [JSExport] public static int OpenSession(byte[] bytes, string settingsJson) => DocxSessionOps.OpenSession(bytes, DocxSessionJson.ParseSettings(settingsJson)); [JSExport] - public static void CloseSession(int handle) => DocxSessionOps.CloseSession(handle); + public static void CloseSession(int handle) + { + foreach (var txHandle in Transactions + .Where(kv => kv.Value.SessionHandle == handle) + .Select(kv => kv.Key) + .OrderByDescending(x => x) + .ToArray()) + { + var transaction = Transactions[txHandle].Transaction; + Transactions.Remove(txHandle); + transaction.Rollback(); + } + DocxSessionOps.CloseSession(handle); + } /// Mint a complete blank DOCX (a "New document" seed) as bytes. [JSExport] @@ -73,6 +89,34 @@ public static string CheckPreconditions(int handle, string preconditionsJson) => DocxSessionOps.CheckPreconditions( handle, DocxSessionJson.ParseMutationPreconditions(preconditionsJson)); + /// Begin a synchronous nested-safe atomic scope for npm's callback batch API. + [JSExport] + public static int BeginTransaction(int handle) + { + var transaction = DocxSessionOps.BeginTransaction(handle); + var transactionHandle = checked(++_nextTransactionHandle); + Transactions.Add(transactionHandle, (handle, transaction)); + return transactionHandle; + } + + [JSExport] + public static void CommitTransaction(int transactionHandle) + { + if (!Transactions.TryGetValue(transactionHandle, out var entry)) + throw new ArgumentException($"unknown transaction handle: {transactionHandle}"); + entry.Transaction.Commit(); + Transactions.Remove(transactionHandle); + } + + [JSExport] + public static void RollbackTransaction(int transactionHandle) + { + if (!Transactions.TryGetValue(transactionHandle, out var entry)) + throw new ArgumentException($"unknown transaction handle: {transactionHandle}"); + entry.Transaction.Rollback(); + Transactions.Remove(transactionHandle); + } + /// /// Ordered top-level render units per scope container (body / footnotes / /// endnotes), as JSON: {"body":[{"id","kind"},…],"footnotes":[…],"endnotes":[…]}.