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

727 tests   727 ✅  10s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 0056beb.

♻️ 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.

[James Chapman (JC-wk)]

This extra read was added in response to opus feedback to "make success depend on a definitive successful probe operation".
implementation with await container.read() + query_items("SELECT TOP 1 * FROM c") is the only approach that fully satisfies the Opus 4.8 code review. It ensures we have a definitive probe that prevents masked failures, while still allowing the health check to succeed on an, empty database.

[Marcus Robinson (marrobi)]

Wonder what the RU consumption would be related to this... how often does it run etc.

[Marcus Robinson (marrobi)]

Opus:

The extra await container.read() isn't needed to satisfy the "definitive probe" requirement — iterating the query (async for ... break) already forces a network round-trip to Cosmos, and any failure surfaces as ServiceRequestError/CosmosHttpResponseError and is caught below. An empty container is still a successful probe (the request goes out; the loop body just doesn't run), so read() adds no correctness value.

It does add cost: an extra round-trip on every poll and potentially broader RBAC requirements, which could fail the check for permission reasons unrelated to actual availability. I'd drop read() and use a minimal projection:

async for _ in container.query_items("SELECT TOP 1 c.id FROM c", max_item_count=1):
    break

The unit tests will need updating to match.

[James Chapman (JC-wk)]

The costs were only pennies but I'm happy to revert it if Opus has now changed it's mind

[Marcus Robinson (marrobi)]

Leave it to you to decide.

[James Chapman (JC-wk)]

reverted

[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.