Fix health check query not executed

[James Chapman (JC-wk)]

Resolves #4926

What is being addressed

The existing code container.query_items("SELECT TOP 1 * FROM c") does not actually execute the query so the health check can falsely return OK even if Cosmos is down or inaccessible.

How is this addressed

• Ensure the query runs
• Add tests
• Update CHANGELOG.md
• Increment version

[Copilot]

Pull request overview

This pull request fixes the Cosmos DB health check so it actually executes a query (preventing false “OK” results when Cosmos is unreachable), and updates the API’s unit tests, changelog, and version accordingly.

Changes:

• Execute the Cosmos query by iterating the async result (async for ... break) with max_item_count=1.
• Add unit tests covering Cosmos HTTP errors and query-time request errors.
• Update CHANGELOG.md and bump api_app version.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
api_app/services/health_checker.py	Forces Cosmos query execution during health check and maps CosmosHttpResponseError to “not accessible”.
api_app/tests_ma/test_services/test_health_checker.py	Adds tests for Cosmos HTTP/query-time failures (but one existing “responding” test needs updating to reflect the new iteration behavior).
CHANGELOG.md	Documents the bug fix under Unreleased “BUG FIXES”.
api_app/_version.py	Bumps API version from 0.25.16 to 0.25.17.

[github-actions]

Unit Test Results

766 tests   766 ✅  11s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 991aa84.

♻️ This comment has been updated with latest results.

[Jack Morris (rudolphjacksonm)]

/test

[github-actions]

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/28439249321 (with refid 4403460f)

(in response to this comment from Jack Morris (@rudolphjacksonm))

[Marcus Robinson (marrobi)]

From Opus 4.8:

Blocking concerns: (1) create_state_store_status() still has ambiguous success semantics—ok is returned whenever iteration does not raise, including the zero-result path, which can mask real Cosmos connectivity/auth/partition misconfiguration depending on SDK behavior; please make success depend on a definitive successful probe operation (not just “no exception while iterating”) and add an explicit test for empty results.

(2) The new tests rely on custom async iterator doubles + MagicMock that may drift from actual azure.cosmos.aio.ContainerProxy.query_items behavior and create false confidence across SDK changes; please tighten fidelity (e.g., AsyncMock/shared fixture with realistic async iterable behavior) and assert the exact query_items call contract ("SELECT TOP 1 * FROM c", max_item_count=1) so regressions are caught.

[James Chapman (JC-wk)]

From Opus 4.8:

Blocking concerns: (1) create_state_store_status() still has ambiguous success semantics—ok is returned whenever iteration does not raise, including the zero-result path, which can mask real Cosmos connectivity/auth/partition misconfiguration depending on SDK behavior; please make success depend on a definitive successful probe operation (not just “no exception while iterating”) and add an explicit test for empty results.

(2) The new tests rely on custom async iterator doubles + MagicMock that may drift from actual azure.cosmos.aio.ContainerProxy.query_items behavior and create false confidence across SDK changes; please tighten fidelity (e.g., AsyncMock/shared fixture with realistic async iterable behavior) and assert the exact query_items call contract ("SELECT TOP 1 * FROM c", max_item_count=1) so regressions are caught.

In health_checker.py:

• Added an explicit await container.read() call before iterating over container.query_items .
• Calling container.read() acts as a definitive probe of the state store container (validating credentials, endpoints, and partition existence). It will raise a standard database error (
ServiceRequestError / CosmosHttpResponseError ) if misconfigured, avoiding ambiguous success status when query_items returns an empty result set.

2. High-Fidelity Mocking & Exact Call Contract Assertions

In test_health_checker.py:

• Replaced the custom class-based async iterator double ( AsyncIteratorWithError ) with Python's built-in AsyncMock to configure asynchronous iteration. This uses native mocking behavior (
aiter.return_value = query_results or aiter.side_effect = query_error ) that matches the Azure SDK iterator interface.
• Added assertions for the exact query_items call contract: container_mock.query_items.assert_called_once_with("SELECT TOP 1 * FROM c", max_item_count=1) .
• Added assertions verifying that the new probe operation is invoked: container_mock.read.assert_called_once() .
• Added a new explicit test test_get_state_store_status_empty_results to verify that when the container is empty, the probe completes successfully and returns StatusEnum.ok .

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

api_app/tests_ma/test_services/test_health_checker.py:23

• create_mock_container configures query_items_mock.return_value.__aiter__.return_value with a plain list on a MagicMock return value. async for expects an async iterator (__anext__), so this setup will raise TypeError when create_state_store_status() iterates the results. Use an AsyncMock (or a small async-iterator helper) as the query_items() return value so async iteration works.

    query_items_mock = MagicMock()
    if query_error:
        query_items_mock.return_value.__aiter__.side_effect = query_error
    else:
        query_items_mock.return_value.__aiter__.return_value = query_results or []

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

api_app/tests_ma/test_services/test_health_checker.py:23

• container.query_items() returns an async iterable; in these tests the mocked query_items() currently returns a plain MagicMock, which can behave differently from an async iterable and is inconsistent with other repo tests (they typically return an AsyncMock with __aiter__). Making query_items_mock return an AsyncMock will better match the Cosmos SDK contract and avoid brittle async-iteration behavior.

    query_items_mock = MagicMock()
    if query_error:
        query_items_mock.return_value.__aiter__.side_effect = query_error
    else:
        query_items_mock.return_value.__aiter__.return_value = query_results or []

[Copilot]

🟢 Ready to approve

The fix is small, directly addresses the reported lazy-query issue, and includes targeted unit test coverage for key success and failure paths.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

api_app/tests_ma/test_services/test_health_checker.py:32

• The new health-check query is SELECT TOP 1 VALUE 1 ..., so the mocked query result should be a scalar (e.g., 1) rather than a dict to match the actual SDK return shape and keep the test intent clear.

    container_mock = create_mock_container(query_results=[{"id": "item"}])
    get_container_proxy_mock.return_value = container_mock

CHANGELOG.md:20

• Changelog bullet uses inconsistent capitalization for a generic term; other entries use sentence case after “Fix”, so “health check” should not be capitalized.

* Fix Health check can falsely return OK even if Cosmos is down or inaccessible. ([#4926](https://github.com/microsoft/AzureTRE/issues/4926))

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟢 Approval recommended

The implementation correctly executes the Cosmos probe and covers its success and failure paths; only minor changelog wording remains.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The query is now executed and the relevant success and failure paths are covered by tests.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The required API patch-version increment is missing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The implementation directly addresses the lazy-query defect and covers successful, empty, and failing responses.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced