From ca5f078d8a575ea3e2991771f8b4ea85e3dcaaa0 Mon Sep 17 00:00:00 2001 From: jcwalker3 Date: Tue, 28 Jul 2026 17:08:44 -0500 Subject: [PATCH] fix(mcp): separate not-connected from Connected-but-unattached (review 637 B2) A required namespace absent from the connected-service inventory was never counted as missing, because missing_namespaces was populated only while walking connected_servers. The session therefore reported attachment_healthy true, discovery_status namespaces_attached and error_type None while holding no attachment proof for that namespace, and the gate text told the operator "the host reports Connected" about a service nothing had reported Connected. assess_connected_namespace_attachment now classifies the two conditions apart. missing_namespaces keeps its meaning: required, Connected at the host, absent from the session tool surface -- the actual #708 defect, still typed mcp_connected_namespaces_missing. Required namespaces absent from the connected inventory land in the new not_connected_namespaces list, typed mcp_required_namespaces_not_connected, so a caller is never pointed at an attachment recovery for a service that never connected. attachment_healthy is false whenever any required namespace lacks attachment proof under either condition, error_types reports every condition present so neither hides the other, and namespace_conditions carries the per-namespace connected/attached verdict. Evidence that disagrees with itself -- attached in the session yet absent from the connected inventory -- is reported as contradictory and fails closed rather than being read as proof. attachment_gate_from_session states only what the recorded evidence supports: the Connected wording appears solely when connected is true, the not-connected wording when it is false, and a neutral refusal when connected status was never recorded. Review and merge stay fail-closed in all three cases. _record_live_namespace_attachment consumes the per-namespace verdicts instead of pinning the session-wide error_type onto every unattached namespace and instead of deriving health from a missing list that tracked only one of the two conditions. The two existing cases in test_issue_708_mcp_namespace_attachment.py declared no required_namespaces, so they inherited a default set containing a namespace their connected list omitted. Under the corrected rule that is a false-healthy assertion, so each now declares the required set it actually means. Closes #708 Co-Authored-By: Claude Opus 4.8 (1M context) --- gitea_mcp_server.py | 24 +- mcp_namespace_health.py | 197 +++++++++--- ...test_issue_708_mcp_namespace_attachment.py | 8 + ..._issue_708_not_connected_classification.py | 283 ++++++++++++++++++ 4 files changed, 473 insertions(+), 39 deletions(-) create mode 100644 tests/test_issue_708_not_connected_classification.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index bd01860..e82a7bd 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -6731,17 +6731,33 @@ def _record_live_namespace_attachment(assessment: dict | None) -> None: return missing = set(assessment.get("missing_namespaces") or []) error_type = assessment.get("error_type") + # Per-namespace verdicts when the assessment supplies them (#708 B2): a session-wide + # error_type must never be pinned onto a namespace whose own condition differs, and a + # namespace that was never connected must not be recorded healthy just because the + # session-wide "missing" list only tracks Connected-but-unattached namespaces. + conditions = assessment.get("namespace_conditions") + conditions = conditions if isinstance(conditions, dict) else {} for ns, state in proof.items(): ns_name = str(ns or "").strip() if not ns_name or not isinstance(state, dict): continue - attached = bool(state.get("attached")) + condition = conditions.get(ns_name) + condition = condition if isinstance(condition, dict) else {} + connected = bool(condition.get("connected", state.get("connected"))) + attached = bool(condition.get("attached", state.get("attached"))) + if condition: + healthy = bool(condition.get("attachment_healthy")) + ns_error = condition.get("condition") + else: + healthy = attached and connected and ns_name not in missing + ns_error = None if healthy else error_type _LIVE_NAMESPACE_ATTACHMENT[ns_name] = { "namespace": ns_name, - "connected": bool(state.get("connected")), + "connected": connected, "attached": attached, - "attachment_healthy": attached and ns_name not in missing, - "error_type": None if attached else error_type, + "attachment_healthy": healthy, + "condition": ns_error, + "error_type": ns_error, "discovery_status": assessment.get("discovery_status"), "reconnect_required": bool(assessment.get("reconnect_required")), "auto_recovered": bool(assessment.get("auto_recovered")), diff --git a/mcp_namespace_health.py b/mcp_namespace_health.py index 89999a5..e8e4910 100644 --- a/mcp_namespace_health.py +++ b/mcp_namespace_health.py @@ -53,6 +53,19 @@ EOF_PATTERNS = ( ERROR_CONNECTED_NAMESPACES_MISSING = "mcp_connected_namespaces_missing" +# Distinct from the condition above. ``mcp_connected_namespaces_missing`` is reserved for +# the actual #708 defect: the host *does* report the service Connected, yet the namespace +# never entered the active session tool surface. A required namespace that is absent from +# the connected-service inventory has no Connected claim behind it at all, so reporting it +# under the #708 condition would assert something the evidence does not support and would +# point an operator at the wrong recovery. +ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED = "mcp_required_namespaces_not_connected" + +DISCOVERY_STATUS_ATTACHED = "namespaces_attached" +DISCOVERY_STATUS_CONNECTED_MISSING = "connected_but_namespaces_missing" +DISCOVERY_STATUS_NOT_CONNECTED = "required_namespaces_not_connected" +DISCOVERY_STATUS_DISCONNECTED = "disconnected" + # Namespaces that must be *attached to the active session* for a mutation task (#708). # Connected-at-host is not attached-in-session; these are gated separately from the # #543 health map because a namespace can be healthy on probe yet absent from the @@ -115,27 +128,78 @@ def assess_connected_namespace_attachment( attached = set(str(ns).strip() for ns in (attached_session_namespaces or []) if str(ns).strip()) req = [str(r).strip() for r in (required_namespaces or DEFAULT_NAMESPACES) if str(r).strip()] + required = list(dict.fromkeys(req)) + connected_set = set(connected) + proof: dict[str, dict[str, bool]] = {} - missing: list[str] = [] - for s in connected: - is_attached = s in attached - proof[s] = {"connected": True, "attached": is_attached} - if not is_attached and s in req: - missing.append(s) - - for r in req: + proof[s] = {"connected": True, "attached": s in attached} + for r in required: if r not in proof: - proof[r] = {"connected": r in connected, "attached": r in attached} - if r in connected and r not in attached and r not in missing: - missing.append(r) + proof[r] = {"connected": r in connected_set, "attached": r in attached} - attachment_healthy = len(missing) == 0 and len(connected) > 0 - discovery_status = ( - "namespaces_attached" if attachment_healthy - else ("connected_but_namespaces_missing" if len(connected) > 0 else "disconnected") + # The genuine #708 condition: the host reports the service Connected and the namespace + # still never entered the active session tool surface. + missing = [r for r in required if r in connected_set and r not in attached] + # A separate condition: the service is required but absent from the connected-service + # inventory. Nothing here is Connected, so this must not borrow the #708 wording or its + # recovery — see ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED. + not_connected = [r for r in required if r not in connected_set] + # Evidence that disagrees with itself: reported attached to the session while absent + # from the connected inventory. Neither statement is proof, so it fails closed. + contradictory = [r for r in not_connected if r in attached] + + # No required namespace may lack attachment proof and still be called healthy. + attachment_healthy = ( + not missing + and not not_connected + and len(connected) > 0 + and len(required) > 0 ) + if attachment_healthy: + discovery_status = DISCOVERY_STATUS_ATTACHED + elif not connected: + discovery_status = DISCOVERY_STATUS_DISCONNECTED + elif not_connected: + discovery_status = DISCOVERY_STATUS_NOT_CONNECTED + else: + discovery_status = DISCOVERY_STATUS_CONNECTED_MISSING + + # Every condition actually present is reported; ``error_type`` names the primary one. + # Not-connected outranks Connected-but-unattached because a service that never + # connected cannot be recovered by attaching its namespace. + error_types: list[str] = [] + if not_connected: + error_types.append(ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED) + if missing: + error_types.append(ERROR_CONNECTED_NAMESPACES_MISSING) + if not attachment_healthy and not error_types: + error_types.append(ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED) + error_type = None if attachment_healthy else error_types[0] + + # Per-namespace verdict, so a mutation gate never has to infer one namespace's state + # from a whole-session summary. Each entry states only what its own evidence supports. + namespace_conditions: dict[str, dict[str, Any]] = {} + for ns_name, state in proof.items(): + ns_connected = bool(state["connected"]) + ns_attached = bool(state["attached"]) + if ns_connected and ns_attached: + condition = None + elif ns_connected: + condition = ERROR_CONNECTED_NAMESPACES_MISSING + else: + condition = ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED + namespace_conditions[ns_name] = { + "namespace": ns_name, + "required": ns_name in required, + "connected": ns_connected, + "attached": ns_attached, + "attachment_healthy": ns_connected and ns_attached, + "condition": condition, + "contradictory_evidence": ns_attached and not ns_connected, + } + # Startup-ordering race: a namespace that finished connecting *after* the session # tool snapshot was taken cannot be in that snapshot, however healthy it looks now. connected_at = { @@ -156,6 +220,9 @@ def assess_connected_namespace_attachment( reasons: list[str] = [] remediation: list[str] = [] + if not connected: + reasons.append("No MCP servers reported Connected.") + remediation.append("Start or reconnect Gitea MCP servers in client config.") if missing: reasons.append( f"MCP server(s) {missing} report Connected at host/CLI layer but tool namespaces " @@ -167,11 +234,28 @@ def assess_connected_namespace_attachment( "(gitea_whoami -> gitea_resolve_task_capability -> task). " "Do not use direct imports, CLI API mutations, profile hopping, or session file overrides." ) - elif not connected: - reasons.append("No MCP servers reported Connected.") - remediation.append("Start or reconnect Gitea MCP servers in client config.") - else: - reasons.append("All connected MCP server namespaces are attached to the active session.") + if not_connected: + reasons.append( + f"required MCP namespace(s) {not_connected} are absent from the connected-service " + "inventory: nothing reports them Connected, so there is no attachment to claim and " + "the Connected-but-unattached condition does not apply to them (#708)." + ) + remediation.append( + f"Connect the required MCP server(s) {not_connected} through the client, then " + "reconnect the IDE/client MCP session so their namespaces attach " + f"(sanctioned path: {SANCTIONED_ATTACH_RECOVERY_TOOL}), and re-run full preflight. " + "Do not use direct imports, CLI API mutations, profile hopping, or session file overrides." + ) + if contradictory: + reasons.append( + f"contradictory evidence for namespace(s) {contradictory}: reported attached to the " + "active session while absent from the connected-service inventory; neither statement " + "is proof, so attachment is treated as unproven (fail closed, #708)." + ) + if attachment_healthy: + reasons.append( + "All required MCP server namespaces are connected and attached to the active session." + ) if startup_ordering_race: reasons.append( @@ -193,15 +277,29 @@ def assess_connected_namespace_attachment( "connected_servers": connected, "attached_session_namespaces": list(attached), "missing_namespaces": missing, + "not_connected_namespaces": not_connected, + "contradictory_namespaces": contradictory, "proof_of_connected_vs_attached": proof, - "error_type": None if attachment_healthy else ERROR_CONNECTED_NAMESPACES_MISSING, + "namespace_conditions": namespace_conditions, + "error_type": error_type, + "error_types": error_types, "reasons": reasons, "remediation": remediation, "exact_next_action": ( - "Reconnect the IDE/client MCP session so tool namespaces attach to the active session. " - "Do not use direct imports, CLI API mutations, profile hopping, or session-state overrides." - if not attachment_healthy - else "None; session tool namespaces attached." + "None; session tool namespaces attached." + if attachment_healthy + else ( + f"Connect the required MCP server(s) {not_connected} through the client, then " + "reconnect the IDE/client MCP session so their tool namespaces attach to the " + "active session. Do not use direct imports, CLI API mutations, profile hopping, " + "or session-state overrides." + if not_connected + else ( + "Reconnect the IDE/client MCP session so tool namespaces attach to the active " + "session. Do not use direct imports, CLI API mutations, profile hopping, or " + "session-state overrides." + ) + ) ), "unsafe_fallback_policy": UNSAFE_FALLBACK_WARNING, "sanctioned_recovery_tool": SANCTIONED_ATTACH_RECOVERY_TOOL, @@ -215,8 +313,11 @@ def assess_connected_namespace_attachment( "telemetry": { "connected_count": len(connected), "attached_count": len(attached), - "required_count": len(req), + "required_count": len(required), "missing_count": len(missing), + "not_connected_count": len(not_connected), + "contradictory_count": len(contradictory), + "required_attached_count": sum(1 for r in required if r in attached), "discovery_status": discovery_status, "discovery_cache_hit": ( None if discovery_cache_hit is None else bool(discovery_cache_hit) @@ -230,7 +331,8 @@ def assess_connected_namespace_attachment( "auto_attach_attempted": bool(auto_attach_attempted), "auto_recovered": auto_recovered, "startup_ordering_race": startup_ordering_race, - "error_type": None if attachment_healthy else ERROR_CONNECTED_NAMESPACES_MISSING, + "error_type": error_type, + "error_types": error_types, }, } @@ -261,14 +363,39 @@ def attachment_gate_from_session( return [] if entry.get("attached") and entry.get("attachment_healthy"): return [] - detail = entry.get("error_type") or ERROR_CONNECTED_NAMESPACES_MISSING - return [ - f"live MCP namespace '{ns}' is recorded {detail}: the host reports Connected but the " - f"namespace is not attached to the active session tool surface; reconnect the " - f"IDE/client MCP session and re-run preflight before {(task or 'mutation')} " - "(fail closed, #708)", - UNSAFE_FALLBACK_WARNING, - ] + + what = task or "mutation" + connected = entry.get("connected") + detail = entry.get("condition") or entry.get("error_type") + if connected is True: + # Only here has anything actually reported the service Connected, so only here may + # the block say so. + detail = detail or ERROR_CONNECTED_NAMESPACES_MISSING + blocked = ( + f"live MCP namespace '{ns}' is recorded {detail}: the host reports Connected but the " + f"namespace is not attached to the active session tool surface; reconnect the " + f"IDE/client MCP session and re-run preflight before {what} " + "(fail closed, #708)" + ) + elif connected is False: + detail = ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED + blocked = ( + f"live MCP namespace '{ns}' is recorded {detail}: it is absent from the " + f"connected-service inventory, so it is neither connected nor attached and no " + f"Connected status is claimed for it; connect the required MCP server, then " + f"reconnect the IDE/client MCP session and re-run preflight before {what} " + "(fail closed, #708)" + ) + else: + # Connected status was never recorded. Refuse without asserting either condition. + detail = detail or ERROR_CONNECTED_NAMESPACES_MISSING + blocked = ( + f"live MCP namespace '{ns}' is recorded {detail} with no connected-status evidence, " + f"so attachment to the active session tool surface is unproven; reconnect the " + f"IDE/client MCP session and re-run preflight before {what} " + "(fail closed, #708)" + ) + return [blocked, UNSAFE_FALLBACK_WARNING] diff --git a/tests/test_issue_708_mcp_namespace_attachment.py b/tests/test_issue_708_mcp_namespace_attachment.py index 3d26b3a..e5a8fcd 100644 --- a/tests/test_issue_708_mcp_namespace_attachment.py +++ b/tests/test_issue_708_mcp_namespace_attachment.py @@ -4,11 +4,16 @@ import mcp_namespace_health def test_assess_connected_namespace_attachment_success(): + # required_namespaces is declared explicitly: these three are the namespaces this case + # is about. Relying on the default (which also requires gitea-tools) would ask for a + # healthy verdict covering a required namespace that was never connected — exactly the + # false-healthy classification these tests now forbid. connected = ["gitea-author", "gitea-reviewer", "gitea-merger"] attached = ["gitea-author", "gitea-reviewer", "gitea-merger"] res = mcp_namespace_health.assess_connected_namespace_attachment( connected_servers=connected, attached_session_namespaces=attached, + required_namespaces=connected, ) assert res["success"] is True assert res["attachment_healthy"] is True @@ -19,11 +24,14 @@ def test_assess_connected_namespace_attachment_success(): def test_assess_connected_namespace_attachment_missing(): + # Every required namespace here *is* connected, so the only condition present is the + # #708 one: Connected at the host, absent from the session tool surface. connected = ["gitea-author", "gitea-reviewer", "gitea-merger"] attached = ["gitea-author"] res = mcp_namespace_health.assess_connected_namespace_attachment( connected_servers=connected, attached_session_namespaces=attached, + required_namespaces=connected, ) assert res["success"] is False assert res["attachment_healthy"] is False diff --git a/tests/test_issue_708_not_connected_classification.py b/tests/test_issue_708_not_connected_classification.py new file mode 100644 index 0000000..02bcc39 --- /dev/null +++ b/tests/test_issue_708_not_connected_classification.py @@ -0,0 +1,283 @@ +"""Regression tests for Issue #708 B2: not-connected is not Connected-but-unattached. + +The first #708 slice counted a namespace as ``missing`` only when it appeared in +``connected_servers``. A *required* namespace absent from that inventory was therefore +never counted at all, so the session reported ``attachment_healthy: true`` with +``error_type: None`` while holding no attachment proof for it — and the gate text told the +operator "the host reports Connected" about a service nothing had reported Connected. + +These tests pin the corrected distinction: + +* required and not connected never yields a healthy verdict, +* it is typed ``mcp_required_namespaces_not_connected``, never the #708 condition, +* ``mcp_connected_namespaces_missing`` stays reserved for genuinely Connected namespaces, +* both categories still fail review and merge closed, with their own reason, +* per-namespace ``connected``/``attached`` evidence is reported accurately, +* one session's evidence cannot clear another session's block. +""" + +import mcp_namespace_health + + +ROLES = ["gitea-author", "gitea-reviewer", "gitea-merger", "gitea-tools"] + +NOT_CONNECTED = mcp_namespace_health.ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED +CONNECTED_MISSING = mcp_namespace_health.ERROR_CONNECTED_NAMESPACES_MISSING + + +def _assess(connected, attached, required): + return mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=connected, + attached_session_namespaces=attached, + required_namespaces=required, + ) + + +def _store(assessment): + """The recorder contract: per-namespace verdicts feed the gate.""" + return dict(assessment["namespace_conditions"]) + + +# --- the four evidence combinations ---------------------------------------- + + +def test_connected_and_attached_is_healthy(): + res = _assess(ROLES, ROLES, ROLES) + assert res["attachment_healthy"] is True + assert res["error_type"] is None + assert res["missing_namespaces"] == [] + assert res["not_connected_namespaces"] == [] + assert res["discovery_status"] == "namespaces_attached" + + +def test_connected_but_unattached_keeps_the_708_condition(): + res = _assess(ROLES, ["gitea-author"], ROLES) + assert res["attachment_healthy"] is False + assert res["error_type"] == CONNECTED_MISSING + assert res["not_connected_namespaces"] == [] + assert sorted(res["missing_namespaces"]) == [ + "gitea-merger", + "gitea-reviewer", + "gitea-tools", + ] + assert res["discovery_status"] == "connected_but_namespaces_missing" + + +def test_required_but_not_connected_is_never_healthy(): + """The reviewer's exact reproduction from review 637.""" + res = _assess( + ["gitea-reviewer"], ["gitea-reviewer"], ["gitea-reviewer", "gitea-merger"] + ) + assert res["attachment_healthy"] is False + assert res["success"] is False + assert res["error_type"] is not None + assert res["telemetry"]["not_connected_count"] == 1 + + +def test_required_but_not_connected_is_typed_distinctly(): + res = _assess( + ["gitea-reviewer"], ["gitea-reviewer"], ["gitea-reviewer", "gitea-merger"] + ) + assert res["error_type"] == NOT_CONNECTED + assert res["discovery_status"] == "required_namespaces_not_connected" + assert res["not_connected_namespaces"] == ["gitea-merger"] + # Not collapsed into the #708 condition, nor into config drift (#672) or + # transport-closed (#584). + assert CONNECTED_MISSING not in res["error_types"] + assert res["missing_namespaces"] == [] + assert res["error_type"] not in {"mcp_config_drift", "transport_closed"} + + +def test_not_connected_reason_makes_no_connected_claim(): + res = _assess( + ["gitea-reviewer"], ["gitea-reviewer"], ["gitea-reviewer", "gitea-merger"] + ) + about_merger = [r for r in res["reasons"] if "gitea-merger" in r] + assert about_merger, "the not-connected namespace must be named in the reasons" + for reason in about_merger: + assert "report Connected at host/CLI layer" not in reason + assert "gitea-merger" in res["exact_next_action"] + + +def test_neither_connected_nor_attached_reports_disconnected(): + res = _assess([], [], ROLES) + assert res["attachment_healthy"] is False + assert res["discovery_status"] == "disconnected" + assert res["error_type"] == NOT_CONNECTED + assert sorted(res["not_connected_namespaces"]) == sorted(ROLES) + assert res["missing_namespaces"] == [] + + +# --- mixed required namespaces --------------------------------------------- + + +def test_mixed_connected_attached_and_not_connected(): + res = _assess( + ["gitea-author"], ["gitea-author"], ["gitea-author", "gitea-merger"] + ) + assert res["attachment_healthy"] is False + assert res["not_connected_namespaces"] == ["gitea-merger"] + assert res["missing_namespaces"] == [] + conditions = res["namespace_conditions"] + assert conditions["gitea-author"]["attachment_healthy"] is True + assert conditions["gitea-author"]["condition"] is None + assert conditions["gitea-merger"]["attachment_healthy"] is False + assert conditions["gitea-merger"]["condition"] == NOT_CONNECTED + + +def test_both_conditions_present_are_both_reported(): + """One namespace Connected-but-unattached, another never connected.""" + res = _assess( + ["gitea-author", "gitea-reviewer"], + ["gitea-author"], + ["gitea-author", "gitea-reviewer", "gitea-merger"], + ) + assert res["missing_namespaces"] == ["gitea-reviewer"] + assert res["not_connected_namespaces"] == ["gitea-merger"] + # Neither condition is hidden by the other; error_type names the primary one. + assert sorted(res["error_types"]) == sorted([NOT_CONNECTED, CONNECTED_MISSING]) + assert res["error_type"] == NOT_CONNECTED + + +# --- unknown, partial, malformed, contradictory evidence -------------------- + + +def test_unknown_attachment_evidence_is_not_healthy(): + """No session tool surface reported at all is unproven, not proven good.""" + res = _assess(ROLES, None, ROLES) + assert res["attachment_healthy"] is False + assert res["telemetry"]["attached_count"] == 0 + assert res["error_type"] == CONNECTED_MISSING + + +def test_partial_evidence_gates_only_the_unproven_roles(): + res = _assess(ROLES, ["gitea-author", "gitea-tools"], ROLES) + store = _store(res) + assert mcp_namespace_health.attachment_gate_from_session("work_issue", store) == [] + assert mcp_namespace_health.attachment_gate_from_session("review_pr", store) + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + + +def test_malformed_namespace_entries_are_discarded_not_trusted(): + """Blank and whitespace-only names must not become namespaces or proof.""" + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=["gitea-author", "", " "], + attached_session_namespaces=["gitea-author", ""], + required_namespaces=["gitea-author", " "], + ) + assert "" not in res["proof_of_connected_vs_attached"] + assert " " not in res["proof_of_connected_vs_attached"] + assert res["attachment_healthy"] is True + assert res["telemetry"]["required_count"] == 1 + + +def test_contradictory_attached_without_connected_fails_closed(): + """Attached in the session yet absent from the connected inventory.""" + res = _assess(["gitea-author"], ["gitea-author", "gitea-merger"], ROLES) + assert res["attachment_healthy"] is False + assert "gitea-merger" in res["contradictory_namespaces"] + assert "gitea-merger" in res["not_connected_namespaces"] + assert res["namespace_conditions"]["gitea-merger"]["contradictory_evidence"] is True + assert res["namespace_conditions"]["gitea-merger"]["attachment_healthy"] is False + assert any("contradictory evidence" in r for r in res["reasons"]) + assert res["telemetry"]["contradictory_count"] >= 1 + + +def test_duplicate_required_entries_are_counted_once(): + res = _assess(["gitea-author"], ["gitea-author"], ["gitea-author", "gitea-author"]) + assert res["telemetry"]["required_count"] == 1 + assert res["attachment_healthy"] is True + + +# --- review and merge behaviour for both failure categories ----------------- + + +def test_merge_fails_closed_when_required_namespace_never_connected(): + store = _store(_assess(["gitea-author"], ["gitea-author"], ROLES)) + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + assert reasons + assert NOT_CONNECTED in reasons[0] + assert "fail closed, #708" in reasons[0] + + +def test_review_fails_closed_when_required_namespace_never_connected(): + store = _store(_assess(["gitea-author"], ["gitea-author"], ROLES)) + reasons = mcp_namespace_health.attachment_gate_from_session("review_pr", store) + assert reasons + assert NOT_CONNECTED in reasons[0] + assert "fail closed, #708" in reasons[0] + + +def test_not_connected_block_never_claims_the_host_reports_connected(): + store = _store(_assess(["gitea-author"], ["gitea-author"], ROLES)) + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + assert "the host reports Connected" not in reasons[0] + assert "absent from the connected-service inventory" in reasons[0] + + +def test_connected_but_unattached_block_still_says_connected(): + store = _store(_assess(ROLES, ["gitea-author"], ROLES)) + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + assert CONNECTED_MISSING in reasons[0] + assert "the host reports Connected" in reasons[0] + + +def test_both_categories_offer_only_the_sanctioned_recovery(): + for store in ( + _store(_assess(ROLES, [], ROLES)), + _store(_assess(["gitea-author"], ["gitea-author"], ROLES)), + ): + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + joined = " ".join(reasons) + assert "reconnect" in joined.lower() + assert "Workflow Safety Hard Stop (#708)" in joined + for forbidden in ("pkill", "chmod", "curl ", ".env", "sys.path"): + assert forbidden not in reasons[0] + + +def test_entry_without_connected_evidence_blocks_without_asserting_either(): + """A legacy/partial store entry must fail closed and claim nothing it cannot prove.""" + store = {"gitea-merger": {"namespace": "gitea-merger", "attached": False}} + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + assert reasons + assert "no connected-status evidence" in reasons[0] + assert "the host reports Connected" not in reasons[0] + assert "fail closed, #708" in reasons[0] + + +# --- session isolation ------------------------------------------------------ + + +def test_one_session_evidence_does_not_clear_another_session_block(): + """Attachment evidence is per-session state; it must not travel between sessions.""" + blocked_session = _store(_assess(["gitea-author"], ["gitea-author"], ROLES)) + healthy_session = _store(_assess(ROLES, ROLES, ROLES)) + + assert ( + mcp_namespace_health.attachment_gate_from_session("merge_pr", healthy_session) + == [] + ) + # The healthy session's verdict is not consulted for the blocked session. + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", blocked_session) + # And the blocked session's store is unchanged by the healthy one existing. + assert blocked_session["gitea-merger"]["attachment_healthy"] is False + + +def test_gate_reads_only_the_store_it_is_given(): + healthy_session = _store(_assess(ROLES, ROLES, ROLES)) + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", {}) == [] + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", None) == [] + assert ( + mcp_namespace_health.attachment_gate_from_session("merge_pr", healthy_session) + == [] + ) + + +# --- telemetry stays secret-free ------------------------------------------- + + +def test_not_connected_telemetry_leaks_no_secrets(): + res = _assess(["gitea-author"], ["gitea-author"], ROLES) + blob = repr(res["telemetry"]).lower() + for leak in ("token", "authorization", "password", "secret", "/users/", "http"): + assert leak not in blob