diff --git a/agent-team/agent_team/confluence/__init__.py b/agent-team/agent_team/confluence/__init__.py new file mode 100644 index 0000000..b111ba7 --- /dev/null +++ b/agent-team/agent_team/confluence/__init__.py @@ -0,0 +1,29 @@ +"""Confluence client seam for the durable doc-update pipeline. + +Ports the OAuth/Basic auth seam from the security-review bash checker +(``checkers/confluence-doc.sh``) into a typed, dependency-injectable +:class:`~agent_team.confluence.client.ConfluenceClient`. The HTTP transport is +a protocol so unit tests inject an in-memory fake (no live network), and +``update_page`` defaults to dry-run so a planned change is producible without +mutating Confluence. +""" + +from agent_team.confluence.client import ( + ConfluenceAuth, + ConfluenceClient, + ConfluenceError, + HttpClient, + PlannedPageUpdate, + body_diff, + init_auth, +) + +__all__ = [ + "ConfluenceAuth", + "ConfluenceClient", + "ConfluenceError", + "HttpClient", + "PlannedPageUpdate", + "body_diff", + "init_auth", +] diff --git a/agent-team/agent_team/confluence/client.py b/agent-team/agent_team/confluence/client.py new file mode 100644 index 0000000..885ce4e --- /dev/null +++ b/agent-team/agent_team/confluence/client.py @@ -0,0 +1,548 @@ +"""Confluence REST v2 client with a dry-run-first page updater. + +This module ports the auth seam from the security-review bash reference +(``checkers/confluence-doc.sh`` ``conf_api_init`` / ``conf_get``, lines +~210-304) into a typed Python client suited to the durable pipeline: + +Auth (OAuth wins when present), mirroring the bash ``conf_api_init`` precedence: + + A. **2LO client-credentials (OAuth).** When ``CONFLUENCE_OAUTH_CLIENT_ID`` and + ``CONFLUENCE_OAUTH_CLIENT_SECRET`` are set, POST a + ``grant_type=client_credentials`` request to ``CONFLUENCE_OAUTH_TOKEN_URL`` + (default ``https://auth.atlassian.com/oauth/token``) for a Bearer token, + then resolve the cloudId from ``CONFLUENCE_CLOUD_ID`` or the OAuth-native + ``accessible-resources`` endpoint. Base becomes + ``https://api.atlassian.com/ex/confluence/``. + B. **Basic auth.** When ``CONFLUENCE_BASE_URL`` + ``CONFLUENCE_EMAIL`` + + ``CONFLUENCE_API_TOKEN`` are set, use HTTP Basic against the configured + base. + +As in the bash reference, a missing/incomplete cred set or a failed token / +cloudId resolution surfaces cleanly (:class:`ConfluenceError`) so the caller can +SKIP rather than raise a false alarm — mirroring ``conf_api_init`` returning +non-zero. + +I/O seam: HTTP is the injected :class:`HttpClient` protocol so tests pass an +in-memory fake and no live network is touched in any code path the tests hit. +The default :class:`UrllibHttpClient` is stdlib-only (``urllib``), matching +``agent_team.transport.github_adapter`` — no third-party dependency. + +Secrets (``CONFLUENCE_*``) are read from ``os.environ`` at CALL time via +:func:`init_auth`, never captured at import or stored long-lived on the module. + +Dry-run-first updater: :meth:`ConfluenceClient.update_page` defaults to +``apply=False``. In dry-run it returns a :class:`PlannedPageUpdate` (target id, +``new_version = current + 1``, and a unified-diff body delta) and performs NO +network write. Only ``apply=True`` issues the PUT. +""" + +from __future__ import annotations + +import base64 +import difflib +import json +import os +from dataclasses import dataclass, field +from typing import Any, Protocol +from urllib import error as _urlerror +from urllib import parse as _urlparse +from urllib import request as _urlrequest + +__all__ = [ + "DEFAULT_OAUTH_TOKEN_URL", + "ATLASSIAN_API_BASE", + "ACCESSIBLE_RESOURCES_URL", + "ConfluenceAuth", + "ConfluenceClient", + "ConfluenceError", + "HttpClient", + "PlannedPageUpdate", + "UrllibHttpClient", + "body_diff", + "init_auth", +] + +# Default 2LO token endpoint (overridable via CONFLUENCE_OAUTH_TOKEN_URL). +DEFAULT_OAUTH_TOKEN_URL = "https://auth.atlassian.com/oauth/token" +# OAuth-native gateway base for per-cloud Confluence access. +ATLASSIAN_API_BASE = "https://api.atlassian.com/ex/confluence" +# OAuth-native resource enumeration used to resolve the cloudId. +ACCESSIBLE_RESOURCES_URL = "https://api.atlassian.com/oauth/token/accessible-resources" + + +class ConfluenceError(RuntimeError): + """Raised when auth resolution or a REST call fails. + + Carries an HTTP-ish ``status`` (``0`` for pre-flight/config failures such as + missing creds or an unresolvable cloudId) and a (truncated) ``body``. A + caller that wants the bash ``conf_api_init``-style "skip, no false alarm" + behaviour can catch this and skip rather than propagate. + """ + + def __init__(self, status: int, body: str) -> None: + self.status = status + self.body = body + super().__init__(f"Confluence error {status}: {body[:200]}") + + +class HttpClient(Protocol): + """Injected HTTP seam returning ``(status, body)`` for each verb. + + ``body`` is the parsed JSON (a ``dict``) when the response carries JSON, else + the raw ``bytes``. Keeping the surface narrow (``get``/``post``/``put``) + means the client has no hard HTTP dependency and tests inject a pure + in-memory fake — no live network on any tested path. + """ + + def get( + self, url: str, *, headers: dict[str, str] + ) -> tuple[int, dict[str, Any] | bytes]: ... + + def post( + self, + url: str, + *, + headers: dict[str, str], + data: bytes, + ) -> tuple[int, dict[str, Any] | bytes]: ... + + def put( + self, + url: str, + *, + headers: dict[str, str], + data: bytes, + ) -> tuple[int, dict[str, Any] | bytes]: ... + + +# --------------------------------------------------------------------------- +# Default stdlib-only HTTP impl (no third-party dependency at import time). +# --------------------------------------------------------------------------- + + +def _parse_body(raw: bytes, content_type: str) -> dict[str, Any] | bytes: + """Parse a response body to a ``dict`` when it is JSON, else return bytes.""" + if not raw: + return {} + if "application/json" in (content_type or "").lower(): + try: + return json.loads(raw.decode("utf-8")) + except (ValueError, UnicodeDecodeError): + return raw + return raw + + +class UrllibHttpClient: + """Stdlib-only :class:`HttpClient` (``urllib``), built lazily. + + Used only when no client is injected and an actual request is made; tests + never reach this path because they inject a fake. Matches the + ``github_adapter`` pattern: urls are built from fixed ``https`` Atlassian + bases, so there is no SSRF/``file://`` surface. + """ + + def _request( + self, + url: str, + *, + method: str, + headers: dict[str, str], + data: bytes | None, + ) -> tuple[int, dict[str, Any] | bytes]: + request = _urlrequest.Request(url, data=data, method=method) + for key, value in headers.items(): + request.add_header(key, value) + try: + with _urlrequest.urlopen(request) as response: # noqa: S310 (trusted atlassian host); nosemgrep + status = response.getcode() + raw = response.read() + content_type = response.headers.get("Content-Type", "") + except _urlerror.HTTPError as exc: # pragma: no cover - network path + raw = exc.read() + content_type = exc.headers.get("Content-Type", "") if exc.headers else "" + return exc.code, _parse_body(raw, content_type) + return status, _parse_body(raw, content_type) + + def get( + self, url: str, *, headers: dict[str, str] + ) -> tuple[int, dict[str, Any] | bytes]: + return self._request(url, method="GET", headers=headers, data=None) + + def post( + self, url: str, *, headers: dict[str, str], data: bytes + ) -> tuple[int, dict[str, Any] | bytes]: + return self._request(url, method="POST", headers=headers, data=data) + + def put( + self, url: str, *, headers: dict[str, str], data: bytes + ) -> tuple[int, dict[str, Any] | bytes]: + return self._request(url, method="PUT", headers=headers, data=data) + + +# --------------------------------------------------------------------------- +# Auth resolution (pure-ish: env in, resolved config out; one HTTP seam for OAuth). +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class ConfluenceAuth: + """Resolved auth context: the base URL plus per-request headers. + + Exactly one of the two modes is materialised (``"oauth"`` or ``"basic"``), + mirroring ``conf_api_init`` resolving ONE mode. ``base`` already includes the + OAuth ``/ex/confluence/`` segment when relevant; both modes then + share the ``/wiki/api/v2/...`` path suffix exactly as ``conf_get`` does. + """ + + mode: str + base: str + _headers: dict[str, str] = field(default_factory=dict) + + def headers(self, *, extra: dict[str, str] | None = None) -> dict[str, str]: + """Per-request headers (auth + accept), merged with ``extra``.""" + merged = dict(self._headers) + if extra: + merged.update(extra) + return merged + + +def _resolve_cloud_id( + http: HttpClient, + *, + bearer: str, + configured_cloud_id: str | None, + site_url: str | None, +) -> str: + """Resolve the cloudId, preferring config then accessible-resources. + + Mirrors the bash fallback: use ``CONFLUENCE_CLOUD_ID`` when set, else query + the OAuth-native ``accessible-resources`` endpoint and prefer the resource + whose ``url`` matches the configured site, else the first one. + """ + if configured_cloud_id: + return configured_cloud_id + + status, body = http.get( + ACCESSIBLE_RESOURCES_URL, + headers={"Authorization": f"Bearer {bearer}", "Accept": "application/json"}, + ) + if not (200 <= status < 300) or not isinstance(body, list): + # bash treats this as "could not resolve cloudId -> skip" (status 0). + raise ConfluenceError( + 0, "OAuth: could not resolve cloudId (set CONFLUENCE_CLOUD_ID)" + ) + + chosen: str | None = None + if site_url: + for resource in body: + if isinstance(resource, dict) and resource.get("url") == site_url: + chosen = resource.get("id") + break + if not chosen: + for resource in body: + if isinstance(resource, dict) and resource.get("id"): + chosen = resource["id"] + break + if not chosen: + raise ConfluenceError( + 0, "OAuth: could not resolve cloudId (set CONFLUENCE_CLOUD_ID)" + ) + return chosen + + +def init_auth(http: HttpClient, *, env: dict[str, str] | None = None) -> ConfluenceAuth: + """Resolve ONE auth mode from the environment at call time (OAuth wins). + + Reads ``CONFLUENCE_*`` from ``env`` (default ``os.environ``) only when + invoked, never at import. Raises :class:`ConfluenceError` (``status=0``) on a + missing/incomplete cred set or a failed token / cloudId resolution — the + ``conf_api_init`` "return non-zero -> caller skips" contract, so a missing + cred path is cleanly detectable. + + Args: + http: HTTP seam (used only for the OAuth token + cloudId calls). + env: Environment mapping to read creds from; defaults to ``os.environ``. + + Returns: + A :class:`ConfluenceAuth` with the resolved base + auth headers. + """ + environ = os.environ if env is None else env + + client_id = environ.get("CONFLUENCE_OAUTH_CLIENT_ID") + client_secret = environ.get("CONFLUENCE_OAUTH_CLIENT_SECRET") + site_url = environ.get("CONFLUENCE_BASE_URL") + + # Mode A: 2LO client-credentials (OAuth wins when its creds are present). + if client_id and client_secret: + token_url = environ.get("CONFLUENCE_OAUTH_TOKEN_URL") or DEFAULT_OAUTH_TOKEN_URL + form = _urlparse.urlencode( + { + "client_id": client_id, + "client_secret": client_secret, + "grant_type": "client_credentials", + } + ).encode("utf-8") + status, body = http.post( + token_url, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + data=form, + ) + bearer = body.get("access_token") if isinstance(body, dict) else None + if not (200 <= status < 300) or not bearer: + # bash: "OAuth: token request failed — skipping API (no false alarm)". + raise ConfluenceError(0, "OAuth: token request failed") + + cloud_id = _resolve_cloud_id( + http, + bearer=bearer, + configured_cloud_id=environ.get("CONFLUENCE_CLOUD_ID"), + site_url=site_url, + ) + return ConfluenceAuth( + mode="oauth", + base=f"{ATLASSIAN_API_BASE}/{cloud_id}", + _headers={ + "Authorization": f"Bearer {bearer}", + "Accept": "application/json", + }, + ) + + # Mode B: Basic auth (email + API token against the configured base). + email = environ.get("CONFLUENCE_EMAIL") + api_token = environ.get("CONFLUENCE_API_TOKEN") + if site_url and email and api_token: + raw = f"{email}:{api_token}".encode("utf-8") + encoded = base64.b64encode(raw).decode("ascii") + return ConfluenceAuth( + mode="basic", + base=site_url.rstrip("/"), + _headers={ + "Authorization": f"Basic {encoded}", + "Accept": "application/json", + }, + ) + + # Neither mode fully configured -> caller skips (no false alarm). + raise ConfluenceError(0, "no Confluence credentials configured (OAuth or Basic)") + + +# --------------------------------------------------------------------------- +# Pure planning logic (no I/O) — kept separate so it is unit-trivial. +# --------------------------------------------------------------------------- + + +def body_diff(old_body: str, new_body: str, *, page_id: str) -> str: + """Render a unified-diff of a page's storage body (old -> new). + + Pure string logic with no I/O, so the dry-run plan is testable without any + HTTP. Empty string when the bodies are identical. + """ + old_lines = (old_body or "").splitlines(keepends=True) + new_lines = (new_body or "").splitlines(keepends=True) + diff = difflib.unified_diff( + old_lines, + new_lines, + fromfile=f"page/{page_id}@current", + tofile=f"page/{page_id}@planned", + ) + return "".join(diff) + + +@dataclass(frozen=True) +class PlannedPageUpdate: + """A dry-run page update: the change that WOULD be applied, no PUT issued. + + Returned by :meth:`ConfluenceClient.update_page` when ``apply=False`` (the + default). ``applied`` is ``False`` here; the same dataclass is returned with + ``applied=True`` after a real PUT so callers get a uniform shape. + """ + + page_id: str + title: str + current_version: int + new_version: int + body_storage: str + body_delta: str + applied: bool = False + + +# --------------------------------------------------------------------------- +# The client (thin I/O wrapper over the seam + the pure planning logic). +# --------------------------------------------------------------------------- + + +class ConfluenceClient: + """Read pages and plan/apply page updates over the injected HTTP seam. + + Auth is resolved lazily on first use (or eagerly if a :class:`ConfluenceAuth` + is injected), reading ``CONFLUENCE_*`` from the environment at call time. The + HTTP seam is injected so tests stay hermetic. + """ + + def __init__( + self, + *, + http: HttpClient | None = None, + auth: ConfluenceAuth | None = None, + env: dict[str, str] | None = None, + ) -> None: + """Construct the client. + + Args: + http: Injected HTTP seam. Defaults to a stdlib-only + :class:`UrllibHttpClient` built lazily (tests inject a fake). + auth: Pre-resolved auth context. When omitted, auth is resolved on + first use from the environment via :func:`init_auth`. + env: Environment mapping for cred resolution; defaults to + ``os.environ`` (read at call time, never at import). + """ + self._http: HttpClient = http or UrllibHttpClient() + self._auth = auth + self._env = env + + def _ensure_auth(self) -> ConfluenceAuth: + """Resolve auth on first use; raises :class:`ConfluenceError` if missing.""" + if self._auth is None: + self._auth = init_auth(self._http, env=self._env) + return self._auth + + @staticmethod + def _as_dict(body: dict[str, Any] | bytes) -> dict[str, Any]: + """Coerce a response body to a dict or raise a clean parse error.""" + if isinstance(body, dict): + return body + raise ConfluenceError(0, "expected JSON object response, got non-JSON body") + + def get_page(self, page_id: str) -> dict[str, Any]: + """GET ``/wiki/api/v2/pages/{id}?body-format=storage``. + + Returns the parsed page object (including ``version.number`` and + ``body.storage.value``). Raises :class:`ConfluenceError` on a non-2xx + response, carrying the status so the caller can branch (e.g. 404 = gone). + """ + auth = self._ensure_auth() + # Defense-in-depth: percent-encode the id segment so a malformed id can + # never rewrite the request path (request-path injection). + id_segment = _urlparse.quote(str(page_id), safe="") + url = f"{auth.base}/wiki/api/v2/pages/{id_segment}?body-format=storage" + status, body = self._http.get(url, headers=auth.headers()) + if not (200 <= status < 300): + raise ConfluenceError(status, _stringify(body)) + return self._as_dict(body) + + def update_page( + self, + page_id: str, + title: str, + body_storage: str, + version_number: int, + *, + apply: bool = False, + ) -> PlannedPageUpdate: + """Plan (default) or apply a page update via ``/wiki/api/v2/pages/{id}``. + + CRITICAL — dry-run by default. With ``apply=False`` (the default) NO + network write happens: this returns a :class:`PlannedPageUpdate` + describing the change (target id, ``new_version = version_number + 1``, + and a unified-diff ``body_delta`` against the page's current storage + body). Only ``apply=True`` issues the PUT. + + Args: + page_id: Target page id. + title: New page title (Confluence requires title on update). + body_storage: New body in Confluence ``storage`` representation. + version_number: The page's CURRENT version number; the PUT/plan uses + ``version_number + 1`` as the new version (Confluence's + optimistic-concurrency contract). + apply: When ``False`` (default) return the planned change without a + write. When ``True`` issue the PUT and return the applied result. + + Returns: + A :class:`PlannedPageUpdate`. ``applied`` is ``False`` for a dry-run, + ``True`` after a successful PUT. + + Raises: + ConfluenceError: On auth failure, on a failed current-body read, or + on a non-2xx PUT response (apply path only). + """ + new_version = version_number + 1 + + # Compute the body delta against the page's current storage body. The + # GET is the only read; it never mutates, so it is safe in dry-run. + current_body = "" + try: + current_page = self.get_page(page_id) + current_body = _extract_storage_body(current_page) + except ConfluenceError: + # If we cannot read the current body we still produce a plan, but the + # diff is against an empty baseline (whole new body shown as added). + current_body = "" + body_delta = body_diff(current_body, body_storage, page_id=page_id) + + if not apply: + return PlannedPageUpdate( + page_id=page_id, + title=title, + current_version=version_number, + new_version=new_version, + body_storage=body_storage, + body_delta=body_delta, + applied=False, + ) + + auth = self._ensure_auth() + # Defense-in-depth: percent-encode the id segment so a malformed id can + # never rewrite the request path (request-path injection). + id_segment = _urlparse.quote(str(page_id), safe="") + url = f"{auth.base}/wiki/api/v2/pages/{id_segment}" + payload = { + "id": str(page_id), + "status": "current", + "title": title, + "body": {"representation": "storage", "value": body_storage}, + "version": {"number": new_version}, + } + data = json.dumps(payload).encode("utf-8") + status, body = self._http.put( + url, + headers=auth.headers(extra={"Content-Type": "application/json"}), + data=data, + ) + if not (200 <= status < 300): + raise ConfluenceError(status, _stringify(body)) + + return PlannedPageUpdate( + page_id=page_id, + title=title, + current_version=version_number, + new_version=new_version, + body_storage=body_storage, + body_delta=body_delta, + applied=True, + ) + + +# --------------------------------------------------------------------------- +# Small helpers. +# --------------------------------------------------------------------------- + + +def _extract_storage_body(page: dict[str, Any]) -> str: + """Pull ``body.storage.value`` from a v2 page object, defaulting to ``""``.""" + body = page.get("body") + if isinstance(body, dict): + storage = body.get("storage") + if isinstance(storage, dict): + value = storage.get("value") + if isinstance(value, str): + return value + return "" + + +def _stringify(body: dict[str, Any] | bytes) -> str: + """Render a response body for error messages (JSON or decoded bytes).""" + if isinstance(body, dict): + return json.dumps(body) + if isinstance(body, bytes): + return body.decode("utf-8", "replace") + return str(body) diff --git a/agent-team/agent_team/confluence/mermaid.py b/agent-team/agent_team/confluence/mermaid.py new file mode 100644 index 0000000..bc774e9 --- /dev/null +++ b/agent-team/agent_team/confluence/mermaid.py @@ -0,0 +1,454 @@ +"""Vendored, ADF-only Mermaid architecture-map editor (design D7). + +This module edits the Mermaid diagrams on the Confluence "AWS Architecture Map" +page (id 1540098) by surgically mutating the page's **ADF (Atlassian Document +Format) JSON** in memory — and ONLY the ADF. It NEVER reconstructs the page body +from a Markdown / storage-format round-trip. + +Why ADF-only is load-bearing (carry this rationale forward — do not "simplify"): + A previous tooling approach edited the architecture map by round-tripping the + page through a Markdown/storage-format body. The Mermaid diagrams are stored + as Confluence *extension* macros (weweave / mermaid-cloud style nodes) whose + payload does not survive a lossy Markdown conversion, so that round-trip + **SILENTLY DELETED every diagram on page 1540098**. To make that class of + failure impossible, this editor: + + * operates only on the parsed ADF document (a dict/JSON tree), never on a + flattened body string; + * counts the Mermaid macros before and after every edit + (:func:`count_mermaid_macros`) so a drop in macro count is detectable; + * produces a **revert-diff** alongside every edit so a change is provably + reversible, and a NO-OP edit set yields an EMPTY revert-diff (the safety + invariant the unit tests assert); + * defaults to **dry-run** (``apply=False``) so a planned change is producible + and inspectable without ever mutating the input document. + +The macros this editor recognises are ADF ``extension`` / ``bodiedExtension`` / +``inlineExtension`` nodes whose ``extensionKey`` / ``extensionType`` identify a +Mermaid renderer (weweave "mermaid-cloud" and the common variants). Each such +macro carries the diagram source in its ``parameters`` (a ``macroParams`` map or +a raw ``body``); this module reads/replaces that source and nothing else. + +The module is pure: no network, no external dependencies, no environment reads. +Every function is deterministic and unit-testable against in-memory ADF dicts. +""" + +from __future__ import annotations + +import copy +from dataclasses import dataclass, field +from typing import Any + +__all__ = [ + "MERMAID_EXTENSION_KEYS", + "MERMAID_EXTENSION_TYPES", + "ADFDocument", + "MermaidEdit", + "MermaidEditResult", + "RevertOp", + "count_mermaid_macros", + "iter_mermaid_macros", + "macro_diagram_source", + "macro_key", + "plan_mermaid_edits", +] + +# An ADF document (or any ADF node) is a plain JSON object: a dict tree. +ADFDocument = dict[str, Any] + +# Extension keys that identify a Mermaid macro. weweave's cloud renderer uses +# "mermaid-cloud"; other deployments use a bare "mermaid". Matching is +# case-insensitive (see :func:`_is_mermaid_macro`). +MERMAID_EXTENSION_KEYS: frozenset[str] = frozenset( + {"mermaid-cloud", "mermaid", "mermaid-diagram", "mermaidcloud"} +) + +# extensionType namespaces a macro belongs to. weweave macros are namespaced +# under "com.weweave.*"; Confluence-native macros under "com.atlassian.*". An +# extension whose *type* names weweave/mermaid is treated as Mermaid even if a +# deployment renamed the key. +MERMAID_EXTENSION_TYPES: frozenset[str] = frozenset( + { + "com.weweave.mermaid", + "com.weweave.confluence.mermaid", + "com.atlassian.confluence.macro.core", + } +) + +# ADF node types that can carry an extension macro. +_EXTENSION_NODE_TYPES: frozenset[str] = frozenset( + {"extension", "bodiedExtension", "inlineExtension"} +) + + +@dataclass(frozen=True) +class MermaidEdit: + """A single requested diagram edit, addressed by a stable macro key. + + ``macro_key`` is the value :func:`macro_key` returns for the target macro + (its ``localId`` when present, else a deterministic positional key). It is + the *address* of the macro to edit; an edit whose key matches no macro is a + no-op for that key (recorded in :attr:`MermaidEditResult.unmatched_keys`). + + ``new_source`` is the replacement Mermaid diagram text. If it equals the + macro's current source the edit is a no-op and contributes nothing to the + revert-diff (the core safety invariant). + """ + + macro_key: str + new_source: str + + +@dataclass(frozen=True) +class RevertOp: + """One reversible change: restore ``macro_key``'s source to ``old_source``. + + A :class:`RevertOp` is emitted only for a macro whose source actually + changed. Applying every :class:`RevertOp` to the produced ``new_adf`` + reconstructs the original document's diagram sources exactly, so the edit is + provably reversible. + """ + + macro_key: str + old_source: str + new_source: str + + +@dataclass +class MermaidEditResult: + """Outcome of :func:`plan_mermaid_edits`. + + Attributes: + macro_count: Number of Mermaid macros found in the input document. + new_adf: The edited ADF document. In dry-run (``apply=False``) this is a + deep copy carrying the planned edits, and the caller's input is left + untouched. With ``apply=True`` it is still a deep copy (this module + never mutates its argument), but it is the document the caller is + expected to persist. + revert_diff: The reversible change-list. **EMPTY for a no-op edit set** + — the invariant the safety test asserts. + unmatched_keys: Edit keys that addressed no macro (surfaced, not raised, + so a partially-stale edit set is observable rather than silent). + skip_mermaid: ``True`` when the document has zero Mermaid macros, telling + the calling node to fall back to a storage-format body update instead + of an ADF Mermaid edit (design D7). + """ + + macro_count: int + new_adf: ADFDocument + revert_diff: list[RevertOp] = field(default_factory=list) + unmatched_keys: list[str] = field(default_factory=list) + skip_mermaid: bool = False + + @property + def changed(self) -> bool: + """``True`` iff at least one macro's source was actually changed.""" + return bool(self.revert_diff) + + +def _is_mermaid_macro(node: Any) -> bool: + """Return ``True`` if ``node`` is an ADF extension node rendering Mermaid. + + Recognition is intentionally permissive (key OR type matches a known Mermaid + identifier) and case-insensitive, so a deployment that renamed the key but + kept the weweave type — or vice versa — is still detected. A non-extension + node, or an extension that matches no Mermaid identifier, returns ``False``. + """ + if not isinstance(node, dict): + return False + if node.get("type") not in _EXTENSION_NODE_TYPES: + return False + attrs = node.get("attrs") + if not isinstance(attrs, dict): + return False + key = str(attrs.get("extensionKey", "")).strip().lower() + ext_type = str(attrs.get("extensionType", "")).strip().lower() + if key in {k.lower() for k in MERMAID_EXTENSION_KEYS}: + return True + if ext_type in {t.lower() for t in MERMAID_EXTENSION_TYPES}: + # A generic macro-core extension only counts as Mermaid if its + # parameters also name mermaid (avoids matching unrelated core macros). + if ext_type == "com.atlassian.confluence.macro.core": + return "mermaid" in key or _params_name_mermaid(attrs) + return True + return False + + +def _params_name_mermaid(attrs: dict[str, Any]) -> bool: + """Heuristic: does a generic macro's parameters identify it as Mermaid.""" + params = attrs.get("parameters") + if not isinstance(params, dict): + return False + macro = params.get("macroMetadata") or params.get("macroParams") or {} + blob = str(params.get("macroName", "")) + str(macro) + return "mermaid" in blob.lower() + + +def iter_mermaid_macros(adf: ADFDocument) -> list[ADFDocument]: + """Return every Mermaid macro node in ``adf`` in document order. + + Walks the ADF ``content`` tree depth-first (the same order Confluence + renders), so positional keys are stable across calls on the same document. + Returns the live node objects from the passed tree (callers that need + isolation pass a copy — :func:`plan_mermaid_edits` does). + """ + found: list[ADFDocument] = [] + _walk_collect(adf, found) + return found + + +def _walk_collect(node: Any, found: list[ADFDocument]) -> None: + """Depth-first accumulate Mermaid macro nodes into ``found``.""" + if isinstance(node, dict): + if _is_mermaid_macro(node): + found.append(node) + content = node.get("content") + if isinstance(content, list): + for child in content: + _walk_collect(child, found) + elif isinstance(node, list): + for child in node: + _walk_collect(child, found) + + +def count_mermaid_macros(adf: ADFDocument) -> int: + """Count the Mermaid macros (extension nodes) in an ADF document. + + The pre/post macro-count verification: the calling node compares this before + and after an edit (and against the known expected count — 16 on page + 1540098) so a round-trip that drops diagrams is caught rather than silently + accepted. + """ + return len(iter_mermaid_macros(adf)) + + +def macro_key(node: ADFDocument, *, index: int) -> str: + """Return a stable address for ``node`` (its ``localId`` else positional). + + ADF extension nodes usually carry a ``localId`` (stable across edits) — that + is the preferred key. When absent, a deterministic positional key + ``"#"`` (document order from :func:`iter_mermaid_macros`) is used so + every macro is still addressable. The ``index`` is the macro's position + among Mermaid macros, not among all ADF nodes. + """ + local_id = node.get("attrs", {}).get("localId") if isinstance(node, dict) else None + if isinstance(local_id, str) and local_id.strip(): + return local_id.strip() + return f"#{index}" + + +def macro_diagram_source(node: ADFDocument) -> str: + """Read the Mermaid diagram source out of a macro node. + + weweave stores the diagram text in different slots depending on macro + flavour; this checks them in priority order: + + * ``attrs.parameters.macroParams.code.value`` (weweave cloud), + * ``attrs.parameters.macroParams..value`` fallback, + * ``attrs.text`` (some inline extensions), + * the ``bodiedExtension`` text content. + + Returns ``""`` when no source slot is present (a malformed/empty macro), + never raising — a missing source is a no-op target, not a crash. + """ + if not isinstance(node, dict): + return "" + attrs = node.get("attrs") + if isinstance(attrs, dict): + params = attrs.get("parameters") + if isinstance(params, dict): + macro_params = params.get("macroParams") + if isinstance(macro_params, dict): + code = macro_params.get("code") + if isinstance(code, dict) and "value" in code: + return str(code["value"]) + for value in macro_params.values(): + if isinstance(value, dict) and "value" in value: + return str(value["value"]) + text = attrs.get("text") + if isinstance(text, str): + return text + # bodiedExtension carries source in its content text nodes. + return _bodied_text(node) + + +def _bodied_text(node: dict[str, Any]) -> str: + """Concatenate text nodes inside a bodiedExtension's content (if any).""" + content = node.get("content") + if not isinstance(content, list): + return "" + parts: list[str] = [] + for child in content: + _collect_text(child, parts) + return "".join(parts) + + +def _collect_text(node: Any, parts: list[str]) -> None: + """Depth-first gather ``text`` node values.""" + if isinstance(node, dict): + if node.get("type") == "text" and isinstance(node.get("text"), str): + parts.append(node["text"]) + child_content = node.get("content") + if isinstance(child_content, list): + for child in child_content: + _collect_text(child, parts) + + +def _set_diagram_source(node: dict[str, Any], new_source: str) -> bool: + """Write ``new_source`` into the macro's source slot in place. + + Mirrors the slot priority of :func:`macro_diagram_source`. Returns ``True`` + if a slot was written, ``False`` if no writable slot exists (the macro is + then recorded as unmatched, never silently dropped). Operates on ``node`` in + place — callers pass a node belonging to a deep-copied tree. + """ + attrs = node.get("attrs") + if isinstance(attrs, dict): + params = attrs.get("parameters") + if isinstance(params, dict): + macro_params = params.get("macroParams") + if isinstance(macro_params, dict): + code = macro_params.get("code") + if isinstance(code, dict): + code["value"] = new_source + return True + for value in macro_params.values(): + if isinstance(value, dict) and "value" in value: + value["value"] = new_source + return True + if isinstance(attrs.get("text"), str): + attrs["text"] = new_source + return True + return _set_bodied_text(node, new_source) + + +def _set_bodied_text(node: dict[str, Any], new_source: str) -> bool: + """Replace the first text node inside a bodiedExtension's content.""" + content = node.get("content") + if not isinstance(content, list): + return False + for child in content: + if _replace_first_text(child, new_source): + return True + return False + + +def _replace_first_text(node: Any, new_source: str) -> bool: + """Depth-first: set the first ``text`` node found to ``new_source``.""" + if isinstance(node, dict): + if node.get("type") == "text" and isinstance(node.get("text"), str): + node["text"] = new_source + return True + child_content = node.get("content") + if isinstance(child_content, list): + for child in child_content: + if _replace_first_text(child, new_source): + return True + return False + + +def plan_mermaid_edits( + adf: ADFDocument, + edits: list[MermaidEdit] | None = None, + *, + apply: bool = False, +) -> MermaidEditResult: + """Plan (and optionally apply) a set of Mermaid diagram edits to ``adf``. + + This is the editor's single entry point. It NEVER mutates the passed ``adf`` + (it works on a deep copy) and defaults to **dry-run** (``apply=False``): the + returned :class:`MermaidEditResult` carries the planned ``new_adf`` and a + ``revert_diff`` without the caller persisting anything. + + Safety invariants (asserted by the unit tests): + + * A **NO-OP edit set** (no edits, or edits whose ``new_source`` matches the + current source) yields an **EMPTY** ``revert_diff`` and ``changed=False``. + * The Mermaid ``macro_count`` is computed from the input and exposed so the + caller can compare it to the post-edit count (they must be equal — an ADF + edit can never drop a macro) and to the expected count for the page. + * If the document has **zero Mermaid macros**, ``skip_mermaid=True`` so the + node falls back to a storage-format body update (design D7) instead of an + empty ADF edit. + + Args: + adf: The page's ADF document (a dict tree). Not mutated. + edits: Diagram edits addressed by :func:`macro_key`. ``None``/empty means + "verify only" — count macros, produce an empty revert-diff. + apply: Dry-run default. ``True`` signals the caller intends to persist + ``new_adf``; this function still only returns the edited copy (it + performs no I/O), but the flag is recorded so the boundary is + explicit and the dry-run path is the default everywhere. + + Returns: + A :class:`MermaidEditResult`. + """ + macros_in = iter_mermaid_macros(adf) + macro_count = len(macros_in) + + # Zero macros -> tell the node to fall back to a storage-format body update, + # rather than attempting (and "succeeding" at) an empty ADF edit (D7). + if macro_count == 0: + return MermaidEditResult( + macro_count=0, + new_adf=copy.deepcopy(adf), + revert_diff=[], + unmatched_keys=[edit.macro_key for edit in (edits or [])], + skip_mermaid=True, + ) + + # Work on a deep copy so the caller's document is never mutated, even with + # apply=True (this module performs no persistence; the caller does). + new_adf = copy.deepcopy(adf) + + edit_list = list(edits or []) + if not edit_list: + # Verify-only: no edits requested. Empty revert-diff by construction. + return MermaidEditResult( + macro_count=macro_count, + new_adf=new_adf, + revert_diff=[], + unmatched_keys=[], + skip_mermaid=False, + ) + + # Index the copy's macros by stable key (document order matches the input, + # so positional keys line up with the keys the caller derived from `adf`). + macros_out = iter_mermaid_macros(new_adf) + by_key: dict[str, ADFDocument] = {} + for index, node in enumerate(macros_out): + by_key.setdefault(macro_key(node, index=index), node) + + revert_diff: list[RevertOp] = [] + unmatched_keys: list[str] = [] + + for edit in edit_list: + target = by_key.get(edit.macro_key) + if target is None: + unmatched_keys.append(edit.macro_key) + continue + old_source = macro_diagram_source(target) + if edit.new_source == old_source: + # No-op edit: identical source contributes nothing to the + # revert-diff (the safety invariant). + continue + if not _set_diagram_source(target, edit.new_source): + # Macro has no writable source slot; record as unmatched rather than + # silently dropping the change. + unmatched_keys.append(edit.macro_key) + continue + revert_diff.append( + RevertOp( + macro_key=edit.macro_key, + old_source=old_source, + new_source=edit.new_source, + ) + ) + + return MermaidEditResult( + macro_count=macro_count, + new_adf=new_adf, + revert_diff=revert_diff, + unmatched_keys=unmatched_keys, + skip_mermaid=False, + ) diff --git a/agent-team/agent_team/coordinator.py b/agent-team/agent_team/coordinator.py index 0fe4ba0..fe2da66 100644 --- a/agent-team/agent_team/coordinator.py +++ b/agent-team/agent_team/coordinator.py @@ -150,6 +150,13 @@ AlarmHook = Callable[[str], None] ListenerFactory = Callable[[], Any] +def _truthy_env(value: str | None) -> bool: + """Interpret an env-flag string as a boolean (case-folded; None -> False).""" + if not value: + return False + return value.strip().lower() in {"1", "true", "yes", "on"} + + def _open_question_id_for_thread(conn: Any, thread_id: str) -> str | None: """Return the ``question_id`` of an open ledger row for ``thread_id``, or None. @@ -699,6 +706,7 @@ class Coordinator: ci_timeout: timedelta | None = None, draft_pr_provider: "Callable[[], list[Any]] | None" = None, plan_gate: bool = True, + confluence: bool | None = None, ) -> None: self._db_path = Path(db_path) self._transport = transport @@ -766,6 +774,15 @@ class Coordinator: # serve/P2 path gate-enabled out of the box; tests build both shapes by # toggling this with/without ``review_wiring``. self._plan_gate = plan_gate + # Confluence-writer sub-pipeline (opt-in, default OFF). Gated behind the + # AGENT_TEAM_CONFLUENCE_ENABLED env flag UNLESS an explicit bool is passed + # (tests pass confluence=True/False directly). When OFF, ``setup`` builds + # the graph with confluence=False so NO conf_* nodes / route_after_intake + # exist and the daemon is byte-identical to today (every current test + # unchanged). The env is read at construction time, never at import. + if confluence is None: + confluence = _truthy_env(os.environ.get("AGENT_TEAM_CONFLUENCE_ENABLED")) + self._confluence = bool(confluence) # Built by setup(). self._graph: Any = None @@ -889,6 +906,7 @@ class Coordinator: build_verify=build_verify, dispatch_node=dispatch_node_callable, plan_gate=plan_gate, + confluence=self._confluence, ) # The ResumeWorker is satisfied directly by the compiled LangGraph app @@ -920,7 +938,12 @@ class Coordinator: # ------------------------------------------------------------------ # def start_task( - self, *, task_text: str, transport_name: str, slack_thread_ts: str = "" + self, + *, + task_text: str, + transport_name: str, + slack_thread_ts: str = "", + task_kind: str = "", ) -> str: """Start one task: run to the first human gate, then notify (§3.3, §3.3.1). @@ -965,6 +988,7 @@ class Coordinator: transport=transport_name, task=task_text, slack_thread_ts=slack_thread_ts, + task_kind=task_kind, ) question = graph_mod.pending_question(self._graph, thread_id=thread_id) @@ -975,6 +999,20 @@ class Coordinator: _LOG.info("start_task %s produced no pending question", thread_id) return thread_id + # A direct Confluence task (Flow A) suspends at CONF_GATE on the FIRST + # invoke (kind='confluence_approval') instead of the clarifier — deliver + # the approval gate exactly like the lifecycle drain does (durable row + + # decision presentation) rather than the clarifier notify path. + if question.get("kind") == graph_mod.CONFLUENCE_APPROVAL_KIND: + label = self._task_label(thread_id, task_text) + self._post_confluence_gate( + question, + label=label, + root_ts=slack_thread_ts or None, + short=thread_id[:8], + ) + return thread_id + question_set = question["question_set"] deadline = question.get("deadline") or self._default_deadline() @@ -1211,12 +1249,37 @@ class Coordinator: # the gate (B2a). Branch on the kind so the plan gate posts its # plan + findings presentation while the clarifier keeps its # existing follow-up path. - if question.get("kind") == graph_mod.PLAN_DECISION_KIND: + kind = question.get("kind") + if kind == graph_mod.PLAN_DECISION_KIND: self._post_plan_decision_gate( question, label=label, root_ts=root_ts, short=short ) continue + if kind == graph_mod.CONFLUENCE_APPROVAL_KIND: + # The task is suspended at CONF_GATE (Confluence Flow A/B). Open + # + present the approve / request_changes / abandon gate exactly + # like the plan-decision gate so the human is notified and a + # durable pending_questions row is written (without this branch + # the gate is never delivered and the task suspends forever). + self._post_confluence_gate( + question, label=label, root_ts=root_ts, short=short + ) + continue + + # A clarifier QUESTION gate (kind == 'clarify' / legacy no-kind). + # An UNKNOWN kind reaching here would have no question_set, so + # .get() avoids a KeyError that would silently drop the gate. + question_set = question.get("question_set") + if question_set is None: + _LOG.warning( + "pending interrupt for %s has unknown kind %r and no " + "question_set; skipping delivery", + short, + kind, + ) + continue + # Multi-turn clarify: a new clarifier question is waiting. Post it # to the transport (the drain path otherwise leaves it unposted) # and tell the human more input is needed. Thread it (and its @@ -1228,7 +1291,7 @@ class Coordinator: responder_mod.notify_question( conn, self._transport, - question["question_set"], + question_set, deadline=question.get("deadline") or self._default_deadline(), thread_ts=root_ts, @@ -1601,6 +1664,208 @@ class Coordinator: }, ) + # ------------------------------------------------------------------ # + # Confluence-approval gate (Flow A/B). Mirrors the plan-decision gate. + # ------------------------------------------------------------------ # + + def _task_label(self, thread_id: str, task_text: str = "") -> str: + """Render the ``"desc" (`short`)`` label for a task's lifecycle posts. + + Prefers the live graph state's ``task`` description, falling back to the + supplied ``task_text`` (the intake string, available before the first + checkpoint read). Mirrors the inline label built in :meth:`tick`. + """ + short = thread_id[:8] + desc = "" + try: + snap = self._graph.get_state(graph_mod.thread_config(thread_id)) + values = getattr(snap, "values", {}) or {} + desc = str(values.get("task") or "").strip() + except Exception: # noqa: BLE001 - fall back to the intake text + pass + if not desc: + desc = str(task_text or "").strip() + desc = desc or "(no description)" + if len(desc) > 90: + desc = desc[:90] + "…" + return f'"{desc}" (`{short}`)' + + def _post_confluence_gate( + self, + question: "dict[str, Any]", + *, + label: str, + root_ts: str | None, + short: str, + ) -> None: + """Open + present the Confluence-approval gate for a suspended task. + + The graph has suspended on the resumable CONF_GATE interrupt whose payload + carries ``kind == 'confluence_approval'`` plus the + ``question_set``/``preview``. This mirrors :meth:`_post_plan_decision_gate`: + + 1. Writes the durable ``pending_questions`` row with + ``kind='confluence_approval'`` (guarded by the single-open-gate + invariant), threaded under the task root. + 2. POSTs the approve / request_changes / abandon presentation, threaded + under the task root, truncated to Slack's block limit. + + The decision answer flows back through the UNCHANGED submit_answer → + resume-queue → ResumeWorker path (``conf_gate_node`` consumes the resume + value). Best-effort + fully guarded so a post failure never breaks tick. + """ + deadline = question.get("deadline") or self._default_deadline() + thread_id = str(question.get("thread_id") or "") + question_id = str(question.get("question_id") or "") + turn = int(question.get("turn") or 0) + transport_name = str(question.get("transport") or "") + + body = self._confluence_presentation(question, label=label) + try: + conn = connect(self._db_path) + try: + opened = self._open_confluence_row_if_absent( + conn, + thread_id=thread_id, + question_id=question_id, + turn=turn, + transport_name=transport_name, + deadline=deadline, + root_ts=root_ts, + presentation=body, + ) + finally: + conn.close() + except Exception: # noqa: BLE001 - a ledger error must not break the tick + _LOG.warning( + "failed to open confluence gate row for %s", short, exc_info=True + ) + opened = False + + if not opened: + return + + self._emit(body, thread_ts=root_ts) + + def _confluence_presentation( + self, question: "dict[str, Any]", *, label: str + ) -> str: + """Render the Confluence-gate presentation (draft preview + instructions). + + Uses the human-readable ``preview`` the gate node built (the draft title + + action + body snippet), then appends the explicit decision instructions. + Truncated to Slack's section-block limit. + """ + preview = " ".join(str(question.get("preview") or "").split()) + if not preview: + preview = "(no draft preview recorded)" + + instructions = ( + "• Decide: use the buttons below, OR reply *approve* / " + "*request changes * / *abandon* in this thread." + ) + header = f"📄 {label} — Confluence update needs your approval." + body = "\n".join([header, preview, instructions]) + return self._truncate_for_slack(body) + + def _open_confluence_row_if_absent( + self, + conn: Any, + *, + thread_id: str, + question_id: str, + turn: int, + transport_name: str, + deadline: str, + root_ts: str | None, + presentation: str = "", + ) -> bool: + """Open a ``kind='confluence_approval'`` ledger row (one-open-gate, B-2). + + Mirrors :meth:`_open_plan_decision_row_if_absent`: enforces the + single-open-gate invariant, writes the durable ``open`` row FIRST, then + posts the gate presentation threaded under ``root_ts`` and records the + ``channel_ref``. Returns ``True`` iff a fresh row was opened. + """ + existing = _open_question_id_for_thread(conn, thread_id) + if existing is not None: + _LOG.warning( + "single-open-gate invariant: thread %s already has open question " + "%s; not opening a second (confluence_approval) gate row", + thread_id[:8], + existing, + ) + return False + + from agent_team import ledger as ledger_mod # noqa: PLC0415 + + ledger_mod.post_question( + conn, + question_id=question_id, + thread_id=thread_id, + turn=turn, + transport=transport_name or type(self._transport).__name__, + deadline_at=deadline, + kind=graph_mod.CONFLUENCE_APPROVAL_KIND, + ) + + channel_ref: str | None = None + try: + post_kwargs: dict[str, Any] = {} + if root_ts: + post_kwargs["thread_ts"] = root_ts + posted_ref = self._transport.post_question( + thread_id=thread_id, + question_id=question_id, + turn=turn, + question_set=self._confluence_question_set( + thread_id=thread_id, + question_id=question_id, + turn=turn, + presentation=presentation, + ), + deadline=deadline, + **post_kwargs, + ) + channel_ref = root_ts if root_ts else posted_ref + except Exception: # noqa: BLE001 - lost post is recoverable; keep the row + _LOG.warning( + "confluence gate post failed for thread %s; row stays open " + "for redelivery", + thread_id[:8], + exc_info=True, + ) + return True + + if channel_ref: + ledger_mod.set_channel_ref( + conn, question_id=question_id, channel_ref=channel_ref + ) + return True + + @staticmethod + def _confluence_question_set( + *, thread_id: str, question_id: str, turn: int, presentation: str = "" + ) -> "QuestionSet": + """Build the QuestionSet for the Confluence gate post (decision surface). + + Mirrors :meth:`_plan_decision_question_set`: ``context`` carries + ``kind == 'confluence_approval'`` (so the Slack transport renders the + three decision buttons) and the human-readable ``presentation`` body. + """ + from agent_team.transport.base import QuestionSet # noqa: PLC0415 + + return QuestionSet( + thread_id=thread_id, + question_id=question_id, + turn=turn, + questions=["Approve, request changes, or abandon this Confluence update?"], + context={ + "kind": graph_mod.CONFLUENCE_APPROVAL_KIND, + "presentation": presentation, + }, + ) + @staticmethod def _summarize_plan(values: "dict[str, Any]") -> str: """Condensed, Slack-friendly view of the approved plan (summary + phases). @@ -2213,7 +2478,19 @@ class Coordinator: # destroyed and the human is never re-asked. if not self._guarded_clear_unposted_open(row["question_id"]): continue - question_set = question["question_set"] + # Every gate kind (clarify / plan_decision / confluence_approval) + # carries a question_set in its interrupt payload; .get() so an + # unknown kind without one is skipped rather than KeyError-ing and + # vanishing the redelivery. + question_set = question.get("question_set") + if question_set is None: + _LOG.warning( + "redeliver: pending interrupt for %s has no question_set " + "(kind=%r); skipping re-post", + thread_id[:8], + question.get("kind"), + ) + continue deadline = ( row["deadline_at"] or question.get("deadline") diff --git a/agent-team/agent_team/db/schema.py b/agent-team/agent_team/db/schema.py index a569a55..81df35e 100644 --- a/agent-team/agent_team/db/schema.py +++ b/agent-team/agent_team/db/schema.py @@ -53,7 +53,7 @@ __all__ = [ ] # Bump when the DDL below changes; migrate() steps a connection forward. -SCHEMA_VERSION: int = 4 +SCHEMA_VERSION: int = 5 # Default SQLite busy timeout (ms) so concurrent writers wait for the write # lock rather than failing immediately. @@ -79,7 +79,7 @@ CREATE TABLE IF NOT EXISTS pending_questions ( answered_at TEXT, answered_via TEXT, kind TEXT NOT NULL DEFAULT 'clarify' - CHECK (kind IN ('clarify', 'plan_decision')) + CHECK (kind IN ('clarify', 'plan_decision', 'confluence_approval')) ) """.strip() @@ -331,7 +331,88 @@ def migrate(conn: sqlite3.Connection) -> None: _ensure_pending_questions_kind(conn) current = 4 - # Future steps go here: `if current < 5: ...; current = 5`. + if current < 5: + # v5: WIDEN the pending_questions ``kind`` CHECK from + # ('clarify','plan_decision') to add 'confluence_approval', so the new + # Confluence-approval human gate can persist its question rows. SQLite + # cannot ALTER a CHECK constraint in place, so this does the standard + # 12-step table rebuild: build a replacement table carrying the WIDENED + # CHECK, copy every existing row across, drop the old table, rename the + # new one into place, and recreate ALL of pending_questions' indexes + # (including the partial unique uq_pending_questions_open_channel_ref). + # Existing rows are preserved verbatim; re-running is safe because the + # version stamp gates this block (and the unconditional index tail below + # re-asserts the indexes idempotently). Placed in its own version block + # ABOVE the unconditional tail per the ORDERING CONSTRAINT below — the + # tail's CREATE ... IF NOT EXISTS would NOT rebuild an existing table. + # + # foreign_keys must be OFF during the table rebuild so the DROP/RENAME + # does not trip referential checks; connect() sets it ON. It is a no-op + # for this table (no FKs reference pending_questions) but follows the + # documented SQLite procedure. It is restored to ON afterwards. + # + # The whole DROP/RENAME swap is wrapped in an explicit BEGIN IMMEDIATE + # transaction so it is ATOMIC: connect() opens in autocommit mode + # (isolation_level=None), so without this each statement would commit on + # its own and a crash between DROP TABLE pending_questions and the RENAME + # would DESTROY the live ledger while schema_version stayed 4 — on restart + # the block re-runs and CREATE pending_questions_new fails ('already + # exists'), crash-looping with data loss. Wrapping makes a mid-rebuild + # failure roll back to the original table intact. PRAGMA foreign_keys + # cannot change inside a transaction, so it is toggled OUTSIDE the BEGIN. + # DROP ... _new IF EXISTS first clears any orphan left by a prior partial + # run (belt-and-suspenders; the transaction already prevents one). + conn.execute("PRAGMA foreign_keys=OFF") + try: + conn.execute("BEGIN IMMEDIATE") + try: + conn.execute("DROP TABLE IF EXISTS pending_questions_new") + conn.execute( + """ + CREATE TABLE pending_questions_new ( + question_id TEXT PRIMARY KEY, + thread_id TEXT NOT NULL, + turn INTEGER NOT NULL, + status TEXT NOT NULL + CHECK (status IN ('open', 'answered', 'expired', 'superseded')), + transport TEXT NOT NULL, + channel_ref TEXT, + posted_at TEXT, + deadline_at TEXT, + answer_json TEXT, + answered_at TEXT, + answered_via TEXT, + kind TEXT NOT NULL DEFAULT 'clarify' + CHECK (kind IN ('clarify', 'plan_decision', 'confluence_approval')) + ) + """.strip() + ) + # EXPLICIT column list (not SELECT *) so the copy survives a + # future column reorder/add on either table. + conn.execute( + "INSERT INTO pending_questions_new " + "(question_id, thread_id, turn, status, transport, " + "channel_ref, posted_at, deadline_at, answer_json, " + "answered_at, answered_via, kind) " + "SELECT question_id, thread_id, turn, status, transport, " + "channel_ref, posted_at, deadline_at, answer_json, " + "answered_at, answered_via, kind FROM pending_questions" + ) + conn.execute("DROP TABLE pending_questions") + conn.execute( + "ALTER TABLE pending_questions_new RENAME TO pending_questions" + ) + for stmt in _split_statements(PENDING_QUESTIONS_INDEXES_DDL): + conn.execute(stmt) + conn.execute("COMMIT") + except Exception: + conn.execute("ROLLBACK") + raise + finally: + conn.execute("PRAGMA foreign_keys=ON") + current = 5 + + # Future steps go here: `if current < 6: ...; current = 6`. # Applied UNCONDITIONALLY (idempotent IF NOT EXISTS) so an already-stamped DB # — which skips the version blocks above — still gains these tables without a diff --git a/agent-team/agent_team/db/schema.sql b/agent-team/agent_team/db/schema.sql index b410d57..f082033 100644 --- a/agent-team/agent_team/db/schema.sql +++ b/agent-team/agent_team/db/schema.sql @@ -27,11 +27,12 @@ CREATE TABLE IF NOT EXISTS pending_questions ( answered_at TEXT, answered_via TEXT, -- kind: discriminates the human gate this question belongs to — - -- 'clarify' (the clarifier) or 'plan_decision' (the plan-review gate). + -- 'clarify' (the clarifier), 'plan_decision' (the plan-review gate), or + -- 'confluence_approval' (the Confluence-approval gate; widened in v5). -- Defaults to 'clarify' so an in-place ALTER on a legacy ledger and any -- existing rows take the clarifier value. kind TEXT NOT NULL DEFAULT 'clarify' - CHECK (kind IN ('clarify', 'plan_decision')) + CHECK (kind IN ('clarify', 'plan_decision', 'confluence_approval')) ); CREATE INDEX IF NOT EXISTS idx_pending_questions_thread diff --git a/agent-team/agent_team/graph.py b/agent-team/agent_team/graph.py index f9affdc..1e7056b 100644 --- a/agent-team/agent_team/graph.py +++ b/agent-team/agent_team/graph.py @@ -50,6 +50,12 @@ from typing import TYPE_CHECKING, Any from langgraph.graph import END, START, StateGraph from langgraph.types import Command, interrupt +from agent_team.nodes.confluence_writer import ( + conf_draft_node, + conf_gate_node, + conf_write_node, + route_after_conf_gate, +) from agent_team.task_model import ( Phase, PipelineState, @@ -87,6 +93,13 @@ __all__ = [ "BUILD_NODE", "BUILD_ROUTE", "CLARIFY", + "CONFLUENCE_APPROVAL_KIND", + "CONF_APPROVE_ROUTE", + "CONF_DRAFT_NODE", + "CONF_GATE", + "CONF_REVISE_ROUTE", + "CONF_TERMINAL_ROUTE", + "CONF_WRITE_NODE", "DEFAULT_CLARIFY_DEADLINE", "DISPATCH_NODE", "GATE_APPROVE_ROUTE", @@ -112,6 +125,8 @@ __all__ = [ "plan_node", "plan_phase", "resume_task", + "route_after_conf_gate", + "route_after_intake", "route_after_plan_gate", "start_task", "thread_config", @@ -153,6 +168,43 @@ GATE_TERMINAL_ROUTE = "gate_terminal" # can tell the two apart. PLAN_DECISION_KIND = "plan_decision" +# --- Confluence-writer sub-pipeline (opt-in via build_graph(confluence=True)). - +# The Confluence stage documents a task's work on Confluence, gated by a human +# exactly like the plan gate: +# +# CONF_DRAFT -> CONF_GATE -> {CONF_WRITE | back to CONF_DRAFT | terminal} +# +# It is reached two ways (both behind the ``confluence`` flag — when the flag is +# off NONE of this is wired and the graph is byte-for-byte identical to today): +# +# * Flow A (direct doc task): INTAKE routes on ``state['task_kind']`` — when it +# is "confluence" the task goes straight to CONF_DRAFT instead of CLARIFY. +# * Flow B (post-build doc): when the build/verify subgraph is present, the +# approved-build terminus feeds into CONF_DRAFT so a shipped change documents +# itself. (If build_verify is absent, Flow B simply isn't wired.) +# +# The node functions + router live in agent_team.nodes.confluence_writer (imported +# above); these are the GRAPH VERTEX names + route ids, kept distinct from each +# other so a route key never collides with a vertex id (same discipline as the +# plan gate). +CONF_DRAFT_NODE = "conf_draft" +CONF_GATE = "conf_gate" +CONF_WRITE_NODE = "conf_write" + +# The route ids route_after_conf_gate returns (mirror review_loop/plan-gate +# value-coupling: the node's router returns these literals, the conditional-edge +# map below keys on them). +CONF_APPROVE_ROUTE = "approve" +CONF_REVISE_ROUTE = "revise" +CONF_TERMINAL_ROUTE = "terminal" + +# Interrupt-payload discriminator marking a Confluence-approval gate (vs the +# clarifier question-set / plan-decision gate). The confluence_writer node imports +# THIS constant from graph when present and falls back to the same literal, so +# this is the authoritative definition keeping the responder/ledger discriminator +# in sync (sibling to PLAN_DECISION_KIND). +CONFLUENCE_APPROVAL_KIND = "confluence_approval" + # Combined ceiling on plan-gate visits (Phase B2a, bounded termination). A human # ``request_changes`` re-enters plan<->review, which can hit the review cap and # gate AGAIN. Each gate visit increments ``plan_gate_visits``; once it reaches @@ -540,6 +592,27 @@ def route_after_plan_gate(state: PipelineState) -> str: return GATE_TERMINAL_ROUTE +def route_after_intake(state: PipelineState) -> str: + """LangGraph conditional-edge after INTAKE (Confluence Flow A dispatch). + + Reads ``state['task_kind']`` and dispatches a direct Confluence-documentation + task straight to the Confluence draft stage; every other task (the default, + ``task_kind`` empty) goes to the clarifier exactly as before. Returns a node + name (the conditional-edge map below is an identity map, keying each route on + the destination vertex): + + * ``task_kind == "confluence"`` -> :data:`CONF_DRAFT_NODE`; + * anything else -> :data:`CLARIFY` (unchanged P1 behaviour). + + This function only matters when ``build_graph(confluence=True)`` wired the + conditional edge; with the flag off INTAKE has its original unconditional + edge to CLARIFY and ``task_kind`` is never read. + """ + if state.get("task_kind") == "confluence": + return CONF_DRAFT_NODE + return CLARIFY + + def _author_questions(state: PipelineState) -> list[str]: """Deterministic stand-in for the Claude clarifier's question authoring. @@ -643,6 +716,7 @@ def build_graph( | None = None, dispatch_node: Callable[[PipelineState], Any] | None = None, plan_gate: bool = False, + confluence: bool = False, ) -> CompiledStateGraph: """Assemble + compile the P1 pipeline ``StateGraph`` (§3.3, §7.1). @@ -718,6 +792,27 @@ def build_graph( falls back to ``BUILD -> VERIFY`` directly. ``dispatch_node`` requires ``build_verify`` (there is no BUILD/VERIFY pair to splice it between otherwise). + + ``confluence`` (default ``False``) opts the Confluence-writer sub-pipeline + into the graph: ``CONF_DRAFT -> CONF_GATE -> {CONF_WRITE | CONF_DRAFT | + END}``, gated by a human exactly like the plan gate. When it is ON: + + * **Flow A (direct doc task):** INTAKE's unconditional edge to CLARIFY is + replaced by a conditional edge on :func:`route_after_intake`, so a task with + ``state['task_kind'] == "confluence"`` goes straight to CONF_DRAFT while + every other task still goes to CLARIFY (existing behaviour preserved). + * **Flow B (post-build doc):** when ``build_verify`` is also wired, the + verifier's approved (PR) terminus is repointed from END into CONF_DRAFT, so + a shipped change documents itself before the graph ends. If ``build_verify`` + is absent, Flow B is simply not wired (the build path doesn't exist). + * **Tail:** ``CONF_DRAFT -> CONF_GATE``; the gate routes ``approve -> + CONF_WRITE``, ``revise -> CONF_DRAFT`` (bounded redraft loop), ``terminal -> + END``; and ``CONF_WRITE -> END``. + + When ``confluence`` is ``False`` (the default) NONE of these nodes or edges + exist and the compiled graph is byte-for-byte behaviourally identical to the + P1/P2/P3 graph — INTAKE keeps its plain ``add_edge(INTAKE, CLARIFY)`` and + ``task_kind`` is never consulted. This keeps every existing test untouched. """ clarify = live_clarify_node if live_clarify_node is not None else clarify_node plan = live_plan_node if live_plan_node is not None else plan_node @@ -756,7 +851,18 @@ def build_graph( builder.add_node(PLAN, _instrument(PLAN, plan, transition_recorder)) builder.add_edge(START, INTAKE) - builder.add_edge(INTAKE, CLARIFY) + # Confluence Flow A: route INTAKE on task_kind so a direct documentation task + # ("confluence") skips the clarifier and goes straight to CONF_DRAFT, while + # every other task still flows to CLARIFY. When the flag is off INTAKE keeps + # its original unconditional edge (so the graph is unchanged). + if confluence: + builder.add_conditional_edges( + INTAKE, + route_after_intake, + {CLARIFY: CLARIFY, CONF_DRAFT_NODE: CONF_DRAFT_NODE}, + ) + else: + builder.add_edge(INTAKE, CLARIFY) builder.add_edge(CLARIFY, PLAN) if review_node is None: @@ -841,13 +947,51 @@ def build_graph( builder.add_edge(DISPATCH_NODE, VERIFY_NODE) # VERIFY's verdict routes to {approved -> END (PR terminus), # build -> BUILD (bounded build<->verify loop), parked -> END - # (escalation)} regardless of whether DISPATCH is wired. + # (escalation)} regardless of whether DISPATCH is wired. When the + # Confluence stage is opted in (Flow B), the approved (PR) terminus is + # repointed from END into CONF_DRAFT so a shipped change documents + # itself before the graph ends. + approved_target = CONF_DRAFT_NODE if confluence else END builder.add_conditional_edges( VERIFY_NODE, route_after_verify, - {APPROVED_ROUTE: END, BUILD_ROUTE: BUILD_NODE, PARKED_ROUTE: END}, + { + APPROVED_ROUTE: approved_target, + BUILD_ROUTE: BUILD_NODE, + PARKED_ROUTE: END, + }, ) + if confluence: + # Confluence-writer tail: CONF_DRAFT -> CONF_GATE -> {CONF_WRITE | redraft + # | END}. The draft + write nodes take an optional config/client kwarg + # (LangGraph injects only the state + RunnableConfig); _instrument + # preserves each node's real signature so the right arity is passed. The + # gate suspends on interrupt() exactly like the plan gate. The entry edges + # (INTAKE Flow A, VERIFY Flow B) were wired above. + builder.add_node( + CONF_DRAFT_NODE, + _instrument(CONF_DRAFT_NODE, conf_draft_node, transition_recorder), + ) + builder.add_node( + CONF_GATE, _instrument(CONF_GATE, conf_gate_node, transition_recorder) + ) + builder.add_node( + CONF_WRITE_NODE, + _instrument(CONF_WRITE_NODE, conf_write_node, transition_recorder), + ) + builder.add_edge(CONF_DRAFT_NODE, CONF_GATE) + builder.add_conditional_edges( + CONF_GATE, + route_after_conf_gate, + { + CONF_APPROVE_ROUTE: CONF_WRITE_NODE, + CONF_REVISE_ROUTE: CONF_DRAFT_NODE, + CONF_TERMINAL_ROUTE: END, + }, + ) + builder.add_edge(CONF_WRITE_NODE, END) + if checkpointer is None: return builder.compile() return builder.compile(checkpointer=checkpointer) @@ -970,6 +1114,7 @@ def start_task( transport: str = "", task: str = "", slack_thread_ts: str = "", + task_kind: str = "", ) -> tuple[str, PipelineState]: """Start a new pipeline task and run it up to the first human gate (§3.3). @@ -996,6 +1141,15 @@ def start_task( The graph MUST be compiled with a checkpointer for the suspend to persist; an uncheckpointed graph would run straight through without honouring the interrupt. + + ``task_kind`` selects the pipeline lane (Confluence Flow A). The default ``""`` + is the ordinary clarify->plan task; ``"confluence"`` marks a direct + Confluence-documentation task that, in a graph compiled with + ``build_graph(confluence=True)``, routes INTAKE straight to CONF_DRAFT (see + :func:`route_after_intake`). Like ``task``/``slack_thread_ts`` it is seeded + into the initial invoke and persists through INTAKE (``intake_node`` returns + only a partial state). With ``confluence=False`` the value is harmless and + never consulted (INTAKE's edge is unconditional). """ tid = thread_id or new_thread_id() now = _utc_now_iso() @@ -1004,6 +1158,7 @@ def start_task( status=TaskStatus.ACTIVE.value, current_phase=_phase_value(Phase.INTAKE), task=task, + task_kind=task_kind, slack_thread_ts=slack_thread_ts, qa_history=[], transport=transport, diff --git a/agent-team/agent_team/nodes/confluence_writer.py b/agent-team/agent_team/nodes/confluence_writer.py new file mode 100644 index 0000000..688ff6c --- /dev/null +++ b/agent-team/agent_team/nodes/confluence_writer.py @@ -0,0 +1,691 @@ +"""Confluence-writer pipeline stage — draft -> human gate -> (dry-run) write. + +Three LangGraph nodes plus a router that document a task's work on Confluence, +gated by a human exactly like the plan gate (design §3.3 human-in-the-loop): + + ... -> CONF_DRAFT -> CONF_GATE -> {CONF_WRITE | back to CONF_DRAFT | terminal} + +* :func:`conf_draft_node` — an **AGENTIC** Claude call that inspects the repo + (read-only) and emits a Confluence page update as DATA (a ``confluence_draft`` + dict). It writes nothing to the repo. Source: Flow A uses ``state['task']``; + Flow B folds in ``state['plan']`` + ``state['candidate_diff']`` + the repo + name. On a ``request_changes`` loop-back the prior ``confluence_feedback`` is + folded into the prompt. +* :func:`conf_gate_node` — the resumable HUMAN GATE. Mirrors + :func:`agent_team.graph.plan_gate_node` VERBATIM in shape: a ceiling guard + FIRST (cap on ``confluence_gate_visits`` -> terminal PARKED), then an + ``interrupt()`` carrying a stable ``question_id`` derived from a ``turn`` in a + HIGH namespace disjoint from BOTH the clarifier and the plan gate, a deadline, + and a ``kind`` discriminator. On resume it parses the decision verb and routes + approve / request_changes / abandon. +* :func:`conf_write_node` — performs the write through the injected Confluence + client. **DRY-RUN by default**: a live write is issued ONLY when an explicit + apply flag is set (``config['confluence_apply']`` or env + ``AGENT_TEAM_CONFLUENCE_APPLY`` truthy) AND the gate approved; otherwise it + composes the planned change + a revert diff and writes NOTHING. +* :func:`route_after_conf_gate` — the conditional-edge function mirroring + :func:`agent_team.graph.route_after_plan_gate`. + +The HTTP/Confluence I/O is an INJECTED seam: :func:`conf_write_node` takes an +optional ``client`` (defaulting to a real :class:`ConfluenceClient`) so tests +pass an in-memory fake and no live network is touched on the paths tests hit. +The Confluence package import is DEFERRED to call time (and only on the live +node) so this module imports cleanly before that package exists. Secrets/creds +are read from ``os.environ`` at call time, never at module load. + +Graph-wiring constants this module references by NAME (the graph-agent owns +``graph.py`` and must define / wire these): + +* ``CONFLUENCE_APPROVAL_KIND`` — the interrupt-payload ``kind`` discriminator + (defined here with the literal value ``"confluence_approval"``; the graph may + re-export it). The graph must keep its value in sync. +* The CONF_DRAFT / CONF_GATE / CONF_WRITE phase values — this module emits the + :class:`~agent_team.task_model.Phase` ``.value`` strings the graph routes on. + See :data:`CONF_DRAFT_PHASE` / :data:`CONF_GATE_PHASE` / :data:`CONF_WRITE_PHASE` + / :data:`CONF_DONE_PHASE` and the note in the returned summary. +""" + +from __future__ import annotations + +import os +import uuid +from datetime import datetime, timedelta, timezone +from typing import TYPE_CHECKING, Any + +from langgraph.types import interrupt + +from agent_team.billing import ClaudeResult, claude_invoke +from agent_team.transport import QuestionSet +from agent_team.nodes.confluence_writer_llm import ( + build_confluence_prompt, + parse_confluence_reply, +) +from agent_team.task_model import Phase, PipelineState, TaskStatus + +if TYPE_CHECKING: # pragma: no cover - typing only; package may not exist yet + from agent_team.confluence.client import ConfluenceClient + +__all__ = [ + "CONFLUENCE_APPROVAL_KIND", + "MAX_CONFLUENCE_GATE_VISITS", + "ConfluenceWriteError", + "conf_draft_node", + "conf_gate_node", + "conf_write_node", + "route_after_conf_gate", +] + +# -- Graph-shared constants (see module docstring). -------------------------- + +# Interrupt-payload discriminator telling the responder/ledger this is a +# Confluence-approval gate (vs the clarifier question-set / plan-decision gate). +# The graph-agent defines the authoritative constant; this local fallback keeps +# the node self-contained and importable before graph.py wires it. KEEP THE +# VALUE IN SYNC with graph.PLAN_DECISION_KIND's sibling. +try: # pragma: no cover - exercised only once the graph defines it + from agent_team.graph import CONFLUENCE_APPROVAL_KIND # type: ignore +except Exception: # noqa: BLE001 - graph may not define it yet on this branch + CONFLUENCE_APPROVAL_KIND = "confluence_approval" + +# Combined ceiling on Confluence-gate visits (bounded termination, mirrors +# graph.MAX_PLAN_GATE_VISITS). A human ``request_changes`` re-enters the draft +# stage, which gates AGAIN; once the visit count reaches this cap the gate stops +# offering request_changes and the task goes terminal PARKED so the human loop +# always terminates. +MAX_CONFLUENCE_GATE_VISITS = 3 + +# Per-call turn headroom + budget for the AGENTIC draft call. The subscription +# invoker defaults to single-shot (max_turns=1), which an agentic repo-inspecting +# call exhausts before it can finish ("Reached maximum number of turns (1)"; PR +# #61 / issue #60). This call READS the repo to write accurate docs, so it needs +# real turns and a READ-ONLY toolset — it emits the draft as DATA and writes +# nothing, so no Edit/Write/Bash. +_DRAFT_MAX_TURNS = 8 +_DRAFT_ALLOWED_TOOLS: tuple[str, ...] = ("Read", "Grep", "Glob") +_DRAFT_BUDGET_USD = 4.0 + +# Phase .value strings this stage routes on. The graph wires vertices that read +# these; kept as Phase members so a future dedicated Phase enum value is a +# one-line swap. Until the foundation adds CONF_* members the stage reuses the +# closest existing phases (VERIFY/BUILD/DONE/PARKED) for routing — the graph-agent +# repoints these if dedicated phases are introduced. +CONF_DRAFT_PHASE = Phase.VERIFY.value +CONF_GATE_PHASE = Phase.REVIEW.value +CONF_WRITE_PHASE = Phase.BUILD.value +CONF_DONE_PHASE = Phase.DONE.value + +# Disjoint namespace base for the Confluence gate's stable ``turn`` derivation. +# The clarifier numbers turns 0..N (qa_history length); the plan gate uses +# 1_000_000 + visits (graph._PLAN_GATE_TURN_BASE). This base is HIGHER and +# distinct from BOTH so a Confluence-gate turn can never collide with either for +# the same thread (the ResumeWorker turn guard matches a resume to its open +# interrupt by turn). +_CONF_GATE_TURN_BASE = 2_000_000 + +# Fixed namespace for deriving a STABLE question_id from (thread_id, turn) — +# uuid5 so the id is uuid-shaped yet deterministic across the resume replay of +# the node (mirrors graph._QUESTION_ID_NAMESPACE; a distinct namespace so a +# Confluence-gate id never collides with a clarifier/plan-gate id). +_CONF_QUESTION_ID_NAMESPACE = uuid.UUID("c0f1e2d3-4a5b-6c7d-8e9f-0a1b2c3d4e5f") + +# Default open window for a Confluence-gate question (mirrors +# graph.DEFAULT_CLARIFY_DEADLINE). +_DEFAULT_CONF_DEADLINE = timedelta(hours=24) + + +class ConfluenceWriteError(Exception): + """Raised when the live Confluence write cannot be performed. + + Distinct from a draft-parse failure (:class:`ConfluenceDraftError`) and from + a gate rejection: this is an I/O-time failure on the apply path, so the + coordinator can fail/park the task rather than mark it documented. + """ + + +# -- small helpers (mirror graph.py). ---------------------------------------- + + +def _utc_now_iso() -> str: + """Return the current UTC time as an ISO-8601 string (ledger-compatible).""" + return datetime.now(timezone.utc).isoformat() + + +def _question_id_for(thread_id: str, turn: int) -> str: + """Return the stable Confluence-gate question_id for ``(thread_id, turn)``.""" + return uuid.uuid5(_CONF_QUESTION_ID_NAMESPACE, f"{thread_id}:{turn}").hex + + +def _conf_gate_turn(visits: int) -> int: + """Return the stable gate ``turn`` for the ``visits``-th Confluence-gate visit. + + Offset into a high, disjoint namespace (:data:`_CONF_GATE_TURN_BASE`) so a + Confluence-gate turn can never collide with a clarifier turn (qa_history + length) or a plan-gate turn (1_000_000 + visits) for the same thread. + Monotonic in ``visits`` so each successive suspend has its own stable + ``(thread_id, turn)`` identity. + """ + return _CONF_GATE_TURN_BASE + visits + + +def _truthy(value: Any) -> bool: + """Interpret a config/env flag as a boolean (env strings are case-folded).""" + if isinstance(value, bool): + return value + if isinstance(value, str): + return value.strip().lower() in {"1", "true", "yes", "on"} + return bool(value) + + +def _draft_preview(draft: dict[str, Any]) -> str: + """Render a short human-readable preview of the draft for the gate payload.""" + title = str(draft.get("title", "(untitled)")) + page_id = draft.get("page_id") + target = f"update page {page_id}" if page_id else "create new page" + body = str(draft.get("body_storage", "")) + snippet = body[:400] + ("..." if len(body) > 400 else "") + mermaid = draft.get("mermaid_edits") or [] + lines = [f"Title: {title}", f"Action: {target}"] + if mermaid: + lines.append(f"Mermaid edits: {len(mermaid)}") + lines.append("") + lines.append(snippet) + return "\n".join(lines) + + +# Decision options the human picks from at the Confluence approval gate. Mirrors +# the plan gate's approve / request_changes / abandon surface so a downstream +# coordinator handler can render the three decision buttons. +_CONF_DECISION_OPTIONS: tuple[str, ...] = ("approve", "request_changes", "abandon") + + +def _conf_approval_question_set( + *, thread_id: str, question_id: str, turn: int, preview: str +) -> QuestionSet: + """Build the QuestionSet carried in the CONF_GATE interrupt payload. + + The coordinator's notify path requires a ``question_set`` so the inbound + answer maps back to ``question_id`` (mirrors + :meth:`coordinator._plan_decision_question_set`). The single question's + prompt is the human-readable draft ``preview`` followed by the + approve / request_changes / abandon options; ``context`` carries the + ``kind`` discriminator (:data:`CONFLUENCE_APPROVAL_KIND`) so the transport + renders decision buttons instead of generic question blocks. + """ + options = " / ".join(_CONF_DECISION_OPTIONS) + prompt = ( + f"{preview}\n\n" + f"Approve, request changes, or abandon this Confluence update? ({options})" + ) + return QuestionSet( + thread_id=thread_id, + question_id=question_id, + turn=turn, + questions=[prompt], + context={ + "kind": CONFLUENCE_APPROVAL_KIND, + "options": list(_CONF_DECISION_OPTIONS), + "presentation": preview, + }, + ) + + +# -- Node 1: CONF_DRAFT (agentic). ------------------------------------------- + + +def conf_draft_node( + state: PipelineState, config: dict[str, Any] | None = None +) -> PipelineState: + """CONF_DRAFT stage: agentic repo inspection -> Confluence draft (as DATA). + + Builds the draft prompt (Flow A from ``state['task']``; Flow B folding in + ``plan`` + ``candidate_diff`` + repo name; ``confluence_feedback`` folded in + on a ``request_changes`` loop-back), then makes ONE **agentic** Claude call + that may READ the repository to write accurate docs. The call is pinned to a + READ-ONLY toolset (``Read``/``Grep``/``Glob``), ``max_turns=8``, and a + ``budget_usd`` cap — the agentic-config rationale builders use (PR #61 / + issue #60: single-shot defaults die with "Reached maximum number of turns + (1)"). The node emits the draft as DATA and writes NOTHING to the repo, so no + Edit/Write/Bash tool is granted. + + Returns a **partial** :class:`PipelineState`: the parsed ``confluence_draft`` + plus the phase advanced to the Confluence gate and ``status`` ACTIVE. + + ``config`` is forwarded to the billing seam so the caller can pin the billing + mode; it is threaded through to :func:`claude_invoke` as ``config``. + """ + prompt = build_confluence_prompt(state) + + result: ClaudeResult = claude_invoke( + prompt, + config=config, + max_turns=_DRAFT_MAX_TURNS, + allowed_tools=list(_DRAFT_ALLOWED_TOOLS), + budget_usd=_DRAFT_BUDGET_USD, + ) + draft = parse_confluence_reply(result.text) + + return PipelineState( + confluence_draft=draft, # type: ignore[typeddict-unknown-key] + current_phase=CONF_GATE_PHASE, + status=TaskStatus.ACTIVE.value, + updated_at=_utc_now_iso(), + ) + + +# -- Node 2: CONF_GATE (resumable human gate; mirrors plan_gate_node). -------- + + +def conf_gate_node( + state: PipelineState, config: dict[str, Any] | None = None +) -> PipelineState: + """CONF_GATE stage: the resumable human approval of the Confluence draft. + + Mirrors :func:`agent_team.graph.plan_gate_node` in shape: + + * **Ceiling guard FIRST.** If ``confluence_gate_visits`` already reached + :data:`MAX_CONFLUENCE_GATE_VISITS` the gate does NOT interrupt: it returns + terminal PARKED with a ``failure_reason``, so every suspend strictly + consumes one of a finite number of visits and the human loop terminates. + * **Suspend.** Otherwise it bumps the visit count and ``interrupt()``s with a + payload carrying ``thread_id``, a stable ``question_id`` (from a ``turn`` + in a HIGH namespace disjoint from the clarifier AND the plan gate), the + ``turn``, ``kind`` = :data:`CONFLUENCE_APPROVAL_KIND`, ``transport``, + ``deadline``, ``slack_thread_ts``, the ``confluence_draft``, and a + human-readable ``preview``. + + On resume, ``interrupt()`` returns the decision (the value passed to + ``Command(resume=...)``). The verb is parsed via the shared safe normalizer + (unrecognized -> ``request_changes``, like the plan gate's + ``_parse_decision``): + + * ``approve`` -> phase CONF_WRITE, status ACTIVE; + * ``request_changes`` -> phase CONF_DRAFT, status ACTIVE, ``confluence_feedback`` + set to the notes, ``confluence_gate_visits`` bumped (loop back to redraft); + * ``abandon`` -> terminal FAILED with a ``failure_reason``. + """ + prior_visits = int(state.get("confluence_gate_visits", 0) or 0) # type: ignore[call-overload] + + if prior_visits >= MAX_CONFLUENCE_GATE_VISITS: + return PipelineState( + status=TaskStatus.PARKED.value, + current_phase=Phase.PARKED.value, + failure_reason=( + "confluence-gate revision ceiling reached " + f"({prior_visits}/{MAX_CONFLUENCE_GATE_VISITS} gate visits)" + ), + updated_at=_utc_now_iso(), + ) + + visits = prior_visits + 1 + thread_id = state.get("thread_id", "") + transport = state.get("transport", "") + slack_thread_ts = state.get("slack_thread_ts", "") + draft = dict(state.get("confluence_draft") or {}) # type: ignore[call-overload] + turn = _conf_gate_turn(visits) + question_id = _question_id_for(thread_id, turn) + deadline = (datetime.now(timezone.utc) + _DEFAULT_CONF_DEADLINE).isoformat() + preview = _draft_preview(draft) + question_set = _conf_approval_question_set( + thread_id=thread_id, + question_id=question_id, + turn=turn, + preview=preview, + ) + + decision = interrupt( + { + "thread_id": thread_id, + "question_id": question_id, + "turn": turn, + "kind": CONFLUENCE_APPROVAL_KIND, + "question_set": question_set, + "transport": transport, + "deadline": deadline, + "slack_thread_ts": slack_thread_ts, + "confluence_draft": draft, + "preview": preview, + } + ) + + return _apply_conf_decision(state, decision, visits=visits) + + +def _parse_decision(decision: Any) -> tuple[str, str]: + """Normalize a resume decision into ``(verb, notes)`` (mirrors graph). + + Delegates to the transport-neutral + :func:`agent_team.decisions.normalize_decision` (``allow_abandon=True``) so + every writer fails safe at this single chokepoint: an explicit approve / + request_changes / abandon is honoured, and anything UNRECOGNIZED maps to + ``request_changes`` carrying the full reply as notes (never a silent + terminal abandon). + """ + from agent_team.decisions import normalize_decision + + result = normalize_decision(decision, allow_abandon=True) + return result["decision"], result["notes"] + + +def _apply_conf_decision( + state: PipelineState, decision: Any, *, visits: int +) -> PipelineState: + """Consume the owner's resume decision and return the routing state. + + Mirrors :func:`agent_team.graph._apply_plan_decision`. The default for an + unrecognized/empty decision is the SAFE direction (``request_changes``), + never an accidental approve and never a silent terminal abandon. + """ + verb, notes = _parse_decision(decision) + now = _utc_now_iso() + + if verb == "approve": + return PipelineState( + status=TaskStatus.ACTIVE.value, + current_phase=CONF_WRITE_PHASE, + confluence_gate_visits=visits, # type: ignore[typeddict-unknown-key] + updated_at=now, + ) + + if verb == "request_changes": + return PipelineState( + status=TaskStatus.ACTIVE.value, + current_phase=CONF_DRAFT_PHASE, + confluence_feedback=notes, # type: ignore[typeddict-unknown-key] + confluence_gate_visits=visits, # type: ignore[typeddict-unknown-key] + updated_at=now, + ) + + # Explicit abandon only (unrecognized was already mapped to request_changes). + return PipelineState( + status=TaskStatus.FAILED.value, + current_phase=Phase.PARKED.value, + confluence_gate_visits=visits, # type: ignore[typeddict-unknown-key] + failure_reason=f"confluence draft abandoned at human gate: {notes}".rstrip( + ": " + ), + updated_at=now, + ) + + +def route_after_conf_gate(state: PipelineState) -> str: + """Conditional-edge after the Confluence gate (mirrors route_after_plan_gate). + + Reads the routing state :func:`conf_gate_node` wrote on resume (or on the + ceiling-reached terminal park) and maps it to a route id: + + * status ACTIVE + phase CONF_DRAFT -> ``'revise'`` (loop back to redraft); + * status ACTIVE + phase CONF_WRITE -> ``'approve'`` (advance to the write); + * anything else (FAILED, or PARKED ceiling) -> ``'terminal'``. + """ + status = state.get("status") + phase = state.get("current_phase") + if status == TaskStatus.ACTIVE.value and phase == CONF_DRAFT_PHASE: + return "revise" + if status == TaskStatus.ACTIVE.value and phase == CONF_WRITE_PHASE: + return "approve" + return "terminal" + + +# -- Node 3: CONF_WRITE (dry-run by default; injected client). ---------------- + + +def _default_confluence_client() -> ConfluenceClient: + """Construct the real Confluence client at call time (deferred import). + + The import is deferred so this module loads cleanly before the + :mod:`agent_team.confluence` package exists. Credentials are read from the + environment inside the client at call time, never captured here. + """ + from agent_team.confluence.client import ConfluenceClient + + return ConfluenceClient() + + +def _apply_enabled(state: PipelineState, config: dict[str, Any] | None) -> bool: + """Decide whether a LIVE write may be issued (dry-run is the default). + + A live write requires an explicit apply flag — ``config['confluence_apply']`` + truthy OR the env ``AGENT_TEAM_CONFLUENCE_APPLY`` truthy — AND the gate must + have approved (the node only runs on the approve route, but this is checked + defensively). The env is read at call time, never at import. + """ + flag = False + if config is not None: + flag = _truthy(config.get("confluence_apply")) + if not flag: + flag = _truthy(os.environ.get("AGENT_TEAM_CONFLUENCE_APPLY")) + return flag + + +def conf_write_node( + state: PipelineState, + config: dict[str, Any] | None = None, + *, + client: ConfluenceClient | None = None, +) -> PipelineState: + """CONF_WRITE stage: perform (or dry-run) the Confluence page update. + + **DRY-RUN by default.** A LIVE write is issued ONLY when + :func:`_apply_enabled` is true (an explicit ``confluence_apply`` config flag + or the ``AGENT_TEAM_CONFLUENCE_APPLY`` env truthy). Otherwise the node + composes the *planned* change plus a revert diff and writes NOTHING — so the + default path tests hit touches no live network. + + Write routing: + + * If the draft carries ``mermaid_edits`` AND the target page has diagram + macros, the update is routed through + :func:`agent_team.confluence.mermaid.plan_mermaid_edits`; + * otherwise it is a storage-format body update. + + The Confluence client is INJECTED (``client``) so tests pass an in-memory + fake; the default is a real client constructed at call time (deferred import, + creds read from the environment). The Confluence package import is deferred + so this module loads before that package exists. + + Returns a **partial** :class:`PipelineState`: terminal DONE (phase + CONF_DONE) with a ``confluence_result`` dict describing what was (or would + have been) written. + """ + draft = dict(state.get("confluence_draft") or {}) # type: ignore[call-overload] + if not draft.get("title") or not draft.get("body_storage"): + raise ConfluenceWriteError( + "conf_write_node requires a confluence_draft with title + body_storage" + ) + + apply = _apply_enabled(state, config) + page_id = draft.get("page_id") + mermaid_edits = draft.get("mermaid_edits") or [] + + if not apply: + # Dry run: compose the planned change + revert diff, write NOTHING. + result: dict[str, Any] = { + "applied": False, + "dry_run": True, + "action": "update" if page_id else "create", + "page_id": page_id, + "title": draft.get("title"), + "mermaid_edits": len(mermaid_edits), + "planned_change": { + "title": draft.get("title"), + "body_storage": draft.get("body_storage"), + }, + "revert_diff": _compose_revert(state, draft), + } + return PipelineState( + status=TaskStatus.DONE.value, + current_phase=CONF_DONE_PHASE, + confluence_result=result, # type: ignore[typeddict-unknown-key] + updated_at=_utc_now_iso(), + ) + + # Live write path (only with an explicit apply flag AND prior gate approval). + conf = client if client is not None else _default_confluence_client() + try: + if mermaid_edits and _page_has_macros(conf, page_id): + outcome, applied = _apply_mermaid_edits(conf, page_id, mermaid_edits) + else: + outcome, applied = _apply_storage_update(conf, page_id, draft) + except ConfluenceWriteError: + raise + except Exception as exc: # noqa: BLE001 - normalized to a typed write error + raise ConfluenceWriteError(f"confluence write failed: {exc}") from exc + + result = { + "applied": applied, + "dry_run": False, + "action": "update" if page_id else "create", + "page_id": getattr(outcome, "page_id", None) + if not isinstance(outcome, dict) + else outcome.get("page_id", page_id), + "title": draft.get("title"), + "mermaid_edits": len(mermaid_edits), + "outcome": outcome if isinstance(outcome, dict) else _outcome_to_dict(outcome), + } + return PipelineState( + status=TaskStatus.DONE.value, + current_phase=CONF_DONE_PHASE, + confluence_result=result, # type: ignore[typeddict-unknown-key] + updated_at=_utc_now_iso(), + ) + + +def _current_page_version(client: ConfluenceClient, page_id: Any) -> int: + """Read the target page's CURRENT version number (Confluence concurrency). + + ``ConfluenceClient.update_page`` requires the CURRENT version (it derives the + new version as ``version_number + 1`` itself). A page object missing a usable + ``version.number`` is treated as version 0 so the update still issues against + a sane baseline rather than crashing. + """ + page = client.get_page(str(page_id)) + version = page.get("version") if isinstance(page, dict) else None + if isinstance(version, dict): + number = version.get("number") + if isinstance(number, int): + return number + return 0 + + +def _apply_storage_update( + client: ConfluenceClient, page_id: Any, draft: dict[str, Any] +) -> tuple[Any, bool]: + """Issue the live storage-format page update; return ``(outcome, applied)``. + + Fetches the page's CURRENT version (``update_page`` expects the current + number and bumps it internally per Confluence's optimistic-concurrency + contract) and calls ``update_page(..., apply=True)``. ``applied`` is read off + the returned :class:`PlannedPageUpdate` (``outcome.applied``) rather than + hardcoded, so a client that declines to apply is reported honestly. + """ + version_number = _current_page_version(client, page_id) if page_id else 0 + outcome = client.update_page( + page_id=page_id, + title=draft.get("title"), + body_storage=draft.get("body_storage"), + version_number=version_number, + apply=True, + ) + applied = bool(getattr(outcome, "applied", True)) + return outcome, applied + + +def _apply_mermaid_edits( + client: ConfluenceClient, page_id: Any, mermaid_edits: list[dict[str, Any]] +) -> tuple[Any, bool]: + """Plan the Mermaid ADF edits for a macro page; return ``(outcome, applied)``. + + :func:`agent_team.confluence.mermaid.plan_mermaid_edits` is PURE ADF: it takes + the parsed ADF document and a list of :class:`~agent_team.confluence.mermaid.MermaidEdit` + (``macro_key`` / ``new_source``) and never touches the client. The draft's + edit dicts (``{"mermaid": ..., "macro_id"/"anchor": ...}``) are converted to + ``MermaidEdit`` objects here. + + The Confluence client currently exposes NO ADF fetch/persist methods (only + storage-format ``get_page`` / ``update_page``). When the injected client adds + an ADF capability (``get_page_adf`` + ``update_page_adf``) this routes through + it and reports ``applied`` off that persistence; until then there is no way to + persist an ADF edit, so this raises :class:`ConfluenceWriteError` rather than + falsely recording a successful Mermaid write. + """ + from agent_team.confluence import mermaid as mermaid_mod + + edits = [ + mermaid_mod.MermaidEdit( + macro_key=str(item.get("macro_id") or item.get("anchor") or ""), + new_source=str(item.get("mermaid", "")), + ) + for item in mermaid_edits + ] + + get_adf = getattr(client, "get_page_adf", None) + put_adf = getattr(client, "update_page_adf", None) + if not callable(get_adf) or not callable(put_adf): + raise ConfluenceWriteError( + "Mermaid live apply needs ADF persistence: the Confluence client " + "lacks get_page_adf/update_page_adf (ADF-only edits cannot round-trip " + "through storage format without dropping diagram macros)." + ) + + adf = get_adf(str(page_id)) + plan = mermaid_mod.plan_mermaid_edits(adf, edits, apply=True) + if plan.skip_mermaid: + # The page turned out to carry zero Mermaid macros after all — defer to + # the caller's storage-format fallback path semantics by signalling skip. + raise ConfluenceWriteError( + "Mermaid live apply found no Mermaid macros on the page (skip_mermaid)." + ) + outcome = put_adf(str(page_id), plan.new_adf) + applied = bool(getattr(outcome, "applied", True)) + return outcome, applied + + +def _page_has_macros(client: ConfluenceClient, page_id: Any) -> bool: + """Best-effort check that the target page carries diagram macros. + + Delegates to the injected client's ``page_has_macros`` when available; a + missing capability (older client / no page id) degrades to ``False`` so the + update falls back to a plain storage-format body update rather than crashing. + """ + if not page_id: + return False + checker = getattr(client, "page_has_macros", None) + if checker is None: + return False + try: + return bool(checker(page_id)) + except Exception: # noqa: BLE001 - capability probe must never crash the write + return False + + +def _compose_revert(state: PipelineState, draft: dict[str, Any]) -> dict[str, Any]: + """Compose a revert descriptor for the dry-run record. + + Captures enough of the prior state to describe how the planned change would + be reverted — the prior page id (if updating) and a marker that the original + body is unchanged on disk (the dry run wrote nothing). Pure data; no I/O. + """ + page_id = draft.get("page_id") + return { + "page_id": page_id, + "note": ( + "dry run — no write performed; revert is a no-op. On apply, revert " + "restores the page version prior to this update." + if page_id + else "dry run — would create a new page; revert deletes it." + ), + } + + +def _outcome_to_dict(outcome: Any) -> dict[str, Any]: + """Coerce a client write-outcome object into a JSON-safe dict (best effort).""" + for attr in ("to_dict", "_asdict"): + fn = getattr(outcome, attr, None) + if callable(fn): + try: + return dict(fn()) + except Exception: # noqa: BLE001 + pass + if isinstance(outcome, dict): + return dict(outcome) + return {"repr": repr(outcome)} diff --git a/agent-team/agent_team/nodes/confluence_writer_llm.py b/agent-team/agent_team/nodes/confluence_writer_llm.py new file mode 100644 index 0000000..19fa50f --- /dev/null +++ b/agent-team/agent_team/nodes/confluence_writer_llm.py @@ -0,0 +1,384 @@ +"""Prompt-build / reply-parse split for the Confluence-writer node. + +This module is the pure, model-output-facing half of the Confluence-writer +stage — the same ``build_*_prompt`` / ``parse_*_reply`` split +:mod:`agent_team.nodes.planner` (``build_plan_prompt`` / ``parse_plan``) and +:mod:`agent_team.nodes.clarifier_llm` (``ClaudeClarifier._build_prompt`` / +``_parse``) use, so the node body (:mod:`agent_team.nodes.confluence_writer`) +stays a thin LangGraph state-transition wrapper. + +The Confluence-writer node drafts a **Confluence page update** as DATA (it never +writes to a repo, and it never posts to Confluence from the draft node — the +write is a later, dry-run-by-default node). Two source flows feed the draft: + +* **Flow A** — a direct documentation ask: the draft is built from the task + description alone (``state['task']``). +* **Flow B** — a post-build documentation update: the draft folds in the + approved ``plan``, the ``candidate_diff``, and the repo name so the page + reflects what the change actually did. + +On a ``request_changes`` loop-back the prior gate's ``confluence_feedback`` is +folded into the prompt so the redraft answers the reviewer's notes (mirrors the +planner's ``_format_review_feedback`` loop-back). + +The LLM binding is an INJECTED seam (``context_provider``), exactly like the +planner's, so the prompt assembly stays pure and unit-testable and no live model +is bound here. + +Parsing is DEFENSIVE: the model output is UNTRUSTED. A garbled reply must not +silently produce an empty page write, so :func:`parse_confluence_reply` raises +:class:`ConfluenceDraftError` when it cannot recover a usable +``{title, body_storage}`` draft, and the node decides what to do with that. +""" + +from __future__ import annotations + +import json +import re +from collections.abc import Callable, Mapping, Sequence +from typing import Any + +from agent_team.task_model import PipelineState + +# Optional context-provider callable (mirrors planner.ContextProvider): () -> str. +# When injected, its result is prepended to the Confluence-writer prompt. Default +# None = unchanged behavior so existing callers are unaffected. +ContextProvider = Callable[[], str] + +__all__ = [ + "ConfluenceDraftError", + "build_confluence_prompt", + "parse_confluence_reply", +] + + +class ConfluenceDraftError(Exception): + """Raised when the model reply cannot be parsed into a usable draft. + + Distinct from a *gate rejection* (which loops back through the human gate): + this signals the draft model output itself was empty/garbled, so the node + can fail loudly rather than advance an empty page update toward a write. + """ + + +def _task_description(state: PipelineState) -> str: + """Pull the task description out of the graph state (mirrors planner.py).""" + plan = state.get("plan") or {} + if isinstance(plan, dict): + desc = plan.get("task") or plan.get("description") + if isinstance(desc, str) and desc.strip(): + return desc.strip() + desc = state.get("task") # type: ignore[call-overload] + if isinstance(desc, str) and desc.strip(): + return desc.strip() + return "" + + +def _repo_name(state: PipelineState) -> str: + """Pull the repo name out of the graph state (Flow B), defensively. + + Looked for under a top-level ``repo`` key and, failing that, inside the plan + dict — the same conventional places the clarifier reads ``repo`` from. An + absent repo yields an empty string so the prompt section is simply omitted. + """ + repo = state.get("repo") # type: ignore[call-overload] + if isinstance(repo, str) and repo.strip(): + return repo.strip() + plan = state.get("plan") or {} + if isinstance(plan, dict): + repo = plan.get("repo") + if isinstance(repo, str) and repo.strip(): + return repo.strip() + return "" + + +def _candidate_diff(state: PipelineState) -> str: + """Pull the candidate diff (Flow B) out of the graph state, defensively.""" + diff = state.get("candidate_diff") + if isinstance(diff, str) and diff.strip(): + return diff.strip() + return "" + + +def _format_plan(plan: Any) -> str: + """Render the approved plan into prompt text (Flow B).""" + if not isinstance(plan, Mapping): + return "" + try: + return json.dumps(dict(plan), sort_keys=True, indent=2) + except (TypeError, ValueError): + return repr(plan) + + +def _truncate(text: str, limit: int) -> str: + """Truncate ``text`` to ``limit`` chars with a marker (keep prompts bounded). + + A candidate diff can be large; the draft only needs enough of it to describe + the change, so we cap it rather than blow the context budget. + """ + if len(text) <= limit: + return text + return text[:limit] + f"\n... [truncated {len(text) - limit} chars]" + + +def build_confluence_prompt( + state: PipelineState, + *, + context_provider: ContextProvider | None = None, +) -> str: + """Build the Claude prompt that drafts a Confluence page update. + + Pure string assembly over the graph state — no I/O — so the prompt shape is + directly unit-testable (mirrors :func:`planner.build_plan_prompt`). + + Source selection: + + * **Flow A** (no plan/diff) — the draft is built from ``state['task']``. + * **Flow B** (plan and/or candidate diff present) — the draft folds in the + approved plan, the candidate diff, and the repo name. + + On a ``request_changes`` loop-back the prior ``confluence_feedback`` is + folded in so the redraft addresses every note (mirrors the planner's + review-feedback loop-back). ``context_provider`` is the optional injection + seam: when set it is called once and its result prepended (and a raising + provider degrades to an empty prefix, never a crash). + """ + description = _task_description(state) + repo = _repo_name(state) + diff = _candidate_diff(state) + plan = state.get("plan") + plan_text = _format_plan(plan) + feedback = state.get("confluence_feedback") + flow_b = bool(plan_text or diff) + + sections: list[str] = [] + + if context_provider is not None: + try: + ctx = context_provider() + except Exception: # noqa: BLE001 - a context provider must never crash the draft + ctx = "" + if ctx: + sections += [ctx, ""] + + sections += [ + "You are the CONFLUENCE-WRITER stage of an agentic SDLC pipeline. " + "Inspect the repository (read-only) and draft a Confluence page update " + "that documents this work. Emit the draft as DATA only — do NOT edit, " + "create, or write any files, and do NOT post to Confluence; a later " + "human-gated node performs the actual write.", + "", + "## Task", + description or "(no task description provided)", + ] + + if repo: + sections += ["", "## Repository", repo] + + if flow_b: + if plan_text: + sections += ["", "## Approved plan (what was decided)", plan_text] + if diff: + sections += [ + "", + "## Candidate diff (what the change actually did)", + _truncate(diff, 8000), + ] + sections += [ + "", + "## Documentation intent", + "Update the Confluence page so it reflects the architecture AFTER " + "this change: new/removed/modified resources, data flow, and " + "configuration. If the page carries an architecture diagram macro, " + "describe the mermaid edits needed rather than rewriting unrelated " + "content.", + ] + else: + sections += [ + "", + "## Documentation intent", + "Draft the Confluence page update this documentation task asks for.", + ] + + if isinstance(feedback, str) and feedback.strip(): + sections += [ + "", + "## Reviewer feedback on the previous draft (address every point)", + feedback.strip(), + ] + + sections += [ + "", + "## Output format", + "Return ONLY a JSON object with keys: " + '"title" (string, the page title), ' + '"body_storage" (string, the page body in Confluence STORAGE format / ' + "XHTML), " + '"page_id" (optional string, the id of an existing page to update; omit ' + "or null to create), and " + '"mermaid_edits" (optional list of objects each with "macro_id" or ' + '"anchor" and a "mermaid" string, for architecture-diagram macro ' + "updates). Do not include prose outside the JSON. Do NOT use any " + "write/edit tools.", + ] + return "\n".join(sections) + + +# A fenced ```...``` block, if the model wrapped its JSON in Markdown. +_FENCE_RE = re.compile( + r"```(?:json)?\s*\n?(?P.*?)\n?\s*```", + flags=re.DOTALL | re.IGNORECASE, +) + + +def _strip_code_fence(text: str) -> str: + """Strip a leading/trailing Markdown code fence if present (mirrors planner).""" + fenced = re.match( + r"^\s*```(?:json)?\s*\n(?P.*?)\n?\s*```\s*$", + text, + flags=re.DOTALL | re.IGNORECASE, + ) + if fenced: + return fenced.group("body") + return text + + +def _first_brace_span(text: str) -> str | None: + """Return the first balanced ``{...}`` span in ``text`` (string-aware).""" + start = text.find("{") + if start == -1: + return None + depth = 0 + in_string = False + escaped = False + for idx in range(start, len(text)): + ch = text[idx] + if in_string: + if escaped: + escaped = False + elif ch == "\\": + escaped = True + elif ch == '"': + in_string = False + continue + if ch == '"': + in_string = True + elif ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return text[start : idx + 1] + return None + + +def _extract_json_object(text: str) -> dict[str, Any] | None: + """Extract a JSON object from UNTRUSTED model output, or ``None``. + + Tolerates the common deviations from "JSON only" (leading apology, trailing + prose, ```json fences) by trying, in order: the whole string, the fenced + block body, then the first balanced ``{...}`` span. Returns ``None`` (never + raises) when nothing parses to a JSON object. + """ + if not isinstance(text, str) or not text.strip(): + return None + candidates: list[str] = [_strip_code_fence(text).strip(), text.strip()] + fence = _FENCE_RE.search(text) + if fence: + candidates.append(fence.group("body").strip()) + span = _first_brace_span(text) + if span is not None: + candidates.append(span) + for candidate in candidates: + if not candidate: + continue + try: + parsed = json.loads(candidate) + except (json.JSONDecodeError, ValueError): + continue + if isinstance(parsed, dict): + return parsed + return None + + +def _coerce_mermaid_edits(value: Any) -> list[dict[str, Any]]: + """Coerce the optional ``mermaid_edits`` into a clean list of edit dicts. + + Each kept entry must carry a non-empty ``mermaid`` string; an anchor is kept + when present (either ``macro_id`` or ``anchor``). Anything malformed is + dropped so a garbled edit can never silently corrupt a macro update. + """ + if not isinstance(value, Sequence) or isinstance(value, (str, bytes)): + return [] + edits: list[dict[str, Any]] = [] + for item in value: + if not isinstance(item, Mapping): + continue + mermaid = item.get("mermaid") + if not isinstance(mermaid, str) or not mermaid.strip(): + continue + edit: dict[str, Any] = {"mermaid": mermaid.strip()} + macro_id = item.get("macro_id") + anchor = item.get("anchor") + if isinstance(macro_id, str) and macro_id.strip(): + edit["macro_id"] = macro_id.strip() + if isinstance(anchor, str) and anchor.strip(): + edit["anchor"] = anchor.strip() + edits.append(edit) + return edits + + +def parse_confluence_reply(text: str) -> dict[str, Any]: + """Parse the model reply into a validated ``confluence_draft`` dict. + + Returns ``{title, body_storage, page_id?, mermaid_edits?}``: + + * ``title`` — non-empty page title (required); + * ``body_storage`` — the Confluence storage-format body (required); + * ``page_id`` — included only when the model named an existing page; + * ``mermaid_edits`` — included only when at least one well-formed edit was + supplied. + + Raises :class:`ConfluenceDraftError` when the reply is empty, not a JSON + object, or missing a usable ``title`` / ``body_storage`` — a garbled draft + must fail loudly so the node never advances an empty page write. + """ + data = _extract_json_object(text or "") + if data is None: + raise ConfluenceDraftError( + "confluence draft reply was empty or not a JSON object" + ) + + title = data.get("title") + if not isinstance(title, str) or not title.strip(): + raise ConfluenceDraftError("confluence draft is missing a non-empty 'title'") + + body = data.get("body_storage") + if not isinstance(body, str) or not body.strip(): + raise ConfluenceDraftError( + "confluence draft is missing a non-empty 'body_storage'" + ) + + draft: dict[str, Any] = { + "title": title.strip(), + "body_storage": body, + } + + page_id = data.get("page_id") + if isinstance(page_id, (str, int)) and str(page_id).strip(): + page_id_str = str(page_id).strip() + # Confluence content ids are numeric. Reject anything else: a non-numeric + # value (e.g. "123?expand=foo" or "123/child/456") would, if interpolated + # into the REST path, rewrite the authenticated request — a request-path + # injection carrying the service-account credential. + if not re.fullmatch(r"[0-9]+", page_id_str): + raise ConfluenceDraftError( + "confluence draft 'page_id' must be numeric (a Confluence content id)" + ) + draft["page_id"] = page_id_str + + mermaid_edits = _coerce_mermaid_edits(data.get("mermaid_edits")) + if mermaid_edits: + draft["mermaid_edits"] = mermaid_edits + + return draft diff --git a/agent-team/agent_team/task_model.py b/agent-team/agent_team/task_model.py index a40810f..fec1884 100644 --- a/agent-team/agent_team/task_model.py +++ b/agent-team/agent_team/task_model.py @@ -63,6 +63,12 @@ class Phase(Enum): REVIEW = "review" BUILD = "build" VERIFY = "verify" + # Direct-Confluence task lane (task_kind == "confluence"): draft the page + # change, gate it for owner approve / request_changes, then write it. The + # terminals reuse the shared DONE / PARKED phases. + CONF_DRAFT = "conf_draft" + CONF_GATE = "conf_gate" + CONF_WRITE = "conf_write" PARKED = "parked" DONE = "done" @@ -129,6 +135,19 @@ class TaskRecord: # Set when a pipeline node raised during resume and the coordinator failed # the task (mirrors PipelineState.failure_reason). Empty on a healthy task. failure_reason: str = "" + # Direct-Confluence lane state (mirrors the PipelineState keys below). All + # defaulted so an existing record (no Confluence lane) round-trips unchanged. + # ``task_kind`` "" keeps the default clarify->plan->review path; "confluence" + # routes through CONF_DRAFT->CONF_GATE->CONF_WRITE. ``confluence_draft`` is the + # drafted page change awaiting the gate; ``confluence_feedback`` the owner's + # request_changes notes the redraft reads; ``confluence_gate_visits`` the + # ceiling on draft<->gate revision loops (mirrors plan_gate_visits); + # ``confluence_result`` the written-page outcome (id, version, url). + task_kind: str = "" + confluence_draft: dict[str, Any] | None = None + confluence_feedback: str = "" + confluence_gate_visits: int = 0 + confluence_result: dict[str, Any] | None = None class PipelineState(TypedDict, total=False): @@ -185,6 +204,24 @@ class PipelineState(TypedDict, total=False): # short "ExcType: message" so the failure notification can say what broke. # Absent on a healthy task. failure_reason: str + # Intake discriminator routing a task into a non-default lane. "" (default, + # absent) keeps the existing clarify->plan->review path; "confluence" routes + # a direct Confluence-documentation task through CONF_DRAFT->CONF_GATE-> + # CONF_WRITE. + task_kind: str + # The drafted Confluence page change awaiting the gate (title, body, space, + # parent, target page id, etc.). None until CONF_DRAFT produces it. + confluence_draft: dict | None + # Free-text owner feedback captured at CONF_GATE on a request_changes + # decision; CONF_DRAFT reads it to revise the draft. Empty when none. + confluence_feedback: str + # Count of CONF_GATE human-decision visits consumed for THIS task. The + # combined ceiling on confluence draft<->gate revision loops so a + # perpetually-revised page can never spin forever (mirrors plan_gate_visits). + confluence_gate_visits: int + # The result of the CONF_WRITE Confluence API call (page id, version, url). + # None until the page has been written. + confluence_result: dict | None def task_to_dict(record: TaskRecord) -> dict[str, Any]: @@ -217,6 +254,12 @@ def task_from_dict(data: dict[str, Any]) -> TaskRecord: transport=data.get("transport", ""), created_at=data.get("created_at"), updated_at=data.get("updated_at"), + failure_reason=data.get("failure_reason", ""), + task_kind=data.get("task_kind", ""), + confluence_draft=data.get("confluence_draft"), + confluence_feedback=data.get("confluence_feedback", ""), + confluence_gate_visits=data.get("confluence_gate_visits", 0), + confluence_result=data.get("confluence_result"), ) diff --git a/agent-team/tests/test_confluence_client.py b/agent-team/tests/test_confluence_client.py new file mode 100644 index 0000000..1871c28 --- /dev/null +++ b/agent-team/tests/test_confluence_client.py @@ -0,0 +1,473 @@ +"""Unit tests for agent_team.confluence.client (OAuth 2LO + basic auth + dry-run updater). + +Fully hermetic: the HTTP transport is dependency-injected with an in-memory +fake :class:`HttpClient`, so no network call, OAuth token, or live Confluence is +exercised. Credentials are injected via ``monkeypatch.setenv``/``delenv`` and +never read from a real environment. +""" + +from __future__ import annotations + +import base64 +import json +from typing import Any +from urllib import parse as _urlparse + +import pytest + +from agent_team.nodes.confluence_writer_llm import ( + ConfluenceDraftError, + parse_confluence_reply, +) +from agent_team.confluence.client import ( + ACCESSIBLE_RESOURCES_URL, + ATLASSIAN_API_BASE, + DEFAULT_OAUTH_TOKEN_URL, + ConfluenceAuth, + ConfluenceClient, + ConfluenceError, + PlannedPageUpdate, + body_diff, + init_auth, +) + +# --------------------------------------------------------------------------- # +# Fakes / fixtures +# --------------------------------------------------------------------------- # + +_CONFLUENCE_ENV_KEYS = ( + "CONFLUENCE_OAUTH_CLIENT_ID", + "CONFLUENCE_OAUTH_CLIENT_SECRET", + "CONFLUENCE_OAUTH_TOKEN_URL", + "CONFLUENCE_CLOUD_ID", + "CONFLUENCE_BASE_URL", + "CONFLUENCE_EMAIL", + "CONFLUENCE_API_TOKEN", +) + + +class FakeHttp: + """In-memory :class:`HttpClient` recording calls and returning scripted + ``(status, body)`` per verb. + + ``responses`` maps a verb to a list of scripted ``(status, body)`` tuples, + consumed in order; a single tuple is reused for every call to that verb. + """ + + def __init__( + self, + *, + get: Any = None, + post: Any = None, + put: Any = None, + ) -> None: + self._scripts: dict[str, Any] = {"get": get, "post": post, "put": put} + self.calls: list[dict[str, Any]] = [] + + def _next(self, verb: str) -> tuple[int, Any]: + script = self._scripts[verb] + if script is None: + raise AssertionError(f"unexpected {verb.upper()} call") + if isinstance(script, list): + return script.pop(0) + return script + + def get( + self, url: str, *, headers: dict[str, str] + ) -> tuple[int, dict[str, Any] | bytes]: + self.calls.append({"verb": "get", "url": url, "headers": headers}) + return self._next("get") + + def post( + self, url: str, *, headers: dict[str, str], data: bytes + ) -> tuple[int, dict[str, Any] | bytes]: + self.calls.append( + {"verb": "post", "url": url, "headers": headers, "data": data} + ) + return self._next("post") + + def put( + self, url: str, *, headers: dict[str, str], data: bytes + ) -> tuple[int, dict[str, Any] | bytes]: + self.calls.append({"verb": "put", "url": url, "headers": headers, "data": data}) + return self._next("put") + + +def _clear_confluence_env(monkeypatch: pytest.MonkeyPatch) -> None: + for key in _CONFLUENCE_ENV_KEYS: + monkeypatch.delenv(key, raising=False) + + +def _storage_page(page_id: str, version: int, value: str) -> dict[str, Any]: + return { + "id": page_id, + "title": "Existing Title", + "version": {"number": version}, + "body": {"storage": {"representation": "storage", "value": value}}, + } + + +# --------------------------------------------------------------------------- # +# OAuth 2LO path +# --------------------------------------------------------------------------- # + + +def test_oauth_token_post_and_cloud_id_from_env( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_SECRET", "secret") + monkeypatch.setenv("CONFLUENCE_CLOUD_ID", "cloud-123") + + http = FakeHttp(post=(200, {"access_token": "tok-abc"})) + auth = init_auth(http) + + assert auth.mode == "oauth" + assert auth.base == f"{ATLASSIAN_API_BASE}/cloud-123" + assert auth.base == "https://api.atlassian.com/ex/confluence/cloud-123" + assert auth.headers()["Authorization"] == "Bearer tok-abc" + + # Exactly one POST (the token request) — cloudId came from env, no GET. + (post_call,) = [c for c in http.calls if c["verb"] == "post"] + assert post_call["url"] == DEFAULT_OAUTH_TOKEN_URL + form = dict(_urlparse.parse_qsl(post_call["data"].decode("utf-8"))) + assert form["grant_type"] == "client_credentials" + assert form["client_id"] == "cid" + assert form["client_secret"] == "secret" + assert not [c for c in http.calls if c["verb"] == "get"] + + +def test_oauth_cloud_id_resolved_from_accessible_resources( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_SECRET", "secret") + monkeypatch.setenv("CONFLUENCE_BASE_URL", "https://seahaven.atlassian.net") + + resources = [ + {"id": "other-cloud", "url": "https://other.atlassian.net"}, + {"id": "match-cloud", "url": "https://seahaven.atlassian.net"}, + ] + http = FakeHttp( + post=(200, {"access_token": "tok-xyz"}), + get=(200, resources), + ) + auth = init_auth(http) + + # Site URL matches the second resource, so its id wins. + assert auth.base == f"{ATLASSIAN_API_BASE}/match-cloud" + get_call = next(c for c in http.calls if c["verb"] == "get") + assert get_call["url"] == ACCESSIBLE_RESOURCES_URL + assert get_call["headers"]["Authorization"] == "Bearer tok-xyz" + + +def test_oauth_cloud_id_falls_back_to_first_resource( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_SECRET", "secret") + # No CONFLUENCE_BASE_URL -> no site match -> first resource id wins. + + resources = [{"id": "first-cloud", "url": "https://a.atlassian.net"}] + http = FakeHttp(post=(200, {"access_token": "t"}), get=(200, resources)) + auth = init_auth(http) + assert auth.base == f"{ATLASSIAN_API_BASE}/first-cloud" + + +def test_oauth_token_request_failure_raises_skip( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_SECRET", "secret") + + http = FakeHttp(post=(401, {"error": "invalid_client"})) + with pytest.raises(ConfluenceError) as exc: + init_auth(http) + assert exc.value.status == 0 # conf_api_init-style "skip, no false alarm". + + +def test_oauth_unresolvable_cloud_id_raises_skip( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_SECRET", "secret") + + http = FakeHttp(post=(200, {"access_token": "t"}), get=(200, [])) + with pytest.raises(ConfluenceError) as exc: + init_auth(http) + assert exc.value.status == 0 + + +def test_oauth_token_url_override(monkeypatch: pytest.MonkeyPatch) -> None: + _clear_confluence_env(monkeypatch) + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_SECRET", "secret") + monkeypatch.setenv("CONFLUENCE_CLOUD_ID", "cloud-1") + monkeypatch.setenv("CONFLUENCE_OAUTH_TOKEN_URL", "https://custom.example/oauth") + + http = FakeHttp(post=(200, {"access_token": "t"})) + init_auth(http) + post_call = next(c for c in http.calls if c["verb"] == "post") + assert post_call["url"] == "https://custom.example/oauth" + + +# --------------------------------------------------------------------------- # +# Basic-auth fallback +# --------------------------------------------------------------------------- # + + +def test_basic_auth_fallback_when_oauth_absent( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + monkeypatch.setenv("CONFLUENCE_BASE_URL", "https://seahaven.atlassian.net/") + monkeypatch.setenv("CONFLUENCE_EMAIL", "adam@seahavenind.com") + monkeypatch.setenv("CONFLUENCE_API_TOKEN", "api-tok") + + http = FakeHttp() # No HTTP call expected for basic-auth resolution. + auth = init_auth(http) + + assert auth.mode == "basic" + # Trailing slash stripped. + assert auth.base == "https://seahaven.atlassian.net" + expected = base64.b64encode(b"adam@seahavenind.com:api-tok").decode("ascii") + assert auth.headers()["Authorization"] == f"Basic {expected}" + assert http.calls == [] + + +def test_oauth_wins_over_basic_when_both_present( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("CONFLUENCE_OAUTH_CLIENT_SECRET", "secret") + monkeypatch.setenv("CONFLUENCE_CLOUD_ID", "cloud-1") + monkeypatch.setenv("CONFLUENCE_BASE_URL", "https://seahaven.atlassian.net") + monkeypatch.setenv("CONFLUENCE_EMAIL", "adam@seahavenind.com") + monkeypatch.setenv("CONFLUENCE_API_TOKEN", "api-tok") + + http = FakeHttp(post=(200, {"access_token": "tok"})) + auth = init_auth(http) + assert auth.mode == "oauth" + + +# --------------------------------------------------------------------------- # +# Missing-creds path (detectable skip, no crash) +# --------------------------------------------------------------------------- # + + +def test_missing_creds_raises_skip_status_zero( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + http = FakeHttp() + with pytest.raises(ConfluenceError) as exc: + init_auth(http) + assert exc.value.status == 0 # mirrors conf_api_init returning non-zero. + assert http.calls == [] + + +def test_incomplete_basic_creds_raises_skip( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _clear_confluence_env(monkeypatch) + # Base URL + email but NO api token -> not fully configured -> skip. + monkeypatch.setenv("CONFLUENCE_BASE_URL", "https://seahaven.atlassian.net") + monkeypatch.setenv("CONFLUENCE_EMAIL", "adam@seahavenind.com") + with pytest.raises(ConfluenceError) as exc: + init_auth(FakeHttp()) + assert exc.value.status == 0 + + +def test_client_uses_injected_env_mapping() -> None: + # No monkeypatch: the explicit env mapping is read at call time, not import. + env = { + "CONFLUENCE_BASE_URL": "https://seahaven.atlassian.net", + "CONFLUENCE_EMAIL": "adam@seahavenind.com", + "CONFLUENCE_API_TOKEN": "tok", + } + page = _storage_page("100", 4, "

hi

") + http = FakeHttp(get=(200, page)) + client = ConfluenceClient(http=http, env=env) + fetched = client.get_page("100") + assert fetched["version"]["number"] == 4 + assert http.calls[0]["url"].startswith("https://seahaven.atlassian.net") + + +# --------------------------------------------------------------------------- # +# get_page +# --------------------------------------------------------------------------- # + + +def test_get_page_parses_storage_format() -> None: + page = _storage_page("12345", 7, "

Current body

") + http = FakeHttp(get=(200, page)) + auth = ConfluenceAuth( + mode="oauth", + base="https://api.atlassian.com/ex/confluence/cloud-1", + _headers={"Authorization": "Bearer t", "Accept": "application/json"}, + ) + client = ConfluenceClient(http=http, auth=auth) + + result = client.get_page("12345") + assert result["version"]["number"] == 7 + assert result["body"]["storage"]["value"] == "

Current body

" + (call,) = http.calls + assert call["url"] == ( + "https://api.atlassian.com/ex/confluence/cloud-1" + "/wiki/api/v2/pages/12345?body-format=storage" + ) + + +def test_get_page_encodes_injected_id_in_request_path() -> None: + # Defense-in-depth: a malformed (injected) id must not appear verbatim in the + # request path — it would otherwise rewrite the authenticated request + # (request-path injection carrying the service-account credential). + page = _storage_page("123", 1, "

x

") + http = FakeHttp(get=(200, page)) + auth = ConfluenceAuth(mode="basic", base="https://x.atlassian.net") + client = ConfluenceClient(http=http, auth=auth) + + injected = "123?body-format=atlas_doc_format" + client.get_page(injected) + (call,) = http.calls + # The raw injected query must NOT survive as a literal path/query segment. + assert injected not in call["url"] + # The whole id is percent-encoded into a single path segment; the "?" that + # would start a new query string is escaped to %3F and cannot inject. + assert ( + "/pages/123%3Fbody-format%3Datlas_doc_format?body-format=storage" in call["url"] + ) + + +def test_get_page_non_2xx_raises_with_status() -> None: + http = FakeHttp(get=(404, {"message": "Not Found"})) + auth = ConfluenceAuth(mode="basic", base="https://x.atlassian.net") + client = ConfluenceClient(http=http, auth=auth) + with pytest.raises(ConfluenceError) as exc: + client.get_page("999") + assert exc.value.status == 404 + assert "Not Found" in exc.value.body + + +# --------------------------------------------------------------------------- # +# update_page — dry-run (default) issues NO PUT +# --------------------------------------------------------------------------- # + + +def _basic_client(http: FakeHttp) -> ConfluenceClient: + auth = ConfluenceAuth( + mode="basic", + base="https://seahaven.atlassian.net", + _headers={"Authorization": "Basic x", "Accept": "application/json"}, + ) + return ConfluenceClient(http=http, auth=auth) + + +def test_update_page_dry_run_default_issues_no_put() -> None: + current = _storage_page("777", 3, "

old

") + http = FakeHttp(get=(200, current)) # PUT script omitted -> would assert. + client = _basic_client(http) + + planned = client.update_page("777", "New Title", "

new

", 3) + + assert isinstance(planned, PlannedPageUpdate) + assert planned.applied is False + assert planned.new_version == 4 # current + 1 + assert planned.current_version == 3 + assert planned.body_storage == "

new

" + # A unified diff against the current body shows the change. + assert "

old

" in planned.body_delta + assert "

new

" in planned.body_delta + # Only the read (GET) happened — NO PUT. + assert [c["verb"] for c in http.calls] == ["get"] + + +def test_update_page_dry_run_diff_against_empty_when_read_fails() -> None: + # get_page returns 404 -> client swallows and diffs against an empty baseline. + http = FakeHttp(get=(404, {"message": "gone"})) + client = _basic_client(http) + planned = client.update_page("404id", "T", "

brand new

", 0) + assert planned.applied is False + assert planned.new_version == 1 + assert "

brand new

" in planned.body_delta + + +# --------------------------------------------------------------------------- # +# update_page — apply=True issues the PUT +# --------------------------------------------------------------------------- # + + +def test_update_page_apply_true_issues_put() -> None: + current = _storage_page("555", 9, "

old

") + updated = _storage_page("555", 10, "

updated

") + http = FakeHttp(get=(200, current), put=(200, updated)) + client = _basic_client(http) + + result = client.update_page("555", "Title", "

updated

", 9, apply=True) + + assert result.applied is True + assert result.new_version == 10 + + put_call = next(c for c in http.calls if c["verb"] == "put") + assert put_call["url"] == "https://seahaven.atlassian.net/wiki/api/v2/pages/555" + payload = json.loads(put_call["data"].decode("utf-8")) + assert payload["id"] == "555" + assert payload["status"] == "current" + assert payload["title"] == "Title" + assert payload["version"]["number"] == 10 + assert payload["body"]["representation"] == "storage" + assert payload["body"]["value"] == "

updated

" + assert put_call["headers"]["Content-Type"] == "application/json" + + +def test_update_page_apply_true_non_2xx_put_raises() -> None: + current = _storage_page("321", 1, "

x

") + http = FakeHttp(get=(200, current), put=(409, {"message": "version conflict"})) + client = _basic_client(http) + with pytest.raises(ConfluenceError) as exc: + client.update_page("321", "T", "

y

", 1, apply=True) + assert exc.value.status == 409 + assert "version conflict" in exc.value.body + + +# --------------------------------------------------------------------------- # +# Pure planning helper +# --------------------------------------------------------------------------- # + + +def test_parse_confluence_reply_rejects_non_numeric_page_id() -> None: + # Layer-1 guard: an LLM-supplied page_id that is not a bare numeric content + # id (here carrying a request-path-injection payload) must fail the draft. + reply = json.dumps( + { + "title": "T", + "body_storage": "

b

", + "page_id": "123?body-format=atlas_doc_format", + } + ) + with pytest.raises(ConfluenceDraftError): + parse_confluence_reply(reply) + + +def test_parse_confluence_reply_accepts_numeric_page_id() -> None: + reply = json.dumps({"title": "T", "body_storage": "

b

", "page_id": "123456"}) + draft = parse_confluence_reply(reply) + assert draft["page_id"] == "123456" + + +def test_body_diff_empty_when_identical() -> None: + assert body_diff("

same

", "

same

", page_id="1") == "" + + +def test_body_diff_shows_added_and_removed() -> None: + delta = body_diff("line a\nline b\n", "line a\nline c\n", page_id="99") + assert "page/99@current" in delta + assert "page/99@planned" in delta + assert "-line b" in delta + assert "+line c" in delta diff --git a/agent-team/tests/test_confluence_coordinator.py b/agent-team/tests/test_confluence_coordinator.py new file mode 100644 index 0000000..6342ccd --- /dev/null +++ b/agent-team/tests/test_confluence_coordinator.py @@ -0,0 +1,186 @@ +"""Integration tests for the Confluence-writer sub-pipeline wired through the +COORDINATOR (Confluence Flow A — direct documentation task). + +These pin the two confirmed-defect contracts: + +* **Gate delivery (CRITICAL).** With ``confluence=True`` a task started via + :meth:`Coordinator.start_task(task_kind="confluence")` routes INTAKE straight + to CONF_DRAFT and suspends at CONF_GATE on a ``kind='confluence_approval'`` + interrupt. The coordinator MUST deliver that gate — open a durable + ``pending_questions`` row with ``kind='confluence_approval'`` AND post the + approve / request_changes / abandon presentation to the transport. (Before the + fix the gate was never delivered: the coordinator branched only on + ``PLAN_DECISION_KIND`` and then KeyError-ed / fell through, so the task + suspended forever with no human notified and no ledger row.) +* **Flag OFF = byte-identical to today (MEDIUM).** With ``confluence=False`` the + compiled graph has NO ``conf_*`` vertices and a ``task_kind="confluence"`` task + flows through the ordinary clarify path (INTAKE -> CLARIFY), so existing daemon + behaviour is unchanged when the flag is unset. + +The Confluence draft node is AGENTIC (it calls ``claude_invoke``); the billing +seam is stubbed to return a fixed JSON draft so no SDK / network is touched. +""" + +from __future__ import annotations + +import queue +from pathlib import Path +from typing import Any + +import pytest + +try: # InMemorySaver is the modern name; fall back on older langgraph. + from langgraph.checkpoint.memory import InMemorySaver as _Saver +except ImportError: # pragma: no cover - environment-dependent + from langgraph.checkpoint.memory import MemorySaver as _Saver + +from agent_team import graph as graph_mod +from agent_team.billing import ClaudeResult +from agent_team.coordinator import Coordinator +from agent_team.db.schema import connect, init_db +from agent_team.transport.base import QuestionSet, Transport + + +class FakeTransport(Transport): + """Record-only transport (mirrors tests/test_coordinator.py's fake).""" + + def __init__(self) -> None: + self.posted: list[QuestionSet] = [] + self.thread_tss: list[str | None] = [] + + def post_question( + self, + *, + thread_id: str, + question_id: str, + turn: int, + question_set: QuestionSet, + deadline: str, + thread_ts: str | None = None, + ) -> str: + self.posted.append(question_set) + self.thread_tss.append(thread_ts) + return f"fake:{question_id}" + + def parse_answer(self, raw: Any) -> tuple[str, Any, str]: + return raw["question_id"], raw["answer"], raw.get("via", "fake") + + +_DRAFT_JSON = ( + '{"title": "Agent-Team Architecture", ' + '"body_storage": "

How the daemon works.

"}' +) + + +@pytest.fixture(autouse=True) +def _stub_confluence_draft_invoke(monkeypatch: pytest.MonkeyPatch) -> None: + """Stub the agentic draft call so CONF_DRAFT returns a fixed draft (no SDK).""" + + def _fake_invoke(prompt: str, **_kw: Any) -> ClaudeResult: + return ClaudeResult(text=_DRAFT_JSON, mode="subscription") + + monkeypatch.setattr( + "agent_team.nodes.confluence_writer.claude_invoke", _fake_invoke + ) + + +def _make_coordinator( + db_path: Path, *, confluence: bool, transport: Transport | None = None +) -> Coordinator: + saver = _Saver() + return Coordinator( + db_path=db_path, + transport=transport or FakeTransport(), + build_clarify_node=lambda: graph_mod.clarify_node, + build_checkpointer=lambda _path: saver, + resume_queue=queue.Queue(), + confluence=confluence, + ) + + +@pytest.fixture() +def db_path(tmp_path: Path) -> Path: + path = tmp_path / "state" / "agent_team.sqlite" + init_db(path) + return path + + +def _open_rows(db_path: Path) -> list[dict[str, Any]]: + conn = connect(db_path) + try: + rows = conn.execute( + "SELECT * FROM pending_questions WHERE status='open'" + ).fetchall() + finally: + conn.close() + return [dict(r) for r in rows] + + +# --------------------------------------------------------------------------- # +# (a) Flag ON: confluence task routes to CONF_DRAFT and the gate is delivered. +# --------------------------------------------------------------------------- # + + +def test_confluence_flow_a_delivers_approval_gate(db_path: Path) -> None: + transport = FakeTransport() + coord = _make_coordinator(db_path, confluence=True, transport=transport) + coord.setup() + + thread_id = coord.start_task( + task_text="document the architecture", + transport_name="slack", + task_kind="confluence", + ) + assert thread_id + + # The task suspended at CONF_GATE on a confluence_approval interrupt (NOT the + # clarifier) — Flow A routed INTAKE straight to CONF_DRAFT. + pending = graph_mod.pending_question(coord.graph, thread_id=thread_id) + assert pending is not None + assert pending["kind"] == graph_mod.CONFLUENCE_APPROVAL_KIND + + # The gate was DELIVERED: a durable open ledger row with the confluence kind. + rows = _open_rows(db_path) + assert len(rows) == 1 + assert rows[0]["kind"] == graph_mod.CONFLUENCE_APPROVAL_KIND + assert rows[0]["thread_id"] == thread_id + + # AND the approve/request_changes/abandon presentation was posted to the + # transport, tagged with the confluence-approval discriminator so the Slack + # adapter renders decision buttons. + assert len(transport.posted) == 1 + assert transport.posted[0].context.get("kind") == graph_mod.CONFLUENCE_APPROVAL_KIND + + +# --------------------------------------------------------------------------- # +# (b) Flag OFF: no conf_* nodes; a confluence task uses the ordinary clarify path. +# --------------------------------------------------------------------------- # + + +def test_confluence_off_has_no_conf_nodes_and_ignores_task_kind( + db_path: Path, +) -> None: + transport = FakeTransport() + coord = _make_coordinator(db_path, confluence=False, transport=transport) + coord.setup() + + # The compiled graph has NO conf_* vertices when the flag is off. + nodes = set(coord.graph.get_graph().nodes) + assert graph_mod.CONF_DRAFT_NODE not in nodes + assert graph_mod.CONF_GATE not in nodes + assert graph_mod.CONF_WRITE_NODE not in nodes + + # A task_kind="confluence" task is harmless: it flows INTAKE -> CLARIFY and + # suspends on the ordinary clarifier question (no confluence kind). + thread_id = coord.start_task( + task_text="document the architecture", + transport_name="slack", + task_kind="confluence", + ) + pending = graph_mod.pending_question(coord.graph, thread_id=thread_id) + assert pending is not None + assert pending.get("kind") != graph_mod.CONFLUENCE_APPROVAL_KIND + + rows = _open_rows(db_path) + assert len(rows) == 1 + assert rows[0]["kind"] == "clarify" diff --git a/agent-team/tests/test_confluence_e2e.py b/agent-team/tests/test_confluence_e2e.py new file mode 100644 index 0000000..fcfa254 --- /dev/null +++ b/agent-team/tests/test_confluence_e2e.py @@ -0,0 +1,401 @@ +"""End-to-end Confluence-stage tests through a compiled graph + SQLite checkpointer. + +These drive the Confluence-writer sub-pipeline through the WHOLE graph wiring +composed by :func:`agent_team.graph.build_graph` (with ``confluence=True``), over +a REAL on-disk SQLite checkpointer so the durable suspend/resume can be proven +across a *fresh graph instance* that shares the same checkpoint DB. Only the +``billing.claude_invoke`` seam and the Confluence client are faked — no live +model, no Confluence, no network. + +Coverage: + +* **Flow A** — ``start_task(..., task_kind="confluence")`` routes INTAKE straight + to CONF_DRAFT (NOT the clarifier), suspends at CONF_GATE; an ``approve`` resume + drives CONF_WRITE -> END. The suspend persists across a brand-new graph object + sharing the checkpointer DB, and a DUPLICATE resume against the now-settled + thread is a no-op (the terminal state is unchanged). +* **Flow B** — the post-build documentation edge: when the build/verify subgraph + is wired, the verifier's approved (PR) terminus feeds CONF_DRAFT. Exercised by + graph STRUCTURE (the edge exists) rather than driving the heavy P3 stack. +* **Regression** — ``build_graph()`` with ``confluence=False`` wires NO conf_* + nodes (the node set is unchanged), proving the flag gates the whole stage. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import pytest + +from agent_team import billing +from agent_team.billing import BillingMode, ClaudeResult +from agent_team.graph import ( + CLARIFY, + CONF_DRAFT_NODE, + CONF_GATE, + CONF_WRITE_NODE, + INTAKE, + PLAN, + build_graph, + build_sqlite_checkpointer, + get_pipeline_state, + pending_question, + resume_task, + start_task, +) +from agent_team.task_model import Phase, PipelineState, TaskStatus + +# A well-formed draft the faked Claude clarifier/drafter returns; conf_draft_node +# parses this JSON into the confluence_draft dict. +_VALID_DRAFT = { + "title": "AWS Architecture Map — agent-team", + "body_storage": "

The coordinator daemon runs on the R720.

", + "page_id": "1540098", +} + + +# --------------------------------------------------------------------------- # +# Fixtures / fakes (mirror tests/test_confluence_writer.py + test_graph.py). +# --------------------------------------------------------------------------- # + + +@pytest.fixture(autouse=True) +def _restore_invoker(): + """Restore the billing module invoker after each test (mirrors siblings).""" + original = billing._invoker + yield + billing._invoker = original + + +@pytest.fixture(autouse=True) +def _clear_apply_env(monkeypatch): + """Ensure the live-write apply flag never leaks from the host environment.""" + monkeypatch.delenv("AGENT_TEAM_CONFLUENCE_APPLY", raising=False) + + +def _bind_invoker(reply: str) -> None: + """Bind a fake Claude invoker returning ``reply`` (no live model).""" + + def _fake(prompt: str, *, mode: BillingMode, **kw: Any) -> ClaudeResult: + return ClaudeResult(text=reply, mode=mode, usage={"input_tokens": 1}) + + billing.set_invoker(_fake) + + +@pytest.fixture() +def sqlite_db(tmp_path: Path) -> Path: + """A tmp SQLite checkpoint DB path (one per test).""" + return tmp_path / "state" / "checkpoints.sqlite" + + +# --------------------------------------------------------------------------- # +# Flow A — direct Confluence doc task: INTAKE -> CONF_DRAFT (skip clarify). +# --------------------------------------------------------------------------- # + + +def test_flow_a_routes_intake_to_conf_draft_and_suspends_at_gate( + sqlite_db: Path, +) -> None: + """task_kind='confluence' skips the clarifier and suspends at the CONF_GATE. + + A direct documentation task routes INTAKE straight to CONF_DRAFT (NOT + CLARIFY), drafts the page (faked Claude), and suspends on the Confluence + human gate's ``interrupt()`` — proven by the pending interrupt's + ``confluence_approval`` kind, NOT a clarifier question-set. + """ + _bind_invoker(json.dumps(_VALID_DRAFT)) + cm = build_sqlite_checkpointer(sqlite_db) + with cm as saver: + graph = build_graph(checkpointer=saver, confluence=True) + thread_id, state = start_task( + graph, transport="slack", task="document the stack", task_kind="confluence" + ) + + # Suspended at the Confluence gate, not the clarifier. + assert "__interrupt__" in state + payload = pending_question(graph, thread_id=thread_id) + assert payload is not None + # The gate payload carries the confluence-approval kind + the draft. It + # ALSO carries a ``question_set`` (the approve/request_changes/abandon + # decision surface the coordinator delivers — mirroring the plan-decision + # gate) whose context discriminator marks it as a confluence approval, so + # it is NOT mistaken for a clarifier question-set. + assert payload["kind"] == "confluence_approval" + assert payload["question_set"].context.get("kind") == "confluence_approval" + assert payload["confluence_draft"]["title"] == _VALID_DRAFT["title"] + + # The draft was reached WITHOUT visiting the clarifier (no qa_history). + live = get_pipeline_state(graph, thread_id=thread_id) + assert live.get("qa_history", []) == [] + assert live["task_kind"] == "confluence" + + +def test_flow_a_default_kind_still_routes_to_clarify(sqlite_db: Path) -> None: + """The default (empty) task_kind still goes to the clarifier, not CONF_DRAFT. + + Proves route_after_intake only diverts ``confluence`` tasks — every other + task keeps the original INTAKE -> CLARIFY behaviour even with the flag on. + """ + _bind_invoker(json.dumps(_VALID_DRAFT)) + cm = build_sqlite_checkpointer(sqlite_db) + with cm as saver: + graph = build_graph(checkpointer=saver, confluence=True) + thread_id, state = start_task(graph, transport="slack", task="do a thing") + + assert "__interrupt__" in state + payload = pending_question(graph, thread_id=thread_id) + assert payload is not None + # A clarifier suspend: a question-set, no confluence kind. + assert "question_set" in payload + assert payload.get("kind") != "confluence_approval" + + +def test_flow_a_durable_suspend_resume_across_fresh_graph(sqlite_db: Path) -> None: + """The Flow-A suspend persists across a NEW graph object sharing the DB. + + A first graph instance starts the task and suspends at the gate. A brand-new + graph instance — same on-disk SQLite checkpointer, fresh compile — resumes the + SAME thread with an ``approve`` decision, drives CONF_DRAFT -> CONF_GATE -> + CONF_WRITE -> END, and the task settles DONE (dry-run write, nothing applied). + This is the durability proof: only the shared checkpoint DB carries the state. + """ + _bind_invoker(json.dumps(_VALID_DRAFT)) + + # Instance #1: start + suspend at the gate, then drop the graph entirely. + cm1 = build_sqlite_checkpointer(sqlite_db) + with cm1 as saver1: + graph1 = build_graph(checkpointer=saver1, confluence=True) + thread_id, state1 = start_task( + graph1, transport="slack", task="doc it", task_kind="confluence" + ) + assert "__interrupt__" in state1 + assert pending_question(graph1, thread_id=thread_id) is not None + + # Instance #2: a FRESH checkpointer connection + freshly compiled graph over + # the SAME DB file. It must see the suspended thread and resume it. + cm2 = build_sqlite_checkpointer(sqlite_db) + with cm2 as saver2: + graph2 = build_graph(checkpointer=saver2, confluence=True) + + # The pending gate is visible purely from the shared checkpoint DB. + resumed_payload = pending_question(graph2, thread_id=thread_id) + assert resumed_payload is not None + assert resumed_payload["kind"] == "confluence_approval" + + final = resume_task( + graph2, thread_id=thread_id, answer={"decision": "approve", "notes": ""} + ) + + # CONF_WRITE ran (dry-run by default) and the task is terminal DONE. + assert final["status"] == TaskStatus.DONE.value + assert final["confluence_result"]["applied"] is False + assert final["confluence_result"]["dry_run"] is True + assert pending_question(graph2, thread_id=thread_id) is None + + +def test_flow_a_duplicate_resume_is_noop(sqlite_db: Path) -> None: + """A DUPLICATE approve resume against the settled thread changes nothing. + + Once the gate is approved and the task settles DONE, replaying the same + ``Command(resume=...)`` must NOT re-open the gate or mutate the terminal + state — there is no open interrupt to consume, so it is an idempotent no-op. + """ + _bind_invoker(json.dumps(_VALID_DRAFT)) + cm = build_sqlite_checkpointer(sqlite_db) + with cm as saver: + graph = build_graph(checkpointer=saver, confluence=True) + thread_id, _ = start_task( + graph, transport="slack", task="doc it", task_kind="confluence" + ) + + first = resume_task( + graph, thread_id=thread_id, answer={"decision": "approve", "notes": ""} + ) + assert first["status"] == TaskStatus.DONE.value + first_result = first["confluence_result"] + assert pending_question(graph, thread_id=thread_id) is None + + # Replay the same resume: no open gate -> no state change. + second = resume_task( + graph, thread_id=thread_id, answer={"decision": "approve", "notes": ""} + ) + assert second["status"] == TaskStatus.DONE.value + assert second["confluence_result"] == first_result + assert pending_question(graph, thread_id=thread_id) is None + + +def test_flow_a_request_changes_loops_back_to_draft(sqlite_db: Path) -> None: + """A request_changes decision loops the gate back to CONF_DRAFT (redraft). + + Proves the gate's revise route is wired CONF_GATE -> CONF_DRAFT: the task + re-drafts (faked Claude again) and re-suspends at the gate, with the human + feedback folded into state and the visit count bumped. + """ + _bind_invoker(json.dumps(_VALID_DRAFT)) + cm = build_sqlite_checkpointer(sqlite_db) + with cm as saver: + graph = build_graph(checkpointer=saver, confluence=True) + thread_id, _ = start_task( + graph, transport="slack", task="doc it", task_kind="confluence" + ) + + state = resume_task( + graph, + thread_id=thread_id, + answer={"decision": "request_changes", "notes": "tighten the intro"}, + ) + + # Looped back through CONF_DRAFT and re-suspended at the gate. + assert "__interrupt__" in state + payload = pending_question(graph, thread_id=thread_id) + assert payload is not None + assert payload["kind"] == "confluence_approval" + live = get_pipeline_state(graph, thread_id=thread_id) + assert live["confluence_feedback"] == "tighten the intro" + assert live["confluence_gate_visits"] >= 1 + + +# --------------------------------------------------------------------------- # +# Flow B — post-build doc edge: approved-build terminus feeds CONF_DRAFT. +# --------------------------------------------------------------------------- # + + +def test_flow_b_approved_build_terminus_routes_into_conf_draft( + restore_review_invoker_b, +) -> None: + """When build_verify is wired with confluence=True, the verifier's approved + (PR) terminus is repointed from END into CONF_DRAFT (Flow B). + + Exercised via graph STRUCTURE rather than driving the heavy P3 stack: we + assert the VERIFY_NODE -> CONF_DRAFT edge exists (and that without + confluence the same wiring lands VERIFY at END instead), which is the + load-bearing Flow-B claim. + """ + from agent_team.graph import VERIFY_NODE + from agent_team.nodes import review_loop + from agent_team.nodes.build_verify_subgraph import ( + make_build_node, + make_verify_node, + route_after_verify, + ) + from agent_team.nodes.verifier import VerifierConfig + + review_loop.set_review_invoker(lambda prompt, **kw: "VERDICT: APPROVE\nok") + build_verify = ( + make_build_node(diff_builder=lambda *, plan, config: "diff"), + make_verify_node(VerifierConfig(expected_run_id="r1")), + route_after_verify, + ) + + def _plan_to_review(state: PipelineState) -> PipelineState: + return PipelineState( + plan={"summary": "p"}, + current_phase=Phase.REVIEW.value, + status=TaskStatus.ACTIVE.value, + ) + + graph = build_graph( + live_plan_node=_plan_to_review, + review_node=review_loop.bind_review_node(), + route_review=review_loop.route_after_review, + build_verify=build_verify, + confluence=True, + ) + + g = graph.get_graph() + nodes = set(g.nodes) + assert {VERIFY_NODE, CONF_DRAFT_NODE, CONF_GATE, CONF_WRITE_NODE} <= nodes + + edges = {(e.source, e.target) for e in g.edges} + # Flow B: the verifier's approved terminus feeds the Confluence draft so a + # shipped change documents itself before the graph ends. + assert (VERIFY_NODE, CONF_DRAFT_NODE) in edges + + +def test_flow_b_without_confluence_verify_terminus_is_not_conf_draft( + restore_review_invoker_b, +) -> None: + """With confluence=False, the same P3 wiring lands VERIFY at END, not the + Confluence draft — the Flow-B repoint is gated on the flag.""" + from agent_team.graph import VERIFY_NODE + from agent_team.nodes import review_loop + from agent_team.nodes.build_verify_subgraph import ( + make_build_node, + make_verify_node, + route_after_verify, + ) + from agent_team.nodes.verifier import VerifierConfig + + review_loop.set_review_invoker(lambda prompt, **kw: "VERDICT: APPROVE\nok") + build_verify = ( + make_build_node(diff_builder=lambda *, plan, config: "diff"), + make_verify_node(VerifierConfig(expected_run_id="r1")), + route_after_verify, + ) + + def _plan_to_review(state: PipelineState) -> PipelineState: + return PipelineState( + plan={"summary": "p"}, + current_phase=Phase.REVIEW.value, + status=TaskStatus.ACTIVE.value, + ) + + graph = build_graph( + live_plan_node=_plan_to_review, + review_node=review_loop.bind_review_node(), + route_review=review_loop.route_after_review, + build_verify=build_verify, + confluence=False, + ) + + g = graph.get_graph() + nodes = set(g.nodes) + # No Confluence nodes exist, so the approved terminus cannot feed CONF_DRAFT. + assert CONF_DRAFT_NODE not in nodes + edges = {(e.source, e.target) for e in g.edges} + assert (VERIFY_NODE, CONF_DRAFT_NODE) not in edges + + +@pytest.fixture() +def restore_review_invoker_b(): + """Save/restore the review-loop module-global invoker around Flow-B tests.""" + from agent_team.nodes import review_loop + + saved = review_loop._review_invoker + yield + review_loop._review_invoker = saved + + +# --------------------------------------------------------------------------- # +# Regression — confluence=False wires NO conf_* nodes (flag-gating). +# --------------------------------------------------------------------------- # + + +def test_confluence_disabled_wires_no_conf_nodes() -> None: + """build_graph() with confluence off (the default) has NO conf_* vertices. + + The node set is byte-for-byte the P1 set (intake/clarify/plan); none of the + Confluence stage exists, proving the entire sub-pipeline is flag-gated. + """ + graph = build_graph() + nodes = set(graph.get_graph().nodes) + + # The plain P1 nodes are present. + assert {INTAKE, CLARIFY, PLAN} <= nodes + # NONE of the Confluence vertices are wired. + assert CONF_DRAFT_NODE not in nodes + assert CONF_GATE not in nodes + assert CONF_WRITE_NODE not in nodes + assert not ({CONF_DRAFT_NODE, CONF_GATE, CONF_WRITE_NODE} & nodes) + + +def test_confluence_enabled_adds_exactly_the_conf_nodes() -> None: + """Turning the flag on adds precisely the three Confluence vertices to the + otherwise-unchanged P1 node set (no other topology drift).""" + base_nodes = set(build_graph().get_graph().nodes) + conf_nodes = set(build_graph(confluence=True).get_graph().nodes) + + added = conf_nodes - base_nodes + assert added == {CONF_DRAFT_NODE, CONF_GATE, CONF_WRITE_NODE} diff --git a/agent-team/tests/test_confluence_mermaid.py b/agent-team/tests/test_confluence_mermaid.py new file mode 100644 index 0000000..d8612d6 --- /dev/null +++ b/agent-team/tests/test_confluence_mermaid.py @@ -0,0 +1,182 @@ +"""Unit tests for agent_team.confluence.mermaid (ADF-only Mermaid editor, D7). + +Pure in-memory module: no seams, no network, no Claude. Fixtures are small +inline ADF dicts. The load-bearing assertions are the page-1540098 safety +invariants documented in the module: + +* a NO-OP edit set yields an EMPTY revert-diff and mutates nothing; +* a real edit yields a non-empty revert-diff and an edited ``new_adf`` while + leaving the caller's inputs untouched (dry-run default); +* a zero-macro document signals ``skip_mermaid``. +""" + +from __future__ import annotations + +import copy + +from agent_team.confluence.mermaid import ( + MermaidEdit, + count_mermaid_macros, + plan_mermaid_edits, +) + + +def _mermaid_macro(local_id: str, source: str) -> dict: + """A weweave mermaid-cloud extension node carrying ``source``.""" + return { + "type": "extension", + "attrs": { + "extensionType": "com.weweave.mermaid", + "extensionKey": "mermaid-cloud", + "localId": local_id, + "parameters": {"macroParams": {"code": {"value": source}}}, + }, + } + + +def _doc(*macros: dict) -> dict: + """An ADF doc wrapping ``macros`` with some non-mermaid filler content.""" + return { + "version": 1, + "type": "doc", + "content": [ + { + "type": "paragraph", + "content": [{"type": "text", "text": "intro"}], + }, + *macros, + ], + } + + +# --- count_mermaid_macros ------------------------------------------------- + + +def test_count_returns_n_for_n_macros() -> None: + doc = _doc( + _mermaid_macro("a", "graph TD; A-->B"), + _mermaid_macro("b", "graph TD; C-->D"), + _mermaid_macro("c", "graph TD; E-->F"), + ) + assert count_mermaid_macros(doc) == 3 + + +def test_count_returns_zero_when_no_macros() -> None: + doc = { + "version": 1, + "type": "doc", + "content": [ + {"type": "paragraph", "content": [{"type": "text", "text": "no diagrams"}]}, + { + "type": "extension", + "attrs": { + "extensionType": "com.atlassian.confluence.macro.core", + "extensionKey": "info", + "parameters": {"macroParams": {}}, + }, + }, + ], + } + assert count_mermaid_macros(doc) == 0 + + +# --- NO-OP safety invariant (page 1540098) -------------------------------- + + +def test_noop_empty_edits_yields_empty_revert_and_no_mutation() -> None: + doc = _doc(_mermaid_macro("a", "graph TD; A-->B")) + before = copy.deepcopy(doc) + + result = plan_mermaid_edits(doc, []) + + # EMPTY revert-diff is the safety invariant. + assert result.revert_diff == [] + assert result.changed is False + assert result.skip_mermaid is False + assert result.macro_count == 1 + # Writes nothing: the caller's input is byte-for-byte unchanged. + assert doc == before + + +def test_noop_identical_source_edit_yields_empty_revert() -> None: + doc = _doc(_mermaid_macro("a", "graph TD; A-->B")) + before = copy.deepcopy(doc) + + # An edit whose new_source equals the current source is a no-op. + result = plan_mermaid_edits( + doc, [MermaidEdit(macro_key="a", new_source="graph TD; A-->B")] + ) + + assert result.revert_diff == [] + assert result.changed is False + assert result.unmatched_keys == [] + assert doc == before + + +# --- real edit -> non-empty revert-diff, edited new_adf, inputs untouched -- + + +def test_real_edit_produces_revert_diff_and_edited_new_adf() -> None: + doc = _doc(_mermaid_macro("a", "graph TD; A-->B")) + doc_before = copy.deepcopy(doc) + edits = [MermaidEdit(macro_key="a", new_source="graph TD; X-->Y")] + edits_before = copy.deepcopy(edits) + + result = plan_mermaid_edits(doc, edits, apply=False) + + # Non-empty, reversible revert-diff capturing old + new. + assert result.changed is True + assert len(result.revert_diff) == 1 + op = result.revert_diff[0] + assert op.macro_key == "a" + assert op.old_source == "graph TD; A-->B" + assert op.new_source == "graph TD; X-->Y" + assert result.unmatched_keys == [] + + # new_adf reflects the change; macro count is preserved. + new_macro = result.new_adf["content"][1] + assert ( + new_macro["attrs"]["parameters"]["macroParams"]["code"]["value"] + == "graph TD; X-->Y" + ) + assert count_mermaid_macros(result.new_adf) == 1 + + # DRY-RUN DEFAULT: apply=False leaves both inputs unmutated. + assert doc == doc_before + assert edits == edits_before + assert result.new_adf is not doc + + +def test_unmatched_key_is_surfaced_not_raised() -> None: + doc = _doc(_mermaid_macro("a", "graph TD; A-->B")) + before = copy.deepcopy(doc) + + result = plan_mermaid_edits(doc, [MermaidEdit(macro_key="nope", new_source="x")]) + + assert result.unmatched_keys == ["nope"] + assert result.revert_diff == [] + assert result.changed is False + assert doc == before + + +# --- zero-macro contract: skip_mermaid ------------------------------------ + + +def test_zero_macro_doc_signals_skip_mermaid() -> None: + doc = { + "version": 1, + "type": "doc", + "content": [{"type": "paragraph", "content": [{"type": "text", "text": "hi"}]}], + } + before = copy.deepcopy(doc) + + result = plan_mermaid_edits(doc, [MermaidEdit(macro_key="a", new_source="x")]) + + # Contract: zero macros -> skip_mermaid True so the caller falls back to a + # storage-format body update (D7) rather than an empty ADF edit. + assert result.skip_mermaid is True + assert result.macro_count == 0 + assert result.revert_diff == [] + # Unmatched edit keys are still surfaced. + assert result.unmatched_keys == ["a"] + assert doc == before diff --git a/agent-team/tests/test_confluence_writer.py b/agent-team/tests/test_confluence_writer.py new file mode 100644 index 0000000..49c6407 --- /dev/null +++ b/agent-team/tests/test_confluence_writer.py @@ -0,0 +1,537 @@ +"""Unit tests for agent_team.nodes.confluence_writer (CONF_DRAFT/GATE/WRITE). + +Covers the three LangGraph nodes plus the conditional-edge router: + +* :func:`conf_draft_node` — the AGENTIC draft call. Pins the PR #61 contract: + ``max_turns > 1`` and a READ-ONLY toolset (Read/Grep/Glob, NO Edit/Write/Bash). +* :func:`conf_gate_node` — the resumable human gate. Driven through a real + in-memory LangGraph (interrupt + ``Command(resume=...)``) to assert the + interrupt payload ``kind`` and the approve / request_changes / abandon / + unrecognized / ceiling routing. +* :func:`conf_write_node` — dry-run-by-default vs. apply-flag live write through + an INJECTED fake ConfluenceClient; the mermaid path routing. +* :func:`route_after_conf_gate` — approve / revise / terminal mapping. +""" + +from __future__ import annotations + +import json +from typing import Any + +import pytest + +try: # InMemorySaver is the modern name; fall back on older langgraph. + from langgraph.checkpoint.memory import InMemorySaver as _Saver +except ImportError: # pragma: no cover - environment-dependent + from langgraph.checkpoint.memory import MemorySaver as _Saver + +from langgraph.graph import END, START, StateGraph +from langgraph.types import Command + +from agent_team import billing +from agent_team.billing import BillingMode, ClaudeResult +from agent_team.nodes import confluence_writer as cw +from agent_team.nodes.confluence_writer import ( + CONFLUENCE_APPROVAL_KIND, + MAX_CONFLUENCE_GATE_VISITS, + ConfluenceWriteError, + conf_draft_node, + conf_gate_node, + conf_write_node, + route_after_conf_gate, +) +from agent_team.task_model import Phase, PipelineState, TaskStatus + +# --------------------------------------------------------------------------- # +# Fixtures / helpers +# --------------------------------------------------------------------------- # + +_VALID_DRAFT = { + "title": "AWS Architecture Map — agent-team", + "body_storage": "

The coordinator daemon runs on the R720.

", + "page_id": "1540098", +} + + +@pytest.fixture(autouse=True) +def _restore_invoker(): + """Restore the module invoker after each test (mirrors test_planner).""" + original = billing._invoker + yield + billing._invoker = original + + +@pytest.fixture(autouse=True) +def _clear_apply_env(monkeypatch): + """Ensure the apply env flag never leaks in from the host environment.""" + monkeypatch.delenv("AGENT_TEAM_CONFLUENCE_APPLY", raising=False) + + +def _bind_invoker(reply: str) -> list[dict[str, Any]]: + """Bind a fake Claude invoker returning ``reply``; capture its calls.""" + calls: list[dict[str, Any]] = [] + + def _fake(prompt: str, *, mode: BillingMode, **kw: Any) -> ClaudeResult: + calls.append({"prompt": prompt, "mode": mode, "kw": kw}) + return ClaudeResult(text=reply, mode=mode, usage={"input_tokens": 1}) + + billing.set_invoker(_fake) + return calls + + +def _state(**overrides: Any) -> PipelineState: + base: PipelineState = PipelineState( + thread_id="t-1", + status=TaskStatus.ACTIVE.value, + current_phase=cw.CONF_DRAFT_PHASE, + ) + base.update(overrides) # type: ignore[typeddict-item] + return base + + +class _FakeUpdateOutcome: + """Stand-in for the client's PlannedPageUpdate (carries ``.applied``).""" + + def __init__(self, *, page_id, applied: bool) -> None: + self.page_id = page_id + self.applied = applied + + +class FakeConfluenceClient: + """In-memory ConfluenceClient seam: records writes, no network. + + Mirrors the REAL injected-client shape :func:`conf_write_node` expects: + ``update_page(page_id, title, body_storage, version_number, *, apply=False)`` + (the live path passes ``apply=True`` and a ``version_number`` it reads off + ``get_page``), and an optional ``page_has_macros`` capability drives the + mermaid-routing branch. ``get_page`` returns the current version so the node + can derive ``version_number``. + """ + + def __init__(self, *, has_macros: bool = False, current_version: int = 7) -> None: + self.update_calls: list[dict[str, Any]] = [] + self.macro_checks: list[Any] = [] + self.get_page_calls: list[Any] = [] + self._has_macros = has_macros + self._current_version = current_version + + def get_page(self, page_id) -> dict[str, Any]: + self.get_page_calls.append(page_id) + return { + "id": page_id, + "version": {"number": self._current_version}, + "body": {"storage": {"value": "

old

"}}, + } + + def update_page( + self, *, page_id, title, body_storage, version_number, apply=False + ) -> _FakeUpdateOutcome: + self.update_calls.append( + { + "page_id": page_id, + "title": title, + "body_storage": body_storage, + "version_number": version_number, + "apply": apply, + } + ) + return _FakeUpdateOutcome(page_id=page_id or "new-123", applied=apply) + + def page_has_macros(self, page_id) -> bool: + self.macro_checks.append(page_id) + return self._has_macros + + +class FakeAdfConfluenceClient(FakeConfluenceClient): + """Fake client that ALSO exposes the ADF seam the mermaid live path needs. + + ``get_page_adf`` returns a parsed ADF doc; ``update_page_adf`` records the + persisted doc and returns an outcome carrying ``.applied`` so the mermaid + branch reports its applied state honestly. + """ + + def __init__(self, *, adf: dict[str, Any] | None = None, **kw: Any) -> None: + super().__init__(**kw) + self.adf_get_calls: list[Any] = [] + self.adf_put_calls: list[dict[str, Any]] = [] + self._adf = adf or {"type": "doc", "version": 1, "content": []} + + def get_page_adf(self, page_id) -> dict[str, Any]: + self.adf_get_calls.append(page_id) + return self._adf + + def update_page_adf(self, page_id, new_adf) -> _FakeUpdateOutcome: + self.adf_put_calls.append({"page_id": page_id, "new_adf": new_adf}) + return _FakeUpdateOutcome(page_id=page_id, applied=True) + + +# A minimal single-node graph wrapping conf_gate_node so interrupt()/resume work +# against a real checkpointer (mirrors the e2e harness, lighter). +def _gate_graph(): + builder: StateGraph = StateGraph(PipelineState) + builder.add_node("gate", conf_gate_node) + builder.add_edge(START, "gate") + builder.add_edge("gate", END) + return builder.compile(checkpointer=_Saver()) + + +def _run_to_interrupt(graph, state: PipelineState, thread_id: str = "t-1") -> dict: + """Invoke the gate graph; return the single interrupt payload.""" + cfg = {"configurable": {"thread_id": thread_id}} + result = graph.invoke(state, cfg) + interrupts = result["__interrupt__"] + assert len(interrupts) == 1 + return interrupts[0].value + + +def _resume(graph, decision: Any, thread_id: str = "t-1") -> dict: + """Resume the suspended gate graph with ``decision``; return final state.""" + cfg = {"configurable": {"thread_id": thread_id}} + return graph.invoke(Command(resume=decision), cfg) + + +# --------------------------------------------------------------------------- # +# conf_draft_node — agentic-invoker contract (PR #61) +# --------------------------------------------------------------------------- # + + +def test_conf_draft_node_builds_draft_and_advances_to_gate() -> None: + _bind_invoker(json.dumps(_VALID_DRAFT)) + out = conf_draft_node(_state(task="document the agent-team stack")) + assert out["current_phase"] == cw.CONF_GATE_PHASE + assert out["status"] == TaskStatus.ACTIVE.value + assert out["confluence_draft"]["title"] == _VALID_DRAFT["title"] + assert out["confluence_draft"]["page_id"] == "1540098" + + +def test_conf_draft_node_passes_agentic_max_turns() -> None: + # PR #61: single-shot defaults die with "Reached maximum number of turns (1)"; + # the agentic draft call must request real turn headroom (> 1). + calls = _bind_invoker(json.dumps(_VALID_DRAFT)) + conf_draft_node(_state(task="x")) + assert calls[0]["kw"].get("max_turns", 1) > 1 + + +def test_conf_draft_node_grants_only_read_only_tools() -> None: + # The draft emits DATA and writes nothing, so it gets a READ-ONLY toolset: + # Read/Grep/Glob present, and NO Edit/Write/Bash (the load-bearing pin). + calls = _bind_invoker(json.dumps(_VALID_DRAFT)) + conf_draft_node(_state(task="x")) + tools = set(calls[0]["kw"].get("allowed_tools") or []) + assert {"Read", "Grep", "Glob"} <= tools + assert tools.isdisjoint({"Edit", "Write", "Bash", "MultiEdit", "NotebookEdit"}) + + +def test_conf_draft_node_passes_budget_cap() -> None: + calls = _bind_invoker(json.dumps(_VALID_DRAFT)) + conf_draft_node(_state(task="x")) + assert isinstance(calls[0]["kw"].get("budget_usd"), (int, float)) + + +def test_conf_draft_node_forwards_config_to_billing_seam() -> None: + calls = _bind_invoker(json.dumps(_VALID_DRAFT)) + conf_draft_node(_state(task="x"), config={"billing_mode": "api"}) + assert calls[0]["mode"] is BillingMode.API + + +def test_conf_draft_node_sends_task_into_prompt() -> None: + calls = _bind_invoker(json.dumps(_VALID_DRAFT)) + conf_draft_node(_state(task="UNIQUE-DOC-MARKER")) + assert "UNIQUE-DOC-MARKER" in calls[0]["prompt"] + + +def test_conf_draft_node_garbled_reply_raises() -> None: + from agent_team.nodes.confluence_writer_llm import ConfluenceDraftError + + _bind_invoker("not json at all") + with pytest.raises(ConfluenceDraftError): + conf_draft_node(_state(task="x")) + + +# --------------------------------------------------------------------------- # +# conf_gate_node — interrupt payload + resume routing (real graph) +# --------------------------------------------------------------------------- # + + +def test_conf_gate_interrupts_with_confluence_kind() -> None: + graph = _gate_graph() + payload = _run_to_interrupt(graph, _state(confluence_draft=dict(_VALID_DRAFT))) + assert payload["kind"] == CONFLUENCE_APPROVAL_KIND + assert payload["kind"] == "confluence_approval" + assert payload["thread_id"] == "t-1" + assert payload["confluence_draft"]["title"] == _VALID_DRAFT["title"] + assert "preview" in payload and _VALID_DRAFT["title"] in payload["preview"] + # The gate turn lives in the high disjoint namespace (>= the plan-gate base). + assert payload["turn"] >= cw._CONF_GATE_TURN_BASE + # The coordinator notify path requires a question_set; it must carry the same + # ids/turn and a single decision question whose prompt is the draft preview. + qs = payload["question_set"] + assert qs.thread_id == "t-1" + assert qs.question_id == payload["question_id"] + assert qs.turn == payload["turn"] + assert len(qs.questions) == 1 + assert _VALID_DRAFT["title"] in qs.questions[0] + assert "approve" in qs.questions[0] and "abandon" in qs.questions[0] + assert qs.context["kind"] == CONFLUENCE_APPROVAL_KIND + + +def test_conf_gate_approve_routes_to_write() -> None: + graph = _gate_graph() + _run_to_interrupt(graph, _state(confluence_draft=dict(_VALID_DRAFT))) + out = _resume(graph, {"decision": "approve", "notes": ""}) + assert out["current_phase"] == cw.CONF_WRITE_PHASE + assert out["status"] == TaskStatus.ACTIVE.value + assert out["confluence_gate_visits"] == 1 + + +def test_conf_gate_request_changes_loops_to_draft_with_feedback() -> None: + graph = _gate_graph() + _run_to_interrupt(graph, _state(confluence_draft=dict(_VALID_DRAFT))) + out = _resume(graph, {"decision": "request_changes", "notes": "fix the title"}) + assert out["current_phase"] == cw.CONF_DRAFT_PHASE + assert out["status"] == TaskStatus.ACTIVE.value + assert out["confluence_feedback"] == "fix the title" + assert out["confluence_gate_visits"] == 1 + + +def test_conf_gate_abandon_fails_with_reason() -> None: + graph = _gate_graph() + _run_to_interrupt(graph, _state(confluence_draft=dict(_VALID_DRAFT))) + out = _resume(graph, {"decision": "abandon", "notes": "drop it"}) + assert out["status"] == TaskStatus.FAILED.value + assert out["current_phase"] == Phase.PARKED.value + assert "abandoned" in (out.get("failure_reason") or "") + + +def test_conf_gate_unrecognized_decision_maps_to_request_changes() -> None: + # Free-text prose (no recognized verb) must fail SAFE to request_changes — + # never an accidental approve or silent abandon (mirrors the plan gate). + graph = _gate_graph() + _run_to_interrupt(graph, _state(confluence_draft=dict(_VALID_DRAFT))) + out = _resume(graph, "please tighten the architecture section") + assert out["current_phase"] == cw.CONF_DRAFT_PHASE + assert out["status"] == TaskStatus.ACTIVE.value + assert "tighten the architecture section" in out["confluence_feedback"] + + +def test_conf_gate_ceiling_parks_without_interrupt() -> None: + # At the visit ceiling the gate does NOT interrupt: it returns terminal PARKED + # so the human loop always terminates. + out = conf_gate_node( + _state( + confluence_draft=dict(_VALID_DRAFT), + confluence_gate_visits=MAX_CONFLUENCE_GATE_VISITS, + ) + ) + assert out["status"] == TaskStatus.PARKED.value + assert out["current_phase"] == Phase.PARKED.value + assert "ceiling" in (out.get("failure_reason") or "") + + +def test_conf_gate_visits_bump_monotonically_across_loops() -> None: + graph = _gate_graph() + _run_to_interrupt( + graph, + _state(confluence_draft=dict(_VALID_DRAFT), confluence_gate_visits=1), + thread_id="t-loop", + ) + out = _resume(graph, {"decision": "approve"}, thread_id="t-loop") + assert out["confluence_gate_visits"] == 2 + + +# --------------------------------------------------------------------------- # +# route_after_conf_gate +# --------------------------------------------------------------------------- # + + +def test_route_after_conf_gate_approve() -> None: + state = _state(status=TaskStatus.ACTIVE.value, current_phase=cw.CONF_WRITE_PHASE) + assert route_after_conf_gate(state) == "approve" + + +def test_route_after_conf_gate_revise() -> None: + state = _state(status=TaskStatus.ACTIVE.value, current_phase=cw.CONF_DRAFT_PHASE) + assert route_after_conf_gate(state) == "revise" + + +def test_route_after_conf_gate_terminal_on_failed() -> None: + state = _state(status=TaskStatus.FAILED.value, current_phase=Phase.PARKED.value) + assert route_after_conf_gate(state) == "terminal" + + +def test_route_after_conf_gate_terminal_on_parked() -> None: + state = _state(status=TaskStatus.PARKED.value, current_phase=Phase.PARKED.value) + assert route_after_conf_gate(state) == "terminal" + + +# --------------------------------------------------------------------------- # +# conf_write_node — dry-run default vs. apply flag, injected client +# --------------------------------------------------------------------------- # + + +def test_conf_write_dry_run_writes_nothing_by_default() -> None: + client = FakeConfluenceClient() + out = conf_write_node(_state(confluence_draft=dict(_VALID_DRAFT)), client=client) + assert client.update_calls == [] # NOTHING was written + assert out["status"] == TaskStatus.DONE.value + assert out["current_phase"] == cw.CONF_DONE_PHASE + result = out["confluence_result"] + assert result["applied"] is False + assert result["dry_run"] is True + assert result["action"] == "update" + assert "planned_change" in result and "revert_diff" in result + + +def test_conf_write_dry_run_create_action_when_no_page_id() -> None: + draft = {k: v for k, v in _VALID_DRAFT.items() if k != "page_id"} + out = conf_write_node(_state(confluence_draft=draft), client=FakeConfluenceClient()) + assert out["confluence_result"]["action"] == "create" + + +def test_conf_write_apply_via_config_calls_client() -> None: + client = FakeConfluenceClient() + out = conf_write_node( + _state(confluence_draft=dict(_VALID_DRAFT)), + config={"confluence_apply": True}, + client=client, + ) + assert len(client.update_calls) == 1 + call = client.update_calls[0] + assert call["page_id"] == "1540098" + # The live path MUST pass apply=True and a version_number (read off get_page); + # without both the client's update_page raises / never writes (the defect). + assert call["apply"] is True + assert call["version_number"] == 7 # FakeConfluenceClient.get_page current + assert client.get_page_calls == ["1540098"] + result = out["confluence_result"] + # ``applied`` is DERIVED from the returned outcome (.applied), not hardcoded. + assert result["applied"] is True + assert result["dry_run"] is False + assert out["status"] == TaskStatus.DONE.value + + +def test_conf_write_apply_via_env_calls_client(monkeypatch) -> None: + monkeypatch.setenv("AGENT_TEAM_CONFLUENCE_APPLY", "1") + client = FakeConfluenceClient() + conf_write_node(_state(confluence_draft=dict(_VALID_DRAFT)), client=client) + assert len(client.update_calls) == 1 + + +def test_conf_write_missing_draft_raises() -> None: + with pytest.raises(ConfluenceWriteError): + conf_write_node(_state(confluence_draft={"title": "only title"})) + + +def test_conf_write_mermaid_path_only_when_edits_and_macros(monkeypatch) -> None: + # The mermaid route fires ONLY when the draft has edits AND the page carries + # macros. plan_mermaid_edits is PURE ADF: (adf, edits, *, apply=False). We + # assert routing by patching it and checking it received the parsed ADF plus + # MermaidEdit objects converted from the draft dicts. + import agent_team.confluence.mermaid as mermaid_mod + + routed: dict[str, Any] = {} + + def _fake_plan(adf, edits=None, *, apply=False): + routed["called"] = True + routed["adf"] = adf + routed["edits"] = edits + routed["apply"] = apply + return mermaid_mod.MermaidEditResult( + macro_count=1, new_adf=adf, revert_diff=[], skip_mermaid=False + ) + + monkeypatch.setattr(mermaid_mod, "plan_mermaid_edits", _fake_plan) + + draft = dict(_VALID_DRAFT) + draft["mermaid_edits"] = [{"macro_id": "m1", "mermaid": "graph TD; A-->B"}] + + adf_doc = {"type": "doc", "version": 1, "content": ["macro"]} + client = FakeAdfConfluenceClient(has_macros=True, adf=adf_doc) + out = conf_write_node( + _state(confluence_draft=draft), + config={"confluence_apply": True}, + client=client, + ) + assert routed.get("called") is True + assert routed["apply"] is True + assert routed["adf"] is adf_doc # pure ADF fetched via get_page_adf + # draft edit dicts were converted to MermaidEdit(macro_key, new_source). + assert routed["edits"][0].macro_key == "m1" + assert routed["edits"][0].new_source == "graph TD; A-->B" + assert client.adf_get_calls == ["1540098"] + assert len(client.adf_put_calls) == 1 # persisted via update_page_adf + assert client.update_calls == [] # NOT routed through storage update_page + assert out["confluence_result"]["applied"] is True + + +def test_conf_write_mermaid_apply_without_adf_seam_raises() -> None: + # The base fake client has NO ADF persistence (get_page_adf/update_page_adf): + # a Mermaid live apply MUST fail loudly rather than falsely record success + # (ADF-only edits cannot round-trip through storage without dropping macros). + draft = dict(_VALID_DRAFT) + draft["mermaid_edits"] = [{"macro_id": "m1", "mermaid": "graph TD; A-->B"}] + client = FakeConfluenceClient(has_macros=True) + with pytest.raises(ConfluenceWriteError, match="ADF persistence"): + conf_write_node( + _state(confluence_draft=draft), + config={"confluence_apply": True}, + client=client, + ) + + +def test_conf_write_applied_reflects_outcome_not_hardcoded() -> None: + # The result's ``applied`` is DERIVED from the returned outcome, so a client + # that declines to apply is reported as applied=False (never a FALSE success). + class _DeclineClient(FakeConfluenceClient): + def update_page( + self, *, page_id, title, body_storage, version_number, apply=False + ): + self.update_calls.append({"page_id": page_id, "apply": apply}) + return _FakeUpdateOutcome(page_id=page_id, applied=False) + + out = conf_write_node( + _state(confluence_draft=dict(_VALID_DRAFT)), + config={"confluence_apply": True}, + client=_DeclineClient(), + ) + assert out["confluence_result"]["applied"] is False + assert out["confluence_result"]["dry_run"] is False + + +def test_conf_write_no_mermaid_route_when_page_lacks_macros() -> None: + # Edits present but the page has NO macros: fall back to a plain body update. + draft = dict(_VALID_DRAFT) + draft["mermaid_edits"] = [{"macro_id": "m1", "mermaid": "graph TD; A-->B"}] + + client = FakeConfluenceClient(has_macros=False) + conf_write_node( + _state(confluence_draft=draft), + config={"confluence_apply": True}, + client=client, + ) + assert len(client.update_calls) == 1 # fell back to update_page + assert client.macro_checks == ["1540098"] # the probe ran + + +def test_conf_write_no_mermaid_route_without_edits() -> None: + # No mermaid edits at all: plain body update even on a macro page. + client = FakeConfluenceClient(has_macros=True) + conf_write_node( + _state(confluence_draft=dict(_VALID_DRAFT)), + config={"confluence_apply": True}, + client=client, + ) + assert len(client.update_calls) == 1 + + +def test_conf_write_client_failure_normalized_to_write_error() -> None: + class _BoomClient(FakeConfluenceClient): + def update_page(self, **kw): + raise RuntimeError("network down") + + with pytest.raises(ConfluenceWriteError, match="confluence write failed"): + conf_write_node( + _state(confluence_draft=dict(_VALID_DRAFT)), + config={"confluence_apply": True}, + client=_BoomClient(), + ) diff --git a/agent-team/tests/test_ledger.py b/agent-team/tests/test_ledger.py index e2a2b17..b879b97 100644 --- a/agent-team/tests/test_ledger.py +++ b/agent-team/tests/test_ledger.py @@ -8,7 +8,7 @@ from pathlib import Path import pytest -from agent_team.db.schema import connect, init_db +from agent_team.db.schema import SCHEMA_VERSION, connect, init_db, migrate from agent_team.ledger import ( QUESTION_STATES, PendingQuestion, @@ -415,3 +415,350 @@ def test_concurrent_answer_single_winner_via_ledger(tmp_path: Path) -> None: t.join() assert sorted(results) == [False, True] + + +# -------------------------------------------------------------------------- +# v5 schema migration: WIDEN pending_questions.kind CHECK to add +# 'confluence_approval' via the 12-step table rebuild (db/schema.py). +# -------------------------------------------------------------------------- + + +# The OLD (pre-v5) pending_questions DDL, whose kind CHECK accepts only +# ('clarify', 'plan_decision'). Used to build a v4-stamped DB so the v4 -> v5 +# upgrade can be exercised against a realistic starting point. +_PENDING_QUESTIONS_DDL_V4 = """ +CREATE TABLE pending_questions ( + question_id TEXT PRIMARY KEY, + thread_id TEXT NOT NULL, + turn INTEGER NOT NULL, + status TEXT NOT NULL + CHECK (status IN ('open', 'answered', 'expired', 'superseded')), + transport TEXT NOT NULL, + channel_ref TEXT, + posted_at TEXT, + deadline_at TEXT, + answer_json TEXT, + answered_at TEXT, + answered_via TEXT, + kind TEXT NOT NULL DEFAULT 'clarify' + CHECK (kind IN ('clarify', 'plan_decision')) +) +""".strip() + +_PENDING_QUESTIONS_INDEXES_V4 = [ + "CREATE INDEX idx_pending_questions_thread ON pending_questions (thread_id, turn)", + "CREATE INDEX idx_pending_questions_status ON pending_questions (status)", + "CREATE UNIQUE INDEX uq_pending_questions_open_channel_ref " + "ON pending_questions (channel_ref) " + "WHERE channel_ref IS NOT NULL AND status = 'open'", +] + + +def _build_v4_db(db: Path) -> None: + """Create a DB stamped at schema_version=4 with the OLD kind CHECK. + + Mirrors the production-shaped v4 state: pending_questions carrying only + ('clarify', 'plan_decision') in its kind CHECK, all three pending_questions + indexes present, and the schema_meta version row pinned at 4. + """ + conn = connect(db) + try: + conn.execute( + "CREATE TABLE IF NOT EXISTS schema_meta (" + "id INTEGER PRIMARY KEY CHECK (id = 1), " + "schema_version INTEGER NOT NULL)" + ) + conn.execute(_PENDING_QUESTIONS_DDL_V4) + for stmt in _PENDING_QUESTIONS_INDEXES_V4: + conn.execute(stmt) + conn.execute("INSERT INTO schema_meta (id, schema_version) VALUES (1, 4)") + finally: + conn.close() + + +def _kind_check_accepts(conn: sqlite3.Connection, kind: str) -> bool: + """True if inserting a pending_questions row with ``kind`` is permitted. + + Probes the live CHECK constraint by attempting an insert and rolling it + back; an IntegrityError means the CHECK rejected the value. + """ + try: + conn.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, kind) " + "VALUES (?, 't', 0, 'open', 'slack', ?)", + (f"probe-{kind}", kind), + ) + except sqlite3.IntegrityError: + return False + conn.execute( + "DELETE FROM pending_questions WHERE question_id = ?", (f"probe-{kind}",) + ) + return True + + +def _schema_version(conn: sqlite3.Connection) -> int: + row = conn.execute("SELECT schema_version FROM schema_meta WHERE id = 1").fetchone() + return int(row["schema_version"]) + + +def _index_names(conn: sqlite3.Connection) -> set[str]: + rows = conn.execute( + "SELECT name FROM sqlite_master " + "WHERE type = 'index' AND tbl_name = 'pending_questions'" + ).fetchall() + return {row["name"] for row in rows} + + +def test_fresh_init_db_stamps_v5_and_accepts_confluence_approval( + tmp_path: Path, +) -> None: + """A fresh init_db lands at SCHEMA_VERSION and accepts 'confluence_approval'.""" + db = tmp_path / "fresh.sqlite" + init_db(db) + conn = connect(db) + try: + assert SCHEMA_VERSION == 5 + assert _schema_version(conn) == 5 + assert _kind_check_accepts(conn, "confluence_approval") is True + assert _kind_check_accepts(conn, "clarify") is True + assert _kind_check_accepts(conn, "plan_decision") is True + assert _kind_check_accepts(conn, "nope") is False + finally: + conn.close() + + +def test_v4_to_v5_preserves_rows_and_widens_kind_check(tmp_path: Path) -> None: + """v4 -> v5: existing rows survive verbatim; widened CHECK takes effect.""" + db = tmp_path / "upgrade.sqlite" + _build_v4_db(db) + + # Seed pre-existing v4 rows: a clarify and a plan_decision, with full + # column coverage so the rebuild's `INSERT ... SELECT *` copy is exercised. + seed = connect(db) + try: + assert _schema_version(seed) == 4 + seed.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, channel_ref, " + "posted_at, deadline_at, kind) " + "VALUES ('q-clar', 't1', 0, 'open', 'slack', 'ts-1', " + "'2026-06-17T01:00:00+00:00', '2026-06-17T02:00:00+00:00', 'clarify')" + ) + seed.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, answer_json, " + "answered_at, answered_via, kind) " + "VALUES ('q-plan', 't2', 1, 'answered', 'github', '{\"d\":\"go\"}', " + "'2026-06-17T03:00:00+00:00', 'github', 'plan_decision')" + ) + # Pre-rebuild the OLD CHECK must reject confluence_approval. + assert _kind_check_accepts(seed, "confluence_approval") is False + finally: + seed.close() + + # Run the migration. + mig = connect(db) + try: + migrate(mig) + finally: + mig.close() + + conn = connect(db) + try: + assert _schema_version(conn) == 5 + + # Existing rows preserved intact (every column round-trips). + clar = conn.execute( + "SELECT * FROM pending_questions WHERE question_id = 'q-clar'" + ).fetchone() + assert clar is not None + assert clar["thread_id"] == "t1" + assert clar["turn"] == 0 + assert clar["status"] == "open" + assert clar["transport"] == "slack" + assert clar["channel_ref"] == "ts-1" + assert clar["posted_at"] == "2026-06-17T01:00:00+00:00" + assert clar["deadline_at"] == "2026-06-17T02:00:00+00:00" + assert clar["kind"] == "clarify" + + plan = conn.execute( + "SELECT * FROM pending_questions WHERE question_id = 'q-plan'" + ).fetchone() + assert plan is not None + assert plan["status"] == "answered" + assert plan["transport"] == "github" + assert plan["answer_json"] == '{"d":"go"}' + assert plan["answered_at"] == "2026-06-17T03:00:00+00:00" + assert plan["answered_via"] == "github" + assert plan["kind"] == "plan_decision" + + # Widened CHECK: confluence_approval now accepted, garbage still rejected. + conn.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, kind) " + "VALUES ('q-conf', 't3', 0, 'open', 'slack', 'confluence_approval')" + ) + got = conn.execute( + "SELECT kind FROM pending_questions WHERE question_id = 'q-conf'" + ).fetchone() + assert got["kind"] == "confluence_approval" + + with pytest.raises(sqlite3.IntegrityError): + conn.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, kind) " + "VALUES ('q-bad', 't4', 0, 'open', 'slack', 'nope')" + ) + finally: + conn.close() + + +def test_v4_to_v5_rebuild_preserves_partial_unique_channel_ref_index( + tmp_path: Path, +) -> None: + """The partial unique index survives the rebuild and still enforces.""" + db = tmp_path / "uq.sqlite" + _build_v4_db(db) + + mig = connect(db) + try: + migrate(mig) + finally: + mig.close() + + conn = connect(db) + try: + # Index exists after the rebuild. + assert "uq_pending_questions_open_channel_ref" in _index_names(conn) + + # It still enforces: two OPEN rows cannot share a non-null channel_ref. + conn.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, channel_ref) " + "VALUES ('a', 't', 0, 'open', 'slack', 'dup-ts')" + ) + with pytest.raises(sqlite3.IntegrityError): + conn.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, channel_ref) " + "VALUES ('b', 't', 1, 'open', 'slack', 'dup-ts')" + ) + finally: + conn.close() + + +def test_v5_rebuild_is_atomic_failure_rolls_back_to_intact_table( + tmp_path: Path, +) -> None: + """A failure mid-rebuild must roll back: the ORIGINAL table and rows survive, + version stays 4, and a subsequent migrate() recovers cleanly (no crash-loop). + + Without the explicit BEGIN IMMEDIATE wrapping the swap, connect()'s autocommit + mode commits the DROP TABLE pending_questions on its own; a failure after that + point destroys the live ledger and leaves an orphan pending_questions_new that + crash-loops the next migrate(). We inject a failure at the index-creation step + (which runs AFTER the DROP/RENAME inside the txn) and assert the rebuild was + atomic. + """ + db = tmp_path / "atomic.sqlite" + _build_v4_db(db) + + seed = connect(db) + try: + seed.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, channel_ref, kind) " + "VALUES ('survivor', 't1', 0, 'open', 'slack', 'ts-keep', 'clarify')" + ) + finally: + seed.close() + + # Patch the index splitter the v5 block calls AFTER the DROP/RENAME so the + # rebuild fails mid-flight, exercising the rollback path. + import agent_team.db.schema as schema_mod + + original_split = schema_mod._split_statements + + def _boom(ddl: str) -> list[str]: + if "idx_pending_questions" in ddl: + raise RuntimeError("injected mid-rebuild crash") + return original_split(ddl) + + mig = connect(db) + try: + schema_mod._split_statements = _boom + try: + with pytest.raises(RuntimeError, match="injected mid-rebuild crash"): + migrate(mig) + finally: + schema_mod._split_statements = original_split + finally: + mig.close() + + # Rollback held: original table intact, the row survives, version still 4, + # and NO orphan pending_questions_new remains to crash the retry. + conn = connect(db) + try: + assert _schema_version(conn) == 4 + tables = { + row["name"] + for row in conn.execute( + "SELECT name FROM sqlite_master WHERE type = 'table'" + ).fetchall() + } + assert "pending_questions" in tables + assert "pending_questions_new" not in tables + row = conn.execute( + "SELECT thread_id, channel_ref, kind FROM pending_questions " + "WHERE question_id = 'survivor'" + ).fetchone() + assert row is not None + assert row["thread_id"] == "t1" + assert row["channel_ref"] == "ts-keep" + assert row["kind"] == "clarify" + # Pre-rebuild CHECK still in force: the widening did not partially apply. + assert _kind_check_accepts(conn, "confluence_approval") is False + finally: + conn.close() + + # Retry recovers cleanly to v5 with the row still present. + retry = connect(db) + try: + migrate(retry) + assert _schema_version(retry) == 5 + assert _kind_check_accepts(retry, "confluence_approval") is True + row = retry.execute( + "SELECT kind FROM pending_questions WHERE question_id = 'survivor'" + ).fetchone() + assert row["kind"] == "clarify" + finally: + retry.close() + + +def test_v5_migrate_is_idempotent(tmp_path: Path) -> None: + """Re-running migrate on a v5 DB is a no-op that keeps rows and version.""" + db = tmp_path / "idem.sqlite" + _build_v4_db(db) + + first = connect(db) + try: + migrate(first) + first.execute( + "INSERT INTO pending_questions " + "(question_id, thread_id, turn, status, transport, kind) " + "VALUES ('keep', 't', 0, 'open', 'slack', 'confluence_approval')" + ) + finally: + first.close() + + again = connect(db) + try: + migrate(again) + assert _schema_version(again) == 5 + row = again.execute( + "SELECT kind FROM pending_questions WHERE question_id = 'keep'" + ).fetchone() + assert row["kind"] == "confluence_approval" + finally: + again.close() diff --git a/agent-team/tests/test_task_model.py b/agent-team/tests/test_task_model.py index dbcb626..b3db8bd 100644 --- a/agent-team/tests/test_task_model.py +++ b/agent-team/tests/test_task_model.py @@ -31,6 +31,9 @@ def test_phase_members() -> None: "REVIEW", "BUILD", "VERIFY", + "CONF_DRAFT", + "CONF_GATE", + "CONF_WRITE", "PARKED", "DONE", }