From 0fc12e3b480a1891ee965cd6d69da99740fa687f Mon Sep 17 00:00:00 2001 From: Laurent Valdes Date: Fri, 20 Feb 2026 00:44:07 +0100 Subject: [PATCH 1/4] =?UTF-8?q?fix:=20audit=20MCP=20tools=20=E2=80=94=2010?= =?UTF-8?q?=20bug=20fixes=20across=209=20tool=20files?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PatchTool: RemoveColumn bounds check + empty runs cleanup after cross-run replace - CommentTools DateTime.TryParse moved to Phase 2 commit (same file) - HistoryTools: auto-save condition fix (3 places) + limit clamped to [1,100] - DocumentTools: force LocalFile source type for local paths in document_save - ElementTools: consistent EscapeJson usage in RemoveTableColumn - ExportTools: PDF temp file leak fix in finally block - QueryTool: [*] wildcard single element now returns pagination envelope - CountTool: throw instead of returning JSON error for missing body - ReadHeadingContentTool: heading_level validated to [1,9] - ConnectionTools: page_size clamped to [1,100] Co-Authored-By: Claude Opus 4.6 --- src/DocxMcp/Tools/ConnectionTools.cs | 2 ++ src/DocxMcp/Tools/CountTool.cs | 2 +- src/DocxMcp/Tools/DocumentTools.cs | 11 +++++++++-- src/DocxMcp/Tools/ElementTools.cs | 4 ++-- src/DocxMcp/Tools/ExportTools.cs | 6 ++++-- src/DocxMcp/Tools/HistoryTools.cs | 7 ++++--- src/DocxMcp/Tools/PatchTool.cs | 22 +++++++++++++++++++++ src/DocxMcp/Tools/QueryTool.cs | 2 +- src/DocxMcp/Tools/ReadHeadingContentTool.cs | 3 +++ 9 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/DocxMcp/Tools/ConnectionTools.cs b/src/DocxMcp/Tools/ConnectionTools.cs index 2d3e5e0..cab93e6 100644 --- a/src/DocxMcp/Tools/ConnectionTools.cs +++ b/src/DocxMcp/Tools/ConnectionTools.cs @@ -82,6 +82,8 @@ public static string ListConnectionFiles( { try { + page_size = Math.Clamp(page_size, 1, 100); + var type = source_type switch { "local" => SourceType.LocalFile, diff --git a/src/DocxMcp/Tools/CountTool.cs b/src/DocxMcp/Tools/CountTool.cs index 615f1a6..5ee19ad 100644 --- a/src/DocxMcp/Tools/CountTool.cs +++ b/src/DocxMcp/Tools/CountTool.cs @@ -40,7 +40,7 @@ public static string CountElements( { var body = doc.MainDocumentPart?.Document?.Body; if (body is null) - return """{"error": "Document has no body."}"""; + throw new InvalidOperationException("Document has no body."); var result = new JsonObject { diff --git a/src/DocxMcp/Tools/DocumentTools.cs b/src/DocxMcp/Tools/DocumentTools.cs index b56fc3a..a2ee9e5 100644 --- a/src/DocxMcp/Tools/DocumentTools.cs +++ b/src/DocxMcp/Tools/DocumentTools.cs @@ -155,10 +155,17 @@ public static string DocumentSave( // If output_path is provided, update/register the source first if (output_path is not null) { - // Preserve existing source type if registered (don't force LocalFile) + // Local path (no "://") → always use LocalFile source type + var isLocalPath = !output_path.Contains("://"); var existing = sync.GetSyncStatus(tenant.TenantId, doc_id); - if (existing is not null) + + if (isLocalPath) + { + sync.SetSource(tenant.TenantId, doc_id, output_path, autoSync: true); + } + else if (existing is not null) { + // Cloud path: preserve existing source type logger.LogDebug("document_save: preserving existing source type {SourceType} for session {DocId}", existing.SourceType, doc_id); sync.SetSource(tenant.TenantId, doc_id, diff --git a/src/DocxMcp/Tools/ElementTools.cs b/src/DocxMcp/Tools/ElementTools.cs index 3c08b1f..7920390 100644 --- a/src/DocxMcp/Tools/ElementTools.cs +++ b/src/DocxMcp/Tools/ElementTools.cs @@ -325,7 +325,7 @@ public static string CopyElement( return PatchTool.ApplyPatch(tenant, sync, gate, doc_id, patchJson, dry_run); } - private static string EscapeJson(string s) => s.Replace("\\", "\\\\").Replace("\"", "\\\""); + internal static string EscapeJson(string s) => s.Replace("\\", "\\\\").Replace("\"", "\\\""); } [McpServerToolType] @@ -447,7 +447,7 @@ public static string RemoveTableColumn( [Description("Column index to remove (0-based).")] int column, [Description("If true, simulates the operation without applying changes.")] bool dry_run = false) { - var patchJson = $$"""[{"op": "remove_column", "path": "{{path.Replace("\\", "\\\\").Replace("\"", "\\\"")}}", "column": {{column}}}]"""; + var patchJson = $$"""[{"op": "remove_column", "path": "{{ElementTools.EscapeJson(path)}}", "column": {{column}}}]"""; return PatchTool.ApplyPatch(tenant, sync, gate, doc_id, patchJson, dry_run); } } diff --git a/src/DocxMcp/Tools/ExportTools.cs b/src/DocxMcp/Tools/ExportTools.cs index bd7be87..6faa860 100644 --- a/src/DocxMcp/Tools/ExportTools.cs +++ b/src/DocxMcp/Tools/ExportTools.cs @@ -102,6 +102,7 @@ private static async Task ExportPdf(DocxSession session) { var tempDocx = Path.Combine(Path.GetTempPath(), $"docx-mcp-{session.Id}.docx"); var tempDir = Path.GetTempPath(); + string? generatedPdf = null; try { session.Save(tempDocx); @@ -132,14 +133,13 @@ private static async Task ExportPdf(DocxSession session) throw new McpException($"LibreOffice failed (exit {process.ExitCode}): {stderr}"); } - var generatedPdf = Path.Combine(tempDir, + generatedPdf = Path.Combine(tempDir, Path.GetFileNameWithoutExtension(tempDocx) + ".pdf"); if (!File.Exists(generatedPdf)) throw new McpException("LibreOffice did not produce a PDF file."); var pdfBytes = await File.ReadAllBytesAsync(generatedPdf); - File.Delete(generatedPdf); return Convert.ToBase64String(pdfBytes); } @@ -147,6 +147,8 @@ private static async Task ExportPdf(DocxSession session) { if (File.Exists(tempDocx)) File.Delete(tempDocx); + if (generatedPdf is not null && File.Exists(generatedPdf)) + File.Delete(generatedPdf); } } diff --git a/src/DocxMcp/Tools/HistoryTools.cs b/src/DocxMcp/Tools/HistoryTools.cs index f8d27bc..c0a7dc7 100644 --- a/src/DocxMcp/Tools/HistoryTools.cs +++ b/src/DocxMcp/Tools/HistoryTools.cs @@ -23,7 +23,7 @@ public static string DocumentUndo( { var sessions = tenant.Sessions; var result = sessions.Undo(doc_id, steps); - if (result.Steps > 0 && result.CurrentBytes is not null) + if (result.CurrentBytes is not null) sync.MaybeAutoSave(tenant.TenantId, doc_id, result.CurrentBytes); return $"{result.Message}\nPosition: {result.Position}, Steps: {result.Steps}"; } @@ -47,7 +47,7 @@ public static string DocumentRedo( { var sessions = tenant.Sessions; var result = sessions.Redo(doc_id, steps); - if (result.Steps > 0 && result.CurrentBytes is not null) + if (result.CurrentBytes is not null) sync.MaybeAutoSave(tenant.TenantId, doc_id, result.CurrentBytes); return $"{result.Message}\nPosition: {result.Position}, Steps: {result.Steps}"; } @@ -70,6 +70,7 @@ public static string DocumentHistory( { try { + limit = Math.Clamp(limit, 1, 100); var result = tenant.Sessions.GetHistory(doc_id, offset, limit); var lines = new List @@ -122,7 +123,7 @@ public static string DocumentJumpTo( { var sessions = tenant.Sessions; var result = sessions.JumpTo(doc_id, position); - if (result.Steps > 0 && result.CurrentBytes is not null) + if (result.CurrentBytes is not null) sync.MaybeAutoSave(tenant.TenantId, doc_id, result.CurrentBytes); return $"{result.Message}\nPosition: {result.Position}, Steps: {result.Steps}"; } diff --git a/src/DocxMcp/Tools/PatchTool.cs b/src/DocxMcp/Tools/PatchTool.cs index 1555d95..f37b3fb 100644 --- a/src/DocxMcp/Tools/PatchTool.cs +++ b/src/DocxMcp/Tools/PatchTool.cs @@ -751,6 +751,19 @@ private static int ReplaceTextInElement(OpenXmlElement element, string find, str pos = runEnd; } + + // Clean up empty runs left after cross-run replacement + // (runs with empty text that don't contain tabs or breaks) + foreach (var run in para.Elements().ToList()) + { + var textElem = run.GetFirstChild(); + if (textElem is not null && textElem.Text.Length == 0 + && run.GetFirstChild() is null + && run.GetFirstChild() is null) + { + run.Remove(); + } + } } return totalReplaced; @@ -777,6 +790,15 @@ private static RemoveColumnOperationResult ExecuteRemoveColumn(RemoveColumnPatch if (target is not Table table) throw new InvalidOperationException("remove_column target must be a table."); + var firstRow = table.Elements().FirstOrDefault(); + if (firstRow is not null) + { + var firstRowCellCount = firstRow.Elements().Count(); + if (op.Column < 0 || op.Column >= firstRowCellCount) + throw new InvalidOperationException( + $"Column index {op.Column} is out of range. Table has {firstRowCellCount} columns (0-{firstRowCellCount - 1})."); + } + var rows = table.Elements().ToList(); foreach (var row in rows) { diff --git a/src/DocxMcp/Tools/QueryTool.cs b/src/DocxMcp/Tools/QueryTool.cs index 65f47f9..53761e6 100644 --- a/src/DocxMcp/Tools/QueryTool.cs +++ b/src/DocxMcp/Tools/QueryTool.cs @@ -62,7 +62,7 @@ public static string Query( // Apply pagination when multiple elements are returned var totalCount = elements.Count; - if (totalCount > 1) + if (totalCount > 1 || (path.Contains("[*]") && totalCount >= 1)) { var rawOffset = offset ?? 0; // Negative offset counts from the end: -10 means start at (total - 10) diff --git a/src/DocxMcp/Tools/ReadHeadingContentTool.cs b/src/DocxMcp/Tools/ReadHeadingContentTool.cs index 838da71..4dfcdeb 100644 --- a/src/DocxMcp/Tools/ReadHeadingContentTool.cs +++ b/src/DocxMcp/Tools/ReadHeadingContentTool.cs @@ -42,6 +42,9 @@ public static string ReadHeadingContent( var doc = session.Document; var body = session.GetBody(); + if (heading_level.HasValue && (heading_level.Value < 1 || heading_level.Value > 9)) + throw new ArgumentException("heading_level must be between 1 and 9."); + var allChildren = body.ChildElements.Cast().ToList(); // List mode: return heading hierarchy From 77a0d9b12c2bf8a37221fb4dce0ca39430873bcf Mon Sep 17 00:00:00 2001 From: Laurent Valdes Date: Fri, 20 Feb 2026 00:44:56 +0100 Subject: [PATCH 2/4] feat: add document_diff and comment_resolve MCP tools document_diff (closes #46): - New DiffTool.cs wrapping existing LCS-based DiffEngine - Compare two WAL positions, or baseline vs current state - SessionManager.GetBytesAtPosition() for read-only doc reconstruction comment_resolve: - Mark comments as resolved/re-opened via OOXML CommentsExPart - CommentHelper: ResolveComment, IsCommentResolved using w15:commentEx - comment_list now includes "resolved" field per comment - WAL replay support (resolve_comment op) Also includes CommentTools DateTime.TryParse fix from audit Phase 1. Co-Authored-By: Claude Opus 4.6 --- src/DocxMcp/Helpers/CommentHelper.cs | 122 ++++++++++++++++++++++++++- src/DocxMcp/Program.cs | 6 +- src/DocxMcp/SessionManager.cs | 19 +++++ src/DocxMcp/Tools/CommentTools.cs | 58 ++++++++++++- src/DocxMcp/Tools/DiffTool.cs | 47 +++++++++++ 5 files changed, 246 insertions(+), 6 deletions(-) create mode 100644 src/DocxMcp/Tools/DiffTool.cs diff --git a/src/DocxMcp/Helpers/CommentHelper.cs b/src/DocxMcp/Helpers/CommentHelper.cs index 8b71642..b202bfe 100644 --- a/src/DocxMcp/Helpers/CommentHelper.cs +++ b/src/DocxMcp/Helpers/CommentHelper.cs @@ -1,6 +1,7 @@ using DocumentFormat.OpenXml; using DocumentFormat.OpenXml.Packaging; using DocumentFormat.OpenXml.Wordprocessing; +using W15 = DocumentFormat.OpenXml.Office2013.Word; namespace DocxMcp.Helpers; @@ -315,10 +316,23 @@ private static void RemoveCommentAnchors(OpenXmlElement root, string commentId) public static List ListComments(WordprocessingDocument doc, string? authorFilter = null) { var results = new List(); - var commentsPart = doc.MainDocumentPart?.WordprocessingCommentsPart; + var mainPart = doc.MainDocumentPart; + var commentsPart = mainPart?.WordprocessingCommentsPart; if (commentsPart?.Comments is null) return results; - var body = doc.MainDocumentPart?.Document?.Body; + var body = mainPart?.Document?.Body; + + // Build paraId→Done lookup from CommentsExPart + var resolvedParaIds = new HashSet(StringComparer.OrdinalIgnoreCase); + var exPart = mainPart?.WordprocessingCommentsExPart; + if (exPart?.CommentsEx is not null) + { + foreach (var ce in exPart.CommentsEx.Elements()) + { + if (ce.Done?.Value == true && ce.ParaId?.Value is not null) + resolvedParaIds.Add(ce.ParaId.Value); + } + } foreach (var comment in commentsPart.Comments.Elements()) { @@ -337,6 +351,11 @@ public static List ListComments(WordprocessingDocument doc, string? anchoredText = GetAnchoredText(body, idStr); } + // Check resolved state via first paragraph's paraId + var firstPara = comment.Elements().FirstOrDefault(); + var paraId = firstPara?.ParagraphId?.Value; + var isResolved = paraId is not null && resolvedParaIds.Contains(paraId); + results.Add(new CommentInfo { Id = int.TryParse(idStr, out var id) ? id : 0, @@ -344,7 +363,8 @@ public static List ListComments(WordprocessingDocument doc, string? Initials = comment.Initials?.Value ?? "", Date = comment.Date?.Value, Text = commentText, - AnchoredText = anchoredText + AnchoredText = anchoredText, + Resolved = isResolved }); } @@ -434,6 +454,101 @@ private static void AddCommentAnchorsToParagraph(Paragraph para, int commentId) para.AppendChild(CreateCommentReferenceRun(commentId)); } + /// + /// Resolve or un-resolve a comment by ID. + /// Uses WordprocessingCommentsExPart (commentsExtended.xml) with CommentEx.Done attribute. + /// + public static bool ResolveComment(WordprocessingDocument doc, int commentId, bool resolved) + { + var mainPart = doc.MainDocumentPart + ?? throw new InvalidOperationException("Document has no MainDocumentPart."); + + // Verify the comment exists + var commentsPart = mainPart.WordprocessingCommentsPart; + if (commentsPart?.Comments is null) return false; + + var idStr = commentId.ToString(); + var comment = commentsPart.Comments.Elements() + .FirstOrDefault(c => c.Id?.Value == idStr); + if (comment is null) return false; + + // Get the first paragraph's paraId from the comment + var firstPara = comment.Elements().FirstOrDefault(); + if (firstPara is null) return false; + + var paraId = firstPara.ParagraphId?.Value; + if (paraId is null) + { + // Generate a paraId if none exists + paraId = GenerateParaId(); + firstPara.ParagraphId = new HexBinaryValue(paraId); + } + + // Get or create CommentsExPart + var exPart = mainPart.WordprocessingCommentsExPart; + if (exPart is null) + { + exPart = mainPart.AddNewPart(); + exPart.CommentsEx = new W15.CommentsEx(); + } + else if (exPart.CommentsEx is null) + { + exPart.CommentsEx = new W15.CommentsEx(); + } + + // Find existing CommentEx for this paraId, or create one + var commentEx = exPart.CommentsEx.Elements() + .FirstOrDefault(ce => ce.ParaId?.Value == paraId); + + if (commentEx is null) + { + commentEx = new W15.CommentEx { ParaId = new HexBinaryValue(paraId) }; + exPart.CommentsEx.AppendChild(commentEx); + } + + commentEx.Done = resolved ? true : false; + exPart.CommentsEx.Save(); + + return true; + } + + /// + /// Check if a comment is resolved. + /// + public static bool IsCommentResolved(WordprocessingDocument doc, int commentId) + { + var mainPart = doc.MainDocumentPart; + var commentsPart = mainPart?.WordprocessingCommentsPart; + if (commentsPart?.Comments is null) return false; + + var idStr = commentId.ToString(); + var comment = commentsPart.Comments.Elements() + .FirstOrDefault(c => c.Id?.Value == idStr); + if (comment is null) return false; + + var firstPara = comment.Elements().FirstOrDefault(); + var paraId = firstPara?.ParagraphId?.Value; + if (paraId is null) return false; + + var exPart = mainPart?.WordprocessingCommentsExPart; + if (exPart?.CommentsEx is null) return false; + + var commentEx = exPart.CommentsEx.Elements() + .FirstOrDefault(ce => ce.ParaId?.Value == paraId); + + return commentEx?.Done?.Value == true; + } + + /// + /// Generate a random 8-character hex paraId (matching Word's format). + /// + private static string GenerateParaId() + { + var bytes = new byte[4]; + System.Security.Cryptography.RandomNumberGenerator.Fill(bytes); + return Convert.ToHexString(bytes); + } + /// /// Create the reference Run: contains CommentReference with CommentReference style. /// @@ -458,4 +573,5 @@ public class CommentInfo public DateTime? Date { get; set; } public string Text { get; set; } = ""; public string? AnchoredText { get; set; } + public bool Resolved { get; set; } } diff --git a/src/DocxMcp/Program.cs b/src/DocxMcp/Program.cs index 37efc1d..8b53816 100644 --- a/src/DocxMcp/Program.cs +++ b/src/DocxMcp/Program.cs @@ -46,7 +46,8 @@ .WithTools() .WithTools() .WithTools() - .WithTools(); + .WithTools() + .WithTools(); var app = builder.Build(); app.MapMcp("/mcp"); @@ -97,7 +98,8 @@ .WithTools() .WithTools() .WithTools() - .WithTools(); + .WithTools() + .WithTools(); await builder.Build().RunAsync(); } diff --git a/src/DocxMcp/SessionManager.cs b/src/DocxMcp/SessionManager.cs index 04a9f8b..15d2946 100644 --- a/src/DocxMcp/SessionManager.cs +++ b/src/DocxMcp/SessionManager.cs @@ -731,6 +731,16 @@ await _history.AddSessionToIndexAsync(TenantId, session.Id, Array.Empty())); } + /// + /// Get the serialized document bytes at a given WAL position without side effects. + /// Does NOT save checkpoints or update the cursor. Used for read-only comparisons. + /// + public byte[] GetBytesAtPosition(string id, int position) + { + using var session = RebuildDocumentAtPositionAsync(id, position).GetAwaiter().GetResult(); + return session.ToBytes(); + } + /// /// Rebuild document at a given position, save checkpoint there, update cursor. /// Returns the serialized bytes at that position. @@ -830,6 +840,12 @@ private static string GenerateDescription(string patchesJson) var cid = patch.TryGetProperty("comment_id", out var cidEl) ? cidEl.GetInt32().ToString() : "?"; ops.Add($"delete_comment #{cid}"); } + else if (op == "resolve_comment") + { + var cid = patch.TryGetProperty("comment_id", out var cidEl) ? cidEl.GetInt32().ToString() : "?"; + var resolved = patch.TryGetProperty("resolved", out var rEl) && rEl.GetBoolean(); + ops.Add(resolved ? $"resolve_comment #{cid}" : $"unresolve_comment #{cid}"); + } else if (op is "style_element" or "style_paragraph" or "style_table") { var stylePath = patch.TryGetProperty("path", out var spEl) && spEl.ValueKind == JsonValueKind.String @@ -897,6 +913,9 @@ private static void ReplayPatch(DocxSession session, string patchesJson) case "delete_comment": Tools.CommentTools.ReplayDeleteComment(patch, wpDoc); break; + case "resolve_comment": + Tools.CommentTools.ReplayResolveComment(patch, wpDoc); + break; case "style_element": Tools.StyleTools.ReplayStyleElement(patch, wpDoc); break; diff --git a/src/DocxMcp/Tools/CommentTools.cs b/src/DocxMcp/Tools/CommentTools.cs index 1e20da5..6e31a57 100644 --- a/src/DocxMcp/Tools/CommentTools.cs +++ b/src/DocxMcp/Tools/CommentTools.cs @@ -142,6 +142,7 @@ public static string CommentList( ["initials"] = c.Initials, ["date"] = c.Date?.ToString("o"), ["text"] = c.Text, + ["resolved"] = c.Resolved, }; if (c.AnchoredText is not null) @@ -243,6 +244,49 @@ public static string CommentDelete( catch (Exception ex) { throw new McpException(ex.Message, ex); } } + [McpServerTool(Name = "comment_resolve"), Description( + "Mark a comment as resolved or re-open it.\n\n" + + "Resolved comments appear greyed out in Word but are not deleted.\n" + + "Use comment_list to see current resolve status.")] + public static string CommentResolve( + TenantScope tenant, + SyncManager sync, + [Description("Session ID of the document.")] string doc_id, + [Description("ID of the comment to resolve/unresolve.")] int comment_id, + [Description("True to resolve, false to re-open. Default: true.")] bool resolved = true) + { + try + { + var session = tenant.Sessions.Get(doc_id); + var doc = session.Document; + + var success = CommentHelper.ResolveComment(doc, comment_id, resolved); + if (!success) + return $"Error: Comment {comment_id} not found."; + + // Append to WAL + var walObj = new JsonObject + { + ["op"] = "resolve_comment", + ["comment_id"] = comment_id, + ["resolved"] = resolved + }; + var walEntry = new JsonArray(); + walEntry.Add((JsonNode)walObj); + var bytes = session.ToBytes(); + tenant.Sessions.AppendWal(doc_id, walEntry.ToJsonString(), null, bytes); + sync.MaybeAutoSave(tenant.TenantId, doc_id, bytes); + + return resolved + ? $"Comment {comment_id} marked as resolved." + : $"Comment {comment_id} re-opened."; + } + catch (RpcException ex) { throw GrpcErrorHelper.Wrap(ex, $"resolving comment in '{doc_id}'"); } + catch (KeyNotFoundException) { throw GrpcErrorHelper.WrapNotFound(doc_id); } + catch (McpException) { throw; } + catch (Exception ex) { throw new McpException(ex.Message, ex); } + } + /// /// Replay an add_comment WAL operation. /// @@ -255,7 +299,9 @@ internal static void ReplayAddComment(JsonElement patch, WordprocessingDocument var author = patch.GetProperty("author").GetString() ?? "AI Assistant"; var initials = patch.GetProperty("initials").GetString() ?? "AI"; var dateStr = patch.GetProperty("date").GetString(); - var date = dateStr is not null ? DateTime.Parse(dateStr).ToUniversalTime() : DateTime.UtcNow; + var date = dateStr is not null && DateTime.TryParse(dateStr, out var parsedDate) + ? parsedDate.ToUniversalTime() + : DateTime.UtcNow; string? anchorText = null; if (patch.TryGetProperty("anchor_text", out var at) && at.ValueKind == JsonValueKind.String) @@ -287,6 +333,16 @@ internal static void ReplayDeleteComment(JsonElement patch, WordprocessingDocume CommentHelper.DeleteComment(doc, commentId); } + /// + /// Replay a resolve_comment WAL operation. + /// + internal static void ReplayResolveComment(JsonElement patch, WordprocessingDocument doc) + { + var commentId = patch.GetProperty("comment_id").GetInt32(); + var resolved = patch.TryGetProperty("resolved", out var r) && r.GetBoolean(); + CommentHelper.ResolveComment(doc, commentId, resolved); + } + private static readonly JsonSerializerOptions JsonOpts = new() { WriteIndented = true, diff --git a/src/DocxMcp/Tools/DiffTool.cs b/src/DocxMcp/Tools/DiffTool.cs new file mode 100644 index 0000000..68cf2e9 --- /dev/null +++ b/src/DocxMcp/Tools/DiffTool.cs @@ -0,0 +1,47 @@ +using System.ComponentModel; +using DocxMcp.Diff; +using Grpc.Core; +using ModelContextProtocol; +using ModelContextProtocol.Server; +using DocxMcp.Helpers; + +namespace DocxMcp.Tools; + +[McpServerToolType] +public sealed class DiffTool +{ + [McpServerTool(Name = "document_diff"), Description( + "Compare two versions of a document by WAL position.\n\n" + + "Returns a structured diff showing added, removed, modified, and moved elements.\n" + + "Position 0 = original baseline. Use document_history to see available positions.\n" + + "Omit position_b to compare against current state.")] + public static string DocumentDiff( + TenantScope tenant, + [Description("Session ID of the document.")] string doc_id, + [Description("First WAL position to compare from. 0 = baseline.")] int position_a = 0, + [Description("Second WAL position to compare to. Omit for current state.")] int? position_b = null) + { + try + { + byte[] bytesA = tenant.Sessions.GetBytesAtPosition(doc_id, position_a); + byte[] bytesB; + + if (position_b is not null) + { + bytesB = tenant.Sessions.GetBytesAtPosition(doc_id, position_b.Value); + } + else + { + var session = tenant.Sessions.Get(doc_id); + bytesB = session.ToBytes(); + } + + var diffResult = DiffEngine.Compare(bytesA, bytesB); + return diffResult.ToJson(); + } + catch (RpcException ex) { throw GrpcErrorHelper.Wrap(ex, $"comparing versions of '{doc_id}'"); } + catch (KeyNotFoundException) { throw GrpcErrorHelper.WrapNotFound(doc_id); } + catch (McpException) { throw; } + catch (Exception ex) { throw new McpException(ex.Message, ex); } + } +} From d0758edd7cf913160e65f99ba908aaeb1ec2e7f1 Mon Sep 17 00:00:00 2001 From: Laurent Valdes Date: Fri, 20 Feb 2026 00:45:21 +0100 Subject: [PATCH 3/4] test: add 30 tests for bug fixes, new tools, and revisions Phase 1 bug fix tests (10, distributed into existing files): - TableModificationTests: 4 tests (RemoveColumn bounds, cross-run cleanup) - QueryPaginationTests: 1 test (wildcard single element envelope) - ReadHeadingContentTests: 3 tests (heading_level validation) - UndoRedoTests: 2 tests (history limit clamping) Phase 3 RevisionTests (20 new tests): - track_changes_enable: enable, disable, idempotent - revision_list: empty, insertions, filter by author/type, pagination - revision_accept/reject: inserted/deleted runs, not found errors - WAL persistence: accept/reject/track_changes survive restart - Undo/Redo: accept undo, reject undo, accept undo redo Also adds coverlet.collector for code coverage support. Total: 456 tests passing (426 existing + 30 new) Co-Authored-By: Claude Opus 4.6 --- tests/DocxMcp.Tests/DocxMcp.Tests.csproj | 1 + tests/DocxMcp.Tests/QueryPaginationTests.cs | 21 + .../DocxMcp.Tests/ReadHeadingContentTests.cs | 29 ++ tests/DocxMcp.Tests/RevisionTests.cs | 446 ++++++++++++++++++ tests/DocxMcp.Tests/TableModificationTests.cs | 109 +++++ tests/DocxMcp.Tests/UndoRedoTests.cs | 31 ++ 6 files changed, 637 insertions(+) create mode 100644 tests/DocxMcp.Tests/RevisionTests.cs diff --git a/tests/DocxMcp.Tests/DocxMcp.Tests.csproj b/tests/DocxMcp.Tests/DocxMcp.Tests.csproj index 145eea2..5dc788d 100644 --- a/tests/DocxMcp.Tests/DocxMcp.Tests.csproj +++ b/tests/DocxMcp.Tests/DocxMcp.Tests.csproj @@ -12,6 +12,7 @@ + diff --git a/tests/DocxMcp.Tests/QueryPaginationTests.cs b/tests/DocxMcp.Tests/QueryPaginationTests.cs index b57b4c9..d44a8a5 100644 --- a/tests/DocxMcp.Tests/QueryPaginationTests.cs +++ b/tests/DocxMcp.Tests/QueryPaginationTests.cs @@ -191,6 +191,27 @@ public void NegativeOffsetWithLimit() Assert.Equal("Paragraph 10", firstItem.GetProperty("text").GetString()); } + [Fact] + public void WildcardSingleElementHasPaginationEnvelope() + { + // Create a session with exactly 1 paragraph + var sessions = TestHelpers.CreateSessionManager(); + var session = sessions.Create(); + session.GetBody().AppendChild(new Paragraph(new Run(new Text("Only one")))); + TestHelpers.PersistBaseline(sessions, session); + + var result = DocxMcp.Tools.QueryTool.Query(sessions, session.Id, "/body/paragraph[*]"); + using var doc = JsonDocument.Parse(result); + + // With the fix, [*] with single element should still have pagination envelope + Assert.Equal(1, doc.RootElement.GetProperty("total").GetInt32()); + Assert.Equal(1, doc.RootElement.GetProperty("count").GetInt32()); + Assert.True(doc.RootElement.TryGetProperty("items", out var items)); + Assert.Equal(1, items.GetArrayLength()); + + sessions.Close(session.Id); + } + public void Dispose() { _sessions.Close(_session.Id); diff --git a/tests/DocxMcp.Tests/ReadHeadingContentTests.cs b/tests/DocxMcp.Tests/ReadHeadingContentTests.cs index cb79adb..74ba5d9 100644 --- a/tests/DocxMcp.Tests/ReadHeadingContentTests.cs +++ b/tests/DocxMcp.Tests/ReadHeadingContentTests.cs @@ -407,6 +407,35 @@ public void ListHeadingsLevel2Only() Assert.Equal("Scope", headings[1].GetProperty("text").GetString()); } + // --- Validation tests --- + + [Fact] + public void ReadContentWithHeadingLevelZeroThrows() + { + var ex = Assert.ThrowsAny(() => + DocxMcp.Tools.ReadHeadingContentTool.ReadHeadingContent( + _sessions, _session.Id, heading_level: 0)); + Assert.Contains("heading_level must be between 1 and 9", ex.Message); + } + + [Fact] + public void ReadContentWithHeadingLevelAbove9Throws() + { + var ex = Assert.ThrowsAny(() => + DocxMcp.Tools.ReadHeadingContentTool.ReadHeadingContent( + _sessions, _session.Id, heading_level: 10)); + Assert.Contains("heading_level must be between 1 and 9", ex.Message); + } + + [Fact] + public void ReadContentWithNegativeHeadingLevelThrows() + { + var ex = Assert.ThrowsAny(() => + DocxMcp.Tools.ReadHeadingContentTool.ReadHeadingContent( + _sessions, _session.Id, heading_level: -1)); + Assert.Contains("heading_level must be between 1 and 9", ex.Message); + } + // --- Helper methods --- private static Paragraph MakeHeading(int level, string text) diff --git a/tests/DocxMcp.Tests/RevisionTests.cs b/tests/DocxMcp.Tests/RevisionTests.cs new file mode 100644 index 0000000..dcbbcd5 --- /dev/null +++ b/tests/DocxMcp.Tests/RevisionTests.cs @@ -0,0 +1,446 @@ +using System.Text.Json; +using DocumentFormat.OpenXml; +using DocumentFormat.OpenXml.Packaging; +using DocumentFormat.OpenXml.Wordprocessing; +using DocxMcp.ExternalChanges; +using DocxMcp.Helpers; +using DocxMcp.Tools; +using Xunit; + +namespace DocxMcp.Tests; + +public class RevisionTests +{ + private SessionManager CreateManager() => TestHelpers.CreateSessionManager(); + + private SyncManager CreateSyncManager() => TestHelpers.CreateSyncManager(); + + private static string AddParagraphPatch(string text) => + $"[{{\"op\":\"add\",\"path\":\"/body/children/0\",\"value\":{{\"type\":\"paragraph\",\"text\":\"{text}\"}}}}]"; + + /// + /// Create a session with a paragraph containing a tracked insertion (w:ins). + /// The tracked change is included in the baseline so it's visible via gRPC. + /// Returns (manager, sessionId, revisionId). + /// + private (SessionManager mgr, string id, int revisionId) CreateSessionWithInsertion() + { + var mgr = CreateManager(); + var session = mgr.Create(); + + // Build the paragraph with a tracked insertion directly in the session body + var body = session.GetBody(); + var para = new Paragraph(new Run(new Text("Hello") { Space = SpaceProcessingModeValues.Preserve })); + var insRun = new InsertedRun + { + Id = "100", + Author = "TestAuthor", + Date = new DateTime(2026, 1, 15, 12, 0, 0, DateTimeKind.Utc) + }; + insRun.AppendChild(new Run(new Text(" world") { Space = SpaceProcessingModeValues.Preserve })); + para.AppendChild(insRun); + body.AppendChild(para); + + // Persist baseline so gRPC storage has the tracked changes + TestHelpers.PersistBaseline(mgr, session); + + return (mgr, session.Id, 100); + } + + /// + /// Create a session with a paragraph containing a tracked deletion (w:del). + /// The tracked change is included in the baseline so it's visible via gRPC. + /// Returns (manager, sessionId, revisionId). + /// + private (SessionManager mgr, string id, int revisionId) CreateSessionWithDeletion() + { + var mgr = CreateManager(); + var session = mgr.Create(); + + var body = session.GetBody(); + var para = new Paragraph(new Run(new Text("Hello") { Space = SpaceProcessingModeValues.Preserve })); + var delRun = new DeletedRun + { + Id = "200", + Author = "TestAuthor", + Date = new DateTime(2026, 1, 15, 12, 0, 0, DateTimeKind.Utc) + }; + delRun.AppendChild(new Run(new DeletedText(" world") { Space = SpaceProcessingModeValues.Preserve })); + para.AppendChild(delRun); + body.AppendChild(para); + + TestHelpers.PersistBaseline(mgr, session); + + return (mgr, session.Id, 200); + } + + // --- track_changes_enable --- + + [Fact] + public void TrackChanges_Enable() + { + var mgr = CreateManager(); + var session = mgr.Create(); + var id = session.Id; + + var result = RevisionTools.TrackChangesEnable(mgr, CreateSyncManager(), id, enabled: true); + Assert.Contains("Track Changes enabled", result); + + var doc = mgr.Get(id).Document; + Assert.True(RevisionHelper.IsTrackChangesEnabled(doc)); + } + + [Fact] + public void TrackChanges_Disable() + { + var mgr = CreateManager(); + var session = mgr.Create(); + var id = session.Id; + + RevisionTools.TrackChangesEnable(mgr, CreateSyncManager(), id, enabled: true); + var result = RevisionTools.TrackChangesEnable(mgr, CreateSyncManager(), id, enabled: false); + Assert.Contains("disabled", result); + + var doc = mgr.Get(id).Document; + Assert.False(RevisionHelper.IsTrackChangesEnabled(doc)); + } + + [Fact] + public void TrackChanges_EnableTwice_Idempotent() + { + var mgr = CreateManager(); + var session = mgr.Create(); + var id = session.Id; + + RevisionTools.TrackChangesEnable(mgr, CreateSyncManager(), id, enabled: true); + RevisionTools.TrackChangesEnable(mgr, CreateSyncManager(), id, enabled: true); + + var doc = mgr.Get(id).Document; + Assert.True(RevisionHelper.IsTrackChangesEnabled(doc)); + } + + // --- revision_list --- + + [Fact] + public void RevisionList_Empty() + { + var mgr = CreateManager(); + var session = mgr.Create(); + var id = session.Id; + + var result = RevisionTools.RevisionList(mgr, id); + var json = JsonDocument.Parse(result).RootElement; + + Assert.Equal(0, json.GetProperty("total").GetInt32()); + Assert.Equal(0, json.GetProperty("count").GetInt32()); + } + + [Fact] + public void RevisionList_WithInsertions() + { + var (mgr, id, _) = CreateSessionWithInsertion(); + + var result = RevisionTools.RevisionList(mgr, id); + var json = JsonDocument.Parse(result).RootElement; + + Assert.True(json.GetProperty("total").GetInt32() >= 1); + var revisions = json.GetProperty("revisions"); + var firstRev = revisions[0]; + Assert.Equal("insertion", firstRev.GetProperty("type").GetString()); + Assert.Equal("TestAuthor", firstRev.GetProperty("author").GetString()); + Assert.Contains("world", firstRev.GetProperty("content").GetString()!); + } + + [Fact] + public void RevisionList_FilterByAuthor() + { + var (mgr, id, _) = CreateSessionWithInsertion(); + + // Filter by matching author + var result = RevisionTools.RevisionList(mgr, id, author: "TestAuthor"); + var json = JsonDocument.Parse(result).RootElement; + Assert.True(json.GetProperty("total").GetInt32() >= 1); + + // Filter by non-matching author + var result2 = RevisionTools.RevisionList(mgr, id, author: "Nobody"); + var json2 = JsonDocument.Parse(result2).RootElement; + Assert.Equal(0, json2.GetProperty("total").GetInt32()); + } + + [Fact] + public void RevisionList_FilterByType() + { + var (mgr, id, _) = CreateSessionWithInsertion(); + + var result = RevisionTools.RevisionList(mgr, id, type: "insertion"); + var json = JsonDocument.Parse(result).RootElement; + Assert.True(json.GetProperty("total").GetInt32() >= 1); + + var result2 = RevisionTools.RevisionList(mgr, id, type: "deletion"); + var json2 = JsonDocument.Parse(result2).RootElement; + Assert.Equal(0, json2.GetProperty("total").GetInt32()); + } + + [Fact] + public void RevisionList_Pagination() + { + var mgr = CreateManager(); + var session = mgr.Create(); + + var body = session.GetBody(); + var para = new Paragraph(new Run(new Text("Base") { Space = SpaceProcessingModeValues.Preserve })); + + for (int i = 0; i < 5; i++) + { + var insRun = new InsertedRun + { + Id = (100 + i).ToString(), + Author = "TestAuthor", + Date = DateTime.UtcNow + }; + insRun.AppendChild(new Run(new Text($" word{i}") { Space = SpaceProcessingModeValues.Preserve })); + para.AppendChild(insRun); + } + body.AppendChild(para); + + TestHelpers.PersistBaseline(mgr, session); + + var result = RevisionTools.RevisionList(mgr, session.Id, offset: 2, limit: 2); + var json = JsonDocument.Parse(result).RootElement; + + Assert.Equal(5, json.GetProperty("total").GetInt32()); + Assert.Equal(2, json.GetProperty("count").GetInt32()); + Assert.Equal(2, json.GetProperty("offset").GetInt32()); + } + + // --- revision_accept --- + + [Fact] + public void RevisionAccept_InsertedRun() + { + var (mgr, id, revId) = CreateSessionWithInsertion(); + + var result = RevisionTools.RevisionAccept(mgr, CreateSyncManager(), id, revId); + Assert.Contains("Accepted", result); + + // After accepting, InsertedRun should be gone but content remains + var doc = mgr.Get(id).Document; + var body = doc.MainDocumentPart!.Document!.Body!; + Assert.Empty(body.Descendants()); + + // Content should still be there + var paraText = body.Elements().First().InnerText; + Assert.Contains("world", paraText); + } + + [Fact] + public void RevisionAccept_DeletedRun() + { + var (mgr, id, revId) = CreateSessionWithDeletion(); + + var result = RevisionTools.RevisionAccept(mgr, CreateSyncManager(), id, revId); + Assert.Contains("Accepted", result); + + // After accepting deletion, both DeletedRun and its content should be gone + var doc = mgr.Get(id).Document; + var body = doc.MainDocumentPart!.Document!.Body!; + Assert.Empty(body.Descendants()); + } + + [Fact] + public void RevisionAccept_NotFound_ReturnsError() + { + var mgr = CreateManager(); + var session = mgr.Create(); + var id = session.Id; + + var result = RevisionTools.RevisionAccept(mgr, CreateSyncManager(), id, 999); + Assert.Contains("Error", result); + Assert.Contains("not found", result); + } + + // --- revision_reject --- + + [Fact] + public void RevisionReject_InsertedRun() + { + var (mgr, id, revId) = CreateSessionWithInsertion(); + + var result = RevisionTools.RevisionReject(mgr, CreateSyncManager(), id, revId); + Assert.Contains("Rejected", result); + + // After rejecting insertion, InsertedRun AND its content should be gone + var doc = mgr.Get(id).Document; + var body = doc.MainDocumentPart!.Document!.Body!; + Assert.Empty(body.Descendants()); + var paraText = body.Elements().First().InnerText; + Assert.DoesNotContain("world", paraText); + } + + [Fact] + public void RevisionReject_DeletedRun() + { + var (mgr, id, revId) = CreateSessionWithDeletion(); + + var result = RevisionTools.RevisionReject(mgr, CreateSyncManager(), id, revId); + Assert.Contains("Rejected", result); + + // After rejecting deletion, content should be restored as normal text + var doc = mgr.Get(id).Document; + var body = doc.MainDocumentPart!.Document!.Body!; + Assert.Empty(body.Descendants()); + + // The deleted text should be back as normal text + var paraText = body.Elements().First().InnerText; + Assert.Contains("world", paraText); + } + + [Fact] + public void RevisionReject_NotFound_ReturnsError() + { + var mgr = CreateManager(); + var session = mgr.Create(); + var id = session.Id; + + var result = RevisionTools.RevisionReject(mgr, CreateSyncManager(), id, 999); + Assert.Contains("Error", result); + Assert.Contains("not found", result); + } + + // --- WAL persistence (survives restart) --- + + [Fact] + public void Accept_SurvivesRestart() + { + var tenantId = $"test-rev-accept-{Guid.NewGuid():N}"; + var mgr = TestHelpers.CreateSessionManager(tenantId); + var session = mgr.Create(); + var id = session.Id; + + // Build baseline with tracked insertion + var body = session.GetBody(); + var para = new Paragraph(new Run(new Text("Hello") { Space = SpaceProcessingModeValues.Preserve })); + var insRun = new InsertedRun { Id = "100", Author = "Test", Date = DateTime.UtcNow }; + insRun.AppendChild(new Run(new Text(" world") { Space = SpaceProcessingModeValues.Preserve })); + para.AppendChild(insRun); + body.AppendChild(para); + TestHelpers.PersistBaseline(mgr, session); + + // Accept via tool (appends WAL) + RevisionTools.RevisionAccept(mgr, CreateSyncManager(), id, 100); + + // Simulate restart + var mgr2 = TestHelpers.CreateSessionManager(tenantId); + var doc2 = mgr2.Get(id).Document; + var body2 = doc2.MainDocumentPart!.Document!.Body!; + + // InsertedRun should be gone after WAL replay + Assert.Empty(body2.Descendants()); + } + + [Fact] + public void Reject_SurvivesRestart() + { + var tenantId = $"test-rev-reject-{Guid.NewGuid():N}"; + var mgr = TestHelpers.CreateSessionManager(tenantId); + var session = mgr.Create(); + var id = session.Id; + + // Build baseline with tracked insertion + var body = session.GetBody(); + var para = new Paragraph(new Run(new Text("Hello") { Space = SpaceProcessingModeValues.Preserve })); + var insRun = new InsertedRun { Id = "100", Author = "Test", Date = DateTime.UtcNow }; + insRun.AppendChild(new Run(new Text(" world") { Space = SpaceProcessingModeValues.Preserve })); + para.AppendChild(insRun); + body.AppendChild(para); + TestHelpers.PersistBaseline(mgr, session); + + // Reject via tool (appends WAL) + RevisionTools.RevisionReject(mgr, CreateSyncManager(), id, 100); + + // Simulate restart + var mgr2 = TestHelpers.CreateSessionManager(tenantId); + var doc2 = mgr2.Get(id).Document; + var body2 = doc2.MainDocumentPart!.Document!.Body!; + + Assert.Empty(body2.Descendants()); + // Rejected insertion means text is removed + var paraText = body2.Elements().First().InnerText; + Assert.DoesNotContain("world", paraText); + } + + [Fact] + public void TrackChanges_SurvivesRestart() + { + var tenantId = $"test-rev-track-{Guid.NewGuid():N}"; + var mgr = TestHelpers.CreateSessionManager(tenantId); + var session = mgr.Create(); + var id = session.Id; + + RevisionTools.TrackChangesEnable(mgr, CreateSyncManager(), id, enabled: true); + + // Simulate restart + var mgr2 = TestHelpers.CreateSessionManager(tenantId); + var doc2 = mgr2.Get(id).Document; + Assert.True(RevisionHelper.IsTrackChangesEnabled(doc2)); + } + + // --- Undo/Redo --- + + [Fact] + public void Accept_Undo() + { + var (mgr, id, revId) = CreateSessionWithInsertion(); + + // Accept the revision + RevisionTools.RevisionAccept(mgr, CreateSyncManager(), id, revId); + + // Verify InsertedRun is gone + var doc1 = mgr.Get(id).Document; + Assert.Empty(doc1.MainDocumentPart!.Document!.Body!.Descendants()); + + // Undo should restore the InsertedRun + mgr.Undo(id); + + var doc2 = mgr.Get(id).Document; + var insRuns = doc2.MainDocumentPart!.Document!.Body!.Descendants().ToList(); + Assert.NotEmpty(insRuns); + } + + [Fact] + public void Reject_Undo() + { + var (mgr, id, revId) = CreateSessionWithInsertion(); + + // Reject the insertion (removes the inserted content) + RevisionTools.RevisionReject(mgr, CreateSyncManager(), id, revId); + + var doc1 = mgr.Get(id).Document; + Assert.DoesNotContain("world", doc1.MainDocumentPart!.Document!.Body!.InnerText); + + // Undo should restore the InsertedRun with content + mgr.Undo(id); + + var doc2 = mgr.Get(id).Document; + Assert.Contains("world", doc2.MainDocumentPart!.Document!.Body!.InnerText); + } + + [Fact] + public void Accept_Undo_Redo() + { + var (mgr, id, revId) = CreateSessionWithInsertion(); + + // Accept + RevisionTools.RevisionAccept(mgr, CreateSyncManager(), id, revId); + Assert.Empty(mgr.Get(id).Document.MainDocumentPart!.Document!.Body!.Descendants()); + + // Undo + mgr.Undo(id); + Assert.NotEmpty(mgr.Get(id).Document.MainDocumentPart!.Document!.Body!.Descendants()); + + // Redo + mgr.Redo(id); + Assert.Empty(mgr.Get(id).Document.MainDocumentPart!.Document!.Body!.Descendants()); + Assert.Contains("world", mgr.Get(id).Document.MainDocumentPart!.Document!.Body!.InnerText); + } +} diff --git a/tests/DocxMcp.Tests/TableModificationTests.cs b/tests/DocxMcp.Tests/TableModificationTests.cs index 6922ab2..1d9cfc2 100644 --- a/tests/DocxMcp.Tests/TableModificationTests.cs +++ b/tests/DocxMcp.Tests/TableModificationTests.cs @@ -855,6 +855,115 @@ public void MultiplePatchOperationsOnTable() Assert.Equal(2, headerCells.Count); } + // =========================== + // RemoveColumn Bounds Checks + // =========================== + + [Fact] + public void RemoveColumn_NegativeIndex_ReturnsError() + { + var result = PatchTool.ApplyPatch(_sessions, _sync, _gate, _session.Id, + """[{"op": "remove_column", "path": "/body/table[0]", "column": -1}]"""); + // Validate() rejects negative column values + Assert.Contains("\"success\": false", result); + } + + [Fact] + public void RemoveColumn_IndexBeyondColumnCount_ReturnsError() + { + // Table has 3 columns (0-2), so column 10 is invalid + var result = PatchTool.ApplyPatch(_sessions, _sync, _gate, _session.Id, + """[{"op": "remove_column", "path": "/body/table[0]", "column": 10}]"""); + Assert.Contains("out of range", result); + } + + [Fact] + public void RemoveColumn_ExactBoundary_ReturnsError() + { + // Table has 3 columns (0-2), so column 3 is just past the end + var result = PatchTool.ApplyPatch(_sessions, _sync, _gate, _session.Id, + """[{"op": "remove_column", "path": "/body/table[0]", "column": 3}]"""); + Assert.Contains("out of range", result); + } + + // =========================== + // ReplaceText Empty Runs Cleanup + // =========================== + + [Fact] + public void ReplaceTextCrossRun_CleansUpEmptyRuns() + { + // Create paragraph with multiple styled runs via patch (not in-memory) + var sessions = TestHelpers.CreateSessionManager(); + var sync = TestHelpers.CreateSyncManager(); + var gate = TestHelpers.CreateExternalChangeGate(); + var session = sessions.Create(); + + // Add paragraph with 3 differently-styled runs through the patch system + PatchTool.ApplyPatch(sessions, sync, gate, session.Id, """ + [{ + "op": "add", + "path": "/body/children/0", + "value": { + "type": "paragraph", + "runs": [ + {"text": "Hello ", "style": {"bold": true}}, + {"text": "World", "style": {"italic": true}}, + {"text": " today"} + ] + } + }] + """); + + // Verify runs are separate and check their exact text content + using (var check = sessions.Get(session.Id)) + { + var para = check.GetBody().Elements().First(); + var runs = para.Elements().ToList(); + var runTexts = runs.Select(r => { + var t = r.GetFirstChild(); + return t?.Text ?? "(null)"; + }).ToList(); + Assert.True(runs.Count >= 3, + $"Expected 3+ runs but got {runs.Count}. Run texts: [{string.Join(", ", runTexts.Select(t => $"'{t}'"))}]"); + } + + // Now do cross-run replacement: "Hello World" spans run0 and run1 + var result = PatchTool.ApplyPatch(sessions, sync, gate, session.Id, """ + [{ + "op": "replace_text", + "path": "/body/paragraph[text~='Hello World']", + "find": "Hello World", + "replace": "Hi" + }] + """); + + Assert.Contains("\"success\": true", result); + + using var reloaded = sessions.Get(session.Id); + var modified = reloaded.GetBody().Elements() + .FirstOrDefault(par => par.InnerText.Contains("Hi")); + Assert.NotNull(modified); + + // Dump actual run content for debugging + var modifiedRuns = modified!.Elements().ToList(); + var modifiedRunTexts = modifiedRuns.Select(r => { + var t = r.GetFirstChild(); + return t?.Text ?? "(null)"; + }).ToList(); + + // After cross-run replacement of "Hello World" -> "Hi": + // - The replacement was applied (we found "Hi" in the paragraph) + // - Verify no empty runs remain + foreach (var (runText, i) in modifiedRunTexts.Select((t, i) => (t, i))) + { + Assert.True(runText != "" && runText != "(null)", + $"Run {i} is empty. All runs: [{string.Join(", ", modifiedRunTexts.Select(t => $"'{t}'"))}]. Full text: '{modified.InnerText}'"); + } + + sessions.Close(session.Id); + } + public void Dispose() { _sessions.Close(_session.Id); diff --git a/tests/DocxMcp.Tests/UndoRedoTests.cs b/tests/DocxMcp.Tests/UndoRedoTests.cs index b223ade..1a988a5 100644 --- a/tests/DocxMcp.Tests/UndoRedoTests.cs +++ b/tests/DocxMcp.Tests/UndoRedoTests.cs @@ -556,6 +556,37 @@ public void HistoryTools_JumpTo_Integration() Assert.Contains("Jumped to position 0", result); } + [Fact] + public void HistoryTools_History_LimitClampedToMin1() + { + var mgr = CreateManager(); + var session = mgr.Create(); + var id = session.Id; + + PatchTool.ApplyPatch(mgr, CreateSyncManager(), TestHelpers.CreateExternalChangeGate(), id, AddParagraphPatch("A")); + PatchTool.ApplyPatch(mgr, CreateSyncManager(), TestHelpers.CreateExternalChangeGate(), id, AddParagraphPatch("B")); + + // limit=0 should be clamped to 1 and not crash + var result = HistoryTools.DocumentHistory(mgr, id, limit: 0); + Assert.Contains("History for document", result); + // Should show at least the baseline entry + Assert.Contains("Baseline", result); + } + + [Fact] + public void HistoryTools_History_NegativeLimitClampedToMin1() + { + var mgr = CreateManager(); + var session = mgr.Create(); + var id = session.Id; + + PatchTool.ApplyPatch(mgr, CreateSyncManager(), TestHelpers.CreateExternalChangeGate(), id, AddParagraphPatch("A")); + + // Negative limit should be clamped to 1 + var result = HistoryTools.DocumentHistory(mgr, id, limit: -5); + Assert.Contains("History for document", result); + } + [Fact] public void DocumentSnapshot_WithDiscard_Integration() { From 1aaff546437119b6d7fca66555c4018ba101017e Mon Sep 17 00:00:00 2001 From: Laurent Valdes Date: Fri, 20 Feb 2026 00:49:36 +0100 Subject: [PATCH 4/4] chore: add unified coverage script for .NET and Rust Co-Authored-By: Claude Opus 4.6 --- scripts/coverage.sh | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100755 scripts/coverage.sh diff --git a/scripts/coverage.sh b/scripts/coverage.sh new file mode 100755 index 0000000..fb195a5 --- /dev/null +++ b/scripts/coverage.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Generate test coverage reports for .NET and Rust services +set -euo pipefail + +COVERAGE_DIR="coverage" +rm -rf "$COVERAGE_DIR" + +echo "=== .NET coverage ===" +dotnet test tests/DocxMcp.Tests/ \ + --collect:"XPlat Code Coverage" \ + --results-directory "$COVERAGE_DIR/dotnet" + +if command -v reportgenerator &> /dev/null; then + reportgenerator \ + -reports:"$COVERAGE_DIR/dotnet/**/coverage.cobertura.xml" \ + -targetdir:"$COVERAGE_DIR/dotnet/html" \ + -reporttypes:"Html;TextSummary" + cat "$COVERAGE_DIR/dotnet/html/Summary.txt" +else + echo " (install reportgenerator for HTML reports: dotnet tool install -g dotnet-reportgenerator-globaltool)" + echo " Raw coverage: $COVERAGE_DIR/dotnet/**/coverage.cobertura.xml" +fi + +echo "" +echo "=== Rust coverage ===" +if command -v cargo-tarpaulin &> /dev/null; then + cargo tarpaulin --workspace \ + --exclude-files "*/build.rs" \ + --out Html Xml \ + --output-dir "$COVERAGE_DIR/rust" +else + echo " (install tarpaulin for Rust coverage: cargo install cargo-tarpaulin)" + echo " Running tests without coverage..." + cargo test --workspace +fi + +echo "" +echo "Reports:" +echo " .NET: $COVERAGE_DIR/dotnet/html/index.html" +echo " Rust: $COVERAGE_DIR/rust/tarpaulin-report.html"