diff --git a/agent-team/agent_team/confluence/client.py b/agent-team/agent_team/confluence/client.py index 2f011f2..6577461 100644 --- a/agent-team/agent_team/confluence/client.py +++ b/agent-team/agent_team/confluence/client.py @@ -43,6 +43,7 @@ import difflib import json import os import re +from collections import Counter from dataclasses import dataclass, field from typing import Any, Protocol from urllib import error as _urlerror @@ -62,6 +63,7 @@ __all__ = [ "body_diff", "count_storage_macros", "init_auth", + "storage_macro_signature", ] # Confluence storage-format macros (Mermaid diagrams, info panels, etc.) are @@ -72,6 +74,14 @@ __all__ = [ _STORAGE_MACRO_RE = re.compile( r"]*`` matches both self-closing and paired opening tags. +_STORAGE_MACRO_TAG_RE = re.compile( + r"]*)>", re.IGNORECASE +) +_AC_NAME_RE = re.compile(r'ac:name\s*=\s*"([^"]*)"', re.IGNORECASE) # Default 2LO token endpoint (overridable via CONFLUENCE_OAUTH_TOKEN_URL). DEFAULT_OAUTH_TOKEN_URL = "https://auth.atlassian.com/oauth/token" @@ -574,6 +584,34 @@ def count_storage_macros(body: str) -> int: return len(_STORAGE_MACRO_RE.findall(body)) +def storage_macro_signature(body: str) -> Counter[str]: + """Return a multiset of macro IDENTITIES in a storage-format body. + + Each ```` is keyed by its ``ac:name`` (e.g. + ``"macro:mermaid-cloud"``), unnamed macros under ``"macro:_unnamed"``, and + ```` under ``"adf-extension"``. Comparing the current + page's signature to a proposed body's lets a writer refuse a storage update + that DROPS a specific diagram macro even when an unrelated macro keeps the + raw count equal (an identity-blind count guard would miss that). Empty/None + body yields an empty ``Counter``. + """ + sig: Counter[str] = Counter() + if not body: + return sig + for kind, attrs in _STORAGE_MACRO_TAG_RE.findall(body): + if kind.lower() == "adf-extension": + sig["adf-extension"] += 1 + continue + name_match = _AC_NAME_RE.search(attrs) + key = ( + f"macro:{name_match.group(1).strip().lower()}" + if name_match + else "macro:_unnamed" + ) + sig[key] += 1 + return sig + + def _extract_storage_body(page: dict[str, Any]) -> str: """Pull ``body.storage.value`` from a v2 page object, defaulting to ``""``.""" body = page.get("body") diff --git a/agent-team/agent_team/nodes/confluence_writer.py b/agent-team/agent_team/nodes/confluence_writer.py index a9476ed..393d5fe 100644 --- a/agent-team/agent_team/nodes/confluence_writer.py +++ b/agent-team/agent_team/nodes/confluence_writer.py @@ -635,21 +635,29 @@ def _assert_macros_preserved(page_id: Any, current_body: str, new_body: Any) -> A wholesale storage-format body replacement silently destroys ```` macro extensions (Mermaid diagrams) the model-authored body does not - reproduce — the exact failure that erased every diagram on page 1540098. If - the current page carries more macros than the proposed body, refuse rather - than overwrite. Counting is delegated to the client's storage-macro counter. - """ - from agent_team.confluence.client import count_storage_macros + reproduce — the exact failure that erased every diagram on page 1540098. - current = count_storage_macros(current_body or "") - proposed = count_storage_macros(str(new_body or "")) - if current > proposed: + The check is IDENTITY-aware, not a raw count: it compares the per-macro-name + multiset (``storage_macro_signature``) of the current page against the + proposed body and refuses if ANY macro identity loses occurrences. So a body + that drops the real Mermaid macro but adds an unrelated macro (keeping the + raw count equal) is still refused. + """ + from agent_team.confluence.client import storage_macro_signature + + current = storage_macro_signature(current_body or "") + proposed = storage_macro_signature(str(new_body or "")) + dropped = { + key: count - proposed.get(key, 0) + for key, count in current.items() + if count > proposed.get(key, 0) + } + if dropped: raise ConfluenceWriteError( - f"refusing storage write to page {page_id}: the current page carries " - f"{current} macro(s) but the proposed body has {proposed} — a " - "wholesale storage update would DROP diagram/extension macros (e.g. " - "Mermaid). Edit via the ADF path (get_page_adf/update_page_adf) or " - "preserve the existing macros in body_storage." + f"refusing storage write to page {page_id}: a wholesale storage update " + f"would DROP diagram/extension macros {dropped} (e.g. Mermaid). Edit via " + "the ADF path (get_page_adf/update_page_adf) or preserve the existing " + "macros in body_storage." ) diff --git a/agent-team/tests/test_confluence_client.py b/agent-team/tests/test_confluence_client.py index 0a96de8..67563d6 100644 --- a/agent-team/tests/test_confluence_client.py +++ b/agent-team/tests/test_confluence_client.py @@ -30,6 +30,7 @@ from agent_team.confluence.client import ( body_diff, count_storage_macros, init_auth, + storage_macro_signature, ) # --------------------------------------------------------------------------- # @@ -497,6 +498,21 @@ def test_count_storage_macros_counts_structured_and_adf_extensions() -> None: assert count_storage_macros("

no macros here

") == 0 +def test_storage_macro_signature_keys_by_identity() -> None: + body = ( + '' + '' + "x" # unnamed + "y" + ) + sig = storage_macro_signature(body) + assert sig["macro:mermaid-cloud"] == 1 + assert sig["macro:info"] == 1 + assert sig["macro:_unnamed"] == 1 + assert sig["adf-extension"] == 1 + assert storage_macro_signature("") == {} + + def test_page_has_macros_true_when_body_carries_macro() -> None: page = _storage_page("1540098", 7, '') http = FakeHttp(get=(200, page)) diff --git a/agent-team/tests/test_confluence_writer.py b/agent-team/tests/test_confluence_writer.py index 9243b9b..f88b302 100644 --- a/agent-team/tests/test_confluence_writer.py +++ b/agent-team/tests/test_confluence_writer.py @@ -577,6 +577,25 @@ def test_conf_write_storage_refuses_to_drop_macros() -> None: assert client.update_calls == [] # nothing was written +def test_conf_write_storage_refuses_macro_swap_at_equal_count() -> None: + # Identity-aware guard: the proposed body DROPS the Mermaid macro but ADDS an + # unrelated macro, keeping the raw count equal (1 -> 1). A count-only guard + # would wave this through; the signature guard must still refuse. + client = _MacroBodyClient(current_macros=1) # current: one mermaid-cloud macro + draft = dict(_VALID_DRAFT) + draft["body_storage"] = ( + '' + "

note

" + ) + with pytest.raises(ConfluenceWriteError, match="would DROP diagram/extension"): + conf_write_node( + _state(confluence_draft=draft), + config={"confluence_apply": True}, + client=client, + ) + assert client.update_calls == [] + + def test_conf_write_storage_allows_when_macros_preserved() -> None: # When the new body reproduces the macro count, the write proceeds. client = _MacroBodyClient(current_macros=1)