diff --git a/CHANGELOG.md b/CHANGELOG.md index 52a2365d..8f53ea8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,32 @@ All notable changes to this project will be documented in this file. ## [Unreleased] ### Added +- **Idempotent mutation transaction identities for the MCP server** (issue #449). + An applying `docxodus_mutations` batch may carry a caller-chosen root + `transactionId` (non-blank, at most 256 Unicode scalar values). The first + terminal response — success, partial, structured failure, precondition failure, + or safely-caught exception — is retained for the lifetime of that open session, + and an identical retry returns it byte-for-byte without executing anything or + re-evaluating preconditions, so generated anchors, timestamps, versions and + `packageHash` all survive a lost response. Results gain a top-level + `transaction: { schemaVersion, transactionId, requestFingerprint }`; the + fingerprint is a SHA-256 over a canonical rendering that excludes only the root + `sessionId`/`transactionId`. Reusing an id for a different request returns + `transaction_conflict`. Preview/dry-run batches, nested step args, and the other + tools reject transaction ids rather than ignoring them. Retention is bounded per + session by both a count and a byte budget (128 responses, 32 MiB) followed by + 1,024 response-less tombstones; there is no TTL and the number of open sessions + is not bounded, and once a tombstone expires a late retry applies again — both + documented as hazards in + [`docs/architecture/docx_agent_server.md`](docs/architecture/docx_agent_server.md). + Idempotency is MCP-only: `execute_batch` through WASM/npm and the stdio host has + no equivalent. Adds `EditErrorCode.InvalidTransaction`, `TransactionConflict`, + `TransactionResultEvicted` and `TransactionIncomplete`, rippled to npm + `EditErrorCode` and Python `EditErrorCode`. MCP session dispatch is now + serialized per session so a retry cannot race another action on the same + document. Coverage: `McpMutationTransactionTests` MCP449, + `python/tests/test_transaction_error_codes.py`, and + `npm/tests/transaction-error-codes.spec.ts`. - **Structural tracked revisions and one live revision registry (#455).** *(Breaking — see the Breaking changes section below.)* `DocxSession` now enumerates and resolves the structural revision families it previously ignored — diff --git a/Docxodus.Tests/McpMutationTransactionTests.cs b/Docxodus.Tests/McpMutationTransactionTests.cs new file mode 100644 index 00000000..71b1446c --- /dev/null +++ b/Docxodus.Tests/McpMutationTransactionTests.cs @@ -0,0 +1,1104 @@ +#nullable enable + +using System.Collections.Concurrent; +using System.Text.Json; +using Docxodus.McpServer; +using Xunit; + +namespace Docxodus.Tests; + +/// Issue #449: in-session idempotency and session-wide dispatch ordering. +[Collection("MCP session registry isolation")] +public sealed class McpMutationTransactionTests : IDisposable +{ + private readonly string _root; + private readonly string _path; + private readonly SessionStore _store; + + public McpMutationTransactionTests() + { + _root = Path.Combine(Path.GetTempPath(), $"mcp-transactions-{Guid.NewGuid():N}"); + Directory.CreateDirectory(_root); + _path = Path.Combine(_root, "document.docx"); + File.WriteAllBytes(_path, DocxSession.CreateBlankDocxBytes()); + _store = new SessionStore(new LocalFileDocumentStore(_root)); + } + + public void Dispose() + { + _store.CloseAll(); + if (Directory.Exists(_root)) Directory.Delete(_root, recursive: true); + } + + [Fact] + public void MCP449_ResponseLossRetryIsByteExactAndDoesNotDisturbUndoRedo() + { + var sessionId = OpenSession(_store, _path); + var anchor = FirstAnchor(_store, sessionId); + var args = MutationArgs(sessionId, "tx-response-loss", anchor, "inserted exactly once"); + + // Simulate transport loss by discarding the first returned string. + var original = Dispatcher.Call(_store, "docxodus_mutations", J(args)); + var parsed = J(original); + Assert.True(parsed.GetProperty("success").GetBoolean()); + Assert.False(parsed.GetProperty("rolledBack").GetBoolean()); + Assert.Equal("ok", parsed.GetProperty("status").GetString()); + Assert.Equal(0, parsed.GetProperty("baseVersion").GetInt64()); + Assert.Equal(1, parsed.GetProperty("resultVersion").GetInt64()); + Assert.NotEmpty(parsed.GetProperty("packageHash").GetString()!); + var step = Assert.Single(parsed.GetProperty("steps").EnumerateArray()); + Assert.Equal("docxodus_edit", step.GetProperty("tool").GetString()); + Assert.Equal("insert_paragraph", step.GetProperty("action").GetString()); + var edit = Assert.Single(step.GetProperty("results").EnumerateArray()); + Assert.NotEmpty(edit.GetProperty("created")[0].GetProperty("id").GetString()!); + var identity = parsed.GetProperty("transaction"); + Assert.Equal(1, identity.GetProperty("schemaVersion").GetInt32()); + Assert.Equal("tx-response-loss", identity.GetProperty("transactionId").GetString()); + Assert.StartsWith("sha256:", identity.GetProperty("requestFingerprint").GetString()); + var retained = Assert.IsType( + _store.Get(sessionId).MutationTransactions.GetRecord("tx-response-loss")); + Assert.Equal(original, retained.SerializedResponse); + var retainedResult = J(retained.SerializedResponse!); + Assert.Equal(0, retainedResult.GetProperty("baseVersion").GetInt64()); + Assert.Equal(1, retainedResult.GetProperty("resultVersion").GetInt64()); + Assert.True(retainedResult.GetProperty("success").GetBoolean()); + Assert.False(retainedResult.GetProperty("rolledBack").GetBoolean()); + Assert.NotEmpty(retainedResult.GetProperty("packageHash").GetString()!); + var retainedStep = Assert.Single(retainedResult.GetProperty("steps").EnumerateArray()); + Assert.Equal("docxodus_edit", retainedStep.GetProperty("tool").GetString()); + Assert.Equal("insert_paragraph", retainedStep.GetProperty("action").GetString()); + Assert.NotEmpty(Assert.Single(retainedStep.GetProperty("results").EnumerateArray()) + .GetProperty("created")[0].GetProperty("id").GetString()!); + + Assert.True(J(Dispatcher.Call(_store, "docxodus_edit", J(JsonSerializer.Serialize(new + { + sessionId, + action = "undo", + })))).GetProperty("success").GetBoolean()); + + var replay = Dispatcher.Call(_store, "docxodus_mutations", J(args)); + Assert.Equal(original, replay); + Assert.True(J(Dispatcher.Call(_store, "docxodus_edit", J(JsonSerializer.Serialize(new + { + sessionId, + action = "redo", + })))).GetProperty("success").GetBoolean()); + var markdown = GetMarkdown(_store, sessionId); + Assert.Equal(1, Occurrences(markdown, "inserted exactly once")); + } + + [Fact] + public void MCP449_OmittedAtomicModeAndExplicitAtomicModeReplayExactly() + { + var sessionId = OpenSession(_store, _path); + var anchor = FirstAnchor(_store, sessionId); + var omitted = MutationArgs(sessionId, "tx-default-mode", anchor, "default mode"); + var explicitAtomic = omitted.Replace( + "\"transactionId\":\"tx-default-mode\",", + "\"mode\":\"atomic\",\"transactionId\":\"tx-default-mode\","); + + var first = Dispatcher.Call(_store, "docxodus_mutations", J(omitted)); + var second = Dispatcher.Call(_store, "docxodus_mutations", J(explicitAtomic)); + + Assert.Equal(first, second); + Assert.Equal(1, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(sessionId).Handle)); + } + + [Fact] + public void MCP449_SameIdDifferentRequestReturnsTypedConflictWithoutMutation() + { + var sessionId = OpenSession(_store, _path); + var anchor = FirstAnchor(_store, sessionId); + var first = Dispatcher.Call(_store, "docxodus_mutations", + J(MutationArgs(sessionId, "tx-conflict", anchor, "first"))); + var conflict = J(Dispatcher.Call(_store, "docxodus_mutations", + J(MutationArgs(sessionId, "tx-conflict", anchor, "second")))); + + Assert.True(J(first).GetProperty("success").GetBoolean()); + Assert.Equal("transaction_conflict", + conflict.GetProperty("failure").GetProperty("error").GetProperty("code").GetString()); + Assert.Equal(1, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(sessionId).Handle)); + Assert.DoesNotContain("second", GetMarkdown(_store, sessionId)); + } + + [Fact] + public void MCP449_AtomicRollbackAndBestEffortPartialResultsAreCached() + { + var atomicSession = OpenSession(_store, _path); + var atomicAnchor = FirstAnchor(_store, atomicSession); + var atomicArgs = JsonSerializer.Serialize(new + { + sessionId = atomicSession, + transactionId = "tx-atomic-failure", + mode = "atomic", + steps = new object[] + { + Step("replace_text", atomicAnchor, "speculative"), + Step("replace_text", "p:body:missing", "fail"), + }, + }); + var atomic = Dispatcher.Call(_store, "docxodus_mutations", J(atomicArgs)); + Assert.Equal(atomic, Dispatcher.Call(_store, "docxodus_mutations", J(atomicArgs))); + var atomicResult = J(atomic); + Assert.Equal("failed", atomicResult.GetProperty("status").GetString()); + Assert.True(atomicResult.GetProperty("rolledBack").GetBoolean()); + Assert.Equal(0, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(atomicSession).Handle)); + + var partialSession = OpenSession(_store, _path); + var partialAnchor = FirstAnchor(_store, partialSession); + var partialArgs = JsonSerializer.Serialize(new + { + sessionId = partialSession, + transactionId = "tx-partial", + mode = "best_effort", + steps = new object[] + { + Step("replace_text", partialAnchor, "retained partial"), + Step("replace_text", "p:body:missing", "fail"), + }, + }); + var partial = Dispatcher.Call(_store, "docxodus_mutations", J(partialArgs)); + Assert.Equal(partial, Dispatcher.Call(_store, "docxodus_mutations", J(partialArgs))); + Assert.Equal("partial", J(partial).GetProperty("status").GetString()); + Assert.Equal(1, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(partialSession).Handle)); + Assert.Contains("retained partial", GetMarkdown(_store, partialSession)); + } + + [Fact] + public void MCP449_PreconditionAndValidationFailuresReplayBeforeCurrentStateChecks() + { + var sessionId = OpenSession(_store, _path); + var anchor = FirstAnchor(_store, sessionId); + var guardedArgs = JsonSerializer.Serialize(new + { + sessionId, + transactionId = "tx-precondition", + preconditions = new { expectedVersion = 99 }, + steps = new[] { Step("replace_text", anchor, "must not apply") }, + }); + var failed = Dispatcher.Call(_store, "docxodus_mutations", J(guardedArgs)); + Assert.Equal("precondition_failed", J(failed).GetProperty("failure") + .GetProperty("error").GetProperty("code").GetString()); + + ReplaceDirect(_store, sessionId, anchor, "later state"); + Assert.Equal(failed, Dispatcher.Call(_store, "docxodus_mutations", J(guardedArgs))); + Assert.Contains("later state", GetMarkdown(_store, sessionId)); + + var invalidArgs = $$""" + {"sessionId":{{JsonSerializer.Serialize(sessionId)}},"transactionId":"tx-validation","mode":"sideways","steps":[]} + """; + var invalid = Dispatcher.Call(_store, "docxodus_mutations", J(invalidArgs)); + Assert.Equal("invalid_batch_step", J(invalid).GetProperty("failure") + .GetProperty("error").GetProperty("code").GetString()); + Assert.Equal(invalid, Dispatcher.Call(_store, "docxodus_mutations", J(invalidArgs))); + } + + [Fact] + public void MCP449_PreviewDryRunDirectToolsAndStepArgsRejectTransactionIds() + { + var sessionId = OpenSession(_store, _path); + var anchor = FirstAnchor(_store, sessionId); + foreach (var previewProperties in new[] + { + "\"mode\":\"preview\"", + "\"mode\":\"atomic\",\"preview\":true", + }) + { + var args = "{\"sessionId\":" + JsonSerializer.Serialize(sessionId) + + ",\"transactionId\":\"tx-preview\"," + previewProperties + + ",\"steps\":[{\"tool\":\"docxodus_edit\",\"args\":{\"action\":\"replace_text\"" + + ",\"anchorId\":" + JsonSerializer.Serialize(anchor) + + ",\"markdown\":\"shadow\"}}]}"; + var rejected = J(Dispatcher.Call(_store, "docxodus_mutations", J(args))); + Assert.Equal("invalid_transaction", rejected.GetProperty("failure") + .GetProperty("error").GetProperty("code").GetString()); + Assert.False(rejected.TryGetProperty("transaction", out _)); + } + Assert.Equal(0, _store.Get(sessionId).MutationTransactions.FullRecordCount); + + Assert.Throws(() => Dispatcher.Call(_store, "docxodus_edit", + J(JsonSerializer.Serialize(new + { + sessionId, + transactionId = "nested-direct", + action = "replace_text", + anchorId = anchor, + markdown = "no", + })))); + + var nested = J(Dispatcher.Call(_store, "docxodus_mutations", J(JsonSerializer.Serialize(new + { + sessionId, + steps = new[] + { + new + { + tool = "docxodus_edit", + args = new + { + transactionId = "nested-step", + action = "replace_text", + anchorId = anchor, + markdown = "no", + }, + }, + }, + })))); + Assert.Equal("invalid_transaction", nested.GetProperty("failure") + .GetProperty("error").GetProperty("code").GetString()); + } + + [Fact] + public void MCP449_CanonicalFingerprintNormalizesObjectsWhitespaceAndEscapesOnly() + { + var left = J(""" + { + "sessionId": "session-a", + "transactionId": "tx-a", + "unknown": { "z": "\u0061", "a": true }, + "steps": [1, { "right": null, "left": "same" }] + } + """); + var right = J("""{"steps":[1,{"left":"same","right":null}],"unknown":{"a":true,"z":"a"},"mode":"atomic","transactionId":"tx-b","sessionId":"session-b"}"""); + Assert.Equal( + MutationTransactions.Fingerprint(left), + MutationTransactions.Fingerprint(right)); + + var baseline = MutationTransactions.Fingerprint(J("""{"steps":[],"mode":"atomic","unknown":1}""")); + Assert.NotEqual(baseline, + MutationTransactions.Fingerprint(J("""{"steps":[],"mode":"atomic","unknown":1.0}"""))); + Assert.NotEqual(baseline, + MutationTransactions.Fingerprint(J("""{"steps":[],"mode":"atomic","unknown":"1"}"""))); + Assert.NotEqual( + MutationTransactions.Fingerprint(J("""{"steps":[],"unknown":"Spelling"}""")), + MutationTransactions.Fingerprint(J("""{"steps":[],"unknown":"spelling"}"""))); + Assert.NotEqual(baseline, + MutationTransactions.Fingerprint(J("""{"steps":[],"mode":"atomic","unknown":1,"extra":null}"""))); + Assert.NotEqual( + MutationTransactions.Fingerprint(J("""{"steps":[1,2]}""")), + MutationTransactions.Fingerprint(J("""{"steps":[2,1]}"""))); + Assert.NotEqual( + MutationTransactions.Fingerprint(J("""{"steps":[],"mode":"apply"}""")), + MutationTransactions.Fingerprint(J("""{"steps":[],"mode":"best_effort"}"""))); + Assert.NotEqual( + MutationTransactions.Fingerprint(J("""{"steps":[],"preview":false}""")), + MutationTransactions.Fingerprint(J("""{"steps":[]}"""))); + } + + [Fact] + public void MCP449_DuplicateKeysAtAnyDepthAreRejectedBeforeReservation() + { + var sessionId = OpenSession(_store, _path); + var duplicate = "{\"sessionId\":" + JsonSerializer.Serialize(sessionId) + + ",\"transactionId\":\"tx-duplicate\",\"steps\":[{\"tool\":\"docxodus_edit\"" + + ",\"args\":{\"action\":\"replace_text\",\"action\":\"replace_text\"}}]}"; + + var error = Assert.Throws(() => + Dispatcher.Call(_store, "docxodus_mutations", J(duplicate))); + Assert.Contains("duplicate JSON property", error.Message, StringComparison.Ordinal); + Assert.Equal(0, _store.Get(sessionId).MutationTransactions.FullRecordCount); + Assert.Null(_store.Get(sessionId).MutationTransactions.GetRecord("tx-duplicate")); + } + + [Fact] + public void MCP449_JournalUsesGeneratedMetadataAndBoundedFullThenTombstoneFifos() + { + var now = new DateTimeOffset(2026, 8, 14, 12, 0, 0, TimeSpan.Zero); + var recordNumber = 0; + var journal = new MutationTransactions( + fullRecordCapacity: 1, + tombstoneCapacity: 1, + utcNow: () => now, + recordIdFactory: () => $"record-{++recordNumber}"); + + var a = AssertReserved(journal.Begin("a", "sha256:a")); + Assert.Equal("record-1", a.RecordId); + Assert.Equal(now, a.StartedAt); + now = now.AddSeconds(1); + var completedA = journal.Complete(a, "{\"a\":1}"); + Assert.Equal(now, completedA.CompletedAt); + + var b = AssertReserved(journal.Begin("b", "sha256:b")); + now = now.AddSeconds(1); + journal.Complete(b, "{\"b\":1}"); + Assert.Equal(1, journal.FullRecordCount); + Assert.Equal(1, journal.TombstoneCount); + Assert.Equal(MutationTransactionDecisionKind.ResultEvicted, + journal.Begin("a", "sha256:a").Kind); + Assert.Equal(MutationTransactionDecisionKind.Conflict, + journal.Begin("a", "sha256:different").Kind); + + var c = AssertReserved(journal.Begin("c", "sha256:c")); + now = now.AddSeconds(1); + journal.Complete(c, "{\"c\":1}"); + Assert.Null(journal.GetTombstone("a")); + Assert.Equal(MutationTransactionDecisionKind.Reserved, + journal.Begin("a", "sha256:fresh-after-both-fifos").Kind); + } + + [Fact] + public void MCP449_CompletionClockFailureAfterCommitStillReplaysWithoutApplyingAgain() + { + var now = new DateTimeOffset(2026, 8, 14, 13, 0, 0, TimeSpan.Zero); + var clockCalls = 0; + var journal = new MutationTransactions( + utcNow: () => ++clockCalls == 2 + ? throw new InvalidOperationException("completion clock failed") + : now, + recordIdFactory: () => "completion-record"); + using var store = new TestSessionStore( + new LocalFileDocumentStore(_root), () => journal); + var sessionId = OpenSession(store.Value, _path); + var args = MutationArgs( + sessionId, + "completion", + FirstAnchor(store.Value, sessionId), + "committed before clock failure"); + + var original = Dispatcher.Call(store.Value, "docxodus_mutations", J(args)); + var replay = Dispatcher.Call(store.Value, "docxodus_mutations", J(args)); + + Assert.True(J(original).GetProperty("success").GetBoolean()); + Assert.Equal(original, replay); + Assert.Equal(1, + Docxodus.Internal.DocxSessionOps.GetVersion(store.Value.Get(sessionId).Handle)); + Assert.Equal(1, Occurrences( + GetMarkdown(store.Value, sessionId), "committed before clock failure")); + var completed = Assert.IsType( + journal.GetRecord("completion")); + Assert.Equal(now, completed.StartedAt); + Assert.Equal(now, completed.CompletedAt); + Assert.Equal(original, completed.SerializedResponse); + Assert.Null(journal.GetTombstone("completion")); + } + + [Fact] + public void MCP449_EvictionClockFailureStillMovesTheExactIdentityToATombstone() + { + var start = new DateTimeOffset(2026, 8, 14, 14, 0, 0, TimeSpan.Zero); + var clockCalls = 0; + var recordNumber = 0; + DateTimeOffset Clock() + { + clockCalls++; + if (clockCalls == 5) throw new InvalidOperationException("eviction clock failed"); + return start.AddSeconds(clockCalls - 1); + } + var journal = new MutationTransactions( + fullRecordCapacity: 1, + tombstoneCapacity: 2, + utcNow: Clock, + recordIdFactory: () => $"eviction-record-{++recordNumber}"); + var first = AssertReserved(journal.Begin("first", "sha256:first")); + journal.Complete(first, "{\"result\":\"first exact\"}"); + var second = AssertReserved(journal.Begin("second", "sha256:second")); + + var completedSecond = journal.Complete(second, "{\"result\":\"second exact\"}"); + + Assert.Equal(1, journal.FullRecordCount); + Assert.Equal(1, journal.TombstoneCount); + Assert.Null(journal.GetRecord("first")); + var tombstone = Assert.IsType( + journal.GetTombstone("first")); + Assert.Equal(first.RecordId, tombstone.RecordId); + Assert.Equal(first.Identity, tombstone.Identity); + Assert.Equal(first.StartedAt, tombstone.StartedAt); + Assert.Equal(start.AddSeconds(1), tombstone.CompletedAt); + Assert.Equal(tombstone.CompletedAt, tombstone.EvictedAt); + Assert.Equal(MutationTransactionDecisionKind.ResultEvicted, + journal.Begin("first", "sha256:first").Kind); + Assert.Equal(MutationTransactionDecisionKind.Conflict, + journal.Begin("first", "sha256:different").Kind); + var replay = journal.Begin("second", "sha256:second"); + Assert.Equal(MutationTransactionDecisionKind.Replay, replay.Kind); + Assert.Same(completedSecond, replay.Record); + Assert.Equal("{\"result\":\"second exact\"}", replay.SerializedResponse); + } + + [Fact] + public void MCP449_AttachIdentitySerializesTheIdentitySchemaVersion() + { + var serialized = MutationTransactions.AttachIdentity( + "{\"success\":true}\n", + new MutationTransactionIdentity(7, "tx-version", "sha256:version")); + + Assert.EndsWith("\n", serialized, StringComparison.Ordinal); + var transaction = J(serialized).GetProperty("transaction"); + Assert.Equal(7, transaction.GetProperty("schemaVersion").GetInt32()); + Assert.Equal("tx-version", transaction.GetProperty("transactionId").GetString()); + Assert.Equal("sha256:version", + transaction.GetProperty("requestFingerprint").GetString()); + } + + [Fact] + public void MCP449_DispatcherSerializesEvictedResultAndConflictAndReusesOnlyAfterTombstone() + { + using var store = new TestSessionStore( + new LocalFileDocumentStore(_root), + () => new MutationTransactions(fullRecordCapacity: 1, tombstoneCapacity: 1)); + var sessionId = OpenSession(store.Value, _path); + var anchor = FirstAnchor(store.Value, sessionId); + + string Args(string id, string markdown) => JsonSerializer.Serialize(new + { + sessionId, + transactionId = id, + steps = new[] { Step("replace_text", anchor, markdown) }, + }); + + var firstA = Dispatcher.Call(store.Value, "docxodus_mutations", J(Args("a", "a1"))); + Dispatcher.Call(store.Value, "docxodus_mutations", J(Args("b", "b1"))); + + var evicted = J(Dispatcher.Call( + store.Value, "docxodus_mutations", J(Args("a", "a1")))); + Assert.Equal("transaction_result_evicted", evicted.GetProperty("failure") + .GetProperty("error").GetProperty("code").GetString()); + var conflict = J(Dispatcher.Call( + store.Value, "docxodus_mutations", J(Args("a", "different")))); + Assert.Equal("transaction_conflict", conflict.GetProperty("failure") + .GetProperty("error").GetProperty("code").GetString()); + Assert.Equal(2, Docxodus.Internal.DocxSessionOps.GetVersion(store.Value.Get(sessionId).Handle)); + + Dispatcher.Call(store.Value, "docxodus_mutations", J(Args("c", "c1"))); + var reused = Dispatcher.Call( + store.Value, "docxodus_mutations", J(Args("a", "fresh after tombstone"))); + Assert.NotEqual(firstA, reused); + Assert.True(J(reused).GetProperty("success").GetBoolean()); + Assert.Equal(4, Docxodus.Internal.DocxSessionOps.GetVersion(store.Value.Get(sessionId).Handle)); + } + + [Fact] + public void MCP449_UnexpectedPostReservationFailureIsStructuredAndCached() + { + var sessionId = OpenSession(_store, _path); + var anchor = FirstAnchor(_store, sessionId); + var args = MutationArgs(sessionId, "tx-unexpected", anchor, "cannot execute"); + + // Invalidate only the lower-level handle while leaving the MCP session registered. This + // forces an unexpected registry failure after transaction reservation. + Docxodus.Internal.DocxSessionOps.CloseSession(_store.Get(sessionId).Handle); + var first = Dispatcher.Call(_store, "docxodus_mutations", J(args)); + var failure = J(first); + Assert.Equal("internal_error", failure.GetProperty("failure") + .GetProperty("error").GetProperty("code").GetString()); + Assert.Equal(first, Dispatcher.Call(_store, "docxodus_mutations", J(args))); + Assert.Equal(1, _store.Get(sessionId).MutationTransactions.FullRecordCount); + } + + [Fact] + public void MCP449_GeneratedRevisionTimestampAndIdentityReplayExactly() + { + var sessionId = OpenSession(_store, _path); + var anchor = FirstAnchor(_store, sessionId); + ReplaceDirect(_store, sessionId, anchor, "revision target"); + Dispatcher.Call(_store, "docxodus_track_changes", J(JsonSerializer.Serialize(new + { + sessionId, + action = "set_mode", + mode = "render_inline", + revisionAuthor = "Reviewer", + }))); + var args = JsonSerializer.Serialize(new + { + sessionId, + transactionId = "tx-generated-revision", + steps = new[] + { + new + { + tool = "docxodus_edit", + args = new + { + action = "replace_text", + anchorId = anchor, + markdown = "Generated metadata", + }, + }, + }, + }); + + var original = Dispatcher.Call(_store, "docxodus_mutations", J(args)); + var revisions = J(original).GetProperty("revisionChanges") + .GetProperty("added").EnumerateArray().ToArray(); + Assert.NotEmpty(revisions); + Assert.All(revisions, revision => + { + Assert.NotEmpty(revision.GetProperty("id").GetString()!); + Assert.False(string.IsNullOrWhiteSpace(revision.GetProperty("date").GetString())); + }); + Assert.Equal(original, Dispatcher.Call(_store, "docxodus_mutations", J(args))); + } + + [Fact] + public void MCP449_TransactionIdsAreSessionScopedAndCloseClearsTheirLifecycle() + { + var firstSession = OpenSession(_store, _path); + var secondSession = OpenSession(_store, _path); + var firstResult = Dispatcher.Call(_store, "docxodus_mutations", + J(MutationArgs(firstSession, "same-id", FirstAnchor(_store, firstSession), "first session"))); + var secondResult = Dispatcher.Call(_store, "docxodus_mutations", + J(MutationArgs(secondSession, "same-id", FirstAnchor(_store, secondSession), "second session"))); + Assert.True(J(firstResult).GetProperty("success").GetBoolean()); + Assert.True(J(secondResult).GetProperty("success").GetBoolean()); + + _store.Close(firstSession); + Assert.Throws(() => Dispatcher.Call(_store, "docxodus_mutations", + J(MutationArgs(firstSession, "same-id", "p:body:any", "retry after close")))); + Assert.Contains("second session", GetMarkdown(_store, secondSession)); + } + + [Fact] + public void MCP449_ConcurrentIdenticalCallsSerializeToOneMutationAndOneExactReplay() + { + var sessionId = OpenSession(_store, _path); + var args = J(MutationArgs( + sessionId, "tx-concurrent", FirstAnchor(_store, sessionId), "concurrent once")); + var start = new ManualResetEventSlim(false); + var results = new ConcurrentBag(); + var calls = Enumerable.Range(0, 8).Select(_ => Task.Run(() => + { + start.Wait(); + results.Add(Dispatcher.Call(_store, "docxodus_mutations", args)); + })).ToArray(); + + start.Set(); + Task.WaitAll(calls); + Assert.Equal(8, results.Count); + Assert.Single(results.Distinct(StringComparer.Ordinal)); + Assert.Equal(1, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(sessionId).Handle)); + Assert.Equal(1, Occurrences(GetMarkdown(_store, sessionId), "concurrent once")); + } + + [Fact] + public void MCP449_SaveCloseAndCloseAllWaitForTheSameSessionDispatchGate() + { + var blockingStore = new BlockingDocumentStore(DocxSession.CreateBlankDocxBytes()); + var store = new SessionStore(blockingStore); + try + { + var sessionId = OpenSession(store, "document.docx"); + var save = Task.Run(() => Dispatcher.Call(store, "docxodus_save", + J(JsonSerializer.Serialize(new { sessionId })))); + Assert.True(blockingStore.WriteEntered.Wait(TimeSpan.FromSeconds(5))); + var close = Task.Run(() => Dispatcher.Call(store, "docxodus_close", + J(JsonSerializer.Serialize(new { sessionId })))); + Assert.False(close.Wait(TimeSpan.FromMilliseconds(100))); + blockingStore.ReleaseWrite.Set(); + Assert.True(save.Wait(TimeSpan.FromSeconds(5))); + Assert.True(close.Wait(TimeSpan.FromSeconds(5))); + + var closeAllSession = OpenSession(store, "document.docx"); + var entered = new ManualResetEventSlim(false); + var release = new ManualResetEventSlim(false); + var action = Task.Run(() => store.Dispatch(closeAllSession, () => + { + entered.Set(); + release.Wait(); + return "{}"; + })); + Assert.True(entered.Wait(TimeSpan.FromSeconds(5))); + var closeAll = Task.Run(store.CloseAll); + Assert.False(closeAll.Wait(TimeSpan.FromMilliseconds(100))); + release.Set(); + Assert.True(action.Wait(TimeSpan.FromSeconds(5))); + Assert.True(closeAll.Wait(TimeSpan.FromSeconds(5))); + Assert.Throws(() => store.Get(closeAllSession)); + } + finally + { + blockingStore.ReleaseWrite.Set(); + store.CloseAll(); + } + } + + [Fact] + public void MCP449_ReentrantDispatchAndLifecycleCallsFailBeforeLocksWhileExternalCloseWaits() + { + using var store = new TestSessionStore(new LocalFileDocumentStore(_root)); + var session = store.Value.Open( + DocxSession.CreateBlankDocxBytes(), null, new DocxSessionSettings()); + var otherSession = store.Value.Open( + DocxSession.CreateBlankDocxBytes(), null, new DocxSessionSettings()); + using var entered = new ManualResetEventSlim(false); + using var release = new ManualResetEventSlim(false); + var errors = new ConcurrentBag(); + var action = Task.Run(() => store.Value.Dispatch(session.Id, () => + { + errors.Add(Record.Exception(() => store.Value.Open( + DocxSession.CreateBlankDocxBytes(), null, new DocxSessionSettings()))); + errors.Add(Record.Exception(() => store.Value.Close(session.Id))); + errors.Add(Record.Exception(store.Value.CloseAll)); + errors.Add(Record.Exception(() => + store.Value.Dispatch(session.Id, () => "nested same session"))); + errors.Add(Record.Exception(() => + store.Value.Dispatch(otherSession.Id, () => "nested cross session"))); + entered.Set(); + release.Wait(); + return "{}"; + })); + Task? close = null; + try + { + Assert.True(entered.Wait(TimeSpan.FromSeconds(5))); + Assert.Equal(5, errors.Count); + Assert.All(errors, error => + { + var typed = Assert.IsType(error); + Assert.Contains("session dispatch callback", typed.Message, + StringComparison.Ordinal); + }); + close = Task.Run(() => store.Value.Close(session.Id)); + Assert.False(close.Wait(TimeSpan.FromMilliseconds(100))); + + release.Set(); + Assert.True(action.Wait(TimeSpan.FromSeconds(5))); + Assert.True(close.Wait(TimeSpan.FromSeconds(5))); + Assert.Throws(() => store.Value.Get(session.Id)); + } + finally + { + release.Set(); + action.Wait(TimeSpan.FromSeconds(5)); + close?.Wait(TimeSpan.FromSeconds(5)); + } + } + + [Fact] + public void MCP449_DifferentSessionsStillDispatchInParallel() + { + using var store = new TestSessionStore(new LocalFileDocumentStore(_root)); + var first = store.Value.Open( + DocxSession.CreateBlankDocxBytes(), null, new DocxSessionSettings()); + var second = store.Value.Open( + DocxSession.CreateBlankDocxBytes(), null, new DocxSessionSettings()); + using var firstEntered = new ManualResetEventSlim(false); + using var secondEntered = new ManualResetEventSlim(false); + using var release = new ManualResetEventSlim(false); + var firstAction = Task.Run(() => store.Value.Dispatch(first.Id, () => + { + firstEntered.Set(); + release.Wait(); + return "first"; + })); + var secondAction = Task.Run(() => store.Value.Dispatch(second.Id, () => + { + secondEntered.Set(); + release.Wait(); + return "second"; + })); + try + { + Assert.True(firstEntered.Wait(TimeSpan.FromSeconds(5))); + Assert.True(secondEntered.Wait(TimeSpan.FromSeconds(5))); + } + finally + { + release.Set(); + } + Assert.True(Task.WaitAll(new[] { firstAction, secondAction }, TimeSpan.FromSeconds(5))); + Assert.Equal("first", firstAction.Result); + Assert.Equal("second", secondAction.Result); + } + + [Fact] + public void MCP449_FlowedChildDispatchRejectsWhileActiveButRunsAfterParentReturns() + { + using var parentStore = new TestSessionStore(new LocalFileDocumentStore(_root)); + using var childStore = new TestSessionStore(new LocalFileDocumentStore(_root)); + var parentSession = parentStore.Value.Open( + DocxSession.CreateBlankDocxBytes(), null, new DocxSessionSettings()); + var childSession = childStore.Value.Open( + DocxSession.CreateBlankDocxBytes(), null, new DocxSessionSettings()); + using var releaseDeferredChild = new ManualResetEventSlim(false); + Task? concurrentChild = null; + Task? deferredChild = null; + + var parent = Task.Run(() => parentStore.Value.Dispatch(parentSession.Id, () => + { + concurrentChild = Task.Run(() => Record.Exception(() => + childStore.Value.Dispatch(childSession.Id, () => "must reject"))); + if (!concurrentChild.Wait(TimeSpan.FromSeconds(5))) + throw new TimeoutException("concurrent child did not finish"); + + deferredChild = Task.Run(() => + { + releaseDeferredChild.Wait(); + return parentStore.Value.Dispatch(parentSession.Id, () => "deferred child"); + }); + return "parent"; + })); + + try + { + Assert.True(parent.Wait(TimeSpan.FromSeconds(5))); + Assert.Equal("parent", parent.Result); + var reentrant = Assert.IsType(concurrentChild!.Result); + Assert.Contains("session dispatch callback", reentrant.Message, + StringComparison.Ordinal); + + releaseDeferredChild.Set(); + Assert.True(deferredChild!.Wait(TimeSpan.FromSeconds(5))); + Assert.Equal("deferred child", deferredChild.Result); + } + finally + { + releaseDeferredChild.Set(); + deferredChild?.Wait(TimeSpan.FromSeconds(5)); + } + } + + [Fact] + public void MCP449_JournalFactoryFailureCannotLeakACoreSession() + { + var countBefore = Docxodus.Internal.SessionRegistry.Count; + using var store = new TestSessionStore( + new LocalFileDocumentStore(_root), + () => throw new InvalidOperationException("journal factory failed")); + + var error = Assert.Throws(() => store.Value.Open( + DocxSession.CreateBlankDocxBytes(), null, new DocxSessionSettings())); + + Assert.Equal("journal factory failed", error.Message); + Assert.Equal(countBefore, Docxodus.Internal.SessionRegistry.Count); + } + + [Fact] + public void MCP449_TransactionIdRuntimeUsesBlankAndUnicodeScalarContract() + { + var sessionId = OpenSession(_store, _path); + var anchor = FirstAnchor(_store, sessionId); + foreach (var invalid in new[] { "", " \t\r\n", "\u0085" }) + { + var error = Assert.Throws(() => Dispatcher.Call( + _store, + "docxodus_mutations", + J(MutationArgs(sessionId, invalid, anchor, "must not execute")))); + Assert.Contains("empty or whitespace", error.Message, StringComparison.Ordinal); + } + + const string byteOrderMark = "\uFEFF"; + var byteOrderMarkResult = J(Dispatcher.Call(_store, "docxodus_mutations", + J(MutationArgs(sessionId, byteOrderMark, anchor, "BOM is not whitespace")))); + Assert.True(byteOrderMarkResult.GetProperty("success").GetBoolean()); + Assert.NotNull(_store.Get(sessionId).MutationTransactions.GetRecord(byteOrderMark)); + + var ascii256 = new string('a', 256); + var asciiResult = J(Dispatcher.Call(_store, "docxodus_mutations", + J(MutationArgs(sessionId, ascii256, anchor, "ascii boundary")))); + Assert.True(asciiResult.GetProperty("success").GetBoolean()); + Assert.NotNull(_store.Get(sessionId).MutationTransactions.GetRecord(ascii256)); + + var asciiError = Assert.Throws(() => Dispatcher.Call( + _store, + "docxodus_mutations", + J(MutationArgs(sessionId, new string('a', 257), anchor, "too long")))); + Assert.Contains("Unicode scalar values", asciiError.Message, StringComparison.Ordinal); + + var emoji256 = string.Concat(Enumerable.Repeat("\U0001F600", 256)); + Assert.Equal(512, emoji256.Length); + var emojiResult = J(Dispatcher.Call(_store, "docxodus_mutations", + J(MutationArgs(sessionId, emoji256, anchor, "emoji boundary")))); + Assert.True(emojiResult.GetProperty("success").GetBoolean()); + Assert.NotNull(_store.Get(sessionId).MutationTransactions.GetRecord(emoji256)); + + var emoji257 = emoji256 + "\U0001F600"; + var emojiError = Assert.Throws(() => Dispatcher.Call( + _store, + "docxodus_mutations", + J(MutationArgs(sessionId, emoji257, anchor, "too many emoji")))); + Assert.Contains("Unicode scalar values", emojiError.Message, StringComparison.Ordinal); + } + + [Fact] + public void MCP449_SaveThenRetryPreservesTheSavedMutationWithoutApplyingAgain() + { + var sessionId = OpenSession(_store, _path); + var args = MutationArgs( + sessionId, "tx-save", FirstAnchor(_store, sessionId), "saved transaction"); + var original = Dispatcher.Call(_store, "docxodus_mutations", J(args)); + Dispatcher.Call(_store, "docxodus_save", J(JsonSerializer.Serialize(new { sessionId }))); + Assert.Equal(original, Dispatcher.Call(_store, "docxodus_mutations", J(args))); + Assert.Equal(1, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(sessionId).Handle)); + + Dispatcher.Call(_store, "docxodus_close", J(JsonSerializer.Serialize(new { sessionId }))); + var reopened = OpenSession(_store, _path); + Assert.Contains("saved transaction", GetMarkdown(_store, reopened)); + var reused = Dispatcher.Call(_store, "docxodus_mutations", J(MutationArgs( + reopened, "tx-save", FirstAnchor(_store, reopened), "fresh identity after reopen"))); + Assert.True(J(reused).GetProperty("success").GetBoolean()); + Assert.Equal(1, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(reopened).Handle)); + Assert.Contains("fresh identity after reopen", GetMarkdown(_store, reopened)); + } + + [Fact] + public void MCP449_ToolSchemaDocumentsBoundedApplyingTransactionIdentity() + { + var tool = Assert.Single(ToolCatalog.Tools, + candidate => candidate.Name == "docxodus_mutations"); + var schema = J(tool.InputSchemaJson); + var transactionId = schema.GetProperty("properties").GetProperty("transactionId"); + Assert.Equal("string", transactionId.GetProperty("type").GetString()); + Assert.Equal(1, transactionId.GetProperty("minLength").GetInt32()); + Assert.Equal(MutationTransactions.MaxTransactionIdLength, + transactionId.GetProperty("maxLength").GetInt32()); + Assert.Equal( + @"[^\u0009-\u000D\u0020\u0085\u00A0\u1680\u2000-\u200A\u2028-\u2029\u202F\u205F\u3000]", + transactionId.GetProperty("pattern").GetString()); + Assert.Contains("APPLYING", transactionId.GetProperty("description").GetString(), + StringComparison.Ordinal); + Assert.Contains("non-blank", transactionId.GetProperty("description").GetString(), + StringComparison.Ordinal); + Assert.Contains("Unicode scalar values", + transactionId.GetProperty("description").GetString(), StringComparison.Ordinal); + Assert.Contains(MutationTransactions.TransactionIdWhiteSpaceDescription, + transactionId.GetProperty("description").GetString(), StringComparison.Ordinal); + Assert.Contains("U+FEFF is non-whitespace", + transactionId.GetProperty("description").GetString(), StringComparison.Ordinal); + Assert.Equal(128, MutationTransactions.DefaultFullRecordCapacity); + Assert.Equal(1024, MutationTransactions.DefaultTombstoneCapacity); + Assert.Equal(32L * 1024 * 1024, MutationTransactions.DefaultResponseByteBudget); + } + + [Fact] + public void MCP449_RetainedResponsesAreBoundedByBytesNotOnlyByCount() + { + // The count cap is deliberately generous here so only the byte budget can evict. + var journal = new MutationTransactions( + fullRecordCapacity: 64, tombstoneCapacity: 8, responseByteBudget: 200); + string Response(char fill) => "{\"r\":\"" + new string(fill, 40) + "\"}"; + Assert.Equal(96, (long)Response('a').Length * sizeof(char)); + + journal.Complete(AssertReserved(journal.Begin("a", "sha256:a")), Response('a')); + journal.Complete(AssertReserved(journal.Begin("b", "sha256:b")), Response('b')); + Assert.Equal(192, journal.RetainedResponseBytes); + Assert.Equal(2, journal.FullRecordCount); + Assert.Equal(0, journal.TombstoneCount); + + // 288 bytes would exceed the 200-byte budget, so the oldest retained response goes even + // though the count cap is nowhere near reached. + journal.Complete(AssertReserved(journal.Begin("c", "sha256:c")), Response('c')); + Assert.Equal(192, journal.RetainedResponseBytes); + Assert.Equal(2, journal.FullRecordCount); + Assert.Equal(1, journal.TombstoneCount); + Assert.Null(journal.GetRecord("a")); + Assert.Equal(MutationTransactionDecisionKind.ResultEvicted, + journal.Begin("a", "sha256:a").Kind); + Assert.Equal(MutationTransactionDecisionKind.Replay, journal.Begin("c", "sha256:c").Kind); + + // A single response larger than the whole budget evicts itself rather than raising the + // ceiling: the identity stays bound and answers "evicted", and the bound stays a bound. + journal.Complete( + AssertReserved(journal.Begin("huge", "sha256:huge")), new string('x', 200)); + Assert.Equal(0, journal.RetainedResponseBytes); + Assert.Equal(0, journal.FullRecordCount); + Assert.Equal(MutationTransactionDecisionKind.ResultEvicted, + journal.Begin("huge", "sha256:huge").Kind); + } + + [Fact] + public void MCP449_RetainedResponseCostIsMeasuredFromRealBatchResults() + { + var sessionId = OpenSession(_store, _path); + var journal = _store.Get(sessionId).MutationTransactions; + var response = Dispatcher.Call(_store, "docxodus_mutations", J(MutationArgs( + sessionId, "tx-cost", FirstAnchor(_store, sessionId), "one small step"))); + + Assert.Equal((long)response.Length * sizeof(char), journal.RetainedResponseBytes); + // Anchors the per-session memory cost documented in tools/mcp-server/README.md: even a + // one-step batch retains kilobytes, because a MutationBatchResult carries every step's + // results twice plus the markdown patch and the semantic delta sets. + Assert.InRange(journal.RetainedResponseBytes, 1_024L, 128L * 1024); + } + + [Fact] + public void MCP449_IncompleteReservationIsReportedTruthfullyAndIsEvictable() + { + var journal = new MutationTransactions(fullRecordCapacity: 4, tombstoneCapacity: 4); + var reservation = AssertReserved(journal.Begin("stranded", "sha256:original")); + + // The fingerprints match, so reporting a conflict would state the opposite of the truth. + var identical = journal.Begin("stranded", "sha256:original"); + Assert.Equal(MutationTransactionDecisionKind.Incomplete, identical.Kind); + Assert.Equal("sha256:original", identical.ExistingIdentity!.RequestFingerprint); + + // A genuinely different request still conflicts, against the ORIGINAL fingerprint. + var different = journal.Begin("stranded", "sha256:other"); + Assert.Equal(MutationTransactionDecisionKind.Conflict, different.Kind); + Assert.Equal("sha256:original", different.ExistingIdentity!.RequestFingerprint); + + journal.Abandon(reservation); + Assert.Null(journal.GetRecord("stranded")); + var tombstone = Assert.IsType( + journal.GetTombstone("stranded")); + Assert.Null(tombstone.CompletedAt); + Assert.Equal(MutationTransactionDecisionKind.Incomplete, + journal.Begin("stranded", "sha256:original").Kind); + Assert.Equal(MutationTransactionDecisionKind.Conflict, + journal.Begin("stranded", "sha256:other").Kind); + + journal.Abandon(reservation); // idempotent — no second tombstone, no resurrection + Assert.Equal(1, journal.TombstoneCount); + Assert.Throws(() => journal.Complete(reservation, "{}")); + } + + [Fact] + public void MCP449_UncompletedReservationsAreBoundedByTheirOwnFifo() + { + var journal = new MutationTransactions(fullRecordCapacity: 2, tombstoneCapacity: 8); + AssertReserved(journal.Begin("r1", "sha256:r1")); + AssertReserved(journal.Begin("r2", "sha256:r2")); + AssertReserved(journal.Begin("r3", "sha256:r3")); + + Assert.Null(journal.GetRecord("r1")); + Assert.Null(Assert.IsType( + journal.GetTombstone("r1")).CompletedAt); + Assert.Equal(MutationTransactionDecisionKind.Incomplete, + journal.Begin("r1", "sha256:r1").Kind); + Assert.NotNull(journal.GetRecord("r2")); + Assert.NotNull(journal.GetRecord("r3")); + } + + [Fact] + public void MCP449_DispatcherReportsAnIncompleteTransactionInsteadOfALyingConflict() + { + var sessionId = OpenSession(_store, _path); + var args = MutationArgs( + sessionId, "tx-stranded", FirstAnchor(_store, sessionId), "must not execute"); + + // Strand a reservation the way only a direct component caller can, then retry identically. + AssertReserved(_store.Get(sessionId).MutationTransactions.Begin( + "tx-stranded", MutationTransactions.Fingerprint(J(args)))); + + var error = J(Dispatcher.Call(_store, "docxodus_mutations", J(args))) + .GetProperty("failure").GetProperty("error"); + Assert.Equal("transaction_incomplete", error.GetProperty("code").GetString()); + Assert.DoesNotContain("different request fingerprint", + error.GetProperty("message").GetString()!, StringComparison.Ordinal); + Assert.Equal(0, Docxodus.Internal.DocxSessionOps.GetVersion(_store.Get(sessionId).Handle)); + Assert.Equal(0, Occurrences(GetMarkdown(_store, sessionId), "must not execute")); + } + + private static MutationTransactionRecord AssertReserved(MutationTransactionDecision decision) + { + Assert.Equal(MutationTransactionDecisionKind.Reserved, decision.Kind); + return Assert.IsType(decision.Record); + } + + private static object Step(string action, string anchorId, string markdown) => new + { + tool = "docxodus_edit", + args = new { action, anchorId, markdown }, + }; + + private static string MutationArgs( + string sessionId, + string transactionId, + string anchorId, + string markdown) => JsonSerializer.Serialize(new + { + sessionId, + transactionId, + steps = new[] + { + new + { + tool = "docxodus_edit", + args = new + { + action = "insert_paragraph", + anchorId, + position = "after", + markdown, + }, + }, + }, + }); + + private static string OpenSession(SessionStore store, string path) + { + var opened = J(Dispatcher.Call(store, "docxodus_open", + J(JsonSerializer.Serialize(new { path })))); + return opened.GetProperty("sessionId").GetString()!; + } + + private static string FirstAnchor(SessionStore store, string sessionId) + { + var content = J(Dispatcher.Call(store, "docxodus_get_content", + J(JsonSerializer.Serialize(new { sessionId, format = "markdown" })))); + return content.GetProperty("anchorIndex").EnumerateObject().First().Name; + } + + private static string GetMarkdown(SessionStore store, string sessionId) => + J(Dispatcher.Call(store, "docxodus_get_content", + J(JsonSerializer.Serialize(new { sessionId, format = "markdown" })))) + .GetProperty("markdown").GetString()!; + + private static void ReplaceDirect( + SessionStore store, + string sessionId, + string anchorId, + string markdown) + { + var result = J(Dispatcher.Call(store, "docxodus_edit", J(JsonSerializer.Serialize(new + { + sessionId, + action = "replace_text", + anchorId, + markdown, + })))); + Assert.True(result.GetProperty("success").GetBoolean()); + } + + private static int Occurrences(string value, string search) + { + var count = 0; + var index = 0; + while ((index = value.IndexOf(search, index, StringComparison.Ordinal)) >= 0) + { + count++; + index += search.Length; + } + return count; + } + + private static JsonElement J(string json) + { + using var document = JsonDocument.Parse(json); + return document.RootElement.Clone(); + } + + private sealed class BlockingDocumentStore : IDocumentStore + { + private readonly byte[] _bytes; + + public BlockingDocumentStore(byte[] bytes) => _bytes = bytes; + + public string Kind => "blocking-test"; + public string RootDescription => "blocking-test"; + public ManualResetEventSlim WriteEntered { get; } = new(false); + public ManualResetEventSlim ReleaseWrite { get; } = new(false); + public string Resolve(string location) => location; + public byte[] Read(string resolvedLocation) => _bytes.ToArray(); + + public void Write(string resolvedLocation, byte[] bytes) + { + WriteEntered.Set(); + ReleaseWrite.Wait(); + } + } + + private sealed class TestSessionStore : IDisposable + { + public TestSessionStore( + IDocumentStore documents, + Func? journalFactory = null) => + Value = new SessionStore(documents, journalFactory); + + public SessionStore Value { get; } + + public void Dispose() => Value.CloseAll(); + } +} + +[CollectionDefinition("MCP session registry isolation", DisableParallelization = true)] +public sealed class McpSessionRegistryIsolationCollection; diff --git a/Docxodus/DocxSession.cs b/Docxodus/DocxSession.cs index b1d26bb3..74b28eef 100644 --- a/Docxodus/DocxSession.cs +++ b/Docxodus/DocxSession.cs @@ -1702,6 +1702,18 @@ public enum EditErrorCode /// A mutation batch step names an unsupported operation or a read-only action. InvalidBatchStep, + /// A transaction identity was supplied where it cannot safely identify an applying batch. + InvalidTransaction, + + /// A transaction id was already reserved for a different canonical request. + TransactionConflict, + + /// The exact response for a known transaction was evicted from bounded retention. + TransactionResultEvicted, + + /// A known transaction never recorded a terminal response, so its outcome is unknown. + TransactionIncomplete, + /// The revision family is visible but has no safe selective resolver. RevisionUnsupported, diff --git a/docs/architecture/docx_agent_server.md b/docs/architecture/docx_agent_server.md index f834cf53..17a601d5 100644 --- a/docs/architecture/docx_agent_server.md +++ b/docs/architecture/docx_agent_server.md @@ -551,6 +551,76 @@ modulo those generated ids/timestamps. Such receipts carry warnings; clients mus id or `packageHash` equality unless the operation supplies stable ids/timestamps or is otherwise known deterministic. +Applying batches may carry a caller-chosen root `transactionId` (a non-blank string up to 256 +Unicode scalar values). Its first terminal success, partial result, structured failure, +precondition failure, or safely-caught exception is recorded for the lifetime of that open session. +An identical retry returns the original serialized `MutationBatchResult` byte-for-byte before +evaluating current preconditions or running a step; it therefore preserves generated anchors, +timestamps, versions, outcome, semantic deltas, and `packageHash` from the original call. The +result has one additional top-level identity — not a parallel receipt model: + +```json +{ + "transaction": { + "schemaVersion": 1, + "transactionId": "caller-operation-42", + "requestFingerprint": "sha256:..." + } +} +``` + +The request fingerprint excludes only root `sessionId` and `transactionId`; it sorts object keys, +normalizes JSON whitespace and equivalent string escapes, preserves array order, string spelling, +numeric token spelling, unknown properties, and every omitted/explicit distinction except the root +default `mode` (`mode` omitted is canonicalized as `"atomic"`). Deprecated `apply` remains distinct +from `best_effort`. Duplicate keys are rejected at any depth. Reusing an id for a different +fingerprint returns `transaction_conflict`. Transaction ids are for mutating batches only: direct +tools, step args, `mode: preview`, and `preview: true` reject them. +Replay after an ordinary undo or redo still returns the historical response: it never reapplies, +undoes, or redoes the mutation, changes the current document, or moves either history cursor. A +caller that wants an undone mutation present again must use ordinary `redo` while it remains +available. Saving preserves the in-session journal. Closing clears it, and reopening the document +starts a new transaction-identity namespace even when it opens the same saved file. + +**Retention is bounded by count *and* bytes.** The per-session journal retains at most 128 full +responses *and* at most 32 MiB of retained response text (`DefaultFullRecordCapacity` / +`DefaultResponseByteBudget`), evicting oldest-first until both hold, then keeps 1,024 response-less +FIFO tombstones. A count alone is not a memory bound: a retained entry is the complete serialized +`MutationBatchResult`, which emits every step's `results` twice (`steps[].results` plus the +duplicate top-level `results`, `DocxSessionJson.cs`) alongside `patch.markdown` and the +revision/comment/annotation deltas. A one-step `insert_paragraph` batch on a blank document already +measures ~3.2 KB retained; scoped batches over real documents are far larger, so the worst case is +~32 MiB per open session. A single response exceeding the whole budget evicts itself rather than +raising the ceiling — the identity stays bound and answers `transaction_result_evicted`, which is a +safe answer, where an unbounded retained response would not be a bound at all. **Not bounded:** +the number of open sessions, and elapsed time — there is no TTL and no idle-session eviction, so +sessions that are never closed accumulate. A TTL / idle-session reaper is deliberately left as a +separate design question rather than smuggled in here. + +An identical retry whose response has been evicted returns `transaction_result_evicted`, while its +tombstone still prevents conflicting reuse. An id whose reservation never recorded a terminal +response returns `transaction_incomplete`: the fingerprints match, so reporting a conflict would +state the opposite of the truth, and the caller's real situation is that the outcome is unknown. +The Dispatcher cannot strand a reservation — it completes or abandons every one — so this is +reachable through direct component use; an abandoned reservation becomes an outcome-unknown +tombstone, and uncompleted reservations are FIFO-bounded like completed ones. + +Three lifecycle facts a client must design around: + +- **A validation failure burns the id.** `mode: "sideways"` is a terminal structured response and is + cached as such, so correcting the typo and resending under the same id yields + `transaction_conflict`. Use a fresh id after any failure you intend to correct; reuse an id only + to resend a byte-identical request. +- **HAZARD — tombstone expiry lets a stale retry silently re-apply.** Once ~128 further + transactions plus 1,024 further tombstones have passed, the identity is genuinely forgotten and a + late retry executes as a *fresh mutation* (`MCP449_DispatcherSerializesEvictedResultAndConflictAndReusesOnlyAfterTombstone` + pins exactly this: version 2 → 4). Idempotency here is a bounded-window guarantee, not a + permanent one. +- **The guarantee is MCP-only.** `execute_batch` via WASM/npm and via the stdio host / + `docx-scalpel` carries no transaction identity and no replay; a retry on those transports + re-applies. This asymmetry is within issue #449's scope, but callers must not generalize the + guarantee across transports. + 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 diff --git a/npm/src/types.ts b/npm/src/types.ts index 858bbb6e..68589fff 100644 --- a/npm/src/types.ts +++ b/npm/src/types.ts @@ -1356,6 +1356,10 @@ export type EditErrorCode = | "revision_not_found" | "precondition_failed" | "invalid_batch_step" + | "invalid_transaction" + | "transaction_conflict" + | "transaction_result_evicted" + | "transaction_incomplete" | "hyperlink_not_found" | "bookmark_not_found" | "duplicate_bookmark_name" diff --git a/npm/tests/transaction-error-codes.spec.ts b/npm/tests/transaction-error-codes.spec.ts new file mode 100644 index 00000000..9cef52cb --- /dev/null +++ b/npm/tests/transaction-error-codes.spec.ts @@ -0,0 +1,27 @@ +import { test, expect } from '@playwright/test'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import type { EditErrorCode } from '../src/types.js'; + +// The MCP server produces these codes; the browser package has no transaction surface of its +// own (idempotent retries are MCP-only). It does type every EditError it decodes, so the union +// has to name them. Declaring the list as EditErrorCode[] makes `npm run typecheck` fail if a +// member is dropped or misspelled, and the source assertion below catches it at runtime too. +const TRANSACTION_ERROR_CODES: readonly EditErrorCode[] = [ + 'invalid_transaction', + 'transaction_conflict', + 'transaction_result_evicted', + 'transaction_incomplete', +]; + +const typesSource = readFileSync( + fileURLToPath(new URL('../src/types.ts', import.meta.url)), + 'utf8', +); + +test('EditErrorCode names every MCP mutation-transaction wire string', () => { + for (const code of TRANSACTION_ERROR_CODES) { + expect(typesSource).toContain(`| "${code}"`); + } + expect(new Set(TRANSACTION_ERROR_CODES).size).toBe(TRANSACTION_ERROR_CODES.length); +}); diff --git a/python/src/docx_scalpel/enums.py b/python/src/docx_scalpel/enums.py index 37a91a3a..801ec430 100644 --- a/python/src/docx_scalpel/enums.py +++ b/python/src/docx_scalpel/enums.py @@ -180,6 +180,10 @@ class EditErrorCode(str, Enum): EMPTY_COMMENT_SPAN = "empty_comment_span" REVISION_NOT_FOUND = "revision_not_found" INVALID_BATCH_STEP = "invalid_batch_step" + INVALID_TRANSACTION = "invalid_transaction" + TRANSACTION_CONFLICT = "transaction_conflict" + TRANSACTION_RESULT_EVICTED = "transaction_result_evicted" + TRANSACTION_INCOMPLETE = "transaction_incomplete" HYPERLINK_NOT_FOUND = "hyperlink_not_found" BOOKMARK_NOT_FOUND = "bookmark_not_found" DUPLICATE_BOOKMARK_NAME = "duplicate_bookmark_name" diff --git a/python/tests/test_transaction_error_codes.py b/python/tests/test_transaction_error_codes.py new file mode 100644 index 00000000..48bf8f9f --- /dev/null +++ b/python/tests/test_transaction_error_codes.py @@ -0,0 +1,40 @@ +"""Wire-string parity for the MCP mutation-transaction ``EditErrorCode`` members. + +These codes are produced by ``tools/mcp-server`` and generated from the C# enum by +``EnumToSnake``. ``docx-scalpel`` itself has no transaction surface (idempotent retries +are MCP-only), but it decodes every ``EditError`` on the wire, so a client that talks to +the MCP server through another path must still be able to name them. +""" + +from __future__ import annotations + +from pathlib import Path + +from docx_scalpel.enums import EditErrorCode + +TRANSACTION_CODES = { + "invalid_transaction": "INVALID_TRANSACTION", + "transaction_conflict": "TRANSACTION_CONFLICT", + "transaction_result_evicted": "TRANSACTION_RESULT_EVICTED", + "transaction_incomplete": "TRANSACTION_INCOMPLETE", +} + + +def test_transaction_error_codes_round_trip_from_their_wire_strings() -> None: + for wire, member_name in TRANSACTION_CODES.items(): + member = getattr(EditErrorCode, member_name) + assert member.value == wire + # ``_missing_`` degrades an unknown code to INTERNAL_ERROR, so a member that was + # never added would decode silently. Decoding the wire string and demanding the + # exact member is what actually catches an absent or drifted code. + assert EditErrorCode(wire) is member + + +def test_transaction_error_codes_are_declared_in_this_checkout() -> None: + # Guards against an unrelated installed copy of docx_scalpel satisfying the import + # above: the source of record for this repository must declare them too. + source = ( + Path(__file__).resolve().parents[1] / "src" / "docx_scalpel" / "enums.py" + ).read_text(encoding="utf-8") + for wire, member_name in TRANSACTION_CODES.items(): + assert f'{member_name} = "{wire}"' in source diff --git a/tools/mcp-server/Dispatcher.cs b/tools/mcp-server/Dispatcher.cs index 62e41204..12b6ad33 100644 --- a/tools/mcp-server/Dispatcher.cs +++ b/tools/mcp-server/Dispatcher.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Text; using System.Text.Json; using System.Text.RegularExpressions; using Docxodus; @@ -26,29 +27,48 @@ namespace Docxodus.McpServer; /// internal static class Dispatcher { - public static string Call(SessionStore store, string tool, JsonElement args) => tool switch - { - "docxodus_open" => Open(store, args), - "docxodus_save" => Save(store, args), - "docxodus_close" => Close(store, args), - "docxodus_get_content" => GetContent(store, args), - "docxodus_preview" => Preview(store, args), - "docxodus_pagination" => Pagination(store, args), - "docxodus_search" => Search(store, args), - "docxodus_edit" => Edit(store, args), - "docxodus_format" => Format(store, args), - "docxodus_create" => Create(store, args), - "docxodus_list" => ListTool(store, args), - "docxodus_comment" => Comment(store, args), - "docxodus_links" => Links(store, args), - "docxodus_images" => Images(store, args), - "docxodus_content_controls" => ContentControls(store, args), - "docxodus_annotate" => Annotate(store, args), - "docxodus_track_changes" => TrackChanges(store, args), - "docxodus_mutations" => Mutations(store, args), - "docxodus_table" => Table(store, args), - _ => throw new McpToolException($"unknown tool: {tool}"), - }; + public static string Call(SessionStore store, string tool, JsonElement args) + { + // Transaction identities belong only to an applying batch. Rejecting the property at + // this central seam also covers future direct tools instead of silently ignoring it. + if (tool != "docxodus_mutations" + && args.ValueKind == JsonValueKind.Object + && args.TryGetProperty("transactionId", out _)) + throw new McpToolException( + "transactionId is only valid on a mutating docxodus_mutations batch"); + + if (tool == "docxodus_open") return Open(store, args); + if (tool == "docxodus_close") return Close(store, args); + // Static capability discovery has no document state to serialize against. + if (tool == "docxodus_images" && OptStr(args, "action") == "capabilities") + return Images(store, args); + + // Session-bound calls are synchronous from lookup through serialized response creation. + // This includes reads and saves, because their relative order with a mutation/replay is + // observable, and makes HTTP's request-level concurrency safe without transport locks. + var sessionId = Str(args, "sessionId"); + return store.Dispatch(sessionId, () => tool switch + { + "docxodus_save" => Save(store, args), + "docxodus_get_content" => GetContent(store, args), + "docxodus_preview" => Preview(store, args), + "docxodus_pagination" => Pagination(store, args), + "docxodus_search" => Search(store, args), + "docxodus_edit" => Edit(store, args), + "docxodus_format" => Format(store, args), + "docxodus_create" => Create(store, args), + "docxodus_list" => ListTool(store, args), + "docxodus_comment" => Comment(store, args), + "docxodus_links" => Links(store, args), + "docxodus_images" => Images(store, args), + "docxodus_content_controls" => ContentControls(store, args), + "docxodus_annotate" => Annotate(store, args), + "docxodus_track_changes" => TrackChanges(store, args), + "docxodus_mutations" => Mutations(store, args), + "docxodus_table" => Table(store, args), + _ => throw new McpToolException($"unknown tool: {tool}"), + }); + } // ─── Lifecycle ────────────────────────────────────────────────────── @@ -829,6 +849,115 @@ private static string FilterRevisions(string revisionsJson, string? author, stri private static string Mutations(SessionStore store, JsonElement args) { var liveSession = Session(store, args); + var transactionId = TransactionId(args); + if (transactionId is null) + return ExecuteMutationRequest(liveSession, args, transactional: false); + + // Canonicalization also performs the duplicate-key rejection. It deliberately precedes + // preview policy validation so an ambiguous request can never acquire a transaction id. + var requestFingerprint = MutationTransactions.Fingerprint(args); + var identity = new MutationTransactionIdentity( + MutationTransactions.SchemaVersion, transactionId, requestFingerprint); + if (RequestsPreview(args)) + { + return MutationTransactions.SerializeFailure( + RequestedCoreMode(args), + preview: true, + SafeVersion(liveSession), + EditErrorCode.InvalidTransaction, + "transactionId is not valid for preview or dry-run mutation batches", + "transaction"); + } + + var decision = liveSession.MutationTransactions.Begin(transactionId, requestFingerprint); + switch (decision.Kind) + { + case MutationTransactionDecisionKind.Replay: + return decision.SerializedResponse!; + case MutationTransactionDecisionKind.Conflict: + { + var original = decision.ExistingIdentity?.RequestFingerprint ?? "unknown"; + var conflict = MutationTransactions.SerializeFailure( + RequestedCoreMode(args), + preview: false, + SafeVersion(liveSession), + EditErrorCode.TransactionConflict, + $"transactionId is already bound to a different request fingerprint ({original})", + "transaction"); + return MutationTransactions.AttachIdentity(conflict, identity); + } + case MutationTransactionDecisionKind.ResultEvicted: + { + var expired = MutationTransactions.SerializeFailure( + RequestedCoreMode(args), + preview: false, + SafeVersion(liveSession), + EditErrorCode.TransactionResultEvicted, + "the transaction is known, but its exact response has expired from bounded retention", + "transaction"); + return MutationTransactions.AttachIdentity(expired, identity); + } + case MutationTransactionDecisionKind.Incomplete: + { + var incomplete = MutationTransactions.SerializeFailure( + RequestedCoreMode(args), + preview: false, + SafeVersion(liveSession), + EditErrorCode.TransactionIncomplete, + "this transactionId is bound to this exact request but never recorded a " + + "terminal response, so whether the mutation applied is unknown; inspect the " + + "document and retry under a new transactionId", + "transaction"); + return MutationTransactions.AttachIdentity(incomplete, identity); + } + case MutationTransactionDecisionKind.Reserved: + break; + default: + throw new InvalidOperationException("unknown mutation transaction decision"); + } + + var reservation = decision.Record!; + var completed = false; + try + { + string terminalResponse; + try + { + terminalResponse = MutationTransactions.AttachIdentity( + ExecuteMutationRequest(liveSession, args, transactional: true), identity); + } + catch (Exception ex) + { + var callerError = ex is McpToolException + or FormatException or JsonException or OverflowException; + terminalResponse = MutationTransactions.AttachIdentity( + MutationTransactions.SerializeFailure( + RequestedCoreMode(args), + preview: false, + SafeVersion(liveSession), + callerError ? EditErrorCode.InvalidBatchStep : EditErrorCode.InternalError, + ex.Message, + callerError ? "validation" : "dispatch"), + identity); + } + liveSession.MutationTransactions.Complete(reservation, terminalResponse); + completed = true; + return terminalResponse; + } + finally + { + // If serializing the failure or retaining the response itself threw, the reservation + // would otherwise be stranded in the journal forever. Retire it to an outcome-unknown + // tombstone so the identity stays bound, stays truthful, and stays evictable. + if (!completed) liveSession.MutationTransactions.Abandon(reservation); + } + } + + private static string ExecuteMutationRequest( + DocSession liveSession, + JsonElement args, + bool transactional) + { var mode = args.TryGetProperty("mode", out _) ? Str(args, "mode") : "atomic"; @@ -891,11 +1020,65 @@ private static string Mutations(SessionStore store, JsonElement args) } var liveBatchCheck = Check(liveSession, ParsePreconditions(args, MutationTarget(args))); - if (liveBatchCheck is not null) return liveBatchCheck; + if (liveBatchCheck is not null) + { + if (!transactional) return liveBatchCheck; + var error = DocxSessionJson.DeserializeEditResults(liveBatchCheck) + .FirstOrDefault()?.Error + ?? new EditError(EditErrorCode.PreconditionFailed, + "batch precondition failed"); + return MutationTransactions.SerializeFailure( + coreMode, + preview: false, + SafeVersion(liveSession), + error, + "preconditions", + rolledBack: coreMode == MutationBatchMode.Atomic); + } var liveSteps = BuildMutationBatchSteps(liveSession, stepsEl, legacyApply: mode == "apply"); return DocxSessionOps.ExecuteBatch(liveSession.Handle, coreMode, liveSteps); } + private static string? TransactionId(JsonElement args) + { + if (args.ValueKind != JsonValueKind.Object + || !args.TryGetProperty("transactionId", out var value)) + return null; + if (value.ValueKind != JsonValueKind.String) + throw new McpToolException("transactionId must be a string"); + var id = value.GetString()!; + if (MutationTransactions.IsBlankTransactionId(id)) + throw new McpToolException("transactionId must not be empty or whitespace"); + var scalarLength = 0; + foreach (var _ in id.EnumerateRunes()) scalarLength++; + if (scalarLength > MutationTransactions.MaxTransactionIdLength) + throw new McpToolException( + $"transactionId must not exceed {MutationTransactions.MaxTransactionIdLength} Unicode scalar values"); + return id; + } + + private static bool RequestsPreview(JsonElement args) => + args.ValueKind == JsonValueKind.Object + && ((args.TryGetProperty("mode", out var mode) + && mode.ValueKind == JsonValueKind.String + && mode.GetString() == "preview") + || (args.TryGetProperty("preview", out var preview) + && preview.ValueKind == JsonValueKind.True)); + + private static MutationBatchMode RequestedCoreMode(JsonElement args) => + args.ValueKind == JsonValueKind.Object + && args.TryGetProperty("mode", out var mode) + && mode.ValueKind == JsonValueKind.String + && mode.GetString() is "best_effort" or "apply" + ? MutationBatchMode.BestEffort + : MutationBatchMode.Atomic; + + private static long SafeVersion(DocSession session) + { + try { return DocxSessionOps.GetVersion(session.Handle); } + catch { return 0; } + } + private static IReadOnlyList BuildMutationBatchSteps( DocSession session, JsonElement stepsEl, @@ -985,6 +1168,11 @@ private static IReadOnlyList BuildMutationBatchSteps( string action, JsonElement args) { + if (args.TryGetProperty("transactionId", out _)) + return new EditError( + EditErrorCode.InvalidTransaction, + "mutation step args cannot contain transactionId; use the batch root"); + var actionError = ValidateMutationBatchAction(tool, action); if (actionError is not null) return actionError; diff --git a/tools/mcp-server/HttpTransport.cs b/tools/mcp-server/HttpTransport.cs index 3c953924..e92d3118 100644 --- a/tools/mcp-server/HttpTransport.cs +++ b/tools/mcp-server/HttpTransport.cs @@ -13,8 +13,13 @@ namespace Docxodus.McpServer; /// streamable-HTTP binding allows a server to choose. No SSE, no session-id handshake, no TLS: /// this exists so the stdio server can be put behind a tunnel (e.g. ngrok http PORT) and /// pointed at from a ChatGPT Apps / remote-MCP developer setup, which cannot spawn a local -/// process. Requests are processed one at a time under a lock — and -/// the Docxodus session registry assume single-threaded access, exactly as stdio provides. +/// process. Requests are processed one at a time: handles each request inline +/// on the serial accept loop, and the lock below pins that even if handling ever moves off it. +/// does now serialize per session rather than requiring a +/// single-threaded caller, but nothing in the shipped server exercises that: this transport is +/// serial and stdio is single-threaded, so the per-session dispatch gate is forward-looking. +/// Making concurrent requests real means dispatching handling off the accept loop AND dropping +/// this lock — a deliberate behaviour change, not a comment fix. /// internal static class HttpTransport { diff --git a/tools/mcp-server/MutationTransactions.cs b/tools/mcp-server/MutationTransactions.cs new file mode 100644 index 00000000..b23a9481 --- /dev/null +++ b/tools/mcp-server/MutationTransactions.cs @@ -0,0 +1,551 @@ +#nullable enable + +using System.Buffers; +using System.Security.Cryptography; +using System.Text; +using System.Text.Encodings.Web; +using System.Text.Json; +using Docxodus; +using Docxodus.Internal; + +namespace Docxodus.McpServer; + +/// The stable, versioned identity attached to a transaction-aware batch result. +internal sealed record MutationTransactionIdentity( + int SchemaVersion, + string TransactionId, + string RequestFingerprint); + +/// +/// One retained transaction result. Record ids and timestamps are server bookkeeping for tests, +/// diagnostics, and a future durable implementation; only is placed on +/// the MCP response in this in-session epic. +/// +internal sealed record MutationTransactionRecord( + string RecordId, + MutationTransactionIdentity Identity, + DateTimeOffset StartedAt, + DateTimeOffset? CompletedAt, + string? SerializedResponse); + +/// +/// An identity that is still bound but whose response is gone. is the +/// discriminator: a value means a terminal response existed and was evicted; null means the +/// reservation never recorded one, so the mutation's outcome is unknown. +/// +internal sealed record MutationTransactionTombstone( + string RecordId, + MutationTransactionIdentity Identity, + DateTimeOffset StartedAt, + DateTimeOffset? CompletedAt, + DateTimeOffset EvictedAt); + +internal enum MutationTransactionDecisionKind +{ + Reserved, + Replay, + Conflict, + ResultEvicted, + + /// The id is bound to this exact request, but no terminal response was ever + /// recorded for it, so whether the mutation applied is unknown. + Incomplete, +} + +internal sealed record MutationTransactionDecision( + MutationTransactionDecisionKind Kind, + MutationTransactionRecord? Record = null, + MutationTransactionIdentity? ExistingIdentity = null, + string? SerializedResponse = null); + +/// +/// Bounded, per-session transaction-id registry. Full responses and response-less tombstones use +/// independent FIFO limits. A tombstone keeps an evicted id bound to its original fingerprint for +/// a further window, preventing a recently forgotten retry from becoming a fresh mutation. +/// Retained responses are additionally bounded by a byte budget, because a batch result is +/// unbounded in size while a count is not a memory bound. +/// +internal sealed class MutationTransactions +{ + public const int SchemaVersion = 1; + public const int DefaultFullRecordCapacity = 128; + public const int DefaultTombstoneCapacity = 1024; + public const int MaxTransactionIdLength = 256; + + /// + /// Ceiling on the UTF-16 payload of all retained responses for one session. Chosen so the + /// 128-response count cap stays the binding constraint for ordinary batches and this budget + /// only bites on unusually large ones; see tools/mcp-server/README.md for the measured + /// per-response cost this is sized against. + /// + public const long DefaultResponseByteBudget = 32L * 1024 * 1024; + + // Stable Unicode White_Space definition for transaction ids. Runtime validation, the schema + // regex, and its prose are all derived from this one table so their blank-string semantics + // cannot drift with .NET or ECMAScript whitespace classifications. + private static readonly (int Start, int End)[] TransactionIdWhiteSpaceRanges = + { + (0x0009, 0x000D), + (0x0020, 0x0020), + (0x0085, 0x0085), + (0x00A0, 0x00A0), + (0x1680, 0x1680), + (0x2000, 0x200A), + (0x2028, 0x2029), + (0x202F, 0x202F), + (0x205F, 0x205F), + (0x3000, 0x3000), + }; + + internal static string TransactionIdNonBlankPattern { get; } = + BuildTransactionIdNonBlankPattern(); + + internal static string TransactionIdWhiteSpaceDescription { get; } = + string.Join(", ", TransactionIdWhiteSpaceRanges.Select(static range => + range.Start == range.End + ? "U+" + ScalarHex(range.Start) + : "U+" + ScalarHex(range.Start) + "-U+" + ScalarHex(range.End))); + + internal static string TransactionIdSchemaDescription { get; } = + "Optional non-blank caller identity for an APPLYING batch only, limited to " + + MaxTransactionIdLength.ToString(System.Globalization.CultureInfo.InvariantCulture) + + " Unicode " + + "scalar values. Blank means composed only of exactly these Unicode White_Space code " + + "points: " + + TransactionIdWhiteSpaceDescription + + "; U+FEFF is non-whitespace. The first terminal response is retained in this open " + + "session; an identical retry returns that exact serialized response without executing " + + "or rechecking preconditions. Reusing the id for a different canonical request returns " + + "transaction_conflict. Preview/dry-run rejects this field."; + + private readonly int _fullRecordCapacity; + private readonly int _tombstoneCapacity; + private readonly long _responseByteBudget; + private readonly Func _utcNow; + private readonly Func _recordIdFactory; + private readonly Dictionary _records = + new(StringComparer.Ordinal); + private readonly Queue _completedFifo = new(); + private readonly Dictionary _tombstones = + new(StringComparer.Ordinal); + private readonly Queue _tombstoneFifo = new(); + + // Reservations are tracked by reference, not id: an id can be reserved again after its + // tombstone expires, and a stale queue entry must never prune that later live reservation. + private readonly Queue _reservationFifo = new(); + private long _retainedResponseBytes; + + public MutationTransactions( + int fullRecordCapacity = DefaultFullRecordCapacity, + int tombstoneCapacity = DefaultTombstoneCapacity, + Func? utcNow = null, + Func? recordIdFactory = null, + long responseByteBudget = DefaultResponseByteBudget) + { + if (fullRecordCapacity < 1) + throw new ArgumentOutOfRangeException(nameof(fullRecordCapacity)); + if (tombstoneCapacity < 0) + throw new ArgumentOutOfRangeException(nameof(tombstoneCapacity)); + if (responseByteBudget < 1) + throw new ArgumentOutOfRangeException(nameof(responseByteBudget)); + _fullRecordCapacity = fullRecordCapacity; + _tombstoneCapacity = tombstoneCapacity; + _responseByteBudget = responseByteBudget; + _utcNow = utcNow ?? (() => DateTimeOffset.UtcNow); + _recordIdFactory = recordIdFactory ?? NewRecordId; + } + + internal int FullRecordCount + { + get { lock (_records) return _completedFifo.Count; } + } + + internal int TombstoneCount + { + get { lock (_records) return _tombstones.Count; } + } + + /// UTF-16 payload of every retained response — the quantity the byte budget caps. + internal long RetainedResponseBytes + { + get { lock (_records) return _retainedResponseBytes; } + } + + private static long ResponseByteCost(string? serializedResponse) => + serializedResponse is null ? 0L : (long)serializedResponse.Length * sizeof(char); + + internal MutationTransactionRecord? GetRecord(string transactionId) + { + lock (_records) + return _records.TryGetValue(transactionId, out var record) ? record : null; + } + + internal MutationTransactionTombstone? GetTombstone(string transactionId) + { + lock (_records) + return _tombstones.TryGetValue(transactionId, out var tombstone) ? tombstone : null; + } + + internal static bool IsBlankTransactionId(string transactionId) + { + foreach (var rune in transactionId.EnumerateRunes()) + { + if (!TransactionIdWhiteSpaceRanges.Any(range => + rune.Value >= range.Start && rune.Value <= range.End)) + return false; + } + return true; + } + + private static string BuildTransactionIdNonBlankPattern() + { + var pattern = new StringBuilder("[^"); + foreach (var range in TransactionIdWhiteSpaceRanges) + { + pattern.Append("\\u").Append(ScalarHex(range.Start)); + if (range.Start != range.End) + pattern.Append("-\\u").Append(ScalarHex(range.End)); + } + return pattern.Append(']').ToString(); + } + + private static string ScalarHex(int value) => + value.ToString("X4", System.Globalization.CultureInfo.InvariantCulture); + + /// Reserve a new identity, or resolve it to replay/conflict/expired/incomplete + /// deterministically. + public MutationTransactionDecision Begin(string transactionId, string requestFingerprint) + { + var requested = new MutationTransactionIdentity( + SchemaVersion, transactionId, requestFingerprint); + lock (_records) + { + if (_records.TryGetValue(transactionId, out var record)) + { + if (!string.Equals(record.Identity.RequestFingerprint, requestFingerprint, + StringComparison.Ordinal)) + return new MutationTransactionDecision( + MutationTransactionDecisionKind.Conflict, + ExistingIdentity: record.Identity); + if (record.SerializedResponse is not null) + return new MutationTransactionDecision( + MutationTransactionDecisionKind.Replay, + record, + record.Identity, + record.SerializedResponse); + + // The id is bound to this exact request but never recorded a terminal response. + // Per-session dispatch serialization plus Abandon mean this cannot occur through + // Dispatcher; it is reachable when the component is driven directly. Reporting a + // conflict here would be false — the fingerprints match — so it gets its own kind. + return new MutationTransactionDecision( + MutationTransactionDecisionKind.Incomplete, + ExistingIdentity: record.Identity); + } + + if (_tombstones.TryGetValue(transactionId, out var tombstone)) + { + if (!string.Equals(tombstone.Identity.RequestFingerprint, requestFingerprint, + StringComparison.Ordinal)) + return new MutationTransactionDecision( + MutationTransactionDecisionKind.Conflict, + ExistingIdentity: tombstone.Identity); + return new MutationTransactionDecision( + tombstone.CompletedAt is null + ? MutationTransactionDecisionKind.Incomplete + : MutationTransactionDecisionKind.ResultEvicted, + ExistingIdentity: tombstone.Identity); + } + + var reserved = new MutationTransactionRecord( + _recordIdFactory(), requested, _utcNow(), null, null); + _records.Add(transactionId, reserved); + _reservationFifo.Enqueue(reserved); + EvictStaleReservations(); + return new MutationTransactionDecision( + MutationTransactionDecisionKind.Reserved, reserved, requested); + } + } + + /// + /// Release a reservation that will never record a terminal response, keeping the identity + /// bound as an outcome-unknown tombstone rather than stranding it in the live record map. + /// Idempotent, and a no-op once the reservation has completed. + /// + public void Abandon(MutationTransactionRecord reservation) + { + ArgumentNullException.ThrowIfNull(reservation); + lock (_records) + RetireReservation(reservation); + } + + private bool RetireReservation(MutationTransactionRecord reservation) + { + var id = reservation.Identity.TransactionId; + if (!_records.TryGetValue(id, out var current) + || !ReferenceEquals(current, reservation) + || current.SerializedResponse is not null) + return false; + _records.Remove(id); + Entomb(current); + return true; + } + + /// Atomically retain the exact response and apply FIFO eviction. + public MutationTransactionRecord Complete( + MutationTransactionRecord reservation, + string serializedResponse) + { + ArgumentNullException.ThrowIfNull(reservation); + ArgumentNullException.ThrowIfNull(serializedResponse); + lock (_records) + { + if (!_records.TryGetValue(reservation.Identity.TransactionId, out var current) + || !ReferenceEquals(current, reservation) + || current.SerializedResponse is not null) + throw new InvalidOperationException("mutation transaction reservation is no longer active"); + + var completed = current with + { + // The document mutation has already committed when Complete is called. Clock + // injection is diagnostic metadata and must never strand that committed result + // behind an active reservation that cannot replay. + CompletedAt = UtcNowOr(current.StartedAt), + SerializedResponse = serializedResponse, + }; + _records[completed.Identity.TransactionId] = completed; + _completedFifo.Enqueue(completed.Identity.TransactionId); + _retainedResponseBytes += ResponseByteCost(serializedResponse); + EvictCompletedRecords(); + return completed; + } + } + + /// + /// Bound retained responses by BOTH the count cap and the byte budget, oldest first. A single + /// response larger than the whole budget evicts itself rather than raising the ceiling: an + /// identical retry then answers transaction_result_evicted, which is a safe answer, + /// whereas an unbounded retained response is not a bound at all. + /// + private void EvictCompletedRecords() + { + while (_completedFifo.Count > 0 + && (_completedFifo.Count > _fullRecordCapacity + || _retainedResponseBytes > _responseByteBudget)) + { + var id = _completedFifo.Dequeue(); + if (!_records.Remove(id, out var evicted)) continue; + // Decrement on every path a retained response leaves _records, or the running total + // drifts upward and the budget silently becomes permanent. + _retainedResponseBytes -= ResponseByteCost(evicted.SerializedResponse); + Entomb(evicted); + } + + TrimTombstones(); + } + + /// + /// Bound reservations that were never completed or abandoned. Only reachable through direct + /// component use; the Dispatcher always completes or abandons. Identity stays bound as an + /// outcome-unknown tombstone so a stale retry cannot silently become a fresh mutation. + /// + private void EvictStaleReservations() + { + while (_reservationFifo.Count > _fullRecordCapacity) + RetireReservation(_reservationFifo.Dequeue()); + TrimTombstones(); + } + + private void Entomb(MutationTransactionRecord evicted) + { + if (_tombstoneCapacity == 0) return; + var id = evicted.Identity.TransactionId; + _tombstones[id] = new MutationTransactionTombstone( + evicted.RecordId, + evicted.Identity, + evicted.StartedAt, + evicted.CompletedAt, + // Eviction must move the identity from a full record to a tombstone as one + // logical operation even when an injected diagnostics clock fails. + UtcNowOr(evicted.CompletedAt ?? evicted.StartedAt)); + _tombstoneFifo.Enqueue(id); + } + + private void TrimTombstones() + { + while (_tombstoneFifo.Count > _tombstoneCapacity) + { + var id = _tombstoneFifo.Dequeue(); + _tombstones.Remove(id); + } + } + + private DateTimeOffset UtcNowOr(DateTimeOffset fallback) + { + try + { + return _utcNow(); + } + catch (Exception) + { + return fallback; + } + } + + /// + /// SHA-256 over a deterministic JSON rendering. Root session/transaction identity is excluded; + /// objects are sorted, arrays and scalar spelling are retained, and numeric tokens are copied + /// verbatim. Parsing already normalizes insignificant whitespace and equivalent string escapes. + /// + public static string Fingerprint(JsonElement request) + { + if (request.ValueKind != JsonValueKind.Object) + throw new McpToolException("docxodus_mutations arguments must be an object"); + + var buffer = new ArrayBufferWriter(); + using (var writer = new Utf8JsonWriter(buffer, new JsonWriterOptions + { + Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, + })) + { + WriteCanonical(writer, request, isRoot: true, "$arguments"); + } + var hash = SHA256.HashData(buffer.WrittenSpan); + return "sha256:" + Convert.ToHexString(hash).ToLowerInvariant(); + } + + private static void WriteCanonical( + Utf8JsonWriter writer, + JsonElement value, + bool isRoot, + string path) + { + switch (value.ValueKind) + { + case JsonValueKind.Object: + { + var properties = value.EnumerateObject().ToArray(); + var names = new HashSet(StringComparer.Ordinal); + foreach (var property in properties) + { + if (!names.Add(property.Name)) + throw new McpToolException( + $"duplicate JSON property {JsonSerializer.Serialize(property.Name)} at {path}"); + } + Array.Sort(properties, static (left, right) => + StringComparer.Ordinal.Compare(left.Name, right.Name)); + + writer.WriteStartObject(); + var synthesizeAtomicMode = isRoot && !names.Contains("mode"); + foreach (var property in properties) + { + if (isRoot && property.Name is "sessionId" or "transactionId") continue; + if (synthesizeAtomicMode + && StringComparer.Ordinal.Compare("mode", property.Name) < 0) + { + writer.WriteString("mode", "atomic"); + synthesizeAtomicMode = false; + } + writer.WritePropertyName(property.Name); + WriteCanonical(writer, property.Value, isRoot: false, + path + "." + property.Name); + } + if (synthesizeAtomicMode) + writer.WriteString("mode", "atomic"); + writer.WriteEndObject(); + break; + } + case JsonValueKind.Array: + { + writer.WriteStartArray(); + var index = 0; + foreach (var item in value.EnumerateArray()) + { + WriteCanonical(writer, item, isRoot: false, $"{path}[{index}]"); + index++; + } + writer.WriteEndArray(); + break; + } + case JsonValueKind.String: + writer.WriteStringValue(value.GetString()); + break; + case JsonValueKind.Number: + writer.WriteRawValue(value.GetRawText(), skipInputValidation: true); + break; + case JsonValueKind.True: + writer.WriteBooleanValue(true); + break; + case JsonValueKind.False: + writer.WriteBooleanValue(false); + break; + case JsonValueKind.Null: + writer.WriteNullValue(); + break; + default: + throw new McpToolException($"unsupported JSON value at {path}"); + } + } + + /// Add the versioned identity to the already-serialized core batch result. + public static string AttachIdentity( + string serializedBatchResult, + MutationTransactionIdentity identity) + { + var end = serializedBatchResult.Length - 1; + while (end >= 0 && char.IsWhiteSpace(serializedBatchResult[end])) end--; + if (end < 1 || serializedBatchResult[0] != '{' || serializedBatchResult[end] != '}') + throw new InvalidOperationException("mutation batch result was not a JSON object"); + + var suffix = serializedBatchResult[(end + 1)..]; + return serializedBatchResult[..end] + + ",\"transaction\":{\"schemaVersion\":" + + identity.SchemaVersion.ToString(System.Globalization.CultureInfo.InvariantCulture) + + ",\"transactionId\":" + + JsonRpcIo.JsonString(identity.TransactionId) + + ",\"requestFingerprint\":" + + JsonRpcIo.JsonString(identity.RequestFingerprint) + + "}}" + + suffix; + } + + /// Serialize a transport-level terminal outcome through MutationBatchResult. + public static string SerializeFailure( + MutationBatchMode mode, + bool preview, + long version, + EditErrorCode code, + string message, + string action, + bool rolledBack = false) + => SerializeFailure( + mode, preview, version, new EditError(code, message), action, rolledBack); + + public static string SerializeFailure( + MutationBatchMode mode, + bool preview, + long version, + EditError error, + string action, + bool rolledBack = false) + { + var edit = new EditResult { Success = false, Error = error }; + var step = new MutationBatchStepResult( + 0, "docxodus_mutations", action, new[] { edit }, rolledBack); + return DocxSessionJson.SerializeMutationBatchResult(new MutationBatchResult + { + Mode = mode, + Preview = preview, + Success = false, + RolledBack = rolledBack, + BaseVersion = version, + ResultVersion = version, + Steps = new[] { step }, + Failure = new MutationBatchFailure( + step.Index, step.Tool, step.Action, error, rolledBack), + }); + } + + private static string NewRecordId() => + "mtx_" + Convert.ToHexString(RandomNumberGenerator.GetBytes(16)).ToLowerInvariant(); +} diff --git a/tools/mcp-server/README.md b/tools/mcp-server/README.md index 9c9e1852..ac161d40 100644 --- a/tools/mcp-server/README.md +++ b/tools/mcp-server/README.md @@ -97,6 +97,52 @@ markdown projection and search tools return: | `docxodus_mutations` | Apply or safely preview a batch atomically by default; opt explicitly into best-effort | | `docxodus_table` | Create/read tables; resolve canonical cell anchors ↔ grid coordinates; edit rows/columns/cell content/style | +Applying `docxodus_mutations` batches can include a caller-chosen, non-blank root `transactionId` of +at most 256 Unicode scalar values. During the open session, retrying the same canonical request +returns the exact original serialized batch result without applying again or rechecking guards; a +different request with that id fails with +`transaction_conflict`. Results expose +`transaction: { schemaVersion: 1, transactionId, requestFingerprint }`. Preview/dry-run batches and +direct or nested step calls reject transaction ids — a client that blanket-attaches an idempotency +key to *every* tool call gets a hard error on the other tools, so attach it only to applying +`docxodus_mutations` batches. Save preserves this journal; close clears it, and reopen starts a new +identity namespace. Replay after undo/redo returns the historical response without changing the +document or either history cursor; use ordinary redo to restore an undone mutation. + +### Retention bound and its memory cost + +Retention is bounded per session by **both** a count and a byte budget: at most 128 complete +responses, at most 32 MiB of retained response text, whichever binds first, followed by 1,024 +response-less FIFO tombstones that keep an id bound without holding its payload. A single response +larger than the whole budget evicts itself rather than raising the ceiling. + +The byte budget exists because a count is not a memory bound. A retained entry is the complete +serialized `MutationBatchResult`, which carries every step's `results` **twice** (once under +`steps[].results` and once in the duplicate top-level `results`) plus `patch.markdown` and the +revision/comment/annotation delta sets. Measured: a single-step `insert_paragraph` batch against a +blank document already retains **~3.2 KB**, so 128 of those is ~400 KB — and a large scoped batch +over a real document is orders of magnitude bigger, which is what the 32 MiB cap is there for. The +worst case is therefore **~32 MiB per open session**; the number of open sessions is *not* bounded, +and there is no TTL or idle-session eviction, so a long-lived server that is never sent +`docxodus_close` still grows with the number of sessions. + +### Lifecycle hazards + +- **A validation failure burns the id.** A structured rejection (say `mode: "sideways"`) is itself + a terminal response and is cached. Fixing the typo and resending under the same `transactionId` + gets `transaction_conflict`, not a retry. Always use a **fresh** id after any failure you intend + to correct; reuse an id only for a byte-identical resend of the same request. +- **Tombstone expiry lets a stale retry re-apply.** After roughly 128 further transactions plus + 1,024 more tombstones, an id is genuinely forgotten and a retry that arrives after that becomes a + **fresh mutation that applies again**. Idempotency is a bounded-window guarantee, not a permanent + one; a client holding a request for a long time must not assume the window is still open. +- **`transaction_incomplete`** means the id is bound to this exact request but no terminal response + was ever recorded, so whether the mutation applied is *unknown*. Inspect the document and retry + under a new id. +- **Idempotency is MCP-only.** `execute_batch` through WASM/npm and through the stdio host / + `docx-scalpel` has no transaction identity and no replay: a retry there re-applies. Do not + assume the MCP guarantee from another transport. + ## Known gaps A few capabilities a full-featured document-editing agent surface might want are not yet diff --git a/tools/mcp-server/SessionStore.cs b/tools/mcp-server/SessionStore.cs index a0405c59..1b566c59 100644 --- a/tools/mcp-server/SessionStore.cs +++ b/tools/mcp-server/SessionStore.cs @@ -16,6 +16,19 @@ internal sealed class DocSession required public string Id { get; init; } public int Handle { get; init; } + /// + /// Serializes every action addressed to this session. The core session has its own mutation + /// lock, but MCP dispatch also has to order reads, saves, closes, and transaction-id replay + /// decisions with mutations so a retry cannot race a different session action. + /// + internal object DispatchGate { get; } = new(); + + /// In-session mutation transaction identities and their exact serialized results. + internal MutationTransactions MutationTransactions { get; init; } = new(); + + /// False after close has won the dispatch race for this session. + internal volatile bool Active = true; + /// Store-resolved location this session was opened from — already checked to be in /// scope, so a save back to it needs no re-validation. Null only if a session was opened from /// bytes with no origin. @@ -30,13 +43,31 @@ internal sealed class DocSession /// internal sealed class SessionStore { + private sealed class DispatchFrame + { + public volatile bool Active = true; + } + + // Static scope rejects cross-store reentry too. ExecutionContext copies the frame reference, + // while Active is shared so deferred children cease being reentrant when the parent returns. + private static readonly AsyncLocal CurrentDispatch = new(); + private readonly ConcurrentDictionary _sessions = new(); + private readonly object _lifecycleGate = new(); + private readonly System.Func _mutationTransactionsFactory; /// Backing document store. Defaults to a local store rooted at the /// process's current directory, which is only appropriate for tests — the server proper /// passes the environment-configured store from . - public SessionStore(IDocumentStore? documents = null) => - Documents = documents ?? new LocalFileDocumentStore(System.IO.Directory.GetCurrentDirectory()); + public SessionStore( + IDocumentStore? documents = null, + System.Func? mutationTransactionsFactory = null) + { + Documents = documents + ?? new LocalFileDocumentStore(System.IO.Directory.GetCurrentDirectory()); + _mutationTransactionsFactory = mutationTransactionsFactory + ?? (() => new MutationTransactions()); + } /// Where this server's documents are read from and written to. Every session in the /// process shares it, and it is already rooted at the configured scope. @@ -44,15 +75,35 @@ public SessionStore(IDocumentStore? documents = null) => public DocSession Open(byte[] bytes, string? location, DocxSessionSettings settings) { - var handle = DocxSessionOps.OpenSession(bytes, settings); - var session = new DocSession + RejectReentrantLifecycle("open"); + // Construct fallible per-session collaborators before allocating a core handle. If the + // factory fails, there is nothing in either registry to clean up. + var mutationTransactions = _mutationTransactionsFactory(); + // One lifecycle-to-session lock order means the document parse inside OpenSession runs + // under the process-wide lifecycle gate, so opens do not overlap each other. Acceptable + // while both shipped transports are serial anyway; revisit if request handling ever moves + // off the accept loop, since open is the one genuinely slow operation here. + lock (_lifecycleGate) { - Id = NewSessionId(), - Handle = handle, - Location = location, - }; - _sessions[session.Id] = session; - return session; + var handle = DocxSessionOps.OpenSession(bytes, settings); + try + { + var session = new DocSession + { + Id = NewSessionId(), + Handle = handle, + Location = location, + MutationTransactions = mutationTransactions, + }; + _sessions[session.Id] = session; + return session; + } + catch + { + DocxSessionOps.CloseSession(handle); + throw; + } + } } /// @@ -66,22 +117,86 @@ private static string NewSessionId() => public DocSession Get(string sessionId) { - if (!_sessions.TryGetValue(sessionId, out var session)) + if (!_sessions.TryGetValue(sessionId, out var session) || !session.Active) throw new McpToolException($"unknown session_id: {sessionId}"); return session; } + /// + /// Run one complete session-bound dispatch while holding the session's synchronous gate. + /// The active check after taking the gate closes the lookup/close race: a caller that found + /// the session before close removed it still cannot enter the disposed core handle. + /// Callbacks are non-reentrant so lifecycle operations retain one lifecycle-to-session lock order. + /// Callback-spawned work must preserve flow; + /// deliberately suppressed or unsafe flow is unsupported by this synchronous dispatch contract. + /// + public string Dispatch(string sessionId, System.Func action) + { + if (CurrentDispatch.Value?.Active == true) + throw new McpToolException( + "cannot nest a session dispatch within a session dispatch callback"); + if (!_sessions.TryGetValue(sessionId, out var session)) + throw new McpToolException($"unknown session_id: {sessionId}"); + lock (session.DispatchGate) + { + if (!session.Active + || !_sessions.TryGetValue(sessionId, out var current) + || !ReferenceEquals(session, current)) + throw new McpToolException($"unknown session_id: {sessionId}"); + var priorFrame = CurrentDispatch.Value; + var frame = new DispatchFrame(); + CurrentDispatch.Value = frame; + try + { + return action(); + } + finally + { + frame.Active = false; + CurrentDispatch.Value = priorFrame; + } + } + } + public void Close(string sessionId) { - if (_sessions.TryRemove(sessionId, out var session)) - DocxSessionOps.CloseSession(session.Handle); + RejectReentrantLifecycle("close"); + lock (_lifecycleGate) + { + if (!_sessions.TryGetValue(sessionId, out var session)) return; + lock (session.DispatchGate) + { + if (!session.Active) return; + session.Active = false; + _sessions.TryRemove(sessionId, out _); + DocxSessionOps.CloseSession(session.Handle); + } + } } public void CloseAll() { - foreach (var kv in _sessions) - DocxSessionOps.CloseSession(kv.Value.Handle); - _sessions.Clear(); + RejectReentrantLifecycle("close all sessions"); + lock (_lifecycleGate) + { + foreach (var kv in _sessions) + { + lock (kv.Value.DispatchGate) + { + if (!kv.Value.Active) continue; + kv.Value.Active = false; + _sessions.TryRemove(kv.Key, out _); + DocxSessionOps.CloseSession(kv.Value.Handle); + } + } + } + } + + private void RejectReentrantLifecycle(string operation) + { + if (CurrentDispatch.Value?.Active == true) + throw new McpToolException( + $"cannot {operation} from within a session dispatch callback"); } } diff --git a/tools/mcp-server/ToolCatalog.cs b/tools/mcp-server/ToolCatalog.cs index 2bed996d..66f8eb20 100644 --- a/tools/mcp-server/ToolCatalog.cs +++ b/tools/mcp-server/ToolCatalog.cs @@ -1,6 +1,7 @@ #nullable enable using System.Collections.Generic; +using System.Text.Json; namespace Docxodus.McpServer; @@ -502,12 +503,13 @@ internal static class ToolCatalog """), new ToolDefinition( "docxodus_mutations", - "Apply or safely preview a batch of mutating edit/format/create/table/list/comment/link/image/content-control/track-changes actions. Atomic mode commits as one unit; preview executes the identical batch path against an isolated complete package clone and never mutates the live session or its undo/redo history.", - """ + "Apply or safely preview a batch of mutating edit/format/create/table/list/comment/link/image/content-control/track-changes actions. Atomic mode commits as one unit. An optional transactionId makes applying retries idempotent within this open session; preview is isolated and cannot carry a transactionId.", + $$""" { "type": "object", "properties": { "sessionId": { "type": "string" }, + "transactionId": { "type": "string", "minLength": 1, "maxLength": {{MutationTransactions.MaxTransactionIdLength}}, "pattern": {{JsonSerializer.Serialize(MutationTransactions.TransactionIdNonBlankPattern)}}, "description": {{JsonSerializer.Serialize(MutationTransactions.TransactionIdSchemaDescription)}} }, "preconditions": { "type": "object", "description": "Optional batch-start guards. Each step args object may also carry its own preconditions." }, "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: isolated dry-run shorthand using atomic policy unless previewPolicy says best_effort." }, "preview": { "type": "boolean", "default": false, "description": "Dry-run mode for mode=atomic or mode=best_effort. The complete package is cloned and the live document, version, caches, configuration, and undo/redo history are never touched." }, @@ -520,7 +522,7 @@ internal static class ToolCatalog "type": "object", "properties": { "tool": { "type": "string", "enum": ["docxodus_edit", "docxodus_format", "docxodus_create", "docxodus_table", "docxodus_list", "docxodus_comment", "docxodus_links", "docxodus_images", "docxodus_content_controls", "docxodus_track_changes"] }, - "args": { "type": "object", "description": "The same arguments that tool's action takes, minus sessionId (inherited from the batch)." } + "args": { "type": "object", "description": "The same arguments that tool's action takes, minus sessionId (inherited from the batch). transactionId is forbidden here; it belongs only at the batch root." } }, "required": ["tool", "args"] }