diff --git a/application/single_app/config.py b/application/single_app/config.py index 30c1c5e92..163df3cd3 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.346" +VERSION = "0.261.347" 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_search_service.py b/application/single_app/functions_search_service.py index f3d043525..33f344c42 100644 --- a/application/single_app/functions_search_service.py +++ b/application/single_app/functions_search_service.py @@ -1053,13 +1053,12 @@ def get_document_chunks_payload( return { "document": _serialize_document(document_context.get("document"), document_context.get("scope")), "scope": document_context.get("scope"), - "scope_id": ( - document_context.get("conversation_id") - or document_context.get("document", {}).get("conversation_id") - or document_context.get("public_workspace_id") - or document_context.get("group_id") - or document_context.get("document", {}).get("user_id") - ), + "scope_id": { + "personal": document_context["document"].get("user_id"), + "group": document_context.get("group_id"), + "public": document_context.get("public_workspace_id"), + "chat": document_context.get("conversation_id") or document_context["document"].get("conversation_id"), + }[document_context["scope"]], "conversation_id": document_context.get("conversation_id") or document_context.get("document", {}).get("conversation_id"), "chunk_count": len(chunks), "returned_chunk_count": len(selected_chunks), diff --git a/docs/explanation/features/ORCHESTRATION_GATHER_REASON_RENDER.md b/docs/explanation/features/ORCHESTRATION_GATHER_REASON_RENDER.md index 868a63843..34a48db09 100644 --- a/docs/explanation/features/ORCHESTRATION_GATHER_REASON_RENDER.md +++ b/docs/explanation/features/ORCHESTRATION_GATHER_REASON_RENDER.md @@ -1,6 +1,12 @@ # Gather / Reason / Render orchestration -**Version: 0.261.341** +**Version: 0.261.347** + +Chat-associated workspace source identity fixed in version: **0.261.347**, +recorded in `application/single_app/config.py`. Analyze and Compare preserve +the authorized workspace identity when a document also records its upload +conversation. See the +[source-identity fix](../fixes/DOCUMENT_SOURCE_SCOPE_IDENTITY_FIX.md). Search/result integrity fixed in version: **0.261.341**, recorded in `application/single_app/config.py`. Search retains authoritative returned @@ -134,6 +140,15 @@ resolver defaults. Headless owners that catch an error must keep the strict source-authority scope around the complete source/model decision so a fallback cannot consume unverified data. +Document payloads identify personal sources by the document owner, group sources +by the authorized group access path, public sources by the authorized public +workspace, and chat-only uploads by the authorized conversation. A workspace +document's `conversation_id` remains an association, not a replacement for its +`scope_id`. Approved shared-group access can use a different group from the +owning group recorded in provenance; neither identity overwrites the other. +Analysis manifest matching, comparison evidence checks, and retained-result +consumers continue to enforce current access, screening holds, and revisions. + The same root supplies the server-only `native_bridge_for_step` callable to runtime binding and capability discovery. The default factory validates the explicit native arguments and binds the real compute-only request builder with diff --git a/docs/explanation/fixes/DOCUMENT_SOURCE_SCOPE_IDENTITY_FIX.md b/docs/explanation/fixes/DOCUMENT_SOURCE_SCOPE_IDENTITY_FIX.md new file mode 100644 index 000000000..1ba8cdcec --- /dev/null +++ b/docs/explanation/fixes/DOCUMENT_SOURCE_SCOPE_IDENTITY_FIX.md @@ -0,0 +1,116 @@ +# Document Source Scope Identity Fix + +**Version: 0.261.347** + +Fixed/Implemented in version: **0.261.347**, recorded in +`application/single_app/config.py`. + +Refs [#1777](https://github.com/microsoft/simplechat/issues/1777). + +## Issue and root cause + +FED-04 uploaded two policy revisions and approved a document comparison followed +by composition. Both source analyses succeeded, but comparison failed and its +dependent composition was skipped. Application Insights showed a source +authorization failure before the pairwise comparison model call, not a provider +timeout or a failed UI selector. + +The shared `get_document_chunks_payload()` function selected `scope_id` using a +conversation-first fallback. Workspace uploads legitimately retain a +`conversation_id`, so their payload could identify a group, public, or personal +source using its upload conversation instead of its authorized workspace +identity. Coverage provenance could remain correct while other result entries +carried the wrong identity. Comparison's recursive evidence checks then refused +those entries. Explicit Analyze manifest matching could reject the same mismatch. + +## Changes and impact + +`application/single_app/functions_search_service.py` now selects `scope_id` +from the already resolved source scope: + +| Resolved scope | Payload identity | +| --- | --- | +| Personal | Document owner's user ID, including approved personal shares | +| Group | Authorized group access path, including approved shared-group access | +| Public | Authorized public workspace ID | +| Chat-only upload | Authorized conversation ID | + +The separate conversation association remains available. Owning provenance is +not rewritten to match a shared access path. This fixes the shared retrieval +contract used by Analyze and Compare rather than special-casing FED-04, its +filenames, prompts, or policy values. + +No authorization, screening, revision validation, result-completeness rule, +provider selection, automatic retry, or UI acceptance criterion is relaxed. +Failed comparison still skips its required dependent composition. No new +capability, setting, route, file output, or storage migration is introduced. + +## Regression coverage + +`functional_tests/test_document_source_scope_identity.py` exercises real +retrieval, source authorization, Analyze, Compare, retained results, and +dependent composition. External storage transport, membership/directory I/O, +document chunks, and model responses are isolated. Point-document containers +reuse the existing SDK-shaped Cosmos double. + +Coverage includes personal, group, and public sources with and without chat +associations; approved shares; chat-only attachments and foreign-user denial; +manifest mismatch; and revocation, deletion, screening holds, and revision +changes after comparison model work. The executor cases require both comparison +and composition to complete, retain the full comparison, preserve both source +markers, and create no artifacts. A provider-failure control still requires +Failed/Skipped, not fabricated success. + +Synthetic policy values exercise 125000 to 150000 and 10 to 7 days, with both +markers reaching the producer and its complete report reaching composition. +Deterministic model replies validate orchestration plumbing, not the quality +of a live provider's arithmetic or citations. Synthetic owner and marker names +are not asserted to be the original live fixture values. + +## Validation and limitations + +Run the new regressions with application and functional-test directories on +`PYTHONPATH` and network access blocked: + +```powershell +$env:PYTHONPATH = "$PWD\application\single_app;$PWD\functional_tests" +python -m pytest -p test_support.cosmos_network_guard .\functional_tests\test_document_source_scope_identity.py -q +``` + +The 28 new regression cases pass. The combined retrieval, native analysis, +dependency, and source-authority run reports 300 passing tests, 24 failures, and +14 setup errors. The same 24 failures and 14 errors reproduce in the existing +source-authority suite with unchanged HEAD retrieval code and without the new +test module. They originate in source-free durable recovery/wait fixtures +before their adapters execute; they are not treated as passing validation. +Restoring the repository-pinned application dependencies in a session-local +environment does not change that baseline result. + +Excluding only those 38 baseline cases and including the existing chat-upload +storage-owner SDK contract yields **301 passed, 38 deselected**. Documentation +coverage and site-quality checks pass all 13 tests. The reviewed Cosmos +inventory has 1,066 owners, 409 doubles, and zero review gaps; only the two +unchanged chat-upload owners' containing-module fingerprints were regenerated. + +After restoring the pinned dependencies, the mandatory network-blocked +`python .\scripts\check_cosmos_response_boundaries.py --run-contracts` gate +passes **1,079 tests and 244 subtests**. Earlier attempts exposed inherited +Flask/Werkzeug incompatibility and a missing Yamcs client; dependency restoration +was limited to the session-local virtual environment, not global packages. + +A separate network-blocked optimized-Python probe passes seven explicit checks +for all six workspace/association combinations and approved shared-group +provenance. + +Before the fix, an upload conversation could replace a workspace identity, +causing a valid comparison or Analyze manifest to be refused. After the fix, +the payload matches the authorized scope while genuine access, hold, deletion, +revision, and provider failures remain failures. + +This change has not been deployed, and the original live FED-04 attempt has +not been rerun. A live acceptance result requires the authorized original +fixtures and harness after deployment; offline tests do not change the original +attempt's failed verdict. + +See [Gather / Reason / Render](../features/ORCHESTRATION_GATHER_REASON_RENDER.md) +for the shared source-access and retained-result contracts. diff --git a/functional_tests/cosmos_response_boundaries.json b/functional_tests/cosmos_response_boundaries.json index ef18882e8..1cb8b67cf 100644 --- a/functional_tests/cosmos_response_boundaries.json +++ b/functional_tests/cosmos_response_boundaries.json @@ -15722,7 +15722,7 @@ ], "queries": [], "batches": [], - "module_sha256": "e84fc160cad4acaebc296f8b0864dc040f1bb1d704a55785ee4c65e60c81a80e" + "module_sha256": "e1e1d5fd22d370d860867adf4561e3d492ee3e9450a6ba9dfa5eb6f1282c4d95" }, "classification": "sdk-compatible", "contract": "_authorize_chat_upload_conversation: point documents use Mapping field reads/JSON encoding/metadata projections; batch status/body lists are separate, never top-level-normalized as documents. Strict domain sinks remain with their storage owners.", @@ -15749,7 +15749,7 @@ } ], "batches": [], - "module_sha256": "e84fc160cad4acaebc296f8b0864dc040f1bb1d704a55785ee4c65e60c81a80e" + "module_sha256": "e1e1d5fd22d370d860867adf4561e3d492ee3e9450a6ba9dfa5eb6f1282c4d95" }, "classification": "sdk-compatible", "contract": "_resolve_chat_upload_context: point documents use Mapping field reads/JSON encoding/metadata projections; batch status/body lists are separate, never top-level-normalized as documents. Strict domain sinks remain with their storage owners.", diff --git a/functional_tests/test_document_source_scope_identity.py b/functional_tests/test_document_source_scope_identity.py new file mode 100644 index 000000000..9495ff67d --- /dev/null +++ b/functional_tests/test_document_source_scope_identity.py @@ -0,0 +1,306 @@ +# test_document_source_scope_identity.py +""" +Regression tests for chat-associated workspace source identity. +Version: 0.261.347 +Implemented in: 0.261.347 + +Real retrieval, source authorization, Analyze and Compare consume SDK-shaped +storage responses. A conversation association must not become a workspace ID. +Only external document/chunk, membership, model and result-storage I/O is doubled. +""" + +from contextlib import contextmanager +import importlib +import json +import sys +from types import SimpleNamespace + +import pytest + +from content_screening import access +from content_screening.contracts import DocumentHeldError, ScreeningConflictError +from test_orchestration_dependency_runtime import ( # noqa: F401 - shared offline runtime fixture + binding, + compose, + execute, + runtime, + source_input, +) +from test_support import document_analysis as sources +from test_support.orchestration_research import document_action_policy_module +from test_support.orchestration_revisions import AtomicMemoryContainer + + +UPLOAD_CONVERSATION = "upload-conversation" +CURRENT_CONVERSATION = "conversation-1" + + +@contextmanager +def source_world(monkeypatch, scope, *, associated=True, shared=False): + scope_id = {"personal": sources.USER_ID, "group": "reading-group", "public": "public-workspace"}[scope] + owner_scope_id = "owning-group" if shared else scope_id + documents = { + document_id: sources.original_document(document_id, [text], scope, owner_scope_id) + for document_id, text in ( + ("policy-old", "Threshold: 125000. Review: 10 days. The service owner is Mira. Marker: POLICY-ALPHA."), + ("policy-new", "Threshold: 150000. Review: 7 days. The service owner is Mira. Marker: POLICY-BETA."), + ) + } + for value in documents.values(): + if associated: + value["document"]["conversation_id"] = UPLOAD_CONVERSATION + if shared: + value["document"]["shared_group_ids"] = [f"{scope_id},approved"] + + with sources.document_analysis_runtime(documents) as native: + with monkeypatch.context() as patcher: + config = sys.modules["config"] + containers = {} + for source_scope in ("personal", "group", "public"): + container = AtomicMemoryContainer("id") + containers[source_scope] = container + name = f"cosmos_{'user' if source_scope == 'personal' else source_scope}_documents_container" + patcher.setattr(config, name, container) + for value in documents.values(): + saved = containers[scope].upsert_item(value["document"]) + value["document"]["_etag"] = saved["_etag"] + groups = sys.modules["functions_group"] + memberships = {scope_id} + + def require_membership(user_id, group_id, **kwargs): + if user_id != sources.USER_ID or group_id not in memberships: + raise PermissionError("Membership denied by the source fixture.") + + patcher.setattr(groups, "assert_group_role", require_membership) + patcher.setattr(groups, "get_user_groups", lambda user_id: [{"id": key} for key in memberships]) + patcher.setitem(sys.modules, "functions_document_actions", document_action_policy_module()) + comparison = importlib.import_module("functions_document_comparison") + patcher.setattr(comparison, "run_document_analysis", native.producer.run_document_analysis) + # Search imports these external-I/O functions by value. + patcher.setattr(native.search, "get_user_groups", groups.get_user_groups) + patcher.setattr(native.search, "cosmos_conversations_container", AtomicMemoryContainer("id")) + patcher.setattr(native.search, "cosmos_messages_container", AtomicMemoryContainer("conversation_id")) + manifest = [ + { + "document_id": document_id, "scope": scope, "scope_id": scope_id, + "source_kind": "narrative", "file_name": value["document"]["file_name"], + "source_version": 1, "source_revision": value["document"]["_etag"], + "authorization_status": "authorized", + } + for document_id, value in documents.items() + ] + yield SimpleNamespace( + native=native, comparison=comparison, documents=documents, containers=containers, + scope=scope, scope_id=scope_id, manifest=manifest, memberships=memberships, + options={ + "doc_scope": scope, "active_group_ids": [scope_id] if scope == "group" else None, + "active_public_workspace_id": [scope_id] if scope == "public" else None, + "conversation_id": CURRENT_CONVERSATION, + }, + ) + + +def read_payload(world, document_id="policy-old"): + return world.native.search.get_document_chunks_payload( + document_id, sources.USER_ID, **world.options, + ) + + +@pytest.mark.parametrize("scope", ["personal", "group", "public"]) +@pytest.mark.parametrize("associated", [False, True]) +def test_workspace_scope_is_not_its_upload_conversation(monkeypatch, scope, associated): + with source_world(monkeypatch, scope, associated=associated) as world, access.strict_source_authority(): + payload = read_payload(world) + built = world.native.results.build_analysis_source( + "policy-old", payload, source_snapshot=world.manifest[0], + ) + assert payload["scope"] == built["scope"] == scope + assert payload["scope_id"] == built["scope_id"] == world.scope_id + assert payload["conversation_id"] == (UPLOAD_CONVERSATION if associated else None) + assert payload["document"][access.PROVENANCE_FIELD]["scope_id"] == world.scope_id + + +def test_approved_shared_group_keeps_the_authorized_access_path(monkeypatch): + with source_world(monkeypatch, "group", shared=True) as world, access.strict_source_authority(): + payload = read_payload(world) + built = world.native.results.build_analysis_source( + "policy-old", payload, source_snapshot=world.manifest[0], + ) + assert payload["scope_id"] == built["scope_id"] == "reading-group" + assert payload["document"][access.PROVENANCE_FIELD]["scope_id"] == "owning-group" + world.memberships.clear() + with pytest.raises((LookupError, PermissionError)): + read_payload(world) + + +def test_personal_share_keeps_owner_not_actor_identity(monkeypatch): + with source_world(monkeypatch, "personal") as world, access.strict_source_authority(): + document = world.documents["policy-old"]["document"] + document.update(user_id="source-owner", shared_user_ids=[f"{sources.USER_ID},approved"]) + world.containers["personal"].upsert_item(document) + payload = read_payload(world) + assert payload["scope"] == "personal" + assert payload["scope_id"] == payload["document"]["user_id"] == "source-owner" + + +def test_actual_chat_source_keeps_its_authorized_conversation(monkeypatch): + with source_world(monkeypatch, "personal") as world, access.strict_source_authority(): + conversations = AtomicMemoryContainer("id") + conversations.upsert_item({"id": CURRENT_CONVERSATION, "user_id": sources.USER_ID}) + messages = AtomicMemoryContainer("conversation_id") + messages.upsert_item({ + "id": "chat-file", "conversation_id": CURRENT_CONVERSATION, + "role": "file", "filename": "chat-only.txt", "file_content": "An original chat-only attachment.", + }) + monkeypatch.setattr(world.native.search, "cosmos_conversations_container", conversations) + monkeypatch.setattr(world.native.search, "cosmos_messages_container", messages) + payload = read_payload(world, "chat-file") + assert payload["scope"] == "chat" + assert payload["scope_id"] == payload["conversation_id"] == CURRENT_CONVERSATION + assert payload["document"]["source_type"] == "chat_upload" + with pytest.raises(LookupError): + world.native.search.get_document_chunks_payload( + "chat-file", "not-the-owner", **world.options, + ) + + +@pytest.mark.parametrize("scope", ["personal", "group", "public"]) +def test_linked_workspace_upload_resolves_as_workspace_not_chat(monkeypatch, scope): + with source_world(monkeypatch, scope) as world, access.strict_source_authority(): + def no_chat_fallback(*args, **kwargs): + raise AssertionError("A resolved workspace document must not fall back to a chat blob.") + + monkeypatch.setattr(world.native.search, "_resolve_chat_upload_context", no_chat_fallback) + payload = read_payload(world) + assert payload["scope"] == scope + assert payload["scope_id"] == world.scope_id + assert payload["document"]["source_type"] == "workspace_document" + + +@pytest.mark.parametrize("scope", ["personal", "group", "public"]) +def test_explicit_analyze_accepts_chat_association_without_changing_source_identity(monkeypatch, scope): + with source_world(monkeypatch, scope) as world, access.strict_source_authority(): + client = sources.FixtureAnalysisClient() + result = world.native.producer.run_document_analysis( + sources.USER_ID, "Identify the service owner.", ["policy-old"], client.invoke_prompt, + result_version="analyze-final-v1", source_manifest=[world.manifest[0]], + **world.options, + ) + assert result["analysis_validation"]["status"] == "valid" + assert result["analysis_sources"][0]["scope_id"] == world.scope_id + assert result["authoritative_result"]["value"][0]["values"] == {"owner": "Mira"} + assert len(client.calls) == 1 + assert "POLICY-ALPHA" in client.calls[0]["prompt"] + + +def test_analyze_still_refuses_a_different_manifest_scope(monkeypatch): + with source_world(monkeypatch, "group") as world, access.strict_source_authority(): + payload = read_payload(world) + changed = {**world.manifest[0], "scope_id": "another-group"} + with pytest.raises(world.native.results.AnalysisResultUnavailable): + world.native.results.build_analysis_source("policy-old", payload, source_snapshot=changed) + + +@pytest.mark.parametrize("scope", ["group", "public"]) +@pytest.mark.parametrize("mutation", ["revoked", "deleted", "held", "revision"]) +def test_comparison_rechecks_sources_after_model_work(monkeypatch, scope, mutation): + with source_world(monkeypatch, scope) as world, access.strict_source_authority(): + stages = [] + + def invoke(prompt, stage=None, metadata=None): + stages.append(stage) + if stage == "comparison": + container = world.containers[scope] + document = container.read_item("policy-old", "policy-old") + if mutation == "revoked": + if scope == "group": + world.memberships.clear() + else: + monkeypatch.setattr( + sys.modules["functions_public_workspaces"], "find_public_workspace_by_id", + lambda workspace_id: None, + ) + elif mutation == "deleted": + container.delete_item("policy-old", "policy-old", etag=document["_etag"]) + else: + document["content_screening" if mutation == "held" else "version"] = ( + {"state": "pending_review"} if mutation == "held" else 2 + ) + container.upsert_item(document) + return "Complete summary or comparison." + + with pytest.raises((PermissionError, LookupError, DocumentHeldError, ScreeningConflictError)): + world.comparison.run_document_comparison( + sources.USER_ID, "Compare both revisions.", + { + "type": world.comparison.DOCUMENT_ACTION_TYPE_COMPARISON, + "left_document_id": "policy-old", "right_document_ids": ["policy-new"], + **world.options, + }, + invoke, conversation_id=CURRENT_CONVERSATION, result_version="comparison-v1", + ) + assert stages == ["window_analysis", "window_analysis", "comparison"] + + +@pytest.mark.parametrize("scope", ["group", "public"]) +@pytest.mark.parametrize("provider_failed", [False, True]) +def test_comparison_and_dependent_composition_use_complete_workspace_sources( + runtime, monkeypatch, scope, provider_failed, +): + monkeypatch.setattr(sources, "USER_ID", "owner") + summaries = [ + "Threshold: 125000. Review: 10 days. Owner: Mira. Marker: POLICY-ALPHA.", + "Threshold: 150000. Review: 7 days. Owner: Mira. Marker: POLICY-BETA.", + ] + answer = ( + "Threshold: 125000 to 150000 (+25000). Review: 10 to 7 days (-3). Owner remains Mira. " + "[POLICY-ALPHA](#policy-old) and [POLICY-BETA](#policy-new)." + ) + capability = runtime.registry.get_capability("document_compare", contract_version=2) + steps = [ + { + "step_id": "compare", "capability_id": "document_compare", + "arguments": { + "left_document_id": "policy-old", "right_document_ids": ["policy-new"], + "comparison_prompt": "Compare thresholds, review deadlines, owner and exact markers; cite both.", + }, + "outputs": [ + {"name": name, "kind": kind} + for name, kind in { + **capability["result_outputs"], **capability["optional_result_outputs"], + }.items() + ], + }, + compose("answer", inputs={"source": source_input("compare", "report")}), + ] + case = runtime.make( + steps, [*summaries, RuntimeError("Synthetic provider failure") if provider_failed else answer, answer], + final_response=binding("answer"), + ) + with source_world(monkeypatch, scope) as world, access.strict_source_authority(): + case.fixture.sources = {item["document_id"]: item for item in world.manifest} + case.fixture.service.access.source_metadata_reader = access._read_authorized_document + case.context.active_group_ids = world.options["active_group_ids"] + case.context.active_public_workspace_id = world.options["active_public_workspace_id"] + result = execute(runtime, case) + assert [step["status"] for step in result["steps"]] == ( + ["failed", "skipped"] if provider_failed else ["completed", "completed"] + ), result + assert result["status"] == ("failed" if provider_failed else "completed") + assert "POLICY-ALPHA" in case.model.calls[0][0] + assert "POLICY-BETA" in case.model.calls[1][0] + assert case.model.calls[2][1]["stage"] == "comparison" + if not provider_failed: + composed_input = json.dumps(case.model.calls[3][0]) + assert answer in composed_input + assert result["message"] == answer + task = runtime.contracts.TaskResult.from_dict(result["task_results"]["compare"]) + retained = case.fixture.service.open_result(task.output("report")).read_text() + assert retained == answer + assert len(case.model.calls) == (3 if provider_failed else 4) + assert result["artifacts"] == result["outputs"] == [] + assert {step["capability_id"] for step in result["steps"]} == {"document_compare", "compose"} + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-q"]))