From e85c06d43c339ae96fc9c5fb24c8483d14d84f50 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Tue, 6 Oct 2026 15:03:45 -0400 Subject: [PATCH 1/4] Fix Orchestrate action and agent steps and local agent plans Action and agent steps now trust the signed-in session the same way manual chat does, instead of re-attesting the live configuration. Microsoft 365 action steps no longer fail with "A required retained result is unavailable or changed", and local agents (including agents with Microsoft 365 actions) can run as plan steps. - Session-trusted references for action and agent sources (access is still re-checked on every read; older attested results stay readable). - Agent steps enter a Microsoft 365 step scope for the agent's own actions. - Specific refusal codes are kept in diagnostics, and integration failures get a dedicated message. - Composer seeds an explicit @agent or @model tag into the Orchestrate plan. Fixes #1660 Fixes #1661 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../single_app/agent_delegation_runtime.py | 27 +++- application/single_app/config.py | 2 +- .../single_app/functions_m365_runtime.py | 37 +++-- .../functions_orchestration_adapters.py | 41 +++-- .../functions_orchestration_bootstrap.py | 38 +++-- .../functions_orchestration_executor.py | 2 +- ...ns_orchestration_external_configuration.py | 17 +- ...unctions_orchestration_external_sources.py | 72 +++++++-- .../functions_orchestration_m365.py | 44 ++++-- .../functions_orchestration_schema.py | 16 +- .../v2_ui/src/components/chat/Composer.tsx | 20 ++- ...st_orchestration_catalog_auto_execution.py | 9 +- .../test_orchestration_external_bootstrap.py | 144 ++++++++++++++++- ...hestration_external_capture_integration.py | 12 +- ...stration_external_configuration_capture.py | 74 ++++----- ...rchestration_external_preflight_adapter.py | 30 ++-- .../test_orchestration_external_sources.py | 36 ++++- .../test_orchestration_local_agent_capture.py | 9 +- .../test_orchestration_m365_actions.py | 68 ++++++++ ...t_orchestration_session_trusted_sources.py | 149 ++++++++++++++++++ .../test_orchestration_visual_outputs.py | 6 +- 21 files changed, 703 insertions(+), 150 deletions(-) create mode 100644 functional_tests/test_orchestration_session_trusted_sources.py diff --git a/application/single_app/agent_delegation_runtime.py b/application/single_app/agent_delegation_runtime.py index 254bc5af8..1d5c37572 100644 --- a/application/single_app/agent_delegation_runtime.py +++ b/application/single_app/agent_delegation_runtime.py @@ -1,8 +1,9 @@ # agent_delegation_runtime.py """Isolated asynchronous agent calls. -Version: 0.261.127 +Version: 0.261.269 Implemented in: 0.261.093 +Optional step scope entered inside the agent's bridge in: 0.261.269 Provider and loader imports are deliberately lazy: plugin discovery imports the Call agent class before the application's Semantic Kernel loader is initialized. @@ -301,8 +302,13 @@ async def _target_messages(target, task, context, frame, settings): return messages, citations -async def execute_target(target, task, context, frame): - """Invoke precisely this canonical target; never select a default or by name.""" +async def execute_target(target, task, context, frame, *, scope=None): + """Invoke precisely this canonical target; never select a default or by name. + + ``scope``, when given, returns a context manager entered inside the target's own bridge, + so request state it installs, such as an orchestration step's Microsoft 365 context, + reaches the agent's tools. + """ active = current_agent_execution() if active is not None and active.invocation_capture is not None and ( frame.invocation_capture is not active.invocation_capture @@ -320,7 +326,7 @@ async def execute_target(target, task, context, frame): _prepare_captured_agent(target, frame.invocation_settings, frame.identity, frame.invocation_capture) bridge = frame.identity.bridge(target) if frame.identity.bridge else nullcontext() kernel = None - with bridge: + with bridge, (scope(target) if scope is not None else nullcontext()): _check_cancelled(frame) _preflight_captured_agent(target, frame) settings = deepcopy(frame.invocation_settings) if frame.invocation_settings is not None else get_settings() @@ -632,8 +638,12 @@ def prepare_agent_execution(agent, reference, *, user_id, settings, conversation async def invoke_scoped_agent( reference, task, *, identity, budget, cancel_requested=None, settings=None, invocation_capture=None, + scope=None, ): - """Worker-safe root execution; Flask compatibility belongs to the captured bridge.""" + """Worker-safe root execution; Flask compatibility belongs to the captured bridge. + + ``scope`` is passed to ``execute_target`` and entered inside the agent's bridge. + """ parent = current_agent_execution() if parent is not None and parent.invocation_capture is not None: parent.invocation_capture.refuse() @@ -651,7 +661,12 @@ async def invoke_scoped_agent( invocation_settings=deepcopy(settings) if invocation_capture is not None and settings is not None else None, ) with agent_execution(frame) as frame: - result = await await_agent_operation(execute_target(canonical, task, "", frame), frame) + # The existing call shape is kept when no scope is supplied. + target = ( + execute_target(canonical, task, "", frame, scope=scope) if scope is not None + else execute_target(canonical, task, "", frame) + ) + result = await await_agent_operation(target, frame) if frame.invocation_capture is not None: frame.invocation_capture.require_valid(captured=True) return result diff --git a/application/single_app/config.py b/application/single_app/config.py index 175ad4db0..9e17b837c 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -101,7 +101,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.268" +VERSION = "0.261.269" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_m365_runtime.py b/application/single_app/functions_m365_runtime.py index 181677b1e..bdc2cb5e1 100644 --- a/application/single_app/functions_m365_runtime.py +++ b/application/single_app/functions_m365_runtime.py @@ -564,11 +564,26 @@ def _step_action_manifest(action): def step_selected_m365_manifests(selection, actor_user_id): - """Resolve an orchestration action step's selection from current storage, never the plan. + """Resolve an orchestration step's Microsoft 365 selection from current storage, never the plan. - The selection names the approved step's saved action by its exact reference, and the - catalog reauthorizes that reference on every read. + An action step names the approved step's saved action by its exact reference, and the + catalog reauthorizes that reference on every read. An agent step names the step's agent, + whose Microsoft 365 actions and overrides resolve the way a chat-selected agent's do. """ + if isinstance(selection, dict) and selection.get("kind") == "agent": + agent = selection.get("agent") + if not isinstance(agent, dict) or not (agent.get("id") or agent.get("name")): + raise M365PolicyError( + "m365_action_selection_unavailable", + "The selected Microsoft 365 actions could not be resolved.", + ) + manifests, _ = workflow_m365_manifests({ + "user_id": actor_user_id, + "group_id": agent.get("group_id") if agent.get("is_group") else None, + "selected_agent": agent, + "tasks": [], + }) + return manifests if not isinstance(selection, dict) or selection.get("kind") != "action": raise M365PolicyError( "m365_action_selection_unavailable", @@ -697,20 +712,25 @@ def _stopping_approval_id(error): @contextmanager def step_m365_context(*, user_id, conversation_id, request_id, selection, origin=None): - """Authorize Microsoft 365 for one orchestration action step inside its execution bridge. + """Authorize Microsoft 365 for one orchestration action or agent step inside its bridge. A plan step runs in a fresh request context, so the chat request's Microsoft 365 context never reaches it. This context uses the signed-in user as actor and data user, - selects only the step's own action, and never enables send, invite or read-state - functions. Shared conversations are refused: their source-sharing approvals resume chat - requests, not plan steps. Delegated sign-in is checked here, before any model call or - Microsoft Graph request. + selects only the step's own action or agent, and never enables send, invite or + read-state functions. Shared conversations are refused: their source-sharing approvals + resume chat requests, not plan steps. Delegated sign-in is checked here, before any + model call or Microsoft Graph request. An agent step whose agent loads no Microsoft 365 + action gets no context and leaves no request record. """ if not has_request_context() or (session.get("user") or {}).get("oid") != user_id: raise M365PolicyError("m365_session_required", "Sign in again to use Microsoft 365.") conversation, _access, shared = _conversation_access(user_id, conversation_id) if conversation is None or conversation.get("user_id") != user_id: raise M365PolicyError("conversation_not_found", "Conversation not found.") + selected = step_selected_m365_manifests(selection, user_id) + if not selected and selection.get("kind") == "agent": + yield None + return if shared is not None: raise M365PolicyError( "m365_shared_conversation_unsupported", @@ -727,7 +747,6 @@ def step_m365_context(*, user_id, conversation_id, request_id, selection, origin audience_version=_audience_version(conversation, None), ) install_m365_context(context) - selected = step_selected_m365_manifests(selection, user_id) manifests = _readable_m365_manifests(selected) if not manifests: if selected: diff --git a/application/single_app/functions_orchestration_adapters.py b/application/single_app/functions_orchestration_adapters.py index ad0d97776..0a1f942f0 100644 --- a/application/single_app/functions_orchestration_adapters.py +++ b/application/single_app/functions_orchestration_adapters.py @@ -1903,14 +1903,16 @@ def run_action_invoke(step, context, *, settings, user_id, emit, cancel_requeste visual_request = _step_visuals(context, settings, task, step=step) task = _with_conversation_reference(task, context) try: - invocation_kwargs = { - 'invocation_capture': _external_invocation_capture( - step, context, settings, user_id=user_id, - capability_id=CAPABILITY_ACTION_INVOKE, selector=action_ref, - ), - } + # The step trusts the signed-in session, like classic chat: current access to the + # conversation, the run's step and this exact action is rechecked here, and the action + # then runs without configuration attestation. + access = _external_invocation_capture( + step, context, settings, user_id=user_id, + capability_id=CAPABILITY_ACTION_INVOKE, selector=action_ref, + ) settings = deepcopy(settings) - invocation_kwargs['invocation_capture']('action', settings=settings, selector=action_ref) + access('action', settings=settings, selector=action_ref) + invocation_kwargs = {} if visual_request: invocation_kwargs['visual_request'] = visual_request invocation_kwargs['m365_request_key'], invocation_kwargs['m365_origin'] = _m365_step_identity(step, context) @@ -1922,7 +1924,6 @@ def run_action_invoke(step, context, *, settings, user_id, emit, cancel_requeste cancel_requested=lambda: _is_cancelled(cancel_requested), **invocation_kwargs, )) - invocation_kwargs['invocation_capture'].require_valid(captured=True) except (AgentExecutionCancelled, OrchestrationInvocationCancelledError): return _cancelled_result('Action execution was cancelled.') except ( @@ -2037,13 +2038,17 @@ def run_agent_invoke(step, context, *, settings, user_id, emit, cancel_requested expected_selector = f"{reference['scope_type']}:{reference['scope_id']}:{reference['id']}" if type(selector) is not str or selector != expected_selector: raise ResultContractError('result_external_configuration_selection_mismatch') - invocation_capture = _external_invocation_capture( + # Like an action step, the agent trusts the signed-in session: current access to the + # conversation, the run's step and this exact agent is rechecked here, and the agent then + # runs as a delegated agent does in chat, without configuration attestation. + access = _external_invocation_capture( step, context, settings, user_id=user_id, capability_id=CAPABILITY_AGENT_INVOKE, selector=selector, ) invocation_settings = deepcopy(settings) - invocation_capture('agent', settings=invocation_settings, selector=selector) + access('agent', settings=invocation_settings, selector=selector) from agent_delegation_runtime import delegation_citations, invoke_scoped_agent + from functions_orchestration_m365 import agent_loads_actions, agent_step_scope from semantic_kernel_plugins.plugin_invocation_logger import get_plugin_logger budget = _ctx(context, 'delegation_budget', None) @@ -2053,12 +2058,22 @@ def run_agent_invoke(step, context, *, settings, user_id, emit, cancel_requested plugin_logger = get_plugin_logger() conversation_id = _ctx(context, 'conversation_id', None) seen_before = {id(inv) for inv in budget.invocations()} + scope = None + if agent_loads_actions(agent_cfg): + # An agent's Microsoft 365 actions need the step's own Microsoft 365 context, entered + # inside the agent's bridge; an agent with no such action gets none. The catalog + # entry, not the stored record, carries the agent's normalized scope. + m365_request_key, m365_origin = _m365_step_identity(step, context) + + def scope(_target): + return agent_step_scope( + agent_cfg, user_id=user_id, conversation_id=conversation_id, + request_key=m365_request_key, origin=m365_origin, + ) result = asyncio.run(invoke_scoped_agent( agent_cfg, task, identity=execution_identity, budget=budget, - cancel_requested=cancel_requested, - settings=invocation_settings, invocation_capture=invocation_capture, + cancel_requested=cancel_requested, scope=scope, )) - invocation_capture.require_valid(captured=True) except (AgentExecutionCancelled, OrchestrationInvocationCancelledError): return _cancelled_result('Agent execution was cancelled.') except ( diff --git a/application/single_app/functions_orchestration_bootstrap.py b/application/single_app/functions_orchestration_bootstrap.py index 899847609..56bce51b5 100644 --- a/application/single_app/functions_orchestration_bootstrap.py +++ b/application/single_app/functions_orchestration_bootstrap.py @@ -1,19 +1,22 @@ # functions_orchestration_bootstrap.py """Application-owned factories shared by web requests and scheduler continuations. -Version: 0.261.245 +Version: 0.261.269 Unlike the result/rendering services, this is an application composition root. Import it only after config has initialized the existing clients. Registering the artifact factory performs no I/O; each use rebuilds current actor/source access. External-source identity trusts the signed-in session's app roles, like classic chat, and makes no Microsoft Graph calls (0.261.209). Merged documents are rendered -from their own original files through a screening-aware reader (0.261.245). +from their own original files through a screening-aware reader (0.261.245). Agent +and action steps are session-trusted: their access is rechecked on every capture, +but their configuration is not attested (0.261.269). """ import hashlib import hmac from copy import deepcopy +from threading import Lock from flask import has_request_context @@ -30,7 +33,9 @@ OrchestrationExternalConfigurationAttestor, _read_metadata, ) from functions_orchestration_external_metadata import build_external_metadata_reader -from functions_orchestration_external_sources import OrchestrationExternalSourceProvider +from functions_orchestration_external_sources import ( + SESSION_TRUSTED_CAPABILITIES, SESSION_TRUSTED_SOURCE_TYPES, OrchestrationExternalSourceProvider, +) from functions_orchestration_output_store import ( OrchestrationOutputStore, OutputError, OutputUnavailableError, ) @@ -364,24 +369,37 @@ def read_external_conversation(cid): configuration_admitter=attestor.for_admission, acquisition_validator=attestor.validate_acquisition, ) + # The selector each session-trusted agent or action step acquired, for its admission. + session_selectors = {} + session_selectors_lock = Lock() def capture(source_type, *, producer, settings, source=None, selector=None): + if source_type in SESSION_TRUSTED_SOURCE_TYPES: + # Agents and actions trust the signed-in session, like classic chat: each call + # rechecks current access, and their configuration is not captured or compared. + provider.preflight_session_acquisition(source_type, producer=producer, selector=selector) + with session_selectors_lock: + known = session_selectors.setdefault(producer, selector) + if known != selector: + raise ResultUnavailableError("result_external_selection_mismatch") + return provider.preflight_gather_acquisition( source_type, producer=producer, settings=settings, source=source, selector=selector, ) - if source is None and (source_type, producer.capability_id) in ( - ("agent", "agent_invoke"), ("action", "action_invoke"), - ): - return attestor.capture( source_type, producer=producer, settings=settings, source=source, selector=selector, ) def admit(*, producer, prepared): + if producer.capability_id in SESSION_TRUSTED_CAPABILITIES: + with session_selectors_lock: + selector = session_selectors.get(producer) + if selector is None: + raise ResultUnavailableError("result_external_capture_required") + else: + selector = attestor.selector_for(producer) # The runtime validates and installs these aliases in the facade's copied catalog. - return provider.admit_gather_result( - producer=producer, prepared=prepared, selector=attestor.selector_for(producer), - ) + return provider.admit_gather_result(producer=producer, prepared=prepared, selector=selector) def authorize_output(record, *, operation): return _authorize_render_output( diff --git a/application/single_app/functions_orchestration_executor.py b/application/single_app/functions_orchestration_executor.py index f27869985..f5901c945 100644 --- a/application/single_app/functions_orchestration_executor.py +++ b/application/single_app/functions_orchestration_executor.py @@ -1018,7 +1018,7 @@ def _run_dependency_step( if isinstance(exc, OrchestrationFilePolicyError): failure = build_failure('file_publication_not_allowed') elif isinstance(exc, (ResultUnavailableError, ElicitationContextError, PermissionError, ScreeningError)): - failure = access_failure(exc) + failure = access_failure(exc, capability_id=step['capability_id']) elif isinstance(exc, PartialInputNotAcceptedError): failure = build_failure('input_partial_not_accepted') elif isinstance(exc, ResultContractError): diff --git a/application/single_app/functions_orchestration_external_configuration.py b/application/single_app/functions_orchestration_external_configuration.py index d508bc32f..c7648b193 100644 --- a/application/single_app/functions_orchestration_external_configuration.py +++ b/application/single_app/functions_orchestration_external_configuration.py @@ -1,12 +1,14 @@ # functions_orchestration_external_configuration.py """Opaque invocation-configuration proof and capture-independent current reads. -Version: 0.261.209 +Version: 0.261.269 Research planner profile restriction removed in: 0.261.209 +Metadata refusals keep their own reason code in: 0.261.269 The owning engines supply actual acquisition evidence. The application root supplies current metadata reads; this module discovers no settings or clients, -invokes no model/tool, and never fetches source content. +invokes no model/tool, and never fetches source content. Agent and action steps +no longer use these proofs: they trust the signed-in session (0.261.269). """ from dataclasses import dataclass @@ -40,6 +42,7 @@ FOUNDRY_PINNED_REQUEST = "pinned-request-v1" MAX_CONFIGURATION_BYTES = 1048576 _SHA256 = re.compile(r"[0-9a-f]{64}\Z") +_REFUSAL_CODE = re.compile(r"[a-z][a-z0-9_]{0,79}\Z") _SOURCE_CAPABILITIES = { "web": "web_search", "url": "url_fetch", "deep_research": "deep_research", "agent": "agent_invoke", "action": "action_invoke", @@ -105,6 +108,12 @@ def _http_failure(status): return ExternalConfigurationServiceError("external_configuration_metadata_invalid") +def _refusal_code(error): + """A refusal's own stable code, so diagnostics name the real reason, never exception text.""" + code = getattr(error, "code", None) if isinstance(error, ResultUnavailableError) else None + return code if type(code) is str and _REFUSAL_CODE.fullmatch(code) else "external_configuration_unavailable" + + def _read_metadata(callback, *args, **kwargs): try: return callback(*args, **kwargs) @@ -112,8 +121,8 @@ def _read_metadata(callback, *args, **kwargs): raise except ExternalConfigurationServiceError as error: failure = ExternalConfigurationServiceError(error.code) - except (ResultUnavailableError, ClientAuthenticationError, ResourceNotFoundError, PermissionError): - failure = ResultUnavailableError("external_configuration_unavailable") + except (ResultUnavailableError, ClientAuthenticationError, ResourceNotFoundError, PermissionError) as error: + failure = ResultUnavailableError(_refusal_code(error)) except HttpResponseError as error: failure = _http_failure(error.status_code) except HTTPError as error: diff --git a/application/single_app/functions_orchestration_external_sources.py b/application/single_app/functions_orchestration_external_sources.py index 6e32ae1ae..77a60227e 100644 --- a/application/single_app/functions_orchestration_external_sources.py +++ b/application/single_app/functions_orchestration_external_sources.py @@ -1,11 +1,16 @@ # functions_orchestration_external_sources.py """Server admission and current access for retained external content. -Version: 0.261.209 +Version: 0.261.269 +Session-trusted agent and action sources in: 0.261.269 No fetch, recall, plugin invocation, settings discovery, or credential persistence occurs here. Content digests attest the exact retained payload, not a remote page revision. Committed result lineage supplies the binding after a process restart. + +Agent and action sources trust the signed-in session the way classic chat does: every +admission and read rechecks the user's current access to the conversation, the run's +step and the exact agent or action, but their configuration is not attested or compared. """ from dataclasses import dataclass @@ -62,6 +67,13 @@ } _OPAQUE_TOKEN = re.compile(r"[A-Za-z0-9][A-Za-z0-9_.:-]{0,255}\Z") MAX_CATALOG_ITEMS = 4096 +# Gather sources that trust the signed-in session instead of attesting their configuration. +SESSION_TRUSTED_SOURCE_TYPES = frozenset({"agent", "action"}) +SESSION_TRUSTED_CAPABILITIES = frozenset( + capability for capability, source_type in _GATHER_SOURCES.items() + if source_type in SESSION_TRUSTED_SOURCE_TYPES +) +_SESSION_TRUSTED_VERSION = "orchestration-session-trusted-source-v1" @dataclass(frozen=True) @@ -118,6 +130,13 @@ def _content_digest(prepared): return hashlib.sha256(value).hexdigest() +def _session_configuration_identity(source_type, reference): + """A stable identity for one agent or action, from its scope and id alone.""" + return f"source:{source_type}:" + canonical_digest({ + "version": _SESSION_TRUSTED_VERSION, "source_type": source_type, "reference": reference, + }) + + def _reference_id(producer, source_type, selector, content_sha256): return "retained:" + canonical_digest({ "producer": producer.to_dict(), "source_type": source_type, @@ -145,6 +164,9 @@ class OrchestrationExternalSourceProvider: alone. All callbacks run again on reads, including with a fresh provider and an empty admission catalog. The owner must supply restart-capable callbacks; read_identity refuses when no signed-in session is available. + + The configuration callbacks apply to web, URL and deep research sources. Agent + and action sources are session-trusted: they recheck current access only. """ def __init__( @@ -405,6 +427,20 @@ def preflight_gather_acquisition(self, source_type, *, producer, settings, sourc if validated is not None: raise ResultContractError("result_external_acquisition_validation_invalid") + def preflight_session_acquisition(self, source_type, *, producer, selector=None): + """Recheck current access before a session-trusted agent or action acquisition. + + Like classic chat, the signed-in user's current access to the conversation, the + run's step and the exact agent or action is what authorizes it. Its configuration + is neither captured nor compared, so this needs no acquisition evidence. + """ + if ( + type(producer) is not ProducerIdentity or source_type not in SESSION_TRUSTED_SOURCE_TYPES + or _GATHER_SOURCES.get(producer.capability_id) != source_type + ): + raise ResultContractError("result_external_producer_invalid") + self._gather_invocation_state(producer, selector) + def _gather_invocation_state(self, producer, selector): if type(producer) is not ProducerIdentity or producer.capability_id not in _GATHER_SOURCES: raise ResultContractError("result_external_producer_invalid") @@ -455,10 +491,10 @@ def _reference( catalog = self._catalog(source_type, identity, settings) if source_type in {"agent", "action"} else [] self._capability(source_type, producer, identity, settings, catalog) revision = None - configuration_identity = None + configuration_suffix = None if source_type == "fact_memory": selector = self._memory_scope(identity, settings, conversation, run, audience) - elif source_type in {"agent", "action"}: + elif source_type in SESSION_TRUSTED_SOURCE_TYPES: key = "catalog_key" if source_type == "agent" else "action_ref" matches = [ item for item in catalog @@ -474,15 +510,23 @@ def _reference( selector = selected[key] if previous is None: self._check_integration_selection(source_type, producer, run, selected, selector) - source = self._resolve_integration(source_type, selected, identity, settings) - configuration = self._configuration( - source_type, producer, settings, source, - selector=selector, for_admission=previous is None, - ) - configuration_identity = configuration.identity - revision = "configuration:" + canonical_digest({ - "identity": configuration.identity, "revision": configuration.revision, - }) + self._resolve_integration(source_type, selected, identity, settings) + if previous is None: + reference = ( + agent_reference(selected, identity.user_id) if source_type == "agent" + else {name: selected[name] for name in ("id", "scope_type", "scope_id")} + ) + configuration_identity = _session_configuration_identity(source_type, reference) + configuration_suffix = canonical_digest(configuration_identity) + revision = "configuration:" + canonical_digest({ + "identity": configuration_identity, "revision": _SESSION_TRUSTED_VERSION, + }) + else: + # Access was rechecked above. Configuration is not compared, so a reference + # keeps its own identity and revision, including one admitted while + # agent and action configuration was still attested. + configuration_suffix = previous.reference_id.rsplit(":", 1)[1] + revision = previous.source_revision else: if selector is not None: raise ResultContractError("result_external_selection_invalid") @@ -494,8 +538,8 @@ def _reference( "identity": configuration.identity, "revision": configuration.revision, }) reference_id = _reference_id(producer, source_type, selector, content_sha256) - if configuration_identity is not None: - reference_id += ":" + canonical_digest(configuration_identity) + if configuration_suffix is not None: + reference_id += ":" + configuration_suffix return ExternalSourceRef( source_type, producer.capability_id, reference_id, "audience:" + canonical_digest(audience), content_sha256, revision, diff --git a/application/single_app/functions_orchestration_m365.py b/application/single_app/functions_orchestration_m365.py index 8444ee42e..d90f39ce9 100644 --- a/application/single_app/functions_orchestration_m365.py +++ b/application/single_app/functions_orchestration_m365.py @@ -1,19 +1,18 @@ # functions_orchestration_m365.py -"""Microsoft 365 for orchestration action steps. +"""Microsoft 365 for orchestration action and agent steps. -Each action step runs in its own request context, the execution identity's bridge. Classic -chat and workflows install a Microsoft 365 execution context on their own request, so a -bridge never had one: every Microsoft 365 function refused the call with +Each action or agent step runs in its own request context, the execution identity's +bridge. Classic chat and workflows install a Microsoft 365 execution context on their own +request, so a bridge never had one: every Microsoft 365 function refused the call with ``m365_context_required`` before reaching Microsoft Graph, and the step still reported -completed. ``action_step_scope`` installs a context for one approved action step. The -helpers here turn Microsoft 365 sign-in, approval and policy refusals into +completed. ``action_step_scope`` installs a context for one approved action step, and +``agent_step_scope`` one for an approved agent step whose agent loads Microsoft 365 +actions. The helpers here turn Microsoft 365 sign-in, approval and policy refusals into application-owned step failures, instead of findings the model reports as data. -Agent steps need no scope: a plan runs only Foundry agents, whose tools run in Foundry, -and refuses local agents, the only agents that load Microsoft 365 actions. - -Version: 0.261.238 +Version: 0.261.269 Implemented in: 0.261.238 +Agent steps get their own Microsoft 365 scope in: 0.261.269 """ import hashlib @@ -167,3 +166,28 @@ def action_step_scope(action_ref, *, user_id, conversation_id, request_key, user user_id=user_id, conversation_id=conversation_id, request_key=request_key, selection=selection, origin={**(origin or {}), 'capability_id': 'action_invoke'}, ) + + +def agent_loads_actions(agent): + """Whether an agent step could need a Microsoft 365 scope: only an agent that loads actions.""" + actions = agent.get('actions_to_load') if isinstance(agent, dict) else None + return isinstance(actions, list) and any(isinstance(action, str) and action for action in actions) + + +def agent_step_scope(agent, *, user_id, conversation_id, request_key, origin=None): + """Authorize one approved agent step's Microsoft 365 actions, selecting only that agent's. + + The agent's actions and their overrides resolve from current storage as a chat-selected + agent's do. An agent that loads no Microsoft 365 action gets no context. + """ + selection = { + 'kind': 'agent', + 'agent': { + key: agent.get(key) for key in ('id', 'name', 'is_global', 'is_group', 'group_id') + if isinstance(agent, dict) and key in agent + }, + } + return _step_scope( + user_id=user_id, conversation_id=conversation_id, request_key=request_key, + selection=selection, origin={**(origin or {}), 'capability_id': 'agent_invoke'}, + ) diff --git a/application/single_app/functions_orchestration_schema.py b/application/single_app/functions_orchestration_schema.py index c7e18d2fd..803abb4e1 100644 --- a/application/single_app/functions_orchestration_schema.py +++ b/application/single_app/functions_orchestration_schema.py @@ -1738,6 +1738,10 @@ def summarize_plan(plan): 'analysis_result_not_saved': 'The analysis completed, but its final data could not be saved for reuse.', 'analysis_input_too_large': 'The complete saved analysis exceeds the selected model input budget. No data was truncated or re-analyzed. Select a larger model or use a supported complete-record reader.', 'result_unavailable': 'A required retained result is unavailable or changed. No preview was substituted.', + 'integration_unavailable': ( + "This step's agent or action isn't available to you right now, so it didn't run. Check " + 'that it still exists, is enabled and is shared with you, then retry.' + ), 'external_session_required': ( 'This step continued in the background, where your sign-in is not available to confirm ' 'access to web search, web pages, deep research, agents or actions, or to start a saved ' @@ -1858,6 +1862,8 @@ def summarize_plan(plan): }) # Failures an exception may carry by code, through its ``orchestration_failure_code``. EXCEPTION_FAILURE_CODES = M365_STEP_FAILURE_CODES | {'external_session_required'} +# Steps that hand work to one of the user's agents or actions; they take no retained inputs. +INTEGRATION_CAPABILITIES = frozenset({'action_invoke', 'agent_invoke'}) def build_failure( @@ -1916,17 +1922,21 @@ def failure_from_exception(exc, *, answering=False, _depth=0): return build_failure('model_failed' if answering else 'step_failed') -def access_failure(exc, *, _depth=0): +def access_failure(exc, *, capability_id=None, _depth=0): """Explain a refused retained source, naming a missing signed-in session when that was why. - Only the refusal's stable reason code is read, never exception text. + An agent or action step takes no retained inputs, so a refusal there is about the agent + or action itself, not a saved result. Only the refusal's stable reason code is read, + never exception text. """ for reason in (getattr(exc, 'authority_reason', None), getattr(exc, 'code', None)): if type(reason) is str and reason == EXTERNAL_SESSION_UNAVAILABLE_REASON: return build_failure('external_session_required') cause = getattr(exc, '__cause__', None) if cause is not None and cause is not exc and _depth < 3: - return access_failure(cause, _depth=_depth + 1) + return access_failure(cause, capability_id=capability_id, _depth=_depth + 1) + if capability_id in INTEGRATION_CAPABILITIES: + return build_failure('integration_unavailable') return build_failure('result_unavailable') diff --git a/application/v2_ui/src/components/chat/Composer.tsx b/application/v2_ui/src/components/chat/Composer.tsx index 9fe1e93cc..3435d1d73 100644 --- a/application/v2_ui/src/components/chat/Composer.tsx +++ b/application/v2_ui/src/components/chat/Composer.tsx @@ -86,7 +86,7 @@ import { type TabularRunSettings, } from '../../lib/tabularRunEstimate'; import { LargeRunDialog } from './LargeRunDialog'; -import type { MentionSuggestion } from '../../lib/mentions'; +import { resolveInvocationTarget, type MentionSuggestion } from '../../lib/mentions'; import { useUiStore } from '../../stores/uiStore'; import { toast } from '../../stores/toastStore'; import { chatWidthClass } from '../../lib/chatWidth'; @@ -107,7 +107,7 @@ import { messageToPlainText } from '../../lib/messageText'; import { ANALYSIS_CONTEXT_NOTICE } from '../../lib/savedAnalysis'; import { WORKFLOW_RESULT_PLACEHOLDER } from '../../lib/workflowResults'; import { WorkflowResultChip } from './WorkflowResultChip'; -import type { Json, PromptOption, WorkspaceRef } from '../../lib/types'; +import type { AgentOption, Json, PromptOption, WorkspaceRef } from '../../lib/types'; import { rememberPromptValues } from '../../lib/promptVariableMemory'; import { EMPTY_PROMPT_DRAFT, @@ -1146,17 +1146,27 @@ export function Composer({ initialAgentSelection }: { initialAgentSelection?: st if (options.imageGeneration && draftImageReferences.length > 0) { seeds.image_references = draftImageReferences; } + // An explicit @agent or @model tag says who should answer, the way it does in a shared + // chat: an agent tag seeds that agent, so the plan asks it rather than using its actions + // directly, and a model tag pins that model. A tag wins over the manual controls. + const tagged = resolveInvocationTarget( + message, + bootstrap?.catalogs?.agents as AgentOption[] | undefined, + bootstrap?.catalogs?.models as ModelCatalogEntry[] | undefined, + ); + const taggedAgent = tagged?.target_type === 'agent' ? tagged.agent_selection_key : undefined; + const taggedModel = tagged?.target_type === 'model' ? tagged.selection_key : undefined; Object.assign( seeds, buildSelectionFields({ agents: bootstrap?.catalogs?.agents as Record[] | undefined, models: bootstrap?.catalogs?.models as ModelCatalogEntry[] | undefined, - agentSelection: options.agentSelection, - modelDeployment: orchestrationModel || options.modelDeployment, + agentSelection: taggedAgent ?? (taggedModel ? undefined : options.agentSelection), + modelDeployment: taggedModel ?? (orchestrationModel || options.modelDeployment), reasoningEffort: options.reasoningEffort, }), ); - if (autoModelRouting) { + if (autoModelRouting && !taggedModel) { seeds.model_routing = 'auto'; for (const key of ['model_deployment', 'model_id', 'model_endpoint_id', 'model_provider', 'reasoning_effort']) { delete seeds[key]; diff --git a/functional_tests/test_orchestration_catalog_auto_execution.py b/functional_tests/test_orchestration_catalog_auto_execution.py index 224c2d42e..d740d5f4e 100644 --- a/functional_tests/test_orchestration_catalog_auto_execution.py +++ b/functional_tests/test_orchestration_catalog_auto_execution.py @@ -1,9 +1,10 @@ # test_orchestration_catalog_auto_execution.py """Exercise catalog-selected agent/action execution and Auto routing. -Version: 0.261.139 +Version: 0.261.269 Implemented in: 0.261.126 Single orchestration contract updated in: 0.261.139 +Agent and action engines run without an invocation capture in: 0.261.269 Uses production adapters, model routing, and dependency-plan validation with offline provider responses and explicitly authorized candidate inventories. @@ -74,7 +75,8 @@ def test_catalog_selected_action_step_executes_and_forwards_visual_flags(modules async def invoke_action(action_ref, task, context, **kwargs): calls.append((action_ref, task, kwargs)) - kwargs["invocation_capture"]("action", settings={}, source={"action_ref": action_ref}, selector=action_ref) + # Action steps trust the signed-in session, so the engine gets no invocation capture. + assert kwargs.get("invocation_capture") is None return {"findings": "Action result", "artifacts": [], "invocations": [], "root_id": "root", "calls": 1, "charts": 0} stubs = { @@ -135,7 +137,8 @@ def test_catalog_selected_agent_step_executes_with_the_server_identity(modules): async def invoke_scoped_agent(agent_cfg, task, **kwargs): calls.append((deepcopy(agent_cfg), task, kwargs)) - kwargs["invocation_capture"]("agent", settings={}, source={"agent_id": agent_cfg["id"]}, selector=agent_cfg["catalog_key"]) + # Agent steps trust the signed-in session, so the engine gets no invocation capture. + assert kwargs.get("invocation_capture") is None return {"response": "Agent result", "citations": [], "usage": {"prompt_tokens": 2, "completion_tokens": 3, "total_tokens": 5}} stubs = { diff --git a/functional_tests/test_orchestration_external_bootstrap.py b/functional_tests/test_orchestration_external_bootstrap.py index e846b703a..566575c97 100644 --- a/functional_tests/test_orchestration_external_bootstrap.py +++ b/functional_tests/test_orchestration_external_bootstrap.py @@ -1,16 +1,19 @@ # test_orchestration_external_bootstrap.py """ Functional test for application-owned session identity and acquisition wiring on retained results. -Version: 0.261.209 +Version: 0.261.269 Implemented in: 0.261.127 Single orchestration contract updated in: 0.261.139 Signed-in session roles replaced per-call Microsoft Graph reads in: 0.261.209 +Session-trusted agent and action steps implemented in: 0.261.269 Runs the real bootstrap, session capture, current-access reader, retention and result store with only Cosmos storage doubled and networking blocked. Roles come from the signed-in session, as in classic chat, so no MSAL client or directory call is made. Background continuations without that session fail closed with a -distinct reason instead of restoring saved roles. Refs microsoft/simplechat#1509. +distinct reason instead of restoring saved roles. Action steps recheck current +access but never re-read their configuration (microsoft/simplechat#1660). +Refs microsoft/simplechat#1509. """ import hashlib @@ -623,3 +626,140 @@ def test_existing_auth_factory_keeps_its_default_arguments(external_root, monkey APP_ID, authority=runtime.auth.AUTHORITY, client_credential="synthetic-client-secret", token_cache=cache, ) + + +def action_root(runtime, monkeypatch): + """The real root with one personal Microsoft 365 action step and only its stores doubled. + + The configuration reader fails if it is ever called: re-reading an action's prepared + manifest inside a step's Microsoft 365 scope is what refused every Microsoft 365 step. + """ + manifests = importlib.import_module("functions_action_manifest") + catalog_module = importlib.import_module("functions_action_catalog") + registry = importlib.import_module("functions_orchestration_registry") + contracts = importlib.import_module("functions_orchestration_result_contracts") + settings = { + "enable_chat_orchestration": True, "enable_semantic_kernel": True, + "enable_chat_orchestration_actions": True, "chat_orchestration_enabled_capabilities": [], + "max_generated_chat_artifact_size_mb": 1, + } + monkeypatch.setattr(runtime.root, "get_settings", lambda: deepcopy(settings)) + monkeypatch.setitem(runtime.root.config.CLIENTS, "storage_account_office_docs_client", None) + selector = catalog_module._action_ref("personal", USER_ID, "mail-action") + entry = { + "id": "mail-action", "name": "m365_email", "display_name": "M365 Email", "type": "m365_email", + "scope_type": "personal", "scope_id": USER_ID, "action_ref": selector, + } + state = SimpleNamespace(catalog=[entry], description="Delegated mail reads.", metadata_reads=[]) + + def resolve(user_id, action_ref, *, settings=None, user_groups=None): + if user_id != USER_ID or action_ref != selector or not state.catalog: + raise PermissionError("The action is unavailable.") + return manifests.bind_action_origin({**entry, "description": state.description}, "personal", USER_ID) + + defaults = runtime.root.OrchestrationExternalSourceProvider.__init__.__kwdefaults__ + monkeypatch.setitem(defaults, "action_catalog_reader", lambda user_id, settings=None: deepcopy(state.catalog)) + monkeypatch.setitem(defaults, "action_resolver", resolve) + + def reread(*args, **kwargs): + state.metadata_reads.append(args) + raise AssertionError("An action step must not re-read its configuration.") + + monkeypatch.setattr(runtime.root, "build_external_metadata_reader", lambda *_args, **_kwargs: reread) + step = { + "step_id": "gather", "capability_id": "action_invoke", "enabled": True, + "arguments": {"action_ref": selector, "task": "Read my latest emails."}, + } + runtime.runs.create_item({ + "id": "action-root-run", "user_id": USER_ID, "conversation_id": CONVERSATION_ID, + "attempt_index": 1, "status": "running", "user_message": "what are my emails", + "memory_audience": {"kind": "personal", "owner_id": USER_ID, "collaboration_id": ""}, + "plan": {"planner_contract_version": 2, "steps": [deepcopy(step)]}, + }) + contract = registry.get_capability("action_invoke", contract_version=2)["result_contract_version"] + return SimpleNamespace( + settings=settings, selector=selector, state=state, step=step, + producer=contracts.ProducerIdentity( + USER_ID, CONVERSATION_ID, "action-root-run", 1, "gather", "action_invoke", contract, + ), + prepared={ + "version": "orchestration-gathered-content-v1", "capability_id": "action_invoke", + "content_scope": "reported_external_content", "evidence": [], "citations": [], + "notes": ['Action "M365 Email" findings:\nAda sent Quarterly review.'], + "limitations": ["Returned content, not whole-source coverage."], + }, + ) + + +def test_root_action_steps_trust_the_session_without_rereading_configuration(external_root, monkeypatch): + runtime = external_root + bound = action_root(runtime, monkeypatch) + # The result runtime and its fixtures load only after the offline application is initialized. + results = importlib.import_module("functions_orchestration_results") + from test_support.orchestration_results import complete + + services = services_for(runtime, bound.settings) + assert services.external_source_preflight(producer=bound.producer, selector=bound.selector) is None + services.capture_external_source_configuration( + "action", producer=bound.producer, settings=deepcopy(bound.settings), selector=bound.selector, + ) + # An engine may still report the manifest it resolved, as one running inside a step's + # Microsoft 365 scope did; it is not compared with anything. + services.capture_external_source_configuration( + "action", producer=bound.producer, settings=deepcopy(bound.settings), selector=bound.selector, + source={ + "version": "orchestration-external-acquisition-v1", "kind": "action", "phase": "resolved", + "reference": {"id": "mail-action", "scope_type": "personal", "scope_id": USER_ID}, + "manifest": {"id": "mail-action"}, "prepared_manifest": {"id": "mail-action"}, "model": None, + }, + ) + with pytest.raises(ResultUnavailableError) as raised: + services.capture_external_source_configuration( + "action", producer=bound.producer, settings=deepcopy(bound.settings), selector="another-action", + ) + assert raised.value.code == "result_external_source_unavailable" + + admitted = services.external_source_admission(producer=bound.producer, prepared=deepcopy(bound.prepared)) + assert len(admitted) == 1 + reference = next(iter(admitted.values())) + assert (reference.source_type, reference.capability_id) == ("action", "action_invoke") + services.results.access.external_source_catalog.update(admitted) + task = services.results.persist_task_result( + producer=bound.producer, role="gather", status="complete", + outputs=[results.NamedOutput("prepared", "structured-v1", deepcopy(bound.prepared), complete(1))], + sources=[], origin="grounded", external_sources=tuple(admitted), + guard_token="server-attempt-token", input_fingerprint=canonical_digest({"step": bound.step}), + ) + + restarted = services_for(runtime, bound.settings) + assert restarted.results.open_result( + task.output("prepared"), require_current_sources=True, + ).read_value() == bound.prepared + bound.state.description = "Edited after the result was saved." + assert restarted.results.open_result( + task.output("prepared"), require_current_sources=True, + ).read_value() == bound.prepared + bound.state.catalog = [] + with pytest.raises(ResultUnavailableError): + restarted.results.open_result(task.output("prepared"), require_current_sources=True) + assert bound.state.metadata_reads == [] + + +def test_root_refuses_action_admission_without_its_session_checked_acquisition(external_root, monkeypatch): + runtime = external_root + bound = action_root(runtime, monkeypatch) + services = services_for(runtime, bound.settings) + with pytest.raises(ResultUnavailableError) as raised: + services.external_source_admission(producer=bound.producer, prepared=deepcopy(bound.prepared)) + assert raised.value.code == "result_external_capture_required" + + revoked = services_for(runtime, bound.settings, roles=("Viewer",)) + with pytest.raises(ResultUnavailableError): + revoked.capture_external_source_configuration( + "action", producer=bound.producer, settings=deepcopy(bound.settings), selector=bound.selector, + ) + with pytest.raises(ResultUnavailableError) as raised: + revoked.external_source_admission(producer=bound.producer, prepared=deepcopy(bound.prepared)) + assert raised.value.code == "result_external_capture_required" + assert services.results.access.external_source_catalog == {} + assert bound.state.metadata_reads == [] diff --git a/functional_tests/test_orchestration_external_capture_integration.py b/functional_tests/test_orchestration_external_capture_integration.py index 23a2e23dc..506d91781 100644 --- a/functional_tests/test_orchestration_external_capture_integration.py +++ b/functional_tests/test_orchestration_external_capture_integration.py @@ -1,13 +1,14 @@ # test_orchestration_external_capture_integration.py """ Real external acquisition hooks through preflight, capture, admission and restart. -Version: 0.261.127 +Version: 0.261.269 Implemented in: 0.261.127 +Action results admitted on the signed-in session's current access in: 0.261.269 Reuse the engine owner's provider/page I/O doubles, not a capture callback double. Adapters, configuration attestation, authorization, central retention and result readers are real. No network, credential grants or live model calls are permitted. -Refs microsoft/simplechat#1509. +Refs microsoft/simplechat#1509 and microsoft/simplechat#1660. """ import asyncio @@ -335,8 +336,11 @@ def capture(source_type, *, settings, source, selector): capture("action", settings=world.settings, source=None, selector=world.action_selector) assert observed == [] assert attestor._captures == {} - with pytest.raises(configuration.ResultUnavailableError): - world.admit(provider) + # Action results trust the signed-in session (microsoft/simplechat#1660): the provider + # admits them on current access alone and never consults configuration attestation. + # The application root still refuses an action result without its session-checked + # acquisition (test_orchestration_external_bootstrap). + assert world.admit(provider) state.replies = [tool_message("42", "43")] result = asyncio.run(state.runtime.invoke_action( diff --git a/functional_tests/test_orchestration_external_configuration_capture.py b/functional_tests/test_orchestration_external_configuration_capture.py index 7abf84b6a..e3555dab9 100644 --- a/functional_tests/test_orchestration_external_configuration_capture.py +++ b/functional_tests/test_orchestration_external_configuration_capture.py @@ -1,14 +1,17 @@ # test_orchestration_external_configuration_capture.py """ Functional tests for private, invocation-time Gather configuration capture. -Version: 0.261.139 +Version: 0.261.269 Implemented in: 0.261.127 Acquisition-boundary coverage updated in: 0.261.129 Pre-acquisition provider failure classification updated in: 0.261.134 Single orchestration contract updated in: 0.261.139 +Agent and action steps trust the signed-in session, without attestation, in: 0.261.269 Real adapters, web search and source review run with provider/page I/O doubled. -No live provider, remote configuration or user artifact is accessed. +No live provider, remote configuration or user artifact is accessed. Web search, URL +fetch and Deep Research keep configuration attestation; agent and action steps check +current access once and run as manual chat runs them (microsoft/simplechat#1660). """ import asyncio @@ -744,7 +747,7 @@ async def delete_thread(*_args, **_kwargs): assert "Actual SDK retained final line." in "\n".join(result["notes"]) -def test_real_action_configuration_captures_prepared_origin_and_constructed_model(capture_runtime, monkeypatch): +def test_real_action_step_checks_session_access_and_runs_the_classic_engine(capture_runtime, monkeypatch): runtime = capture_runtime actions = importlib.import_module("functions_orchestration_actions") manifests = importlib.import_module("functions_action_manifest") @@ -754,7 +757,6 @@ def test_real_action_configuration_captures_prepared_origin_and_constructed_mode settings_module = importlib.import_module("functions_settings") kernel_loader = importlib.import_module("semantic_kernel_loader") policy = importlib.import_module("functions_orchestration_execution_policy") - configuration = runtime.modules.configuration runtime.settings.update({ "enable_chat_orchestration": True, "enable_chat_orchestration_actions": True, "azure_openai_gpt_endpoint": "https://model.invalid", "azure_openai_gpt_api_version": "2024-10-21", @@ -803,31 +805,12 @@ async def reply(service, history, execution): ])] return [ChatMessageContent(role=AuthorRole.ASSISTANT, content="First retained row\nFinal retained row 1000")] - def metadata(_source_type, *, producer, settings, source, selector): - return { - "version": configuration.EXTERNAL_ACQUISITION_VERSION, "kind": "action", "phase": "current", - "reference": {"id": "lookup", "scope_type": "personal", "scope_id": "owner"}, - "manifest": deepcopy(manifest), - "prepared_manifest": kernel_loader.prepare_action_plugin_manifest(manifest, settings), - "model": { - "provider": "aoai", "protocol": "azure_openai", - "endpoint": settings["azure_openai_gpt_endpoint"], "api_version": settings["azure_openai_gpt_api_version"], - "deployment": "gpt-4o", "endpoint_id": None, "model_id": None, - "parameters": {"parallel_tool_calls": False, "tool_choice": "auto"}, - }, - } - - attestor = configuration.OrchestrationExternalConfigurationAttestor( - user_id="owner", conversation_id="conversation-1", read_current_source=metadata, private_digest=_private_digest, - ) - def capture(source_type, **kwargs): if kwargs["source"] is None: calls.append("preflight") return calls.append("capture") captures.append(deepcopy(kwargs)) - attestor.capture(source_type, **kwargs) runtime.context.capture_external_source_configuration = capture monkeypatch.setattr(actions, "resolve_action_manifest", lambda *_args, **_kwargs: deepcopy(manifest)) @@ -837,26 +820,16 @@ def capture(source_type, **kwargs): with policy.orchestration_file_policy(allow_generated_files=False): step, result = run_gather(runtime, "action_invoke", arguments={"action_ref": selector, "task": "Read all rows."}) assert result["status"] == "completed", result - assert calls == ["preflight", "preflight", "capture", "load", "model", "capture", "tool", "model"] - assert len(captures) == 2 - assert captures[0]["source"] == captures[1]["source"] - assert captures[0]["source"]["prepared_manifest"] == loaded[0] - origin = manifests.get_action_origin(captures[0]["source"]["manifest"]) - assert origin == manifests.McpActionOrigin("personal", "owner", "lookup") - assert captures[0]["source"]["model"]["deployment"] == models[0].ai_model_id + # One session access check for the exact action, then the classic engine, as manual chat + # runs it: no configuration is captured or attested (microsoft/simplechat#1660). + assert calls == ["preflight", "load", "model", "tool", "model"] + assert captures == [] + assert loaded[0] == kernel_loader.prepare_action_plugin_manifest(manifest, runtime.settings) + assert models[0].ai_model_id == "gpt-4o" assert models[0].client.is_closed() assert "Final retained row 1000" in "\n".join(result["notes"]) assert not result["artifacts"] assert "SYNTHETIC_PRIVATE" not in json.dumps(result) - producer = runtime.context.result_producer(step) - proof = attestor.for_admission( - "action", producer=producer, settings=runtime.settings, source=manifest, selector=selector, - ) - restarted = configuration.OrchestrationExternalConfigurationAttestor( - user_id="owner", conversation_id="conversation-1", read_current_source=metadata, private_digest=_private_digest, - ) - current = restarted.current("action", producer=producer, settings=runtime.settings, source=manifest) - assert proof == current @pytest.mark.parametrize("field,value", [ @@ -977,17 +950,27 @@ def test_research_does_not_rediscover_planner_after_capture(capture_runtime): @pytest.mark.parametrize("capability", ["agent_invoke", "action_invoke"]) -def test_freshly_resolved_engines_are_not_attested_from_catalog_candidates(capture_runtime, monkeypatch, capability): +def test_agent_and_action_steps_run_on_a_session_access_check_without_attestation( + capture_runtime, monkeypatch, capability, +): + """Agent and action steps trust the signed-in session as manual chat does (#1660, #1661). + + Current access to the exact catalog selection is checked once, and the engine then runs + without an invocation capture, so its output is accepted without configuration attestation. + """ runtime = capture_runtime runtime.settings["allow_user_agents"] = True + runtime.settings["enable_semantic_kernel"] = True contexts = importlib.import_module("agent_execution_context") catalog = importlib.import_module("functions_action_catalog") selector = catalog._action_ref("personal", "owner", "original-action-id") engine_name = "agent_delegation_runtime" if capability == "agent_invoke" else "functions_orchestration_actions" engine = importlib.import_module(engine_name) - async def unobserved_engine(*args, **kwargs): - return {"findings": "Unattested output", "calls": 1, "root_id": "root", "invocations": [], "artifacts": []} + async def session_engine(*args, **kwargs): + if capability == "agent_invoke": + return {"response": "Session-trusted output", "citations": []} + return {"findings": "Session-trusted output", "calls": 1, "root_id": "root", "invocations": [], "artifacts": []} preparations = [] @@ -995,7 +978,7 @@ def prepare(source_type, *, producer, settings, source=None, selector=None): assert source is None preparations.append((source_type, selector)) - invocation = Mock(side_effect=unobserved_engine) + invocation = Mock(side_effect=session_engine) monkeypatch.setattr(engine, "invoke_scoped_agent" if capability == "agent_invoke" else "invoke_action", invocation) runtime.context.agent_catalog = [{ "id": "original-agent-id", "name": "original-agent", @@ -1011,8 +994,8 @@ def prepare(source_type, *, producer, settings, source=None, selector=None): runtime.context.capture_external_source_configuration = prepare arguments = {"action_ref": selector, "task": "Gather facts"} if capability == "action_invoke" else None _, result = run_gather(runtime, capability, arguments=arguments) - assert result["status"] == "failed" - assert result["notes"] == result["artifacts"] == [] + assert result["status"] == "completed", result + assert "Session-trusted output" in "\n".join(result["notes"]) assert runtime.state.calls == [] expected = ( ("agent", "personal:owner:original-agent-id") if capability == "agent_invoke" @@ -1020,6 +1003,7 @@ def prepare(source_type, *, producer, settings, source=None, selector=None): ) assert preparations == [expected] invocation.assert_called_once() + assert invocation.call_args.kwargs.get("invocation_capture") is None @pytest.mark.parametrize("capability", ["web_search", "url_fetch", "deep_research", "agent_invoke", "action_invoke"]) diff --git a/functional_tests/test_orchestration_external_preflight_adapter.py b/functional_tests/test_orchestration_external_preflight_adapter.py index 3c4470600..051d00019 100644 --- a/functional_tests/test_orchestration_external_preflight_adapter.py +++ b/functional_tests/test_orchestration_external_preflight_adapter.py @@ -1,15 +1,18 @@ # test_orchestration_external_preflight_adapter.py """Strict invocation authorization and acquisition support for all five v2 adapters. -Version: 0.261.209 +Version: 0.261.269 Implemented in: 0.261.127 Early acquisition-support regressions implemented in: 0.261.129 Missing signed-in session refusal reason implemented in: 0.261.209 +Agent and action steps accepted on a session access check in: 0.261.269 The auth-only runtime hook invokes the real provider before capture or engine setup. Capture separately invokes the real combined acquisition guard. The current-resource cases use the real metadata reader and attestor with only -external/storage I/O doubled; neither authorization nor preparation is proof. +external/storage I/O doubled; neither authorization nor preparation is proof +for web search, URL fetch or Deep Research. Agent and action steps trust the +signed-in session instead (microsoft/simplechat#1660). """ from contextlib import nullcontext @@ -530,7 +533,12 @@ def test_real_current_or_actual_unsupported_configuration_stops_before_effects(c @pytest.mark.parametrize("current_acquisition_runtime", ["agent_invoke", "action_invoke"], indirect=True) -def test_preparation_only_engine_result_cannot_be_accepted(current_acquisition_runtime, monkeypatch): +def test_session_checked_engine_result_is_accepted_without_attestation(current_acquisition_runtime, monkeypatch): + """Agent and action steps trust the signed-in session as manual chat does (#1660, #1661). + + The root checks current access to the exact selection once. The engine then runs without an + invocation capture, and its result is accepted without configuration attestation. + """ state = current_acquisition_runtime state.stop_after_preparation = False engine_name = ( @@ -538,18 +546,22 @@ def test_preparation_only_engine_result_cannot_be_accepted(current_acquisition_r ) engine = importlib.import_module(engine_name) - async def unobserved_engine(*_args, **_kwargs): - return {"findings": "Unattested content", "calls": 0, "invocations": [], "artifacts": []} + async def session_engine(*_args, **kwargs): + assert kwargs.get("invocation_capture") is None + if state.capability == "agent_invoke": + return {"response": "Session-trusted content", "citations": []} + return {"findings": "Session-trusted content", "calls": 0, "root_id": "root", "invocations": [], "artifacts": []} - invocation = Mock(side_effect=unobserved_engine) + invocation = Mock(side_effect=session_engine) monkeypatch.setattr( engine, "invoke_scoped_agent" if state.capability == "agent_invoke" else "invoke_action", invocation, ) result = execute(state) invocation.assert_called_once() - assert result["status"] == "failed" - assert result["notes"] == result["artifacts"] == [] - assert len(state.prepared) == 1 + assert result["status"] == "completed", result + assert "Session-trusted content" in "\n".join(result["notes"]) + owner = state.runtime.context.result_producer(state.step) + assert state.prepared == [(SOURCE_TYPES[state.capability], owner, None, state.selector)] assert state.world.attestor._captures == {} assert_no_current_effects(state) diff --git a/functional_tests/test_orchestration_external_sources.py b/functional_tests/test_orchestration_external_sources.py index 3a3293e43..e588cb55b 100644 --- a/functional_tests/test_orchestration_external_sources.py +++ b/functional_tests/test_orchestration_external_sources.py @@ -1,9 +1,10 @@ # test_orchestration_external_sources.py """ Functional coverage for retained external Gather source admission and access. -Version: 0.261.139 +Version: 0.261.269 Implemented in: 0.261.127 Single orchestration contract updated in: 0.261.139 +Agent and action sources trust the signed-in session in: 0.261.269 Real result contracts/store/readers, capability gates, scoped integration resolvers and memory authorization run with identity, configuration and storage @@ -11,6 +12,8 @@ bindings do not depend on the live admission catalog. Network and implicit memory recall are prohibited. Invocation preflight must reauthorize before capture/effects without creating content authority or a URL fetch grant. +Web search, URL fetch and Deep Research keep configuration attestation; agent and +action results recheck current access only (microsoft/simplechat#1660). Refs microsoft/simplechat#1509. """ @@ -609,15 +612,22 @@ def test_configuration_admission_is_required_only_for_new_external_gathers(self) self.assertEqual(value, world.prepared) def test_configuration_admitter_receives_the_original_exact_integration_selector(self): - for capability in ("web_search", "url_fetch", "deep_research", "agent_invoke", "action_invoke"): + for capability in ("web_search", "url_fetch", "deep_research"): with self.subTest(capability=capability), ExternalSourceWorld(capability) as world: admission = Mock(side_effect=world.admit_configuration) provider = world.provider(configuration_admitter=admission) world.admit(provider) self.assertEqual(admission.call_count, 1) self.assertEqual(admission.call_args.kwargs["producer"], world.fixture.producer) - expected = {"agent_invoke": world.agent_selector, "action_invoke": world.action_selector}.get(capability) - self.assertEqual(admission.call_args.kwargs["selector"], expected) + self.assertIsNone(admission.call_args.kwargs["selector"]) + # Agents and actions trust the signed-in session (microsoft/simplechat#1660): admission + # rechecks access to the exact selection and never consults configuration. + for capability in ("agent_invoke", "action_invoke"): + with self.subTest(capability=capability), ExternalSourceWorld(capability) as world: + admission = Mock(side_effect=world.admit_configuration) + provider = world.provider(configuration_admitter=admission) + self.assertTrue(world.admit(provider)) + self.assertEqual(admission.call_count, 0) def test_current_configuration_cannot_replace_the_captured_admission_configuration(self): with ExternalSourceWorld() as world: @@ -671,7 +681,9 @@ def test_agent_preference_and_exact_selection_cannot_fall_back_to_a_name(self): world.service(world.provider()).open_result(reference) def test_current_configuration_revision_is_not_a_remote_content_revision(self): - for capability in ("web_search", "agent_invoke", "action_invoke"): + # Agent and action references carry no configuration revision; see + # test_orchestration_session_trusted_sources.py (microsoft/simplechat#1660). + for capability in ("web_search",): with self.subTest(capability=capability), ExternalSourceWorld(capability) as world: provider, catalog, reference = world.save() external = next(iter(catalog.values())) @@ -834,10 +846,18 @@ def test_committed_receipt_recovery_rechecks_each_source_without_an_admission_ca self.assertGreater(world.identity_reads, reads_before) if not memory: world.configuration_revision = "revision-2" - with self.assertRaises(ResultUnavailableError): - restarted.recover_task_result( + if capability in ("agent_invoke", "action_invoke"): + # Agent and action results trust the signed-in session + # (microsoft/simplechat#1660): editing the configuration leaves a + # committed result recoverable, while losing access below does not. + self.assertEqual(restarted.recover_task_result( producer=world.fixture.producer, input_fingerprint=fingerprint, - ) + ), original) + else: + with self.assertRaises(ResultUnavailableError): + restarted.recover_task_result( + producer=world.fixture.producer, input_fingerprint=fingerprint, + ) world.configuration_revision = "revision-1" if capability in {"agent_invoke", "action_invoke", "compose"}: world.services.roles.clear() diff --git a/functional_tests/test_orchestration_local_agent_capture.py b/functional_tests/test_orchestration_local_agent_capture.py index e7eb36ec8..ff83f6a81 100644 --- a/functional_tests/test_orchestration_local_agent_capture.py +++ b/functional_tests/test_orchestration_local_agent_capture.py @@ -1,11 +1,14 @@ # test_orchestration_local_agent_capture.py """Actual local-agent loader bindings through the private acquisition contract. -Version: 0.261.127 +Version: 0.261.269 Implemented in: 0.261.127 +Local agent results trust the signed-in session in: 0.261.269 Uses the real resolver, SDK construction, prepared manifests and strict attestor. External storage/model I/O is doubled; no live model or user artifact is created. +Since 0.261.269 a saved agent result is reread on current access to the agent, not +on its configuration (microsoft/simplechat#1661). """ import asyncio @@ -268,7 +271,11 @@ def capture(source_type, **kwargs): assert calls_before_restart == (len(state.services), len(state.prompts)) assert "PRIVATE_AGENT_KEY" not in json.dumps(task.to_dict()) + # A local agent's results trust the signed-in session (microsoft/simplechat#1661): editing + # the agent leaves its saved result readable, and losing access to the agent does not. record["instructions"] = "Changed current configuration." + assert reader.read_value() == world.prepared + record["is_enabled"] = False with pytest.raises(state.modules.configuration.ResultUnavailableError): reader.read_value() assert calls_before_restart == (len(state.services), len(state.prompts)) diff --git a/functional_tests/test_orchestration_m365_actions.py b/functional_tests/test_orchestration_m365_actions.py index 4ce015342..96c40bd77 100644 --- a/functional_tests/test_orchestration_m365_actions.py +++ b/functional_tests/test_orchestration_m365_actions.py @@ -143,7 +143,14 @@ def participation(user_id, conversation): "functions_global_actions": module("functions_global_actions", get_global_actions=lambda **kwargs: []), "functions_group_actions": module( "functions_group_actions", get_governed_group_actions=lambda *args, **kwargs: [], + get_group_actions=lambda *args, **kwargs: [], ), + "functions_personal_agents": module( + "functions_personal_agents", + get_personal_agents=lambda user_id: deepcopy(getattr(state, "agents", [])) if user_id == USER else [], + ), + "functions_global_agents": module("functions_global_agents", get_global_agents=lambda: []), + "functions_group_agents": module("functions_group_agents", get_group_agents=lambda group_id: []), "functions_governance": module( "functions_governance", filter_actions_by_action_type_access=lambda user_id, actions, *args: actions, @@ -517,6 +524,67 @@ def test_a_step_without_a_signed_in_session_is_refused(env, world): assert world.jobs.items == {} +def mail_agent(**changes): + """A personal agent that loads the mail action, as the agent catalog serializes it.""" + return { + "id": "mail-agent", "name": "mail_agent", "display_name": "Mail agent", + "is_global": False, "is_group": False, "group_id": None, + "actions_to_load": ["mail"], "other_settings": {}, **changes, + } + + +def agent_scope(env, world, agent): + scope = env.orchestration.agent_step_scope( + agent, user_id=USER, conversation_id=CONVERSATION, request_key=REQUEST_KEY, + origin={"run_id": "run-2", "attempt_index": 1, "step_id": "ask_mail_agent"}, + ) + request = world.app.test_request_context("/internal/agent-execution") + return scope, request + + +def test_agent_step_scope_selects_only_the_agents_microsoft_365_actions(env, world): + world.agents = [mail_agent()] + assert env.orchestration.agent_loads_actions(mail_agent()) + assert not env.orchestration.agent_loads_actions(mail_agent(actions_to_load=[])) + scope, request = agent_scope(env, world, mail_agent()) + with request: + session["user"] = {"oid": USER, "tid": TENANT} + with scope as context: + assert context.request_id == env.orchestration.step_request_id(REQUEST_KEY) + assert context.shared is False and context.workflow_id is None + assert getattr(g, env.runtime.M365_STEP_SELECTION_KEY)["kind"] == "agent" + assert env.runtime.resolve_m365_action_selection(context) == ["mail"] + assert env.runtime.get_m365_execution_context() is None + record = request_record(env, world) + assert record["orchestration"] == { + "run_id": "run-2", "attempt_index": 1, "step_id": "ask_mail_agent", "capability_id": "agent_invoke", + } + + +def test_an_agent_without_microsoft_365_actions_gets_no_step_context(env, world): + world.action = {**mail_action(), "id": "weather", "name": "weather", "type": "openapi"} + world.agents = [mail_agent(actions_to_load=["weather"])] + scope, request = agent_scope(env, world, mail_agent(actions_to_load=["weather"])) + with request: + session["user"] = {"oid": USER, "tid": TENANT} + with scope as context: + assert context is None and env.runtime.get_m365_execution_context() is None + assert world.jobs.items == {} and world.token_requests == [] + + +def test_a_removed_agent_stops_its_step_before_microsoft_365_work(env, world): + world.agents = [] + scope, request = agent_scope(env, world, mail_agent()) + with request: + session["user"] = {"oid": USER, "tid": TENANT} + with pytest.raises(env.orchestration.OrchestrationM365Error) as caught: + with scope: + pass + assert caught.value.orchestration_failure_code == "m365_unavailable" + assert caught.value.m365_code == "m365_agent_unavailable" + assert world.jobs.items == {} and world.token_requests == [] + + def test_refusal_codes_map_to_step_failures(env, world): refusal = env.orchestration.result_refusal for value in ( diff --git a/functional_tests/test_orchestration_session_trusted_sources.py b/functional_tests/test_orchestration_session_trusted_sources.py new file mode 100644 index 000000000..4b93d1968 --- /dev/null +++ b/functional_tests/test_orchestration_session_trusted_sources.py @@ -0,0 +1,149 @@ +#!/usr/bin/env python3 +# test_orchestration_session_trusted_sources.py +""" +Functional test for session-trusted agent and action orchestration steps. +Version: 0.261.269 +Implemented in: 0.261.269 + +Agent and action steps trust the signed-in session the way classic chat does. Their +admission and retained reads recheck the user's current access to the conversation, the +run's step and the exact agent or action, but never read, capture or compare the agent's +or action's configuration. Results admitted while that configuration was still attested +stay readable. Fixes microsoft/simplechat#1660 and refs #1661. +""" + +from dataclasses import replace +import os +import sys +from unittest.mock import Mock + +import pytest + +sys.path.append(os.path.dirname(os.path.abspath(__file__))) + +from test_orchestration_external_sources import ExternalSourceWorld # noqa: E402 +from functions_orchestration_result_contracts import ResultContractError, canonical_digest # noqa: E402 +from functions_orchestration_results import ResultUnavailableError # noqa: E402 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +INTEGRATIONS = ("agent_invoke", "action_invoke") +SOURCE_TYPES = {"agent_invoke": "agent", "action_invoke": "action"} +RECORDS = { + "agent_invoke": (("agents", "group"), ("group-a", "agent-1")), + "action_invoke": (("actions", "group"), ("group-a", "action-1")), +} + + +def unused_configuration(): + return Mock(side_effect=AssertionError("Session-trusted sources never read their configuration.")) + + +def configuration_free_provider(world): + unused = unused_configuration() + provider = world.provider( + read_configuration=unused, configuration_admitter=unused, acquisition_validator=unused, + ) + return provider, unused + + +def read(world, provider, reference): + return world.service(provider).open_result(reference, require_current_sources=True).read_value() + + +def test_version_includes_session_trusted_sources(): + assert_app_version_at_least("0.261.269") + + +@pytest.mark.parametrize("capability", INTEGRATIONS) +def test_admission_rechecks_access_without_reading_configuration(capability): + with ExternalSourceWorld(capability) as world: + provider, unused = configuration_free_provider(world) + world.preflight(provider) + provider.preflight_session_acquisition( + SOURCE_TYPES[capability], producer=world.fixture.producer, selector=world.selected_integration(), + ) + catalog = world.admit(provider) + assert len(catalog) == 1 + reference = next(iter(catalog.values())) + assert reference.source_type == SOURCE_TYPES[capability] + assert reference.capability_id == capability + assert reference.source_revision.startswith("configuration:") + assert world.admit(provider) == catalog + assert world.identity_reads >= 2 and world.catalog_reads >= 2 + unused.assert_not_called() + + +@pytest.mark.parametrize("capability", INTEGRATIONS) +def test_retained_results_survive_configuration_edits_until_access_is_revoked(capability): + with ExternalSourceWorld(capability) as world: + _provider, _catalog, output = world.save() + reader, unused = configuration_free_provider(world) + assert read(world, reader, output) == world.prepared + + collection, key = RECORDS[capability] + record = world.services.records[collection][key] + record["description"] = "Edited after the result was saved." + record["endpoint"] = "https://changed.invalid" + assert read(world, reader, output) == world.prepared + + world.services.roles.clear() + with pytest.raises(ResultUnavailableError): + read(world, reader, output) + unused.assert_not_called() + + +@pytest.mark.parametrize("capability", INTEGRATIONS) +def test_references_admitted_under_configuration_attestation_stay_readable(capability): + with ExternalSourceWorld(capability) as world: + provider = world.provider() + current = next(iter(world.admit(provider).values())) + prefix = current.reference_id.rsplit(":", 1)[0] + attested = replace( + current, + reference_id=f"{prefix}:{canonical_digest('attested-configuration-identity')}", + source_revision="configuration:" + canonical_digest({"identity": "attested", "revision": "hmac"}), + ) + catalog = {"external_" + canonical_digest(attested.to_dict())[:48]: attested} + task = world.persist(world.service(provider, catalog), catalog) + reader, unused = configuration_free_provider(world) + assert read(world, reader, task.output("prepared")) == world.prepared + + world.services.roles.clear() + with pytest.raises(ResultUnavailableError): + read(world, reader, task.output("prepared")) + unused.assert_not_called() + + +@pytest.mark.parametrize("capability", INTEGRATIONS) +def test_session_preflight_requires_current_access_to_the_exact_selection(capability): + with ExternalSourceWorld(capability) as world: + provider, unused = configuration_free_provider(world) + producer = world.fixture.producer + source_type = SOURCE_TYPES[capability] + selector = world.selected_integration() + assert provider.preflight_session_acquisition(source_type, producer=producer, selector=selector) is None + + with pytest.raises(ResultUnavailableError) as raised: + provider.preflight_session_acquisition(source_type, producer=producer, selector="other-selection") + assert raised.value.code == "result_external_source_unavailable" + + other = "action" if source_type == "agent" else "agent" + for wrong in (other, "web", "url"): + with pytest.raises(ResultContractError): + provider.preflight_session_acquisition(wrong, producer=producer, selector=selector) + + world.roles = () + with pytest.raises(ResultUnavailableError) as raised: + provider.preflight_session_acquisition(source_type, producer=producer, selector=selector) + assert raised.value.code == "result_external_identity_unavailable" + + world.roles = ("User",) + world.services.roles.clear() + with pytest.raises(ResultUnavailableError): + provider.preflight_session_acquisition(source_type, producer=producer, selector=selector) + unused.assert_not_called() + + +if __name__ == "__main__": + raise SystemExit(pytest.main([__file__, "-q"])) diff --git a/functional_tests/test_orchestration_visual_outputs.py b/functional_tests/test_orchestration_visual_outputs.py index 698960837..77336524f 100644 --- a/functional_tests/test_orchestration_visual_outputs.py +++ b/functional_tests/test_orchestration_visual_outputs.py @@ -2,9 +2,10 @@ # test_orchestration_visual_outputs.py """ Functional test for charts, Mermaid diagrams, and image proposals in orchestrated answers. -Version: 0.261.139 +Version: 0.261.269 Implemented in: 0.261.132 Single orchestration contract updated in: 0.261.139 +Action steps run without an invocation capture in: 0.261.269 This test ensures orchestration carries chart payloads safely, builds visual guidance only from planner-declared visual flags, preserves saved instruction memory precedence, @@ -278,7 +279,8 @@ def test_action_step_uses_planner_visual_flags_and_keeps_chart_citations_whole(m async def invoke_action(action_ref, task, context, **kwargs): requests.append((task, kwargs)) - kwargs['invocation_capture']('action', settings={}, source={'action_ref': action_ref}, selector=action_ref) + # Action steps trust the signed-in session, so the engine gets no invocation capture. + assert kwargs.get('invocation_capture') is None return {'findings': 'Retrieved 900 samples.', 'artifacts': [], 'invocations': invocations, 'root_id': 'root', 'calls': 3, 'charts': 1} From 487589120c3d89846ab2561bbb2c939a5f12d6b3 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Tue, 6 Oct 2026 18:43:21 -0400 Subject: [PATCH 2/4] Make Orchestrate respect shared conversations and Microsoft 365 consent - Composer: in a shared conversation, Orchestrate applies the manual chat send rule first. A message for people only is posted to the thread with no plan. - The conversation owner's requests are planned against a hidden backing record at the shared id. The question and answer are mirrored into the shared thread after membership is rechecked. Lookups by the shared id skip the backing, republished answers update the shared copy, and deleting the shared conversation deletes the backing. - Microsoft 365: in a shared conversation the requester's explicit request is their consent, recorded as a shared_by_request audit event. Workflow "Run as" approvals are unchanged. - Fix docs, admin and guide updates, release notes, and tests for this change and the previous commit. Fixes #1659 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../single_app/collaboration_models.py | 17 + .../single_app/functions_collaboration.py | 14 + .../functions_conversation_metadata.py | 15 +- .../single_app/functions_m365_approvals.py | 28 ++ .../single_app/functions_m365_context.py | 10 +- .../functions_m365_pending_delivery.py | 9 +- .../single_app/functions_m365_runtime.py | 26 +- .../single_app/functions_mcp_server_tools.py | 5 +- .../functions_orchestration_artifacts.py | 20 + .../functions_orchestration_collaboration.py | 373 ++++++++++++++++++ .../functions_orchestration_execution.py | 24 +- .../functions_simplechat_operations.py | 5 +- application/single_app/route_backend_chats.py | 5 + .../route_backend_conversation_export.py | 9 +- .../single_app/route_backend_conversations.py | 107 +++-- .../single_app/route_backend_orchestration.py | 127 +++++- .../single_app/route_frontend_chats.py | 9 + .../route_frontend_conversations.py | 15 + .../v2_ui/src/components/chat/Composer.tsx | 62 ++- application/v2_ui/src/lib/mentions.ts | 30 ++ application/v2_ui/src/stores/chatStore.ts | 49 ++- docs/admin/orchestration.md | 86 +++- .../ORCHESTRATION_M365_ACTION_CONTEXT_FIX.md | 6 + ..._SESSION_TRUSTED_ACTION_AGENT_STEPS_FIX.md | 154 ++++++++ .../ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md | 204 ++++++++++ docs/explanation/release_notes.md | 18 + .../guides/microsoft-365-conversation-data.md | 40 +- ...document_provenance_versions_and_guards.py | 7 +- functional_tests/test_m365_provider_core.py | 65 +++ .../test_m365_shared_request_consent.py | 190 +++++++++ .../test_orchestration_m365_actions.py | 40 +- ...st_orchestration_shared_backing_lookups.py | 215 ++++++++++ ...test_orchestration_shared_conversations.py | 337 ++++++++++++++++ ...orchestration_turn_recovery_identifiers.py | 7 +- .../test_v2_shared_orchestration_routing.mjs | 143 +++++++ .../test_v2_shared_orchestration_routing.py | 273 +++++++++++++ 36 files changed, 2626 insertions(+), 118 deletions(-) create mode 100644 application/single_app/functions_orchestration_collaboration.py create mode 100644 docs/explanation/fixes/ORCHESTRATION_SESSION_TRUSTED_ACTION_AGENT_STEPS_FIX.md create mode 100644 docs/explanation/fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md create mode 100644 functional_tests/test_m365_shared_request_consent.py create mode 100644 functional_tests/test_orchestration_shared_backing_lookups.py create mode 100644 functional_tests/test_orchestration_shared_conversations.py create mode 100644 functional_tests/test_v2_shared_orchestration_routing.mjs create mode 100644 ui_tests/test_v2_shared_orchestration_routing.py diff --git a/application/single_app/collaboration_models.py b/application/single_app/collaboration_models.py index 60dbd875a..93d0acfe3 100644 --- a/application/single_app/collaboration_models.py +++ b/application/single_app/collaboration_models.py @@ -42,6 +42,23 @@ def _clean_string(value): return str(value or '').strip() +def is_shared_conversation_backing(conversation_item): + """Whether a personal conversation record is stored under its own shared conversation's id. + + Orchestrate keeps a shared conversation's plans in such a hidden record (0.261.269). Its id is + the id every participant's browser uses for the shared conversation, so a lookup by that id + must treat the record as part of the shared conversation, never as anyone's personal one. + The classic assistant's hidden source conversation has an id of its own and is not one. + """ + item = conversation_item if isinstance(conversation_item, dict) else {} + conversation_id = _clean_string(item.get('id')) + return bool( + conversation_id + and _clean_string(item.get('conversation_kind')) == COLLABORATION_SOURCE_KIND + and _clean_string(item.get('collaboration_conversation_id')) == conversation_id + ) + + def normalize_collaboration_user(raw_user, fallback_user_id=None): raw_user = raw_user or {} if not isinstance(raw_user, dict): diff --git a/application/single_app/functions_collaboration.py b/application/single_app/functions_collaboration.py index c70156586..7a6a93424 100644 --- a/application/single_app/functions_collaboration.py +++ b/application/single_app/functions_collaboration.py @@ -2815,6 +2815,20 @@ def _delete_collaboration_conversation_records( _cancel_collaboration_pending_deliveries(conversation_doc) + # Orchestrate's backing record, stored under this shared conversation's id, goes with it. + # Removed first, so a failure leaves the shared conversation intact for a retry. + from functions_orchestration_collaboration import delete_orchestration_backing + delete_orchestration_backing( + conversation_doc, + archiving_enabled=archiving_enabled, + expected_user_id=expected_source_user_id, + conversation_container=cosmos_conversations_container, + message_container=cosmos_messages_container, + archived_conversation_container=cosmos_archived_conversations_container if archiving_enabled else None, + archived_message_container=cosmos_archived_messages_container if archiving_enabled else None, + not_found_error=CosmosResourceNotFoundError, + ) + if is_personal_collaboration_conversation(conversation_doc): revocation_conversation_doc = deepcopy(conversation_doc) revocation_conversation_doc['accepted_participant_ids'] = [] diff --git a/application/single_app/functions_conversation_metadata.py b/application/single_app/functions_conversation_metadata.py index fa94d4bed..bab3a2047 100644 --- a/application/single_app/functions_conversation_metadata.py +++ b/application/single_app/functions_conversation_metadata.py @@ -1,6 +1,7 @@ # functions_conversation_metadata.py from datetime import datetime +from collaboration_models import is_shared_conversation_backing from config import * from functions_settings import get_settings from functions_authentication import get_current_user_info @@ -64,7 +65,11 @@ def _normalize_scope_id_list(raw_ids): def _get_conversation_item_with_source(conversation_id): - """Load a conversation from the legacy or collaboration store.""" + """Load a conversation from the legacy or collaboration store. + + Orchestrate's backing record shares its shared conversation's id, and the shared + conversation is the one returned for that id. + """ normalized_conversation_id = str(conversation_id or '').strip() if not normalized_conversation_id: raise CosmosResourceNotFoundError(message='Conversation not found') @@ -74,10 +79,12 @@ def _get_conversation_item_with_source(conversation_id): item=normalized_conversation_id, partition_key=normalized_conversation_id ) - return conversation_item, 'legacy' + if not is_shared_conversation_backing(conversation_item): + return conversation_item, 'legacy' except CosmosResourceNotFoundError: - conversation_item = get_collaboration_conversation(normalized_conversation_id) - return conversation_item, 'collaboration' + pass + conversation_item = get_collaboration_conversation(normalized_conversation_id) + return conversation_item, 'collaboration' def _build_primary_context_from_scope_selection( diff --git a/application/single_app/functions_m365_approvals.py b/application/single_app/functions_m365_approvals.py index 9edae9eb2..c75df1735 100644 --- a/application/single_app/functions_m365_approvals.py +++ b/application/single_app/functions_m365_approvals.py @@ -4,6 +4,12 @@ Dependencies are resolved at operation time, not while config is bootstrapping. The approval container keeps its /group_id partition; that field is a physical partition only and never confers group or administrator authorization. + +Version: 0.261.269 +Shared by your own request in: 0.261.269. In a shared conversation, an interactive +request by the data owner (a context with ``shared_by_request``) is their consent to share +the sources it reads with that conversation; it is audited as ``shared_by_request`` and +needs no pending approval. Workflow Run as and history publication approvals are unchanged. """ import copy @@ -711,6 +717,24 @@ def _grant_use(self, context, source, approval, effective): "decision_event_id": approval.get("decision_event_id"), **effective, } + def _request_consent(self, context, source): + """Record that the data owner shared a source by asking for it in a shared conversation. + + The grant has the shape of an approved request-only grant, so publishing evidence the + request captured references this request's audit event in place of an approval. + """ + event_id = f"m365-request-{material_fingerprint([request_scope_fingerprint(context), source])}" + event = self._read(event_id, context.data_user_id) or self._create_once(self._audit_document( + context.data_user_id, "shared_by_request", event_id, + source=source, context=approval_context(context), + effective_grant={"effective_duration": "request"}, + )) + return { + "source": source, "sharing_required": False, "shared_by_request": True, + "approval_id": event_id, "audit_id": event_id, "decision_event_id": event_id, + "effective_duration": "request", "acknowledged_at": event["created_at"], "expires_at": None, + } + def authorize_sources(self, context, sources): if not isinstance(sources, Mapping) or not sources: raise ValueError("At least one Microsoft 365 source is required.") @@ -719,6 +743,10 @@ def authorize_sources(self, context, sources): return {source: {"source": source, "sharing_required": False} for source in sources} if not context.conversation_id or not context.audience_version: raise M365PolicyError("m365_context_required", "An authoritative conversation audience is required.") + if context.shared_by_request: + # Asking for your own Microsoft 365 data in a shared conversation is your consent to + # share it with that conversation; workflows and history publication still ask. + return {source: self._request_consent(context, source) for source in sources} state = self._state(context.data_user_id) scope = request_scope_fingerprint(context) granted = {} diff --git a/application/single_app/functions_m365_context.py b/application/single_app/functions_m365_context.py index 044a28b90..a2429e6c0 100644 --- a/application/single_app/functions_m365_context.py +++ b/application/single_app/functions_m365_context.py @@ -97,10 +97,18 @@ class M365ExecutionContext: workflow_fingerprint: str | None = None connection_id: str | None = None group_id: str | None = None + # An interactive request in a shared conversation by the data owner, whose own request is + # their consent to share the sources it reads there. Kept out of the approval fingerprint. + shared_by_request: bool = False def __post_init__(self): - if type(self.shared) is not bool or not isinstance(self.action_configs, Mapping): + if ( + type(self.shared) is not bool or type(self.shared_by_request) is not bool + or not isinstance(self.action_configs, Mapping) + ): raise ValueError("Invalid authoritative Microsoft 365 execution context.") + if self.shared_by_request and (not self.shared or self.workflow_id): + raise ValueError("Only an interactive shared request can share by request.") object.__setattr__(self, "action_configs", _immutable(self.action_configs)) approval_context(self) if not self.workflow_id and self.actor_user_id != self.data_user_id: diff --git a/application/single_app/functions_m365_pending_delivery.py b/application/single_app/functions_m365_pending_delivery.py index 3d2652f0c..ccd86aa29 100644 --- a/application/single_app/functions_m365_pending_delivery.py +++ b/application/single_app/functions_m365_pending_delivery.py @@ -1,5 +1,10 @@ # functions_m365_pending_delivery.py -"""Claimed M365 delivery using reviewed material and current subject authorization.""" +"""Claimed M365 delivery using reviewed material and current subject authorization. + +Version: 0.261.269 +A chat delivery keeps its request's ``shared_by_request`` consent in: 0.261.269, so sending a +reviewed action in a shared conversation needs no approval its request didn't need. +""" from copy import deepcopy from datetime import datetime, timedelta, timezone @@ -16,10 +21,12 @@ _dependencies = {} +# A delivery recorded before 0.261.269 has no ``shared_by_request`` and restores it as False. _CONTEXT_FIELDS = ( "actor_user_id", "data_user_id", "tenant_id", "conversation_id", "shared", "request_id", "workflow_id", "run_id", "step_id", "audience_version", "binding_id", "workflow_fingerprint", "connection_id", "group_id", "agent_id", + "shared_by_request", ) _TERMINAL = {"sent", "cancelled", "failed", "recovery_required"} DELIVERY_BINDING_VERSION = 1 diff --git a/application/single_app/functions_m365_runtime.py b/application/single_app/functions_m365_runtime.py index bdc2cb5e1..b004f8ecd 100644 --- a/application/single_app/functions_m365_runtime.py +++ b/application/single_app/functions_m365_runtime.py @@ -18,6 +18,7 @@ cosmos_conversations_container, cosmos_m365_execution_runs_container, ) +from collaboration_models import is_shared_conversation_backing from functions_appinsights import log_event from functions_collaboration import ( assert_user_can_participate_in_collaboration_conversation, @@ -230,6 +231,8 @@ def initialize_m365_chat_context(user_id, conversation_id, *, allow_new=False): actor_user_id=user_id, data_user_id=user_id, tenant_id=TENANT_ID, conversation_id=conversation_id, shared=shared is not None, request_id=request_id, audience_version=_audience_version(conversation, shared), + # Your own chat request in a shared conversation is your consent to share what it reads. + shared_by_request=shared is not None, ) g.m365_request_fingerprint = fingerprint return install_m365_context(context) @@ -717,10 +720,11 @@ def step_m365_context(*, user_id, conversation_id, request_id, selection, origin A plan step runs in a fresh request context, so the chat request's Microsoft 365 context never reaches it. This context uses the signed-in user as actor and data user, selects only the step's own action or agent, and never enables send, invite or - read-state functions. Shared conversations are refused: their source-sharing approvals - resume chat requests, not plan steps. Delegated sign-in is checked here, before any - model call or Microsoft Graph request. An agent step whose agent loads no Microsoft 365 - action gets no context and leaves no request record. + read-state functions. In a shared conversation it carries that conversation's real + audience; the user's own request is their consent to share the sources it reads there. + Delegated sign-in is checked here, before any model call or Microsoft Graph request. An + agent step whose agent loads no Microsoft 365 action gets no context and leaves no + request record. """ if not has_request_context() or (session.get("user") or {}).get("oid") != user_id: raise M365PolicyError("m365_session_required", "Sign in again to use Microsoft 365.") @@ -731,11 +735,6 @@ def step_m365_context(*, user_id, conversation_id, request_id, selection, origin if not selected and selection.get("kind") == "agent": yield None return - if shared is not None: - raise M365PolicyError( - "m365_shared_conversation_unsupported", - "Plans can't use Microsoft 365 in a shared conversation.", - ) previous_state = {name: value for name, value in vars(g).items() if name.startswith("m365_")} for name in previous_state: delattr(g, name) @@ -743,8 +742,9 @@ def step_m365_context(*, user_id, conversation_id, request_id, selection, origin try: context = M365ExecutionContext( actor_user_id=user_id, data_user_id=user_id, tenant_id=TENANT_ID, - conversation_id=conversation_id, shared=False, request_id=request_id, - audience_version=_audience_version(conversation, None), + conversation_id=conversation_id, shared=shared is not None, request_id=request_id, + audience_version=_audience_version(conversation, shared), + shared_by_request=shared is not None, ) install_m365_context(context) manifests = _readable_m365_manifests(selected) @@ -928,7 +928,9 @@ def authorize_m365_conversation_audit(user_id, conversation_id): def resolve_m365_audit_conversation_id(user_id, conversation_id): conversation, _access, _shared = _conversation_access(user_id, conversation_id) - if conversation is not None: + # Orchestrate's backing record shares its shared conversation's id; the shared + # conversation's Microsoft 365 records belong to its classic source conversation. + if conversation is not None and not is_shared_conversation_backing(conversation): return conversation_id shared = get_collaboration_conversation(conversation_id) assert_user_can_participate_in_collaboration_conversation(user_id, shared) diff --git a/application/single_app/functions_mcp_server_tools.py b/application/single_app/functions_mcp_server_tools.py index 8cd24c6f7..058ac6c8d 100644 --- a/application/single_app/functions_mcp_server_tools.py +++ b/application/single_app/functions_mcp_server_tools.py @@ -7,7 +7,7 @@ from azure.cosmos.exceptions import CosmosResourceNotFoundError from background_tasks import acquire_distributed_task_lock, release_distributed_task_lock -from collaboration_models import MEMBERSHIP_STATUS_ACCEPTED +from collaboration_models import MEMBERSHIP_STATUS_ACCEPTED, is_shared_conversation_backing from config import ( cosmos_conversations_container, cosmos_messages_container, @@ -257,6 +257,9 @@ def _authorize_personal_conversation_read(delegated_user_id, conversation_id): ) except CosmosResourceNotFoundError: conversation_item = None + # Orchestrate's backing record shares its shared conversation's id and is part of it. + if is_shared_conversation_backing(conversation_item): + conversation_item = None if conversation_item is not None: if str(conversation_item.get("user_id") or "").strip() != delegated_user_id: diff --git a/application/single_app/functions_orchestration_artifacts.py b/application/single_app/functions_orchestration_artifacts.py index 7abbd988a..9a27973e0 100644 --- a/application/single_app/functions_orchestration_artifacts.py +++ b/application/single_app/functions_orchestration_artifacts.py @@ -148,6 +148,26 @@ def is_orchestration_artifact_source(value): return type(value) is dict and value.get("kind") == ORCHESTRATION_ARTIFACT_KIND +def is_retained_orchestration_file(message): + """Whether a file message is a retained orchestration output, deleted by output cleanup. + + Conversation deletion leaves these records and their bytes to the conditional output + cleanup it enrolls, whether the conversation is personal or a shared conversation's + Orchestrate backing. + """ + if not isinstance(message, dict) or message.get("role") != "file": + return False + metadata = message.get("metadata") + if not isinstance(metadata, dict): + return False + key = metadata.get("generated_artifact_idempotency_key") + return ( + is_orchestration_artifact_source(metadata.get("generated_artifact_source")) + or metadata.get("generated_artifact_origin") == ORCHESTRATION_ARTIFACT_KIND + or isinstance(key, str) and key.startswith(ORCHESTRATION_ARTIFACT_KEY_PREFIX) + ) + + def validate_orchestration_artifact_binding(value): if ( type(value) is not dict or set(value) != _SOURCE_FIELDS diff --git a/application/single_app/functions_orchestration_collaboration.py b/application/single_app/functions_orchestration_collaboration.py new file mode 100644 index 000000000..35fed7bf3 --- /dev/null +++ b/application/single_app/functions_orchestration_collaboration.py @@ -0,0 +1,373 @@ +# functions_orchestration_collaboration.py +"""Orchestrate in a shared conversation. + +Version: 0.261.269 +Implemented in: 0.261.269 + +A shared conversation's plans run in a hidden backing conversation that reuses the shared +conversation's id and belongs to the person who started it, so every plan, run, result and +answer keeps the id the browser already holds. That person's question and each run's final +answer are mirrored into the shared thread and announced to every participant, and an answer +the run republishes later updates its shared copy. Messages that only address people never +reach a plan: the composer posts them to the thread instead. + +Only the person who started a shared conversation can orchestrate in it, the same person who +owns its classic assistant's hidden source conversation, and only while they still take part +in it. Other participants ask the assistant the classic way. Lookups elsewhere treat the +backing as part of the shared conversation (``collaboration_models.is_shared_conversation_backing``), +and deleting the shared conversation deletes its backing. Collaboration storage is imported on +first use rather than with this module, so importing it never initializes Cosmos clients. +""" + +import logging + +from collaboration_models import COLLABORATION_SOURCE_KIND, is_shared_conversation_backing +from functions_appinsights import log_event + + +SHARED_ORCHESTRATION_OWNER_ONLY = ( + 'Only the person who started this shared conversation can use Orchestrate here. ' + 'Turn off Orchestrate to ask the assistant.' +) +SHARED_ORCHESTRATION_STALE_COPY = ( + "Orchestrate can't be used in this shared conversation because an earlier version kept " + 'another participant\'s private copy of it. Turn off Orchestrate to ask the assistant.' +) +# Fields a mirrored answer takes from the run's own message; refreshed when the run republishes. +_MIRRORED_ANSWER_FIELDS = ( + 'content', 'role', 'model_deployment_name', 'augmented', 'hybrid_citations', + 'web_search_citations', 'citation_tracking_version', 'cited_hybrid_citations', + 'cited_web_search_citations', 'agent_citations', 'agent_display_name', 'agent_name', +) +# Metadata the shared copy keeps from when it was first mirrored. +_MIRROR_METADATA_KEPT = ('source_conversation_id', 'source_thought_user_id') + + +class SharedOrchestrationError(PermissionError): + """Orchestrate is not available to this participant of a shared conversation.""" + + def __init__(self, message=SHARED_ORCHESTRATION_OWNER_ONLY): + super().__init__(message) + self.message = message + + +def is_orchestration_backing(conversation): + """Whether a personal conversation record is a shared conversation's orchestration backing.""" + return is_shared_conversation_backing(conversation) + + +def shared_conversation(conversation_id): + """The shared conversation with this id, or None when it is not one.""" + # Collaboration storage initializes Cosmos clients; it is imported on first use. + from azure.cosmos.exceptions import CosmosResourceNotFoundError + from functions_collaboration import get_collaboration_conversation + + if not isinstance(conversation_id, str) or not conversation_id.strip(): + return None + try: + return get_collaboration_conversation(conversation_id) + except CosmosResourceNotFoundError: + return None + + +def authorize_shared_orchestration(collaboration, user_id): + """Require a current participant who started the conversation; return the backing fields.""" + from functions_collaboration import ( + assert_user_can_participate_in_collaboration_conversation, + is_group_collaboration_conversation, + ) + + assert_user_can_participate_in_collaboration_conversation(user_id, collaboration) + if str(collaboration.get('created_by_user_id') or '').strip() != str(user_id or '').strip(): + raise SharedOrchestrationError() + return { + 'conversation_kind': COLLABORATION_SOURCE_KIND, + 'collaboration_conversation_id': collaboration['id'], + 'is_hidden': True, + 'chat_type': 'group' if is_group_collaboration_conversation(collaboration) else 'personal_single_user', + # The shared conversation's workspace lock applies to its plans, as it does to the + # classic assistant's hidden source conversation. + 'context': list(collaboration.get('context') or []), + 'scope_locked': bool(collaboration.get('scope_locked', False)), + 'locked_contexts': list(collaboration.get('locked_contexts') or []), + } + + +def _publish_message(collaboration, message_doc, viewer_user_id): + from functions_collaboration import ( + create_collaboration_message_notifications, + get_collaboration_user_state_or_none, + publish_collaboration_event, + serialize_collaboration_conversation, + serialize_collaboration_message, + ) + from collaboration_models import utc_now_iso + + create_collaboration_message_notifications(collaboration, message_doc) + message = serialize_collaboration_message(message_doc) + message.pop('m365_pending_actions', None) + if isinstance(message.get('metadata'), dict): + message['metadata'] = { + key: value for key, value in message['metadata'].items() if key != 'm365_pending_actions' + } + publish_collaboration_event(collaboration['id'], { + 'conversation_id': collaboration['id'], + 'event_type': 'collaboration.message.created', + 'occurred_at': utc_now_iso(), + 'payload': { + 'conversation': serialize_collaboration_conversation( + collaboration, current_user_id=viewer_user_id, + user_state=get_collaboration_user_state_or_none(viewer_user_id, collaboration['id']), + ), + 'message': message, + }, + }) + + +def _authorized_collaboration(conversation): + """The backing's shared conversation, while its owner may still orchestrate there. + + Rechecked on every post: someone removed from a shared conversation, or whose group no + longer allows chat, keeps their plans' answers to themselves. + """ + from functions_collaboration import get_collaboration_conversation + + collaboration = get_collaboration_conversation(conversation['collaboration_conversation_id']) + authorize_shared_orchestration(collaboration, conversation.get('user_id')) + return collaboration + + +def mirror_orchestration_turn(conversation, user_message, sender_user, *, details=None): + """Post the owner's orchestrated question to the shared thread once, and announce it.""" + from functions_collaboration import ( + get_collaboration_message_by_source_message, + persist_collaboration_message, + resolve_collaboration_mentions, + ) + from collaboration_models import MESSAGE_KIND_AI_REQUEST + + if not is_orchestration_backing(conversation) or not isinstance(user_message, dict): + return None + if str((sender_user or {}).get('user_id') or '') != str(conversation.get('user_id') or ''): + raise SharedOrchestrationError() + collaboration = _authorized_collaboration(conversation) + existing = get_collaboration_message_by_source_message(collaboration['id'], user_message.get('id')) + if existing: + return existing + details = details if isinstance(details, dict) else {} + metadata = user_message.get('metadata') if isinstance(user_message.get('metadata'), dict) else {} + extra = { + 'source_message_id': user_message['id'], + 'source_conversation_id': conversation['id'], + 'source_role': 'user', + 'orchestration_turn_id': metadata.get('orchestration_turn_id'), + 'orchestration': {'turn_id': metadata.get('orchestration_turn_id')}, + } + if isinstance(details.get('invocation_target'), dict): + extra['ai_invocation_target'] = details['invocation_target'] + if isinstance(metadata.get('prompt_selection'), dict): + extra['prompt_selection'] = metadata['prompt_selection'] + message_doc, updated = persist_collaboration_message( + collaboration, sender_user, user_message.get('content') or '', + reply_to_message_id=details.get('reply_to_message_id') or None, + mentioned_participants=resolve_collaboration_mentions( + collaboration, details.get('mentioned_participants'), + ), + message_kind=MESSAGE_KIND_AI_REQUEST, + extra_metadata=extra, + ) + _publish_message(updated, message_doc, sender_user.get('user_id')) + return message_doc + + +def mirror_orchestration_answer(conversation, answer, *, user_message_id=None, owner_user_id=None): + """Post a run's final answer to the shared thread, replying to its question. + + A run republishes its answer under the same id when it finishes later, for example after a + step that was waiting completes or the run times out. The shared copy is then updated in + place and announced, so participants never keep the earlier text. + """ + from functions_collaboration import ( + get_collaboration_message_by_source_message, + mirror_source_message_to_collaboration, + ) + + if not is_orchestration_backing(conversation) or not isinstance(answer, dict): + return None + owner = conversation.get('user_id') + if owner_user_id is not None and owner_user_id != owner: + raise SharedOrchestrationError() + collaboration = _authorized_collaboration(conversation) + existing = get_collaboration_message_by_source_message(collaboration['id'], answer.get('id')) + if existing: + return _refresh_mirrored_answer(collaboration, existing, answer, owner) + question = get_collaboration_message_by_source_message(collaboration['id'], user_message_id) + message_doc, updated, created = mirror_source_message_to_collaboration( + collaboration, answer, {'user_id': owner}, + reply_to_message_id=(question or {}).get('id'), + extra_metadata={'source_conversation_id': conversation['id']}, + ) + if message_doc is None: + return None + if created: + _publish_message(updated, message_doc, owner) + return message_doc + + +def _refresh_mirrored_answer(collaboration, existing, answer, owner): + """Bring an existing shared copy up to date with a republished answer, and announce it.""" + from azure.core import MatchConditions + import functions_collaboration as collaboration_store + from collaboration_models import build_collaboration_message_doc_from_legacy, utc_now_iso + from functions_workflow_result_masking import ( + message_uses_workflow_result, + withhold_workflow_result_message, + ) + + source = withhold_workflow_result_message(answer) if message_uses_workflow_result(answer) else answer + fresh = build_collaboration_message_doc_from_legacy(collaboration['id'], source, {'user_id': owner}) + if not fresh: + return existing + refreshed = dict(existing) + for field in _MIRRORED_ANSWER_FIELDS: + if field in fresh: + refreshed[field] = fresh[field] + else: + refreshed.pop(field, None) + existing_metadata = existing.get('metadata') if isinstance(existing.get('metadata'), dict) else {} + refreshed['metadata'] = { + **(fresh.get('metadata') or {}), + **{key: existing_metadata[key] for key in _MIRROR_METADATA_KEPT if key in existing_metadata}, + } + + def visible(message): + return { + key: value for key, value in message.items() + if not key.startswith('_') and key in (*_MIRRORED_ANSWER_FIELDS, 'metadata') + } + + if visible(refreshed) == visible(existing): + return existing + saved = collaboration_store.cosmos_collaboration_messages_container.replace_item( + item=existing['id'], body=refreshed, etag=existing.get('_etag'), + match_condition=MatchConditions.IfNotModified, + ) + collaboration_store.publish_collaboration_event(collaboration['id'], { + 'conversation_id': collaboration['id'], + 'event_type': 'collaboration.message.updated', + 'occurred_at': utc_now_iso(), + 'payload': {'message': collaboration_store.serialize_collaboration_message(saved)}, + }) + return saved + + +def delete_orchestration_backing( + collaboration, *, archiving_enabled, expected_user_id=None, + conversation_container, message_container, + archived_conversation_container=None, archived_message_container=None, + not_found_error=None, +): + """Delete a shared conversation's Orchestrate backing together with the shared conversation. + + Follows deleting a personal conversation: runs are fenced and their retained files enrolled + for output cleanup, then messages, thoughts and the record are archived or deleted. A + co-owner who didn't start the conversation removes it without deleting its creator's plans, + as with the classic assistant's source conversation. The caller's containers, and the + exception its container raises for a missing item, are used, and the deletion services load + only once a backing is found. Returns the backing that was deleted, or None when there is none. + """ + from azure.cosmos.exceptions import CosmosResourceNotFoundError + + not_found_error = not_found_error or CosmosResourceNotFoundError + conversation_id = str((collaboration or {}).get('id') or '').strip() + if not conversation_id: + return None + + def read_backing(): + try: + return conversation_container.read_item(item=conversation_id, partition_key=conversation_id) + except not_found_error: + return None + + backing = read_backing() + if not is_orchestration_backing(backing): + return None + owner = str(backing.get('user_id') or '').strip() + if not owner or (expected_user_id and owner != str(expected_user_id).strip()): + return None + + # Deletion services reach application storage; they load only for a backing to delete. + from functools import partial + from datetime import datetime, timezone + + from azure.core import MatchConditions + from functions_conversation_cache import invalidate_conversation_cache_for_item + from functions_orchestration_artifacts import is_retained_orchestration_file + from functions_orchestration_recovery import cleanup_conversation_checkpoints + from functions_saved_analysis import cleanup_chat_analysis_conversation + from functions_simplechat_operations import delete_blob_backed_chat_message_files + from functions_thoughts import archive_thoughts_for_conversation, delete_thoughts_for_conversation + + if archiving_enabled and (archived_conversation_container is None or archived_message_container is None): + raise ValueError('Archiving a shared conversation backing needs its archive containers.') + + def authorize(): + current = read_backing() + return is_orchestration_backing(current) and current.get('user_id') == owner + + cleanup_conversation_checkpoints( + conversation_id, owner, authorize, + message_container=message_container, + conversation_container=conversation_container, + output_cleanup=partial(_enroll_backing_outputs, owner, conversation_id), + retain_committed=archiving_enabled, + ) + messages = list(message_container.query_items( + query='SELECT * FROM c WHERE c.conversation_id = @conversation_id', + parameters=[{'name': '@conversation_id', 'value': conversation_id}], + partition_key=conversation_id, + )) + cleanup_chat_analysis_conversation(conversation_id, owner, messages) + direct_messages = [message for message in messages if not is_retained_orchestration_file(message)] + if not archiving_enabled: + delete_blob_backed_chat_message_files(direct_messages, conversation=backing) + archived_at = datetime.now(timezone.utc).isoformat() + direct_message_ids = {message['id'] for message in direct_messages} + for message in messages: + if archiving_enabled: + archived_message_container.upsert_item({**message, 'archived_at': archived_at}) + if message['id'] not in direct_message_ids: + continue + try: + message_container.delete_item(message['id'], partition_key=conversation_id) + except not_found_error: + pass + if archiving_enabled: + archive_thoughts_for_conversation(conversation_id, owner) + archived_conversation_container.upsert_item({**backing, 'archived_at': archived_at}) + else: + delete_thoughts_for_conversation(conversation_id, owner) + current = read_backing() + if current is not None: + # Conditional, so a backing that changed after cleanup is left for a retry. + conversation_container.delete_item( + item=conversation_id, partition_key=conversation_id, + etag=current.get('_etag'), match_condition=MatchConditions.IfNotModified, + ) + invalidate_conversation_cache_for_item(backing, reason='orchestration_backing_deleted') + return backing + + +def _enroll_backing_outputs(user_id, conversation_id, run_id): + """Initialize deletion-only output cleanup for one of the backing's runs.""" + from functions_orchestration_bootstrap import build_orchestration_cleanup_service + + return build_orchestration_cleanup_service(user_id, conversation_id).enroll_run_cleanup(run_id) + + +def log_mirror_failure(stage, error): + """A mirror failure leaves the run's own record intact; log it without content.""" + log_event( + '[ORCHESTRATION_COLLABORATION] A shared conversation message was not mirrored.', + level=logging.WARNING, extra={'stage': stage, 'error_type': type(error).__name__}, + ) diff --git a/application/single_app/functions_orchestration_execution.py b/application/single_app/functions_orchestration_execution.py index 8638b2b78..a11bb3278 100644 --- a/application/single_app/functions_orchestration_execution.py +++ b/application/single_app/functions_orchestration_execution.py @@ -1,8 +1,9 @@ # functions_orchestration_execution.py """Headless preparation and guarded publication for saved orchestration attempts. -Version: 0.261.141 +Version: 0.261.269 Implemented in: 0.261.127 +Shared conversation answers mirrored into the shared thread in: 0.261.269 Every saved attempt uses the Gather / Reason / Render contract; a run from the removed legacy contract is refused before any preparation. The ``Harness*`` names below are the @@ -1204,6 +1205,26 @@ def _touch_conversation(self, documents): except Exception as exc: _log_failure("The saved conversation index could not be refreshed.", self.record, exc) + def _mirror_to_shared_thread(self, answer): + """Show a shared conversation's answer to every participant, replying to its question. + + Collaboration storage loads only for a shared conversation's backing conversation, and + a mirror failure never changes the run's own published answer. + """ + from functions_orchestration_collaboration import ( + is_orchestration_backing, log_mirror_failure, mirror_orchestration_answer, + ) + + try: + conversation = self._read_conversation() + if is_orchestration_backing(conversation): + mirror_orchestration_answer( + conversation, answer, user_message_id=self.record.get("user_message_id"), + owner_user_id=self.record["user_id"], + ) + except Exception as exc: + log_mirror_failure("answer", exc) + def _reads_workflow_results(self): return any( step.get("enabled", True) and step.get("capability_id") == CAPABILITY_WORKFLOW_RESULTS @@ -1522,6 +1543,7 @@ def _finalize(self, result, error, *, refreshes=0): _log_failure("A chat content incident could not be recorded.", self.record, exc) if self._bootstrap is not None: self._touch_conversation(documents) + self._mirror_to_shared_thread(saved) finalized = self.lease.close(release=True) self._released = True self._final_record = finalized diff --git a/application/single_app/functions_simplechat_operations.py b/application/single_app/functions_simplechat_operations.py index f171fb235..d7d33b0b4 100644 --- a/application/single_app/functions_simplechat_operations.py +++ b/application/single_app/functions_simplechat_operations.py @@ -20,7 +20,7 @@ from azure.cosmos.exceptions import CosmosResourceExistsError, CosmosResourceNotFoundError from flask import current_app, has_app_context, session -from collaboration_models import normalize_collaboration_user +from collaboration_models import is_shared_conversation_backing, normalize_collaboration_user from conversation_memory_lifecycle import clone_owned_memory, delete_referenced_conversation_memory, remap_memory_references from functions_conversation_memory import ConversationMemoryStore, MemoryContext, is_conversation_memory_blob_path from config import ( @@ -1158,6 +1158,9 @@ def add_conversation_message_for_current_user( ) except CosmosResourceNotFoundError: conversation_item = None + # Orchestrate's backing record shares its shared conversation's id; posts go to the thread. + if is_shared_conversation_backing(conversation_item): + conversation_item = None if conversation_item is not None: if str(conversation_item.get("user_id") or "").strip() != current_user_info["userId"]: diff --git a/application/single_app/route_backend_chats.py b/application/single_app/route_backend_chats.py index 1804b25ed..8c17a76f0 100644 --- a/application/single_app/route_backend_chats.py +++ b/application/single_app/route_backend_chats.py @@ -271,6 +271,7 @@ merge_cited_documents_into_conversation, resolve_citation_location, ) +from collaboration_models import is_shared_conversation_backing from functions_collaboration import build_conversation_participation_context from functions_m365_action_cards import ( get_request_pending_action_references, @@ -4105,6 +4106,10 @@ def _resolve_authorized_conversation_context(user_id, conversation_id): ) except CosmosResourceNotFoundError as exc: raise LookupError(f"Conversation {conversation_id} not found") from exc + # Orchestrate's backing record shares its shared conversation's id. Chat in a shared + # conversation runs through the shared conversation, never directly in that record. + if is_shared_conversation_backing(conversation_item): + raise LookupError(f"Conversation {conversation_id} not found") access_context = build_conversation_participation_context(user_id, conversation_item) return conversation_item, access_context diff --git a/application/single_app/route_backend_conversation_export.py b/application/single_app/route_backend_conversation_export.py index 2c2599bcf..1b2802ab2 100644 --- a/application/single_app/route_backend_conversation_export.py +++ b/application/single_app/route_backend_conversation_export.py @@ -14,6 +14,7 @@ from typing import Any, Dict, List, Optional, Tuple from bs4 import BeautifulSoup, NavigableString, Tag +from collaboration_models import is_shared_conversation_backing from content_screening.access import public_history_messages from content_screening.contracts import DocumentHeldError from config import * @@ -609,12 +610,18 @@ def _load_exportable_conversation_for_user( user_id: str, conversation_id: str, ) -> Optional[Tuple[Dict[str, Any], List[Dict[str, Any]]]]: - """Load a conversation and its messages when the user is allowed to export it.""" + """Load a conversation and its messages when the user is allowed to export it. + + Orchestrate's backing record shares its shared conversation's id; the shared conversation + is what gets exported. + """ try: conversation = cosmos_conversations_container.read_item( item=conversation_id, partition_key=conversation_id ) + if is_shared_conversation_backing(conversation): + raise LookupError('A shared conversation backing is exported as its shared conversation.') if conversation.get('user_id') != user_id: debug_print(f"Export: user {user_id} does not own conversation {conversation_id}") return None diff --git a/application/single_app/route_backend_conversations.py b/application/single_app/route_backend_conversations.py index 8aaadc331..d543af51a 100644 --- a/application/single_app/route_backend_conversations.py +++ b/application/single_app/route_backend_conversations.py @@ -9,7 +9,11 @@ from content_screening.access import build_available_document_response, public_history_messages, read_available_document_bytes from content_screening.contracts import ScreeningError -from collaboration_models import GROUP_MULTI_USER_CHAT_TYPE, PERSONAL_MULTI_USER_CHAT_TYPE +from collaboration_models import ( + GROUP_MULTI_USER_CHAT_TYPE, + PERSONAL_MULTI_USER_CHAT_TYPE, + is_shared_conversation_backing, +) from config import * from functions_appinsights import log_event from functions_chat_content_checks import attach_chat_check, check_chat_content, strip_private_chat_checks @@ -87,11 +91,7 @@ from functions_activity_logging import log_conversation_creation, log_conversation_deletion, log_conversation_archival from functions_thoughts import archive_thoughts_for_conversation, delete_thoughts_for_conversation from functions_orchestration_recovery import cleanup_conversation_checkpoints -from functions_orchestration_artifacts import ( - ORCHESTRATION_ARTIFACT_KEY_PREFIX, - ORCHESTRATION_ARTIFACT_KIND, - is_orchestration_artifact_source, -) +from functions_orchestration_artifacts import is_retained_orchestration_file from functions_orchestration_external_configuration import ExternalConfigurationServiceError from functions_orchestration_external_identity import ExternalIdentityServiceError from functions_orchestration_output_store import OutputError, OutputStorageError @@ -115,17 +115,7 @@ def _is_retained_orchestration_file(message): """Leave retained file records and bytes to conditional output cleanup.""" - if not isinstance(message, dict) or message.get('role') != 'file': - return False - metadata = message.get('metadata') - if not isinstance(metadata, dict): - return False - key = metadata.get('generated_artifact_idempotency_key') - return ( - is_orchestration_artifact_source(metadata.get('generated_artifact_source')) - or metadata.get('generated_artifact_origin') == ORCHESTRATION_ARTIFACT_KIND - or isinstance(key, str) and key.startswith(ORCHESTRATION_ARTIFACT_KEY_PREFIX) - ) + return is_retained_orchestration_file(message) def _enroll_retained_orchestration_outputs(user_id, conversation_id, run_id): @@ -661,9 +651,11 @@ def _query_legacy_conversations_for_feed( def _count_hidden_legacy_conversations(user_id): + # A shared conversation's orchestration backing record is not a hidden conversation. query = ( 'SELECT VALUE COUNT(1) FROM c ' - 'WHERE c.user_id = @user_id AND c.is_hidden = true' + 'WHERE c.user_id = @user_id AND c.is_hidden = true ' + 'AND (NOT IS_DEFINED(c.collaboration_conversation_id) OR c.collaboration_conversation_id != c.id)' ) results = list(cosmos_conversations_container.query_items( query=query, @@ -772,10 +764,16 @@ def _build_conversation_feed(user_id, page_size, source_offsets, include_priorit collaboration_conversations = [] hidden_count += sum(1 for conversation in collaboration_conversations if conversation.get('is_hidden', False)) + # A shared conversation's own id also names its orchestration backing record, and a + # record an earlier version created there; neither is a separate conversation. collaboration_source_ids = { str(conversation.get('source_conversation_id') or '').strip() for conversation in collaboration_conversations if conversation.get('source_conversation_id') + } | { + str(conversation.get('id') or '').strip() + for conversation in collaboration_conversations + if conversation.get('id') } filtered_collaboration_conversations = _filter_collaboration_conversations_for_feed( @@ -942,7 +940,12 @@ def _promote_remaining_thread_attempt(conversation_id, thread_id, deleted_attemp def _authorize_personal_conversation_read(user_id, conversation_id): - """Load a personal conversation and ensure the caller owns it.""" + """Load a personal conversation and ensure the caller owns it. + + A shared conversation's Orchestrate backing record has the shared conversation's id but is + part of the shared conversation, so it is reported as not found here. Callers that also + serve shared conversations then fall through to them. + """ try: conversation_item = cosmos_conversations_container.read_item( item=conversation_id, @@ -951,6 +954,8 @@ def _authorize_personal_conversation_read(user_id, conversation_id): except CosmosResourceNotFoundError as exc: raise LookupError(f"Conversation {conversation_id} not found") from exc + if is_shared_conversation_backing(conversation_item): + raise LookupError(f"Conversation {conversation_id} not found") if conversation_item.get('user_id') != user_id: raise PermissionError('Forbidden') @@ -1108,9 +1113,12 @@ def _load_scope_lock_conversation(conversation_id, user_id): item=conversation_id, partition_key=conversation_id, ) - if conversation_item.get('user_id') != user_id: - raise PermissionError('Forbidden') - return conversation_item, 'personal' + # Orchestrate's backing record follows its shared conversation's lock; the lock itself + # belongs to the shared conversation below. + if not is_shared_conversation_backing(conversation_item): + if conversation_item.get('user_id') != user_id: + raise PermissionError('Forbidden') + return conversation_item, 'personal' except CosmosResourceNotFoundError: pass @@ -2291,23 +2299,26 @@ def get_conversation_kind_api(conversation_id): A conversation the caller may not see is reported as absent rather than forbidden. The two are indistinguishable to someone who should not know it exists, and the client treats them identically. + + A personal record stored under a shared conversation's id is part of that shared + conversation: Orchestrate's backing record (0.261.269), or a private copy an earlier + version made there. The shared conversation is reported for anyone who can see it, so + reopening it never turns it into a personal chat. A private copy stays reachable as + personal only for an owner who can no longer see the shared conversation. """ user_id = get_current_user_id() if not user_id: return jsonify({'error': 'User not authenticated'}), 401 + personal_response = ({'conversation_id': conversation_id, 'kind': 'personal'}, 200) + not_found_response = ({'error': 'Conversation not found'}, 404) try: conversation_item = cosmos_conversations_container.read_item( item=conversation_id, partition_key=conversation_id, ) - if conversation_item.get('user_id') == user_id: - return jsonify({ - 'conversation_id': conversation_id, - 'kind': 'personal', - }), 200 except CosmosResourceNotFoundError: - pass + conversation_item = None except Exception as e: log_event( f"[CONVERSATION_KIND] Failed to read personal conversation {conversation_id}: {e}", @@ -2315,13 +2326,23 @@ def get_conversation_kind_api(conversation_id): exceptionTraceback=True, ) return jsonify({'error': 'Failed to resolve conversation'}), 500 + owns_personal = bool( + conversation_item + and conversation_item.get('user_id') == user_id + and not is_shared_conversation_backing(conversation_item) + ) + + def personal_or(response, status): + if owns_personal: + return jsonify(personal_response[0]), personal_response[1] + return jsonify(response), status # Checked before answering "collaborative": with the feature off, the collaboration # endpoints refuse everything, so naming a conversation as shared would only send the # client somewhere it cannot go. settings = get_settings() or {} if not settings.get('enable_collaborative_conversations', False): - return jsonify({'error': 'Conversation not found'}), 404 + return personal_or(*not_found_response) try: collaboration_item = get_collaboration_conversation(conversation_id) @@ -2331,20 +2352,21 @@ def get_conversation_kind_api(conversation_id): allow_pending=True, ) except CosmosResourceNotFoundError: - return jsonify({'error': 'Conversation not found'}), 404 + return personal_or(*not_found_response) except LookupError: # Raised when the stored document is not a collaboration conversation, which for a # question about kind is the same answer as it not being there. - return jsonify({'error': 'Conversation not found'}), 404 + return personal_or(*not_found_response) except PermissionError: - return jsonify({'error': 'Conversation not found'}), 404 + return personal_or(*not_found_response) except Exception as e: log_event( f"[CONVERSATION_KIND] Failed to resolve shared conversation {conversation_id}: {e}", level=logging.WARNING, exceptionTraceback=True, ) - return jsonify({'error': 'Failed to resolve conversation'}), 500 + # A personal conversation still opens while shared conversations can't be read. + return personal_or({'error': 'Failed to resolve conversation'}, 500) # Returned alongside the kind because the caller needs this exact document next, and # asking for it twice is the cost the old probe was paying to avoid. @@ -2430,13 +2452,23 @@ def generate_conversation_summary_api(conversation_id): is_collaboration_summary = False try: - conversation_item = cosmos_conversations_container.read_item( + personal_item = cosmos_conversations_container.read_item( item=conversation_id, partition_key=conversation_id ) - if conversation_item.get('user_id') != user_id: - return jsonify({'error': 'Forbidden'}), 403 except CosmosResourceNotFoundError: + personal_item = None + except Exception as e: + debug_print(f"Error reading conversation for summary: {e}") + return jsonify({'error': 'Failed to read conversation'}), 500 + + # Orchestrate's backing record shares its shared conversation's id; the summary is the + # shared conversation's. + if personal_item is not None and not is_shared_conversation_backing(personal_item): + if personal_item.get('user_id') != user_id: + return jsonify({'error': 'Forbidden'}), 403 + conversation_item = personal_item + else: try: conversation_item = get_collaboration_conversation(conversation_id) assert_user_can_view_collaboration_conversation( @@ -2452,9 +2484,6 @@ def generate_conversation_summary_api(conversation_id): except Exception as e: debug_print(f"Error reading collaborative conversation for summary: {e}") return jsonify({'error': 'Failed to read conversation'}), 500 - except Exception as e: - debug_print(f"Error reading conversation for summary: {e}") - return jsonify({'error': 'Failed to read conversation'}), 500 body = request.get_json(silent=True) or {} model_deployment = body.get('model_deployment', '') diff --git a/application/single_app/route_backend_orchestration.py b/application/single_app/route_backend_orchestration.py index eb6074acd..937d0042b 100644 --- a/application/single_app/route_backend_orchestration.py +++ b/application/single_app/route_backend_orchestration.py @@ -24,7 +24,11 @@ restored or continued. Every by-id route answers it with HTTP 409 and the one stable ``LEGACY_PLAN_MESSAGE``, and conversation run listings omit it. -Version: 0.261.141 +In a shared conversation the person who started it plans in a hidden backing conversation +with the shared conversation's id; the question and each final answer are mirrored into the +shared thread for every participant (0.261.269). + +Version: 0.261.269 """ import hashlib @@ -128,6 +132,16 @@ load_orchestration_memory, validate_memory_context, ) +from functions_orchestration_collaboration import ( + SHARED_ORCHESTRATION_STALE_COPY, + SharedOrchestrationError, + authorize_shared_orchestration, + is_orchestration_backing, + log_mirror_failure, + mirror_orchestration_turn, + shared_conversation, +) +from collaboration_models import normalize_collaboration_user from functions_orchestration_external_configuration import ExternalConfigurationServiceError from functions_orchestration_external_identity import ExternalIdentityServiceError from functions_orchestration_workflow_context import ( @@ -752,23 +766,27 @@ def _ensure_conversation(conversation_id, user_id, title=''): exist before a plan can be stored against it. Created here with the same shape ``route_backend_chats`` uses, so a conversation started by orchestration is indistinguishable from any other and the classic interface can open it. + + A shared conversation's id gets a hidden backing conversation with that same id, owned by + the person who started the shared conversation, so plans never become a private copy that + other participants cannot see. A backing record created before shared conversations had + one is adopted in place, and the backing keeps the shared conversation's workspace lock. + Raises ``SharedOrchestrationError`` for any other participant; anyone who is not a + participant gets the same answer as for a conversation that does not exist. A backing + conversation is never reported as created, because the browser already holds its id. """ + requested = bool(conversation_id) conversation_id = conversation_id or f"conv_{uuid.uuid4().hex}" now = _now_iso() try: existing = cosmos_conversations_container.read_item( item=conversation_id, partition_key=conversation_id - ) - if existing.get('user_id') != user_id: - # Someone else's conversation. Treated as absent rather than reported, so an - # id cannot be used to probe for conversations that exist. - return None, False - return conversation_id, False + ) if requested else None except CosmosResourceNotFoundError: - pass + existing = None except Exception as exc: log_event( '[ORCHESTRATION] Conversation ownership could not be verified.', @@ -776,16 +794,55 @@ def _ensure_conversation(conversation_id, user_id, title=''): ) return None, False + try: + # A new chat's generated id can't belong to a shared conversation. + collaboration = shared_conversation(conversation_id) if requested else None + backing = authorize_shared_orchestration(collaboration, user_id) if collaboration is not None else {} + except SharedOrchestrationError: + raise + except Exception as exc: + log_event( + '[ORCHESTRATION] Shared conversation access could not be verified.', + extra={'exception_type': type(exc).__name__}, level=logging.WARNING, + ) + return None, False + + if existing is not None: + if existing.get('user_id') != user_id: + if backing: + # The person who started this shared conversation, blocked by a private copy that + # an earlier version made for another participant under the same id. + log_event( + '[ORCHESTRATION] A shared conversation has another participant\'s private copy.', + extra={'reason': 'shared_conversation_stale_copy'}, level=logging.WARNING, + ) + raise SharedOrchestrationError(SHARED_ORCHESTRATION_STALE_COPY) + # Someone else's conversation. Treated as absent rather than reported, so an + # id cannot be used to probe for conversations that exist. + return None, False + if any(existing.get(key) != value for key, value in backing.items()): + existing.update(backing) + try: + cosmos_conversations_container.upsert_item(existing) + except Exception as exc: + log_event( + '[ORCHESTRATION] A shared conversation backing could not be updated.', + extra={'exception_type': type(exc).__name__}, level=logging.WARNING, + ) + return None, False + return conversation_id, False + try: cosmos_conversations_container.create_item({ 'id': conversation_id, 'user_id': user_id, 'last_updated': now, - 'title': _text(title, 80) or 'New Conversation', + 'title': _text((collaboration or {}).get('title') or title, 80) or 'New Conversation', 'context': [], 'tags': [], 'strict': False, 'chat_type': 'new', + **backing, }) except Exception as exc: log_event( @@ -794,7 +851,7 @@ def _ensure_conversation(conversation_id, user_id, title=''): ) return None, False - return conversation_id, True + return conversation_id, not backing def _request_identity(user_id=None, seeded_agent=None): @@ -891,6 +948,50 @@ def _validate_turn_memory_context(turn_context, user_id, conversation_id): ) +_SHARED_TARGET_KEYS = ( + 'target_type', 'display_name', 'mention_text', 'source_mode', 'selection_key', 'agent_selection_key', +) + + +def _shared_turn_details(data): + """Who a shared conversation's orchestrated question addressed, bounded and display-only. + + Mentions are resolved against the conversation's participants when the question is + mirrored, so nothing here grants access or names anyone who is not already a member. + """ + details = {} + target = data.get('invocation_target') + if isinstance(target, dict): + details['invocation_target'] = { + key: _text(target.get(key), 200) for key in _SHARED_TARGET_KEYS if _text(target.get(key)) + } + mentions = data.get('mentioned_participants') + if isinstance(mentions, list): + details['mentioned_participants'] = [ + {key: _text(mention.get(key), 320) for key in ('user_id', 'display_name', 'email') if _text(mention.get(key))} + for mention in mentions[:50] if isinstance(mention, dict) + ] + reply_to = _text(data.get('reply_to_message_id'), 200) + if reply_to: + details['reply_to_message_id'] = reply_to + return {key: value for key, value in details.items() if value} + + +def _mirror_planned_turn(conversation_id, user_id, message_id, turn_context): + """Show a shared conversation's orchestrated question to everyone; the plan never waits on it.""" + try: + conversation = cosmos_conversations_container.read_item( + item=conversation_id, partition_key=conversation_id, + ) + if not is_orchestration_backing(conversation) or conversation.get('user_id') != user_id: + return + message = cosmos_messages_container.read_item(item=message_id, partition_key=conversation_id) + sender = normalize_collaboration_user(get_current_user_info() or {}) or {'user_id': user_id} + mirror_orchestration_turn(conversation, message, sender, details=turn_context.get('collaboration')) + except Exception as exc: + log_mirror_failure('question', exc) + + def _elicitation_outcome_events(outcome, turn_context, user_id, conversation_id): context = { **turn_context, @@ -939,6 +1040,7 @@ def _persist_planned_turn( ) turn_context['user_message_id'] = message_id turn_context['user_message_fingerprint'] = fingerprint + _mirror_planned_turn(conversation_id, user_id, message_id, turn_context) create_orchestration_run( plan, user_id, conversation_id=conversation_id, idempotent=True, turn_context=turn_context, expected_previous_run=expected_previous_run, @@ -1666,6 +1768,9 @@ def orchestration_plan(): 'replan_hint': replan_hint, 'planner_contract_version': DEPENDENCY_PLAN_CONTRACT_VERSION, } + shared_details = _shared_turn_details(data) + if shared_details: + turn_context['collaboration'] = shared_details # Workflow proposals schedule in the user's browser time zone, and a workflow results step # names the local day a run finished on, so it is validated here and kept with the turn. # Nothing about the turn changes while both are off. @@ -2226,6 +2331,8 @@ def generate(): level=logging.WARNING, extra={'reason': 'capability_context_failed', 'error_type': type(exc).__name__}, ) yield build_error_event(exc.message, resolved_conversation_id) + except SharedOrchestrationError as exc: + yield build_error_event(exc.message, conversation_id) except Exception as exc: log_event( '[ORCHESTRATION] Planning failed.', diff --git a/application/single_app/route_frontend_chats.py b/application/single_app/route_frontend_chats.py index c827d96c1..a3f5bd608 100644 --- a/application/single_app/route_frontend_chats.py +++ b/application/single_app/route_frontend_chats.py @@ -8,6 +8,7 @@ ) from content_screening.contracts import ScreeningError from config import * +from collaboration_models import is_shared_conversation_backing from functions_authentication import * from functions_content import * from functions_settings import * @@ -406,6 +407,14 @@ def _resolve_chat_upload_context(conversation_id, user_id, current_user_info): item=normalized_conversation_id, partition_key=normalized_conversation_id, ) + # Orchestrate's backing record shares its shared conversation's id; files shared in the + # conversation belong to its classic source conversation like any other. + if is_shared_conversation_backing(conversation_item): + return _resolve_collaboration_upload_context( + normalized_conversation_id, + user_id, + current_user_info, + ) except CosmosResourceNotFoundError: try: return _resolve_collaboration_upload_context( diff --git a/application/single_app/route_frontend_conversations.py b/application/single_app/route_frontend_conversations.py index 7a997ea78..a7768e815 100644 --- a/application/single_app/route_frontend_conversations.py +++ b/application/single_app/route_frontend_conversations.py @@ -7,6 +7,7 @@ from flask import Response, jsonify, redirect, render_template, request from config import * +from collaboration_models import is_shared_conversation_backing from functions_appinsights import log_event from functions_chat_content_checks import strip_private_chat_checks from functions_chat_content_review import refresh_checked_message @@ -234,6 +235,13 @@ def get_agent_citation_artifact(conversation_id, artifact_id): partition_key=conversation_id, ) except CosmosResourceNotFoundError: + conversation = None + # Orchestrate's backing record shares its shared conversation's id and is part of it: the + # shared conversation authorizes the reader, and the backing holds orchestrated answers' + # artifacts. + orchestration_backing = is_shared_conversation_backing(conversation) + + if conversation is None or orchestration_backing: try: conversation = get_collaboration_conversation(conversation_id) except CosmosResourceNotFoundError: @@ -269,6 +277,13 @@ def get_agent_citation_artifact(conversation_id, artifact_id): collaboration_messages = list_collaboration_messages(conversation_id) artifact_payload_map = build_message_artifact_payload_map(collaboration_messages) artifact_payload = artifact_payload_map.get(str(artifact_id or '')) + if artifact_payload is None and orchestration_backing: + backing_messages = list(cosmos_messages_container.query_items( + query="SELECT * FROM c WHERE c.conversation_id = @conversation_id", + parameters=[{'name': '@conversation_id', 'value': conversation_id}], + partition_key=conversation_id, + )) + artifact_payload = build_message_artifact_payload_map(backing_messages).get(str(artifact_id or '')) if not isinstance(artifact_payload, dict): return jsonify({'error': 'Agent citation artifact not found'}), 404 diff --git a/application/v2_ui/src/components/chat/Composer.tsx b/application/v2_ui/src/components/chat/Composer.tsx index 3435d1d73..f60bc420b 100644 --- a/application/v2_ui/src/components/chat/Composer.tsx +++ b/application/v2_ui/src/components/chat/Composer.tsx @@ -86,7 +86,13 @@ import { type TabularRunSettings, } from '../../lib/tabularRunEstimate'; import { LargeRunDialog } from './LargeRunDialog'; -import { resolveInvocationTarget, type MentionSuggestion } from '../../lib/mentions'; +import { + conversationParticipants, + extractMentionedParticipants, + resolveInvocationTarget, + sharedConversationTarget, + type MentionSuggestion, +} from '../../lib/mentions'; import { useUiStore } from '../../stores/uiStore'; import { toast } from '../../stores/toastStore'; import { chatWidthClass } from '../../lib/chatWidth'; @@ -236,6 +242,11 @@ export function Composer({ initialAgentSelection }: { initialAgentSelection?: st const canPost = !shared || collaboration?.can_post_messages === true; const awaitingInvite = Boolean(shared && collaboration?.can_accept_invite); const checkingAccess = shared && !collaboration; + // Plans in a shared conversation run for the person who started it (the server serializes + // that person as the conversation's `user_id`); everyone else asks the assistant directly. + const sharedOrchestrationAllowed = Boolean( + shared && collaboration && bootstrap?.user?.id && collaboration.user_id === bootstrap.user.id, + ); const replyTo = useCollaborationStore((state) => state.replyTo); const setReplyTo = useCollaborationStore((state) => state.setReplyTo); @@ -1029,11 +1040,24 @@ export function Composer({ initialAgentSelection }: { initialAgentSelection?: st // Orchestration takes a different road entirely: the server plans the work rather than // running a chat stream, so the large-run confirmation — a manual-flow concern about a // tabular export the planner has not chosen — does not apply. + // + // A shared conversation applies its own rule first, exactly as manual mode does: a + // message that only addresses people is posted to them and never planned. Only the person + // who started the conversation plans in it, so anyone else's request to the assistant is + // answered the classic way. + const sharedRequest = orchestrating && shared ? sharedOrchestrationRequest(outgoing.message) : null; + if (orchestrating && shared && !sharedRequest) { + dispatch(outgoing); + return; + } if (orchestrating) { // Asked inside the click or key press that sent the message, because that is the // only moment a browser shows its prompt. Does nothing once it has been answered. void requestDesktopNotificationPermission(); - dispatchOrchestration(outgoing.message, outgoing.promptInfo); + dispatchOrchestration(outgoing.message, outgoing.promptInfo, sharedRequest); + if (sharedRequest) { + setReplyTo(null); + } return; } @@ -1181,7 +1205,35 @@ export function Composer({ initialAgentSelection }: { initialAgentSelection?: st return seeds; }; - const dispatchOrchestration = (message: string, promptInfo: Json | null = null) => { + /** + * Who a shared conversation's planned question addresses, or null when it isn't planned. + * + * Null when the message addresses only people, or when the reader didn't start the + * conversation. The plan request carries the target, mentions and reply so the server + * can post the question to the shared thread as it was asked. + */ + const sharedOrchestrationRequest = (message: string): Record | null => { + const target = sharedConversationTarget(message, options, { + agents: bootstrap?.catalogs?.agents as AgentOption[] | undefined, + models: bootstrap?.catalogs?.models as ModelCatalogEntry[] | undefined, + }); + if (!target || !sharedOrchestrationAllowed) { + return null; + } + return { + invocation_target: target, + mentioned_participants: extractMentionedParticipants( + message, conversationParticipants(collaboration), + ), + ...(replyTo?.message_id ? { reply_to_message_id: replyTo.message_id } : {}), + }; + }; + + const dispatchOrchestration = ( + message: string, + promptInfo: Json | null = null, + sharedDetails: Record | null = null, + ) => { if (attachedPrompt && promptInfo) { rememberPromptValues(attachedPrompt.id, composerDraftUserPromptValues(draft)); } @@ -1194,7 +1246,9 @@ export function Composer({ initialAgentSelection }: { initialAgentSelection?: st conversationId, message, approvalMode: effectiveApprovalMode, - seeds: buildOrchestrationSeeds(message, promptInfo), + // Who a shared conversation's question addressed travels with the plan request, so + // the server can post the question to the shared thread as it was asked. + seeds: { ...buildOrchestrationSeeds(message, promptInfo), ...(sharedDetails ?? {}) }, }); clearDraft(); stopTyping(); diff --git a/application/v2_ui/src/lib/mentions.ts b/application/v2_ui/src/lib/mentions.ts index 9c6eeebf9..4c9c842aa 100644 --- a/application/v2_ui/src/lib/mentions.ts +++ b/application/v2_ui/src/lib/mentions.ts @@ -516,6 +516,36 @@ export function shouldInvokeAi( return resolveSendTarget(message, options, catalogs) !== null; } +/** + * The AI target of a shared conversation message, from the composer's own options. + * + * Manual sends and Orchestrate both decide with this, so a message that only addresses people is + * posted to them in either mode and never reaches a model or a plan. While a saved analysis is + * being asked about, its source toggles do not address the assistant: that answer comes from the + * stored result. + */ +export function sharedConversationTarget( + message: string, + options: InvocationOptions, + catalogs: { agents?: AgentOption[]; models?: ModelCatalogEntry[] }, + { savedContext = false }: { savedContext?: boolean } = {}, +): InvocationTarget | null { + return resolveSendTarget( + message, + { + agentSelection: options.agentSelection, + promptId: options.promptId, + documentSearch: !savedContext && options.documentSearch, + webSearch: !savedContext && options.webSearch, + imageGeneration: !savedContext && options.imageGeneration, + deepResearch: !savedContext && options.deepResearch, + urlAccess: !savedContext && options.urlAccess, + modelDeployment: options.modelDeployment, + }, + catalogs, + ); +} + /* -------------------------------------------------------------------------- */ /* The mention menu */ /* -------------------------------------------------------------------------- */ diff --git a/application/v2_ui/src/stores/chatStore.ts b/application/v2_ui/src/stores/chatStore.ts index e80277a80..7660186fc 100644 --- a/application/v2_ui/src/stores/chatStore.ts +++ b/application/v2_ui/src/stores/chatStore.ts @@ -70,7 +70,7 @@ import { conversationParticipants, extractMentionedParticipants, mentionsCurrentUser, - resolveSendTarget, + sharedConversationTarget, } from '../lib/mentions'; import { buildSelectionFields } from '../lib/chatRequestSelection'; import { @@ -974,6 +974,12 @@ function stopCollaborationEvents(): void { } } +/** The message a shared-thread copy was mirrored from, or '' when it is not a copy. */ +function mirroredSourceId(message: ChatMessage | CollaborationMessage): string { + const metadata = message.metadata as { source_message_id?: unknown } | undefined; + return typeof metadata?.source_message_id === 'string' ? metadata.source_message_id.trim() : ''; +} + /** * Merge a message that arrived from the server into the thread. * @@ -986,7 +992,7 @@ function stopCollaborationEvents(): void { * matched on the pending id when it is known, and otherwise on the reader's own unsent * text, which is how a message posted from another tab still lands in one place. */ -function mergeCollaborationMessage( +export function mergeCollaborationMessage( messages: ChatMessage[], incoming: CollaborationMessage, options: { pendingId?: string | null; currentUserId?: string } = {}, @@ -1004,6 +1010,26 @@ function mergeCollaborationMessage( return merged; } + // A shared conversation's orchestrated question and answer are mirrored into its thread + // from the run's own messages, so the shared copy and the run's live bubble name each other + // through `metadata.source_message_id`. Either can arrive first; the second updates the first. + const incomingSourceId = mirroredSourceId(incoming); + const mirroredIndex = messages.findIndex((message) => + (incomingSourceId && message.id === incomingSourceId) + || (Boolean(incoming.id) && mirroredSourceId(message) === incoming.id), + ); + if (mirroredIndex !== -1) { + if ( + isReplyRemoved(messages[mirroredIndex].metadata) + && !isReplyRemoved(incoming.metadata) + ) return messages; + const merged = [...messages]; + merged[mirroredIndex] = isReplyRemoved(incoming.metadata) + ? { ...incoming, thoughts: undefined } + : { ...merged[mirroredIndex], ...incoming }; + return merged; + } + const senderId = String(incoming.sender?.user_id ?? '').trim(); const isOwnMessage = Boolean( options.currentUserId && senderId && senderId === options.currentUserId, @@ -2889,25 +2915,18 @@ export const useChatStore = create((set, get) => ({ const replyTo = useCollaborationStore.getState().replyTo; // In a shared conversation most messages are people talking to each other, so the - // AI is only brought in when something asks for it. `resolveSendTarget` applies the - // classic client's rule; null means this message is for the participants alone. + // AI is only brought in when something asks for it. `sharedConversationTarget` applies + // the classic client's rule, the same one Orchestrate applies; null means this message + // is for the participants alone. const invocationTarget = collaborative - ? resolveSendTarget( + ? sharedConversationTarget( trimmed, - { - agentSelection: options.agentSelection, - promptId: options.promptId, - documentSearch: !savedContext && options.documentSearch, - webSearch: !savedContext && options.webSearch, - imageGeneration: !savedContext && options.imageGeneration, - deepResearch: !savedContext && options.deepResearch, - urlAccess: !savedContext && options.urlAccess, - modelDeployment: options.modelDeployment, - }, + options, { agents: bootstrap?.catalogs?.agents as AgentOption[] | undefined, models: bootstrap?.catalogs?.models as ModelCatalogEntry[] | undefined, }, + { savedContext: Boolean(savedContext) }, ) ?? (savedContext ? { target_type: 'model' as const, display_name: 'Model', diff --git a/docs/admin/orchestration.md b/docs/admin/orchestration.md index e4cf68541..53e57e122 100644 --- a/docs/admin/orchestration.md +++ b/docs/admin/orchestration.md @@ -133,6 +133,17 @@ restrictions, capability settings, and agent or action governance. A role change Entra ID takes effect when the user's session is refreshed, for example at their next sign-in, as it does for classic chat. +Since **0.261.269**, agent and action steps trust the signed-in session the way manual +chat does. When the step runs, orchestration checks the user's current access to the +conversation, the run and that exact agent or action, and then runs it as chat would. +It no longer captures and compares the agent's or action's configuration. Editing an +agent or action therefore doesn't invalidate a result it already produced, and a result +stays readable until the user loses access to that agent or action. Web search, +linked-page reading and deep research keep their configuration checks. Earlier versions +compared configuration for agents and actions too. Every Microsoft 365 action step then +failed with "A required retained result is unavailable or changed", and local agents +couldn't run as plan steps. + Work that continues in the background, such as a run recovered after a restart, has no signed-in session. Its steps can't use web search, linked pages, deep research, agents or actions, and they ask the user to send the request again. Document-backed @@ -362,6 +373,12 @@ knowledge or broader procedure. A user-selected agent is not silently replaced w actions, and the planner should not send the same work through both paths. **Call agent** actions remain on the existing agent path and are excluded from direct action selection. +Since **0.261.269**, tagging an agent with @ in an Orchestrate message chooses that agent +just as the agent picker does, and tagging a model with @ chooses that model. A chosen +agent is always offered to the planner, even when the user turned agents off in the +classic interface's settings. Local agents run as plan steps, including agents that use +Microsoft 365 actions. + The opt-in adds no second action allowlist or approval system. Existing scope, ownership, group membership, enablement and governance rules still determine which actions are available, and access is checked again when work runs. An action removed or revoked @@ -390,7 +407,10 @@ and cancellation. No composer action picker is added. Since **0.261.238**, a **Use an action** step can run a Microsoft 365 Calendar, Email, OneDrive, SharePoint Online or legacy Microsoft Graph action. Earlier versions refused every Microsoft 365 call in a plan with `m365_context_required`, before reaching Microsoft Graph, -and still reported the step as completed. +and still reported the step as completed. From 0.261.238 until **0.261.269**, these steps +failed with "A required retained result is unavailable or changed", because the step's +configuration check couldn't see the action's Microsoft 365 selection. See +[Retained external-source authorization](#retained-external-source-authorization). Each such step gets its own Microsoft 365 request, the same kind chat creates, with these limits: @@ -405,8 +425,12 @@ limits: - **Read only.** Send mail, calendar invitations and mark-as-read are removed from a plan step, even when the action enables them. An action that enables only those functions stops with "Plans can only read Microsoft 365 data". -- **Personal conversations only.** In a shared conversation the step stops before reading - anything. Source-sharing approvals resume chat requests, not plan steps. +- **In a shared conversation, asking is consent.** Since **0.261.269**, a step in a shared + conversation reads for that conversation's participants. The user's own request counts + as consent to share what the step reads with them, so no source-sharing approval waits. + Each request and source is recorded as a `shared_by_request` audit event. Workflow Run + as approvals, and approvals to share earlier answers' Microsoft 365 history, still ask. + Earlier versions stopped the step before it read anything. - **Approvals don't resume plans.** A step that needs an approval, such as deeper file analysis, stops and links to Approvals. The user decides there and then selects **Retry from failed step**. A retried step reuses its request, so the decision applies while @@ -416,8 +440,52 @@ A refused Microsoft 365 call stops the step instead of becoming findings that th reports as data. Ordinary Microsoft Graph outcomes, such as nothing found or throttling, are still findings. An answer that used Microsoft 365 keeps the tool calls in its sources, so sharing the conversation later asks the user to approve Microsoft 365 history, as it -does for a chat answer. An **Ask an agent** step doesn't make its agent's Microsoft 365 -actions available. +does for a chat answer. Since **0.261.269**, an **Ask an agent** step whose agent uses +Microsoft 365 actions gets a step request for that agent's own Microsoft 365 actions, with +the same limits. An agent without Microsoft 365 actions gets none. + +### Shared conversations + +Since **0.261.269**, Orchestrate applies the same rule in a shared conversation that a +manual send does: + +- A message that addresses only people, such as "@Ada can you check this?", is posted to + them. It never reaches a model or a plan. +- A message that addresses the assistant is planned. It addresses the assistant when it + tags an agent or model with @, or uses the agent picker, a saved prompt or a source + toggle. +- Only the person who started the shared conversation plans in it. When another + participant asks the assistant with Orchestrate on, they get a classic answer, the same + as with Orchestrate off. +- The question and the run's final answer are posted to the shared thread, replying to + each other, and every participant sees them arrive. + +Plans run in a hidden conversation that has the shared conversation's ID and workspace +lock and belongs to the person who started it. Memory and source rules treat it as shared, +so plans there never propose, run or read personal workflows. Anything that looks up the +shared conversation by ID, such as reopening it, images, exports or uploads, still gets +the shared conversation. Deleting the shared conversation, by its owner or by retention, +deletes the plans too. A co-owner who didn't start the conversation removes it without +deleting its creator's plans. + +The question and answer are posted only while the person who started the conversation is +still a participant, and while a group conversation still allows chat. When a run +finishes later than its first answer, for example after a waiting step, the shared copy +of the answer is updated in place. + +Current limits: + +- Other participants see the question and the answer, not the plan card or run details. +- Files, images and charts that a plan creates stay with the person who started the + conversation. The shared copy of the answer carries its text and citations. +- The planner reads earlier planned turns of the conversation, not messages that + participants exchanged or that the classic assistant answered. + +Before 0.261.269, Orchestrate planned every message in a shared conversation, including +ones that addressed only people, and kept the question and answer in a private copy that +other participants never saw. Those private copies no longer appear in conversation lists. +The copy that belongs to the person who started the conversation becomes its hidden plan +conversation, and its earlier turns aren't posted to the shared thread. ### Charts, diagrams, and images in answers @@ -634,14 +702,18 @@ then report a rejected proposal. | Every web search, linked-page or deep research step says a required retained result is unavailable or changed | Before 0.261.209, orchestration checked each user's roles through Microsoft Graph, which needs `Directory.Read.All`, and refused research whenever query or linked-page planning was on. | Upgrade to 0.261.209 or later; no Graph permission or settings change is needed. If a step still fails, check `sc_authority_reason` on its failure event in [orchestration failure diagnostics](../reference/logging-tags.md#orchestration-failure-diagnostics). | | Every step completed, but the run is partially completed with "A required retained result is unavailable or changed" | Before 0.261.237, settings read from Cosmos DB instead of the shared Redis copy came back as an SDK type that the access recheck refused. That happens while any settings save is in progress, for example the Cosmos throughput autoscale saving its status, and on every read when Redis is off. The failure event logs `sc_authority_reason=result_external_context_unavailable`. | Upgrade to 0.261.237 or later. No setting change is needed. See the [settings document type fix]({{ '/explanation/fixes/ORCHESTRATION_SETTINGS_DOCUMENT_TYPE_FIX/' | relative_url }}). | | Plans never propose deep research or reading a link | The user does not hold the required app role, or the capability is disabled in its own settings group. | Confirm the user holds `DeepResearchUser` or `UrlAccessUser` where your deployment requires them, and that the capability is enabled outside this page. | -| Plans never propose an agent | Semantic Kernel is off, the user has turned agents off in their own settings, or the user has no agent they can reach. | Confirm Semantic Kernel is enabled, then check the user's own agent setting and that at least one agent is shared with them. | +| Plans never propose an agent | Semantic Kernel is off, the user has turned agents off in their own settings, or the user has no agent they can reach. | Confirm Semantic Kernel is enabled, then check the user's own agent setting and that at least one agent is shared with them. An agent the user picks, or tags with @, is always offered to the planner. | | The planner model shows "not in the current model list" | Its connection or model was removed or disabled, or connections were switched on or off since it was chosen. | Choose a listed model, or **Use the answer model (default)**. Until then planning keeps trying the saved model and fails rather than switching. | | Plans never propose Use an action | Action Access is off, Semantic Kernel is off, the capability is excluded, or no eligible action is available to this user. | Check the opt-in and capability selection, then the existing action scope and governance. Call agent actions are not eligible for direct use. | | An action step fails after plan approval | The action or its access changed, or its model/tool connection could not run. | Check current action access and configuration. Review the visible step failure; the run does not silently switch to an agent or another action. | | Before 0.261.238, a Microsoft 365 action step showed completed, but the answer said it couldn't read mail, calendar or files | The plan step had no Microsoft 365 request, so every call was refused with `m365_context_required` before reaching Microsoft Graph. `[MS_GRAPH_PLUGIN]` failure events show it as `sc_error_code_length` 21. | Upgrade to 0.261.238 or later. See [Microsoft 365 actions in plans](#microsoft-365-actions-in-plans). | | A Microsoft 365 step says Microsoft 365 needs the user to sign in or grant access | The user hasn't connected that source for chat, or their sign-in expired or Microsoft Graph rejected it. | The user selects **Connect Microsoft 365** in the run details, or connects in Profile, then **Retry from failed step**. `[ORCHESTRATION_M365] A Microsoft 365 step stopped.` logs the reason as `sc_authority_reason`. | | A Microsoft 365 step says plans can only read Microsoft 365 data | The action enables only send, invitation or mark-as-read functions, which plans never run. | Enable a read function on the action, or use it from chat without a plan. | -| A Microsoft 365 step says plans can't use Microsoft 365 in a shared conversation | Plans don't run Microsoft 365 actions in shared conversations. | Ask from the user's own conversation. | +| Before 0.261.269, a Microsoft 365 step said plans can't use Microsoft 365 in a shared conversation | Earlier versions refused Microsoft 365 steps in shared conversations. | Upgrade to 0.261.269 or later. The user's own request now counts as consent to share what the step reads. See [Microsoft 365 actions in plans](#microsoft-365-actions-in-plans). | +| Before 0.261.269, every action or agent step failed with "A required retained result is unavailable or changed", while the same request worked with Orchestrate off | The step's configuration check refused it. For Microsoft 365 actions, the check couldn't see the action's Microsoft 365 selection. Local agents were refused outright. Failure events log `sc_authority_reason=external_configuration_unavailable` for `action_invoke` or `agent_invoke`. | Upgrade to 0.261.269 or later. Agent and action steps now trust the signed-in session, as manual chat does. See [Retained external-source authorization](#retained-external-source-authorization). | +| Before 0.261.269, an @mention of a person in a shared conversation was sent to the model when Orchestrate was on | Orchestrate planned every message without applying the shared conversation's send rule. | Upgrade to 0.261.269 or later. See [Shared conversations](#shared-conversations). | +| A participant sees "Only the person who started this shared conversation can use Orchestrate here" | A request reached the planner from someone other than the person who started the shared conversation. The V2 composer normally answers their requests the classic way instead. | Turn off Orchestrate for that request, or ask the person who started the conversation to ask it. | +| The person who started a shared conversation sees "Orchestrate can't be used in this shared conversation because an earlier version kept another participant's private copy of it" | Before 0.261.269, Orchestrate saved a private copy for whoever used it first, under the shared conversation's ID. Only one record can have that ID. | Turn off Orchestrate to ask the assistant. To restore Orchestrate, remove that participant's record from the conversations container: its ID is the shared conversation's ID. The `[ORCHESTRATION]` warning logs `sc_reason=shared_conversation_stale_copy`. | | A plan proposed reading a link but found nothing | The link was not available in the eligible user-authored context. | Paste the URL into the current request. Assistant-generated links and omitted historical text do not authorize page reads. | | Earlier runs are missing after switching devices | The conversation list has loaded but its run history has not been fetched yet, or the fetch failed. | The orchestration panel shows its own loading and retry states. If retrying keeps failing, check that the user can reach `/api/v2/orchestration/runs` and is the owner of the conversation. | | A restored plan will not run | It was already approved on the other device. | This is expected. The conversation reloads to show the answer that run produced. | diff --git a/docs/explanation/fixes/ORCHESTRATION_M365_ACTION_CONTEXT_FIX.md b/docs/explanation/fixes/ORCHESTRATION_M365_ACTION_CONTEXT_FIX.md index ccb6b1011..e2ad4aa25 100644 --- a/docs/explanation/fixes/ORCHESTRATION_M365_ACTION_CONTEXT_FIX.md +++ b/docs/explanation/fixes/ORCHESTRATION_M365_ACTION_CONTEXT_FIX.md @@ -197,6 +197,12 @@ The V2 UI type-checks and builds with `npm run typecheck` and `npm run build`. ## Limitations +Since **0.261.269**, shared conversations and agents' Microsoft 365 actions are +supported. See the +[session-trusted action and agent steps fix](ORCHESTRATION_SESSION_TRUSTED_ACTION_AGENT_STEPS_FIX.md) +and the [shared conversations fix](ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md). The +limits below describe 0.261.238. + - Plans only read Microsoft 365 data. Sending mail or invitations from a plan needs pending-action review designed for plan steps. - Shared conversations are refused. Plan steps would need their own diff --git a/docs/explanation/fixes/ORCHESTRATION_SESSION_TRUSTED_ACTION_AGENT_STEPS_FIX.md b/docs/explanation/fixes/ORCHESTRATION_SESSION_TRUSTED_ACTION_AGENT_STEPS_FIX.md new file mode 100644 index 000000000..d7eb4be86 --- /dev/null +++ b/docs/explanation/fixes/ORCHESTRATION_SESSION_TRUSTED_ACTION_AGENT_STEPS_FIX.md @@ -0,0 +1,154 @@ +# Orchestration Session-Trusted Action and Agent Steps Fix + +**Version: 0.261.269** + +Fixed in version: **0.261.269**, recorded in +`application/single_app/config.py`. + +Fixes [#1660](https://github.com/microsoft/simplechat/issues/1660) and +[#1661](https://github.com/microsoft/simplechat/issues/1661). This affects the React +V2 branch (`paullizer-react-v2-ui`) and deployments built from it. + +## Issue + +With **Orchestrate** on, "what are my emails" failed in both personal and shared +chats. The plan's **Use an action** step for the user's Microsoft 365 Email action +failed with "A required retained result is unavailable or changed." The same +request worked with Orchestrate off. + +Plans also couldn't use local agents. Picking an agent and asking it to read mail +produced a plan whose **Ask an agent** step failed the same way. + +Production telemetry showed every affected step failing with +`authority_reason=external_configuration_unavailable` for `action_invoke` and +`agent_invoke`. + +## Root cause + +Before 0.261.269, agent and action steps were held to configuration attestation, the +check that web search, linked-page reading and deep research use. A step captured +the agent's or action's configuration when it ran, and the result was admitted and +read back only while a fresh read of that configuration matched the capture. + +That check couldn't work for these steps: + +1. **Microsoft 365 actions.** `invoke_action` enters the step's Microsoft 365 scope + (0.261.238) before the engine runs. The attestor's fresh read called + `prepare_action_plugin_manifest`, whose Microsoft 365 preflight rebuilt the + manifest as a plain dictionary and dropped its `ScopedActionManifest` origin. + The origin check then refused it with `external_configuration_action_origin_required`, + reported as `external_configuration_unavailable`. Results retained before an + action edit would also have read as "changed". +2. **Local agents.** `_preflight_captured_agent` refused local agents and agents + with knowledge or web sources while a capture was active, and the Semantic Kernel + loader refused most plugins under capture. +3. **Agent Microsoft 365 actions.** An agent step had no Microsoft 365 request, so + an agent's Microsoft 365 actions were refused with `m365_context_required`. +4. **Agent tags.** In Orchestrate mode an explicit @agent tag in the message didn't + choose that agent. Planning only offered agents when one was picked, or when the + user's classic agents switch was on, which V2 users can't change. + +## Changes + +### Agent and action steps trust the signed-in session + +Agent and action steps now trust the signed-in session the way manual chat does. +Web search, linked-page reading and deep research keep configuration attestation. + +- `OrchestrationExternalSourceProvider` (`functions_orchestration_external_sources.py`) + treats agent and action sources as session-trusted. A new reference gets a stable + identity and revision from its scope and ID, without calling the configuration + reader. Each later read rechecks current access to that exact agent or action, and + results admitted under attestation by earlier versions stay readable. + `preflight_session_acquisition()` checks current access to the exact selection + before the step runs. +- `build_orchestration_services` (`functions_orchestration_bootstrap.py`) routes + agent and action captures to that preflight and records the selector the step + checked. Admission of an agent or action result requires that record, so the + root still refuses a result without a session-checked acquisition. +- `run_action_invoke` and `run_agent_invoke` (`functions_orchestration_adapters.py`) + check access once and then run the classic engines without an invocation capture. + +### Agents in plans + +- An agent step whose agent loads actions enters a Microsoft 365 step scope for + that agent's own Microsoft 365 actions (`agent_step_scope()` in + `functions_orchestration_m365.py`). `step_selected_m365_manifests()` resolves a + selection of kind `agent` the way a chat-selected agent's actions resolve, and an + agent without Microsoft 365 actions gets no context and leaves no request record. + `execute_target()` and `invoke_scoped_agent()` in `agent_delegation_runtime.py` + accept an optional `scope`, entered inside the agent's bridge. +- The V2 composer (`Composer.tsx`) seeds the plan with an explicit @agent tag as its + agent, or an @model tag as its model, ahead of the composer's pickers. A seeded + agent is always offered to the planner, and the planner asks it rather than using + its actions directly. + +### Diagnostics + +- `_read_metadata` (`functions_orchestration_external_configuration.py`) keeps a + specific refusal code instead of reporting every refusal as + `external_configuration_unavailable`. +- A refusal from an integration now fails with the new `integration_unavailable` + failure, whose message names the integration instead of retained results. + +## Files modified + +| File | Change | +| --- | --- | +| `functions_orchestration_external_sources.py` | Session-trusted references and `preflight_session_acquisition()`. | +| `functions_orchestration_bootstrap.py` | Session preflight for agent and action captures, and admission from its record. | +| `functions_orchestration_adapters.py` | Access check, then the classic engines; the agent Microsoft 365 scope. | +| `functions_orchestration_m365.py` | `agent_loads_actions()` and `agent_step_scope()`. | +| `functions_m365_runtime.py` | Agent selections in `step_selected_m365_manifests()` and `step_m365_context()`. | +| `agent_delegation_runtime.py` | Optional `scope` for `execute_target()` and `invoke_scoped_agent()`. | +| `functions_orchestration_external_configuration.py` | Specific refusal codes. | +| `functions_orchestration_schema.py`, `functions_orchestration_executor.py` | `integration_unavailable` failure. | +| `v2_ui/src/components/chat/Composer.tsx` | @agent and @model tags seed the plan. | +| `config.py` | Version `0.261.269`. | + +## Validation + +### Tests + +- `functional_tests/test_orchestration_session_trusted_sources.py` runs the real + provider for agents and actions: admission rechecks access without reading + configuration, results survive configuration edits until access is revoked, + results admitted under attestation stay readable, and the session preflight needs + current access to the exact selection. All nine tests fail on 0.261.264. +- `functional_tests/test_orchestration_external_bootstrap.py` runs the real root: + an action step trusts the session without rereading configuration, and admission + without its session-checked acquisition is refused. +- `functional_tests/test_orchestration_m365_actions.py` adds agent step scopes: an + agent's own Microsoft 365 action is the only one selected, an agent without one + gets no context, and a removed agent stops before any Microsoft 365 work. +- `functional_tests/test_orchestration_external_configuration_capture.py` and + `test_orchestration_external_capture_integration.py` were updated deliberately: + their agent and action cases asserted attestation, and now assert one session + access check followed by the classic engine. Their web, URL and deep research + cases are unchanged. +- `functional_tests/test_v2_shared_orchestration_routing.mjs` checks the composer's + tag seeding. + +### Before and after + +| Before | After | +| --- | --- | +| Every Microsoft 365 action step failed with "A required retained result is unavailable or changed." | The step reads mail, calendar or files, as manual chat does. | +| Local agents couldn't run as plan steps. | Local agents run, including their Microsoft 365 actions. | +| Editing an action invalidated results it had already produced. | Results stay readable until access to the action or agent is lost. | +| An @agent tag didn't choose the agent in Orchestrate mode. | An @agent or @model tag seeds the plan. | + +## Limitations + +- This relaxes a check, deliberately. Access to the conversation, run, agent and + action is still checked on every read, but a mid-run edit to an agent or action is + no longer detected. A lighter check may follow. +- Without a seed, planning still offers agents only when the user's classic agents + switch is on. + +## Related + +- [Retained external-source authorization](../../admin/orchestration.md#retained-external-source-authorization) +- [Microsoft 365 actions in plans](../../admin/orchestration.md#microsoft-365-actions-in-plans) +- [Orchestration Microsoft 365 action context fix](ORCHESTRATION_M365_ACTION_CONTEXT_FIX.md) +- [Orchestration shared conversations fix](ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md) diff --git a/docs/explanation/fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md b/docs/explanation/fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md new file mode 100644 index 000000000..a73a14a7f --- /dev/null +++ b/docs/explanation/fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md @@ -0,0 +1,204 @@ +# Orchestration Shared Conversations Fix + +**Version: 0.261.269** + +Fixed in version: **0.261.269**, recorded in +`application/single_app/config.py`. + +Fixes [#1659](https://github.com/microsoft/simplechat/issues/1659). This affects the +React V2 branch (`paullizer-react-v2-ui`) and deployments built from it. + +## Issue + +In a shared conversation with **Orchestrate** on, "@person hey" was sent to the model, +which planned, reasoned and answered. With Orchestrate off, the same message was +posted to that person, as intended. + +Orchestrated questions and answers also never reached the shared thread, so other +participants didn't see them. Microsoft 365 requests in shared conversations waited +for a sharing approval in chat and were refused outright in plans. + +## Root cause + +1. **No shared send rule.** `Composer.submit` sent every Orchestrate message to + `startOrchestrationPlan`. Manual sends apply the shared conversation rule in + `chatStore.sendMessage`, which posts a message that addresses only people. +2. **A private copy.** The plan route's `_ensure_conversation` didn't know about + shared conversations. For a shared conversation's ID it created a personal + conversation with that ID, owned by the sender, so each plan and answer lived in a + private copy that other participants never saw. +3. **Microsoft 365 consent.** Shared conversations needed a source-sharing approval + before Microsoft 365 data was read, and `step_m365_context` refused plan steps in + shared conversations, because approvals resume chat requests, not plan steps. + +## Changes + +### Composer routing + +`Composer.submit` applies `sharedConversationTarget()` (new in `lib/mentions.ts`), +the rule `chatStore.sendMessage` now also uses. A message that addresses only people +is posted through the normal shared send, with its mentions and reply. A message +that addresses the assistant is planned only for the person who started the +conversation. Anyone else's request is answered the classic way. The plan request +carries the AI target, mentions and reply, so the question can be posted as asked. + +`mergeCollaborationMessage` (`stores/chatStore.ts`) matches a shared copy and the +run's own message through `metadata.source_message_id`, in either order, so neither +appears twice. + +### A hidden backing conversation + +`_ensure_conversation` (`route_backend_orchestration.py`) now resolves a shared +conversation's ID to a hidden backing conversation with the same ID. It belongs to the +person who started the shared conversation and has the fields of the classic +assistant's hidden source conversation: `conversation_kind='collaboration_source'`, +`collaboration_conversation_id`, `is_hidden`, and the shared conversation's workspace +lock. Keeping the ID keeps every plan, run and result where the V2 client already +looks for them, and memory rules treat the conversation as shared. + +- Only the person who started the conversation can plan there. Another participant + gets "Only the person who started this shared conversation can use Orchestrate + here." Anyone who isn't a participant gets the same answer as for a conversation + that doesn't exist. +- A private copy that an earlier version created for that person becomes the backing + conversation. Another person's private copy is never adopted. The person who started + the conversation is told that Orchestrate can't be used there and why. +- A backing conversation isn't announced as a new conversation, because the browser + already holds its ID. +- The conversation list treats any personal record with a shared conversation's ID + as part of that shared conversation, so earlier private copies no longer appear. +- Deleting the shared conversation, by its owner or by retention, deletes the backing + first (`delete_orchestration_backing`). Its runs are fenced and their files enrolled + for output cleanup, and its messages, thoughts and record are archived or deleted, as + for a personal conversation. A co-owner who didn't start the conversation removes it + without deleting its creator's plans, as with the classic source conversation. + +### Lookups by a shared conversation's ID + +Because the backing has the shared conversation's ID, every lookup that reads personal +storage first now treats it as part of the shared conversation, using +`is_shared_conversation_backing()` in `collaboration_models.py`. Without this, the +backing would have taken over the shared conversation for anyone looking it up by ID: + +- `/api/conversations//kind` reports the shared conversation, so reloading the page + or following a link never reopens it as a personal chat. It also does so for an + earlier private copy, for anyone who can still see the shared conversation. +- Personal routes treat the backing as absent (`_authorize_personal_conversation_read`). + Images, summaries, exports, scope lock, agent citation details, MCP tools and posts + through SimpleChat operations then use the shared conversation, for every participant. +- `/api/chat/stream` never writes into the backing, and chat uploads go to the shared + conversation's classic source conversation. +- Microsoft 365 action cards and audit records keep resolving to the classic source + conversation (`resolve_m365_audit_conversation_id`). + +### Posting to the shared thread + +`functions_orchestration_collaboration.py` (new) posts the question when the plan is +saved and the final answer when the run finishes, through the same storage helpers +as the classic shared assistant. Each is posted once, keyed by the run message's ID. +The answer replies to its question, and each post publishes +`collaboration.message.created` and the participants' notifications. A failure to +post is logged and never changes the run's own answer. + +- Each post rechecks that the backing's owner is still a participant who started the + conversation, and that a group conversation still allows chat. If not, nothing is + posted. +- A run republishes its answer under the same ID when it finishes later, for example + after a waiting step completes or the run times out. The shared copy is then updated + in place and `collaboration.message.updated` is published, so participants never keep + the earlier text. + +### Microsoft 365: your request is your consent + +`M365ExecutionContext` has a new `shared_by_request` flag. Chat requests and plan steps +in a shared conversation set it, because the user is asking for their own data. It +can't be set for a workflow, and it isn't part of the approval fingerprint. A reviewed +email or invitation keeps the flag with its delivery (`functions_m365_pending_delivery.py`), +so sending it needs no approval its request didn't need. + +`M365ApprovalService.authorize_sources` grants such a request without a pending +approval or notification and records one `shared_by_request` audit event for each +request and source. The grant has the shape of a request-only approval, so publishing +file evidence the request captured references that audit event. History publication +and workflow Run as approvals are unchanged. `step_m365_context` no longer refuses +shared conversations and uses the shared conversation's real audience. + +## Files modified + +| File | Change | +| --- | --- | +| `functions_orchestration_collaboration.py` | New. Backing fields, owner-only authorization, posting and refreshing the question and answer, and deleting the backing. | +| `route_backend_orchestration.py` | Backing conversation in `_ensure_conversation`, bounded shared details, posting the question. | +| `functions_orchestration_execution.py` | Posts the final answer. | +| `collaboration_models.py` | `is_shared_conversation_backing()`. | +| `route_backend_conversations.py` | Earlier private copies and backing records stay out of the conversation list and hidden count; kind, personal routes, summary and scope lock use the shared conversation. | +| `route_backend_conversation_export.py`, `functions_conversation_metadata.py`, `functions_mcp_server_tools.py`, `functions_simplechat_operations.py`, `route_frontend_conversations.py`, `route_frontend_chats.py`, `route_backend_chats.py` | Lookups by a shared conversation's ID use the shared conversation. | +| `functions_collaboration.py` | Deleting a shared conversation deletes its backing. | +| `functions_orchestration_artifacts.py` | `is_retained_orchestration_file()`, shared by both deletion paths. | +| `functions_m365_context.py`, `functions_m365_approvals.py`, `functions_m365_runtime.py`, `functions_m365_pending_delivery.py` | Consent by request. | +| `v2_ui/src/lib/mentions.ts`, `v2_ui/src/stores/chatStore.ts`, `v2_ui/src/components/chat/Composer.tsx` | The shared send rule, routing, and de-duplication. | +| `config.py` | Version `0.261.269`. | + +## Validation + +### Tests + +- `functional_tests/test_orchestration_shared_conversations.py` drives the real + orchestration routes over HTTP with the real collaboration storage functions. It + checks the backing conversation and its lock, that no new conversation is + announced, the posted question with its mentions and AI target, the answer posted + as a reply, the live events, and that posting is idempotent. Another participant + is asked to turn off Orchestrate, a non-participant learns nothing, the starter's + earlier private copy is adopted, and another person's is not. A republished answer + updates its shared copy, nothing is posted after the starter leaves, and deleting + the shared conversation deletes the backing and fences its runs. +- `functional_tests/test_orchestration_shared_backing_lookups.py` runs the real + lookups with a backing present: kind, personal routes and images, scope lock, + metadata, exports, MCP tools, the chat stream, uploads and Microsoft 365 cards. + Sixteen of these checks fail without the lookup changes. +- `functional_tests/test_m365_shared_request_consent.py` runs the real approval + service: a shared request is its own consent with one audit event per request and + source, without the flag a shared request still needs an approval, private + conversations are unchanged, only interactive shared requests can carry the flag, + and it doesn't change the approval fingerprint. A reviewed delivery keeps the + consent, and one recorded before 0.261.269 still asks. +- `functional_tests/test_m365_provider_core.py` publishes file evidence captured by + a shared request without an approval, end to end. With the earlier grant shape this + failed with `KeyError: 'approval_id'`. +- `functional_tests/test_orchestration_m365_actions.py` replaces the shared + conversation refusal with a step that reads mail for the shared audience, and a + step that stops once the user is no longer a participant. +- `functional_tests/test_v2_shared_orchestration_routing.mjs` executes the shared + send rule and the de-duplication, and checks the composer routing. +- `ui_tests/test_v2_shared_orchestration_routing.py` drives the real Composer in + Chromium with HTTP doubled: "@Ada …" is posted without a plan, "@Research Assistant …" + is planned with that agent, its AI target and its mentions, and another + participant's request goes to the shared assistant stream. All three checks fail on + 0.261.264. + +### Before and after + +| Before | After | +| --- | --- | +| "@person hey" with Orchestrate on was planned and answered by the model. | It's posted to that person; no model or plan runs. | +| Orchestrated questions and answers stayed in a private copy. | They're posted to the shared thread for every participant. | +| Microsoft 365 in a shared conversation waited for approval in chat and was refused in plans. | The user's own request is their consent, recorded in their audit history. | + +## Limitations + +- Only the person who started a shared conversation plans in it. +- Other participants see the question and answer, not the plan card or run details. +- Files, images and charts that a plan creates aren't attached to the shared copy of + the answer. +- The planner reads earlier planned turns, not messages that participants exchanged + or that the classic assistant answered. +- If an earlier version made a private copy for another participant, Orchestrate stays + unavailable to the person who started the conversation until that record is + removed. It is the record in the conversations container whose ID is the shared + conversation's ID and whose owner is that participant. + +## Related + +- [Shared conversations](../../admin/orchestration.md#shared-conversations) +- [Microsoft 365 data and approvals](../../guides/microsoft-365-conversation-data.md) +- [Orchestration session-trusted action and agent steps fix](ORCHESTRATION_SESSION_TRUSTED_ACTION_AGENT_STEPS_FIX.md) diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 2a33e8e62..072b81d09 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,24 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.269)** + +#### Bug Fixes + +* **Orchestrate Action and Agent Steps Work Again, Including Microsoft 365** + * With **Orchestrate** on, "what are my emails" failed with "A required retained result is unavailable or changed" in personal and shared chats, while the same request worked with Orchestrate off. Agent and action steps now trust the signed-in session the way manual chat does: access to the conversation, run and exact agent or action is checked, and the step runs without comparing configuration. Web search, linked-page reading and deep research keep their configuration checks. + * Local agents now run as plan steps. An agent that uses Microsoft 365 actions gets a Microsoft 365 request for its own actions, with the same read-only and signed-in-user rules as an action step. + * An @agent or @model tag in an Orchestrate message now chooses that agent or model, like the pickers do. + * Editing an agent or action no longer invalidates results it already produced. A result stays readable until the user loses access to that agent or action. + * (Ref: #1660, #1661, `functions_orchestration_external_sources.py`, `functions_orchestration_bootstrap.py`, `functions_orchestration_adapters.py`, `functions_orchestration_m365.py`, `agent_delegation_runtime.py`, `Composer.tsx`, [Session-Trusted Action and Agent Steps Fix](fixes/ORCHESTRATION_SESSION_TRUSTED_ACTION_AGENT_STEPS_FIX.md)) + +* **Orchestrate Respects Shared Conversations** + * In a shared conversation with Orchestrate on, a message that addresses only people, such as "@person hey", is now posted to them instead of being sent to the model, as it is with Orchestrate off. + * Only the person who started the shared conversation plans in it. Their question and the run's answer are posted to the shared thread, where every participant sees them arrive. Plans run in a hidden conversation with the shared conversation's ID and workspace lock instead of a private copy, and earlier private copies no longer appear in conversation lists. Other participants' requests are answered the classic way. + * Reopening the shared conversation, and its images, summaries, exports, uploads and Microsoft 365 action cards, keep using the shared conversation for every participant. An answer that finishes later, for example after a waiting step, updates its shared copy. Nothing is posted once the person who started the conversation has left it, and deleting the shared conversation deletes its plans. + * Asking for your own Microsoft 365 data in a shared conversation is now your consent to share what that request reads, in chat and in plans, recorded in your audit history without a prompt. Sharing earlier answers' history and workflow Run as approvals still ask. + * (Ref: #1659, `functions_orchestration_collaboration.py`, `route_backend_orchestration.py`, `collaboration_models.py`, `route_backend_conversations.py`, `functions_m365_approvals.py`, `functions_m365_runtime.py`, `mentions.ts`, `chatStore.ts`, [Shared Conversations Fix](fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md)) + ### **(v0.261.268)** #### Bug Fixes diff --git a/docs/guides/microsoft-365-conversation-data.md b/docs/guides/microsoft-365-conversation-data.md index f7b845345..1337f0c33 100644 --- a/docs/guides/microsoft-365-conversation-data.md +++ b/docs/guides/microsoft-365-conversation-data.md @@ -29,7 +29,7 @@ walk; continuation and coverage results describe the available search window. | Decision | What it authorizes | | --- | --- | | Microsoft Entra consent/sign-in | The application may call the selected Microsoft 365 APIs as the user. Source permissions still apply. | -| Sharing acknowledgement | Retrieved answers and retained source evidence may be published to conversation participants. | +| Sharing acknowledgement | Retrieved answers and retained source evidence may be published to conversation participants. In a shared conversation, your own chat or plan request is this acknowledgement for the data it reads. | | Extended analysis | The selected files may receive additional staged processing beyond the fast windows. | | Workflow Run as approval | A specific workflow revision may use the selected person's connected account. A revision that person saved themselves needs no separate approval. | | Outgoing action review | The data owner may send the exact prepared email or invitation in manual mode, or stop a configured delayed delivery. | @@ -43,11 +43,20 @@ The **Settings** tab on Profile keeps independent sharing preferences for Calendar, Email, OneDrive, and SPO. They apply across personal, group, and global actions rather than being tied to one agent you might not be able to edit. -In shared conversations, choose **Allow this request**, **Allow for today**, -**Always allow**, or **No**. Today expires at local midnight in the confirmed -time zone. An action owner can permit a shorter maximum duration. An Always -preference does not silently create a fresh daily approval when that action's -shorter approval has expired. +Since **0.261.269**, asking for your own Microsoft 365 data in a shared +conversation is your consent to share what that request reads with the +conversation's participants, with or without **Orchestrate**. You aren't +prompted, and each request and source is recorded in your audit history as +shared by your request. Before 0.261.269 these requests waited for a sharing +approval, and plans refused them. + +Sharing a private conversation that already used Microsoft 365 data, and a +workflow that runs as you in a shared conversation, still ask. For those, +choose **Allow this request**, **Allow for today**, **Always allow**, or +**No**. Today expires at local midnight in the confirmed time zone. An action +owner can permit a shorter maximum duration. An Always preference does not +silently create a fresh daily approval when that action's shorter approval has +expired. Declining a source lets the request continue without that source. Private single-user chats do not require a sharing acknowledgement merely because @@ -74,11 +83,13 @@ fresh source sign-in. ## Use Microsoft 365 in a plan Implemented in version: **0.261.238** (`application/single_app/config.py`). +Agent steps and shared conversations: **0.261.269**. With **Orchestrate** on in V2 chat, a plan can include a **Use an action** step that runs one of your Calendar, Email, OneDrive, or SharePoint Online actions. The step reads as you, the person who sent the request, and uses only that one -action. An agent step doesn't make its agent's Microsoft 365 actions available. +action. An **Ask an agent** step whose agent uses Microsoft 365 actions reads +with that agent's own actions, under the same rules. Plans only read Microsoft 365 data. Sending mail, sending calendar invitations, and marking mail as read aren't offered to a plan step, even when the action @@ -95,10 +106,12 @@ Some file analysis needs your approval first. The step then stops and links to from failed step** afterward: the retried step continues the same Microsoft 365 request, so an approval you gave applies while the action is unchanged. -Plans don't use Microsoft 365 in shared conversations, and a step there stops -before reading anything. An answer from a plan step that read Microsoft 365 -counts as Microsoft 365 history: sharing that conversation later asks for your -approval, as it does for a chat answer. +In a shared conversation, a plan step reads for the conversation's +participants, and your request is your consent to share what it reads, as it +is in chat. Only the person who started a shared conversation plans in it. An +answer from a plan step that read Microsoft 365 counts as Microsoft 365 +history: sharing a private conversation later asks for your approval, as it +does for a chat answer. ## Review an email or meeting before delivery @@ -149,8 +162,9 @@ configuration is a different failure and must not be described as expired login. ## Share evidence deliberately A sharing acknowledgement covers **retained source evidence, not only the final -answer**. Other participants can reuse the published snapshot without having -access to its original Microsoft 365 file. New remote calls still use the +answer**. Your own request in a shared conversation is that acknowledgement for +the files it reads. Other participants can reuse the published snapshot without +having access to its original Microsoft 365 file. New remote calls still use the requesting participant's delegated identity. The same rules apply when a private conversation is shared later. Published diff --git a/functional_tests/test_document_provenance_versions_and_guards.py b/functional_tests/test_document_provenance_versions_and_guards.py index 01806c851..33197d8e8 100644 --- a/functional_tests/test_document_provenance_versions_and_guards.py +++ b/functional_tests/test_document_provenance_versions_and_guards.py @@ -48,7 +48,11 @@ sys.path.insert(0, str(APP_ROOT)) # Resolve application modules only after configuring the standalone path. -from collaboration_models import GROUP_MULTI_USER_CHAT_TYPE, PERSONAL_MULTI_USER_CHAT_TYPE +from collaboration_models import ( + GROUP_MULTI_USER_CHAT_TYPE, + PERSONAL_MULTI_USER_CHAT_TYPE, + is_shared_conversation_backing, +) from content_screening import access from content_screening.contracts import SCREENING_FIELD, ScreeningValidationError import functions_document_provenance as provenance @@ -881,6 +885,7 @@ def allowed_file(file_name): "get_current_user_info": lambda: {"userId": "actor"}, "cosmos_conversations_container": conversations, "CosmosResourceNotFoundError": ConversationNotFound, + "is_shared_conversation_backing": is_shared_conversation_backing, "get_collaboration_conversation": get_collaboration_conversation, "assert_user_can_participate_in_collaboration_conversation": ( assert_user_can_participate_in_collaboration_conversation diff --git a/functional_tests/test_m365_provider_core.py b/functional_tests/test_m365_provider_core.py index 29131113e..a40ac1adb 100644 --- a/functional_tests/test_m365_provider_core.py +++ b/functional_tests/test_m365_provider_core.py @@ -1340,6 +1340,71 @@ def test_snapshot_only_action_publishes_a_prior_capture_without_remote_access( assert forbidden.call_count == 0 +def test_shared_request_consent_publishes_a_capture_without_an_approval( + execution, memory_runtime, real_content_helpers, monkeypatch, +): + """Asking for your own file in a shared conversation shares it there; no approval waits (#1659).""" + from dataclasses import replace + import functions_m365_approvals as approval_module + import functions_m365_execution as execution_module + from test_m365_runtime_adapters import load_module, module_stub + from test_support.m365 import CosmosContainer, Notifications + + actions = ContentGraphFixture().operations() + prepared = actions.prepare_file("drive-1", "item-1") + manifest = { + "id": actions.action_id, "type": "m365_onedrive", "source": "onedrive", + "enabled_functions": ["read_file_chunk"], + } + actions.manifest["enabled_functions"] = ["read_file_chunk"] + context = execution_module.M365ExecutionContext( + actor_user_id="user-1", data_user_id="user-1", tenant_id="tenant-1", + conversation_id="conversation-1", request_id="shared-chat-request", + shared=True, audience_version="audience-1", action_configs={actions.action_id: manifest}, + shared_by_request=True, + ) + container = CosmosContainer() + notifications = Notifications() + service = approval_module.M365ApprovalService( + container_factory=lambda: container, notification_sender=notifications, + decision_validator=lambda approval: True, + ) + runtime = load_module("conversation_memory_runtime", { + "config": module_stub( + "config", CLIENTS={}, TENANT_ID="tenant-1", cosmos_conversations_container=None, + cosmos_messages_container=None, build_enhanced_citations_blob_service_client=lambda settings: None, + ), + "functions_appinsights": module_stub("functions_appinsights", log_event=Mock()), + "functions_collaboration": module_stub("functions_collaboration", build_conversation_participation_context=lambda *args: None), + "functions_settings": module_stub("functions_settings", get_settings=lambda: {}), + }) + monkeypatch.setattr(memory_runtime.store, "authorize_publish", runtime._authorize_memory_publication) + monkeypatch.setattr(approval_module, "_service", service) + monkeypatch.setattr(execution_module, "_action_config_resolver", lambda *args: manifest) + monkeypatch.setattr(execution_module, "_action_selection_resolver", lambda context: [actions.action_id]) + for module in (retrieval, transport_module): + monkeypatch.setattr(module, "get_m365_context", lambda **kwargs: execution_module.get_m365_execution_context()) + monkeypatch.setattr(retrieval, "authorize_m365_capability", REAL_AUTHORIZE_CAPABILITY) + monkeypatch.setattr(retrieval, "authorize_m365_publication", transport_module.authorize_m365_publication) + forbidden = Mock(side_effect=AssertionError("Publishing a retained snapshot must not fetch source data or tokens.")) + monkeypatch.setattr(retrieval, "authorize_m365_source", forbidden) + monkeypatch.setattr(M365Transport, "get_token", forbidden) + monkeypatch.setattr(M365Transport, "request_json", forbidden) + with execution_module.m365_execution_context(context): + published = actions.read_file_chunk(prepared["memory_id"]) + reader = replace(context, actor_user_id="user-2", data_user_id="user-2", request_id="reader-request") + with execution_module.m365_execution_context(reader): + reused = actions.read_file_chunk(prepared["memory_id"]) + + assert published["snapshot_state"] == reused["snapshot_state"] == "published_snapshot" + assert published["text"] == reused["text"] == "file content" + records = list(container.items.values()) + assert [(record.get("event_type"), record.get("source")) for record in records] == [ + ("shared_by_request", "onedrive"), + ] + assert notifications.calls == [] and forbidden.call_count == 0 + + def test_source_schemas_accept_defaults_and_reject_cross_source_capabilities(): for action_type in operations.M365_ACTION_TYPES: schema = operations.get_m365_schema_for_type(action_type) diff --git a/functional_tests/test_m365_shared_request_consent.py b/functional_tests/test_m365_shared_request_consent.py new file mode 100644 index 000000000..1e58c0b41 --- /dev/null +++ b/functional_tests/test_m365_shared_request_consent.py @@ -0,0 +1,190 @@ +# test_m365_shared_request_consent.py +#!/usr/bin/env python3 +""" +Functional test for Microsoft 365 consent by request in shared conversations. +Version: 0.261.269 +Implemented in: 0.261.269 + +In a shared conversation, an interactive request by the owner of the Microsoft 365 data is +their consent to share the sources it reads with that conversation (microsoft/simplechat#1659). +These tests run the real approval service against a conditional Cosmos double and check that: + +* a request flagged ``shared_by_request`` is granted without a pending approval or a + notification, and records one audit event per request and source; +* the grant has the shape publication of captured evidence needs; +* contexts without the flag (workflow Run as and history publication) still need an approval; +* only an interactive shared request can carry the flag, and it never changes the approval + fingerprint. +""" + +import ast +import sys +from dataclasses import replace +from pathlib import Path + +import pytest + +TESTS = Path(__file__).resolve().parent +APP = TESTS.parent / "application" / "single_app" +sys.path.insert(0, str(APP)) +sys.path.insert(0, str(TESTS)) + +import functions_m365_approvals as approvals # noqa: E402 - the source path is set above +import functions_m365_execution as execution # noqa: E402 +from test_support.m365 import Clock, CosmosContainer, Notifications # noqa: E402 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +def context(**changes): + values = { + "actor_user_id": "user-a", "data_user_id": "user-a", "tenant_id": "tenant-a", + "conversation_id": "shared-conversation", "request_id": "request-a", + "shared": True, "audience_version": "audience-1", "agent_id": "agent-a", + "action_configs": {"action-a": {"source": "email", "maximum_sharing_acknowledgement": "always"}}, + "shared_by_request": True, + } + values.update(changes) + return execution.M365ExecutionContext(**values) + + +@pytest.fixture +def service(): + container = CosmosContainer() + notifications = Notifications() + result = approvals.M365ApprovalService( + container_factory=lambda: container, notification_sender=notifications, + clock=Clock(), decision_validator=lambda approval: True, + ) + result.test_container = container + result.test_notifications = notifications + return result + + +def records(service, **match): + return [ + record for record in service.test_container.items.values() + if all(record.get(key) == value for key, value in match.items()) + ] + + +def test_version_includes_consent_by_request(): + assert_app_version_at_least("0.261.269") + + +def test_a_shared_request_is_its_own_consent_without_an_approval(service): + grants = service.authorize_sources(context(), {"email": "always"}) + + grant = grants["email"] + assert grant["shared_by_request"] is True and grant["sharing_required"] is False + assert grant["approval_id"] == grant["audit_id"] == grant["decision_event_id"] + assert grant["audit_id"].startswith("m365-request-") + assert grant["effective_duration"] == "request" and grant["expires_at"] is None + assert grant["acknowledged_at"] + events = records(service, event_type="shared_by_request") + assert len(events) == 1 and events[0]["id"] == grant["audit_id"] + assert events[0]["source"] == "email" and events[0]["subject_user_id"] == "user-a" + assert events[0]["context"]["request_id"] == "request-a" and events[0]["context"]["shared"] is True + # Nothing waits for a decision and nobody is notified. + assert records(service, status="pending") == [] and service.test_notifications.calls == [] + + +def test_a_repeated_request_reuses_its_consent_record(service): + first = service.authorize_sources(context(), {"email": "always"})["email"] + second = service.authorize_sources(context(), {"email": "always"})["email"] + + assert second == first + assert len(records(service, event_type="shared_by_request")) == 1 + + +def test_each_request_and_source_records_its_own_consent(service): + both = service.authorize_sources(context(), {"email": "always", "calendar": "today"}) + later = service.authorize_sources(context(request_id="request-b"), {"email": "always"}) + + assert set(both) == {"email", "calendar"} + assert len({both["email"]["audit_id"], both["calendar"]["audit_id"], later["email"]["audit_id"]}) == 3 + assert len(records(service, event_type="shared_by_request")) == 3 + + +def test_a_shared_context_without_the_request_flag_still_needs_an_approval(service): + with pytest.raises(approvals.M365ApprovalRequired) as pending: + service.authorize_sources(context(shared_by_request=False), {"email": "always"}) + + assert records(service, event_type="shared_by_request") == [] + assert service.get_approval(pending.value.approval_id, "user-a")["status"] == "pending" + assert len(service.test_notifications.calls) == 1 + + +def test_private_conversations_are_unchanged(service): + grants = service.authorize_sources( + context(shared=False, shared_by_request=False), {"email": "always"}, + ) + assert grants == {"email": {"source": "email", "sharing_required": False}} + assert service.test_container.items == {} + + +@pytest.mark.parametrize(("changes", "message"), [ + ({"shared": False}, "Only an interactive shared request"), + ({"workflow_id": "workflow-a", "run_id": "run-a", "step_id": "step-a"}, "Only an interactive shared request"), + ({"shared_by_request": 1}, "Invalid authoritative"), +]) +def test_only_an_interactive_shared_request_can_share_by_request(changes, message): + with pytest.raises(ValueError, match=message): + context(**changes) + + +def test_the_request_flag_never_changes_the_approval_fingerprint(): + flagged = context() + plain = replace(flagged, shared_by_request=False) + + assert approvals.approval_context(flagged) == approvals.approval_context(plain) + assert approvals.request_scope_fingerprint(flagged) == approvals.request_scope_fingerprint(plain) + assert approvals.logical_request_fingerprint(flagged) == approvals.logical_request_fingerprint(plain) + + +def test_a_reviewed_delivery_keeps_its_requests_consent(service, monkeypatch): + """Sending a reviewed email from a shared conversation needs no approval its request didn't.""" + import functions_m365_pending_delivery as delivery + + monkeypatch.setitem(delivery._dependencies, "capture_agent_reference", lambda context, action_id: {"id": "agent-a"}) + with execution.m365_execution_context(context()): + captured = delivery.capture_workflow_delivery("user-a", "action-a", None, None) + restored = execution.M365ExecutionContext(**captured["context"]) + + assert captured["kind"] == "chat" and restored.shared_by_request is True + assert service.authorize_sources(restored, {"email": "always"})["email"]["shared_by_request"] is True + # A delivery recorded before 0.261.269 has no flag and still asks, as it did then. + legacy = {key: value for key, value in captured["context"].items() if key != "shared_by_request"} + with pytest.raises(approvals.M365ApprovalRequired): + service.authorize_sources(execution.M365ExecutionContext(**legacy), {"email": "always"}) + + +def _context_keywords(path, function_name=None): + """The keyword names of every M365ExecutionContext(...) call in a module or one function.""" + tree = ast.parse(path.read_text(encoding="utf-8")) + scopes = [ + node for node in ast.walk(tree) + if function_name is None or (isinstance(node, ast.FunctionDef) and node.name == function_name) + ] + calls = [] + for scope in scopes[:1] if function_name else [tree]: + for node in ast.walk(scope): + if ( + isinstance(node, ast.Call) and isinstance(node.func, ast.Name) + and node.func.id == "M365ExecutionContext" + ): + calls.append({keyword.arg for keyword in node.keywords}) + return calls + + +def test_chat_and_plan_step_contexts_share_by_request_and_history_publication_does_not(): + runtime = APP / "functions_m365_runtime.py" + for function_name in ("initialize_m365_chat_context", "step_m365_context"): + calls = _context_keywords(runtime, function_name) + assert calls and all("shared_by_request" in keywords for keywords in calls), function_name + # Publishing a conversation's earlier answers keeps its own approval. + history = _context_keywords(APP / "functions_m365_history.py") + assert history and all("shared_by_request" not in keywords for keywords in history) + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-q"])) diff --git a/functional_tests/test_orchestration_m365_actions.py b/functional_tests/test_orchestration_m365_actions.py index 96c40bd77..f76973ac4 100644 --- a/functional_tests/test_orchestration_m365_actions.py +++ b/functional_tests/test_orchestration_m365_actions.py @@ -2,13 +2,15 @@ #!/usr/bin/env python3 """ Functional test for Microsoft 365 actions in chat orchestration. -Version: 0.261.238 +Version: 0.261.269 Implemented in: 0.261.238 +Shared conversations read for their real audience, with the request as consent, in: 0.261.269 An orchestration "Use an action" step runs in its own request context, the execution identity's bridge. Before 0.261.238 no Microsoft 365 execution context existed there, so every Microsoft 365 function refused the call with ``m365_context_required`` before reaching -Microsoft Graph, and the step still reported completed. +Microsoft Graph, and the step still reported completed. Before 0.261.269 a step in a shared +conversation was refused outright (microsoft/simplechat#1659). These tests run the real action runner, step scope, Microsoft 365 runtime, execution boundary, approval service and email plugin through a real Flask bridge. Storage, tokens, @@ -422,11 +424,37 @@ def test_ordinary_graph_outcomes_stay_findings(env, world): assert request_record(env, world)["status"] == "completed" -def test_shared_conversations_are_refused_before_any_microsoft_365_work(env, world): +def test_shared_conversation_step_reads_mail_with_the_requests_own_consent(env, world, monkeypatch): + """A shared conversation's step reads for its real audience; asking for the mail is the consent.""" + approvals = CosmosContainer() + monkeypatch.setattr(env.approvals, "_service", env.approvals.M365ApprovalService( + container_factory=lambda: approvals, notification_sender=lambda approval: None, + )) world.participation = {"collaboration_conversation_id": "shared-1"} - error = stopped(env, world) - assert error.orchestration_failure_code == "m365_shared_conversation" - assert world.token_requests == [] and world.requests == [] and world.jobs.items == {} + world.replies = [tool_call()] + result, _context = run_step(env, world) + + request_id = env.orchestration.step_request_id(REQUEST_KEY) + assert result["calls"] == 1 and "Quarterly review" in result["findings"] + assert len(world.graph_calls) == 1 + assert world.token_requests and all(request["shared"] is True for request in world.token_requests) + records = list(approvals.items.values()) + # One audit event says the requester shared their mail by asking; no approval is waiting. + assert [(record["event_type"], record["source"]) for record in records] == [("shared_by_request", "email")] + assert records[0]["context"]["request_id"] == request_id and records[0]["context"]["shared"] is True + assert request_record(env, world)["status"] == "completed" + + +def test_shared_conversation_step_still_needs_a_current_participant(env, world): + world.participation = {"collaboration_conversation_id": "shared-1"} + + def removed(*args): + raise PermissionError("You are no longer a participant in this conversation.") + + with patch.object(env.runtime, "assert_user_can_participate_in_collaboration_conversation", removed): + with pytest.raises(PermissionError): + run_step(env, world) + assert world.token_requests == [] and world.requests == [] and world.graph_calls == [] def test_send_only_action_is_refused_as_read_only(env, world): diff --git a/functional_tests/test_orchestration_shared_backing_lookups.py b/functional_tests/test_orchestration_shared_backing_lookups.py new file mode 100644 index 000000000..12d5ecfbb --- /dev/null +++ b/functional_tests/test_orchestration_shared_backing_lookups.py @@ -0,0 +1,215 @@ +# test_orchestration_shared_backing_lookups.py +#!/usr/bin/env python3 +""" +Functional test for lookups by a shared conversation's id while Orchestrate's backing exists. +Version: 0.261.269 +Implemented in: 0.261.269 + +Orchestrate keeps a shared conversation's plans in a hidden personal record stored under the +shared conversation's own id (microsoft/simplechat#1659). Code that looks a conversation id up +in personal storage first must treat that record as part of the shared conversation. Otherwise +reloading the conversation reopens it as a personal chat for the person who started it, other +participants are refused images, summaries and exports, uploads land in the wrong place, and +Microsoft 365 action cards disappear. + +These tests run the real lookups over in-memory storage, with the backing record present. +""" + +import ast +import importlib +import sys +from pathlib import Path + +import pytest +from flask import Blueprint, Flask +from werkzeug.test import Client +from werkzeug.wrappers import Response + +TESTS = Path(__file__).resolve().parent +APP = TESTS.parent / "application" / "single_app" +sys.path.insert(0, str(APP)) +sys.path.insert(0, str(TESTS)) + +from test_orchestration_harness_routes import login, modules, real_http_harness # noqa: E402,F401 +from test_orchestration_shared_conversations import GUEST, OWNER, SHARED_ID, shared # noqa: E402,F401 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + +CLASSIC_SOURCE_ID = "classic-source-1" + + +def accepted(conversation): + return { + participant["user_id"] for participant in conversation.get("participants") or [] + if participant.get("status") == "accepted" + } + + +def view(user_id, conversation, allow_pending=False): + if user_id not in accepted(conversation): + raise PermissionError("You are not a participant in this collaborative conversation") + return {"user_state": None, "membership_status": "accepted"} + + +@pytest.fixture +def backed(shared): + """The shared conversation with its Orchestrate backing and a classic source conversation.""" + collaboration = shared.shared.read_item(SHARED_ID, SHARED_ID) + collaboration["source_conversation_id"] = CLASSIC_SOURCE_ID + shared.shared.upsert_item(collaboration) + shared.harness.conversations.create_item({ + "id": SHARED_ID, "user_id": "owner", "title": "Launch plan", + "conversation_kind": "collaboration_source", "collaboration_conversation_id": SHARED_ID, + "is_hidden": True, "context": [], "chat_type": "personal_single_user", + }) + return shared + + +@pytest.fixture +def conversation_routes(backed, monkeypatch): + routes = importlib.import_module("route_backend_conversations") + monkeypatch.setattr(routes, "cosmos_conversations_container", backed.harness.conversations) + monkeypatch.setattr(routes, "get_settings", lambda: {"enable_collaborative_conversations": True}) + monkeypatch.setattr(routes, "assert_user_can_view_collaboration_conversation", view) + app = Flask(__name__) + app.config.update(TESTING=True, SECRET_KEY="shared-backing-lookups") + blueprint = Blueprint("backend_conversations", __name__) + routes.register_route_backend_conversations(blueprint) + app.register_blueprint(blueprint) + backed.routes = routes + backed.conversations_app = app + backed.conversations_client = Client(app, Response) + return backed + + +def kind(runtime, user_id, conversation_id=SHARED_ID): + runtime.app, runtime.client = runtime.conversations_app, runtime.conversations_client + login(runtime, user_id=user_id) + response = runtime.client.get(f"/api/conversations/{conversation_id}/kind") + return response.status_code, response.get_json() + + +def test_version_includes_shared_backing_lookups(): + assert_app_version_at_least("0.261.269") + + +def test_reopening_a_shared_conversation_never_turns_it_into_a_personal_chat(conversation_routes): + for user_id in ("owner", "guest"): + status, body = kind(conversation_routes, user_id) + assert status == 200 and body["kind"] == "collaborative", (user_id, body) + assert body["conversation"]["id"] == SHARED_ID + status, _body = kind(conversation_routes, "stranger") + assert status == 404 + + +def test_an_earlier_private_copy_opens_as_the_shared_conversation(conversation_routes): + conversations = conversation_routes.harness.conversations + conversations.upsert_item({"id": SHARED_ID, "user_id": "guest", "title": "New Conversation", "chat_type": "new"}) + + status, body = kind(conversation_routes, "guest") + assert status == 200 and body["kind"] == "collaborative" + # Someone who can no longer see the shared conversation still reaches their own copy. + collaboration = conversation_routes.shared.read_item(SHARED_ID, SHARED_ID) + collaboration["participants"] = [p for p in collaboration["participants"] if p["user_id"] != "guest"] + conversation_routes.shared.upsert_item(collaboration) + status, body = kind(conversation_routes, "guest") + assert status == 200 and body["kind"] == "personal" + + +def test_personal_conversations_are_unchanged(conversation_routes): + conversation_routes.harness.conversations.create_item({"id": "personal-1", "user_id": "owner", "title": "Mine"}) + assert kind(conversation_routes, "owner", "personal-1") == (200, {"conversation_id": "personal-1", "kind": "personal"}) + assert kind(conversation_routes, "guest", "personal-1")[0] == 404 + + +def test_personal_routes_treat_the_backing_as_part_of_the_shared_conversation(conversation_routes): + routes = conversation_routes.routes + with pytest.raises(LookupError): + routes._authorize_personal_conversation_read("owner", SHARED_ID) + # Images fall through to the shared conversation, so every participant can see them. + for user_id in ("owner", "guest"): + conversation, kind_name = routes._authorize_image_conversation_read(user_id, SHARED_ID) + assert kind_name == "collaboration" and conversation["id"] == SHARED_ID + with pytest.raises(PermissionError): + routes._authorize_image_conversation_read("stranger", SHARED_ID) + monkeypatched = routes.assert_user_can_participate_in_collaboration_conversation + try: + routes.assert_user_can_participate_in_collaboration_conversation = view + item, scope_kind = routes._load_scope_lock_conversation(SHARED_ID, "guest") + finally: + routes.assert_user_can_participate_in_collaboration_conversation = monkeypatched + assert scope_kind == "collaboration" and item["id"] == SHARED_ID + + +def test_metadata_exports_and_tools_use_the_shared_conversation(backed, monkeypatch): + metadata = importlib.import_module("functions_conversation_metadata") + monkeypatch.setattr(metadata, "cosmos_conversations_container", backed.harness.conversations) + item, source = metadata._get_conversation_item_with_source(SHARED_ID) + assert source == "collaboration" and item["id"] == SHARED_ID + + export = importlib.import_module("route_backend_conversation_export") + monkeypatch.setattr(export, "cosmos_conversations_container", backed.harness.conversations) + monkeypatch.setattr(export, "assert_user_can_view_collaboration_conversation", view) + for user_id in ("owner", "guest"): + loaded = export._load_exportable_conversation_for_user(user_id, SHARED_ID) + assert loaded is not None and loaded[0]["id"] == SHARED_ID, user_id + + tools = importlib.import_module("functions_mcp_server_tools") + monkeypatch.setattr(tools, "cosmos_conversations_container", backed.harness.conversations) + monkeypatch.setattr(tools, "assert_user_can_view_collaboration_conversation", view) + item, source = tools._authorize_personal_conversation_read("owner", SHARED_ID) + assert source == "collaboration" and item["id"] == SHARED_ID + + +def test_chat_never_writes_into_the_backing_directly(backed, monkeypatch): + chats = importlib.import_module("route_backend_chats") + monkeypatch.setattr(chats, "cosmos_conversations_container", backed.harness.conversations) + for user_id in ("owner", "guest"): + with pytest.raises(LookupError): + chats._resolve_authorized_conversation_context(user_id, SHARED_ID) + + +def test_uploads_go_to_the_shared_conversation_not_the_backing(backed, monkeypatch): + chats = importlib.import_module("route_frontend_chats") + monkeypatch.setattr(chats, "cosmos_conversations_container", backed.harness.conversations) + monkeypatch.setattr(chats, "assert_user_can_participate_in_collaboration_conversation", view) + sources = [] + + def ensure_source(collaboration, current_user): + sources.append(current_user) + return {"id": CLASSIC_SOURCE_ID, "user_id": "owner"}, collaboration + + monkeypatch.setattr(chats, "ensure_collaboration_source_conversation", ensure_source) + context = chats._resolve_chat_upload_context(SHARED_ID, "guest", {"userId": "guest"}) + assert context["conversation_id"] == CLASSIC_SOURCE_ID + assert context["response_conversation_id"] == SHARED_ID and len(sources) == 1 + + +def test_microsoft_365_cards_keep_resolving_to_the_classic_source(backed, monkeypatch): + runtime = importlib.import_module("functions_m365_runtime") + monkeypatch.setattr(runtime, "cosmos_conversations_container", backed.harness.conversations) + monkeypatch.setattr(runtime, "assert_user_can_participate_in_collaboration_conversation", view) + for user_id in ("owner", "guest"): + assert runtime.resolve_m365_audit_conversation_id(user_id, SHARED_ID) == CLASSIC_SOURCE_ID + backed.harness.conversations.create_item({"id": "personal-2", "user_id": "owner"}) + assert runtime.resolve_m365_audit_conversation_id("owner", "personal-2") == "personal-2" + + +def _function_source(path, name): + tree = ast.parse(path.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == name: + return ast.get_source_segment(path.read_text(encoding="utf-8"), node) + raise AssertionError(f"{name} is not defined in {path.name}") + + +@pytest.mark.parametrize(("module", "function"), [ + ("route_backend_conversations.py", "generate_conversation_summary_api"), + ("functions_simplechat_operations.py", "add_conversation_message_for_current_user"), + ("route_frontend_conversations.py", "get_agent_citation_artifact"), +]) +def test_other_personal_first_lookups_recognize_the_backing(module, function): + assert "is_shared_conversation_backing(" in _function_source(APP / module, function) + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-q"])) diff --git a/functional_tests/test_orchestration_shared_conversations.py b/functional_tests/test_orchestration_shared_conversations.py new file mode 100644 index 000000000..5a9dc4be0 --- /dev/null +++ b/functional_tests/test_orchestration_shared_conversations.py @@ -0,0 +1,337 @@ +# test_orchestration_shared_conversations.py +#!/usr/bin/env python3 +""" +Functional test for Orchestrate in shared conversations. +Version: 0.261.269 +Implemented in: 0.261.269 + +Before 0.261.269 an orchestrated request in a shared conversation created a private +personal conversation under the shared conversation's id, so the question and answer were +never posted to the shared thread (microsoft/simplechat#1659). These tests drive the +production orchestration Blueprint over HTTP, with the real collaboration storage functions +over in-memory containers, and check that: + +* the person who started the shared conversation plans in a hidden backing conversation that + has the shared conversation's id and workspace lock, and no "new conversation" is announced; +* the question is posted to the shared thread with its mentions and AI target, and the run's + answer is posted as a reply to it, each once, with a live event for every participant; +* other participants are told to turn off Orchestrate, and anyone else learns nothing. +""" + +import importlib +import json +import sys +from copy import deepcopy +from pathlib import Path + +import pytest +from azure.cosmos.exceptions import CosmosResourceNotFoundError + +TESTS = Path(__file__).resolve().parent +sys.path.insert(0, str(TESTS.parent / "application" / "single_app")) +sys.path.insert(0, str(TESTS)) + +from test_orchestration_harness_routes import login, modules, real_http_harness # noqa: E402,F401 +from test_support.orchestration_harness_execution import compose_step, input_binding # noqa: E402 +from test_support.orchestration_revisions import AtomicMemoryContainer # noqa: E402 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + +SHARED_ID = "shared-conversation-1" +REQUEST = "@Assistant draft a short launch note for the team." +ANSWER = "Here is a short launch note for the team." +OWNER = {"user_id": "owner", "display_name": "Owner Person", "email": "owner@example.test"} +GUEST = {"user_id": "guest", "display_name": "Guest Person", "email": "guest@example.test"} +TARGET = { + "target_type": "model", "display_name": "Assistant", "mention_text": "@Assistant", + "source_mode": "mention", "ignored": "not kept", +} + + +class CollaborationMessages(AtomicMemoryContainer): + """Collaboration message storage that also answers the source-message lookup.""" + + def query_items(self, query, parameters=None, partition_key=None, **kwargs): + params = {entry["name"]: entry["value"] for entry in parameters or []} + if "@source_message_id" not in params: + return super().query_items(query, parameters, partition_key, **kwargs) + rows = [ + deepcopy(item) for item in self.items.values() + if item.get("conversation_id") == params["@conversation_id"] + and (item.get("metadata") or {}).get("source_message_id") == params["@source_message_id"] + ] + return rows[:1] + + +@pytest.fixture +def shared(real_http_harness, modules, monkeypatch): + collaboration = importlib.import_module("functions_collaboration") + models = importlib.import_module("collaboration_models") + conversations = AtomicMemoryContainer("id") + messages = CollaborationMessages("conversation_id") + events = [] + # The workspace document probe is not under test and would read public workspace settings. + monkeypatch.setattr(modules.route, "resolve_candidate_documents", lambda *args, **kwargs: ([], False)) + document = models.build_personal_collaboration_conversation( + "Launch plan", OWNER, invited_participants=[GUEST], conversation_id=SHARED_ID, + ) + for participant in document["participants"]: + participant["status"] = models.MEMBERSHIP_STATUS_ACCEPTED + conversations.create_item(document) + + def participate(user_id, conversation): + accepted = { + participant["user_id"] for participant in conversation.get("participants") or [] + if participant.get("status") == models.MEMBERSHIP_STATUS_ACCEPTED + } + if user_id not in accepted: + raise PermissionError("You are not a participant in this collaborative conversation") + return {"membership_status": models.MEMBERSHIP_STATUS_ACCEPTED} + + monkeypatch.setattr(collaboration, "cosmos_collaboration_conversations_container", conversations) + monkeypatch.setattr(collaboration, "cosmos_collaboration_messages_container", messages) + monkeypatch.setattr(collaboration, "assert_user_can_participate_in_collaboration_conversation", participate) + monkeypatch.setattr( + collaboration, "COLLABORATION_EVENT_PUBLISHERS", + [lambda conversation_id, event: events.append((conversation_id, deepcopy(event)))], + ) + real_http_harness.shared = conversations + real_http_harness.shared_messages = messages + real_http_harness.events = events + return real_http_harness + + +def frames(response): + return [ + json.loads(frame.partition("data:")[2].strip()) + for frame in response.get_data(as_text=True).split("\n\n") if frame.startswith("data:") + ] + + +def plan(runtime, **body): + # A first question has no history, so the planner is the first model call. + runtime.harness.replies = [ + json.dumps({"kind": "plan", "steps": [compose_step()], "final_response": input_binding("prepare")}), + ANSWER, + ] + response = runtime.client.post("/api/v2/orchestration/plan", json={ + "conversation_id": SHARED_ID, "turn_id": "shared-turn", "message": REQUEST, + "approval_mode": "manual", "planner_contract_version": 1, **body, + }, buffered=True) + assert response.status_code == 200 + return frames(response) + + +def shared_thread(runtime): + return sorted( + (deepcopy(item) for item in runtime.shared_messages.items.values()), + key=lambda item: item.get("timestamp") or "", + ) + + +def test_version_includes_shared_orchestration(): + assert_app_version_at_least("0.261.269") + + +def test_owner_plan_and_answer_are_posted_to_the_shared_thread(shared): + planning = plan( + shared, invocation_target=TARGET, mentioned_participants=[GUEST, {"user_id": "stranger"}], + ) + assert "plan" in planning[-1], planning + assert not [frame for frame in planning if frame.get("type") == "conversation_metadata"] + planned = planning[-1]["plan"] + + backing = shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + assert backing["user_id"] == "owner" and backing["is_hidden"] is True + assert backing["conversation_kind"] == "collaboration_source" + assert backing["collaboration_conversation_id"] == SHARED_ID + assert backing["scope_locked"] is True + assert backing["locked_contexts"] == [{"scope": "personal", "id": "owner"}] + record = shared.harness.runs.read_item(planned["run_id"], SHARED_ID) + assert record["user_id"] == "owner" + assert record["memory_audience"] == {"kind": "shared", "owner_id": "owner", "collaboration_id": SHARED_ID} + + question, = shared_thread(shared) + source = shared.harness.messages.read_item(question["metadata"]["source_message_id"], SHARED_ID) + assert source["role"] == "user" and source["content"] == REQUEST + assert question["conversation_id"] == SHARED_ID and question["content"] == REQUEST + assert question["message_kind"] == "ai_request" + assert question["metadata"]["sender"]["user_id"] == "owner" + # Only the bounded display fields of the AI target are kept. + assert question["metadata"]["ai_invocation_target"] == { + key: value for key, value in TARGET.items() if key != "ignored" + } + assert [mention["user_id"] for mention in question["metadata"]["mentioned_participants"]] == ["guest"] + assert [event["event_type"] for _id, event in shared.events] == ["collaboration.message.created"] + assert shared.events[0][1]["payload"]["message"]["id"] == question["id"] + + executed = frames(shared.client.post("/api/v2/orchestration/run", json={ + "conversation_id": SHARED_ID, "run_id": planned["run_id"], + }, buffered=True)) + assert executed[-1]["status"] == "completed", executed + answer = shared.harness.assistant_messages()[0] + asked, replied = shared_thread(shared) + assert asked["id"] == question["id"] + assert replied["content"] == ANSWER and replied["message_kind"] == "assistant_response" + assert replied["metadata"]["source_message_id"] == answer["id"] + assert replied["reply_to_message_id"] == question["id"] + assert [event["event_type"] for _id, event in shared.events] == ["collaboration.message.created"] * 2 + + # Mirroring is idempotent: the same answer is never posted twice. + mirror = importlib.import_module("functions_orchestration_collaboration") + mirror.mirror_orchestration_answer( + shared.harness.conversations.read_item(SHARED_ID, SHARED_ID), answer, + user_message_id=source["id"], owner_user_id="owner", + ) + assert len(shared_thread(shared)) == 2 and len(shared.events) == 2 + + +def test_other_participants_are_asked_to_turn_off_orchestrate(shared): + # Like the harness's other users, signed in with a role that skips the access-status lookup; + # roles never grant access to a shared conversation. + login(shared, user_id="guest") + planning = plan(shared, invocation_target=TARGET) + + mirror = importlib.import_module("functions_orchestration_collaboration") + assert planning[-1].get("error") == mirror.SHARED_ORCHESTRATION_OWNER_ONLY + assert "plan" not in planning[-1] + with pytest.raises(CosmosResourceNotFoundError): + shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + assert shared_thread(shared) == [] and shared.events == [] + + +def test_anyone_else_cannot_tell_a_shared_conversation_exists(shared): + login(shared, user_id="stranger") + planning = plan(shared) + + assert planning[-1].get("error") == "That conversation could not be opened." + with pytest.raises(CosmosResourceNotFoundError): + shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + assert shared_thread(shared) == [] and shared.events == [] + + +def test_an_earlier_private_copy_becomes_the_owners_backing_conversation(shared): + shared.harness.conversations.create_item({ + "id": SHARED_ID, "user_id": "owner", "title": "New Conversation", "context": [], + "tags": [], "strict": False, "chat_type": "new", + }) + planning = plan(shared, invocation_target=TARGET) + + assert "plan" in planning[-1], planning + backing = shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + assert backing["conversation_kind"] == "collaboration_source" and backing["is_hidden"] is True + assert backing["collaboration_conversation_id"] == SHARED_ID + assert len(shared_thread(shared)) == 1 + + +def test_another_users_private_copy_is_never_adopted(shared): + shared.harness.conversations.create_item({ + "id": SHARED_ID, "user_id": "guest", "title": "New Conversation", "chat_type": "new", + }) + planning = plan(shared, invocation_target=TARGET) + + mirror = importlib.import_module("functions_orchestration_collaboration") + # The person who started the conversation learns why, and how to continue. + assert planning[-1].get("error") == mirror.SHARED_ORCHESTRATION_STALE_COPY + assert shared.harness.conversations.read_item(SHARED_ID, SHARED_ID)["user_id"] == "guest" + assert shared_thread(shared) == [] + + +def run_shared_plan(shared): + """Plan and run one shared turn; return the backing, the question copy and the answer.""" + planned = plan(shared, invocation_target=TARGET)[-1]["plan"] + executed = frames(shared.client.post("/api/v2/orchestration/run", json={ + "conversation_id": SHARED_ID, "run_id": planned["run_id"], + }, buffered=True)) + assert executed[-1]["status"] == "completed", executed + backing = shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + return backing, planned, shared.harness.assistant_messages()[0] + + +def test_a_republished_answer_updates_its_shared_copy(shared): + mirror = importlib.import_module("functions_orchestration_collaboration") + backing, _planned, answer = run_shared_plan(shared) + question, copy = shared_thread(shared) + + final = {**answer, "content": "The final launch note, after the waiting step finished."} + refreshed = mirror.mirror_orchestration_answer( + backing, final, user_message_id=question["metadata"]["source_message_id"], owner_user_id="owner", + ) + + assert refreshed["id"] == copy["id"] + _question, updated = shared_thread(shared) + assert updated["content"] == final["content"] + assert updated["reply_to_message_id"] == question["id"] and updated["timestamp"] == copy["timestamp"] + assert updated["metadata"]["source_message_id"] == answer["id"] + assert updated["metadata"]["source_conversation_id"] == SHARED_ID + assert [event["event_type"] for _id, event in shared.events][-1] == "collaboration.message.updated" + assert shared.events[-1][1]["payload"]["message"]["content"] == final["content"] + # An unchanged republish posts and announces nothing. + count = len(shared.events) + mirror.mirror_orchestration_answer(backing, final, owner_user_id="owner") + assert len(shared.events) == count and len(shared_thread(shared)) == 2 + + +def test_nothing_is_posted_once_the_starter_leaves_the_conversation(shared): + mirror = importlib.import_module("functions_orchestration_collaboration") + backing, _planned, answer = run_shared_plan(shared) + collaboration = shared.shared.read_item(SHARED_ID, SHARED_ID) + for participant in collaboration["participants"]: + if participant["user_id"] == "owner": + participant["status"] = "removed" + shared.shared.upsert_item(collaboration) + posted, events = len(shared_thread(shared)), len(shared.events) + + with pytest.raises(PermissionError): + mirror.mirror_orchestration_answer(backing, {**answer, "content": "Changed."}, owner_user_id="owner") + with pytest.raises(PermissionError): + mirror.mirror_orchestration_turn( + backing, {"id": "user-later", "content": "Later question"}, OWNER, + ) + assert len(shared_thread(shared)) == posted and len(shared.events) == events + + +def test_deleting_the_shared_conversation_deletes_its_backing(shared): + mirror = importlib.import_module("functions_orchestration_collaboration") + _backing, planned, _answer = run_shared_plan(shared) + collaboration = shared.shared.read_item(SHARED_ID, SHARED_ID) + + def delete(**options): + return mirror.delete_orchestration_backing( + collaboration, archiving_enabled=False, + conversation_container=shared.harness.conversations, message_container=shared.harness.messages, + **options, + ) + + # A co-owner who didn't start the conversation leaves its creator's plans alone. + assert delete(expected_user_id="guest") is None + assert shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + + deleted = delete(expected_user_id="owner") + assert deleted["id"] == SHARED_ID + with pytest.raises(CosmosResourceNotFoundError): + shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + assert [ + message for message in shared.harness.messages.items.values() if message.get("conversation_id") == SHARED_ID + ] == [] + run = shared.harness.runs.read_item(planned["run_id"], SHARED_ID) + assert run["checkpoints_deleted"] is True and run["execution_lease"] is None + # Nothing is left to delete the second time. + assert delete() is None + + +def test_shared_conversation_deletion_removes_the_backing_first(): + """The shared conversation's own deletion path deletes the backing before anything else.""" + source = (Path(__file__).resolve().parents[1] / "application" / "single_app" / "functions_collaboration.py") + text = source.read_text(encoding="utf-8") + body = text[text.index("def _delete_collaboration_conversation_records("):] + body = body[:body.index("\ndef ")] + cancel = body.index("_cancel_collaboration_pending_deliveries(conversation_doc)") + backing = body.index("delete_orchestration_backing(") + assert cancel < backing < body.index("cosmos_collaboration_messages_container.query_items(") + assert "expected_user_id=expected_source_user_id" in body + assert "conversation_container=cosmos_conversations_container" in body + assert "not_found_error=CosmosResourceNotFoundError" in body + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-q"])) diff --git a/functional_tests/test_orchestration_turn_recovery_identifiers.py b/functional_tests/test_orchestration_turn_recovery_identifiers.py index 76e1da28f..ac725e57c 100644 --- a/functional_tests/test_orchestration_turn_recovery_identifiers.py +++ b/functional_tests/test_orchestration_turn_recovery_identifiers.py @@ -2,9 +2,10 @@ #!/usr/bin/env python3 """ Functional test for the identifiers that let an orchestration turn be found again. -Version: 0.261.213 +Version: 0.261.269 Implemented in: 0.261.099 Chat content-check metadata binding included in: 0.261.131 +Shared conversation question posting included in: 0.261.269 A run is only recoverable if its question can be found in the thread. The live card stamps ``orchestration_turn_id`` on its optimistic user bubble, but the server saved the same message @@ -154,11 +155,13 @@ def create_run(plan, user_id, **kwargs): context = import_app_module("functions_orchestration_context") authorize = Mock(return_value={"id": "conversation-1", "user_id": "user-1"}) latest_run = Mock(return_value=None) + mirror = Mock() namespace = { '_save_turn_message': save_message, 'create_orchestration_run': create_run, 'validate_memory_context': memory.validate_memory_context, '_authorize_context_conversation': authorize, 'get_latest_turn_run': latest_run, + '_mirror_planned_turn': mirror, 'CHECK_METADATA': checks.CHECK_METADATA, 'image_reference_provenance_from_seeds': context.image_reference_provenance_from_seeds, 'g': g, 'has_request_context': has_request_context, @@ -170,6 +173,8 @@ def create_run(plan, user_id, **kwargs): authorize.assert_called_once_with('conversation-1', 'user-1') latest_run.assert_called_once_with('conversation-1', 'user-1', 'turn-1') + # A shared conversation's question is posted to its thread under the saved message id. + mirror.assert_called_once_with('conversation-1', 'user-1', 'saved-question', turn_context) assert saved_messages == [('conversation-1', 'user-1', 'turn-1', 'The original question')] assert len(created_runs) == 1, 'the turn must be persisted once' stored_plan, stored_user, arguments = created_runs[0] diff --git a/functional_tests/test_v2_shared_orchestration_routing.mjs b/functional_tests/test_v2_shared_orchestration_routing.mjs new file mode 100644 index 000000000..2bd4164e7 --- /dev/null +++ b/functional_tests/test_v2_shared_orchestration_routing.mjs @@ -0,0 +1,143 @@ +// test_v2_shared_orchestration_routing.mjs +// +// Runtime test for Orchestrate in V2 shared conversations. +// Version: 0.261.269 +// Implemented in: 0.261.269 (Refs #1659) +// +// With Orchestrate on, every message in a shared conversation used to start a plan, so a remark +// addressed only to another person was sent to the model. The composer now applies the shared +// conversation send rule first, the same rule manual sends use: a message that addresses only +// people is posted to them. Only the person who started the conversation plans in it. +// +// This file executes the real send rule and the real chat store merge for the shared copies of +// an orchestrated question and answer, and checks the composer wiring that a runtime test cannot +// reach without a browser. +// +// Run directly with `node functional_tests/test_v2_shared_orchestration_routing.mjs`. Requires +// Node 22.6 or newer, which strips the TypeScript types so the real modules can be imported. + +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import test from 'node:test'; +// Registers the resolver for the real V2 modules; it must load before the dynamic imports below. +import './test_support/tsResolve.mjs'; + +const { resolveSendTarget, sharedConversationTarget } = await import('../application/v2_ui/src/lib/mentions.ts'); +const { mergeCollaborationMessage } = await import('../application/v2_ui/src/stores/chatStore.ts'); + +const AGENTS = [ + { id: 'agent-1', name: 'researcher', display_name: 'Research Assistant', scope_type: 'personal' }, +]; +const MODELS = [ + { selection_key: 'user::ep1::gpt-4o', display_name: 'GPT-4o', deployment_name: 'gpt-4o', endpoint_id: 'ep1' }, +]; +const CATALOGS = { agents: AGENTS, models: MODELS }; +const OPTIONS = { + agentSelection: undefined, + promptId: undefined, + documentSearch: false, + webSearch: false, + imageGeneration: false, + deepResearch: false, + urlAccess: false, + modelDeployment: 'user::ep1::gpt-4o', +}; + +const source = (path) => readFileSync(new URL(`../application/v2_ui/src/${path}`, import.meta.url), 'utf8'); + +test('a message that addresses only people never reaches a model or a plan', () => { + assert.equal(sharedConversationTarget('@Ada Lovelace can you check this?', OPTIONS, CATALOGS), null); + assert.equal(sharedConversationTarget('Thanks, everyone.', OPTIONS, CATALOGS), null); +}); + +test('an agent or model tag addresses the assistant exactly as the manual send rule does', () => { + const message = '@Research Assistant summarize the thread'; + const target = sharedConversationTarget(message, OPTIONS, CATALOGS); + + assert.equal(target?.target_type, 'agent'); + assert.equal(target?.source_mode, 'explicit_tag'); + assert.deepEqual(target, resolveSendTarget(message, OPTIONS, CATALOGS)); + assert.equal(sharedConversationTarget('@GPT-4o summarize the thread', OPTIONS, CATALOGS)?.target_type, 'model'); +}); + +test('a source toggle addresses the assistant unless a saved analysis is being asked about', () => { + const searching = { ...OPTIONS, webSearch: true }; + + assert.equal(sharedConversationTarget('What changed this week?', searching, CATALOGS)?.source_mode, 'web_search'); + assert.equal( + sharedConversationTarget('What changed this week?', searching, CATALOGS, { savedContext: true }), + null, + ); + // A selected agent still addresses the assistant while a saved analysis is open. + assert.equal( + sharedConversationTarget('Explain it', { ...OPTIONS, agentSelection: 'agent-key' }, CATALOGS, { + savedContext: true, + })?.target_type, + 'agent', + ); +}); + +test("the shared copy of an orchestrated question replaces the run's own bubble", () => { + const thread = [{ id: 'user_1', role: 'user', content: 'Draft the note', metadata: {} }]; + const copy = { + id: 'collab-1', role: 'user', content: 'Draft the note', + metadata: { source_message_id: 'user_1' }, sender: { user_id: 'owner' }, + }; + const merged = mergeCollaborationMessage(thread, copy, { currentUserId: 'owner' }); + + assert.equal(merged.length, 1); + assert.equal(merged[0].id, 'collab-1'); +}); + +test("a run's answer that lands after its shared copy updates the copy instead of doubling it", () => { + const thread = [{ + id: 'collab-2', role: 'assistant', content: 'Here is the note.', + metadata: { source_message_id: 'assistant_1' }, + }]; + const answer = { + id: 'assistant_1', role: 'assistant', content: 'Here is the note.', + metadata: { orchestration: { run_id: 'run-1' } }, + }; + const merged = mergeCollaborationMessage(thread, answer); + + assert.equal(merged.length, 1); + assert.equal(merged[0].metadata.orchestration.run_id, 'run-1'); + // The copy arriving again, as an event replay would deliver it, still leaves one message. + assert.equal(mergeCollaborationMessage(merged, thread[0]).length, 1); +}); + +test('unrelated messages are still appended', () => { + const thread = [{ id: 'a', role: 'user', content: 'one', metadata: {} }]; + const merged = mergeCollaborationMessage(thread, { id: 'b', role: 'user', content: 'two', metadata: {} }); + + assert.deepEqual(merged.map((message) => message.id), ['a', 'b']); +}); + +test('the composer applies the shared send rule before planning and plans only for the owner', () => { + const composer = source('components/chat/Composer.tsx'); + const helper = composer.slice( + composer.indexOf('const sharedOrchestrationRequest = '), + composer.indexOf('const dispatchOrchestration = ('), + ); + + assert.match( + composer, + /const sharedRequest = orchestrating && shared \? sharedOrchestrationRequest\(outgoing\.message\) : null;\s*if \(orchestrating && shared && !sharedRequest\) \{\s*dispatch\(outgoing\);\s*return;/, + ); + assert.match(composer, /dispatchOrchestration\(outgoing\.message, outgoing\.promptInfo, sharedRequest\);/); + assert.match(helper, /sharedConversationTarget\(message, options,/); + assert.match(helper, /if \(!target \|\| !sharedOrchestrationAllowed\) \{\s*return null;/); + assert.match(helper, /invocation_target: target/); + assert.match(helper, /mentioned_participants: extractMentionedParticipants\(/); + assert.match(composer, /collaboration\.user_id === bootstrap\.user\.id/); + // An explicit tag seeds the plan's agent or model in personal and shared conversations alike. + assert.match(composer, /const tagged = resolveInvocationTarget\(/); + assert.match(composer, /agentSelection: taggedAgent \?\? \(taggedModel \? undefined : options\.agentSelection\)/); +}); + +test('manual sends and Orchestrate share one rule', () => { + const store = source('stores/chatStore.ts'); + + assert.match(store, /\? sharedConversationTarget\(\s*trimmed,\s*options,/); + assert.doesNotMatch(store, /resolveSendTarget\(/); +}); diff --git a/ui_tests/test_v2_shared_orchestration_routing.py b/ui_tests/test_v2_shared_orchestration_routing.py new file mode 100644 index 000000000..c60bc41e5 --- /dev/null +++ b/ui_tests/test_v2_shared_orchestration_routing.py @@ -0,0 +1,273 @@ +# test_v2_shared_orchestration_routing.py +""" +UI test for Orchestrate in V2 shared conversations. +Version: 0.261.269 +Implemented in: 0.261.269 (Refs #1659) + +With Orchestrate on, a shared conversation used to send every message to the planner, so a remark +addressed only to another person was answered by the model. The composer now applies the shared +conversation's send rule first, the same rule a manual send applies. + +This test drives the REAL Composer, chat store and orchestration controller over a seeded +bootstrap, with the browser's fetch replaced, and asserts: + + * "@Ada ..." from the person who started the conversation is posted to the participants: one + message post with the mention, and no plan request. + * "@Research Assistant ..." from that person starts a plan whose request carries the AI target, + the tagged agent and the mentions, and posts no message. + * The same request from another participant is answered the classic way: no plan request, and + one shared assistant stream. + +The browser checks are skipped (reported, not failed) when node_modules is absent. + +Run: python ui_tests/test_v2_shared_orchestration_routing.py +""" + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent / "fixtures" / "orchestration")) +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "functional_tests")) + +import harness_build as hb # noqa: E402 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +pytestmark = pytest.mark.ui + +IMPLEMENTED_IN = "0.261.269" +CONVERSATION = "shared-1" +OWNER = {"user_id": "owner", "display_name": "Owner Person", "email": "owner@example.test"} +ADA = {"user_id": "ada", "display_name": "Ada Lovelace", "email": "ada@example.test"} +AGENTS = [{"id": "agent-1", "name": "researcher", "display_name": "Research Assistant", "scope_type": "personal"}] +MODELS = [{ + "selection_key": "user::ep1::gpt-4o", "display_name": "GPT-4o", + "deployment_name": "gpt-4o", "endpoint_id": "ep1", +}] + +_PAGE = None + +# Seed a shared conversation, replace fetch with a recorder that answers like the server, and mount +# the real Composer. +_SEED = r""" +(spec) => { + const H = window.OrchHarness; + H.reset(); + window.__requests = []; + const encoder = new TextEncoder(); + const json = (value) => new Response(JSON.stringify(value), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + const sse = (frames) => new Response(new ReadableStream({ + start(controller) { + for (const frame of frames) { + controller.enqueue(encoder.encode('data: ' + JSON.stringify(frame) + '\n\n')); + } + controller.close(); + }, + }), { status: 200, headers: { 'Content-Type': 'text/event-stream' } }); + const conversationPath = `/api/collaboration/conversations/${spec.conversation}`; + window.fetch = (url, options = {}) => { + const path = new URL(String(url), window.location.origin).pathname; + const method = String(options.method || 'GET').toUpperCase(); + let body = null; + try { + body = typeof options.body === 'string' ? JSON.parse(options.body) : null; + } catch { + body = null; + } + window.__requests.push({ path, method, body }); + if (path.endsWith('/api/v2/orchestration/plan')) { + return Promise.resolve(sse([{ conversation_id: spec.conversation, error: 'Stopped by the test.' }])); + } + if (path === `${conversationPath}/stream`) { + return Promise.resolve(sse([{ done: true, message_id: 'assistant-1', full_content: 'A classic answer.' }])); + } + if (path === `${conversationPath}/messages` && method === 'POST') { + return Promise.resolve(json({ + conversation: spec.collaboration, + message: { + id: 'posted-1', conversation_id: spec.conversation, role: 'user', + content: body?.content ?? '', metadata: {}, sender: { user_id: spec.user.id }, + }, + })); + } + return Promise.resolve(json({})); + }; + H.stores.bootstrap.useBootstrapStore.setState({ + data: { + features: { enable_chat_orchestration: true, enable_collaborative_conversations: true }, + orchestration: { + enabled: true, show_manual_controls: true, default_approval_mode: 'manual', + allow_user_approval_override: true, timed_approval_seconds: 8, + }, + settings: {}, + catalogs: { prompts: [], models: spec.models, agents: spec.agents }, + user: spec.user, + }, + }); + H.stores.chat.useChatStore.setState({ + activeConversationId: spec.conversation, activeConversationKind: 'collaborative', messages: [], + }); + H.stores.collaboration.useCollaborationStore.setState({ conversation: spec.collaboration, replyTo: null }); + H.mount('mount-a', 'Composer', {}); +} +""" + + +def _collaboration(started_by): + return { + "id": CONVERSATION, "title": "Launch plan", "chat_type": "personal_multi_user", + "conversation_kind": "collaborative", "user_id": started_by, + "participants": [ + {**OWNER, "role": "owner", "status": "accepted"}, + {**ADA, "role": "member", "status": "accepted"}, + ], + "membership_status": "accepted", "can_post_messages": True, "can_accept_invite": False, + } + + +def _seed(page, *, reader, started_by="owner"): + page.evaluate(_SEED, { + "conversation": CONVERSATION, + "collaboration": _collaboration(started_by), + "user": {"id": reader["user_id"], "display_name": reader["display_name"], "email": reader["email"]}, + "agents": AGENTS, + "models": MODELS, + }) + orchestrate = page.locator('#mount-a [title="Orchestrate"]') + orchestrate.wait_for(state="visible", timeout=5000) + assert orchestrate.get_attribute("aria-pressed") == "true", "Orchestrate must be on for these checks" + + +_SENT = "(request) => request.method === 'POST' && !request.path.endsWith('/typing')" + + +def _send(page, text): + """Send ``text`` and return the POSTs it made, leaving out typing indicators.""" + page.get_by_role("textbox", name="Message", exact=True).fill(text) + page.get_by_role("button", name="Send to this conversation", exact=True).click() + page.wait_for_function(f"() => window.__requests.some({_SENT})", timeout=5000) + page.wait_for_timeout(150) + return page.evaluate(f"() => window.__requests.filter({_SENT})") + + +def _paths(requests): + return [request["path"] for request in requests] + + +def test_version_is_at_least_the_implementing_release(): + """Shared Orchestrate routing shipped in IMPLEMENTED_IN, so the app must be at least it.""" + print("Testing the app version is at least the implementing release...") + try: + assert_app_version_at_least(IMPLEMENTED_IN) + print(f" ok config.py VERSION is at least {IMPLEMENTED_IN}") + return True + except Exception as exc: # noqa: BLE001 + print(f"Test failed: {exc}") + return False + + +def test_a_message_to_a_person_is_posted_without_a_plan(): + """'@Ada ...' with Orchestrate on reaches Ada, never the model.""" + print("Testing a message to a person is posted without a plan...") + page = _PAGE + try: + _seed(page, reader=OWNER) + requests = _send(page, "@Ada Lovelace can you check the launch dates?") + + assert _paths(requests) == [f"/api/collaboration/conversations/{CONVERSATION}/messages"], requests + post = requests[0]["body"] + assert post["content"] == "@Ada Lovelace can you check the launch dates?" + assert [person["user_id"] for person in post["mentioned_participants"]] == ["ada"] + print(" ok the message was posted to the participants and no plan was requested") + return True + except Exception as exc: # noqa: BLE001 + print(f"Test failed: {exc}") + import traceback + + traceback.print_exc() + return False + + +def test_a_tagged_agent_is_planned_for_the_person_who_started_the_conversation(): + """'@Research Assistant ...' starts a plan that uses that agent and says who was addressed.""" + print("Testing a tagged agent is planned for the conversation's owner...") + page = _PAGE + try: + _seed(page, reader=OWNER) + requests = _send(page, "@Research Assistant summarize what @Ada Lovelace decided") + + assert _paths(requests) == ["/api/v2/orchestration/plan"], requests + plan = requests[0]["body"] + assert plan["conversation_id"] == CONVERSATION + assert plan["invocation_target"]["target_type"] == "agent" + assert plan["invocation_target"]["display_name"] == "Research Assistant" + assert plan["agent_info"]["name"] == "researcher", plan + assert [person["user_id"] for person in plan["mentioned_participants"]] == ["ada"] + print(" ok the plan request carries the agent, the target and the mentions") + return True + except Exception as exc: # noqa: BLE001 + print(f"Test failed: {exc}") + import traceback + + traceback.print_exc() + return False + + +def test_another_participant_is_answered_the_classic_way(): + """Only the person who started the conversation plans in it; others get a classic answer.""" + print("Testing another participant's request is answered without a plan...") + page = _PAGE + try: + _seed(page, reader=ADA, started_by="owner") + requests = _send(page, "@Research Assistant summarize the launch plan") + + assert "/api/v2/orchestration/plan" not in _paths(requests), requests + assert _paths(requests) == [f"/api/collaboration/conversations/{CONVERSATION}/stream"], requests + print(" ok the request went to the shared assistant stream, not the planner") + return True + except Exception as exc: # noqa: BLE001 + print(f"Test failed: {exc}") + import traceback + + traceback.print_exc() + return False + + +PAGE_TESTS = [ + test_a_message_to_a_person_is_posted_without_a_plan, + test_a_tagged_agent_is_planned_for_the_person_who_started_the_conversation, + test_another_participant_is_answered_the_classic_way, +] + + +def main(): + results = [test_version_is_at_least_the_implementing_release()] + + errors = [] + try: + with hb.harness_page(collect_errors=errors) as page: + global _PAGE + _PAGE = page + for test in PAGE_TESTS: + print(f"\nRunning {test.__name__}...") + page.evaluate("() => window.OrchHarness.reset()") + results.append(test()) + except hb.HarnessUnavailable as exc: + print(f"\n -- skipped the browser-driven checks: {exc}") + results.extend([True] * len(PAGE_TESTS)) + + if errors: + print("\nUncaught page errors observed during the run:") + for message in errors: + print(f" !! {message}") + + print(f"\nResults: {sum(results)}/{len(results)} tests passed") + return all(results) + + +if __name__ == "__main__": + sys.exit(0 if main() else 1) From 173273798e14220a5a261612f6ddd2104fa5b5b5 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Tue, 6 Oct 2026 20:25:51 -0400 Subject: [PATCH 3/4] Address CodeQL and access-control findings Use the personal conversation authorization helper for summary reads, clarify collaboration message return shapes, and remove CodeQL error/notice patterns from shared Orchestrate tests and helpers. Refresh affected isolated tests so they cover the new route dependencies and version policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../single_app/functions_collaboration.py | 6 +- .../functions_orchestration_collaboration.py | 13 +++- .../single_app/route_backend_conversations.py | 26 +++---- ...oration_conversation_summary_export_fix.py | 7 +- ...versations_read_ownership_authorization.py | 75 +++++++++++++++++++ functional_tests/test_m365_provider_core.py | 12 +-- ...st_orchestration_shared_backing_lookups.py | 5 +- ...test_orchestration_shared_conversations.py | 6 +- 8 files changed, 115 insertions(+), 35 deletions(-) diff --git a/application/single_app/functions_collaboration.py b/application/single_app/functions_collaboration.py index 7a6a93424..5e654ef0a 100644 --- a/application/single_app/functions_collaboration.py +++ b/application/single_app/functions_collaboration.py @@ -2239,7 +2239,11 @@ def mirror_source_message_to_collaboration( if source_role == 'image': message_metadata['last_message_preview'] = '[Uploaded image]' if bool(source_metadata.get('is_user_upload')) else '[Generated image]' - return (*_save_collaboration_message_doc(conversation_doc, collaboration_message), True) + saved_message, saved_conversation = _save_collaboration_message_doc( + conversation_doc, + collaboration_message, + ) + return saved_message, saved_conversation, True def _refresh_collaboration_conversation_message_summary(conversation_doc): diff --git a/application/single_app/functions_orchestration_collaboration.py b/application/single_app/functions_orchestration_collaboration.py index 35fed7bf3..9613e7567 100644 --- a/application/single_app/functions_orchestration_collaboration.py +++ b/application/single_app/functions_orchestration_collaboration.py @@ -217,8 +217,12 @@ def mirror_orchestration_answer(conversation, answer, *, user_message_id=None, o def _refresh_mirrored_answer(collaboration, existing, answer, owner): """Bring an existing shared copy up to date with a republished answer, and announce it.""" from azure.core import MatchConditions - import functions_collaboration as collaboration_store from collaboration_models import build_collaboration_message_doc_from_legacy, utc_now_iso + from functions_collaboration import ( + cosmos_collaboration_messages_container, + publish_collaboration_event, + serialize_collaboration_message, + ) from functions_workflow_result_masking import ( message_uses_workflow_result, withhold_workflow_result_message, @@ -248,15 +252,15 @@ def visible(message): if visible(refreshed) == visible(existing): return existing - saved = collaboration_store.cosmos_collaboration_messages_container.replace_item( + saved = cosmos_collaboration_messages_container.replace_item( item=existing['id'], body=refreshed, etag=existing.get('_etag'), match_condition=MatchConditions.IfNotModified, ) - collaboration_store.publish_collaboration_event(collaboration['id'], { + publish_collaboration_event(collaboration['id'], { 'conversation_id': collaboration['id'], 'event_type': 'collaboration.message.updated', 'occurred_at': utc_now_iso(), - 'payload': {'message': collaboration_store.serialize_collaboration_message(saved)}, + 'payload': {'message': serialize_collaboration_message(saved)}, }) return saved @@ -341,6 +345,7 @@ def authorize(): try: message_container.delete_item(message['id'], partition_key=conversation_id) except not_found_error: + # Another deletion path may have removed it between the query and delete. pass if archiving_enabled: archive_thoughts_for_conversation(conversation_id, owner) diff --git a/application/single_app/route_backend_conversations.py b/application/single_app/route_backend_conversations.py index d543af51a..344bb1648 100644 --- a/application/single_app/route_backend_conversations.py +++ b/application/single_app/route_backend_conversations.py @@ -2452,23 +2452,10 @@ def generate_conversation_summary_api(conversation_id): is_collaboration_summary = False try: - personal_item = cosmos_conversations_container.read_item( - item=conversation_id, - partition_key=conversation_id - ) - except CosmosResourceNotFoundError: - personal_item = None - except Exception as e: - debug_print(f"Error reading conversation for summary: {e}") - return jsonify({'error': 'Failed to read conversation'}), 500 - - # Orchestrate's backing record shares its shared conversation's id; the summary is the - # shared conversation's. - if personal_item is not None and not is_shared_conversation_backing(personal_item): - if personal_item.get('user_id') != user_id: - return jsonify({'error': 'Forbidden'}), 403 - conversation_item = personal_item - else: + conversation_item = _authorize_personal_conversation_read(user_id, conversation_id) + except LookupError: + # Orchestrate's backing record shares its shared conversation's id; the summary is + # the shared conversation's. try: conversation_item = get_collaboration_conversation(conversation_id) assert_user_can_view_collaboration_conversation( @@ -2484,6 +2471,11 @@ def generate_conversation_summary_api(conversation_id): except Exception as e: debug_print(f"Error reading collaborative conversation for summary: {e}") return jsonify({'error': 'Failed to read conversation'}), 500 + except PermissionError: + return jsonify({'error': 'Forbidden'}), 403 + except Exception as e: + debug_print(f"Error reading conversation for summary: {e}") + return jsonify({'error': 'Failed to read conversation'}), 500 body = request.get_json(silent=True) or {} model_deployment = body.get('model_deployment', '') diff --git a/functional_tests/test_collaboration_conversation_summary_export_fix.py b/functional_tests/test_collaboration_conversation_summary_export_fix.py index b496676b3..97eaf2961 100644 --- a/functional_tests/test_collaboration_conversation_summary_export_fix.py +++ b/functional_tests/test_collaboration_conversation_summary_export_fix.py @@ -24,6 +24,8 @@ if APP_ROOT not in sys.path: sys.path.insert(0, APP_ROOT) +from test_support.versioning import assert_app_version_at_least # noqa: E402 + CONFIG_FILE = os.path.join(APP_ROOT, 'config.py') FUNCTIONS_CONVERSATION_METADATA_FILE = os.path.join(APP_ROOT, 'functions_conversation_metadata.py') @@ -103,6 +105,7 @@ def _load_metadata_helpers(): 'cosmos_conversations_container': None, 'cosmos_collaboration_conversations_container': None, 'get_collaboration_conversation': None, + 'invalidate_conversation_cache_for_item': lambda *args, **kwargs: None, } exec(compile(helper_module, FUNCTIONS_CONVERSATION_METADATA_FILE, 'exec'), namespace) return namespace @@ -191,11 +194,9 @@ def test_version_and_fix_documentation_alignment(): """Config version and fix documentation must stay aligned for this fix.""" print('Testing version and fix documentation alignment...') - version = _read_version() fix_doc_source = _read(FIX_DOC_FILE) - if version != '0.241.074': - raise AssertionError(f'Expected config VERSION to be 0.241.074, found {version}') + assert_app_version_at_least('0.241.074') if 'Fixed/Implemented in version: **0.241.074**' not in fix_doc_source: raise AssertionError('Fix documentation is missing the current version header.') if '/api/conversations//summary' not in fix_doc_source: diff --git a/functional_tests/test_conversations_read_ownership_authorization.py b/functional_tests/test_conversations_read_ownership_authorization.py index 497d81234..a641d36a1 100644 --- a/functional_tests/test_conversations_read_ownership_authorization.py +++ b/functional_tests/test_conversations_read_ownership_authorization.py @@ -139,6 +139,7 @@ def _install_route_import_stubs(): config_module.cosmos_messages_container = None config_module.CosmosResourceNotFoundError = DummyNotFoundError config_module.CLIENTS = {} + config_module.SECRET_KEY = 'test-secret-key' stub_modules['config'] = config_module appinsights_module = types.ModuleType('functions_appinsights') @@ -218,6 +219,73 @@ def clear_conversation_unread(item): notifications_module.mark_chat_response_notifications_read_for_conversation = lambda *args, **kwargs: None stub_modules['functions_notifications'] = notifications_module + pending_actions_module = types.ModuleType('functions_msgraph_pending_actions') + pending_actions_module.hydrate_m365_pending_action_cards = lambda messages, *args, **kwargs: messages + stub_modules['functions_msgraph_pending_actions'] = pending_actions_module + + azure_maps_module = types.ModuleType('functions_azure_maps') + azure_maps_module.refresh_azure_maps_message_citations = lambda messages, *args, **kwargs: messages + stub_modules['functions_azure_maps'] = azure_maps_module + + orchestration_recovery_module = types.ModuleType('functions_orchestration_recovery') + orchestration_recovery_module.cleanup_conversation_checkpoints = lambda *args, **kwargs: None + stub_modules['functions_orchestration_recovery'] = orchestration_recovery_module + + orchestration_artifacts_module = types.ModuleType('functions_orchestration_artifacts') + orchestration_artifacts_module.ORCHESTRATION_ARTIFACT_KIND = 'orchestration_output' + orchestration_artifacts_module.ORCHESTRATION_ARTIFACT_KEY_PREFIX = 'orchestration-output:v1:' + orchestration_artifacts_module.authorize_orchestration_output_artifact = lambda *args, **kwargs: None + orchestration_artifacts_module.is_orchestration_artifact_source = lambda *args, **kwargs: False + orchestration_artifacts_module.is_retained_orchestration_file = lambda *args, **kwargs: False + orchestration_artifacts_module.load_orchestration_artifact_binding = lambda *args, **kwargs: None + orchestration_artifacts_module.load_orchestration_output_history = lambda *args, **kwargs: [] + orchestration_artifacts_module.validate_orchestration_artifact_binding = lambda *args, **kwargs: None + stub_modules['functions_orchestration_artifacts'] = orchestration_artifacts_module + + orchestration_external_configuration_module = types.ModuleType('functions_orchestration_external_configuration') + orchestration_external_configuration_module.ExternalConfigurationServiceError = RuntimeError + stub_modules['functions_orchestration_external_configuration'] = orchestration_external_configuration_module + + orchestration_external_identity_module = types.ModuleType('functions_orchestration_external_identity') + orchestration_external_identity_module.ExternalIdentityServiceError = RuntimeError + stub_modules['functions_orchestration_external_identity'] = orchestration_external_identity_module + + orchestration_output_store_module = types.ModuleType('functions_orchestration_output_store') + orchestration_output_store_module.OutputConflictError = RuntimeError + orchestration_output_store_module.OutputError = RuntimeError + orchestration_output_store_module.OutputStorageError = RuntimeError + orchestration_output_store_module.OutputUnavailableError = RuntimeError + stub_modules['functions_orchestration_output_store'] = orchestration_output_store_module + + orchestration_result_contracts_module = types.ModuleType('functions_orchestration_result_contracts') + orchestration_result_contracts_module.RESULT_MANIFEST_VERSION = 1 + orchestration_result_contracts_module.RESULT_RECEIPT_VERSION = 1 + orchestration_result_contracts_module.ResultContractError = RuntimeError + orchestration_result_contracts_module.canonical_digest = lambda *args, **kwargs: 'digest' + orchestration_result_contracts_module.result_receipt_binding = lambda *args, **kwargs: {} + stub_modules['functions_orchestration_result_contracts'] = orchestration_result_contracts_module + + workflow_result_masking_module = types.ModuleType('functions_workflow_result_masking') + workflow_result_masking_module.message_asks_about_workflow_result = lambda *args, **kwargs: False + workflow_result_masking_module.message_uses_workflow_result = lambda *args, **kwargs: False + stub_modules['functions_workflow_result_masking'] = workflow_result_masking_module + + workflow_result_reader_module = types.ModuleType('functions_workflow_result_reader') + workflow_result_reader_module.WorkflowResultUnavailable = RuntimeError + workflow_result_reader_module.workflow_result_error_payload = ( + lambda *args, **kwargs: ({'error': 'workflow_result_unavailable'}, 400) + ) + stub_modules['functions_workflow_result_reader'] = workflow_result_reader_module + + workflow_chat_delivery_module = types.ModuleType('functions_workflow_chat_delivery') + workflow_chat_delivery_module.DELIVERY_EDIT_UNSUPPORTED = 'delivery_edit_unsupported' + workflow_chat_delivery_module.DELIVERY_RETRY_UNSUPPORTED = 'delivery_retry_unsupported' + workflow_chat_delivery_module.is_workflow_delivery_message = lambda *args, **kwargs: False + workflow_chat_delivery_module.workflow_delivery_refusal_payload = ( + lambda *args, **kwargs: ({'error': 'workflow_delivery_unavailable'}, 400) + ) + stub_modules['functions_workflow_chat_delivery'] = workflow_chat_delivery_module + debug_module = types.ModuleType('functions_debug') debug_module.debug_print = lambda *args, **kwargs: None stub_modules['functions_debug'] = debug_module @@ -245,6 +313,13 @@ def clear_conversation_unread(item): simplechat_operations_module.fork_personal_conversation_for_user = lambda *args, **kwargs: {} stub_modules['functions_simplechat_operations'] = simplechat_operations_module + saved_analysis_module = types.ModuleType('functions_saved_analysis') + saved_analysis_module.cleanup_chat_analysis_conversation = lambda *args, **kwargs: None + saved_analysis_module.cleanup_chat_analysis_messages = lambda *args, **kwargs: None + saved_analysis_module.is_saved_analysis_unavailable = lambda *args, **kwargs: False + saved_analysis_module.sanitize_saved_analysis_messages = lambda messages, *args, **kwargs: messages + stub_modules['functions_saved_analysis'] = saved_analysis_module + swagger_module = types.ModuleType('swagger_wrapper') swagger_module.swagger_route = lambda **kwargs: (lambda func: func) swagger_module.get_auth_security = lambda: {} diff --git a/functional_tests/test_m365_provider_core.py b/functional_tests/test_m365_provider_core.py index a40ac1adb..19131d549 100644 --- a/functional_tests/test_m365_provider_core.py +++ b/functional_tests/test_m365_provider_core.py @@ -1282,7 +1282,7 @@ def test_snapshot_only_action_publishes_a_prior_capture_without_remote_access( execution, memory_runtime, real_content_helpers, monkeypatch, ): from dataclasses import replace - import functions_m365_approvals as approval_module + from functions_m365_approvals import M365ApprovalService import functions_m365_execution as execution_module from test_m365_runtime_adapters import load_module, module_stub from test_support.m365 import CosmosContainer, Notifications @@ -1300,7 +1300,7 @@ def test_snapshot_only_action_publishes_a_prior_capture_without_remote_access( shared=True, audience_version="audience-1", action_configs={actions.action_id: manifest}, ) container = CosmosContainer() - service = approval_module.M365ApprovalService( + service = M365ApprovalService( container_factory=lambda: container, notification_sender=Notifications(), decision_validator=lambda approval: True, ) @@ -1314,7 +1314,7 @@ def test_snapshot_only_action_publishes_a_prior_capture_without_remote_access( "functions_settings": module_stub("functions_settings", get_settings=lambda: {}), }) monkeypatch.setattr(memory_runtime.store, "authorize_publish", runtime._authorize_memory_publication) - monkeypatch.setattr(approval_module, "_service", service) + monkeypatch.setattr("functions_m365_approvals._service", service) monkeypatch.setattr(execution_module, "_action_config_resolver", lambda *args: manifest) monkeypatch.setattr(execution_module, "_action_selection_resolver", lambda context: [actions.action_id]) for module in (retrieval, transport_module): @@ -1345,7 +1345,7 @@ def test_shared_request_consent_publishes_a_capture_without_an_approval( ): """Asking for your own file in a shared conversation shares it there; no approval waits (#1659).""" from dataclasses import replace - import functions_m365_approvals as approval_module + from functions_m365_approvals import M365ApprovalService import functions_m365_execution as execution_module from test_m365_runtime_adapters import load_module, module_stub from test_support.m365 import CosmosContainer, Notifications @@ -1365,7 +1365,7 @@ def test_shared_request_consent_publishes_a_capture_without_an_approval( ) container = CosmosContainer() notifications = Notifications() - service = approval_module.M365ApprovalService( + service = M365ApprovalService( container_factory=lambda: container, notification_sender=notifications, decision_validator=lambda approval: True, ) @@ -1379,7 +1379,7 @@ def test_shared_request_consent_publishes_a_capture_without_an_approval( "functions_settings": module_stub("functions_settings", get_settings=lambda: {}), }) monkeypatch.setattr(memory_runtime.store, "authorize_publish", runtime._authorize_memory_publication) - monkeypatch.setattr(approval_module, "_service", service) + monkeypatch.setattr("functions_m365_approvals._service", service) monkeypatch.setattr(execution_module, "_action_config_resolver", lambda *args: manifest) monkeypatch.setattr(execution_module, "_action_selection_resolver", lambda context: [actions.action_id]) for module in (retrieval, transport_module): diff --git a/functional_tests/test_orchestration_shared_backing_lookups.py b/functional_tests/test_orchestration_shared_backing_lookups.py index 12d5ecfbb..0b08e6c98 100644 --- a/functional_tests/test_orchestration_shared_backing_lookups.py +++ b/functional_tests/test_orchestration_shared_backing_lookups.py @@ -31,7 +31,7 @@ sys.path.insert(0, str(TESTS)) from test_orchestration_harness_routes import login, modules, real_http_harness # noqa: E402,F401 -from test_orchestration_shared_conversations import GUEST, OWNER, SHARED_ID, shared # noqa: E402,F401 +from test_orchestration_shared_conversations import SHARED_ID, shared # noqa: E402,F401 from test_support.versioning import assert_app_version_at_least # noqa: E402 CLASSIC_SOURCE_ID = "classic-source-1" @@ -208,7 +208,8 @@ def _function_source(path, name): ("route_frontend_conversations.py", "get_agent_citation_artifact"), ]) def test_other_personal_first_lookups_recognize_the_backing(module, function): - assert "is_shared_conversation_backing(" in _function_source(APP / module, function) + source = _function_source(APP / module, function) + assert "is_shared_conversation_backing(" in source or "_authorize_personal_conversation_read(" in source if __name__ == "__main__": diff --git a/functional_tests/test_orchestration_shared_conversations.py b/functional_tests/test_orchestration_shared_conversations.py index 5a9dc4be0..c97b9222c 100644 --- a/functional_tests/test_orchestration_shared_conversations.py +++ b/functional_tests/test_orchestration_shared_conversations.py @@ -303,7 +303,8 @@ def delete(**options): ) # A co-owner who didn't start the conversation leaves its creator's plans alone. - assert delete(expected_user_id="guest") is None + guest_delete = delete(expected_user_id="guest") + assert guest_delete is None assert shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) deleted = delete(expected_user_id="owner") @@ -316,7 +317,8 @@ def delete(**options): run = shared.harness.runs.read_item(planned["run_id"], SHARED_ID) assert run["checkpoints_deleted"] is True and run["execution_lease"] is None # Nothing is left to delete the second time. - assert delete() is None + second_delete = delete() + assert second_delete is None def test_shared_conversation_deletion_removes_the_backing_first(): From 998bae0064d44e44beec6127d706391ebae0a6d0 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Tue, 6 Oct 2026 20:39:27 -0400 Subject: [PATCH 4/4] Cover the shared Orchestrate CodeQL review findings - Capture container reads, HTTP calls and lookups before asserting in the shared Orchestrate tests, so no checked operation runs inside an assert. - Add test_orchestration_collaboration_codeql_alerts.py: the collaboration mirror helper returns three values on every path, the orchestration module uses one collaboration import style, and both collaboration modules import cold and offline in either order, in normal and optimized Python. - List the new test in the shared conversations fix doc. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md | 6 + ...chestration_collaboration_codeql_alerts.py | 128 ++++++++++++++++++ ...st_orchestration_shared_backing_lookups.py | 12 +- ...test_orchestration_shared_conversations.py | 6 +- 4 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 functional_tests/test_orchestration_collaboration_codeql_alerts.py diff --git a/docs/explanation/fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md b/docs/explanation/fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md index a73a14a7f..2916eb4f4 100644 --- a/docs/explanation/fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md +++ b/docs/explanation/fixes/ORCHESTRATION_SHARED_CONVERSATIONS_FIX.md @@ -168,6 +168,12 @@ shared conversations and uses the shared conversation's real audience. - `functional_tests/test_orchestration_m365_actions.py` replaces the shared conversation refusal with a step that reads mail for the shared audience, and a step that stops once the user is no longer a participant. +- `functional_tests/test_orchestration_collaboration_codeql_alerts.py` imports the + real collaboration modules cold in fresh processes, in both orders, in normal and + optimized Python, with network access blocked. The orchestration module imports + collaboration storage lazily, so importing it never initializes Cosmos clients. + It also checks that every return path of the collaboration mirror helper returns + three values. - `functional_tests/test_v2_shared_orchestration_routing.mjs` executes the shared send rule and the de-duplication, and checks the composer routing. - `ui_tests/test_v2_shared_orchestration_routing.py` drives the real Composer in diff --git a/functional_tests/test_orchestration_collaboration_codeql_alerts.py b/functional_tests/test_orchestration_collaboration_codeql_alerts.py new file mode 100644 index 000000000..388678d04 --- /dev/null +++ b/functional_tests/test_orchestration_collaboration_codeql_alerts.py @@ -0,0 +1,128 @@ +# test_orchestration_collaboration_codeql_alerts.py +#!/usr/bin/env python3 +""" +Functional test for shared Orchestrate CodeQL alert remediation. +Version: 0.261.269 +Implemented in: 0.261.269 + +This test ensures that the shared Orchestrate collaboration helpers keep the +CodeQL fixes for tuple arity and a single collaboration import style, and that +the real modules, whose collaboration imports are deliberately lazy, load cold +and offline in either order, in normal and optimized Python. +""" + +import ast +import subprocess +import sys +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +APP = ROOT / "application" / "single_app" +TESTS = ROOT / "functional_tests" +sys.path.insert(0, str(TESTS)) + +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +COLLABORATION = "functions_collaboration" +ORCHESTRATION_COLLABORATION = "functions_orchestration_collaboration" + + +def _tree(module): + return ast.parse((APP / f"{module}.py").read_text(encoding="utf-8-sig")) + + +def _function(tree, name): + for node in tree.body: + if isinstance(node, ast.FunctionDef) and node.name == name: + return node + raise AssertionError(f"{name} was not found") + + +def _return_tuple_lengths(function): + lengths = [] + for node in ast.walk(function): + if isinstance(node, ast.Return): + value = node.value + if isinstance(value, ast.Tuple): + lengths.append(len(value.elts)) + else: + lengths.append(None) + return lengths + + +def test_the_codeql_remediation_ships_in_this_version(): + assert_app_version_at_least("0.261.269") + + +def test_mirror_source_message_to_collaboration_returns_three_values(): + """Callers unpack (message, conversation, created) from every return path.""" + tree = _tree(COLLABORATION) + lengths = _return_tuple_lengths(_function(tree, "mirror_source_message_to_collaboration")) + + assert lengths and all(length == 3 for length in lengths), lengths + + +def test_save_collaboration_message_doc_returns_two_values(): + tree = _tree(COLLABORATION) + lengths = _return_tuple_lengths(_function(tree, "_save_collaboration_message_doc")) + + assert lengths and all(length == 2 for length in lengths), lengths + + +def test_orchestration_collaboration_uses_one_collaboration_import_style(): + tree = _tree(ORCHESTRATION_COLLABORATION) + + module_imports = [ + alias.name for node in ast.walk(tree) + if isinstance(node, ast.Import) + for alias in node.names + if alias.name == COLLABORATION + ] + top_level_from_imports = [ + node for node in tree.body + if isinstance(node, ast.ImportFrom) and node.module == COLLABORATION + ] + + assert module_imports == [] + assert top_level_from_imports == [] + + +PROBE = r''' +import importlib +import sys + +sys.path[:0] = sys.argv[1:3] +from test_support.offline_bootstrap import offline_app_imports + +with offline_app_imports() as environment: + for name in sys.argv[3].split(","): + module = importlib.import_module(name) + if name == "functions_collaboration" and not callable(getattr(module, "mirror_source_message_to_collaboration", None)): + raise AssertionError("functions_collaboration mirror helper is missing") + if name == "functions_orchestration_collaboration" and not callable(getattr(module, "delete_orchestration_backing", None)): + raise AssertionError("functions_orchestration_collaboration delete helper is missing") + if environment.network_attempts: + raise AssertionError("A cold import swallowed a network attempt") +print("PASS: shared Orchestrate collaboration cold imports") +''' + + +@pytest.mark.parametrize("optimized", [False, True], ids=["normal", "optimized"]) +@pytest.mark.parametrize("order", [ + "functions_orchestration_collaboration,functions_collaboration", + "functions_collaboration,functions_orchestration_collaboration", +], ids=["orchestration-first", "collaboration-first"]) +def test_real_modules_load_cold_offline_in_either_order(order, optimized): + command = [sys.executable, "-B", *(["-O"] if optimized else []), "-c", PROBE, str(APP), str(TESTS), order] + process = subprocess.run(command, capture_output=True, text=True, timeout=300, check=False) + + assert process.returncode == 0, process.stdout[-8000:] + process.stderr[-8000:] + assert "PASS:" in process.stdout + + +if __name__ == "__main__": + raise SystemExit(pytest.main([__file__, "-q"])) diff --git a/functional_tests/test_orchestration_shared_backing_lookups.py b/functional_tests/test_orchestration_shared_backing_lookups.py index 0b08e6c98..841aee011 100644 --- a/functional_tests/test_orchestration_shared_backing_lookups.py +++ b/functional_tests/test_orchestration_shared_backing_lookups.py @@ -117,8 +117,10 @@ def test_an_earlier_private_copy_opens_as_the_shared_conversation(conversation_r def test_personal_conversations_are_unchanged(conversation_routes): conversation_routes.harness.conversations.create_item({"id": "personal-1", "user_id": "owner", "title": "Mine"}) - assert kind(conversation_routes, "owner", "personal-1") == (200, {"conversation_id": "personal-1", "kind": "personal"}) - assert kind(conversation_routes, "guest", "personal-1")[0] == 404 + owner_result = kind(conversation_routes, "owner", "personal-1") + guest_status, _guest_body = kind(conversation_routes, "guest", "personal-1") + assert owner_result == (200, {"conversation_id": "personal-1", "kind": "personal"}) + assert guest_status == 404 def test_personal_routes_treat_the_backing_as_part_of_the_shared_conversation(conversation_routes): @@ -189,9 +191,11 @@ def test_microsoft_365_cards_keep_resolving_to_the_classic_source(backed, monkey monkeypatch.setattr(runtime, "cosmos_conversations_container", backed.harness.conversations) monkeypatch.setattr(runtime, "assert_user_can_participate_in_collaboration_conversation", view) for user_id in ("owner", "guest"): - assert runtime.resolve_m365_audit_conversation_id(user_id, SHARED_ID) == CLASSIC_SOURCE_ID + resolved_conversation_id = runtime.resolve_m365_audit_conversation_id(user_id, SHARED_ID) + assert resolved_conversation_id == CLASSIC_SOURCE_ID backed.harness.conversations.create_item({"id": "personal-2", "user_id": "owner"}) - assert runtime.resolve_m365_audit_conversation_id("owner", "personal-2") == "personal-2" + resolved_personal_id = runtime.resolve_m365_audit_conversation_id("owner", "personal-2") + assert resolved_personal_id == "personal-2" def _function_source(path, name): diff --git a/functional_tests/test_orchestration_shared_conversations.py b/functional_tests/test_orchestration_shared_conversations.py index c97b9222c..6edd5865f 100644 --- a/functional_tests/test_orchestration_shared_conversations.py +++ b/functional_tests/test_orchestration_shared_conversations.py @@ -232,7 +232,8 @@ def test_another_users_private_copy_is_never_adopted(shared): mirror = importlib.import_module("functions_orchestration_collaboration") # The person who started the conversation learns why, and how to continue. assert planning[-1].get("error") == mirror.SHARED_ORCHESTRATION_STALE_COPY - assert shared.harness.conversations.read_item(SHARED_ID, SHARED_ID)["user_id"] == "guest" + guest_backing = shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + assert guest_backing["user_id"] == "guest" assert shared_thread(shared) == [] @@ -305,7 +306,8 @@ def delete(**options): # A co-owner who didn't start the conversation leaves its creator's plans alone. guest_delete = delete(expected_user_id="guest") assert guest_delete is None - assert shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + remaining_backing = shared.harness.conversations.read_item(SHARED_ID, SHARED_ID) + assert remaining_backing deleted = delete(expected_user_id="owner") assert deleted["id"] == SHARED_ID