Update docx, pptx, and xlsx skills (#1447)

Add support for template formats (.dotx, .potx, .xltx) across validation
and the helper scripts.

Consolidate the shared office helpers into a single module, replacing the
pack/unpack pipeline with explicit zip/unzip steps. Extraction now rejects
symlink and path-traversal archive entries. Move run merging to a
standalone docx script, since it only ever applied to Word documents.

Fix the redlining validator so it compares against the original even when
a document has no tracked changes, which is when an untracked edit would
otherwise go unreported.

Provision a LibreOffice user profile per invocation so conversions work in
sandboxed environments.

Trim the skill docs to the guidance that earns its place.

skills/docx/scripts/comment.py

@@ -1,26 +1,41 @@
-"""Add comments to DOCX documents.
+"""Add comments to a DOCX document.
+
+Accepts either an unpacked directory OR a .docx/.dotx file directly.
 
 Usage:
-    python comment.py unpacked/ 0 "Comment text"
-    python comment.py unpacked/ 1 "Reply text" --parent 0
+    # Against an unpacked directory (writes satellite files in place)
+    python comment.py unpacked/ "Comment text"
+    python comment.py unpacked/ "Reply text" --parent 0
 
-Text should be pre-escaped XML (e.g., & for &, ’ for smart quotes).
+    # Against a .docx directly (extracts, writes satellite files, rezips)
+    python comment.py contract.docx "This cap is too low" -o annotated.docx
+    python comment.py contract.docx "Comment" --id 5      # explicit ID
 
-After running, add markers to document.xml:
-  <w:commentRangeStart w:id="0"/>
+The comment ID is auto-assigned (max existing + 1) unless --id is given.
+Plain text is XML-escaped automatically; if you pass already-escaped text
+(e.g. &amp;, &#x2019;) use --raw to skip escaping.
+
+After running, add markers to word/document.xml so the comment is visible:
+  <w:commentRangeStart w:id="N"/>
   ... commented content ...
-  <w:commentRangeEnd w:id="0"/>
-  <w:r><w:rPr><w:rStyle w:val="CommentReference"/></w:rPr><w:commentReference w:id="0"/></w:r>
+  <w:commentRangeEnd w:id="N"/>
+  <w:r><w:rPr><w:rStyle w:val="CommentReference"/></w:rPr><w:commentReference w:id="N"/></w:r>
 """
 
 import argparse
 import random
 import shutil
 import sys
+import tempfile
+import zipfile
 from datetime import datetime, timezone
 from pathlib import Path
 
 import defusedxml.minidom
+from xml.parsers.expat import ExpatError
+from xml.sax.saxutils import escape as xml_escape
+
+from office.helpers import opc_target, rezip as _rezip, safe_extract as _safe_extract
 
 TEMPLATE_DIR = Path(__file__).parent / "templates"
 NS = {
@@ -44,39 +59,38 @@ COMMENT_XML = """\
         <w:sz w:val="20"/>
         <w:szCs w:val="20"/>
       </w:rPr>
-      <w:t>{text}</w:t>
+      <w:t xml:space="preserve">{text}</w:t>
     </w:r>
   </w:p>
 </w:comment>"""
 
 COMMENT_MARKER_TEMPLATE = """
-Add to document.xml (markers must be direct children of w:p, never inside w:r):
+Add to word/document.xml (markers must be direct children of w:p, never inside w:r):
   <w:commentRangeStart w:id="{cid}"/>
   <w:r>...</w:r>
   <w:commentRangeEnd w:id="{cid}"/>
   <w:r><w:rPr><w:rStyle w:val="CommentReference"/></w:rPr><w:commentReference w:id="{cid}"/></w:r>"""
 
 REPLY_MARKER_TEMPLATE = """
-Nest markers inside parent {pid}'s markers (markers must be direct children of w:p, never inside w:r):
+Nest markers inside parent {pid}'s markers (direct children of w:p, never inside w:r):
   <w:commentRangeStart w:id="{pid}"/><w:commentRangeStart w:id="{cid}"/>
   <w:r>...</w:r>
   <w:commentRangeEnd w:id="{cid}"/><w:commentRangeEnd w:id="{pid}"/>
   <w:r><w:rPr><w:rStyle w:val="CommentReference"/></w:rPr><w:commentReference w:id="{pid}"/></w:r>
   <w:r><w:rPr><w:rStyle w:val="CommentReference"/></w:rPr><w:commentReference w:id="{cid}"/></w:r>"""
 
+SMART_QUOTE_ENTITIES = {
+    "“": "&#x201C;",
+    "”": "&#x201D;",
+    "‘": "&#x2018;",
+    "’": "&#x2019;",
+}
+
 
 def _generate_hex_id() -> str:
     return f"{random.randint(0, 0x7FFFFFFE):08X}"
 
 
-SMART_QUOTE_ENTITIES = {
-    "\u201c": "&#x201C;",  
-    "\u201d": "&#x201D;",  
-    "\u2018": "&#x2018;",  
-    "\u2019": "&#x2019;",  
-}
-
-
 def _encode_smart_quotes(text: str) -> str:
     for char, entity in SMART_QUOTE_ENTITIES.items():
         text = text.replace(char, entity)
@@ -105,6 +119,19 @@ def _find_para_id(comments_path: Path, comment_id: int) -> str | None:
     return None
 
 
+def _next_comment_id(comments_path: Path) -> int:
+    if not comments_path.exists():
+        return 0
+    dom = defusedxml.minidom.parseString(comments_path.read_text(encoding="utf-8"))
+    ids = []
+    for c in dom.getElementsByTagName("w:comment"):
+        try:
+            ids.append(int(c.getAttribute("w:id")))
+        except ValueError:
+            pass
+    return (max(ids) + 1) if ids else 0
+
+
 def _get_next_rid(rels_path: Path) -> int:
     dom = defusedxml.minidom.parseString(rels_path.read_text(encoding="utf-8"))
     max_rid = 0
@@ -120,151 +147,146 @@ def _get_next_rid(rels_path: Path) -> int:
 
 def _has_relationship(rels_path: Path, target: str) -> bool:
     dom = defusedxml.minidom.parseString(rels_path.read_text(encoding="utf-8"))
-    for rel in dom.getElementsByTagName("Relationship"):
-        if rel.getAttribute("Target") == target:
-            return True
-    return False
+    return any(
+        rel.getAttribute("Target") == target
+        for rel in dom.getElementsByTagName("Relationship")
+    )
 
 
 def _has_content_type(ct_path: Path, part_name: str) -> bool:
     dom = defusedxml.minidom.parseString(ct_path.read_text(encoding="utf-8"))
-    for override in dom.getElementsByTagName("Override"):
-        if override.getAttribute("PartName") == part_name:
-            return True
-    return False
+    return any(
+        o.getAttribute("PartName") == part_name
+        for o in dom.getElementsByTagName("Override")
+    )
+
+
+_COMMENT_RELS = [
+    ("http://schemas.openxmlformats.org/officeDocument/2006/relationships/comments", "comments.xml"),
+    ("http://schemas.microsoft.com/office/2011/relationships/commentsExtended", "commentsExtended.xml"),
+    ("http://schemas.microsoft.com/office/2016/09/relationships/commentsIds", "commentsIds.xml"),
+    ("http://schemas.microsoft.com/office/2018/08/relationships/commentsExtensible", "commentsExtensible.xml"),
+]
+_COMMENT_OVERRIDES = [
+    ("/word/comments.xml", "application/vnd.openxmlformats-officedocument.wordprocessingml.comments+xml"),
+    ("/word/commentsExtended.xml", "application/vnd.openxmlformats-officedocument.wordprocessingml.commentsExtended+xml"),
+    ("/word/commentsIds.xml", "application/vnd.openxmlformats-officedocument.wordprocessingml.commentsIds+xml"),
+    ("/word/commentsExtensible.xml", "application/vnd.openxmlformats-officedocument.wordprocessingml.commentsExtensible+xml"),
+]
 
 
 def _ensure_comment_relationships(unpacked_dir: Path) -> None:
     rels_path = unpacked_dir / "word" / "_rels" / "document.xml.rels"
     if not rels_path.exists():
         return
-
-    if _has_relationship(rels_path, "comments.xml"):
-        return  
-
     dom = defusedxml.minidom.parseString(rels_path.read_text(encoding="utf-8"))
     root = dom.documentElement
+    comment_types = {rel_type for rel_type, _ in _COMMENT_RELS}
+    existing = set()
+    for rel in dom.getElementsByTagName("Relationship"):
+        if rel.getAttribute("Type") not in comment_types:
+            continue
+        part = opc_target(
+            rel.getAttribute("Target"),
+            "word/document.xml",
+            rel.getAttribute("TargetMode"),
+        )
+        if part is not None:
+            existing.add(part)
     next_rid = _get_next_rid(rels_path)
-
-    rels = [
-        (
-            "http://schemas.openxmlformats.org/officeDocument/2006/relationships/comments",
-            "comments.xml",
-        ),
-        (
-            "http://schemas.microsoft.com/office/2011/relationships/commentsExtended",
-            "commentsExtended.xml",
-        ),
-        (
-            "http://schemas.microsoft.com/office/2016/09/relationships/commentsIds",
-            "commentsIds.xml",
-        ),
-        (
-            "http://schemas.microsoft.com/office/2018/08/relationships/commentsExtensible",
-            "commentsExtensible.xml",
-        ),
-    ]
-
-    for rel_type, target in rels:
+    changed = False
+    for rel_type, target in _COMMENT_RELS:
+        if opc_target(target, "word/document.xml") in existing:
+            continue
         rel = dom.createElement("Relationship")
         rel.setAttribute("Id", f"rId{next_rid}")
         rel.setAttribute("Type", rel_type)
         rel.setAttribute("Target", target)
         root.appendChild(rel)  
         next_rid += 1
-
-    rels_path.write_bytes(dom.toxml(encoding="UTF-8"))
+        changed = True
+    if changed:
+        rels_path.write_bytes(dom.toxml(encoding="UTF-8"))
 
 
 def _ensure_comment_content_types(unpacked_dir: Path) -> None:
     ct_path = unpacked_dir / "[Content_Types].xml"
     if not ct_path.exists():
         return
-
-    if _has_content_type(ct_path, "/word/comments.xml"):
-        return  
-
     dom = defusedxml.minidom.parseString(ct_path.read_text(encoding="utf-8"))
     root = dom.documentElement
-
-    overrides = [
-        (
-            "/word/comments.xml",
-            "application/vnd.openxmlformats-officedocument.wordprocessingml.comments+xml",
-        ),
-        (
-            "/word/commentsExtended.xml",
-            "application/vnd.openxmlformats-officedocument.wordprocessingml.commentsExtended+xml",
-        ),
-        (
-            "/word/commentsIds.xml",
-            "application/vnd.openxmlformats-officedocument.wordprocessingml.commentsIds+xml",
-        ),
-        (
-            "/word/commentsExtensible.xml",
-            "application/vnd.openxmlformats-officedocument.wordprocessingml.commentsExtensible+xml",
-        ),
-    ]
-
-    for part_name, content_type in overrides:
+    existing = {
+        o.getAttribute("PartName")
+        for o in dom.getElementsByTagName("Override")
+    }
+    changed = False
+    for part_name, content_type in _COMMENT_OVERRIDES:
+        if part_name in existing:
+            continue
         override = dom.createElement("Override")
         override.setAttribute("PartName", part_name)
         override.setAttribute("ContentType", content_type)
         root.appendChild(override)  
-
-    ct_path.write_bytes(dom.toxml(encoding="UTF-8"))
+        changed = True
+    if changed:
+        ct_path.write_bytes(dom.toxml(encoding="UTF-8"))
 
 
 def add_comment(
-    unpacked_dir: str,
-    comment_id: int,
+    unpacked_dir: Path | str,
     text: str,
+    comment_id: int | None = None,
     author: str = "Claude",
     initials: str = "C",
     parent_id: int | None = None,
-) -> tuple[str, str]:
-    word = Path(unpacked_dir) / "word"
+    raw: bool = False,
+) -> tuple[int, str, str]:
+    unpacked_dir = Path(unpacked_dir)
+    if not raw:
+        text = xml_escape(text)
+    author = xml_escape(author, {'"': "&quot;"})
+    initials = xml_escape(initials, {'"': "&quot;"})
+    word = unpacked_dir / "word"
     if not word.exists():
-        return "", f"Error: {word} not found"
+        raise FileNotFoundError(f"{word} not found (not an unpacked .docx?)")
+
+    comments = word / "comments.xml"
+    if comment_id is None:
+        comment_id = _next_comment_id(comments)
+
+    parent_para = None
+    if parent_id is not None:
+        parent_para = _find_para_id(comments, parent_id) if comments.exists() else None
+        if not parent_para:
+            raise ValueError(f"parent comment {parent_id} not found")
 
     para_id, durable_id = _generate_hex_id(), _generate_hex_id()
     ts = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
 
-    comments = word / "comments.xml"
-    first_comment = not comments.exists()
-    if first_comment:
+    if not comments.exists():
         shutil.copy(TEMPLATE_DIR / "comments.xml", comments)
-        _ensure_comment_relationships(Path(unpacked_dir))
-        _ensure_comment_content_types(Path(unpacked_dir))
+    _ensure_comment_relationships(unpacked_dir)
+    _ensure_comment_content_types(unpacked_dir)
     _append_xml(
         comments,
         "w:comments",
         COMMENT_XML.format(
-            id=comment_id,
-            author=author,
-            date=ts,
-            initials=initials,
-            para_id=para_id,
-            text=text,  
+            id=comment_id, author=author, date=ts, initials=initials,
+            para_id=para_id, text=text,
         ),
     )
 
     ext = word / "commentsExtended.xml"
     if not ext.exists():
         shutil.copy(TEMPLATE_DIR / "commentsExtended.xml", ext)
-    if parent_id is not None:
-        parent_para = _find_para_id(comments, parent_id)
-        if not parent_para:
-            return "", f"Error: Parent comment {parent_id} not found"
+    if parent_para is not None:
         _append_xml(
-            ext,
-            "w15:commentsEx",
+            ext, "w15:commentsEx",
             f'<w15:commentEx w15:paraId="{para_id}" w15:paraIdParent="{parent_para}" w15:done="0"/>',
         )
     else:
         _append_xml(
-            ext,
-            "w15:commentsEx",
+            ext, "w15:commentsEx",
             f'<w15:commentEx w15:paraId="{para_id}" w15:done="0"/>',
         )
 
@@ -272,8 +294,7 @@ def add_comment(
     if not ids.exists():
         shutil.copy(TEMPLATE_DIR / "commentsIds.xml", ids)
     _append_xml(
-        ids,
-        "w16cid:commentsIds",
+        ids, "w16cid:commentsIds",
         f'<w16cid:commentId w16cid:paraId="{para_id}" w16cid:durableId="{durable_id}"/>',
     )
 
@@ -281,38 +302,67 @@ def add_comment(
     if not extensible.exists():
         shutil.copy(TEMPLATE_DIR / "commentsExtensible.xml", extensible)
     _append_xml(
-        extensible,
-        "w16cex:commentsExtensible",
+        extensible, "w16cex:commentsExtensible",
         f'<w16cex:commentExtensible w16cex:durableId="{durable_id}" w16cex:dateUtc="{ts}"/>',
     )
 
     action = "reply" if parent_id is not None else "comment"
-    return para_id, f"Added {action} {comment_id} (para_id={para_id})"
+    return comment_id, para_id, f"Added {action} id={comment_id} (paraId={para_id})"
 
 
-if __name__ == "__main__":
-    p = argparse.ArgumentParser(description="Add comments to DOCX documents")
-    p.add_argument("unpacked_dir", help="Unpacked DOCX directory")
-    p.add_argument("comment_id", type=int, help="Comment ID (must be unique)")
-    p.add_argument("text", help="Comment text")
+def main() -> None:
+    p = argparse.ArgumentParser(description="Add a comment to a DOCX (directory or .docx file).")
+    p.add_argument("input", help="Unpacked DOCX directory OR a .docx/.dotx file")
+    p.add_argument("text", help="Comment text (plain text; XML-escaped automatically)")
+    p.add_argument("--raw", action="store_true",
+                   help="Treat text as pre-escaped XML (skip automatic escaping)")
+    p.add_argument("--id", type=int, dest="comment_id",
+                   help="Comment ID (default: auto-assign as max existing + 1)")
     p.add_argument("--author", default="Claude", help="Author name")
     p.add_argument("--initials", default="C", help="Author initials")
-    p.add_argument("--parent", type=int, help="Parent comment ID (for replies)")
+    p.add_argument("--parent", type=int, help="Parent comment ID (makes this a reply)")
+    p.add_argument("-o", "--output",
+                   help="Output .docx path (only used when input is a .docx; default: overwrite input)")
     args = p.parse_args()
 
-    para_id, msg = add_comment(
-        args.unpacked_dir,
-        args.comment_id,
-        args.text,
-        args.author,
-        args.initials,
-        args.parent,
-    )
-    print(msg)
-    if "Error" in msg:
+    src = Path(args.input)
+
+    try:
+        if src.is_dir():
+            if args.output:
+                print("Warning: --output ignored for directory input", file=sys.stderr)
+            cid, _, msg = add_comment(
+                src, args.text, comment_id=args.comment_id,
+                author=args.author, initials=args.initials,
+                parent_id=args.parent, raw=args.raw,
+            )
+            print(msg)
+        elif src.is_file() and src.suffix.lower() in (".docx", ".dotx"):
+            out = Path(args.output) if args.output else src
+            with tempfile.TemporaryDirectory() as tmp:
+                tmp_path = Path(tmp)
+                with zipfile.ZipFile(src) as zf:
+                    _safe_extract(zf, tmp_path)
+                cid, _, msg = add_comment(
+                    tmp_path, args.text, comment_id=args.comment_id,
+                    author=args.author, initials=args.initials,
+                    parent_id=args.parent, raw=args.raw,
+                )
+                _rezip(tmp_path, out)
+            print(msg)
+            print(f"Wrote {out} (comment defined; add markers to word/document.xml to make it visible)")
+        else:
+            print(f"Error: {src} is neither a directory nor a .docx/.dotx file", file=sys.stderr)
+            sys.exit(1)
+    except (FileNotFoundError, ValueError, zipfile.BadZipFile, ExpatError) as e:
+        print(f"Error: {e}", file=sys.stderr)
         sys.exit(1)
-    cid = args.comment_id
+
     if args.parent is not None:
         print(REPLY_MARKER_TEMPLATE.format(pid=args.parent, cid=cid))
     else:
         print(COMMENT_MARKER_TEMPLATE.format(cid=cid))
+
+
+if __name__ == "__main__":
+    main()