Skip to content

Commit 198e514

Browse files
authored
Merge pull request #1751 from paullizer/paullizer-email-review-flow
Fix orchestrated email review flow and Cosmos response contracts
2 parents 2bc278a + d473f8d commit 198e514

47 files changed

Lines changed: 34054 additions & 119 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
---
2+
applyTo: '**/*.py'
3+
---
4+
5+
# Cosmos document response boundaries
6+
7+
Point reads and returned create/replace/upsert/patch documents in the application's
8+
pinned Cosmos SDK are `CosmosDict`, a mapping subclass. Query rows and nested JSON
9+
objects are plain dictionaries. `deepcopy(CosmosDict)` preserves its subclass.
10+
11+
Storage owners must normalize a point response **before copying or passing it to
12+
an exact-dictionary/strict JSON contract**, using the bootstrap-free
13+
`normalize_cosmos_document` in `cosmos_document_utils.py`. This helper accepts
14+
`Mapping`, returns a plain top-level dictionary, preserves all fields and `_etag`,
15+
and rejects invalid types with `TypeError`. Translate invalid responses through
16+
the owner's existing failure path. Never normalize arbitrary iterables into a
17+
document, substitute `{}`, strip ETags, or recursively repair domain values.
18+
19+
Keep domain validators strict. Normalization is not authorization, sanitization,
20+
metadata stripping, or permission to replay an uncertain external effect. Raw SDK
21+
responses are compatible with mapping consumers; an exposed wrapper by itself is
22+
not a product defect. Prove a strict downstream mismatch with a runtime regression
23+
before changing otherwise-compatible production code.
24+
25+
New Cosmos test containers use the **installed application's SDK `CosmosDict`**
26+
for point reads and returned write documents by default. Query rows stay plain.
27+
Plain point responses require an explicit compatibility mode. Preserve ETags,
28+
partition routing, conditional updates, failure injection and transaction atomicity.
29+
Domain-level repository mocks do not need to impersonate Cosmos.
30+
31+
## Reviewed inventory and deterministic CI
32+
33+
`functional_tests/cosmos_response_boundaries.json` records point/query calls,
34+
helper references, consumer classifications and evidence by storage function.
35+
It separately inventories Cosmos-shaped inline/shared test containers. It is a
36+
review/change guard, not an interprocedural type checker or proof that every
37+
external service behaves correctly.
38+
39+
After adding/changing a storage boundary or double:
40+
41+
1. Run `python scripts/check_cosmos_response_boundaries.py --refresh`. Changed
42+
implementations become `review-required`; regeneration never approves them.
43+
2. Review every changed entry's actual consumer. Classify it as
44+
`normalized-strict`, `sdk-compatible`, `response-unused`, or `not-cosmos`
45+
with a specific contract and runtime test reference. A `not-cosmos` entry
46+
needs a concrete explanation of the similarly named non-Cosmos API.
47+
3. New/changed Cosmos doubles must use `sdk-default`; `legacy-inline` records
48+
document unchanged pre-migration doubles, not permission to introduce another.
49+
Add a real SDK runtime regression to the inventory's `tests` evidence for
50+
changed owners. The gate automatically executes owner-runtime and SDK-default
51+
evidence, in addition to its core contracts, so references are not merely
52+
checked for existence.
53+
4. Run `python scripts/check_cosmos_response_boundaries.py --run-contracts`.
54+
This checks the inventory and runs its selected suites with
55+
`-p test_support.cosmos_network_guard`. Set `PYTHONPATH` to the application
56+
and functional test directories using the platform's path separator.
57+
58+
Do not copy a classification blindly, point every owner at a utility-only test,
59+
or treat a grep match/count as evidence of a defect. Reviews and references are
60+
invalidated when their implementations disappear or change. Legacy inline
61+
doubles remain explicitly recorded for staged, owner-specific migration.
62+
Unchanged legacy mapping consumers distinguish source review plus SDK API
63+
evidence from owner-runtime SDK coverage. Their source fingerprints form a
64+
closed baseline; a new or changed owner cannot borrow that limited assurance.
65+
Replace its baseline entry with real owner-runtime evidence, rather than
66+
expanding the source-only baseline.
67+
Owner-runtime evidence executes representative real-owner behavior; it does not
68+
claim exhaustive branch coverage for every function in that storage owner.
69+
Existing test-local compatibility seams also have a closed fingerprint baseline.
70+
A new or changed point override needs explicit `fault-injection` runtime evidence,
71+
which the gate executes; it cannot silently retain an old easy response override.
72+
73+
The dedicated `cosmos-response-contract-check.yml` installs the existing pinned
74+
application/test requirements, uses no cloud credentials, and blocks socket/DNS
75+
network access before test collection. New application storage modules and test
76+
doubles cannot bypass it by choosing a new filename.
77+
Tokenizer data needed by the pinned application dependency is warmed during
78+
dependency setup into a workspace-local cache, before network access is blocked.
79+
Tests keep the real context-budget calculation; they do not download assets or
80+
replace budget/token behavior merely to run offline.
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
name: Cosmos Response Contracts
2+
3+
on:
4+
pull_request:
5+
paths:
6+
- 'application/**/*.py'
7+
- 'scripts/**/*.py'
8+
- 'deployers/**/*.py'
9+
- 'functional_tests/**/*.py'
10+
- 'functional_tests/cosmos_response_boundaries.json'
11+
- 'ui_tests/**/*.py'
12+
- '**/requirements*.txt'
13+
- '.github/instructions/cosmos-response-boundaries.instructions.md'
14+
- '.github/workflows/cosmos-response-contract-check.yml'
15+
workflow_dispatch:
16+
17+
permissions:
18+
contents: read
19+
20+
jobs:
21+
cosmos-contracts:
22+
runs-on: ubuntu-latest
23+
timeout-minutes: 20
24+
env:
25+
PYTHONPATH: application/single_app:functional_tests
26+
PYTHONDONTWRITEBYTECODE: '1'
27+
TIKTOKEN_CACHE_DIR: ${{ github.workspace }}/.cosmos-contract-assets/tiktoken
28+
steps:
29+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
30+
- uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
31+
with:
32+
python-version: '3.12'
33+
cache: pip
34+
cache-dependency-path: |
35+
application/single_app/requirements.txt
36+
requirements-dev.txt
37+
- name: Install pinned application and test dependencies
38+
run: python -m pip install -r application/single_app/requirements.txt -r requirements-dev.txt
39+
- name: Prepare pinned tokenizer dependency assets before network blocking
40+
run: python -c "import tiktoken; tiktoken.get_encoding('cl100k_base'); tiktoken.get_encoding('o200k_base')"
41+
- name: Require reviewed storage owners and doubles
42+
run: python scripts/check_cosmos_response_boundaries.py
43+
- name: Run contracts with external network blocked before collection
44+
run: python scripts/check_cosmos_response_boundaries.py --run-contracts
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# cosmos_document_utils.py
2+
"""Bootstrap-free normalization of Cosmos point document responses."""
3+
4+
from collections.abc import Mapping
5+
6+
7+
def normalize_cosmos_document(value):
8+
"""Return a plain top-level document, preserving fields and conditional ETags.
9+
10+
Nested values belong to the domain contract and are deliberately unchanged.
11+
This is not a JSON validator, authorization check, or metadata sanitizer.
12+
"""
13+
if not isinstance(value, Mapping):
14+
raise TypeError("A Cosmos document response must be a mapping.")
15+
return dict(value)

‎application/single_app/functions_orchestration_actions.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,13 @@ async def guard_function(invocation, next):
681681
level=logging.ERROR,
682682
extra={'action_ref': action_ref, 'error_type': type(exc).__name__},
683683
)
684-
failure = ActionExecutionError('An action function could not complete.')
684+
journal = current_operation_journal()
685+
failure = (
686+
journal.failure if journal is not None and journal.failure is not None
687+
else ActionExecutionError('An action function could not complete.')
688+
)
689+
if failure is exc:
690+
raise
685691
raise failure from exc
686692
refusal = result_refusal(invocation.result.value) if m365_step and invocation.result is not None else None
687693
if refusal is not None:

‎application/single_app/functions_orchestration_adapters.py‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,19 @@ def _m365_step_identity(step, context):
479479
return f'{root}\x00{step_id}', origin
480480

481481

482+
def _m365_card_callback(step, context, emit):
483+
def publish(reference):
484+
if not callable(emit):
485+
return
486+
# The producer logs a failed callback while retaining the saved reference.
487+
emit({
488+
'type': 'm365_pending_action_reference', 'reference': reference,
489+
'step_id': step.get('step_id'), 'run_id': _ctx(context, 'run_id', None),
490+
'user_message_id': _ctx(context, 'user_message_id', None),
491+
})
492+
return publish
493+
494+
482495
def _progress(step, capability_id, label, status='running'):
483496
step = step or {}
484497
return {
@@ -1936,10 +1949,13 @@ def run_action_invoke(step, context, *, settings, user_id, emit, cancel_requeste
19361949
from functions_orchestration_actions import invoke_action
19371950
from semantic_kernel_plugins.plugin_invocation_logger import sanitize_plugin_invocation_value
19381951
from functions_m365_citations import m365_display_time_zone
1952+
from functions_orchestration_m365 import orchestration_action_card_events
19391953

19401954
# Microsoft 365 email and event times are shown in the turn's browser time zone.
1941-
with m365_display_time_zone(_ctx(context, 'time_zone')), operation_step_scope(
1942-
step, context, user_id, named_inputs,
1955+
with (
1956+
orchestration_action_card_events(_m365_card_callback(step, context, emit)),
1957+
m365_display_time_zone(_ctx(context, 'time_zone')),
1958+
operation_step_scope(step, context, user_id, named_inputs),
19431959
):
19441960
result = asyncio.run(invoke_action(
19451961
action_ref, task, context, settings=settings, user_id=user_id,
@@ -2103,7 +2119,9 @@ def run_agent_invoke(step, context, *, settings, user_id, emit, cancel_requested
21032119
invocation_settings = deepcopy(settings)
21042120
access('agent', settings=invocation_settings, selector=selector)
21052121
from agent_delegation_runtime import delegation_citations, invoke_scoped_agent
2106-
from functions_orchestration_m365 import agent_loads_actions, agent_step_scope
2122+
from functions_orchestration_m365 import (
2123+
agent_loads_actions, agent_step_scope, orchestration_action_card_events,
2124+
)
21072125
from semantic_kernel_plugins.plugin_invocation_logger import get_plugin_logger
21082126

21092127
budget = _ctx(context, 'delegation_budget', None)
@@ -2128,9 +2146,11 @@ def scope(_target):
21282146
)
21292147
from functions_m365_citations import m365_display_time_zone
21302148

2131-
with m365_display_time_zone(_ctx(context, 'time_zone')), operation_step_scope(
2132-
step, context, user_id, named_inputs,
2133-
) as journal:
2149+
with (
2150+
orchestration_action_card_events(_m365_card_callback(step, context, emit)),
2151+
m365_display_time_zone(_ctx(context, 'time_zone')),
2152+
operation_step_scope(step, context, user_id, named_inputs) as journal,
2153+
):
21342154
async def invoke():
21352155
return await invoke_scoped_agent(
21362156
agent_cfg, task, identity=execution_identity, budget=budget,

‎application/single_app/functions_orchestration_bootstrap.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,12 @@
5656
upload_generated_file_artifact_stream_for_user,
5757
)
5858
from functions_workflow_result_store import WorkflowResultStore, _quota_bytes
59+
from cosmos_document_utils import normalize_cosmos_document
5960

6061

6162
def _document_response(value):
6263
"""Remove an SDK mapping wrapper; leave invalid types for the owning contract to reject."""
63-
return dict(value) if isinstance(value, dict) else value
64+
return normalize_cosmos_document(value) if isinstance(value, dict) else value
6465

6566

6667
def read_owned_conversation(user_id, conversation_id, *, request_timeout=None):
@@ -75,7 +76,7 @@ def read_owned_conversation(user_id, conversation_id, *, request_timeout=None):
7576
or conversation.get("orchestration_deleted") or conversation.get("deleted")
7677
):
7778
raise OutputUnavailableError("output_conversation_unavailable")
78-
return dict(conversation)
79+
return normalize_cosmos_document(conversation)
7980

8081

8182
def private_external_configuration_digest(value):

‎application/single_app/functions_orchestration_execution.py‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,9 @@ def _send(self, frame):
11011101
_log_failure("Execution progress delivery stopped.", self.record, exc)
11021102

11031103
def _progress(self, event):
1104+
if type(event) is dict and event.get("type") == "m365_pending_action_reference":
1105+
self._pending_action_created(event)
1106+
return
11041107
if type(event) is not dict or event.get("type") != "step":
11051108
return
11061109
extra = {
@@ -1118,6 +1121,72 @@ def _progress(self, event):
11181121
event.get("step_index"), event.get("capability_id"), **extra,
11191122
))
11201123

1124+
def _pending_action_created(self, event):
1125+
# The producer runs inside a separate request. Only its exact step-owned
1126+
# reference crosses the bridge; tool/model snapshots are never published.
1127+
from functions_orchestration_m365 import step_request_id
1128+
1129+
reference = event.get("reference")
1130+
step_id = event.get("step_id")
1131+
root = self.record.get("attempt_root_run_id") or self.record["id"]
1132+
if (
1133+
type(reference) is not dict
1134+
or event.get("run_id") != self.record["id"]
1135+
or reference.get("user_id") != self.record["user_id"]
1136+
or reference.get("conversation_id") != self.record["conversation_id"]
1137+
or not any(
1138+
step.get("step_id") == step_id for step in self.record["plan"]["steps"]
1139+
)
1140+
or reference.get("request_id") != step_request_id(f"{root}\x00{step_id}")
1141+
or not isinstance(reference.get("id"), str) or not reference["id"]
1142+
):
1143+
return
1144+
with self.lease.lock:
1145+
current = self.lease.read()
1146+
action_ids = list(dict.fromkeys([
1147+
*(current.get("m365_pending_action_ids") or []), reference["id"],
1148+
]))
1149+
self.lease.update({"m365_pending_action_ids": action_ids})
1150+
try:
1151+
from functions_msgraph_pending_actions import get_chat_pending_action_cards
1152+
1153+
cards = get_chat_pending_action_cards(
1154+
self.record["user_id"], self.record["conversation_id"],
1155+
request_id=reference["request_id"], action_ids=[reference["id"]],
1156+
)
1157+
for card in cards:
1158+
self._send(serialize_sse({
1159+
"type": "m365_pending_action", "pending_action": card,
1160+
"conversation_id": self.record["conversation_id"],
1161+
"run_id": self.record["id"], "step_id": step_id,
1162+
"user_message_id": self.record.get("user_message_id"),
1163+
"request_id": reference["request_id"],
1164+
}))
1165+
except Exception as error:
1166+
_log_failure("A saved outgoing action could not be displayed live.", self.record, error)
1167+
1168+
def _pending_action_payload(self):
1169+
try:
1170+
record = self._final_record or self.lease.read()
1171+
action_ids = record.get("m365_pending_action_ids") or []
1172+
if not action_ids:
1173+
return {}
1174+
from functions_msgraph_pending_actions import get_chat_pending_action_cards
1175+
1176+
cards = []
1177+
for offset in range(0, len(action_ids), 100):
1178+
cards.extend(get_chat_pending_action_cards(
1179+
self.record["user_id"], self.record["conversation_id"],
1180+
action_ids=action_ids[offset:offset + 100],
1181+
))
1182+
return {"m365_pending_actions": cards}
1183+
except Exception as error:
1184+
_log_failure("Saved outgoing actions could not be refreshed.", self.record, error)
1185+
return {"m365_pending_actions_error": {
1186+
"error": "m365_pending_actions_unavailable",
1187+
"message": "Outgoing actions could not be refreshed. Reload them; do not repeat the request.",
1188+
}}
1189+
11211190
def _persist(self, record_type, payload):
11221191
if record_type == "run":
11231192
try:
@@ -1571,6 +1640,8 @@ def _finalize(self, result, error, *, refreshes=0):
15711640
if workflow_result_reads and workflow_result_contexts else {}
15721641
),
15731642
}
1643+
if current.get("m365_pending_action_ids"):
1644+
metadata["m365_pending_action_ids"] = list(current["m365_pending_action_ids"])
15741645
thread = self._regeneration_thread()
15751646
if thread:
15761647
metadata["thread_info"] = thread
@@ -1659,6 +1730,7 @@ def _finalize(self, result, error, *, refreshes=0):
16591730
payload.update({
16601731
"metadata": document.get("metadata") or {}, "role": document["role"],
16611732
"replace_content": True, "blocked": blocked,
1733+
**self._pending_action_payload(),
16621734
})
16631735
return [build_content_event(answer), serialize_sse(strip_private_chat_checks(payload))]
16641736

@@ -1690,6 +1762,9 @@ def _finish(self, result, error):
16901762
finalization_status="failed", outputs=[],
16911763
),
16921764
]
1765+
terminal = json.loads(self._frames[-1].partition("data:")[2].strip())
1766+
terminal.update(self._pending_action_payload())
1767+
self._frames[-1] = serialize_sse(terminal)
16931768
if self.record.get("requires_fresh_review"):
16941769
status = (self._final_record or {}).get("status") or "failed"
16951770
state = {"completed": "completed", "failed": "failed", "cancelled": "interrupted"}.get(status)

0 commit comments

Comments
 (0)