From 3228b8bad6028f777a3f2adb42ee831479e59b04 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 18:54:42 +0000 Subject: [PATCH] feat(edit): warn when a whole-paragraph edit resolves other authors' revisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A whole-paragraph content edit (`edit --at pN --text/--markdown/--runs`, a range edit, `--batch`, `code edit`) rebuilds the paragraph under a single-author model, so another author's pending insertion became plain text (or was dropped) and their deletion vanished — silently, exit 0. Interim behavior per maintainer decision on #17: the edit still applies, but the CLI now warns on stderr naming those authors and pointing at span edits, which keep the revisions pending. Detection diffs the edited region's revisions before vs after the rebuild, so it reports only what was actually lost. Refs #17 Triage-Routine: true Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012GoxD36rnNfg6uArkVxtJL --- src/cli/code/edit.ts | 19 +- src/cli/edit/batch.ts | 40 +++- src/cli/edit/index.tsx | 53 +++-- src/cli/edit/pending-revisions-warning.ts | 26 +++ src/core/edit/index.tsx | 35 +++- src/core/index.ts | 1 + src/core/track-changes/CLAUDE.md | 5 +- src/core/track-changes/pending-authors.ts | 76 +++++++ tests/cli/code.test.ts | 24 ++- tests/cli/edit.test.ts | 233 +++++++++++++++++++++- 10 files changed, 477 insertions(+), 35 deletions(-) create mode 100644 src/cli/edit/pending-revisions-warning.ts create mode 100644 src/core/track-changes/pending-authors.ts diff --git a/src/cli/code/edit.ts b/src/cli/code/edit.ts index 3269056..84ff511 100644 --- a/src/cli/code/edit.ts +++ b/src/cli/code/edit.ts @@ -1,4 +1,5 @@ import { Edit, EditError, type ParagraphContentSpec } from "@core"; +import { warnPendingRevisions } from "../edit/pending-revisions-warning"; import { parseParagraphOptions } from "../insert/index"; import { EXIT, @@ -119,15 +120,23 @@ export async function run(args: string[]): Promise { const track = resolveTracked(document, Boolean(parsed.values.track)); const authorFlag = parsed.values.author as string | undefined; + const isRange = /^p\d+-p\d+$/.test(locator); + let resolvedAuthors: string[]; try { - if (/^p\d+-p\d+$/.test(locator)) { + if (isRange) { const rangeRef = await resolveBlockRangeOrFail(document, locator); if (typeof rangeRef === "number") return rangeRef; - new Edit(document).range(rangeRef, spec, { authorFlag, track }); + resolvedAuthors = new Edit(document).range(rangeRef, spec, { + authorFlag, + track, + }); } else { const blockRef = await resolveBlockOrFail(document, locator); if (typeof blockRef === "number") return blockRef; - new Edit(document).paragraph(blockRef, spec, { authorFlag, track }); + ({ resolvedAuthors } = new Edit(document).paragraph(blockRef, spec, { + authorFlag, + track, + })); } } catch (error) { if (error instanceof EditError) { @@ -137,6 +146,10 @@ export async function run(args: string[]): Promise { } await document.save(outputPath); + await warnPendingRevisions(locator, resolvedAuthors, { + paragraphLocator: isRange ? null : locator, + tracked: track, + }); await respondAck({ ok: true, operation: "edit", diff --git a/src/cli/edit/batch.ts b/src/cli/edit/batch.ts index 7361fec..82c4601 100644 --- a/src/cli/edit/batch.ts +++ b/src/cli/edit/batch.ts @@ -10,6 +10,7 @@ import { locatorToBlockTarget, MarkdownImport, MarkdownImportError, + type ParagraphEditResult, parseCellAt, parseLocator, type Run, @@ -40,6 +41,7 @@ import { respond, respondAck, } from "../respond"; +import { warnPendingRevisions } from "./pending-revisions-warning"; import { parseTabsValue, resolveTabsDirective } from "./tabs"; type RawValues = Record< @@ -157,6 +159,12 @@ export async function runEditBatch( } await document.save(outputPath); + for (const entry of resolved) { + await warnPendingRevisions(entry.locatorString, entry.resolvedAuthors, { + paragraphLocator: entry.paragraphLocator, + tracked: track, + }); + } await respondAck({ ok: true, @@ -272,8 +280,14 @@ type ResolvedEntry = { span: { start: number; end: number } | null; /** Bare-cell targets repair Word's mandatory terminal paragraph after apply. */ cellParent?: XmlNode[]; + /** The paragraph a span-edit suggestion addresses: `at` itself, or a bare + * cell's sole `tN:rRcC:p0` (a cell locator takes no `:S-E` span). */ + paragraphLocator: string; /** Performs the mutation via the `Edit` lens. Throws `EditError`. */ apply: () => void; + /** Filled by `apply` for a whole-paragraph content edit: the other authors + * whose pending revisions it resolved (warned after the save, issue #17). */ + resolvedAuthors: string[]; }; /** Validate one JSONL entry and produce its `apply` closure. Markdown content @@ -339,6 +353,7 @@ async function resolveEntry( let blockRef: BlockReference; let span: { start: number; end: number } | null; let cellParent: XmlNode[] | undefined; + let paragraphLocator = at; if (parseCellAt(at)) { try { const resolved = resolveCellParagraphReference( @@ -348,6 +363,7 @@ async function resolveEntry( ); blockRef = resolved.paragraph; cellParent = resolved.cell.parent; + paragraphLocator = `${resolved.cell.id}:p0`; } catch (error) { if (error instanceof CellTargetError) { throw new EntryError( @@ -432,7 +448,9 @@ async function resolveEntry( node: blockRef.node, span: null, ...(cellParent ? { cellParent } : {}), + paragraphLocator, apply, + resolvedAuthors: [], }; } @@ -465,6 +483,7 @@ async function resolveEntry( | "clear" | "props"; + const resolvedAuthors: string[] = []; const apply = await buildApply( document, raw, @@ -473,6 +492,7 @@ async function resolveEntry( blockRef, span, opts, + resolvedAuthors, ); return { index, @@ -480,7 +500,9 @@ async function resolveEntry( node: blockRef.node, span, ...(cellParent ? { cellParent } : {}), + paragraphLocator, apply, + resolvedAuthors, }; } @@ -494,6 +516,7 @@ async function buildApply( blockRef: BlockReference, span: { start: number; end: number } | null, opts: EntryOptions, + resolvedAuthors: string[], ): Promise<() => void> { const author = typeof raw.author === "string" ? raw.author : opts.authorFlag; const clearTags = @@ -590,18 +613,19 @@ async function buildApply( opts, ); const ride = readRunFormatRideAlong(raw, index, kind === "text"); - if (!clearTags && !ride) return () => void contentNode(); return () => { - const node = contentNode(); + const result = contentNode(); + resolvedAuthors.push(...result.resolvedAuthors); const edit = new Edit(document); - if (clearTags) edit.clearFormattingNode(node, null, clearTags); - if (ride) edit.setFormattingNode(node, null, ride); + if (clearTags) edit.clearFormattingNode(result.node, null, clearTags); + if (ride) edit.setFormattingNode(result.node, null, ride); }; } -/** Build a closure that applies one whole-paragraph content edit and returns the - * resulting paragraph node. Split out so the combined content+clear path can - * clear that node afterward. */ +/** Build a closure that applies one whole-paragraph content edit and returns + * the resulting paragraph node (plus the other authors whose pending revisions + * it resolved). Split out so the combined content+clear path can clear that + * node afterward. */ async function buildWholeParagraphContent( document: Document, raw: Record, @@ -610,7 +634,7 @@ async function buildWholeParagraphContent( blockRef: BlockReference, author: string | undefined, opts: EntryOptions, -): Promise<() => XmlNode> { +): Promise<() => ParagraphEditResult> { const paragraphOptions = readParagraphOptions(document, raw, index); if (kind === "text") { // Whole-paragraph empty "text" never reaches here — `resolveEntry` diff --git a/src/cli/edit/index.tsx b/src/cli/edit/index.tsx index 66c1751..b5fbc2e 100644 --- a/src/cli/edit/index.tsx +++ b/src/cli/edit/index.tsx @@ -48,6 +48,7 @@ import { writeStdout, } from "../respond"; import { runEditBatch } from "./batch"; +import { warnPendingRevisions } from "./pending-revisions-warning"; import { parseTabsValue, resolveTabsDirective, @@ -304,7 +305,7 @@ export async function run(args: string[]): Promise { document, opts.locator, ); - return commitBlockEdit(document, paragraph, opts, cell.parent); + return commitBlockEdit(document, paragraph, opts, cell); } catch (error) { if (error instanceof CellTargetError) { return fail(error.code, error.message, error.hint); @@ -434,16 +435,18 @@ async function commitSpanEdit( /** Single-block edit: paragraph content / props dispatch through the Edit * lens. The lens handles tracked-vs-untracked, style ensures, and the - * formatting-preservation decision. */ + * formatting-preservation decision. `cell` is set for a bare-cell target + * (`tN:rRcC`), whose sole paragraph is the edited block. */ async function commitBlockEdit( document: Document, blockRef: BlockReference, opts: ValidatedOptions, - cellParent?: XmlNode[], + cell?: { id: string; parent: XmlNode[] }, ): Promise { if (opts.dryRun) return respondDryRun(opts); const track = resolveTracked(document, opts.trackFlag); + let resolvedAuthors: string[] = []; try { const edit = new Edit(document); // The paragraph node the content edit produced — a combined `--clear` @@ -451,19 +454,27 @@ async function commitBlockEdit( // node, not the original blockRef). let resultNode: XmlNode | null = null; if (opts.spec.kind === "text" || opts.spec.kind === "runs") { - resultNode = edit.paragraph(blockRef, opts.spec, { - authorFlag: opts.authorFlag, - noFormatting: opts.noFormatting, - track, - }); + ({ node: resultNode, resolvedAuthors } = edit.paragraph( + blockRef, + opts.spec, + { + authorFlag: opts.authorFlag, + noFormatting: opts.noFormatting, + track, + }, + )); } else if (opts.spec.kind === "markdown") { const resolved = await resolveMarkdownBlocks(document, opts.spec); if (typeof resolved === "number") return resolved; - resultNode = edit.paragraph(blockRef, resolved, { - authorFlag: opts.authorFlag, - noFormatting: opts.noFormatting, - track, - }); + ({ node: resultNode, resolvedAuthors } = edit.paragraph( + blockRef, + resolved, + { + authorFlag: opts.authorFlag, + noFormatting: opts.noFormatting, + track, + }, + )); } else if (opts.spec.kind === "removeLine") { // Empty `--text` on a whole paragraph removes the line (cell-safe — a // table cell's last paragraph is blanked, not deleted). Same path as @@ -505,8 +516,12 @@ async function commitBlockEdit( throw error; } - if (cellParent) ensureCellEndsWithParagraph(cellParent); + if (cell) ensureCellEndsWithParagraph(cell.parent); await document.save(opts.outputPath); + await warnPendingRevisions(opts.locator, resolvedAuthors, { + paragraphLocator: cell ? `${cell.id}:p0` : opts.locator, + tracked: track, + }); return emitEditAck(opts); } @@ -566,10 +581,12 @@ async function commitRangeEdit( return fail("USAGE", "Unsupported edit spec for range locator"); } + const track = resolveTracked(document, opts.trackFlag); + let resolvedAuthors: string[]; try { - new Edit(document).range(rangeRef, spec, { + resolvedAuthors = new Edit(document).range(rangeRef, spec, { authorFlag: opts.authorFlag, - track: resolveTracked(document, opts.trackFlag), + track, }); } catch (error) { if (error instanceof EditError) { @@ -579,6 +596,10 @@ async function commitRangeEdit( } await document.save(opts.outputPath); + await warnPendingRevisions(opts.locator, resolvedAuthors, { + paragraphLocator: null, + tracked: track, + }); return emitEditAck(opts); } diff --git a/src/cli/edit/pending-revisions-warning.ts b/src/cli/edit/pending-revisions-warning.ts new file mode 100644 index 0000000..bb61101 --- /dev/null +++ b/src/cli/edit/pending-revisions-warning.ts @@ -0,0 +1,26 @@ +import { writeStderr } from "../respond"; + +/** Warn that a content edit resolved other authors' pending tracked changes + * without review (issue #17). The edit still succeeds — this is the interim + * behavior until the rebuild keeps them — so the warning names the span-edit + * alternatives that DO keep them pending. `paragraphLocator` is the paragraph + * the `:START-END` example addresses: the edited locator by default, a bare + * cell's sole `tN:rRcC:p0` (a cell locator takes no span), or null for a + * multi-paragraph range. `tracked` carries the edit's own tracking into both + * suggestions, so following one records the change the same way. */ +export async function warnPendingRevisions( + locator: string, + authors: string[], + { + paragraphLocator = locator, + tracked, + }: { paragraphLocator?: string | null; tracked: boolean }, +): Promise { + if (authors.length === 0) return; + const track = tracked ? " --track" : ""; + await writeStderr( + `warning: ${locator} had pending tracked changes by ${authors.join(", ")}; ` + + "replacing whole paragraphs resolved them without review (they can no longer be accepted or rejected as theirs). " + + `To keep them pending, edit only the words that change: \`docx replace FILE OLD NEW${track}\` or \`docx edit FILE --at ${paragraphLocator ?? "pN"}:START-END --text NEW${track}\`.\n`, + ); +} diff --git a/src/core/edit/index.tsx b/src/core/edit/index.tsx index 6f79adf..3d09af3 100644 --- a/src/core/edit/index.tsx +++ b/src/core/edit/index.tsx @@ -52,6 +52,7 @@ import { TrackChanges, type TrackedMeta, } from "../track-changes"; +import { watchPendingRevisions } from "../track-changes/pending-authors"; import { applyFormattingPreservingEdit, applyTrackedRangeReplace, @@ -129,7 +130,7 @@ export class Edit { blockRef: BlockReference, spec: ParagraphContentSpec, opts: { authorFlag?: string; noFormatting?: boolean; track?: boolean } = {}, - ): XmlNode { + ): ParagraphEditResult { const targetIndex = blockRef.parent.indexOf(blockRef.node); if (targetIndex === -1) { throw new EditError( @@ -137,6 +138,15 @@ export class Edit { "Block reference is stale (parent does not contain it)", ); } + // Snapshot BEFORE the rebuild: the single-author content rebuild resolves + // other authors' pending revisions without review (issue #17), so the + // caller can warn about whichever ones didn't survive. + const listResolvedAuthors = watchPendingRevisions( + blockRef.parent, + targetIndex, + targetIndex, + opts.authorFlag, + ); this.document .ensureStyles() @@ -163,7 +173,7 @@ export class Edit { ); this.reanchorComments(blockRef.node, commentMarkers); // Mutated in place — the same node is the result. - return blockRef.node; + return { node: blockRef.node, resolvedAuthors: listResolvedAuthors() }; } if (spec.kind === "code") { @@ -226,7 +236,10 @@ export class Edit { } // The spliced-in first paragraph is the result (a following clear in a // combined content+clear edit targets this node, not the replaced one). - return anchorTarget ?? blockRef.node; + return { + node: anchorTarget ?? blockRef.node, + resolvedAuthors: listResolvedAuthors(), + }; } /** Properties-only edit: re-apply paragraph properties (`--style`/`--alignment`/ @@ -417,12 +430,13 @@ export class Edit { * cross-paragraph LCS, and we match it). Rejects tracked ranges that span * a non-paragraph block (most commonly a table) because the tracked-range * walker injects `` into every span block, which would corrupt - * ``. */ + * ``. Returns the other authors whose pending revisions the replace + * resolved without review (issue #17), for the caller to warn about. */ range( rangeRef: BlockRangeReference, spec: ParagraphContentSpec, opts: { authorFlag?: string; track?: boolean } = {}, - ): void { + ): string[] { this.document .ensureStyles() .ensureReferencedStyle(spec.paragraphOptions.style); @@ -449,6 +463,12 @@ export class Edit { } } + const listResolvedAuthors = watchPendingRevisions( + rangeRef.parent, + rangeRef.startIndex, + rangeRef.endIndex, + opts.authorFlag, + ); const newParagraphs = buildNewParagraphs(spec); if (tracked) { applyTrackedRangeReplace( @@ -467,9 +487,14 @@ export class Edit { newParagraphs, ); } + return listResolvedAuthors(); } } +/** `Edit.paragraph`'s result: the paragraph node the content edit produced, + * and the other authors whose pending revisions the rebuild resolved. */ +export type ParagraphEditResult = { node: XmlNode; resolvedAuthors: string[] }; + /** The paragraph-content specs that produce one or more new paragraphs. * Shared between `Edit.paragraph` (single block) and `Edit.range` (block * range); equation/task/section have their own method signatures. The diff --git a/src/core/index.ts b/src/core/index.ts index 07c2a49..6ad44d5 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -60,6 +60,7 @@ export { Edit, EditError, type ParagraphContentSpec, + type ParagraphEditResult, type RunFormat, resolveClearTags, } from "./edit"; diff --git a/src/core/track-changes/CLAUDE.md b/src/core/track-changes/CLAUDE.md index 4171a71..b6f70a5 100644 --- a/src/core/track-changes/CLAUDE.md +++ b/src/core/track-changes/CLAUDE.md @@ -1,12 +1,13 @@ # src/core/track-changes — `TrackChanges` lens, accept/reject machinery, tracked-range shapes -Five files behind the `@core/track-changes` barrel ([index.tsx](index.tsx)): +Five files behind the `@core/track-changes` barrel ([index.tsx](index.tsx)), plus one `core/edit` imports directly: - [index.tsx](index.tsx) — the `TrackChanges` lens + small free helpers (`resolveAuthor`, `resolveDate`, `convertTextToDelText`) - [apply.ts](apply.ts) — accept/reject state machine: `previewTrackedChanges`, `applyTrackedChanges`, `applyTrackedDecisions` (the mixed accept+reject pass behind `track-changes apply` — both lists resolved against the pre-mutation tree, applied in one reverse-preorder traversal via the shared `applyResolvedTargets` core), `collectTrackedChanges`, the `actionFor` dispatch table, body-side note pairing, table-grid resync - [emit.tsx](emit.tsx) — leaf primitives: ``, ``, `markParagraphMarkAs(paragraph, kind, meta)` (drops a self-closing ins/del marker into ``) - [replace.tsx](replace.tsx) — range edit/delete shapes empirically validated against Word: `applyTrackedRangeReplace`, `applyTrackedRangeDelete`, `applyUntrackedRangeReplace`, `applyUntrackedRangeDelete`, the shared `assertParagraphOnlyTrackedRange` guard + `TrackedRangeConflictError`, `applyFormattingPreservingEdit` (the word-level diff for `edit --text`), and `removeParagraphLine` — the cell-safe single-paragraph removal shared by `docx delete --at pN` and `docx edit --at pN --text ""` (a ``'s last paragraph is blanked, not deleted, so we never emit an invalid empty ``) - [preserve-formatting.tsx](preserve-formatting.tsx) — the LCS-based word-level diff that drives `applyFormattingPreservingEdit`: `extractOldTokens`, `tokenize`, `diffTokens`, `buildTrackedRuns`, `buildUntrackedRuns` +- [pending-authors.ts](pending-authors.ts) — `watchPendingRevisions`, the before/after snapshot `Edit.paragraph`/`Edit.range` use to report the other authors whose pending revisions a content rebuild resolved (issue #17; not re-exported by the barrel) ## TrackChanges lens API @@ -76,6 +77,8 @@ The shorthand: 3. **LCS diff** aligns new tokens against old (`diffTokens`). 4. **Re-emits runs** grouped by rPr: untracked path emits only kept + inserted tokens (inherited rPr from nearest matched neighbor); tracked path emits kept tokens as plain runs, inserts in ``, deletes in ``. Pure pPr properties (`--style`, `--alignment`) apply in place via `applyParagraphOptionsInPlace`. +**Other authors' pending revisions are resolved, not kept (issue #17, interim).** The diff rebuilds runs from the visible text under the single-author model, so another author's `` becomes plain text (or, on the `--markdown` path, is dropped) and their `` vanishes. Until the rebuild keeps them, the CLI warns on stderr (exit 0) whenever a content edit — whole-paragraph `text`/`markdown`/`runs` (single or `--batch`), a `pN-pM` range replace, or `code edit` — resolves pending revisions by an author other than the editor, and points at span edits (`replace`, `edit --at pN:S-E`), which keep them. `Edit.paragraph` returns the authors as `resolvedAuthors` (`Edit.range` returns the list): `watchPendingRevisions` ([pending-authors.ts](pending-authors.ts)) snapshots the other authors' ``/``/``/``/`` BEFORE the rebuild and reports the ones missing from the edited region AFTER it. Don't replace that with a prediction of what each path keeps — the `--text` diff carries text boxes and inline content controls through and keeps the pPr, while the markdown/runs replace drops the box and any pPrChange and, under tracking, keeps an existing `` but drops every ``; only the before/after diff is right for all of them. + rPr equality is structural (XML-string compare on cloned nodes). The codebase's emitter produces canonical output so this works in practice; non-canonical rPr from an external producer might leave equivalent rPr blocks unmerged (each token gets its own run). Acceptable. ## Adding a new tracked-change kind diff --git a/src/core/track-changes/pending-authors.ts b/src/core/track-changes/pending-authors.ts new file mode 100644 index 0000000..775bd08 --- /dev/null +++ b/src/core/track-changes/pending-authors.ts @@ -0,0 +1,76 @@ +import type { XmlNode } from "../parser"; +import { resolveAuthor } from "./index"; + +/** Watch `parent[startIndex..endIndex]` across an in-place content rebuild + * (`Edit.paragraph` / `Edit.range`) for other authors' pending revisions it + * resolves without review (issue #17). Call BEFORE mutating; the returned + * function, called after, names (in document order) every author other than + * the editor whose run-level revision (``/``/``/ + * ``) or `` no longer appears in the edited region. + * + * Before/after instead of predicting: each rebuild path keeps a different + * subset — the `--text` diff carries text boxes and inline content controls + * through and keeps the pPr; the `--markdown`/`--runs` replace drops the box + * and a pPrChange, and under tracking keeps an existing `` but drops + * every `` — so only diffing what survived stays accurate as those + * paths change. A revision is matched by tag + `w:id` + author + date, so a + * CLONED survivor (the paragraph-mark marker an inherited pPr carries over) + * counts as kept. The region's extent after the edit is read off the parent's + * length change: a replace splices N paragraphs where M stood. */ +export function watchPendingRevisions( + parent: XmlNode[], + startIndex: number, + endIndex: number, + authorFlag: string | undefined, +): () => string[] { + const editor = resolveAuthor(authorFlag); + const pending = new Map(); + for (const node of parent.slice(startIndex, endIndex + 1)) { + visitRevisions(node, (revision) => { + const author = revision.getAttribute("w:author") ?? ""; + if (author === editor) return; + pending.set(revisionKey(revision), author || "(unknown author)"); + }); + } + const lengthBefore = parent.length; + return () => { + if (pending.size === 0) return []; + const editedEnd = endIndex + 1 + parent.length - lengthBefore; + const surviving = new Set(); + for (const node of parent.slice(startIndex, editedEnd)) { + visitRevisions(node, (revision) => surviving.add(revisionKey(revision))); + } + const authors = new Set(); + for (const [key, author] of pending) { + if (!surviving.has(key)) authors.add(author); + } + return [...authors]; + }; +} + +const REVISION_TAGS: ReadonlySet = new Set([ + "w:ins", + "w:del", + "w:moveFrom", + "w:moveTo", + "w:pPrChange", +]); + +function visitRevisions( + node: XmlNode, + visit: (revision: XmlNode) => void, +): void { + for (const child of node.children) { + if (REVISION_TAGS.has(child.tag)) visit(child); + visitRevisions(child, visit); + } +} + +function revisionKey(revision: XmlNode): string { + return [ + revision.tag, + revision.getAttribute("w:id") ?? "", + revision.getAttribute("w:author") ?? "", + revision.getAttribute("w:date") ?? "", + ].join("\u0000"); +} diff --git a/tests/cli/code.test.ts b/tests/cli/code.test.ts index a29b8a3..43710f8 100644 --- a/tests/cli/code.test.ts +++ b/tests/cli/code.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"; import { join } from "node:path"; import { Pkg } from "@core/ast/document/package"; import { runCli, tempWorkspace } from "./harness"; -import { readDocumentXml } from "./helpers"; +import { buildRawDoc, readDocumentXml } from "./helpers"; type AstParagraph = { id: string; @@ -451,6 +451,28 @@ describe("docx code edit", () => { const xml = await readDocumentXml(docPath); expect(xml).toMatch(/ { + const docPath = await buildRawDoc( + `x = 1`, + "code-edit-pending", + ); + const result = await runCli( + "code", + "edit", + docPath, + "--at", + "p0", + "--code", + "x = 2", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(result.stderr).toContain( + "warning: p0 had pending tracked changes by Reviewer A;", + ); + }); }); describe("docx read --markdown for code blocks", () => { diff --git a/tests/cli/edit.test.ts b/tests/cli/edit.test.ts index 6b5767c..f4ba525 100644 --- a/tests/cli/edit.test.ts +++ b/tests/cli/edit.test.ts @@ -4,7 +4,13 @@ import { Pkg } from "@core/ast/document/package"; import { clearFormatting, resolveClearTags } from "@core/edit/clear-formatting"; import { XmlNode } from "@core/parser"; import { runCli, tempWorkspace } from "./harness"; -import { freshFixture, readDocumentXml, trackedKinds } from "./helpers"; +import { + buildRawDoc, + freshFixture, + readDocumentXml, + trackedKinds, + wordTextBoxParagraphXml, +} from "./helpers"; const FIXTURE = "tests/fixtures/word-formatted.docx"; // Layout (built by tests/fixtures/setup/word-formatted.ts): @@ -4044,3 +4050,228 @@ describe("docx edit complex-script font", () => { expect(runs[1]).toMatchObject({ fontEastAsia: "SimSun" }); }); }); + +describe("whole-paragraph edit over other authors' pending changes (#17)", () => { + const NESTED = + `Fees are due. ` + + `The Client may withhold ` + + `any ` + + `disputed amount.`; + + test("warns on stderr naming the other authors, and still applies the edit", async () => { + const docPath = await buildRawDoc(NESTED, "pending-warn"); + const result = await runCli( + "edit", + docPath, + "--at", + "p0", + "--text", + "Fees are due. The Client may withhold disputed sum.", + "--track", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(result.stderr).toContain( + "warning: p0 had pending tracked changes by Reviewer A, Reviewer B", + ); + expect(result.stderr).toContain("--at p0:START-END"); + expect(await readDocumentXml(docPath)).toContain(" sum."); + }); + + test("the editing author's own pending changes don't warn", async () => { + const docPath = await buildRawDoc( + `Hello world`, + "pending-own", + ); + const result = await runCli( + "edit", + docPath, + "--at", + "p0", + "--text", + "Hello there", + "--track", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(result.stderr).not.toContain("warning"); + }); + + test("batch warns per whole-paragraph entry, using the entry's author", async () => { + const docPath = await buildRawDoc(NESTED, "pending-batch"); + const dir = tempWorkspace("pending-batch-jsonl"); + const batchPath = join(dir, "edits.jsonl"); + await Bun.write( + batchPath, + `${JSON.stringify({ at: "p0", text: "Fees are due. X.", author: "Reviewer A" })}\n`, + ); + const result = await runCli("edit", docPath, "--batch", batchPath); + expect(result.exitCode).toBe(0); + expect(result.stderr).toContain( + "warning: p0 had pending tracked changes by Reviewer B;", + ); + }); + + test("a revision inside an anchored text box doesn't warn — the box rides through intact", async () => { + const pendingInStory = `Box note`; + const docPath = await buildRawDoc( + wordTextBoxParagraphXml([pendingInStory], { leadingText: "Anchor " }), + "pending-textbox", + ); + const result = await runCli( + "edit", + docPath, + "--at", + "p0", + "--text", + "Anchor moved", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(result.stderr).not.toContain("warning"); + expect(await readDocumentXml(docPath)).toContain('w:author="Reviewer A"'); + }); + + test("a span edit keeps the changes pending and doesn't warn", async () => { + const docPath = await buildRawDoc(NESTED, "pending-span"); + const result = await runCli( + "replace", + docPath, + "disputed amount", + "disputed sum", + "--track", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(result.stderr).not.toContain("warning"); + }); + + test("a bare-cell target's span suggestion names the cell's paragraph, not the cell", async () => { + const docPath = await buildRawDoc( + `Hi there`, + "pending-cell", + ); + const result = await runCli( + "edit", + docPath, + "--at", + "t0:r0c0", + "--text", + "Hi you", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(result.stderr).toContain("warning: t0:r0c0 had pending"); + expect(result.stderr).toContain("--at t0:r0c0:p0:START-END"); + }); + + test("--markdown over a text box anchor warns: the replace drops the box and its story's revisions", async () => { + const pendingInStory = `Box note`; + const docPath = await buildRawDoc( + wordTextBoxParagraphXml([pendingInStory], { leadingText: "Anchor " }), + "pending-textbox-md", + ); + const result = await runCli( + "edit", + docPath, + "--at", + "p0", + "--markdown", + "Anchor **moved**", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(await readDocumentXml(docPath)).not.toContain("Reviewer A"); + expect(result.stderr).toContain( + "warning: p0 had pending tracked changes by Reviewer A;", + ); + }); + + test("another author's paragraph-property change dropped by --markdown warns", async () => { + const docPath = await buildRawDoc( + `Hello world`, + "pending-pprchange", + ); + const result = await runCli( + "edit", + docPath, + "--at", + "p0", + "--markdown", + "Hello **there**", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(await readDocumentXml(docPath)).not.toContain("w:pPrChange"); + expect(result.stderr).toContain("by Reviewer A;"); + }); + + test("a range replace warns, with a per-paragraph span suggestion", async () => { + const docPath = await buildRawDoc( + `${NESTED}Second.`, + "pending-range", + ); + const result = await runCli( + "edit", + docPath, + "--at", + "p0-p1", + "--text", + "Merged.", + "--track", + "--author", + "Editor", + ); + expect(result.exitCode).toBe(0); + expect(result.stderr).toContain( + "warning: p0-p1 had pending tracked changes by Reviewer A, Reviewer B;", + ); + expect(result.stderr).toContain("--at pN:START-END"); + }); + + test("revisions the edit keeps don't warn: a content control's toggle, a tracked replace's surviving deletion", async () => { + const checkbox = await buildRawDoc( + `☐☒ Buy milk`, + "pending-sdt", + ); + const toggled = await runCli( + "edit", + checkbox, + "--at", + "p0", + "--text", + "Buy bread", + "--author", + "Editor", + ); + expect(toggled.exitCode).toBe(0); + expect(await readDocumentXml(checkbox)).toContain('w:author="Reviewer A"'); + expect(toggled.stderr).not.toContain("warning"); + + const deletion = await buildRawDoc( + `Fees any due.`, + "pending-kept-del", + ); + const replaced = await runCli( + "edit", + deletion, + "--at", + "p0", + "--markdown", + "Fees **due**.", + "--track", + "--author", + "Editor", + ); + expect(replaced.exitCode).toBe(0); + expect(await readDocumentXml(deletion)).toContain('w:author="Reviewer B"'); + expect(replaced.stderr).not.toContain("warning"); + }); +});