From 99838d7bb642174e162cc11c5080ef309042f0cc Mon Sep 17 00:00:00 2001 From: Doga Gursoy Date: Tue, 2 Jun 2026 17:12:33 +0300 Subject: [PATCH 1/2] fix(tests): catch 3 contract-drift failures on main Three tests on main were red, blocking unrelated PRs from getting clean CI signal: 1. tests/e2e/test_family_and_asset.py: asserts body["families"], but GET /assets/{id} now serializes the field as "family_ids" after a recent rename. Updated assertion. 2. tests/contract/test_conduct_procedure_endpoint.py: expects 200 with a lifecycle-failure body when conducting an unregistered procedure, but the conductor re-raises ProcedureNotFoundError which the BC's exception handler maps to 404. Updated to expect 404 with the procedure_id in detail. Per project_conduct_procedure_test_contract_drift memory, this drift was documented but not fixed. 3. tests/contract/test_conduct_procedure_mcp_tool.py: same drift on the MCP path. Expected structuredContent on a path that now raises; FastMCP surfaces the error as isError=true with the message in content. Updated assertion shape to match the standard MCP error contract used by every other test in the file. No code changes; tests catch up to shipped behavior. Co-Authored-By: Claude Opus 4.7 --- .../contract/test_conduct_procedure_endpoint.py | 16 +++++++--------- .../contract/test_conduct_procedure_mcp_tool.py | 17 ++++++++--------- apps/api/tests/e2e/test_family_and_asset.py | 2 +- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/apps/api/tests/contract/test_conduct_procedure_endpoint.py b/apps/api/tests/contract/test_conduct_procedure_endpoint.py index 6089ceb43..d93ea7930 100644 --- a/apps/api/tests/contract/test_conduct_procedure_endpoint.py +++ b/apps/api/tests/contract/test_conduct_procedure_endpoint.py @@ -106,22 +106,20 @@ def test_post_conduct_with_setpoint_to_unconnected_address_returns_not_connected @pytest.mark.contract -def test_post_conduct_against_unregistered_procedure_returns_200_with_lifecycle_failure() -> None: - """conduct() catches start_procedure rejections -> lifecycle failure on result.""" +def test_post_conduct_against_unregistered_procedure_returns_404() -> None: + """conduct() re-raises start_procedure's ProcedureNotFoundError so the + BC's central exception handler maps it to 404. Earlier shape (200 with + lifecycle failure on the body) was rejected by routes.py wiring: see + [[project_conduct_procedure_test_contract_drift]] memory.""" with TestClient(create_app()) as client: unknown_pid = uuid4() run = client.post( f"/procedures/{unknown_pid}/conduct", json={"steps": []}, ) - assert run.status_code == 200 + assert run.status_code == 404 payload = run.json() - assert payload["succeeded"] is False - failure = payload["failure"] - assert failure["step_index"] is None - assert failure["source_kind"] == "lifecycle" - assert failure["target"] == "start" - assert failure["error_class"] == "ProcedureNotFoundError" + assert str(unknown_pid) in payload["detail"] @pytest.mark.contract diff --git a/apps/api/tests/contract/test_conduct_procedure_mcp_tool.py b/apps/api/tests/contract/test_conduct_procedure_mcp_tool.py index d6ec1d920..adaea51e9 100644 --- a/apps/api/tests/contract/test_conduct_procedure_mcp_tool.py +++ b/apps/api/tests/contract/test_conduct_procedure_mcp_tool.py @@ -106,8 +106,11 @@ def test_mcp_conduct_procedure_with_unknown_action_returns_failure_in_structured @pytest.mark.contract -def test_mcp_conduct_procedure_against_unregistered_procedure_returns_lifecycle_failure() -> None: - """conduct() catches start_procedure rejection -> lifecycle failure on result.""" +def test_mcp_conduct_procedure_against_unregistered_procedure_returns_iserror() -> None: + """conduct() re-raises ProcedureNotFoundError; FastMCP surfaces as isError. + Earlier shape (200-with-lifecycle-failure structured content) was rejected + by routes.py wiring: see [[project_conduct_procedure_test_contract_drift]] + memory.""" with TestClient(create_app()) as client: headers = open_session(client) unknown_pid = uuid4() @@ -127,10 +130,6 @@ def test_mcp_conduct_procedure_against_unregistered_procedure_returns_lifecycle_ }, headers=headers, ) - structured: dict[str, Any] = parse_sse_data(response.text)["result"]["structuredContent"] - assert structured["succeeded"] is False - failure = structured["failure"] - assert failure["step_index"] is None - assert failure["source_kind"] == "lifecycle" - assert failure["target"] == "start" - assert failure["error_class"] == "ProcedureNotFoundError" + body = parse_sse_data(response.text) + assert body["result"]["isError"] is True + assert str(unknown_pid) in body["result"]["content"][0]["text"] diff --git a/apps/api/tests/e2e/test_family_and_asset.py b/apps/api/tests/e2e/test_family_and_asset.py index d9531a335..4c0138753 100644 --- a/apps/api/tests/e2e/test_family_and_asset.py +++ b/apps/api/tests/e2e/test_family_and_asset.py @@ -39,4 +39,4 @@ async def test_register_asset_then_add_family_round_trips( assert fetched.status_code == 200 body = fetched.json() assert body["id"] == str(asset_id) - assert body["families"] == [str(family_id)] + assert body["family_ids"] == [str(family_id)] From 71dd14fc338423721b7039ddbf672a797d32f5f5 Mon Sep 17 00:00:00 2001 From: Doga Gursoy Date: Tue, 2 Jun 2026 20:03:59 +0300 Subject: [PATCH 2/2] fix(tests): catch 5 more main failures (Actor rename + Campaign race) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main moved forward while CI was running on the prior commit: - `refactor(access): rename Actor.is_active to Actor.active` (4ea068e55) broke 4 test sites that read the old field name: * tests/contract/test_get_actor_endpoint.py (×2 assertions) * tests/contract/test_get_actor_mcp_tool.py * tests/e2e/test_actor_lifecycle.py Updated to `active`. - `tests/integration/test_add_run_to_campaign_postgres.py::test_concurrent_add_runs_to_same_campaign_settle_atomically` was a known-fragile happy-race test (the docstring itself warned that "if a future change introduces true parallelism, one call would ConcurrencyError and require operator-side retry"). CI hit that case. Rewrote each gather call to retry on ConcurrencyError (max 3 attempts), pinning the documented OCC retry contract rather than the lucky no-conflict shape. The forced-conflict sibling test still pins the immediate-failure path. No production code changes; tests catch up to shipped behavior. Co-Authored-By: Claude Opus 4.7 --- .../tests/contract/test_get_actor_endpoint.py | 4 +- .../tests/contract/test_get_actor_mcp_tool.py | 2 +- apps/api/tests/e2e/test_actor_lifecycle.py | 2 +- .../test_add_run_to_campaign_postgres.py | 51 +++++++++++-------- 4 files changed, 33 insertions(+), 26 deletions(-) diff --git a/apps/api/tests/contract/test_get_actor_endpoint.py b/apps/api/tests/contract/test_get_actor_endpoint.py index 0e5f72f0a..d7731d6ca 100644 --- a/apps/api/tests/contract/test_get_actor_endpoint.py +++ b/apps/api/tests/contract/test_get_actor_endpoint.py @@ -26,7 +26,7 @@ def test_get_actor_returns_200_with_actor_response() -> None: "id": str(actor_id), "name": "Doga", "kind": "human", - "is_active": True, + "active": True, } @@ -38,7 +38,7 @@ def test_get_actor_reflects_deactivation() -> None: response = client.get(f"/actors/{actor_id}") assert response.status_code == 200 - assert response.json()["is_active"] is False + assert response.json()["active"] is False @pytest.mark.contract diff --git a/apps/api/tests/contract/test_get_actor_mcp_tool.py b/apps/api/tests/contract/test_get_actor_mcp_tool.py index 507030eec..e63e748e9 100644 --- a/apps/api/tests/contract/test_get_actor_mcp_tool.py +++ b/apps/api/tests/contract/test_get_actor_mcp_tool.py @@ -70,7 +70,7 @@ def test_mcp_get_actor_tool_returns_structured_actor_for_known_id() -> None: assert structured["id"] == str(actor_id) assert structured["name"] == "Doga" assert structured["kind"] == "human" - assert structured["is_active"] is True + assert structured["active"] is True @pytest.mark.contract diff --git a/apps/api/tests/e2e/test_actor_lifecycle.py b/apps/api/tests/e2e/test_actor_lifecycle.py index f19bcdbd7..240af1916 100644 --- a/apps/api/tests/e2e/test_actor_lifecycle.py +++ b/apps/api/tests/e2e/test_actor_lifecycle.py @@ -29,7 +29,7 @@ async def test_register_then_get_then_list( "id": str(actor_id), "name": "Doga", "kind": "human", - "is_active": True, + "active": True, } # LIST is projection-backed; drain so the bookmark catches up before diff --git a/apps/api/tests/integration/test_add_run_to_campaign_postgres.py b/apps/api/tests/integration/test_add_run_to_campaign_postgres.py index c3fce307f..0cb49a063 100644 --- a/apps/api/tests/integration/test_add_run_to_campaign_postgres.py +++ b/apps/api/tests/integration/test_add_run_to_campaign_postgres.py @@ -460,19 +460,22 @@ async def test_concurrent_add_runs_to_same_campaign_settle_atomically( db_pool: asyncpg.Pool, ) -> None: """asyncio.gather on two add_run_to_campaign calls for the same - Campaign + different Runs: under the shared-pool serialization - that asyncpg's per-connection acquire enforces, both succeed and - end up as members. Pins the happy-race path of the multi-stream - OCC contract. - - Earlier shipped as `xfail strict=False` per Watch #15 deferral; - promoted to a real test in the Tier 2 cleanup once the XPASS - became the documented behavior. The forced-version-conflict - sibling test (`test_forced_concurrent_add_runs_raises_concurrency_error`) - pins the OCC failure path explicitly. + Campaign + different Runs: under true parallelism one call may lose + the optimistic-version race and raise ConcurrencyError, which the + operator (or a calling saga) is expected to retry. This test pins + that contract end-to-end: both Runs land as members via the + documented retry path, never via a lucky no-conflict shape. + + Earlier shipped as `xfail strict=False` per Watch #15 deferral. + The forced-version-conflict sibling test + (`test_forced_concurrent_add_runs_raises_concurrency_error`) pins + the OCC failure-on-first-attempt path explicitly; this test pins + the operator-retries-and-both-land convergent path. """ import asyncio + from cora.infrastructure.ports.event_store import ConcurrencyError + campaign_id = uuid4() run_ids = [uuid4(), uuid4()] lead = uuid4() @@ -501,18 +504,22 @@ async def test_concurrent_add_runs_to_same_campaign_settle_atomically( await _seed_run_via_event_store(deps, run_id, event_id=uuid4()) add = add_run_to_campaign.bind(deps) - await asyncio.gather( - add( - AddRunToCampaign(campaign_id=campaign_id, run_id=run_ids[0]), - principal_id=_PRINCIPAL_ID, - correlation_id=_CORRELATION_ID, - ), - add( - AddRunToCampaign(campaign_id=campaign_id, run_id=run_ids[1]), - principal_id=_PRINCIPAL_ID, - correlation_id=_CORRELATION_ID, - ), - ) + + async def _add_with_retry(run_id: UUID) -> None: + for _attempt in range(3): + try: + await add( + AddRunToCampaign(campaign_id=campaign_id, run_id=run_id), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + return + except ConcurrencyError: + continue + msg = f"add_run_to_campaign(run={run_id}) did not settle after 3 retries" + raise AssertionError(msg) + + await asyncio.gather(*[_add_with_retry(rid) for rid in run_ids]) # End-state: both Runs landed as Campaign members. Single-pool # asyncio serialization makes this the realistic semantic today;