From 36beae9e75fccc16dd36cbd132bbf3ae70eb4cf7 Mon Sep 17 00:00:00 2001 From: Reilly Bova Date: Fri, 11 Sep 2026 00:27:24 -0700 Subject: [PATCH 1/2] feat(announcements): add agent-scoped authoring contracts Add typed announcement drafts and the tenant-and-agent-scoped authoring client. Preserve canonical save results, ownership checks, lifecycle semantics, and indeterminate-write recovery. Keep contract tests independent of the future MCP runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eddd3818-bb74-42d3-bcf3-7e0670a57f27 --- .github/workflows/ci.yml | 25 + .../agentconfig_org_announcements/.gitignore | 12 + .../agentconfig_org_announcements/client.py | 474 ++++++++++ .../agentconfig_org_announcements/drafts.py | 519 +++++++++++ .../requirements.txt | 5 + tests/mcp/_mcp_modules.py | 9 + .../agentconfig_core/test_tenant_context.py | 4 +- .../test_authoring_client.py | 878 ++++++++++++++++++ .../test_drafts.py | 868 +++++++++++++++++ 9 files changed, 2792 insertions(+), 2 deletions(-) create mode 100644 solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/.gitignore create mode 100644 solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/client.py create mode 100644 solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/drafts.py create mode 100644 solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/requirements.txt create mode 100644 tests/mcp/agentconfig_org_announcements/test_authoring_client.py create mode 100644 tests/mcp/agentconfig_org_announcements/test_drafts.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ae6655d1..d1b04cc3b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -142,6 +142,31 @@ jobs: node extension.test.js npm run validate + org-announcements: + name: Org Announcements configuration + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: '3.11' + + - name: Install Python dependencies + run: >- + pip install + -r requirements-dev.txt + -r solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/requirements.txt + + - name: Run Org Announcements tests + run: >- + python -m pytest + tests/mcp/agentconfig_org_announcements/test_authoring_client.py + tests/mcp/agentconfig_org_announcements/test_drafts.py + -q + flightcheck-tests: name: FlightCheck offline test suite runs-on: ubuntu-latest diff --git a/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/.gitignore b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/.gitignore new file mode 100644 index 000000000..57006c3d8 --- /dev/null +++ b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/.gitignore @@ -0,0 +1,12 @@ +# Compiled Python +__pycache__/ +*.pyc + +# This server owns NO token cache of its own. It uses exactly the two shared +# delegated caches: +# - Microsoft Graph -> solutions/ess-maker-skills/.local/.token_cache.bin +# - AgentConfiguration -> src/mcp/agentconfig_core/.local/msal_token_cache.bin +# both of which are ignored by their own directories' rules. This entry is +# belt-and-braces only: if anything ever drops local state here it must not be +# committed. +.local/ diff --git a/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/client.py b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/client.py new file mode 100644 index 000000000..d7e18642c --- /dev/null +++ b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/client.py @@ -0,0 +1,474 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""WeveNova Org Announcements (EssBulletin) authoring client. + +The shared client core — bearer-token acquisition, the JWT ``tid`` decode, the +httpx session, and the retrying ``_request`` — lives in the neutral +``agentconfig_core`` core (``base_client.AgentConfigBaseClient``). This module +keeps only what is specific to the Org Announcements authoring surface: the +v1.1 base URL, the three agent-qualified ``essbulletins`` routes, and the +manager-state classification. + +The collection is keyed by the authenticated tenant and deployed ESS titleId. +This new backend surface must never fall back to tenant-only routes. +""" + +from __future__ import annotations + +import os +import sys +from datetime import datetime, timezone +from typing import Any, Optional + +import httpx + +# The AgentConfiguration MCP family lives at the ``src/mcp`` root as sibling +# folders sharing the neutral ``agentconfig_core`` client core. There is no +# package __init__.py, and each server launches with cwd set to its own folder +# on a flat sys.path, so make the sibling ``agentconfig_core`` folder importable. +sys.path.insert( + 0, + os.path.join( + os.path.dirname(os.path.abspath(__file__)), "..", "agentconfig_core" + ), +) + +from _odata import ( # noqa: E402 + _require_odata_id, + _validate_https_base_url, + _validate_title_id, +) +from agent_discovery import AgentDiscoveryClient # noqa: E402 +from base_client import AgentConfigApiError # noqa: E402 + + +DEFAULT_ORG_ANNOUNCEMENTS_BASE_URL = "https://substrate.office.com/weveb2/api/v1.1" + +# WeveNova returns every current item followed by at most this many archived +# items, with no envelope metadata. Hitting the cap means the archived window is +# truncated; it does not reveal an exact archived total. +ARCHIVED_WINDOW_SIZE = 50 + +_MAX_BULLETIN_ID_LENGTH = 256 + + +class IndeterminateWriteError(AgentConfigApiError): + """An unkeyed create may have committed but was never acknowledged. + + Raised only for writes that cannot be safely replayed: an unkeyed create or + a duplicate that failed with an ambiguous network error or a 502/503/504 + gateway response. The caller must refresh canonical state before retrying so + a committed-but-unacknowledged POST is never duplicated. + """ + + +class BulletinValidationError(AgentConfigApiError): + """The save endpoint returned HTTP 200 carrying structured field errors. + + WeveNova's ``EssBulletinSaveResult`` reports validation failure *inside* a + success response: ``errors`` is non-empty and ``config`` is absent. Every + ``{code, field, message}`` entry is preserved verbatim so the widget can + render its localized copy against the exact backend code and attach the + message to the exact field. Flattening them into one generic message would + silently destroy that mapping and leave the maker with no way to know which + field to fix. + """ + + def __init__(self, errors: list[dict[str, Any]]): + super().__init__( + "The Org Announcements service rejected the announcement.", + http_status=200, + ) + self.errors = errors + + +def _validate_bulletin_id(bulletin_id: str) -> str: + """Validate a path-bound bulletin ID. + + The ID is a backend-assigned opaque identifier, so this rejects anything + that could reshape the route (empty, padded, control characters, or a path + separator) rather than trying to canonicalize it. + """ + if not isinstance(bulletin_id, str) or not bulletin_id: + raise ValueError("bulletinId must be a non-empty string") + if bulletin_id != bulletin_id.strip(): + raise ValueError("bulletinId must not have surrounding whitespace") + if len(bulletin_id) > _MAX_BULLETIN_ID_LENGTH: + raise ValueError( + f"bulletinId must not exceed {_MAX_BULLETIN_ID_LENGTH} characters" + ) + if any( + ord(character) < 0x20 or ord(character) == 0x7F for character in bulletin_id + ): + raise ValueError("bulletinId must not contain control characters") + if "/" in bulletin_id or "\\" in bulletin_id or "?" in bulletin_id: + raise ValueError("bulletinId must not contain path or query separators") + return bulletin_id + + +def _parse_instant(value: Any) -> Optional[datetime]: + """Parse a UTC ISO instant, returning ``None`` for absent or unparseable text. + + ``None`` means "no boundary", which the classifier treats as open-ended + rather than as an error: an unscheduled published item is current. + """ + if not isinstance(value, str) or not value: + return None + text = value.strip() + if not text: + return None + if text.endswith("Z") or text.endswith("z"): + text = f"{text[:-1]}+00:00" + try: + parsed = datetime.fromisoformat(text) + except ValueError: + return None + if parsed.tzinfo is None: + return parsed.replace(tzinfo=timezone.utc) + return parsed.astimezone(timezone.utc) + + +def is_deleted_item(config: dict[str, Any]) -> bool: + """Classify one stored configuration as soft-deleted. + + ``delete`` is a lifecycle transition, not a hard removal, so the list + endpoint can still return the row. A deleted announcement is not a working + item and is not an archived item the maker can restore, so it is excluded + from the manager entirely rather than being counted in either bucket. + """ + return config.get("status") == "deleted" + + +def is_archived_item(config: dict[str, Any], now: datetime) -> bool: + """Classify one stored configuration as archived. + + Archived means Retired, or Published with an end instant already elapsed. + Everything else — Draft, and Published without an elapsed end — is current. + Position in the API response is not used, because the list carries no + envelope metadata that would make position authoritative. + """ + status = config.get("status") + if status == "retired": + return True + if status != "published": + return False + + bulletin = config.get("bulletin") + end = _parse_instant(bulletin.get("endDate")) if isinstance(bulletin, dict) else None + return end is not None and end < now + + +class OrgAnnouncementsClient(AgentDiscoveryClient): + """Async client for the tenant-and-agent-scoped ``essbulletins`` routes. + + Inherits auth, the token decode, the httpx session, and the retrying + ``_request`` from ``AgentConfigBaseClient``; adds only the v1.1 base URL and + the three authoring routes. Response bodies are already lower-camel on this + surface, so no key transform is applied. + """ + + def __init__(self, *, transport: Optional[httpx.AsyncBaseTransport] = None): + base_url = _validate_https_base_url( + os.environ.get( + "ORG_ANNOUNCEMENTS_BASE_URL", DEFAULT_ORG_ANNOUNCEMENTS_BASE_URL + ), + "ORG_ANNOUNCEMENTS_BASE_URL", + ) + super().__init__( + base_url=base_url, + logger_name="ess-org-announcements", + transport=transport, + ) + + def _collection_path(self, title_id: str) -> str: + encoded = _require_odata_id(_validate_title_id(title_id), "titleId") + return f"tenants('{self.tenant_id}')/EmployeeAgents('{encoded}')/essbulletins" + + @staticmethod + def _require_config(payload: Any, title_id: str) -> dict[str, Any]: + """Reject a success-shaped response that is not a canonical record. + + A malformed body must not become an empty default, because the widget + would then render a blank editor over real stored content. + """ + if not isinstance(payload, dict) or not isinstance( + payload.get("bulletin"), dict + ): + raise AgentConfigApiError( + "Org Announcements API returned an invalid bulletin configuration" + ) + if payload.get("titleId") != title_id: + raise AgentConfigApiError( + "Org Announcements API returned a configuration with missing " + "or mismatched titleId" + ) + if "titleId" in payload["bulletin"]: + raise AgentConfigApiError( + "Org Announcements API returned titleId inside bulletin content" + ) + return payload + + @classmethod + def _unwrap_collection(cls, payload: Any, title_id: str) -> list[dict[str, Any]]: + if isinstance(payload, list): + items = payload + elif isinstance(payload, dict) and isinstance(payload.get("value"), list): + items = payload["value"] + else: + raise AgentConfigApiError( + "Org Announcements API returned an invalid collection response" + ) + for item in items: + cls._require_config(item, title_id) + return items + + @staticmethod + def _normalize_save_errors(raw: Any) -> list[dict[str, Any]]: + """Project the wrapper's ``errors`` into ``{code, field, message}`` rows. + + Every entry is preserved — none are collapsed, deduplicated, or dropped + — because the widget maps each backend ``code`` to localized copy and + binds each ``field`` to an input. An entry that is not an object at all + is still represented (with the raw text as its message) rather than + discarded, so a schema drift surfaces as a visible error instead of a + silently successful save. + """ + normalized: list[dict[str, Any]] = [] + for entry in raw: + if isinstance(entry, dict): + code = entry.get("code") + field = entry.get("field") + message = entry.get("message") + normalized.append( + { + "code": ( + code + if isinstance(code, str) and code + else "InvalidRequest" + ), + "field": field if isinstance(field, str) and field else None, + "message": ( + message + if isinstance(message, str) and message + else "The Org Announcements service rejected this value." + ), + } + ) + else: + normalized.append( + { + "code": "InvalidRequest", + "field": None, + "message": str(entry), + } + ) + return normalized + + @classmethod + def _unwrap_save_result( + cls, payload: Any, *, requested_id: Optional[str], title_id: str + ) -> dict[str, Any]: + """Validate and unwrap an ``EssBulletinSaveResult``. + + The save endpoint answers HTTP 200 with ``{id, config, errors}`` — a + shape that list/load do *not* use, and that reports validation failure + inside a success status. Three outcomes are distinguished: + + * ``errors`` non-empty → :class:`BulletinValidationError` carrying every + entry, so field-level backend codes survive to the widget. + * ``errors`` empty and ``config`` canonical → the canonical record, + after checking the wrapper ``id`` agrees with the config's own ID and + with the ID the caller asked to update. + * anything else → :class:`AgentConfigApiError`, never an empty default: + a blank record would render an empty editor over real stored content. + + ``requested_id`` is checked because an update that silently comes back + keyed to a *different* record means the caller is about to replace its + canonical state with someone else's announcement. + """ + if not isinstance(payload, dict): + raise AgentConfigApiError( + "Org Announcements API returned an invalid save response" + ) + + errors = payload.get("errors") + if errors is not None and not isinstance(errors, list): + raise AgentConfigApiError( + "Org Announcements API returned an invalid save error list" + ) + if errors: + raise BulletinValidationError(cls._normalize_save_errors(errors)) + + config = cls._require_config(payload.get("config"), title_id) + + config_id = config["bulletin"].get("id") + wrapper_id = payload.get("id") + canonical_id = ( + wrapper_id + if isinstance(wrapper_id, str) and wrapper_id + else config_id if isinstance(config_id, str) and config_id else None + ) + if canonical_id is None: + # A saved record with no identity cannot be edited, transitioned, or + # duplicated afterwards. Failing here is far better than handing the + # widget a row whose every subsequent action would 404. + raise AgentConfigApiError( + "Org Announcements API returned a saved announcement without an id" + ) + if ( + isinstance(wrapper_id, str) + and wrapper_id + and isinstance(config_id, str) + and config_id + and wrapper_id != config_id + ): + raise AgentConfigApiError( + "Org Announcements API returned a save result whose id does not " + "match the saved configuration" + ) + if requested_id is not None and canonical_id != requested_id: + raise AgentConfigApiError( + "Org Announcements API returned a different announcement " + "than the one that was updated" + ) + return config + + async def list_bulletins(self, title_id: str) -> list[dict[str, Any]]: + """List every current item plus the most recent archived window.""" + payload = await self._request( + "GET", self._collection_path(title_id), transform_payload=False + ) + return self._unwrap_collection(payload, title_id) + + async def get_bulletin(self, title_id: str, bulletin_id: str) -> dict[str, Any]: + """Load one canonical stored configuration.""" + path = f"{self._collection_path(title_id)}/{_validate_bulletin_id(bulletin_id)}" + return self._require_config( + await self._request("GET", path, transform_payload=False), title_id + ) + + async def save_bulletin( + self, title_id: str, payload: dict[str, Any] + ) -> dict[str, Any]: + """Create or update through the authoring ``save`` endpoint. + + Returns the canonical configuration unwrapped from the endpoint's + ``EssBulletinSaveResult`` envelope. A payload without ``id`` is an + unkeyed create; replaying one after an ambiguous failure could duplicate + a committed record, so ambiguous retries are disabled and the failure is + re-raised as :class:`IndeterminateWriteError` for the caller to surface. + """ + requested_id = payload.get("id") + is_create = not requested_id + try: + result = await self._request( + "POST", + f"{self._collection_path(title_id)}/save", + json=payload, + transform_payload=False, + idempotent=not is_create, + ) + except (AgentConfigApiError, httpx.RequestError) as error: + if is_create and _is_ambiguous_write_failure(error): + raise IndeterminateWriteError( + "The announcement may have been created but the response was " + "never received. Refresh before retrying." + ) from error + raise + return self._unwrap_save_result( + result, + requested_id=requested_id if not is_create else None, + title_id=title_id, + ) + + async def transition_bulletin( + self, title_id: str, bulletin_id: str, status: str + ) -> dict[str, Any]: + """Apply a minimal lifecycle status change. + + WeveNova performs the transition inside ``SaveAsync``: it loads the + canonical record server-side, preserves authored content and audience, + validates the transition, and writes the new status. Sending only + ``{id, status}`` therefore avoids a separate read/merge/write and cannot + clobber content with stale client state. + + The response is the same ``EssBulletinSaveResult`` envelope the content + save returns, so it is unwrapped identically — including the ID check + that proves the transition landed on the requested record. + """ + validated_id = _validate_bulletin_id(bulletin_id) + return self._unwrap_save_result( + await self._request( + "POST", + f"{self._collection_path(title_id)}/save", + json={"id": validated_id, "status": status}, + transform_payload=False, + idempotent=True, + ), + requested_id=validated_id, + title_id=title_id, + ) + + +def _is_ambiguous_write_failure( + error: AgentConfigApiError | httpx.RequestError, +) -> bool: + """Decide whether a write failure leaves the commit outcome unknown. + + A transport error never reached a response, and a 502/503/504 came from an + intermediary that may have forwarded the request. A 4xx or a 500 from the + service itself is a definite rejection, so it stays a normal error. + """ + if isinstance(error, httpx.RequestError): + return True + return error.http_status in (502, 503, 504) + + +def build_manager_state( + items: list[dict[str, Any]], + audience_metadata: dict[str, list[dict[str, Any]]], + now: datetime, + *, + tenant_id: str, + title_id: str, +) -> dict[str, Any]: + """Adapt the API list into the widget's manager state. + + Soft-deleted rows are dropped before any counting: they are neither a + working item nor a restorable archived item, so including them would inflate + ``workingSetCount`` or push ``archivedTruncated`` true off records the maker + cannot see or act on. + + ``workingSetCount`` is the exact number of current items, computed from + status and schedule rather than from list position. ``archivedTruncated`` + reports only that the archived window filled, matching the widget copy + "Showing the 50 most recently archived announcements"; it never claims an + exact archived total. API item order is preserved. + """ + archived_count = 0 + working_set_count = 0 + view_models: list[dict[str, Any]] = [] + + for config in items: + if is_deleted_item(config): + continue + if is_archived_item(config, now): + archived_count += 1 + else: + working_set_count += 1 + bulletin_id = config["bulletin"].get("id", "") + view_models.append( + { + "config": config, + "audienceMetadata": audience_metadata.get(bulletin_id, []), + } + ) + + return { + "tenantId": tenant_id, + "titleId": title_id, + "items": view_models, + "workingSetCount": working_set_count, + "archivedTruncated": archived_count >= ARCHIVED_WINDOW_SIZE, + } diff --git a/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/drafts.py b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/drafts.py new file mode 100644 index 000000000..31d5ffe52 --- /dev/null +++ b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/drafts.py @@ -0,0 +1,519 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Typed Org Announcements draft and payload models. + +Two families live here: + +``SuggestedBulletinDraft`` + The *input* contract for a pre-hydrated create. It mirrors Vorpal's + ``suggestedBulletinDraftSchema`` exactly and is deliberately narrower than + the canonical record: it cannot carry a bulletin ID, a persisted lifecycle + status, creator/modifier identity, audit timestamps, or any backend version + or storage field. A suggestion is unpublished client state, never an + authorization to write. + +``AnnouncementEditorDraft`` and the ``open_org_announcements`` payloads + The *output* contract consumed by the widget. ``build_create_draft`` + overlays a validated suggestion onto the editor defaults so an omitted + field keeps the exact value the empty editor would have shown. + +Field mapping between the two families is intentional and one-directional: +suggested ``priority`` becomes ``standardPriority`` and suggested +``secondaryAction`` becomes ``standardSecondaryAction``, matching the widget's +Standard-only editor fields. +""" + +from __future__ import annotations + +import re +from copy import deepcopy +from datetime import date, datetime, timezone +from typing import Any, Literal, Optional + +from pydantic import ( + BaseModel, + ConfigDict, + ModelWrapValidatorHandler, + PrivateAttr, + model_validator, +) + + +AnnouncementType = Literal["standard", "alert"] +AnnouncementPriority = Literal[0, 1] +AnnouncementStatus = Literal["draft", "published", "retired", "deleted"] +EditorMode = Literal["create", "edit", "duplicate"] +BulletinActionType = Literal["externalLink", "copilotChat"] + +# The values the empty Vorpal editor shows before a maker types anything. An +# omitted suggested field must land on exactly these, so they are named rather +# than repeated inline. +DEFAULT_TYPE: AnnouncementType = "standard" +DEFAULT_TITLE = "" +DEFAULT_DESCRIPTION = "" +DEFAULT_PRIMARY_ACTION = None +DEFAULT_START_DATE = "" +DEFAULT_END_DATE = "" +DEFAULT_STANDARD_PRIORITY: AnnouncementPriority = 1 +DEFAULT_STANDARD_SECONDARY_ACTION = None + +# ``EssBulletinInput.startDate``/``endDate`` are nullable ``DateTimeOffset`` +# UTC instants. The widget's DatePicker exchanges instants as +# ``YYYY-MM-DDTHH:MM:SS.mmmZ``, so every accepted suggested value is normalized +# into exactly that shape before it reaches the editor draft. +_INSTANT_FORMAT = "%Y-%m-%dT%H:%M:%S.%f" +_DATE_ONLY_PATTERN = re.compile(r"^\d{4}-\d{2}-\d{2}$") + + +def _format_instant(moment: datetime) -> str: + """Render an aware datetime as a UTC instant with millisecond precision.""" + utc = moment.astimezone(timezone.utc) + # ``%f`` is microseconds; truncate (never round) so a normalized instant is + # always at or before the value the maker supplied. + return f"{utc.strftime(_INSTANT_FORMAT)[:-3]}Z" + + +def normalize_suggested_instant(value: str, *, boundary: str) -> str: + """Normalize one suggested schedule boundary to a UTC millisecond instant. + + Three inputs are accepted, and nothing else: + + * ``""`` — an explicit "unset" that the DatePicker renders as empty. It is + preserved rather than defaulted so the widget shows its own validation. + * ``YYYY-MM-DD`` — a date-only value. A start becomes the first instant of + that UTC day and an end becomes the last, so a single-day announcement + spans the whole day instead of collapsing to midnight-to-midnight. + * A timezone-aware ISO-8601 instant — converted to UTC. + + A timezone-naive datetime is rejected rather than assumed to be UTC: the + maker's local day boundary is not knowable here, and silently guessing would + schedule an announcement at the wrong time. Natural language ("next + Monday") and any other unparseable text are rejected for the same reason — + a suggestion the model invented must never become a schedule nobody + reviewed. + """ + if not isinstance(value, str): + raise ValueError(f"{boundary} must be a string") + text = value.strip() + if not text: + return "" + + if _DATE_ONLY_PATTERN.match(text): + try: + day = date.fromisoformat(text) + except ValueError as error: + raise ValueError( + f"{boundary} must be a calendar date (YYYY-MM-DD) or a UTC instant" + ) from error + moment = ( + datetime(day.year, day.month, day.day, 0, 0, 0, 0, tzinfo=timezone.utc) + if boundary == "startDate" + else datetime( + day.year, day.month, day.day, 23, 59, 59, 999000, tzinfo=timezone.utc + ) + ) + return _format_instant(moment) + + candidate = f"{text[:-1]}+00:00" if text.endswith(("Z", "z")) else text + try: + parsed = datetime.fromisoformat(candidate) + except ValueError as error: + raise ValueError( + f"{boundary} must be an empty string, a calendar date " + f"(YYYY-MM-DD), or a timezone-aware ISO-8601 instant" + ) from error + if parsed.tzinfo is None or parsed.utcoffset() is None: + raise ValueError( + f"{boundary} must carry a timezone offset; a local datetime is " + f"ambiguous and is not assumed to be UTC" + ) + return _format_instant(parsed) + + +class StrictModel(BaseModel): + """Reject any field the contract does not name.""" + + model_config = ConfigDict(extra="forbid") + + +class BulletinAction(StrictModel): + """A primary or secondary announcement action. + + ``url`` belongs to ``externalLink`` and ``prompt`` to ``copilotChat``. A + payload carrying the *wrong* target for its type is malformed at the + contract level, so it is rejected here rather than forwarded. + + A *missing or blank* target is deliberately still accepted. A Draft is + allowed to be incomplete — the maker can add the URL later — and WeveNova + owns publish-time completeness validation, so rejecting it here would make + ``save_bulletin`` refuse drafts the backend accepts and would replace the + backend's field-level error code with a generic contract error. + """ + + actionType: BulletinActionType + label: str + url: Optional[str] = None + prompt: Optional[str] = None + + @model_validator(mode="after") + def require_matching_target(self) -> "BulletinAction": + if self.actionType == "externalLink": + if self.prompt is not None: + raise ValueError("externalLink actions must not carry a prompt") + elif self.url is not None: + raise ValueError("copilotChat actions must not carry a url") + return self + + +class SuggestedBulletinAction(BulletinAction): + """An action inside a *pre-hydrated* create suggestion. + + Stricter than :class:`BulletinAction` on purpose. A stored Draft may hold a + half-finished action the maker is still editing, but a suggestion is + content the model proposed and the maker has not typed: hydrating the + editor with an ``externalLink`` that has no URL, or a ``copilotChat`` with + no prompt, presents an action that cannot work as if it were reviewed + state. The suggestion is rejected so the maker gets an explicit opener + error instead of a silently broken button. + """ + + @model_validator(mode="after") + def require_actionable_target(self) -> "SuggestedBulletinAction": + if self.actionType == "externalLink": + if not (self.url or "").strip(): + raise ValueError( + "a suggested externalLink action requires a non-blank url" + ) + elif not (self.prompt or "").strip(): + raise ValueError( + "a suggested copilotChat action requires a non-blank prompt" + ) + return self + + +class SuggestedBulletinDraft(StrictModel): + """A partial, reviewable proposal for a new announcement. + + Every field is optional; omitted fields fall back to the editor defaults in + :func:`build_create_draft`. An explicit empty string is a real draft value + and is preserved so the widget surfaces its normal validation instead of + silently substituting a default. + """ + + type: Optional[AnnouncementType] = None + priority: Optional[AnnouncementPriority] = None + title: Optional[str] = None + description: Optional[str] = None + primaryAction: Optional[SuggestedBulletinAction] = None + secondaryAction: Optional[SuggestedBulletinAction] = None + startDate: Optional[str] = None + endDate: Optional[str] = None + audience: Optional[list[str]] = None + + _retry_input: dict[str, Any] = PrivateAttr(default_factory=dict) + + @model_validator(mode="wrap") + @classmethod + def preserve_retry_input( + cls, value: Any, handler: ModelWrapValidatorHandler["SuggestedBulletinDraft"] + ) -> "SuggestedBulletinDraft": + # Keep the validated original proposal for retries; date normalization + # belongs to editor hydration, not to the maker's original request. + original = deepcopy(value) if isinstance(value, dict) else None + draft = handler(value) + if original is not None: + draft._retry_input = original + return draft + + def retry_payload(self) -> dict[str, Any]: + return deepcopy(self._retry_input) + + @model_validator(mode="after") + def normalize_schedule(self) -> "SuggestedBulletinDraft": + """Coerce every accepted schedule value into a UTC millisecond instant. + + Normalizing here — rather than in ``build_create_draft`` — means the + opener rejects an unusable suggestion before any Graph audience lookup + is issued, and the editor only ever receives instants the DatePicker + can render. + """ + if self.startDate is not None: + self.startDate = normalize_suggested_instant( + self.startDate, boundary="startDate" + ) + if self.endDate is not None: + self.endDate = normalize_suggested_instant( + self.endDate, boundary="endDate" + ) + return self + + @model_validator(mode="after") + def reject_alert_incompatible_fields(self) -> "SuggestedBulletinDraft": + """An Alert has no priority control and no secondary action. + + Only an *explicit* incompatible value is rejected. Omitted Standard + fields still receive their required editor defaults, because the widget + keeps them in the draft even while they are hidden. + """ + if self.type != "alert": + return self + if self.priority is not None: + raise ValueError("alert announcements do not support priority") + if self.secondaryAction is not None: + raise ValueError( + "alert announcements do not support a secondary action" + ) + if ( + self.primaryAction is not None + and self.primaryAction.actionType != "externalLink" + ): + raise ValueError("alert actions must be externalLink actions") + return self + + @model_validator(mode="after") + def reject_blank_audience_ids(self) -> "SuggestedBulletinDraft": + if self.audience is None: + return self + for group_id in self.audience: + if not group_id or not group_id.strip(): + raise ValueError("audience group IDs must be non-empty") + return self + + +class OpenAnnouncementsRequest(StrictModel): + """Validate flat opener arguments before they can become widget retry state.""" + + titleId: str + view: Literal["manager", "editor"] + mode: Optional[Literal["create", "edit"]] = None + bulletinId: Optional[str] = None + suggestedDraft: Optional[SuggestedBulletinDraft] = None + + @model_validator(mode="after") + def require_valid_editor_intent(self) -> "OpenAnnouncementsRequest": + if self.view == "manager": + if any( + value is not None + for value in (self.mode, self.bulletinId, self.suggestedDraft) + ): + raise ValueError("manager does not take editor arguments") + elif self.mode is None: + raise ValueError("editor requires explicit create or edit mode") + elif self.mode == "create": + if self.bulletinId is not None: + raise ValueError("create must not supply bulletinId") + elif self.bulletinId is None: + raise ValueError("edit requires bulletinId") + elif self.suggestedDraft is not None: + raise ValueError("suggestedDraft applies only to create") + return self + + +class AudienceGroup(StrictModel): + """Display metadata for one canonical audience group ID. + + ``isValid`` is false when the ID could not be resolved through Graph or + resolved to a group category this kit does not author. The ID is always + retained so the widget can require removal or replacement instead of losing + stored state. + """ + + id: str + displayName: str + mail: Optional[str] = None + isValid: bool = True + + +class AnnouncementEditorDraft(StrictModel): + """The complete editor draft the widget renders.""" + + id: Optional[str] = None + type: AnnouncementType + title: str + description: str + primaryAction: Optional[BulletinAction] = None + startDate: str + endDate: str + audience: list[AudienceGroup] + standardPriority: AnnouncementPriority + standardSecondaryAction: Optional[BulletinAction] = None + + +def build_create_draft( + suggestion: Optional[SuggestedBulletinDraft], + audience_metadata: Optional[list[AudienceGroup]] = None, +) -> AnnouncementEditorDraft: + """Overlay a validated suggestion onto the empty-editor defaults. + + ``audience_metadata`` is the resolved, order-preserving metadata for + ``suggestion.audience``; the caller resolves it because resolution needs the + Graph client. A create draft never carries an ID. + """ + if suggestion is None: + suggestion = SuggestedBulletinDraft() + + return AnnouncementEditorDraft( + type=suggestion.type if suggestion.type is not None else DEFAULT_TYPE, + title=suggestion.title if suggestion.title is not None else DEFAULT_TITLE, + description=( + suggestion.description + if suggestion.description is not None + else DEFAULT_DESCRIPTION + ), + primaryAction=( + suggestion.primaryAction + if suggestion.primaryAction is not None + else DEFAULT_PRIMARY_ACTION + ), + startDate=( + suggestion.startDate + if suggestion.startDate is not None + else DEFAULT_START_DATE + ), + endDate=( + suggestion.endDate + if suggestion.endDate is not None + else DEFAULT_END_DATE + ), + audience=list(audience_metadata or []), + standardPriority=( + suggestion.priority + if suggestion.priority is not None + else DEFAULT_STANDARD_PRIORITY + ), + standardSecondaryAction=( + suggestion.secondaryAction + if suggestion.secondaryAction is not None + else DEFAULT_STANDARD_SECONDARY_ACTION + ), + ) + + +def build_editor_draft_from_config( + config: dict[str, Any], + audience_metadata: list[AudienceGroup], +) -> AnnouncementEditorDraft: + """Project canonical content into the normal editor, preserving its schedule.""" + bulletin = config.get("bulletin") + if not isinstance(bulletin, dict): + raise ValueError("bulletin configuration is missing its bulletin content") + + bulletin_id = bulletin.get("id") + if not isinstance(bulletin_id, str) or not bulletin_id: + raise ValueError("bulletin configuration is missing its id") + + primary_action = bulletin.get("primaryAction") + secondary_action = bulletin.get("secondaryAction") + priority = bulletin.get("priority") + + return AnnouncementEditorDraft( + id=bulletin_id, + type=bulletin.get("type") or DEFAULT_TYPE, + title=bulletin.get("title") or DEFAULT_TITLE, + description=bulletin.get("description") or DEFAULT_DESCRIPTION, + primaryAction=( + BulletinAction.model_validate(primary_action) + if isinstance(primary_action, dict) + else DEFAULT_PRIMARY_ACTION + ), + startDate=bulletin.get("startDate") or DEFAULT_START_DATE, + endDate=bulletin.get("endDate") or DEFAULT_END_DATE, + audience=list(audience_metadata), + standardPriority=( + priority if priority in (0, 1) else DEFAULT_STANDARD_PRIORITY + ), + standardSecondaryAction=( + BulletinAction.model_validate(secondary_action) + if isinstance(secondary_action, dict) + else DEFAULT_STANDARD_SECONDARY_ACTION + ), + ) + + +_SCHEDULE_FIELDS = ("startDate", "endDate") + + +def is_blank_schedule_value(value: Any) -> bool: + """Report whether a schedule value is the widget's "unset" sentinel. + + The single definition of the rule, shared by the validated save path and the + duplicate path, so the two cannot drift into disagreeing about what "unset" + means. + """ + return isinstance(value, str) and not value.strip() + + +def without_blank_schedule(bulletin: dict[str, Any]) -> dict[str, Any]: + """Drop schedule keys holding a blank sentinel from a raw content dict. + + Used by the duplicate path, which forwards *stored* content verbatim and so + never passes through :class:`BulletinInput`. Without this, duplicating a + source whose schedule is blank would put ``""`` on the wire and fail the + backend's model binding, even though the equivalent ordinary save is fine. + """ + return { + key: value + for key, value in bulletin.items() + if not (key in _SCHEDULE_FIELDS and is_blank_schedule_value(value)) + } + + +class BulletinInput(StrictModel): + """The authored content a mutation sends to the authoring API. + + ``startDate``/``endDate`` map to nullable ``DateTimeOffset`` on + ``EssBulletinInput``. The widget's DatePicker represents "unset" as the + empty string, so a blank arriving here is coerced to ``None`` and then + dropped entirely by ``exclude_none=True`` at serialization. Sending ``""`` + instead would fail the backend's *model binding* — a JSON string is not a + DateTimeOffset — which surfaces as an opaque 400 rather than as the + field-level validation error the widget knows how to render, and would make + a perfectly ordinary unscheduled Draft unsaveable. + """ + + type: AnnouncementType + priority: Optional[AnnouncementPriority] = None + title: str + description: str + primaryAction: Optional[BulletinAction] = None + secondaryAction: Optional[BulletinAction] = None + startDate: Optional[str] = None + endDate: Optional[str] = None + + @model_validator(mode="after") + def blank_schedule_means_unset(self) -> "BulletinInput": + """Treat a blank or whitespace-only boundary as absent, not as a value. + + Only blanks are touched. A real instant is passed through byte-for-byte: + the backend owns the canonical schedule, and silently rewriting an + instant here could shift a schedule the maker already reviewed. + """ + if self.startDate is not None and is_blank_schedule_value(self.startDate): + self.startDate = None + if self.endDate is not None and is_blank_schedule_value(self.endDate): + self.endDate = None + return self + + +class SaveBulletinRequest(StrictModel): + """A complete create-or-update of authored content and audience. + + ``id`` present means update, ``id`` absent means create. The widget always + sends the whole authored state, so this model never merges with server + state. Agent identity belongs to the route, never to this HTTP body. + """ + + id: Optional[str] = None + bulletin: BulletinInput + audience: list[str] + status: Literal["draft", "published"] + + @model_validator(mode="after") + def reject_blank_identifiers(self) -> "SaveBulletinRequest": + if self.id is not None and not self.id.strip(): + raise ValueError("id must be a non-empty string when provided") + for group_id in self.audience: + if not group_id or not group_id.strip(): + raise ValueError("audience group IDs must be non-empty") + return self diff --git a/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/requirements.txt b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/requirements.txt new file mode 100644 index 000000000..0d0198932 --- /dev/null +++ b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/requirements.txt @@ -0,0 +1,5 @@ +mcp>=1.29.0,<2.0.0 +httpx>=0.27.0,<1.0 +msal>=1.35.0 +pydantic>=2.0,<3.0 +-r ../agentconfig_core/requirements.txt diff --git a/tests/mcp/_mcp_modules.py b/tests/mcp/_mcp_modules.py index 3dd270853..8ded842d1 100644 --- a/tests/mcp/_mcp_modules.py +++ b/tests/mcp/_mcp_modules.py @@ -128,6 +128,15 @@ def load_landing_page_modules() -> dict[str, ModuleType]: ) +def load_org_announcements_client_modules() -> dict[str, ModuleType]: + """Load contract/client tests without importing optional runtime modules.""" + return load_mcp_modules( + MCP_ROOT / "agentconfig_org_announcements", + ("client", "drafts"), + "ess_mcp_org_announcements", + ) + + def load_org_announcements_modules() -> dict[str, ModuleType]: return load_mcp_modules( MCP_ROOT / "agentconfig_org_announcements", diff --git a/tests/mcp/agentconfig_core/test_tenant_context.py b/tests/mcp/agentconfig_core/test_tenant_context.py index afa2c69f5..4b3af6337 100644 --- a/tests/mcp/agentconfig_core/test_tenant_context.py +++ b/tests/mcp/agentconfig_core/test_tenant_context.py @@ -151,7 +151,7 @@ def test_invalid_config_encoding_warns_and_falls_back(config_path, caplog) -> No assert "configuration could not be read" in caplog.text -@pytest.mark.parametrize("launch_folder", ["agentconfig_landing_page"]) +@pytest.mark.parametrize("launch_folder", ["agentconfig_landing_page", "agentconfig_org_announcements"]) def test_discovers_configured_environment_from_feature_launch_directory( config_path, monkeypatch, launch_folder ) -> None: @@ -209,7 +209,7 @@ def test_runtime_install_paths_include_the_shared_foundation() -> None: assert "-r ../src/mcp/agentconfig_core/requirements.txt" in ( solution / "scripts" / "requirements.txt" ).read_text() - for feature in ("agentconfig_landing_page",): + for feature in ("agentconfig_landing_page", "agentconfig_org_announcements"): assert "-r ../agentconfig_core/requirements.txt" in ( CORE_DIR.parent / feature / "requirements.txt" ).read_text() diff --git a/tests/mcp/agentconfig_org_announcements/test_authoring_client.py b/tests/mcp/agentconfig_org_announcements/test_authoring_client.py new file mode 100644 index 000000000..70efaa6ae --- /dev/null +++ b/tests/mcp/agentconfig_org_announcements/test_authoring_client.py @@ -0,0 +1,878 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Tests for the WeveNova Org Announcements authoring client.""" + +from __future__ import annotations + +import asyncio +import base64 +import json +import sys +from datetime import datetime, timezone +from pathlib import Path + +import httpx +import pytest + + +REPO_ROOT = Path(__file__).parents[3] +ORG_ANNOUNCEMENTS_DIR = ( + REPO_ROOT + / "solutions" + / "ess-maker-skills" + / "src" + / "mcp" + / "agentconfig_org_announcements" +) +# Sibling MCP servers share the top-level names ``client``/``server``, so the +# modules are loaded through the shared isolated importer rather than by a plain +# ``import`` off ``sys.path``. See tests/mcp/_mcp_modules.py. +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from _mcp_modules import load_org_announcements_client_modules # noqa: E402 + +_ORG_MODULES = load_org_announcements_client_modules() + +org_client = _ORG_MODULES["client"] + + +TENANT_ID = "11111111-2222-3333-4444-555555555555" +TITLE_ID = "title-1" +BASE_URL = "https://substrate.office.com/weveb2/api/v1.1" +COLLECTION_PATH = f"/weveb2/api/v1.1/tenants('{TENANT_ID}')/EmployeeAgents('{TITLE_ID}')/essbulletins" +NOW = datetime(2026, 9, 4, 12, 0, tzinfo=timezone.utc) +_UNSET = object() + + +def _token(tenant_id: str = TENANT_ID) -> str: + payload = base64.urlsafe_b64encode( + json.dumps({"tid": tenant_id}).encode("utf-8") + ).rstrip(b"=") + return f"header.{payload.decode('ascii')}.signature" + + +def _make_client(monkeypatch, handler, *, tenant_id=TENANT_ID) -> org_client.OrgAnnouncementsClient: + monkeypatch.setenv("ORG_ANNOUNCEMENTS_BASE_URL", BASE_URL) + monkeypatch.setenv("AGENTCONFIG_ACCESS_TOKEN", _token(tenant_id)) + monkeypatch.delenv("AGENTCONFIG_ACCESS_TOKEN_FILE", raising=False) + return org_client.OrgAnnouncementsClient( + transport=httpx.MockTransport(handler) + ) + + +def _config( + bulletin_id: str, + *, + status: str = "draft", + end_date: str | None = None, + audience: list[str] | None = None, + title_id: str = TITLE_ID, +) -> dict: + return { + "titleId": title_id, + "bulletin": { + "id": bulletin_id, + "type": "standard", + "priority": 1, + "title": "Announcement", + "description": "Body", + "startDate": "2026-09-01T00:00:00.000Z", + **({"endDate": end_date} if end_date else {}), + }, + "audience": audience if audience is not None else ["group-a"], + "status": status, + "createdBy": "admin@contoso.com", + "createdOn": "2026-08-01T12:00:00.000Z", + "modifiedDate": "2026-09-02T12:00:00.000Z", + } + + +def _save_result( + bulletin_id: str, + *, + status: str = "draft", + errors: list[dict] | None = None, + config: dict | None = None, + result_id: str | None = _UNSET, +) -> dict: + """Build an ``EssBulletinSaveResult`` exactly as WeveNova returns it. + + Save answers HTTP 200 with ``{id, config, errors}`` — a different shape from + the bare configs list/load return — so every save-path test speaks that + envelope rather than the load shape. + """ + envelope: dict = { + "id": bulletin_id if result_id is _UNSET else result_id, + "config": config if config is not None else _config(bulletin_id, status=status), + "errors": errors if errors is not None else [], + } + return envelope + + +def test_uses_agent_qualified_v11_routes_with_tenant_from_token(monkeypatch) -> None: + requests: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + if request.url.path.endswith("/save"): + return httpx.Response(200, json=_save_result("new-1")) + if request.url.path.endswith("/essbulletins"): + return httpx.Response(200, json=[_config("a")]) + return httpx.Response(200, json=_config("a")) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + await client.list_bulletins(TITLE_ID) + await client.get_bulletin(TITLE_ID, "a") + await client.save_bulletin(TITLE_ID, {"bulletin": {}, "audience": [], "status": "draft"}) + await client.aclose() + + asyncio.run(run()) + + assert client.tenant_id == TENANT_ID + assert [(request.method, request.url.path) for request in requests] == [ + ("GET", COLLECTION_PATH), + ("GET", f"{COLLECTION_PATH}/a"), + ("POST", f"{COLLECTION_PATH}/save"), + ] + + +def test_title_id_is_a_required_route_key_not_a_query_filter(monkeypatch) -> None: + captured: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + captured.append(request) + return httpx.Response(200, json=[_config("a")]) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + await client.list_bulletins(TITLE_ID) + await client.aclose() + + asyncio.run(run()) + + assert captured[0].url.path == COLLECTION_PATH + assert captured[0].url.query == b"" + + +@pytest.mark.parametrize("bad_id", ["", " a", "a/b", "a\\b", "a?b", "a\x01b"]) +def test_rejects_ids_that_could_reshape_the_route(monkeypatch, bad_id) -> None: + client = _make_client( + monkeypatch, lambda request: httpx.Response(200, json=_config("a")) + ) + + async def run() -> None: + with pytest.raises(ValueError): + await client.get_bulletin(TITLE_ID, bad_id) + await client.aclose() + + asyncio.run(run()) + + +def test_unkeyed_create_is_not_retried_and_reports_indeterminate( + monkeypatch, +) -> None: + attempts: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + attempts.append(request) + return httpx.Response(503, json={"Code": "Busy", "Message": "try later"}) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.IndeterminateWriteError): + await client.save_bulletin(TITLE_ID, + {"bulletin": {}, "audience": [], "status": "draft"} + ) + await client.aclose() + + asyncio.run(run()) + + assert len(attempts) == 1, "an unkeyed create must not be replayed" + + +def test_unkeyed_create_network_failure_reports_indeterminate(monkeypatch) -> None: + attempts: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + attempts.append(request) + raise httpx.ConnectError("connection reset", request=request) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.IndeterminateWriteError): + await client.save_bulletin(TITLE_ID, + {"bulletin": {}, "audience": [], "status": "draft"} + ) + await client.aclose() + + asyncio.run(run()) + + assert len(attempts) == 1 + + +def test_keyed_update_retries_a_transient_gateway_failure(monkeypatch) -> None: + attempts: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + attempts.append(request) + if len(attempts) == 1: + return httpx.Response(503, json={"Code": "Busy", "Message": "later"}) + return httpx.Response(200, json=_save_result("a")) + + client = _make_client(monkeypatch, handler) + client.max_retries = 2 + + async def run() -> None: + result = await client.save_bulletin(TITLE_ID, + {"id": "a", "bulletin": {}, "audience": [], "status": "draft"} + ) + assert result["bulletin"]["id"] == "a" + await client.aclose() + + asyncio.run(run()) + + assert len(attempts) == 2 + + +def test_definite_rejection_is_not_reported_as_indeterminate(monkeypatch) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 400, json={"Code": "AudienceRequired", "Message": "audience required"} + ) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError) as caught: + await client.save_bulletin(TITLE_ID, + {"bulletin": {}, "audience": [], "status": "draft"} + ) + assert not isinstance(caught.value, org_client.IndeterminateWriteError) + assert "AudienceRequired" in str(caught.value) + await client.aclose() + + asyncio.run(run()) + + +def test_transition_sends_only_identity_and_status(monkeypatch) -> None: + captured: list[dict] = [] + + def handler(request: httpx.Request) -> httpx.Response: + captured.append(json.loads(request.content.decode("utf-8"))) + return httpx.Response(200, json=_save_result("a", status="retired")) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + changed = await client.transition_bulletin(TITLE_ID, "a", "retired") + # The envelope is unwrapped to the canonical config, not passed through. + assert changed["bulletin"]["id"] == "a" + assert changed["status"] == "retired" + assert "config" not in changed + await client.aclose() + + asyncio.run(run()) + + assert captured == [{"id": "a", "status": "retired"}] + assert "bulletin" not in captured[0] + assert "audience" not in captured[0] + + +# -------------------------------------------------------------------------- +# EssBulletinSaveResult envelope +# -------------------------------------------------------------------------- + + +def test_save_unwraps_the_canonical_config_from_the_result_envelope( + monkeypatch, +) -> None: + """A successful save returns ``config``, never the wrapper itself.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_save_result("created-1")) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + saved = await client.save_bulletin(TITLE_ID, + {"bulletin": {}, "audience": [], "status": "draft"} + ) + assert saved == _config("created-1") + # The wrapper's own keys must not leak into canonical state. + assert "config" not in saved + assert "errors" not in saved + await client.aclose() + + asyncio.run(run()) + + +def test_http_200_with_errors_is_a_structured_validation_failure( + monkeypatch, +) -> None: + """Validation failure arrives inside a 200; every entry must survive.""" + reported = [ + {"code": "AudienceRequired", "field": "audience", "message": "Pick a group."}, + {"code": "TitleRequired", "field": "title", "message": "Add a title."}, + ] + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, json={"id": None, "config": None, "errors": reported} + ) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.BulletinValidationError) as caught: + await client.save_bulletin(TITLE_ID, + {"bulletin": {}, "audience": [], "status": "published"} + ) + assert caught.value.errors == reported + assert caught.value.http_status == 200 + await client.aclose() + + asyncio.run(run()) + + +def test_a_partial_error_entry_keeps_its_field_and_gets_a_stable_code( + monkeypatch, +) -> None: + """A sparse entry is still reported; it is never dropped or merged.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "id": None, + "config": None, + "errors": [{"field": "title"}, "unstructured"], + }, + ) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.BulletinValidationError) as caught: + await client.save_bulletin(TITLE_ID, + {"bulletin": {}, "audience": [], "status": "draft"} + ) + assert len(caught.value.errors) == 2 + assert caught.value.errors[0]["field"] == "title" + assert caught.value.errors[0]["code"] == "InvalidRequest" + assert caught.value.errors[1]["message"] == "unstructured" + await client.aclose() + + asyncio.run(run()) + + +def test_a_success_envelope_without_a_config_is_rejected(monkeypatch) -> None: + """No errors and no config is malformed, never an empty announcement.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={"id": "a", "config": None, "errors": []}) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError): + await client.save_bulletin(TITLE_ID, + {"id": "a", "bulletin": {}, "audience": [], "status": "draft"} + ) + await client.aclose() + + asyncio.run(run()) + + +def test_a_save_result_id_disagreeing_with_its_config_is_rejected( + monkeypatch, +) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, json=_save_result("a", config=_config("b"), result_id="a") + ) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError) as caught: + await client.save_bulletin(TITLE_ID, + {"id": "a", "bulletin": {}, "audience": [], "status": "draft"} + ) + assert "does not match" in str(caught.value) + await client.aclose() + + asyncio.run(run()) + + +def test_an_update_answered_with_a_different_record_is_rejected( + monkeypatch, +) -> None: + """Replacing canonical state with someone else's announcement is a bug.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_save_result("other-1")) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError) as caught: + await client.save_bulletin(TITLE_ID, + {"id": "a", "bulletin": {}, "audience": [], "status": "draft"} + ) + assert "different announcement" in str(caught.value) + await client.aclose() + + asyncio.run(run()) + + +def test_a_transition_answered_for_another_record_is_rejected(monkeypatch) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_save_result("other-1", status="retired")) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError): + await client.transition_bulletin(TITLE_ID, "a", "retired") + await client.aclose() + + asyncio.run(run()) + + +def test_a_transition_rejected_by_validation_preserves_its_codes( + monkeypatch, +) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "id": "a", + "config": None, + "errors": [ + { + "code": "InvalidLifecycleTransition", + "field": "status", + "message": "Cannot unarchive a deleted announcement.", + } + ], + }, + ) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.BulletinValidationError) as caught: + await client.transition_bulletin(TITLE_ID, "a", "draft") + assert caught.value.errors[0]["code"] == "InvalidLifecycleTransition" + await client.aclose() + + asyncio.run(run()) + + +def test_a_non_list_error_field_is_rejected(monkeypatch) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={"id": "a", "config": None, "errors": "bad"}) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError): + await client.save_bulletin(TITLE_ID, + {"id": "a", "bulletin": {}, "audience": [], "status": "draft"} + ) + await client.aclose() + + asyncio.run(run()) + + +def test_a_bare_config_save_response_is_rejected(monkeypatch) -> None: + """The load shape is not the save shape; accepting it would mask drift.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_config("a")) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError): + await client.save_bulletin(TITLE_ID, + {"id": "a", "bulletin": {}, "audience": [], "status": "draft"} + ) + await client.aclose() + + asyncio.run(run()) + + +def test_a_saved_record_without_an_id_is_rejected(monkeypatch) -> None: + """An identity-less record could never be edited or transitioned again.""" + config = _config("placeholder") + del config["bulletin"]["id"] + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={"config": config, "errors": []}) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError) as caught: + await client.save_bulletin(TITLE_ID, + {"bulletin": {}, "audience": [], "status": "draft"} + ) + assert "without an id" in str(caught.value) + await client.aclose() + + asyncio.run(run()) + + +def test_a_create_may_take_its_id_from_either_envelope_position( + monkeypatch, +) -> None: + """The wrapper id and the config id are both authoritative when they agree. + + A create has no requested id to compare against, so either position alone is + enough as long as one is present. + """ + config = _config("created-1") + + def handler(request: httpx.Request) -> httpx.Response: + # Wrapper id omitted; the config still carries the canonical id. + return httpx.Response(200, json={"config": config, "errors": []}) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + saved = await client.save_bulletin(TITLE_ID, + {"bulletin": {}, "audience": [], "status": "draft"} + ) + assert saved["bulletin"]["id"] == "created-1" + await client.aclose() + + asyncio.run(run()) + + +def test_invalid_success_shaped_bodies_are_rejected(monkeypatch) -> None: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/essbulletins"): + return httpx.Response(200, json={"unexpected": True}) + return httpx.Response(200, json={"status": "draft"}) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError): + await client.list_bulletins(TITLE_ID) + with pytest.raises(org_client.AgentConfigApiError): + await client.get_bulletin(TITLE_ID, "a") + await client.aclose() + + asyncio.run(run()) + + +def test_collection_items_missing_content_are_rejected(monkeypatch) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={"value": [{"status": "draft"}]}) + + client = _make_client(monkeypatch, handler) + + async def run() -> None: + with pytest.raises(org_client.AgentConfigApiError): + await client.list_bulletins(TITLE_ID) + await client.aclose() + + asyncio.run(run()) + + +@pytest.mark.parametrize( + ("status", "end_date", "expected_archived"), + [ + ("draft", None, False), + ("draft", "2020-01-01T00:00:00.000Z", False), + ("published", None, False), + ("published", "2030-01-01T00:00:00.000Z", False), + ("published", "2020-01-01T00:00:00.000Z", True), + ("retired", None, True), + ("retired", "2030-01-01T00:00:00.000Z", True), + ], +) +def test_archive_classification_uses_status_and_schedule( + status, end_date, expected_archived +) -> None: + config = _config("a", status=status, end_date=end_date) + assert org_client.is_archived_item(config, NOW) is expected_archived + + +def test_unparseable_end_date_keeps_a_published_item_current() -> None: + config = _config("a", status="published", end_date="not-a-date") + assert org_client.is_archived_item(config, NOW) is False + + +def test_manager_state_counts_current_items_and_preserves_order() -> None: + items = [ + _config("current-1", status="published", end_date="2030-01-01T00:00:00.000Z"), + _config("archived-1", status="retired"), + _config("current-2", status="draft"), + ] + + state = org_client.build_manager_state(items, {}, NOW, tenant_id=TENANT_ID, title_id=TITLE_ID) + + assert state["workingSetCount"] == 2 + assert state["archivedTruncated"] is False + assert [item["config"]["bulletin"]["id"] for item in state["items"]] == [ + "current-1", + "archived-1", + "current-2", + ] + + +def test_manager_state_flags_a_full_archived_window() -> None: + items = [_config("current", status="draft")] + items.extend( + _config(f"archived-{index}", status="retired") + for index in range(org_client.ARCHIVED_WINDOW_SIZE) + ) + + state = org_client.build_manager_state(items, {}, NOW, tenant_id=TENANT_ID, title_id=TITLE_ID) + + assert state["workingSetCount"] == 1 + assert state["archivedTruncated"] is True + # The state never claims an exact archived total. + assert "archivedCount" not in state + assert "archivedTotal" not in state + + +def test_manager_state_excludes_deleted_rows_from_items_and_counts() -> None: + """Delete is a status transition, so the list can still return the row. + + A deleted announcement is neither a working item nor a restorable archived + one, so it must not appear and must not be counted in either bucket. + """ + items = [ + _config("current-1", status="draft"), + _config("deleted-1", status="deleted"), + _config("archived-1", status="retired"), + ] + + state = org_client.build_manager_state(items, {}, NOW, tenant_id=TENANT_ID, title_id=TITLE_ID) + + assert [item["config"]["bulletin"]["id"] for item in state["items"]] == [ + "current-1", + "archived-1", + ] + assert state["workingSetCount"] == 1 + assert state["archivedTruncated"] is False + + +def test_deleted_rows_do_not_fill_the_archived_window() -> None: + """Deleted rows must not push archivedTruncated true on their own.""" + items = [ + _config(f"deleted-{index}", status="deleted") + for index in range(org_client.ARCHIVED_WINDOW_SIZE) + ] + items.append(_config("current", status="draft")) + + state = org_client.build_manager_state(items, {}, NOW, tenant_id=TENANT_ID, title_id=TITLE_ID) + + assert state["workingSetCount"] == 1 + assert state["archivedTruncated"] is False + assert len(state["items"]) == 1 + + +def test_deleted_classification_is_independent_of_schedule() -> None: + assert org_client.is_deleted_item(_config("a", status="deleted")) is True + assert org_client.is_deleted_item(_config("a", status="retired")) is False + assert org_client.is_deleted_item(_config("a", status="draft")) is False + + +def test_manager_state_attaches_per_item_audience_metadata() -> None: + items = [_config("a", audience=["g1", "g2"])] + metadata = { + "a": [ + {"id": "g1", "displayName": "Group One", "mail": None, "isValid": True}, + {"id": "g2", "displayName": "Group Two", "mail": None, "isValid": True}, + ] + } + + state = org_client.build_manager_state(items, metadata, NOW, tenant_id=TENANT_ID, title_id=TITLE_ID) + + assert [group["id"] for group in state["items"][0]["audienceMetadata"]] == [ + "g1", + "g2", + ] + + +def test_base_url_must_be_https(monkeypatch) -> None: + monkeypatch.setenv("ORG_ANNOUNCEMENTS_BASE_URL", "http://example.invalid") + monkeypatch.setenv("AGENTCONFIG_ACCESS_TOKEN", _token()) + + with pytest.raises(ValueError): + org_client.OrgAnnouncementsClient() + + +def test_default_base_url_is_the_agent_qualified_v11_surface(monkeypatch) -> None: + monkeypatch.delenv("ORG_ANNOUNCEMENTS_BASE_URL", raising=False) + monkeypatch.setenv("AGENTCONFIG_ACCESS_TOKEN", _token()) + + client = org_client.OrgAnnouncementsClient() + + assert client.base_url == org_client.DEFAULT_ORG_ANNOUNCEMENTS_BASE_URL + assert client.base_url == BASE_URL + + +def test_repr_never_exposes_the_token(monkeypatch) -> None: + monkeypatch.setenv("AGENTCONFIG_ACCESS_TOKEN", _token()) + monkeypatch.setenv("ORG_ANNOUNCEMENTS_BASE_URL", BASE_URL) + + client = org_client.OrgAnnouncementsClient() + + assert _token() not in repr(client) + + +@pytest.mark.parametrize("operation", ["list", "get", "save", "transition"]) +@pytest.mark.parametrize("title_id", ["", " ", " padded", "padded ", "a\x00b", "a\x7fb", "a" * 257]) +def test_invalid_title_is_rejected_before_any_http_request(monkeypatch, operation, title_id) -> None: + def handler(request): + pytest.fail("invalid title reached the transport") + + client = _make_client(monkeypatch, handler) + + async def run(): + with pytest.raises(ValueError, match="titleId"): + if operation == "list": + await client.list_bulletins(title_id) + elif operation == "get": + await client.get_bulletin(title_id, "a") + elif operation == "save": + await client.save_bulletin(title_id, {"status": "draft"}) + else: + await client.transition_bulletin(title_id, "a", "retired") + await client.aclose() + + asyncio.run(run()) + + +def test_title_key_uses_the_landing_page_odata_encoding(monkeypatch) -> None: + requests = [] + title_id = "a'b/c" + + def handler(request): + requests.append(request) + return httpx.Response(200, json=[_config("a", title_id=title_id)]) + + client = _make_client(monkeypatch, handler) + + async def run(): + await client.list_bulletins(title_id) + await client.aclose() + + asyncio.run(run()) + assert b"/EmployeeAgents('a%27%27b%2Fc')/essbulletins" in requests[0].url.raw_path + assert requests[0].url.query == b"" + + +def test_canonical_scope_cannot_be_embedded_in_bulletin_content(monkeypatch) -> None: + config = _config("a") + config["bulletin"]["titleId"] = TITLE_ID + client = _make_client(monkeypatch, lambda r: httpx.Response(200, json=config)) + + async def run(): + with pytest.raises(org_client.AgentConfigApiError, match="inside bulletin"): + await client.get_bulletin(TITLE_ID, "a") + await client.aclose() + + asyncio.run(run()) + + +@pytest.mark.parametrize("operation", ["list", "get", "save", "transition"]) +@pytest.mark.parametrize("response_title", [None, "", "another-title", "TITLE-1"]) +def test_every_returned_config_must_echo_the_exact_title(monkeypatch, operation, response_title) -> None: + config = _config("a") + if response_title is None: + config.pop("titleId") + else: + config["titleId"] = response_title + requests = [] + + def handler(request): + requests.append(request) + if operation == "list": + body = [_config("valid"), config] + elif operation == "get": + body = config + else: + body = _save_result("a", config=config) + return httpx.Response(200, json=body) + + client = _make_client(monkeypatch, handler) + + async def run(): + with pytest.raises(org_client.AgentConfigApiError, match="titleId"): + if operation == "list": + await client.list_bulletins(TITLE_ID) + elif operation == "get": + await client.get_bulletin(TITLE_ID, "a") + elif operation == "save": + await client.save_bulletin(TITLE_ID, {"id": "a", "status": "draft"}) + else: + await client.transition_bulletin(TITLE_ID, "a", "retired") + await client.aclose() + + asyncio.run(run()) + assert len(requests) == 1 + + +def test_unavailable_agent_route_never_falls_back_to_tenant_collection(monkeypatch) -> None: + paths = [] + + def handler(request): + paths.append(request.url.path) + return httpx.Response(404, json={"Code": "NotFound", "Message": "not deployed"}) + + client = _make_client(monkeypatch, handler) + + async def run(): + with pytest.raises(org_client.AgentConfigApiError): + await client.list_bulletins(TITLE_ID) + await client.aclose() + + asyncio.run(run()) + assert paths == [COLLECTION_PATH] + + +def test_collections_and_manager_limits_are_independent_per_tenant_and_agent(monkeypatch) -> None: + other_tenant = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + pairs = [(TENANT_ID, TITLE_ID, 100, 50), (TENANT_ID, "title-2", 1, 49), + (other_tenant, TITLE_ID, 0, 0)] + responses = {} + for tenant, title, current, archived in pairs: + path = f"/weveb2/api/v1.1/tenants('{tenant}')/EmployeeAgents('{title}')/essbulletins" + responses[path] = [ + _config(f"same-id-{i}", title_id=title, status="draft" if i < current else "retired") + for i in range(current + archived) + ] + clients = { + tenant: _make_client(monkeypatch, lambda r: httpx.Response(200, json=responses[r.url.path]), + tenant_id=tenant) + for tenant in (TENANT_ID, other_tenant) + } + + async def run(): + for tenant, title, current, archived in pairs: + items = await clients[tenant].list_bulletins(title) + state = org_client.build_manager_state( + items, {}, NOW, tenant_id=tenant, title_id=title + ) + assert state["tenantId"] == tenant + assert state["titleId"] == title + assert state["workingSetCount"] == current + assert state["archivedTruncated"] is (archived == 50) + assert len(state["items"]) == current + archived + for client in clients.values(): + await client.aclose() + + asyncio.run(run()) diff --git a/tests/mcp/agentconfig_org_announcements/test_drafts.py b/tests/mcp/agentconfig_org_announcements/test_drafts.py new file mode 100644 index 000000000..c2cd0a77d --- /dev/null +++ b/tests/mcp/agentconfig_org_announcements/test_drafts.py @@ -0,0 +1,868 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Tests for the typed suggested-draft schema and editor-draft projection.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest +from pydantic import ValidationError + + +REPO_ROOT = Path(__file__).parents[3] +ORG_ANNOUNCEMENTS_DIR = ( + REPO_ROOT + / "solutions" + / "ess-maker-skills" + / "src" + / "mcp" + / "agentconfig_org_announcements" +) +# Sibling MCP servers share the top-level names ``client``/``server``, so the +# modules are loaded through the shared isolated importer rather than by a plain +# ``import`` off ``sys.path``. See tests/mcp/_mcp_modules.py. +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from _mcp_modules import load_org_announcements_client_modules # noqa: E402 + +_ORG_MODULES = load_org_announcements_client_modules() + +drafts = _ORG_MODULES["drafts"] + + +EMPTY_EDITOR_DRAFT = { + "id": None, + "type": "standard", + "title": "", + "description": "", + "primaryAction": None, + "startDate": "", + "endDate": "", + "audience": [], + "standardPriority": 1, + "standardSecondaryAction": None, +} + + +# -------------------------------------------------------------------------- +# Canonical metadata can never enter a suggestion +# -------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "forbidden", + [ + {"id": "bulletin-1"}, + {"bulletinId": "bulletin-1"}, + {"status": "published"}, + {"createdBy": "admin@contoso.com"}, + {"createdOn": "2026-08-01T12:00:00.000Z"}, + {"modifiedDate": "2026-09-02T12:00:00.000Z"}, + {"modifiedBy": "admin@contoso.com"}, + {"archivedOn": "2026-09-02T12:00:00.000Z"}, + {"version": 3}, + {"etag": 'W/"3"'}, + {"titleId": "title-1"}, + {"standardPriority": 0}, + {"standardSecondaryAction": None}, + ], +) +def test_suggested_drafts_reject_canonical_and_unknown_fields(forbidden) -> None: + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft.model_validate({"title": "Hi", **forbidden}) + + +def test_a_suggested_draft_accepts_only_the_agreed_contract_fields() -> None: + assert set(drafts.SuggestedBulletinDraft.model_fields) == { + "type", + "priority", + "title", + "description", + "primaryAction", + "secondaryAction", + "startDate", + "endDate", + "audience", + } + + +# -------------------------------------------------------------------------- +# Defaults and field mapping +# -------------------------------------------------------------------------- + + +def test_no_suggestion_produces_the_empty_editor_defaults() -> None: + assert drafts.build_create_draft(None).model_dump(mode="json") == EMPTY_EDITOR_DRAFT + + +def test_every_omitted_field_keeps_its_editor_default() -> None: + suggestion = drafts.SuggestedBulletinDraft(title="Only a title") + + result = drafts.build_create_draft(suggestion).model_dump(mode="json") + + assert result == {**EMPTY_EDITOR_DRAFT, "title": "Only a title"} + + +def test_priority_maps_to_standard_priority() -> None: + suggestion = drafts.SuggestedBulletinDraft(priority=0) + + assert drafts.build_create_draft(suggestion).standardPriority == 0 + + +def test_secondary_action_maps_to_standard_secondary_action() -> None: + suggestion = drafts.SuggestedBulletinDraft( + secondaryAction={ + "actionType": "copilotChat", + "label": "Ask", + "prompt": "Tell me more", + } + ) + + result = drafts.build_create_draft(suggestion) + + assert result.standardSecondaryAction is not None + assert result.standardSecondaryAction.label == "Ask" + assert "secondaryAction" not in result.model_dump(mode="json") + + +def test_explicit_empty_strings_stay_explicit_draft_values() -> None: + suggestion = drafts.SuggestedBulletinDraft( + title="", description="", startDate="", endDate="" + ) + + result = drafts.build_create_draft(suggestion).model_dump(mode="json") + + assert result["title"] == "" + assert result["description"] == "" + assert result["startDate"] == "" + assert result["endDate"] == "" + + +def test_a_create_draft_never_carries_an_identifier() -> None: + suggestion = drafts.SuggestedBulletinDraft(title="New") + + assert drafts.build_create_draft(suggestion).id is None + + +def test_resolved_audience_metadata_is_attached_in_order() -> None: + metadata = [ + drafts.AudienceGroup(id="g2", displayName="Two"), + drafts.AudienceGroup(id="g1", displayName="One"), + ] + + result = drafts.build_create_draft( + drafts.SuggestedBulletinDraft(audience=["g2", "g1"]), metadata + ) + + assert [group.id for group in result.audience] == ["g2", "g1"] + + +# -------------------------------------------------------------------------- +# Alert compatibility +# -------------------------------------------------------------------------- + + +def test_alert_with_explicit_priority_is_rejected() -> None: + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft.model_validate( + {"type": "alert", "priority": 0} + ) + + +def test_alert_with_explicit_secondary_action_is_rejected() -> None: + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft.model_validate( + { + "type": "alert", + "secondaryAction": { + "actionType": "externalLink", + "label": "More", + "url": "https://contoso.com", + }, + } + ) + + +def test_alert_with_a_copilot_chat_action_is_rejected() -> None: + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft.model_validate( + { + "type": "alert", + "primaryAction": { + "actionType": "copilotChat", + "label": "Ask", + "prompt": "Explain", + }, + } + ) + + +def test_alert_still_receives_the_hidden_standard_defaults() -> None: + suggestion = drafts.SuggestedBulletinDraft(type="alert", title="Outage") + + result = drafts.build_create_draft(suggestion) + + assert result.type == "alert" + assert result.standardPriority == drafts.DEFAULT_STANDARD_PRIORITY + assert result.standardSecondaryAction is None + + +# -------------------------------------------------------------------------- +# Action payloads +# -------------------------------------------------------------------------- + + +def test_external_link_actions_must_not_carry_a_prompt() -> None: + with pytest.raises(ValidationError): + drafts.BulletinAction.model_validate( + { + "actionType": "externalLink", + "label": "Go", + "url": "https://contoso.com", + "prompt": "Explain", + } + ) + + +def test_copilot_chat_actions_must_not_carry_a_url() -> None: + with pytest.raises(ValidationError): + drafts.BulletinAction.model_validate( + { + "actionType": "copilotChat", + "label": "Ask", + "prompt": "Explain", + "url": "https://contoso.com", + } + ) + + +def test_blank_audience_ids_are_rejected() -> None: + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft.model_validate({"audience": ["g1", " "]}) + + +# -------------------------------------------------------------------------- +# Canonical config projection +# -------------------------------------------------------------------------- + + +def _config(**overrides) -> dict: + bulletin = { + "id": "bulletin-1", + "type": "standard", + "priority": 0, + "title": "Quarterly update", + "description": "Read this", + "startDate": "2026-09-01T00:00:00.000Z", + "endDate": "2026-10-01T23:59:59.999Z", + "primaryAction": { + "actionType": "externalLink", + "label": "Read", + "url": "https://contoso.com", + }, + "secondaryAction": { + "actionType": "copilotChat", + "label": "Ask", + "prompt": "Summarize", + }, + } + bulletin.update(overrides) + return { + "bulletin": bulletin, + "audience": ["g1"], + "status": "published", + "createdBy": "admin@contoso.com", + "createdOn": "2026-08-01T12:00:00.000Z", + "modifiedDate": "2026-09-02T12:00:00.000Z", + } + + +def test_edit_projection_preserves_canonical_content_and_identity() -> None: + metadata = [drafts.AudienceGroup(id="g1", displayName="One")] + + result = drafts.build_editor_draft_from_config(_config(), metadata) + + assert result.id == "bulletin-1" + assert result.startDate == "2026-09-01T00:00:00.000Z" + assert result.endDate == "2026-10-01T23:59:59.999Z" + assert result.standardPriority == 0 + assert result.standardSecondaryAction is not None + assert result.standardSecondaryAction.actionType == "copilotChat" + + +def test_missing_stored_schedule_uses_empty_strings_not_null() -> None: + metadata = [drafts.AudienceGroup(id="g1", displayName="One")] + + result = drafts.build_editor_draft_from_config( + _config(startDate=None, endDate=None), metadata + ) + + assert result.startDate == "" + assert result.endDate == "" + assert result.id == "bulletin-1" + assert result.title == "Quarterly update" + + +def test_missing_optional_content_falls_back_to_editor_defaults() -> None: + config = { + "bulletin": {"id": "bulletin-1", "type": "standard", "title": "Only title"}, + "audience": [], + "status": "draft", + } + + result = drafts.build_editor_draft_from_config(config, []) + + assert result.description == "" + assert result.primaryAction is None + assert result.standardPriority == drafts.DEFAULT_STANDARD_PRIORITY + assert result.standardSecondaryAction is None + assert result.audience == [] + + +@pytest.mark.parametrize( + "config", + [ + {"audience": [], "status": "draft"}, + {"bulletin": "not-an-object"}, + {"bulletin": {"type": "standard"}}, + {"bulletin": {"id": "", "type": "standard"}}, + ], +) +def test_malformed_configurations_are_rejected(config) -> None: + with pytest.raises(ValueError): + drafts.build_editor_draft_from_config(config, []) + + +# -------------------------------------------------------------------------- +# Save request contract +# -------------------------------------------------------------------------- + + +def test_save_requests_accept_an_absent_id_as_a_create() -> None: + request = drafts.SaveBulletinRequest.model_validate( + { + "bulletin": {"type": "standard", "title": "t", "description": "d"}, + "audience": ["g1"], + "status": "draft", + } + ) + + assert request.id is None + assert "id" not in request.model_dump(mode="json", exclude_none=True) + + +@pytest.mark.parametrize("field", ["titleId", "tenantId"]) +@pytest.mark.parametrize("nested", [False, True]) +def test_http_save_body_and_authored_content_reject_scope(field, nested) -> None: + body = { + "bulletin": {"type": "standard", "title": "t", "description": "d"}, + "audience": ["g1"], + "status": "draft", + } + target = body["bulletin"] if nested else body + target[field] = "not-authored-content" + with pytest.raises(ValidationError, match=field): + drafts.SaveBulletinRequest.model_validate(body) + + +def test_retry_input_preserves_original_values_without_exposing_private_fields() -> None: + original = {"startDate": "2026-09-12", "title": "Review me"} + suggestion = drafts.SuggestedBulletinDraft.model_validate(original) + assert suggestion.startDate == "2026-09-12T00:00:00.000Z" + assert suggestion.retry_payload() == original + assert "_retry_input" not in suggestion.model_dump() + assert "_retry_input" not in drafts.SuggestedBulletinDraft.model_json_schema()["properties"] + retry = suggestion.retry_payload() + retry["title"] = "changed" + assert suggestion.retry_payload() == original + + +def test_save_requests_reject_blank_identity_and_audience_values() -> None: + with pytest.raises(ValidationError): + drafts.SaveBulletinRequest.model_validate( + { + "id": " ", + "bulletin": {"type": "standard", "title": "t", "description": "d"}, + "audience": [], + "status": "draft", + } + ) + with pytest.raises(ValidationError): + drafts.SaveBulletinRequest.model_validate( + { + "bulletin": {"type": "standard", "title": "t", "description": "d"}, + "audience": [""], + "status": "draft", + } + ) + + +def test_save_requests_reject_a_non_authoring_status() -> None: + for status in ("retired", "deleted", "archived"): + with pytest.raises(ValidationError): + drafts.SaveBulletinRequest.model_validate( + { + "bulletin": {"type": "standard", "title": "t", "description": "d"}, + "audience": ["g1"], + "status": status, + } + ) + + +def test_save_requests_reject_audit_fields_on_the_content() -> None: + with pytest.raises(ValidationError): + drafts.SaveBulletinRequest.model_validate( + { + "bulletin": { + "type": "standard", + "title": "t", + "description": "d", + "modifiedDate": "2026-09-02T12:00:00.000Z", + }, + "audience": ["g1"], + "status": "draft", + } + ) + + +# -------------------------------------------------------------------------- +# Suggested schedule normalization +# -------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + # Explicit "unset" is preserved so the DatePicker shows its own state. + ("", ""), + (" ", ""), + # Date-only start anchors to the first instant of the UTC day. + ("2026-09-01", "2026-09-01T00:00:00.000Z"), + ("2026-12-31", "2026-12-31T00:00:00.000Z"), + # Aware instants normalize to UTC milliseconds. + ("2026-09-01T00:00:00.000Z", "2026-09-01T00:00:00.000Z"), + ("2026-09-01T00:00:00Z", "2026-09-01T00:00:00.000Z"), + ("2026-09-01T00:00:00z", "2026-09-01T00:00:00.000Z"), + ("2026-09-01T08:30:00+02:00", "2026-09-01T06:30:00.000Z"), + ("2026-09-01T00:00:00-05:00", "2026-09-01T05:00:00.000Z"), + ("2026-09-01T12:34:56.789Z", "2026-09-01T12:34:56.789Z"), + # Sub-millisecond precision truncates rather than rounding up, so the + # normalized instant is never later than what the maker supplied. + ("2026-09-01T12:34:56.789999Z", "2026-09-01T12:34:56.789Z"), + ], +) +def test_suggested_start_dates_normalize_to_utc_instants(raw, expected) -> None: + result = drafts.build_create_draft( + drafts.SuggestedBulletinDraft(startDate=raw) + ) + + assert result.startDate == expected + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("", ""), + # A date-only END covers the whole day, so a single-day announcement + # does not collapse to a zero-length midnight-to-midnight window. + ("2026-09-01", "2026-09-01T23:59:59.999Z"), + ("2026-12-31", "2026-12-31T23:59:59.999Z"), + ("2026-10-01T23:59:59.999Z", "2026-10-01T23:59:59.999Z"), + ("2026-09-01T08:30:00+02:00", "2026-09-01T06:30:00.000Z"), + ], +) +def test_suggested_end_dates_normalize_to_utc_instants(raw, expected) -> None: + result = drafts.build_create_draft(drafts.SuggestedBulletinDraft(endDate=raw)) + + assert result.endDate == expected + + +@pytest.mark.parametrize( + "raw", + [ + # Natural language the model might invent. + "next Monday", + "tomorrow", + "in two weeks", + "Sept 1 2026", + "09/01/2026", + # Timezone-naive: the maker's local day boundary is not knowable here, + # and guessing UTC would schedule the announcement at the wrong time. + "2026-09-01T00:00:00", + "2026-09-01T08:30:00.000", + "2026-09-01 08:30:00", + # Structurally invalid. + "2026-13-01", + "2026-02-30", + "2026-09-01T25:00:00Z", + "not-a-date", + "Z", + ], +) +@pytest.mark.parametrize("field", ["startDate", "endDate"]) +def test_unparseable_or_naive_schedule_values_are_rejected(raw, field) -> None: + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft(**{field: raw}) + + +def test_both_schedule_boundaries_normalize_independently() -> None: + result = drafts.build_create_draft( + drafts.SuggestedBulletinDraft( + startDate="2026-09-01", endDate="2026-09-01" + ) + ) + + assert result.startDate == "2026-09-01T00:00:00.000Z" + assert result.endDate == "2026-09-01T23:59:59.999Z" + + +def test_normalization_is_idempotent() -> None: + """A normalized instant fed back in must not drift.""" + once = drafts.normalize_suggested_instant("2026-09-01", boundary="endDate") + twice = drafts.normalize_suggested_instant(once, boundary="endDate") + + assert once == twice == "2026-09-01T23:59:59.999Z" + + +def test_an_omitted_schedule_keeps_the_editor_default() -> None: + result = drafts.build_create_draft(drafts.SuggestedBulletinDraft()) + + assert result.startDate == drafts.DEFAULT_START_DATE + assert result.endDate == drafts.DEFAULT_END_DATE + + +def test_stored_schedules_are_not_re_normalized() -> None: + """A canonical stored instant is passed through untouched. + + Normalization is an *input* guard on model-authored suggestions. The backend + already owns the canonical instant, so rewriting it here could silently + change a stored schedule. + """ + config = { + "bulletin": { + "id": "b1", + "type": "standard", + "title": "t", + "description": "d", + "startDate": "2026-09-01T00:00:00.0000000+00:00", + "endDate": "2026-10-01T23:59:59.999Z", + }, + "audience": [], + "status": "published", + } + + result = drafts.build_editor_draft_from_config(config, []) + + assert result.startDate == "2026-09-01T00:00:00.0000000+00:00" + assert result.endDate == "2026-10-01T23:59:59.999Z" + + +# -------------------------------------------------------------------------- +# Suggested action targets +# -------------------------------------------------------------------------- + + +def test_a_suggested_external_link_requires_a_non_blank_url() -> None: + """A suggestion is unreviewed model output, not a half-typed draft.""" + for url in (None, "", " "): + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft( + primaryAction={ + "actionType": "externalLink", + "label": "Open", + **({} if url is None else {"url": url}), + } + ) + + +def test_a_suggested_copilot_chat_requires_a_non_blank_prompt() -> None: + for prompt in (None, "", " "): + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft( + secondaryAction={ + "actionType": "copilotChat", + "label": "Ask", + **({} if prompt is None else {"prompt": prompt}), + } + ) + + +def test_complete_suggested_actions_are_accepted() -> None: + result = drafts.build_create_draft( + drafts.SuggestedBulletinDraft( + primaryAction={ + "actionType": "externalLink", + "label": "Open", + "url": "https://contoso.example/benefits", + }, + secondaryAction={ + "actionType": "copilotChat", + "label": "Ask", + "prompt": "Explain the benefits change", + }, + ) + ) + + assert result.primaryAction.url == "https://contoso.example/benefits" + assert result.standardSecondaryAction.prompt == "Explain the benefits change" + + +@pytest.mark.parametrize( + "action", + [ + {"actionType": "externalLink", "label": "Open"}, + {"actionType": "externalLink", "label": "Open", "url": ""}, + {"actionType": "copilotChat", "label": "Ask"}, + {"actionType": "copilotChat", "label": "Ask", "prompt": " "}, + ], +) +def test_ordinary_saves_still_accept_incomplete_draft_actions(action) -> None: + """A Draft may be unfinished; WeveNova owns publish-time completeness. + + Rejecting these here would make ``save_bulletin`` refuse drafts the backend + accepts and would replace the backend's field-level code with a generic + contract error the widget cannot bind to an input. + """ + request = drafts.SaveBulletinRequest.model_validate( + { + "bulletin": { + "type": "standard", + "title": "", + "description": "", + "primaryAction": action, + }, + "audience": [], + "status": "draft", + } + ) + + assert request.bulletin.primaryAction.actionType == action["actionType"] + + +def test_a_stored_incomplete_action_still_projects_into_the_editor() -> None: + """Opening an existing unfinished Draft must not fail validation.""" + config = { + "bulletin": { + "id": "b1", + "type": "standard", + "title": "t", + "description": "d", + "primaryAction": {"actionType": "externalLink", "label": "Open"}, + }, + "audience": [], + "status": "draft", + } + + result = drafts.build_editor_draft_from_config(config, []) + + assert result.primaryAction.actionType == "externalLink" + assert result.primaryAction.url is None + + +def test_mismatched_action_targets_are_still_rejected_everywhere() -> None: + """Wrong-target actions are malformed at the contract level, not merely + incomplete, so both the strict and the permissive model reject them.""" + with pytest.raises(ValidationError): + drafts.SaveBulletinRequest.model_validate( + { + "bulletin": { + "type": "standard", + "title": "t", + "description": "d", + "primaryAction": { + "actionType": "externalLink", + "label": "Open", + "prompt": "nope", + }, + }, + "audience": [], + "status": "draft", + } + ) + + +# -------------------------------------------------------------------------- +# Blank date sentinel on the save path +# -------------------------------------------------------------------------- + + +def _save_request(**bulletin_overrides) -> drafts.SaveBulletinRequest: + bulletin = { + "type": "standard", + "title": "Quarterly update", + "description": "Read this", + **bulletin_overrides, + } + return drafts.SaveBulletinRequest.model_validate( + {"bulletin": bulletin, "audience": [], "status": "draft"} + ) + + +@pytest.mark.parametrize("blank", ["", " ", "\t", "\n", " \t\n "]) +def test_a_blank_schedule_boundary_becomes_unset(blank) -> None: + """The widget's DatePicker writes ``""`` for unset. + + ``EssBulletinInput.startDate``/``endDate`` are nullable ``DateTimeOffset``, + so an empty string fails the backend's *model binding* rather than its + validation — an opaque 400 instead of a renderable field error. + """ + request = _save_request(startDate=blank, endDate=blank) + + assert request.bulletin.startDate is None + assert request.bulletin.endDate is None + + +def test_a_draft_with_both_dates_blank_serializes_without_them() -> None: + """This is the exact payload ``save_bulletin`` sends to the API.""" + request = _save_request(startDate="", endDate="") + + payload = request.model_dump(mode="json", exclude_none=True) + + assert "startDate" not in payload["bulletin"] + assert "endDate" not in payload["bulletin"] + + +def test_no_empty_string_reaches_the_save_json() -> None: + """Checked against the serialized JSON, not just the model. + + A model-level assertion would still pass if serialization reintroduced the + sentinel, and the wire format is what the backend binds. + """ + request = _save_request(startDate=" ", endDate="") + + encoded = json.dumps(request.model_dump(mode="json", exclude_none=True)) + bulletin = json.loads(encoded)["bulletin"] + + assert "startDate" not in bulletin + assert "endDate" not in bulletin + assert '""' not in encoded + + +def test_omitted_dates_and_blank_dates_produce_the_same_payload() -> None: + """"Cleared in the editor" and "never set" must be indistinguishable.""" + blank = _save_request(startDate="", endDate="").model_dump( + mode="json", exclude_none=True + ) + omitted = _save_request().model_dump(mode="json", exclude_none=True) + + assert blank == omitted + + +def test_valid_instants_survive_the_blank_coercion_untouched() -> None: + """The backend owns the canonical schedule; never rewrite a real instant.""" + request = _save_request( + startDate="2026-09-01T00:00:00.000Z", + endDate="2026-10-01T23:59:59.999Z", + ) + + payload = request.model_dump(mode="json", exclude_none=True) + + assert payload["bulletin"]["startDate"] == "2026-09-01T00:00:00.000Z" + assert payload["bulletin"]["endDate"] == "2026-10-01T23:59:59.999Z" + + +@pytest.mark.parametrize( + "instant", + [ + "2026-09-01T08:30:00+02:00", + "2026-09-01T00:00:00Z", + "2026-09-01T00:00:00.0000000+00:00", + ], +) +def test_aware_instants_are_passed_through_byte_for_byte(instant) -> None: + request = _save_request(startDate=instant) + + assert request.bulletin.startDate == instant + + +def test_only_one_blank_boundary_is_coerced_independently() -> None: + """An open-ended schedule (start set, no end) is legitimate.""" + request = _save_request(startDate="2026-09-01T00:00:00.000Z", endDate="") + + payload = request.model_dump(mode="json", exclude_none=True) + + assert payload["bulletin"]["startDate"] == "2026-09-01T00:00:00.000Z" + assert "endDate" not in payload["bulletin"] + + +def test_a_published_save_with_blank_dates_still_reaches_the_backend() -> None: + """Missing dates on publish stay a *backend* validation concern. + + The contract layer must not pre-empt it: WeveNova owns publish-time + completeness and returns a structured, field-bound code the widget can + render. Rejecting here would replace that with a generic contract error, and + sending ``""`` would turn it into a JSON binding failure instead. + """ + request = drafts.SaveBulletinRequest.model_validate( + { + "bulletin": { + "type": "standard", + "title": "Quarterly update", + "description": "Read this", + "startDate": "", + "endDate": "", + }, + "audience": ["group-a"], + "status": "published", + } + ) + + payload = request.model_dump(mode="json", exclude_none=True) + + assert payload["status"] == "published" + assert "startDate" not in payload["bulletin"] + assert "endDate" not in payload["bulletin"] + + +def test_the_editor_draft_keeps_the_empty_string_representation() -> None: + """Coercion is a *save-path* concern only. + + The editor draft is what the DatePicker renders, and it treats ``""`` — not + ``null`` — as unset, so the two directions must not be conflated. + """ + draft = drafts.build_create_draft(drafts.SuggestedBulletinDraft(startDate="")) + + assert draft.startDate == "" + assert draft.endDate == "" + + +@pytest.mark.parametrize("blank", ["", " ", "\t"]) +def test_without_blank_schedule_drops_only_blank_schedule_keys(blank) -> None: + """The duplicate path's coercion, which bypasses BulletinInput entirely.""" + cleaned = drafts.without_blank_schedule( + { + "type": "standard", + "title": "Keep me", + "description": blank, + "startDate": blank, + "endDate": "2026-10-01T23:59:59.999Z", + } + ) + + assert "startDate" not in cleaned + assert cleaned["endDate"] == "2026-10-01T23:59:59.999Z" + # Only the two schedule keys are in scope; a blank description is real + # authored state the backend validates. + assert cleaned["description"] == blank + assert cleaned["title"] == "Keep me" + + +def test_without_blank_schedule_leaves_non_string_values_alone() -> None: + """A null from the backend is already 'unset' and must round-trip.""" + cleaned = drafts.without_blank_schedule({"startDate": None, "endDate": None}) + + assert cleaned == {"startDate": None, "endDate": None} + + +def test_the_two_blank_schedule_paths_agree() -> None: + """One rule, two call sites; they must not drift.""" + for value in ("", " ", "\t\n", "2026-09-01T00:00:00.000Z", None): + via_helper = "startDate" not in drafts.without_blank_schedule( + {"startDate": value} + ) + via_model = drafts.is_blank_schedule_value(value) + assert via_helper == via_model, value From 0c8a65829a77ab79215506085893ba1e26895884 Mon Sep 17 00:00:00 2001 From: Reilly Bova Date: Fri, 18 Sep 2026 10:35:28 -0700 Subject: [PATCH 2/2] fix(announcements): preserve repairable read-only draft inputs Retain typed primary actions and incomplete targets in editable copies without relaxing structural, scope, or write constraints. Explain Standard priority labels in the model-facing schema and distinguish parsing from backend action validity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15e9d51c-c328-48e6-9948-8819f9e57f90 --- .../agentconfig_org_announcements/drafts.py | 69 ++++------ .../test_drafts.py | 127 ++++++++++++------ 2 files changed, 114 insertions(+), 82 deletions(-) diff --git a/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/drafts.py b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/drafts.py index 31d5ffe52..faaeb4077 100644 --- a/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/drafts.py +++ b/solutions/ess-maker-skills/src/mcp/agentconfig_org_announcements/drafts.py @@ -10,8 +10,9 @@ ``suggestedBulletinDraftSchema`` exactly and is deliberately narrower than the canonical record: it cannot carry a bulletin ID, a persisted lifecycle status, creator/modifier identity, audit timestamps, or any backend version - or storage field. A suggestion is unpublished client state, never an - authorization to write. + or storage field. It carries both proposed content and editable copies, + including actions that need repair. Opening that unpublished client state + is never an authorization to write or proof of save/publish validity. ``AnnouncementEditorDraft`` and the ``open_org_announcements`` payloads The *output* contract consumed by the widget. ``build_create_draft`` @@ -34,6 +35,7 @@ from pydantic import ( BaseModel, ConfigDict, + Field, ModelWrapValidatorHandler, PrivateAttr, model_validator, @@ -144,11 +146,11 @@ class BulletinAction(StrictModel): payload carrying the *wrong* target for its type is malformed at the contract level, so it is rejected here rather than forwarded. - A *missing or blank* target is deliberately still accepted. A Draft is - allowed to be incomplete — the maker can add the URL later — and WeveNova - owns publish-time completeness validation, so rejecting it here would make - ``save_bulletin`` refuse drafts the backend accepts and would replace the - backend's field-level error code with a generic contract error. + A *missing or blank* target is structurally representable so read-only + editors can retain content for repair. Mutation input also uses this model, + but WeveNova validates every present action on both Draft save and Publish. + Parsing a request here does not establish backend acceptance; service-owned + validation failures retain their field-level error codes. """ actionType: BulletinActionType @@ -167,42 +169,33 @@ def require_matching_target(self) -> "BulletinAction": class SuggestedBulletinAction(BulletinAction): - """An action inside a *pre-hydrated* create suggestion. - - Stricter than :class:`BulletinAction` on purpose. A stored Draft may hold a - half-finished action the maker is still editing, but a suggestion is - content the model proposed and the maker has not typed: hydrating the - editor with an ``externalLink`` that has no URL, or a ``copilotChat`` with - no prompt, presents an action that cannot work as if it were reviewed - state. The suggestion is rejected so the maker gets an explicit opener - error instead of a silently broken button. - """ + """A structurally typed action in read-only proposed or copied content. - @model_validator(mode="after") - def require_actionable_target(self) -> "SuggestedBulletinAction": - if self.actionType == "externalLink": - if not (self.url or "").strip(): - raise ValueError( - "a suggested externalLink action requires a non-blank url" - ) - elif not (self.prompt or "").strip(): - raise ValueError( - "a suggested copilotChat action requires a non-blank prompt" - ) - return self + A missing own target remains available for explicit repair in the editor. + Discriminators, field types, unknown fields and mismatched target members + still use :class:`BulletinAction` validation. Action readiness belongs to + save/publish validation, not to opening an unsaved working copy. + """ class SuggestedBulletinDraft(StrictModel): - """A partial, reviewable proposal for a new announcement. + """Partial proposed or copied content for an unsaved create editor. Every field is optional; omitted fields fall back to the editor defaults in :func:`build_create_draft`. An explicit empty string is a real draft value and is preserved so the widget surfaces its normal validation instead of - silently substituting a default. + silently substituting a default. Primary actions are retained for repair + even when incompatible with the selected announcement type. """ type: Optional[AnnouncementType] = None - priority: Optional[AnnouncementPriority] = None + priority: Optional[AnnouncementPriority] = Field( + default=None, + description=( + "Standard announcement priority: 0 = Important; 1 = Informational. " + "Omitting priority defaults to Informational (1). Omit for Alert announcements." + ), + ) title: Optional[str] = None description: Optional[str] = None primaryAction: Optional[SuggestedBulletinAction] = None @@ -249,12 +242,13 @@ def normalize_schedule(self) -> "SuggestedBulletinDraft": return self @model_validator(mode="after") - def reject_alert_incompatible_fields(self) -> "SuggestedBulletinDraft": - """An Alert has no priority control and no secondary action. + def reject_alert_standard_only_fields(self) -> "SuggestedBulletinDraft": + """Keep the frontend's type-aware projection of Standard-only fields. Only an *explicit* incompatible value is rejected. Omitted Standard fields still receive their required editor defaults, because the widget - keeps them in the draft even while they are hidden. + keeps them in the draft even while they are hidden. A primary action, + unlike these Standard-only fields, stays visible for explicit repair. """ if self.type != "alert": return self @@ -264,11 +258,6 @@ def reject_alert_incompatible_fields(self) -> "SuggestedBulletinDraft": raise ValueError( "alert announcements do not support a secondary action" ) - if ( - self.primaryAction is not None - and self.primaryAction.actionType != "externalLink" - ): - raise ValueError("alert actions must be externalLink actions") return self @model_validator(mode="after") diff --git a/tests/mcp/agentconfig_org_announcements/test_drafts.py b/tests/mcp/agentconfig_org_announcements/test_drafts.py index c2cd0a77d..dd1a24393 100644 --- a/tests/mcp/agentconfig_org_announcements/test_drafts.py +++ b/tests/mcp/agentconfig_org_announcements/test_drafts.py @@ -67,6 +67,7 @@ {"version": 3}, {"etag": 'W/"3"'}, {"titleId": "title-1"}, + {"tenantId": "tenant-1"}, {"standardPriority": 0}, {"standardSecondaryAction": None}, ], @@ -107,10 +108,21 @@ def test_every_omitted_field_keeps_its_editor_default() -> None: assert result == {**EMPTY_EDITOR_DRAFT, "title": "Only a title"} -def test_priority_maps_to_standard_priority() -> None: - suggestion = drafts.SuggestedBulletinDraft(priority=0) +@pytest.mark.parametrize("priority", [0, 1], ids=["important", "informational"]) +def test_priority_maps_to_standard_priority(priority: int) -> None: + suggestion = drafts.SuggestedBulletinDraft(priority=priority) - assert drafts.build_create_draft(suggestion).standardPriority == 0 + assert drafts.build_create_draft(suggestion).standardPriority == priority + + +def test_suggestion_schema_explains_priority_labels_and_default() -> None: + priority = drafts.SuggestedBulletinDraft.model_json_schema()["properties"]["priority"] + + assert "0 = Important" in priority["description"] + assert "1 = Informational" in priority["description"] + assert "defaults to Informational (1)" in priority["description"] + assert priority["default"] is None + assert next(variant["enum"] for variant in priority["anyOf"] if "enum" in variant) == [0, 1] def test_secondary_action_maps_to_standard_secondary_action() -> None: @@ -187,18 +199,23 @@ def test_alert_with_explicit_secondary_action_is_rejected() -> None: ) -def test_alert_with_a_copilot_chat_action_is_rejected() -> None: - with pytest.raises(ValidationError): - drafts.SuggestedBulletinDraft.model_validate( - { - "type": "alert", - "primaryAction": { - "actionType": "copilotChat", - "label": "Ask", - "prompt": "Explain", - }, - } - ) +def test_alert_with_a_copilot_chat_action_opens_for_repair() -> None: + action = { + "actionType": "copilotChat", + "label": "Ask", + "prompt": "https://contoso.example/not-a-website-action", + } + suggestion = drafts.SuggestedBulletinDraft.model_validate( + {"type": "alert", "primaryAction": action} + ) + + result = drafts.build_create_draft(suggestion) + + assert result.type == "alert" + assert result.id is None + assert result.primaryAction.model_dump(exclude_unset=True) == action + assert result.primaryAction.url is None + assert suggestion.retry_payload() == {"type": "alert", "primaryAction": action} def test_alert_still_receives_the_hidden_standard_defaults() -> None: @@ -566,29 +583,55 @@ def test_stored_schedules_are_not_re_normalized() -> None: # -------------------------------------------------------------------------- -def test_a_suggested_external_link_requires_a_non_blank_url() -> None: - """A suggestion is unreviewed model output, not a half-typed draft.""" - for url in (None, "", " "): - with pytest.raises(ValidationError): - drafts.SuggestedBulletinDraft( - primaryAction={ - "actionType": "externalLink", - "label": "Open", - **({} if url is None else {"url": url}), - } - ) +@pytest.mark.parametrize( + ("action_type", "target"), + [("externalLink", "url"), ("copilotChat", "prompt")], +) +@pytest.mark.parametrize("target_values", [{}, {"value": None}, {"value": ""}, {"value": " "}]) +@pytest.mark.parametrize( + ("announcement_type", "field", "editor_field"), + [ + ("standard", "primaryAction", "primaryAction"), + ("standard", "secondaryAction", "standardSecondaryAction"), + ("alert", "primaryAction", "primaryAction"), + ], +) +def test_read_only_suggestions_preserve_incomplete_actions_for_repair( + action_type, target, target_values, announcement_type, field, editor_field +) -> None: + action = { + "actionType": action_type, + "label": "", + **({target: target_values["value"]} if target_values else {}), + } + original = {"type": announcement_type, field: action} + suggestion = drafts.SuggestedBulletinDraft.model_validate(original) + result = drafts.build_create_draft(suggestion) -def test_a_suggested_copilot_chat_requires_a_non_blank_prompt() -> None: - for prompt in (None, "", " "): - with pytest.raises(ValidationError): - drafts.SuggestedBulletinDraft( - secondaryAction={ - "actionType": "copilotChat", - "label": "Ask", - **({} if prompt is None else {"prompt": prompt}), - } - ) + assert result.id is None + assert getattr(result, editor_field).model_dump(exclude_unset=True) == action + assert suggestion.retry_payload() == original + + +@pytest.mark.parametrize( + "action", + [ + {"actionType": "unknown", "label": "Ask"}, + {"label": "Ask", "prompt": "Explain"}, + {"actionType": "copilotChat"}, + {"actionType": "copilotChat", "label": None}, + {"actionType": "copilotChat", "label": 123}, + {"actionType": "copilotChat", "label": "Ask", "prompt": []}, + {"actionType": "copilotChat", "label": "Ask", "url": "https://contoso.example"}, + {"actionType": "externalLink", "label": "Open", "prompt": "Explain"}, + {"actionType": "externalLink", "label": "Open", "url": {"value": "invalid"}}, + {"actionType": "externalLink", "label": "Open", "id": "action-1"}, + ], +) +def test_repairable_suggestions_still_reject_structurally_invalid_actions(action) -> None: + with pytest.raises(ValidationError): + drafts.SuggestedBulletinDraft(primaryAction=action) def test_complete_suggested_actions_are_accepted() -> None: @@ -620,12 +663,12 @@ def test_complete_suggested_actions_are_accepted() -> None: {"actionType": "copilotChat", "label": "Ask", "prompt": " "}, ], ) -def test_ordinary_saves_still_accept_incomplete_draft_actions(action) -> None: - """A Draft may be unfinished; WeveNova owns publish-time completeness. +def test_save_request_parsing_delegates_present_action_validity_to_backend(action) -> None: + """Pydantic acceptance is not backend acceptance. - Rejecting these here would make ``save_bulletin`` refuse drafts the backend - accepts and would replace the backend's field-level code with a generic - contract error the widget cannot bind to an input. + WeveNova validates every present action, even on a Draft. The request model + leaves that validation to the backend so its field-level errors reach the + widget instead of being replaced by a generic contract error. """ request = drafts.SaveBulletinRequest.model_validate( { @@ -665,7 +708,7 @@ def test_a_stored_incomplete_action_still_projects_into_the_editor() -> None: def test_mismatched_action_targets_are_still_rejected_everywhere() -> None: """Wrong-target actions are malformed at the contract level, not merely - incomplete, so both the strict and the permissive model reject them.""" + incomplete, so read-only and mutation models both reject them.""" with pytest.raises(ValidationError): drafts.SaveBulletinRequest.model_validate( {