From a02e9e4456fe343c49ca71183eda1ec0ac022686 Mon Sep 17 00:00:00 2001 From: witbrock Date: Tue, 15 Sep 2026 13:50:11 -0400 Subject: [PATCH 1/2] Align browser ontology writes with exact scope and description verification --- src/backend/server/routes/concept_routes.py | 44 +-- src/backend/server/routes/vontology_routes.py | 17 +- src/backend/services/concept_service.py | 189 +++++---- .../ontology_mutation_command_service.py | 22 ++ .../von_interface/static/js/dynamicTabs.js | 47 ++- ...est_ontology_browser_write_verification.py | 371 ++++++++++++++++++ tests/browser/ontologyWriteFixture.py | 122 ++++++ tests/browser/ontologyWriteVerification.cjs | 108 +++++ .../descriptionMutationOutcome.test.js | 31 ++ 9 files changed, 805 insertions(+), 146 deletions(-) create mode 100644 tests/backend/test_ontology_browser_write_verification.py create mode 100644 tests/browser/ontologyWriteFixture.py create mode 100644 tests/browser/ontologyWriteVerification.cjs create mode 100644 tests/frontend/descriptionMutationOutcome.test.js diff --git a/src/backend/server/routes/concept_routes.py b/src/backend/server/routes/concept_routes.py index 6c5b9abde..fb269f6ef 100644 --- a/src/backend/server/routes/concept_routes.py +++ b/src/backend/server/routes/concept_routes.py @@ -30,6 +30,7 @@ from ...services.ontology_mutation_command_service import ( OntologyMutationCommandError, delete_legacy_name, + execute_governed_concept_description_update, ) from ...services.paper_recommendation_profile_vontology_service import ( load_paper_recommendation_profile, @@ -226,10 +227,9 @@ def _get_current_user_concept_id() -> str | None: def _get_current_org_concept_id() -> str | None: - effective = _get_effective_request_context() - return _normalise_concept_id( - effective.get("organisation_id") - ) or _normalise_concept_id(session.get("organisation_concept_id")) + from ...security.access_control import get_effective_organisation_concept_id + + return get_effective_organisation_concept_id() def _is_admin_or_owner_session() -> bool: @@ -630,7 +630,9 @@ def create_concept_route(): created_by_concept_id=trusted_actor, organisation_concept_id=trusted_org, event_namespace=_get_request_namespace(), - visibility_scope_mode=create_arguments.get("scope_mode"), + visibility_scope_mode=( + create_arguments.get("scope_mode") or "user_only_default" + ), maintain_relationship_inverses=False, resolve_visibility_from_event_namespace=False, ), @@ -794,20 +796,8 @@ def single_concept_route(concept_id: str) -> ResponseReturnValue: else: return jsonify(error="Failed to update notes"), 500 elif "description" in data and len(data) == 1: - governed = _execute_governed_http_mutation( - method_name="upsert_singleton_text_relation", - arguments={ - "concept_id": concept_id, - "predicate": "hasDescription", - "text": data["description"], - }, - mutate=lambda: { - "success": bool( - concept_service.update_concept_description( - concept_id, data["description"] - ) - ) - }, + governed = execute_governed_concept_description_update( + concept_id, data["description"] ) if governed.get("success") is False: return _governed_mutation_response(governed) @@ -1510,20 +1500,8 @@ def update_concept_description_route(concept_id: str): 400, ) try: - governed = _execute_governed_http_mutation( - method_name="upsert_singleton_text_relation", - arguments={ - "concept_id": concept_id, - "predicate": "hasDescription", - "text": data["description"], - }, - mutate=lambda: { - "success": bool( - concept_service.update_concept_description( - concept_id, data["description"] - ) - ) - }, + governed = execute_governed_concept_description_update( + concept_id, data["description"] ) if governed.get("success") is False: return _governed_mutation_response(governed) diff --git a/src/backend/server/routes/vontology_routes.py b/src/backend/server/routes/vontology_routes.py index 7a5075c75..f282619a7 100644 --- a/src/backend/server/routes/vontology_routes.py +++ b/src/backend/server/routes/vontology_routes.py @@ -52,7 +52,6 @@ get_concept_by_id, ConceptNotFoundError, resolve_concept_display_names, - update_concept_description, ) from ...services.text_value_service import get_texts_for_concept, get_texts_for_concepts from ...services.concept_relation_service import build_concept_relations_payload @@ -1435,21 +1434,11 @@ def update_description_route(): ) from ...services.ontology_mutation_command_service import ( - execute_governed_ontology_method, + execute_governed_concept_description_update, ) - governed = execute_governed_ontology_method( - method_name="update_concept", - arguments={ - "concept_id": resolved_concept_id, - "update_data": {"description": description}, - "request_id": data.get("request_id"), - }, - mutate=lambda: { - "success": bool( - update_concept_description(resolved_concept_id, description) - ) - }, + governed = execute_governed_concept_description_update( + resolved_concept_id, description ) if governed.get("success") is False: return _governed_mutation_response(governed) diff --git a/src/backend/services/concept_service.py b/src/backend/services/concept_service.py index 180dddb49..45d2344fb 100644 --- a/src/backend/services/concept_service.py +++ b/src/backend/services/concept_service.py @@ -336,12 +336,10 @@ def _resolve_creation_actor_context( if actor_org_id is None: try: - from flask import has_request_context, session as flask_session + from ..security.access_control import get_effective_organisation_concept_id - if has_request_context(): - org_raw = flask_session.get("organisation_concept_id") - if isinstance(org_raw, str) and org_raw.strip(): - actor_org_id = org_raw.strip() + # A window's Personal selection is authoritative, including None. + actor_org_id = get_effective_organisation_concept_id() except Exception: actor_org_id = None @@ -4584,7 +4582,115 @@ def update_concept_notes( return False -def update_concept_description(concept_id: str, new_description: str) -> bool: +def prepare_concept_description_update( + concept_id: str, new_description: str +) -> Dict[str, Any]: + """Resolve the exact stored description and metadata before governed execution. + + This reads canonical metadata but performs no write. The caller must pass the + same plan to the writer and the verifier, including the one edit timestamp. + """ + existing_context: Dict[str, Any] = {} + existing_provenance: Dict[str, Any] = {} + cleaned_description, extracted_inline_metadata = ( + extract_inline_description_metadata(new_description) + ) + description_to_store = ( + cleaned_description if cleaned_description.strip() else new_description + ) + + try: + existing_descriptions = get_texts_for_concept( + subject_concept_id=concept_id, predicate="hasDescription", limit=1 + ) + if existing_descriptions: + first = existing_descriptions[0] + if isinstance(first.get("context"), dict): + existing_context = dict(first["context"]) + if isinstance(first.get("provenance"), dict): + existing_provenance = dict(first["provenance"]) + except Exception as e: + logger.warning( + "update_concept_description: failed to read existing metadata for %s: %s", + concept_id, + e, + ) + # Missing metadata is different from a failed read. Do not replace it + # with empty metadata when its canonical state could not be observed. + raise + + now_iso = datetime.now(timezone.utc).isoformat() + provenance_payload: Dict[str, Any] = dict(existing_provenance) + context_payload: Dict[str, Any] = dict(existing_context) + if not provenance_payload: + provenance_payload["source"] = "update_concept_description" + provenance_payload["last_edited_at"] = now_iso + provenance_payload.setdefault("last_edited_by", "update_concept_description") + context_payload.setdefault("write_strategy", "relations_only_v2") + + if extracted_inline_metadata: + if isinstance(extracted_inline_metadata.get("source"), str): + provenance_payload.setdefault( + "source_label", extracted_inline_metadata["source"] + ) + if isinstance(extracted_inline_metadata.get("attribution"), str): + provenance_payload.setdefault( + "attribution", extracted_inline_metadata["attribution"] + ) + if isinstance(extracted_inline_metadata.get("timestamp"), str): + provenance_payload.setdefault( + "upstream_timestamp", extracted_inline_metadata["timestamp"] + ) + if isinstance(extracted_inline_metadata.get("parent"), str): + context_payload.setdefault( + "parent_concept_id", extracted_inline_metadata["parent"] + ) + confidence_score = extracted_inline_metadata.get("confidence_score") + if isinstance(confidence_score, (int, float)): + context_payload.setdefault("confidence_score", float(confidence_score)) + context_payload["inline_metadata_migrated"] = True + + from .text_value_service import _compute_fingerprint, _prepare_persisted_text + from ..db.repositories.text_value_repository import TextValuesRepository + + stored_text = _prepare_persisted_text(description_to_store) + # Reuse legacy values when the bytes match. A normalised fingerprint can + # otherwise alias a deliberate case/spacing edit to different stored text; + # use the existing exact-identity mode only for that collision. + identity_mode = "normalised" + existing_value = TextValuesRepository.find_one_by_fingerprint( + _compute_fingerprint(stored_text, "en"), "en" + ) + if existing_value and existing_value.get("text") != stored_text: + identity_mode = "exact" + existing_value = TextValuesRepository.find_one_by_fingerprint( + _compute_fingerprint(stored_text, "en", identity_mode="exact"), "en" + ) + if existing_value and existing_value.get("text") != stored_text: + raise ValueError( + "Exact description fingerprint resolved to different bytes" + ) + if existing_value: + # Shared TextValues retain their original provenance on reuse. Bind + # verification to that observed value, without overwriting attribution. + provenance_payload = dict(existing_value.get("provenance") or {}) + return { + "subject_concept_id": concept_id, + "predicate": "hasDescription", + "text": stored_text, + "lang": "en", + "provenance": provenance_payload, + "context": context_payload, + "identity_mode": identity_mode, + } + + +def update_concept_description( + concept_id: str, + new_description: str, + *, + prepared_update: Optional[Dict[str, Any]] = None, +) -> bool: """Update concept description using text_relations only (Phase 1 preserved_fields deprecation). Behaviour: @@ -4618,76 +4724,11 @@ def update_concept_description(concept_id: str, new_description: str) -> bool: ) return False - existing_context: Dict[str, Any] = {} - existing_provenance: Dict[str, Any] = {} - cleaned_description, extracted_inline_metadata = ( - extract_inline_description_metadata(new_description) - ) - description_to_store = ( - cleaned_description if cleaned_description.strip() else new_description + prepared = prepared_update or prepare_concept_description_update( + concept_id, new_description ) - - try: - existing_descriptions = get_texts_for_concept( - subject_concept_id=concept_id, predicate="hasDescription", limit=1 - ) - if existing_descriptions: - first = existing_descriptions[0] - if isinstance(first.get("context"), dict): - existing_context = dict(first["context"]) - if isinstance(first.get("provenance"), dict): - existing_provenance = dict(first["provenance"]) - except Exception as e: # pragma: no cover - logger.warning( - "update_concept_description: failed to read existing metadata for %s: %s", - concept_id, - e, - ) - try: - now_iso = datetime.now(timezone.utc).isoformat() - provenance_payload: Dict[str, Any] = dict(existing_provenance) - context_payload: Dict[str, Any] = dict(existing_context) - if not provenance_payload: - provenance_payload["source"] = "update_concept_description" - provenance_payload["last_edited_at"] = now_iso - provenance_payload.setdefault( - "last_edited_by", "update_concept_description" - ) - context_payload.setdefault("write_strategy", "relations_only_v2") - - if extracted_inline_metadata: - if isinstance(extracted_inline_metadata.get("source"), str): - provenance_payload.setdefault( - "source_label", extracted_inline_metadata["source"] - ) - if isinstance(extracted_inline_metadata.get("attribution"), str): - provenance_payload.setdefault( - "attribution", extracted_inline_metadata["attribution"] - ) - if isinstance(extracted_inline_metadata.get("timestamp"), str): - provenance_payload.setdefault( - "upstream_timestamp", extracted_inline_metadata["timestamp"] - ) - if isinstance(extracted_inline_metadata.get("parent"), str): - context_payload.setdefault( - "parent_concept_id", extracted_inline_metadata["parent"] - ) - confidence_score = extracted_inline_metadata.get("confidence_score") - if isinstance(confidence_score, (int, float)): - context_payload.setdefault( - "confidence_score", float(confidence_score) - ) - context_payload["inline_metadata_migrated"] = True - - tv_payload = upsert_text_for_concept( - subject_concept_id=concept_id, - predicate="hasDescription", - text=description_to_store, - lang="en", - provenance=provenance_payload, - context=context_payload, - ) + tv_payload = upsert_text_for_concept(**prepared) new_text_value_id = tv_payload.get("text_value_id") relation_id = tv_payload.get("relation_id") logger.info( diff --git a/src/backend/services/ontology_mutation_command_service.py b/src/backend/services/ontology_mutation_command_service.py index f6cd6a2e6..82639254d 100644 --- a/src/backend/services/ontology_mutation_command_service.py +++ b/src/backend/services/ontology_mutation_command_service.py @@ -3958,11 +3958,33 @@ def json_dumps_sorted(value: Any) -> str: return json.dumps(value, sort_keys=True, ensure_ascii=True, separators=(",", ":")) +def execute_governed_concept_description_update( + concept_id: str, description: str +) -> dict[str, Any]: + """Bind browser description verification to the writer's exact prepared input.""" + from .concept_service import ( + prepare_concept_description_update, + update_concept_description, + ) + + prepared = prepare_concept_description_update(concept_id, description) + return execute_governed_ontology_method( + method_name="upsert_singleton_text_relation", + arguments=prepared, + mutate=lambda: { + "success": update_concept_description( + concept_id, description, prepared_update=prepared + ) + }, + ) + + __all__ = [ "OntologyMutationCommandError", "build_ontology_mutation_intent", "canonical_read_back_for_method", "delete_legacy_name", + "execute_governed_concept_description_update", "execute_governed_ontology_method", "is_ontology_mutation_method", "issue_same_turn_method_delegation", diff --git a/src/frontend/web/von_interface/static/js/dynamicTabs.js b/src/frontend/web/von_interface/static/js/dynamicTabs.js index cce778430..f112c58f9 100644 --- a/src/frontend/web/von_interface/static/js/dynamicTabs.js +++ b/src/frontend/web/von_interface/static/js/dynamicTabs.js @@ -5,7 +5,7 @@ import { profileButton } from './components/participantProfile.js'; // Removed imported populateContentSection to avoid duplicate with local implementation below import { initializeAnnotationTab } from './annotationTab.js'; -import { getJsonDetailed } from './apiService.js'; +import { getJsonDetailed, postJsonDetailed } from './apiService.js'; import { fetchConceptListWithSuffix, fetchSubtypesWithSuffix, initializeNamesForm, loadConceptAttributes, loadConceptNames, selectConceptWithSuffix } from './conceptTab.js'; import { getCurrentUserConceptId } from './domUtils.js'; import { isAnnotationEnabled } from './featureFlags.js'; @@ -3622,32 +3622,26 @@ async function adaptIndividualConceptTabUI(conceptId, suffix) { } // Unified description: ensure the typeDescriptionSection exists under Step1 for any concept -// Export for testing: dual-write description updater (JVNAUTOSCI-570) +// Export for testing: governed description updater. export async function updateConceptDescription(conceptId, description) { + if (!conceptId || typeof description !== 'string') return false; try { - if (!conceptId || typeof description !== 'string') return false; - const encodedId = encodeURIComponent(conceptId); - const resp = await conceptApiFetch(`/api/concepts/${encodedId}/description`, { - method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ description }) - }); - if (resp.ok) { - const data = await resp.json().catch(() => ({})); - return !data.error; - } - // Fallback: legacy endpoint (will eventually be removed) - if (resp.status === 404 || resp.status === 405) { - const legacy = await fetch('/vontology/api/vontology/update_description', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ identifier: conceptId, description }) - }); - const legacyData = await legacy.json().catch(() => ({})); - return legacy.ok && legacyData && legacyData.success; - } - return false; + const { data } = await postJsonDetailed( + `/api/concepts/${encodeURIComponent(conceptId)}/description`, + { description }, + { method: 'PATCH' } + ); + return !!data && !data.error && data.success !== false; } catch (e) { + if (e.payload?.effect_status === 'indeterminate') { + const receiptId = e.payload.authority_receipt?.receipt_id; + throw new Error( + 'The description may have been saved, but verification is incomplete. ' + + 'Your draft is retained. Check the saved description before trying again.' + + (receiptId ? ` Receipt: ${receiptId}` : ''), + { cause: e } + ); + } return false; } } @@ -5104,8 +5098,9 @@ async function populateTypeDescription(conceptId, suffix, options = {}) { return; } statusEl.textContent = 'Saving...'; + newSave.disabled = true; try { - // JVNAUTOSCI-570 regression fix: use dedicated dual-write description endpoint + // Keep the draft until the governed write is verified. const ok = await updateConceptDescription(conceptId, rawText); if (!ok) throw new Error('Failed to persist description'); await loadDescription(); @@ -5119,6 +5114,8 @@ async function populateTypeDescription(conceptId, suffix, options = {}) { delete textarea.dataset.originalContent; } catch (e) { statusEl.textContent = `Error: ${e.message}`; + } finally { + newSave.disabled = false; } }); diff --git a/tests/backend/test_ontology_browser_write_verification.py b/tests/backend/test_ontology_browser_write_verification.py new file mode 100644 index 000000000..86da45041 --- /dev/null +++ b/tests/backend/test_ontology_browser_write_verification.py @@ -0,0 +1,371 @@ +"""Real governed HTTP writes against isolated storage, including divergent effects.""" + +from urllib.parse import quote + +import pytest +from flask import Flask + +from src.backend.db.repositories.concepts_repository import ConceptsRepository +from src.backend.db.repositories.text_value_repository import ( + TextRelationsRepository, + TextValuesRepository, +) +from src.backend.security.access_control import override_current_actor +from src.backend.server.routes import concept_routes, vontology_routes +from src.backend.services import concept_service, text_value_service +from src.backend.services import ontology_mutation_command_service as command +from src.backend.services import ontology_publication_authority_service as authority +from src.backend.services import window_session_context_service as windows + +ACTOR = "#V#fixture_author" +ORG = "#V#fixture_sail" +CONCEPT = "#V#fixture_description" + + +@pytest.fixture +def browser_writes(isolated_window_session_db, monkeypatch): + monkeypatch.setenv("VON_EVENT_WORKFLOW_INTEGRATION_ENABLE", "0") + monkeypatch.setattr(authority, "_gateway_actor_trust_source", lambda: None) + app = Flask(__name__) + app.secret_key = "isolated-tests-only" + app.register_blueprint(concept_routes.concept_bp, url_prefix="/api/concepts") + app.register_blueprint( + vontology_routes.vontology_bp, url_prefix="/vontology/api/vontology" + ) + # Synthetic fixture seed only; tested mutations go through canonical services. + collection = ConceptsRepository.collection() + collection.insert_many( + [ + { + "concept_id": ACTOR, + "relationships": {"is_an_instance_of": ["#V#person"]}, + }, + {"concept_id": "#V#thing", "relationships": {}}, + {"concept_id": CONCEPT, "relationships": {"#V#specific_to_user": [ACTOR]}}, + ] + ) + client = app.test_client() + with client.session_transaction() as session: + session.update( + user_concept_id=ACTOR, + organisation_concept_id=ORG, + auth_provider="browser_test_fixture", + role_in_org="member", + namespace="#V#fixture_author@fixture_sail", + ) + windows.clear_window_organisation("personal-window", namespace=ACTOR, user_id=ACTOR) + windows.set_window_organisation( + "sail-window", ORG, "member", "#V#fixture_author@fixture_sail", ACTOR + ) + return client + + +@pytest.mark.parametrize("window", ["personal-window", "sail-window"]) +def test_creation_defaults_remain_private_in_both_windows(browser_writes, window): + client = browser_writes + payload = { + "concept_id": "#V#fixture_created", + "name": "Fixture created", + "kind": "instance", + "parent_concept_ids": ["#V#thing"], + } + response = client.post( + "/api/concepts/", json=payload, headers={"X-Von-Window-Session": window} + ) + result = response.get_json() + assert response.status_code == 201, result + assert result["canonical_read_back"]["verified"] is True + assert result["authority_receipt"]["status"] == "succeeded" + with override_current_actor(ACTOR, ORG if window == "sail-window" else None): + stored = concept_service.get_concept_by_concept_id("#V#fixture_created") + assert stored["relationships"]["#V#specific_to_user"] == [ACTOR] + assert not stored["relationships"].get("#V#specific_to_organisation") + # Repeating a create reconciles the exact visible record; no second effect. + repeated = client.post( + "/api/concepts/", json=payload, headers={"X-Von-Window-Session": window} + ) + assert repeated.get_json()["success"] is True, repeated.get_json() + assert ( + ConceptsRepository.collection().count_documents( + {"concept_id": "#V#fixture_created"} + ) + == 1 + ) + + +@pytest.mark.parametrize("route", ["patch", "put", "legacy"]) +@pytest.mark.parametrize("window", ["personal-window", "sail-window"]) +def test_description_exact_metadata_language_and_repeated_edit( + browser_writes, route, window +): + client = browser_writes + with override_current_actor(ACTOR, ORG): + text_value_service.upsert_text_for_concept( + CONCEPT, + "hasDescription", + "Old description", + lang="en", + context={"confidence_score": 0.91}, + provenance={"source": "fixture-source", "attribution": "fixture-author"}, + ) + raw = "Source: imported\nConfidence: 20%\n\n ## Heading\n\nText with spacing. " + expected = "## Heading\n\nText with spacing." + path = f"/api/concepts/{quote(CONCEPT, safe='')}" + headers = {"X-Von-Window-Session": window} + + def edit(text): + if route == "legacy": + return client.post( + "/vontology/api/vontology/update_description", + json={"identifier": CONCEPT, "description": text}, + headers=headers, + ) + if route == "put": + return client.put(path, json={"description": text}, headers=headers) + return client.patch( + path + "/description", json={"description": text}, headers=headers + ) + + for text in [raw, raw, expected.replace("Text", "TEXT")]: + response = edit(text) + assert response.status_code == 200, response.get_json() + with override_current_actor(ACTOR, ORG): + rows = text_value_service.get_texts_for_concept( + CONCEPT, predicate="hasDescription" + ) + assert len(rows) == 1 + assert rows[0]["text"] == (expected if text == raw else text) + assert rows[0]["lang"] == "en" + assert rows[0]["context"]["confidence_score"] == 0.91 + assert rows[0]["context"]["write_strategy"] == "relations_only_v2" + assert rows[0]["provenance"]["source"] == "fixture-source" + receipts = authority.get_ontology_mutation_receipts_collection() + assert receipts.count_documents({"status": "indeterminate"}) == 0 + assert receipts.count_documents({"status": "succeeded"}) == 3 + + +@pytest.mark.parametrize( + "divergence", ["absent", "text", "language", "context", "provenance", "duplicate"] +) +def test_real_divergence_never_becomes_success(browser_writes, monkeypatch, divergence): + original = concept_service.update_concept_description + + def divergent(concept_id, text, **kwargs): + if divergence == "absent": + return True + success = original(concept_id, text, **kwargs) + relation = TextRelationsRepository.collection().find_one( + {"subject_concept_id": concept_id} + ) + text_id = relation["object_text_id"] + from bson import ObjectId + + if divergence in {"text", "language", "provenance"}: + key, value = { + "text": ("text", "Different body"), + "language": ("lang", "fr"), + "provenance": ("provenance", {"source": "wrong"}), + }[divergence] + TextValuesRepository.collection().update_one( + {"_id": ObjectId(text_id)}, {"$set": {key: value}} + ) + elif divergence == "context": + TextRelationsRepository.collection().update_one( + {"_id": relation["_id"]}, {"$set": {"context": {"wrong": True}}} + ) + elif divergence == "duplicate": + text_value_service.upsert_text_for_concept( + concept_id, "hasDescription", "Unpruned other description", lang="en" + ) + return success + + monkeypatch.setattr(concept_service, "update_concept_description", divergent) + response = browser_writes.patch( + f"/api/concepts/{quote(CONCEPT, safe='')}/description", + json={"description": "Requested description"}, + ) + result = response.get_json() + assert response.status_code == 503, result + assert result["effect_status"] == "indeterminate" + assert result["retryable"] is False + assert result["canonical_read_back"]["verified"] is False + assert result["authority_receipt"]["status"] == "indeterminate" + + +def test_personal_scope_does_not_resurrect_browser_wide_org(browser_writes): + with browser_writes.application.test_request_context( + headers={"X-Von-Window-Session": "personal-window"} + ): + from flask import session + + session.update( + user_concept_id=ACTOR, + organisation_concept_id=ORG, + auth_provider="browser_test_fixture", + ) + assert concept_routes._get_current_org_concept_id() is None + assert concept_service._resolve_creation_actor_context( + created_by_concept_id=ACTOR, + organisation_concept_id=None, + event_namespace=None, + ) == (ACTOR, None) + + +@pytest.mark.parametrize( + "window,expected_status", [("personal-window", 400), ("sail-window", 201)] +) +def test_explicit_organisation_uses_only_the_selected_window( + browser_writes, monkeypatch, window, expected_status +): + monkeypatch.setattr( + authority, + "resolve_live_semantic_roles", + lambda actor: ( + authority.AuthorityRoleEvidence( + role=authority.ORGANISATION_ONTOLOGY_ADMINISTRATOR_ROLE, + actor_concept_id=ACTOR, + organisation_concept_id=ORG, + relation_id="fixture-org-role", + revision="1", + ), + ), + ) + response = browser_writes.post( + "/api/concepts/", + headers={"X-Von-Window-Session": window}, + json={ + "name": "Organisation record", + "concept_id": "#V#fixture_org_record", + "kind": "type", + "scope_mode": "organisation_general", + }, + ) + assert response.status_code == expected_status, response.get_json() + stored = ConceptsRepository.collection().find_one( + {"concept_id": "#V#fixture_org_record"} + ) + if expected_status == 400: + assert stored is None + assert response.get_json()["error_code"] == "organisation_context_required" + else: + assert stored["relationships"]["#V#specific_to_organisation"] == [ORG] + assert not stored["relationships"].get("#V#specific_to_user") + + +def test_composite_private_description_preserves_both_restrictions( + browser_writes, monkeypatch +): + ConceptsRepository.collection().update_one( + {"concept_id": CONCEPT}, + {"$set": {"relationships.#V#specific_to_organisation": [ORG]}}, + ) + monkeypatch.setattr( + authority, + "resolve_live_semantic_roles", + lambda actor: ( + authority.AuthorityRoleEvidence( + role=authority.ORGANISATION_ONTOLOGY_ADMINISTRATOR_ROLE, + actor_concept_id=ACTOR, + organisation_concept_id=ORG, + relation_id="fixture-org-role", + revision="1", + ), + ), + ) + response = browser_writes.patch( + f"/api/concepts/{quote(CONCEPT, safe='')}/description", + json={"description": "Composite private description"}, + headers={"X-Von-Window-Session": "sail-window"}, + ) + assert response.status_code == 200, response.get_json() + stored = ConceptsRepository.collection().find_one({"concept_id": CONCEPT}) + assert stored["relationships"] == { + "#V#specific_to_user": [ACTOR], + "#V#specific_to_organisation": [ORG], + } + + +@pytest.mark.parametrize("scope_change", ["global", "add_org"]) +def test_creation_scope_mismatch_stays_indeterminate( + browser_writes, monkeypatch, scope_change +): + original = concept_service.create_concept + + def divergent(**kwargs): + result = original(**kwargs) + patch = ( + {"$unset": {"relationships.#V#specific_to_user": ""}} + if scope_change == "global" + else {"$set": {"relationships.#V#specific_to_organisation": [ORG]}} + ) + ConceptsRepository.collection().update_one( + {"concept_id": result["concept_id"]}, patch + ) + return result + + monkeypatch.setattr(concept_service, "create_concept", divergent) + response = browser_writes.post( + "/api/concepts/", + json={ + "concept_id": "#V#fixture_scope_mismatch", + "name": "Scope mismatch", + "kind": "type", + }, + ) + result = response.get_json() + assert response.status_code == 503, result + assert result["effect_status"] == "indeterminate" + assert result["canonical_read_back"]["verified"] is False + + +def test_metadata_read_failure_does_not_start_a_description_write( + browser_writes, monkeypatch +): + def unavailable(*args, **kwargs): + raise ConnectionError("fixture read unavailable") + + monkeypatch.setattr(concept_service, "get_texts_for_concept", unavailable) + response = browser_writes.patch( + f"/api/concepts/{quote(CONCEPT, safe='')}/description", + json={"description": "Draft"}, + ) + assert response.status_code == 500 + assert ( + TextRelationsRepository.collection().count_documents( + {"subject_concept_id": CONCEPT} + ) + == 0 + ) + assert ( + authority.get_ontology_mutation_receipts_collection().count_documents({}) == 0 + ) + + +def test_existing_identical_text_value_is_reused_without_duplicate_or_provenance_loss( + browser_writes, +): + with override_current_actor(ACTOR, ORG): + seeded = text_value_service.upsert_text_for_concept( + CONCEPT, + "hasDescription", + "Existing exact description", + lang="en", + provenance={"source": "original-import", "record_id": "fixture-17"}, + ) + count_before = TextValuesRepository.collection().count_documents({}) + response = browser_writes.patch( + f"/api/concepts/{quote(CONCEPT, safe='')}/description", + json={"description": "Existing exact description"}, + ) + assert response.status_code == 200, response.get_json() + with override_current_actor(ACTOR, ORG): + rows = text_value_service.get_texts_for_concept( + CONCEPT, predicate="hasDescription" + ) + assert len(rows) == 1 + assert rows[0]["text_value_id"] == seeded["text_value_id"] + assert rows[0]["provenance"] == { + "source": "original-import", + "record_id": "fixture-17", + } + assert TextValuesRepository.collection().count_documents({}) == count_before diff --git a/tests/browser/ontologyWriteFixture.py b/tests/browser/ontologyWriteFixture.py new file mode 100644 index 000000000..e224ca035 --- /dev/null +++ b/tests/browser/ontologyWriteFixture.py @@ -0,0 +1,122 @@ +"""Disposable authenticated ontology-write browser fixture. + +Run with ``pdm run python tests/browser/ontologyWriteFixture.py``. Uses the real +application, localhost fixture login, canonical services and in-memory MongoDB. +No .env, production database, provider requests or real-user roles are used. +POST /fixture/stop to stop the fixture, including across sandbox PID namespaces. +""" + +import os +import sys +from pathlib import Path + +root = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(root)) +os.environ.update( + { + "PYTHON_DOTENV_DISABLED": "1", + "VON_USE_MOCK_DB": "1", + "VON_DB_NAME": "test_ontology_browser_verification", + "VON_AGENT_TEST_INSTANCE": "1", + "VON_BROWSER_TEST_AUTH_ENABLED": "1", + "VON_BROWSER_TEST_PSEUDOUSER_NAME": "Ontology Fixture Author", + "VON_BROWSER_TEST_PSEUDOUSER_EMAIL": "ontology-fixture@example.test", + "VON_BROWSER_TEST_PSEUDOUSER_CONCEPT_ID": "#V#ontology_fixture_author", + "VON_BROWSER_TEST_ORGANISATION_CONCEPT_ID": "#V#ontology_fixture_sail", + "VON_EVENT_WORKFLOW_INTEGRATION_ENABLE": "0", + "VON_DURABLE_WORKFLOWS_ENABLE": "0", + "VON_INTERNAL_MCP_ENABLE": "0", + "VON_PREWARM_DISABLE": "1", + "VON_WEB_PUSH_ENABLED": "0", + "VON_CHAT_PROMPT_DISPATCHER_ENABLE": "0", + "PYTEST_CURRENT_TEST": "isolated-browser-acceptance", + } +) +from src.backend.server.utils_flask import create_flask_app + + +def no_generation(*args, **kwargs): + raise RuntimeError("Generation is disabled in this disposable fixture") + + +app = create_flask_app(lambda: [], no_generation) +from src.backend.security.access_control import bypass_access_control +from src.backend.services.concept_service import create_concept + +with bypass_access_control(): + for concept_id, name, parents in [ + ("#V#thing", "Thing", []), + ("#V#person", "Person", ["#V#thing"]), + ("#V#organisation", "Organisation", ["#V#thing"]), + ("#V#ontology_fixture_sail", "Fixture SAIL", ["#V#organisation"]), + ]: + create_concept(concept_id=concept_id, name=name, parent_concept_ids=parents) +from flask import render_template + +for name in [ + "settings", + "chat", + "vontology", + "import_export", + "entity", + "concept", + "annotation", +]: + path = "/settings" if name == "settings" else f"/{name}_tab" + app.add_url_rule( + path, + endpoint=f"serve_{name}_tab_route", + view_func=lambda template=f"{name}_tab.html": render_template(template), + ) + + +@app.get("/fixture/evidence") +def fixture_evidence(): + import json + from flask import session, Response + from src.backend.services import ontology_publication_authority_service as authority + from src.backend.services.concept_service import get_concept_by_concept_id + from src.backend.services.text_value_service import get_texts_for_concept + + actor = session.get("user_concept_id") + if actor != "#V#ontology_fixture_author": + return {"error": "fixture_login_required"}, 403 + receipt_ids = [ + row["receipt_id"] + for row in authority.get_ontology_mutation_receipts_collection().find( + {"actor_concept_id": actor} + ) + ] + receipts = [ + authority.get_mutation_receipt_for_actor(receipt_id=rid, actor_concept_id=actor) + for rid in receipt_ids + ] + ids = { + cid + for receipt in receipts + for cid in receipt.get("target_concept_ids", []) + if cid != "#V#thing" + } + concepts = {cid: get_concept_by_concept_id(cid) for cid in sorted(ids)} + texts = { + cid: get_texts_for_concept(cid, predicate="hasDescription") + for cid in sorted(ids) + } + return Response( + json.dumps( + {"receipts": receipts, "concepts": concepts, "descriptions": texts}, + default=str, + ), + mimetype="application/json", + ) + + +@app.post("/fixture/stop") +def stop_fixture(): + import threading + + threading.Timer(0.2, lambda: os._exit(0)).start() + return {"stopping": True} + + +app.run(host="127.0.0.1", port=5099, use_reloader=False) diff --git a/tests/browser/ontologyWriteVerification.cjs b/tests/browser/ontologyWriteVerification.cjs new file mode 100644 index 000000000..cd517f852 --- /dev/null +++ b/tests/browser/ontologyWriteVerification.cjs @@ -0,0 +1,108 @@ +// Run only against ontologyWriteFixture.py: this creates disposable concepts. +// PLAYWRIGHT_BROWSERS_PATH=/tmp/von-playwright node tests/browser/ontologyWriteVerification.cjs URL EVIDENCE_DIR +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { chromium, expect } = require('@playwright/test'); +const [base = 'http://127.0.0.1:5099', evidence = '.run/ontology-verification'] = process.argv.slice(2); +assert(['127.0.0.1', 'localhost'].includes(new URL(base).hostname)); +fs.mkdirSync(evidence, { recursive: true }); +(async () => { + const browser = await chromium.launch(); + const page = await browser.newPage({ viewport: { width: 1440, height: 1000 } }); + page.setDefaultTimeout(15000); + const writes = []; + page.on('request', r => { if (r.method() === 'PATCH' || (r.method() === 'POST' && r.url().endsWith('/api/concepts/'))) writes.push({url:r.url(), method:r.method(), data:r.postDataJSON()}); }); + page.on('dialog', d => d.accept()); + try { + const health = await (await page.request.get(`${base}/health`)).json(); + assert.equal(health.agent_test_instance, true); + await page.goto(`${base}/von/`); + await page.locator('#vonBrowserTestLoginButton').click(); + await expect(page.locator('[data-tab="chatTab"]')).toBeVisible(); + const auth = await page.evaluate(async () => (await fetch('/von/api/auth/status')).json()); + assert.equal(auth.authenticated, true); + assert.equal(auth.auth_provider, 'browser_test_fixture'); + assert.equal(auth.user_concept_id, '#V#ontology_fixture_author'); + const results = []; + for (const scope of ['personal', 'sail']) { + if (scope === 'sail') { + await page.locator('.footer-org-menu-trigger').click(); + await page.locator('.footer-org-option[data-organisation-concept-id="#V#ontology_fixture_sail"]').click(); + await expect(page.locator('.footer-org-menu-trigger')).not.toHaveText('Personal'); + } + await expect(async () => { + await page.evaluate(() => document.dispatchEvent(new CustomEvent('von:selectConceptById', { + detail: { conceptId:'#V#thing', createConceptTab:true } + }))); + await expect(page.locator('.tab-button[data-concept-id="#V#thing"]')).toBeVisible({timeout:1000}); + }).toPass({timeout:15000}); + await page.locator('.tab-button[data-concept-id="#V#thing"]').click(); + const name = `Browser verification ${scope}`; + await page.locator('[id^="newInstanceInput_"]:visible').fill(name); + const createdResponse = page.waitForResponse(r => r.request().method() === 'POST' && r.url().endsWith('/api/concepts/')); + await page.locator('[id^="createInstanceButton_"]:visible').click(); + const response = await createdResponse; + const created = await response.json(); + assert.equal(response.status(), 201, JSON.stringify(created)); + assert.equal(created.canonical_read_back.verified, true); + const cid = created.concept.concept_id; + await page.evaluate(conceptId => document.dispatchEvent(new CustomEvent('von:selectConceptById', { + detail: { conceptId, createConceptTab:true } + })), cid); + await page.locator(`.tab-button[data-concept-id="${cid}"]`).click(); + const description = `## ${scope} fixture\n\nExact body with two spaces.\n\n- Evidence retained`; + for (let edit = 0; edit < 2; edit++) { + await page.locator('[id^="typeEditDescriptionButton_"]:visible').click(); + await page.locator('[id^="typeDescriptionTextarea_"]:visible').fill(description); + const savedResponse = page.waitForResponse(r => r.request().method() === 'PATCH' && r.url().endsWith('/description')); + await page.locator('[id^="typeEditDescriptionSave_"]:visible').click(); + const saved = await savedResponse; + assert.equal(saved.status(), 200, await saved.text()); + await expect(page.locator('[id^="typeDescriptionStatus_"]:visible')).toHaveText('Saved'); + } + await page.screenshot({path:path.join(evidence, `${scope}-saved.png`)}); + results.push({scope, concept_id:cid, description, create_receipt:created.authority_receipt}); + } + const canonical = await (await page.request.get(`${base}/fixture/evidence`)).json(); + assert.equal(canonical.receipts.length, 6); + for (const receipt of canonical.receipts) assert.equal(receipt.status, 'succeeded'); + for (const result of results) { + const concept = canonical.concepts[result.concept_id]; + assert.deepEqual(concept.relationships['#V#specific_to_user'], [auth.user_concept_id]); + assert(!concept.relationships['#V#specific_to_organisation']?.length); + const rows = canonical.descriptions[result.concept_id]; + assert.equal(rows.length, 1); + assert.equal(rows[0].text, result.description); + assert.equal(rows[0].lang, 'en'); + assert.equal(rows[0].context.write_strategy, 'relations_only_v2'); + assert.equal(rows[0].provenance.source, 'update_concept_description'); + } + // UI-only transport uncertainty: the backend really saves once, then + // the intercepted response reports uncertainty. Backend divergence is + // separately tested without response interception in the Python suite. + await page.route('**/api/concepts/*/description', async route => { + const response = await route.fetch(); + assert.equal(response.status(), 200); + await route.fulfill({status:503, contentType:'application/json', body:JSON.stringify({ + success:false, effect_status:'indeterminate', retryable:false, + authority_receipt:{receipt_id:'omr_transport_fixture'} + })}); + }); + await page.locator('[id^="typeEditDescriptionButton_"]:visible').click(); + const draft = 'Uncertain response draft retained'; + await page.locator('[id^="typeDescriptionTextarea_"]:visible').fill(draft); + const writesBefore = writes.length; + await page.locator('[id^="typeEditDescriptionSave_"]:visible').click(); + await expect(page.locator('[id^="typeDescriptionStatus_"]:visible')).toContainText('verification is incomplete'); + await expect(page.locator('[id^="typeDescriptionTextarea_"]:visible')).toHaveValue(draft); + assert.equal(writes.length, writesBefore + 1); + await page.screenshot({path:path.join(evidence,'indeterminate-draft.png')}); + fs.writeFileSync(path.join(evidence,'browser-evidence.json'), JSON.stringify({ + observed_at:new Date().toISOString(), health, auth, results, canonical, writes, + indeterminate_ui:{synthetic_response:true, draft_retained:true, writes:1}, + scope:'Local authenticated fixture; in-memory storage; generation disabled; no public OAuth or deployment claim' + },null,2)); + console.log(JSON.stringify({success:true, scopes:results.map(r=>r.scope), receipts:6, draftRetained:true})); + } finally { await browser.close(); } +})().catch(error => { console.error(error); process.exitCode = 1; }); diff --git a/tests/frontend/descriptionMutationOutcome.test.js b/tests/frontend/descriptionMutationOutcome.test.js new file mode 100644 index 000000000..da17e37f9 --- /dev/null +++ b/tests/frontend/descriptionMutationOutcome.test.js @@ -0,0 +1,31 @@ +/** @jest-environment jsdom */ +jest.mock('../../src/frontend/web/von_interface/static/js/apiService.js', () => ({ + postJsonDetailed: jest.fn(), getJsonDetailed: jest.fn() +})); +const { postJsonDetailed } = require('../../src/frontend/web/von_interface/static/js/apiService.js'); +const { updateConceptDescription } = require('../../src/frontend/web/von_interface/static/js/dynamicTabs.js'); + +beforeEach(() => jest.clearAllMocks()); + +test('verified description uses the window-aware helper and preserves draft bytes', async () => { + postJsonDetailed.mockResolvedValue({ data: { concept_id: '#V#fixture' } }); + await expect(updateConceptDescription('#V#fixture', 'Text\n\n indented')).resolves.toBe(true); + expect(postJsonDetailed).toHaveBeenCalledWith('/api/concepts/%23V%23fixture/description', + { description: 'Text\n\n indented' }, { method: 'PATCH' }); +}); + +test('indeterminate outcome explains uncertainty and never retries another write route', async () => { + postJsonDetailed.mockRejectedValue(Object.assign(new Error('HTTP 503'), { + payload: { effect_status: 'indeterminate', authority_receipt: { receipt_id: 'omr_fixture' } } + })); + await expect(updateConceptDescription('#V#fixture', 'Draft')).rejects.toThrow( + 'Your draft is retained. Check the saved description before trying again. Receipt: omr_fixture' + ); + expect(postJsonDetailed).toHaveBeenCalledTimes(1); +}); + +test.each([404, 405, 503])('HTTP %s does not cause a fallback mutation', async status => { + postJsonDetailed.mockRejectedValue(Object.assign(new Error(`HTTP ${status}`), { status })); + await expect(updateConceptDescription('#V#fixture', 'Draft')).resolves.toBe(false); + expect(postJsonDetailed).toHaveBeenCalledTimes(1); +}); From 66d8b856157ade1df36a7ea3b81b58228963590d Mon Sep 17 00:00:00 2001 From: witbrock Date: Tue, 15 Sep 2026 13:51:45 -0400 Subject: [PATCH 2/2] Keep description metadata unit test isolated from storage --- tests/backend/test_description_metadata_preservation.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/backend/test_description_metadata_preservation.py b/tests/backend/test_description_metadata_preservation.py index 72d905d02..ab16e1973 100644 --- a/tests/backend/test_description_metadata_preservation.py +++ b/tests/backend/test_description_metadata_preservation.py @@ -27,6 +27,12 @@ def test_extract_inline_description_metadata_strips_header_block(): def test_update_concept_description_preserves_existing_structured_metadata(monkeypatch): captured = {} + monkeypatch.setattr( + text_value_service.TextValuesRepository, + "find_one_by_fingerprint", + lambda *args, **kwargs: None, + ) + monkeypatch.setattr( concept_service.ConceptsRepository, "find_one",