feat(docx-compare): fail closed when a comparison input already contains tracked changes

openspec/changes/add-tracked-input-comparison-guard/proposal.md

@@ -0,0 +1,89 @@
+# Change: Fail closed when a comparison input already contains tracked changes
+
+## Why
+
+`compareDocuments` accepts an input that already carries revision markup (`w:ins`/`w:del`/moves/`*PrChange`)
+without warning, layers the comparison author's markup over it, and exits 0 with normal stats. The output keeps
+**two revision authors** in `word/document.xml` — the comparison author plus the tracked input's author — and,
+edit-density permitting, directly nested revision elements (`w:ins` inside `w:ins`). Microsoft Word refuses to open
+that file ("Word found unreadable content"). Reproduced at HEAD for issue #742: `compare(clean, tracked)` returned
+`{"insertions":2,"deletions":2,...}`, exit 0, output authors `["Comparison","Original Author"]`, with nested
+`w:ins`-in-`w:ins`; the transitional-schema gate passes the file, so the corruption is behavioral, not schema-visible.
+An independent Codex premise check confirmed both the missing guard and the runtime reproduction.
+
+A separate 520-document SHA-pinned corpus differential independently confirmed the mechanism: in **rebuild** mode
+the comparison unwraps pre-existing tracked changes into bare `w:delText` outside any `w:del` wrapper — precisely
+the Word-unreadable shape — while **inplace** passes the markup through, still merging two authors' revision trees
+into one document. The guard therefore refuses tracked inputs in both reconstruction modes, at the shared boundary.
+
+Given the current behaviour is silent corruption, failing loudly is strictly better than a clever default. This
+change intentionally rejects inputs the comparison previously accepted — a public contract change, hence this
+proposal.
+
+## What Changes
+
+- **BREAKING**: `compareDocuments` and the directly exported `compareDocumentsAtomizer` SHALL refuse to compare
+  when either operand already contains tracked-changes markup, throwing a typed recoverable
+  `TrackedInputRevisionError` (exported from the package root, following the `UnsupportedTextBoxRevisionError`
+  precedent) that names the offending operand (`original` vs `revised`), the package part, and the markers found.
+- The scan covers `word/document.xml` plus every revision story part — footnotes, endnotes, comments, the glossary
+  document, and each numbered header/footer part via `enumerateRevisionStoryPartPaths` — and detects the four
+  content markers (`w:ins`, `w:del`, `w:moveFrom`, `w:moveTo`), the six property-change records (`w:rPrChange`,
+  `w:pPrChange`, `w:sectPrChange`, `w:tblPrChange`, `w:trPrChange`, `w:tcPrChange`), the cell-topology records
+  (`w:cellIns`, `w:cellDel`, `w:cellMerge`), and `w:numberingChange` — the last four added after peer review
+  execution-proved they passed the ten-name scan and survived comparison with their prior author. Row-level
+  markers (`w:trPr > w:ins|w:del`) share those local names and trip the guard. Range-boundary markers
+  (`w:*RangeStart`/`End`, `w:customXml*Range*`) are classified non-triggers: no author-bearing content of their
+  own, content-bearing moves are caught via their wrappers, and an isolated range pair is dropped by the
+  comparison rather than passed through.
+- One guard at the lowest public comparison boundary (`compareDocumentsAtomizer`), not one per surface: the MCP
+  tool and both CLIs funnel through it, so surfaces only *map* the typed error rather than re-scanning.
+  - `compare_documents` (MCP) maps the error to a distinct `INPUT_HAS_TRACKED_CHANGES` code — never the catch-all
+    `COMPARE_ERROR` — with a part-aware recovery hint: `accept_changes` where it applies, but a header/footer
+    detection instead directs the caller to produce a fully accepted/rejected copy, because `accept_changes`
+    does not cover headers or footers and recommending it there would loop.
+  - Both CLIs (`docx-comparison`, `safe-docx compare`) propagate the error to their existing entry-point handler:
+    nonzero exit, message naming the offending operand. No CLI code change is required.
+- Missing story parts are skipped. Parts the scan cannot parse are also skipped **by this guard**: malformed-part
+  failures belong to the package-level ancillary safety boundary (`AncillaryStorySafetyError` /
+  `NOTE_PART_XML_INVALID`), whose precise typed diagnostics the preparatory scan must not pre-empt — the same rule
+  `textBoxRevisionSafety` applies to its own preparatory scan.
+- Engine behaviors that only arise for pre-tracked inputs (original-insertion provenance restoration, revised-side
+  insertion-collision promotion, preserved-move identity seeding, pre-existing-wrapper bookmark splitting,
+  canonical-emission round-trips, [ADV-COMPARE-MODE-PRESERVATION-01]'s mode characterization) remain implemented
+  and tested through `compareDocumentsAtomizerUnguarded`, an explicitly named unguarded seam that is NOT exported
+  from the package root (a test pins its absence — peer review demonstrated that a root export is a live public
+  bypass). docx-compare tests import it from the pipeline module; docx-core integration tests import it through
+  the package's dist subpath, which their vitest config aliases back to the same source module graph the root
+  alias uses (a relative source import violates that package's tsc `rootDir`). A future accept-on-ingest opt-in
+  would route there after projecting its inputs; until then the supported boundary refuses.
+
+## Impact
+
+- Affected specs: `docx-comparison` (ADDED: `Tracked-Input Comparison Refusal`) and `mcp-server`
+  (ADDED: `Tracked-Input Refusal in the Compare Documents Tool`). Both are ADDED rather than MODIFIED: no deployed
+  requirement in either capability makes any promise about comparison-input validation today, so there is nothing
+  to modify without inventing prior text the archiver would then overwrite.
+- Affected code: `packages/docx-compare/src/baselines/atomizer/trackedInputRevisionSafety.ts` (new),
+  `packages/docx-compare/src/baselines/atomizer/pipeline.ts` (guard call + unguarded seam),
+  `packages/docx-compare/src/index.ts` (exports), `packages/docx-mcp/src/tools/compare_documents.ts` (error
+  mapping).
+- Callers that previously compared tracked inputs now get a typed error instead of a Word-unreadable file. That is
+  the point of the change; the prior output was corrupt. MCP session-mode comparison of a document that was opened
+  with pre-existing tracked changes is likewise refused.
+- Existing tests that deliberately drove pre-tracked fixtures through the public entry to pin engine internals now
+  use the unguarded seam, with per-site comments citing this change. Their assertions are unchanged.
+- `compare(clean, clean)` and every clean-input comparison are byte-for-byte unaffected apart from one additional
+  scan per operand.
+- Refs #742.
+
+## Out of scope
+
+- An `--allow-tracked-input` opt-in and accept-on-ingest semantics (re-authoring emitted revisions to the
+  comparison author, matching Word's own compare) — the issue sketches this as a follow-up; the unguarded seam is
+  where it would attach.
+- Nested-revision semantics.
+- Removing or regenerating the two checked-in Word-unreadable sample outputs
+  (`packages/docx-core/src/testing/outputs/atomizer_redline.docx`, `typescript_redline.docx`): both are cited as
+  provenance by `fieldComparisonSemantics.test.ts` and `openspec/changes/add-scoped-field-evaluation/design.md`,
+  so that cleanup belongs to its own change.

openspec/changes/add-tracked-input-comparison-guard/specs/docx-comparison/spec.md

@@ -0,0 +1,70 @@
+## ADDED Requirements
+
+### Requirement: Tracked-Input Comparison Refusal
+
+The comparison boundary SHALL refuse to compare when either input document already contains tracked-changes
+markup, failing closed with a typed recoverable `TrackedInputRevisionError` instead of emitting an output that
+merges two authors' revision markup into one document. The error SHALL name the offending operand (`original` or
+`revised`), the package part in which markup was found, and the revision element names detected. The scan SHALL
+cover `word/document.xml` plus every revision story part the package holds (footnotes, endnotes, comments, the
+glossary document, and each numbered header or footer part) and SHALL detect the content markers `w:ins`, `w:del`,
+`w:moveFrom`, and `w:moveTo`, the property-change records `w:rPrChange`, `w:pPrChange`, `w:sectPrChange`,
+`w:tblPrChange`, `w:trPrChange`, and `w:tcPrChange`, the cell-topology records `w:cellIns`, `w:cellDel`, and
+`w:cellMerge`, and the legacy `w:numberingChange` record, including row-level `w:trPr > w:ins|w:del` markers.
+Range-boundary markers (`w:moveFromRangeStart`/`End`, `w:moveToRangeStart`/`End`, and the `w:customXml*Range*`
+family) are classified as non-triggers: they carry no author-bearing content of their own, content-bearing moves
+are caught via `w:moveFrom`/`w:moveTo`, and an isolated range pair is dropped by the comparison rather than passed
+through as another author's markup. The
+refusal SHALL apply to every supported comparison entry point (`compareDocuments` and the directly exported
+`compareDocumentsAtomizer`) and every reconstruction mode. Missing story parts SHALL be skipped, and parts the
+scan cannot parse SHALL be left to the package-level ancillary safety boundary's own diagnostics rather than
+claimed by this guard. The package MAY export an explicitly named unguarded engine seam
+(`compareDocumentsAtomizerUnguarded`) for engine tests over deliberately pre-tracked fixtures and as the
+attachment point for a future accept-on-ingest opt-in; it is documented as not a supported comparison entry
+point and performs no input validation.
+
+#### Scenario: [SDX-TRKIN-01] a tracked original operand is refused with a typed recoverable error
+- **GIVEN** an original document whose body already carries a `w:del` revision and a clean revised document
+- **WHEN** the documents are compared through `compareDocuments`, in either reconstruction mode
+- **THEN** the comparison SHALL throw `TrackedInputRevisionError` with `operand` = `original`, `partPath` =
+  `word/document.xml`, and `markers` containing `w:del`
+- **AND** the message SHALL tell the caller to accept or reject the original document's tracked changes and retry
+
+#### Scenario: [SDX-TRKIN-02] a tracked revised operand is refused naming the revised operand
+- **GIVEN** a clean original and a revised document that already carries a `w:ins` revision
+- **WHEN** the documents are compared
+- **THEN** the error SHALL carry `operand` = `revised` and `markers` containing `w:ins`
+
+#### Scenario: [SDX-TRKIN-03] revision markup in a revision story part is refused with the part named
+- **GIVEN** a document whose only tracked markup lives in a header, footer, footnotes, endnotes, comments, or
+  glossary part
+- **WHEN** that document is compared as either operand
+- **THEN** the comparison SHALL be refused with `partPath` naming the story part holding the markup
+
+#### Scenario: [SDX-TRKIN-04] every content and property revision kind trips the guard
+- **GIVEN** one fixture per revision kind: `w:ins`, `w:del`, `w:moveFrom`, `w:moveTo`, `w:rPrChange`,
+  `w:pPrChange`, `w:sectPrChange`, `w:tblPrChange`, `w:trPrChange`, `w:tcPrChange`, `w:cellIns`, `w:cellDel`,
+  `w:cellMerge`, `w:numberingChange`, and a row-level `w:trPr > w:del` marker
+- **WHEN** each fixture is compared as each operand
+- **THEN** each comparison SHALL be refused and `markers` SHALL report that revision kind
+
+#### Scenario: [SDX-TRKIN-05] clean inputs continue to compare unchanged
+- **GIVEN** two clean documents with no revision markup and no ancillary story parts
+- **WHEN** the pair is compared, identical or edited
+- **THEN** the comparison SHALL succeed as before, with absent story parts skipped by the scan
+
+#### Scenario: [SDX-TRKIN-06] the directly exported atomizer entry point is guarded
+- **GIVEN** a tracked input that `compareDocuments` refuses
+- **WHEN** the same pair goes through the directly exported `compareDocumentsAtomizer`
+- **THEN** it SHALL raise the same `TrackedInputRevisionError` with the same `operand`, `partPath`, and `markers`
+
+#### Scenario: [SDX-TRKIN-07] malformed revision story parts defer to the ancillary safety boundary
+- **GIVEN** an original whose `word/footnotes.xml` is truncated mid-element and referenced from the body
+- **WHEN** the documents are compared
+- **THEN** the failure SHALL be the ancillary boundary's `AncillaryStorySafetyError`, not
+  `TrackedInputRevisionError`
+
+#### Scenario: [SDX-TRKIN-08] the comparison CLI refuses tracked inputs with the operand named
+- **GIVEN** a tracked revised input staged on disk
+- **WHEN** the `docx-comparison` CLI runs with its real compare dependency (no injected fake)
+- **THEN** the run SHALL fail with a message naming the `revised` operand and SHALL write no output file

openspec/changes/add-tracked-input-comparison-guard/specs/mcp-server/spec.md

@@ -0,0 +1,47 @@
+## ADDED Requirements
+
+### Requirement: Tracked-Input Refusal in the Compare Documents Tool
+
+The `compare_documents` tool and the `safe-docx compare` CLI command SHALL surface the comparison library's
+tracked-input refusal as a deliberate, recoverable outcome. The MCP tool SHALL map `TrackedInputRevisionError` to
+the distinct error code `INPUT_HAS_TRACKED_CHANGES` — never the catch-all `COMPARE_ERROR` — with a message naming
+the offending operand and part and a hint pointing the caller at accepting or rejecting the input's revisions
+first. The hint SHALL be actionable for the named part: `accept_changes` covers the document body and the
+revisionable side stories but not headers or footers, so a detection in a header or footer part SHALL NOT
+recommend `accept_changes` and SHALL instead direct the caller to produce a fully accepted or rejected copy of
+the input. The refusal applies to session-mode comparison as well as two-file mode. The CLI command SHALL
+propagate the error so the process exits nonzero with a message naming the offending operand. Neither surface
+SHALL write an output file for a refused comparison. Clean inputs SHALL be unaffected.
+
+#### Scenario: [SDX-TRKIN-MCP-01] compare_documents refuses tracked inputs with a distinct error code
+- **GIVEN** a clean original and a revised file that already carries `w:ins` markup
+- **WHEN** `compare_documents` is called in two-file mode
+- **THEN** the response SHALL be an `INPUT_HAS_TRACKED_CHANGES` error, not `COMPARE_ERROR`
+- **AND** the message SHALL name the `revised` operand and `word/document.xml`, with a hint referencing
+  `accept_changes`
+- **AND** no file SHALL be written to `save_to_local_path`
+
+#### Scenario: [SDX-TRKIN-MCP-02] the compare CLI command surfaces the tracked-input refusal
+- **GIVEN** a tracked original staged on disk
+- **WHEN** the `safe-docx compare` command runs with its real default dependencies
+- **THEN** the command SHALL reject with `TrackedInputRevisionError` naming the `original` operand, producing a
+  nonzero process exit
+- **AND** no output file SHALL be written
+
+#### Scenario: [SDX-TRKIN-MCP-04] header and footer refusals carry an actionable hint
+- **GIVEN** a revised file whose only tracked markup lives in `word/header1.xml`
+- **WHEN** `compare_documents` is called in two-file mode
+- **THEN** the response SHALL be an `INPUT_HAS_TRACKED_CHANGES` error naming `word/header1.xml`
+- **AND** the hint SHALL NOT recommend `accept_changes`, which cannot clean headers or footers
+- **AND** no output file SHALL be written
+
+#### Scenario: [SDX-TRKIN-MCP-05] session-mode comparison of a tracked document is refused
+- **GIVEN** an open session whose document already carries `w:ins` markup
+- **WHEN** `compare_documents` is called in session mode
+- **THEN** the response SHALL be an `INPUT_HAS_TRACKED_CHANGES` error
+- **AND** no output file SHALL be written
+
+#### Scenario: [SDX-TRKIN-MCP-03] compare_documents with clean inputs is unaffected
+- **GIVEN** two clean documents with an ordinary edit between them
+- **WHEN** `compare_documents` is called in two-file mode
+- **THEN** the comparison SHALL succeed and the redline SHALL be written as before