Add audit-emission test coverage for External Results write endpoints + spec doc update

[Copilot]

Closes the remaining gap from #302: the audit emission code in external_results.py is already correct, but had no assertive test coverage for the case.create, case.create.idempotent, case.update, and artifact.upload paths. The spec doc also omitted the idempotent action from the taxonomy table.

New tests (test_external_results_audit.py)

Per-endpoint audit-emission tests

Four tests mirroring the existing test_session_start_writes_audit shape — monkeypatch write_audit, drive the endpoint happy-path, assert the captured audit entry:

• test_case_create_writes_audit — verifies action, actor_kind, actor_id, and that details contains session_id, outcome, external_id, auto_registered
• test_case_create_idempotent_writes_separate_audit — POSTs same payload twice; asserts exactly 1 × case.create + 1 × case.create.idempotent, second response is HTTP 200
• test_case_update_writes_audit — asserts case.update with previous_outcome/new_outcome in details
• test_artifact_upload_writes_audit — mocks get_storage to a no-op SimpleNamespace; asserts all five required detail fields

Parametrized enforcement test

test_state_changing_endpoint_emits_audit introspects the live router at collection time:

@pytest.mark.parametrize("method,path,_route", list(_state_changing_routes()))
def test_state_changing_endpoint_emits_audit(monkeypatch, db_session, write_token, method, path, _route):
    ...
    assert len(captured) >= 1, (
        f"{method} {path} did not call write_audit. "
        f"Every state-changing External Results endpoint must emit an audit entry. "
        f"If this endpoint is intentionally read-only, add it to _READ_ONLY_OPS."
    )

Produces 5 parametrized cases (one per write endpoint). Adding a new write endpoint without write_audit automatically fails this test. _READ_ONLY_OPS is the explicit allowlist for intentionally non-audited endpoints.

Setup prerequisite calls (create session/case) use the same mocked write_audit; captured.clear() is called immediately before the endpoint under test to prevent setup noise from masking a missing write_audit.

Spec doc (external_results_v1.md)

Added the missing row to the action taxonomy table in § h:

Action	Trigger
external_results.case.create.idempotent	POST /case returning 200 (duplicate external_id)

Original prompt

Goal

Close the remaining gap from #302: add the audit-emission test coverage for external_results.case.create, external_results.case.create.idempotent, external_results.case.update, and external_results.artifact.upload, plus a regression-proof parametrized enforcement test that fails by default if a future endpoint is added without an audit call. Update the spec doc to list the idempotent action.

Closes: #302

Decisions already made — do not re-open

• The audit-emission code is already correct. Don't refactor external_results.py. The router already calls write_audit on every state-changing endpoint. This PR is test-and-doc only, with one tiny spec edit. If you find a real bug in write_audit calls, file a separate issue.
• Use the existing test pattern in backend/tests/api/test_external_results_audit.py — monkeypatch.setattr("app.api.external_results.write_audit", fake_write_audit) + capture list. Don't invent a new fixture pattern.
• The regression-proof enforcement test is the load-bearing deliverable. Without it, this issue's value is just incremental coverage. Make sure that test (a) enumerates every state-changing endpoint by inspecting external_results.router.routes, (b) calls each one happy-path, (c) asserts at least one write_audit call captured per endpoint. If a future PR adds a 7th write endpoint and forgets write_audit, this test must fail.
• Don't add audit calls on GET endpoints. Reads are intentionally not audited. The enforcement test must skip GET methods.
• Don't change the audit_log model. The CheckConstraint enforcing actor_kind invariants is already correct.

Scope

1. Add per-endpoint audit-emission tests (in backend/tests/api/test_external_results_audit.py)

Add four new tests, mirroring the existing test_session_start_writes_audit shape:

• test_case_create_writes_audit — POST /external-results/case (happy path with external_id + requirement_external_ids to also exercise the diagnostic detail fields). Asserts action == "external_results.case.create", actor_kind == "runner_token", actor_id == token_model.id, and that details includes at least session_id, outcome, external_id, auto_registered.
• test_case_create_idempotent_writes_separate_audit — POST /external-results/case twice with the same external_id + same session_id. The second call returns 200, and a external_results.case.create.idempotent audit must be captured. Both audits must appear (1 .create + 1 .create.idempotent).
• test_case_update_writes_audit — POST a case, then PATCH it with a new outcome. Asserts external_results.case.update is captured with previous_outcome and new_outcome in details.
• test_artifact_upload_writes_audit — POST a case result, then POST /external-results/artifact (multipart) with a small in-memory PNG. Asserts external_results.artifact.upload is captured with case_result_id, kind, filename, content_type, size_bytes in details.

For the artifact test: use io.BytesIO(b"\x89PNG\r\n\x1a\n" + b"\x00" * 64) or similar; declare kind=screenshot, filename=test.png, content_type=image/png. The smoke test in CI already exercises the full storage round-trip — this test only needs to confirm audit emission, so monkeypatching write_audit and get_storage (to a no-op SimpleNamespace returning a fake StorageResult) is fine.

2. Add the regression-proof enforcement test

Add test_every_state_changing_endpoint_emits_audit:

import pytest
from app.api.external_results import router as external_results_router

# Endpoints that intentionally do NOT audit (reads):
_READ_ONLY_OPS = frozenset({
    ("GET", "/external-results/session/{session_id}"),
    ("GET", "/external-results/case/{case_result_id}"),
})


def _state_changing_routes():
    """Yield (method, path, route) for every non-GET endpoint in the router."""
    for route in external_results_router.routes:
        if not hasattr(route, "methods"):
            continue
        for method in route.methods:
            if method == "HEAD":
                continue
            if (method, route.path) in _READ_ONLY_OPS:
                continue
            if method == "GET":
                continue
            yield method, route.path, route


@pytest.mark.parametrize("method,path,_route", list(_state_changing_routes()))
def test_state_changing_endpoint_emits_audit(monkeypatch, db_session, write_token, method, path, _route):
    """Every state-changing endpoint MUST call write_audit at least once.

    If a new endpoint is added without an audit call, this test fails by default
    — that's the point. To intentionally skip auditing on a new read endpoint,
    add it to _READ_ONLY_OPS.
    """
    captured: list[dict] = []

    async def fake_write_audit(db, **kwargs):
        captured.append(kwargs)
        return SimpleNamespace(id=uuid.uuid4())

    monkeypatch.setattr("app.api.external_results...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

[bg-playground]

LGTM ✅ — all 11 checks green including External Results contract smoke.

Acceptance check:

Criterion	Status
Four per-endpoint audit tests added	✅
Parametrized enforcement test introspects router at collection time	✅
Test parametrizes to 5 cases (one per write endpoint)	✅
Failure message tells future devs exactly what to do	✅
Spec doc § h includes external_results.case.create.idempotent	✅
Test-and-doc only — no router or model changes	✅

Two pieces of careful engineering worth flagging:

1. captured.clear() immediately before each endpoint-under-test call. Prerequisite setup (creating a session before patching it) also goes through the mocked write_audit, so without clear() the assertion len(captured) >= 1 would silently pass even if the endpoint under test forgot to audit. The clear discipline is what makes this test actually load-bearing.

2. else: raise AssertionError on unknown dispatch. A new write endpoint added without both a write_audit call and a happy-path driver will fail twice with explicit messages. Cannot accidentally ship an unaudited write endpoint.

Closes #302. Merging.