Audit provenance can disagree between tool_calls and query_executions: record_tool_call's source= argument outranks the ContextVar, the query-log writer has no such parameter

[sandeep-agami]

Summary

record_tool_call accepts an explicit source= argument that outranks the call-source ContextVar; the query_executions writer has no such parameter and reads the ContextVar only. An embedder that states its provenance by passing the argument rather than by scoping the ContextVar produces two audit rows, for one execution, that disagree about what drove it.

Severity: sev-3. No data is lost or exposed and both rows are still written — one metadata column can be inconsistent between two tables, and only under a caller pattern no in-repo caller uses today. Filed rather than hotfixed because it is an audit-integrity claim in a governance product, and the commit that introduced the seam asserts the opposite in its message.

Root cause

Two writers, two different resolution rules (packages/agami-core/src/tools.py, main @ f8ae324):

# tools.py:1357 — record_tool_call → tool_calls
"source": current_call_source() if source is None else source,

# tools.py:1057 — _finalize_execution → query_executions
"source": current_call_source(),

AH-022 (#169) added both routes. Its reasoning holds for the ContextVar and is stated in the commit message:

_record_query had no parameter path whatsoever (its source was a dict literal built inside _finalize_execution), so the source is also readable from a ContextVar. The two logs therefore cannot disagree about what drove one execution.

That conclusion follows only while the ContextVar is the sole route. It is not: record_tool_call also takes source=, and the parameter wins. test_an_explicit_source_beats_the_contextvar pins that precedence, so the divergent path is deliberate and tested on one side of the pair.

Impact

Any embedder that dispatches tool handlers itself and states provenance via the argument. The two logs are joinable per execution, so a reviewer asking "what drove this query" gets one answer from tool_calls and a different one from query_executions, with nothing marking either as the less authoritative. The failure is silent: both writes succeed.

Not reached by the MCP transports, which pass neither the argument nor a scope, so every row reads mcp_server on both tables.

Reproduction

Against a configured install (app DB + model + warehouse), with no ContextVar scope set:

body = json.loads(tools.tool_execute_sql(
    {"sql": "SELECT id FROM orders", "datasource": "acme", "raw_query": "how many orders"}))

tools.record_tool_call(
    name="execute_sql",
    arguments={"sql": "SELECT id FROM orders", "datasource": "acme"},
    result_text=json.dumps(body), execution_ms=1, actor="a",
    source="embedded",                     # the argument route, not the ContextVar route
)

Observed:

query_executions.source = ['mcp_server']
tool_calls.source       = ['embedded']

Expected: both rows carry embedded, or the API offers one route so the two cannot be stated independently.

Notes for the fix

Three options, in rough order of how structural the guarantee becomes:

1. Drop source= from record_tool_call, leaving set_call_source() as the only route. Makes the invariant hold by construction rather than by convention. Breaking change to a seam shipped in 0.5.3.
2. Give the query_executions writer the same override, threaded from the tool edge. Symmetric, but adds a parameter no in-repo caller passes, and two independently-settable values can still be set inconsistently — it makes agreement possible, not guaranteed.
3. Leave the API and enforce the convention at the embedder ("scope it, do not pass it"), documenting the argument as tool-call-only.

Whichever is chosen, the assertion in AH-022's commit message and any docstring repeating it should be narrowed to the route that actually carries it.

Note for whoever picks this up: ACE-035 moves the query_executions write from _finalize_execution to _emit (so refusals are recorded at all) and carries the current_call_source() read across with it. The line number moves; the asymmetry does not. ACE-035 adds test_the_audit_row_carries_the_scoped_call_source, which pins the ContextVar route on all three statuses and names this gap in its docstring without closing it.

• Regression test / golden case added so it can't silently return