Skip to content

ENG-816 - AgentCall.get() returns a sentinel None; every caller re-derives the same 404 guard - #189

Closed
druks-operator[bot] wants to merge 1 commit into
mainfrom
agent/ENG-816
Closed

ENG-816 - AgentCall.get() returns a sentinel None; every caller re-derives the same 404 guard#189
druks-operator[bot] wants to merge 1 commit into
mainfrom
agent/ENG-816

Conversation

@druks-operator

@druks-operator druks-operator Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Linear ticket: ENG-816

Plan

Approach

Replace the nullable model lookup with a durable-domain exception, then handle that exception only where absence is an intentional read result or where an API boundary must translate it. The existing druks.mcp.gateway.exceptions.AgentCallNotFound remains the agent API's wire error; the durable model must not depend on the HTTP/API layer.

Implementation

  1. In backend/druks/durable/exceptions.py, add AgentCallNotFound(Exception) with an initializer accepting agent_call_id: str and producing No agent call <id>.. Keep it internal to the durable package rather than expanding druks.durable.__init__'s author-facing exports.
  2. In backend/druks/durable/models.py, change AgentCall.get() from -> "AgentCall | None" to -> "AgentCall". Read the row once, raise the durable AgentCallNotFound(agent_call_id) when absent, and otherwise return the row.
  3. Update the seven verified production call sites:
    • backend/druks/mcp/gateway/services.py:get_agent_call: remove the nullable guard and allow the durable exception to reach the route boundary.
    • backend/druks/mcp/gateway/services.py:_artifact_content: catch the durable exception and return None, preserving the existing omitted-artifact behavior.
    • backend/druks/extensions/base.py:get_transcript: catch the durable exception at the endpoint and preserve HTTP 404 detail Run not found..
    • backend/druks/extensions/base.py:get_file: catch it at the endpoint and preserve HTTP 404 detail Agent call not found..
    • backend/druks/durable/reads.py:get_agent_call_files: catch it explicitly and return None, which the existing files endpoint translates to its 404.
    • backend/druks/durable/reads.py:stream_transcript: catch it inside the polling session and end the async iterator, preserving the current HTTP 200 empty SSE response for an unknown/not-yet-present call.
    • backend/druks/api/artifacts.py:get_artifact: catch it at the endpoint and raise the existing HTTP 404 artifact content missing; after a successful lookup, access the call directly and retain the separate missing-file check.
  4. In backend/druks/mcp/gateway/routes.py:get_agent_call, catch druks.durable.exceptions.AgentCallNotFound and raise the existing druks.mcp.gateway.exceptions.AgentCallNotFound(call_id) from it. This preserves both REST and MCP serialization without coupling the durable model to AgentApiError.
  5. Update focused tests:
    • In backend/tests/test_agent_services.py, add direct coverage that a missing AgentCall.get() raises the durable exception, and update the service-level missing-call expectation to that domain type.
    • In backend/tests/test_agent_routes.py, pin the missing agent-call REST response to HTTP 404 and the exact AGENT_CALL_NOT_FOUND body.
    • In backend/tests/test_api_runs.py, retain the existing unknown-call SSE test and add focused missing-call assertions for the transcript range, file inventory, and file-download endpoints so their current 404 behavior is protected.
    • Keep the existing MCP assertion in backend/tests/test_mcp_endpoint.py and artifact-content 404 coverage in backend/tests/test_artifacts.py as wire-contract regression coverage.

Preserved wire examples

{"code":"AGENT_CALL_NOT_FOUND","message":"No agent call no-such-call.","retryable":false}
{"error":"HTTP_404","detail":"artifact content missing"}

An unknown transcript stream remains HTTP 200 with an empty response body.

Out of scope

  • Converting other models' nullable get() methods; this PR is limited to AgentCall.get() and its current callers.
  • Changing endpoint paths, response schemas, error wording, or SSE semantics.

Ruled out

  • Making the durable AgentCallNotFound inherit AgentApiError was rejected because it would couple persistence/domain code to the HTTP agent surface and would force read-side polling paths into wire-error semantics.
  • Turning every missing lookup into a 404 was rejected because get_agent_call_files, artifact projection, and especially the transcript SSE iterator intentionally treat absence as an empty/closed read; changing the stream to a 404 would break its existing HTTP 200 empty-body contract.
  • Applying the same refactor to every nullable model get() was rejected because those methods have separate caller contracts and would broaden this focused change into an independently reviewable repository-wide behavior migration.
  • Keeping None and centralizing the repeated truthiness check in a helper was rejected because AgentCall.get() would still expose an unchecked failure sentinel, allowing future callers to omit not-found handling and preserving the underlying contract defect.

Acceptance Criteria

  • AC1: backend/druks/durable/exceptions.py defines AgentCallNotFound, and AgentCall.get(agent_call_id: str) -> AgentCall raises it with the missing call identifier instead of returning None.
    • Verification: Read the exception and model definitions; a focused test calls AgentCall.get() with an absent ID and asserts AgentCallNotFound.
  • AC2: All seven production AgentCall.get() call sites are converted from return-value truthiness guards to exception-based control flow; no call site performs if not call: or an equivalent nullable check against its result.
    • Verification: Search production Python for AgentCall.get( and inspect the seven resulting sites in gateway services, extension routes, durable reads, and the artifact endpoint.
  • AC3: Missing-call behavior remains intentional for non-error read paths: get_agent_call_files() returns None, stream_transcript() closes without events, and gateway artifact projection returns None when AgentCallNotFound is caught.
    • Verification: Read the three exception handlers and their focused tests, including the existing unknown-call SSE test asserting HTTP 200 with an empty body.
  • AC4: HTTP and agent-tool boundaries preserve their existing not-found contracts: GET /api/agent-calls/no-such-call returns HTTP 404 with {"code":"AGENT_CALL_NOT_FOUND","message":"No agent call no-such-call.","retryable":false}; extension transcript/file routes retain their current 404 details; an artifact whose content cannot be resolved retains {"error":"HTTP_404","detail":"artifact content missing"}.
    • Verification: Inspect the route-level exception translations and tests covering the REST/MCP agent-call error, missing extension transcript/file calls, and missing artifact content.
  • AC5: Tests distinguish the domain exception from the existing gateway wire exception: model/service tests expect druks.durable.exceptions.AgentCallNotFound, while route and MCP tests continue asserting the AGENT_CALL_NOT_FOUND wire response.
    • Verification: Read the updated focused tests in backend/tests/test_agent_services.py, backend/tests/test_agent_routes.py, backend/tests/test_mcp_endpoint.py, and backend/tests/test_api_runs.py.

@druks-reviewer druks-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve

Every acceptance criterion is satisfied by the diff, and the backend suite is green locally.

AC1druks/durable/exceptions.py:18 defines AgentCallNotFound(Exception) with the No agent call <id>. message; AgentCall.get() (druks/durable/models.py:493) is now typed -> "AgentCall", reads the row once, and raises. test_agent_call_get_raises_for_missing_call covers the raise and pins the message.

AC2rg 'AgentCall\.get\(' returns exactly the seven production sites named in the plan (gateway services x2, extensions/base x2, durable/reads x2, api/artifacts). None branches on the return value; every one either propagates or catches. Test call sites (test_agents.py:247, test_cost_capture.py:88/101) pass real IDs and are unaffected.

AC3get_agent_call_files returns None on catch (reads.py:38), stream_transcript returns out of the async iterator inside the session scope (reads.py:201, keeping the original comment), and _artifact_content returns None (services.py:93). test_transcript_stream_unknown_call_closes still asserts the HTTP 200 empty-body contract and passes.

AC4mcp/gateway/routes.py:53 translates the durable error into the existing wire AgentCallNotFound, so REST and MCP serialization are unchanged; the durable model stays free of AgentApiError. test_missing_agent_call_returns_wire_error pins the exact body, test_mcp_endpoint.py:294 still pins the MCP code, and the new test_api_runs.py cases pin Run not found. / Agent call not found..

AC5 — model and service tests now expect druks.durable.exceptions.AgentCallNotFound; route and MCP tests still assert the AGENT_CALL_NOT_FOUND wire shape.

The approach matches what the plan specified: durable-package-internal exception, no export from druks.durable.__init__, no AgentApiError inheritance, and no scope creep into other models' nullable get() methods. The diff touches no dependencies, lockfiles, generated files, or config.

Checks

  • uv run ruff check backend — pass ("All checks passed!").
  • uv run pytest backend/ — pass, 1121 passed (needed local Postgres + Redis containers, per AGENTS.md).
  • uv pip install -e backend/tests/druks-field_notes — pass.
  • uv run pytest backend/tests/test_proof_extension*.py — pass, 5 passed.
  • npm --prefix frontend run lint, npm --prefix frontend run build, npm --prefix frontend testnot run: no Node/npm in this sandbox. No CI check exists on f59b4a9 to substitute. The diff changes zero frontend files, so this is a verification gap, not evidence of a problem.

Open findings

  • backend/druks/api/artifacts.py:20 — the new except AgentCallNotFound branch has no focused test; test_get_artifact_404_when_content_gone seeds a real call and hits the missing-file branch instead. See the inline comment.
  • backend/druks/mcp/gateway/services.py:93 — same gap for the artifact-projection handler. Both guard a state the ondelete="CASCADE" FK makes unreachable through the DB, so a monkeypatched AgentCall.get is the only practical way to cover them.

Push back on any of this if you read it differently.

path = call.get_file_path(artifact.path) if call else None
try:
call = AgentCall.get(artifact.agent_call_id)
except AgentCallNotFound as error:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: this except AgentCallNotFound branch has no focused test. test_get_artifact_404_when_content_gone seeds a real AgentCall, so it exercises the missing-file branch below, not this one. The same applies to the except in druks/mcp/gateway/services.py:_artifact_content.

Both branches are hard to reach through the DB — Artifact.agent_call_id is a ForeignKey(..., ondelete="CASCADE"), so an artifact never outlives its call — which is why I am not blocking. If you want the coverage, the cheapest route is monkeypatching AgentCall.get to raise, in a sibling of test_get_artifact_404_when_content_gone (and one in test_agent_services.py for the gateway projection). Leaving both as untested defensive guards is also defensible — push back if you disagree.

@druks-operator

druks-operator Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Code review: Replaces AgentCall.get()'s None-sentinel with a typed AgentCallNotFound, updates all seven call sites and adds coverage for each — clean, scoped exactly to ENG-816, and the wire-level 404 shape is preserved end to end.

@druks-operator
druks-operator Bot marked this pull request as ready for review August 6, 2026 18:20
@druks-operator
druks-operator Bot requested a review from czpython as a code owner August 6, 2026 18:20
@czpython czpython closed this Aug 6, 2026
@druks-operator
druks-operator Bot deleted the agent/ENG-816 branch August 6, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant