Skip to content

[AAASM-5787] 🐛 (adapters): Build an audit record on the deny branch of eight adapters - #320

Merged
Chisanan232 merged 19 commits into
mainfrom
v0.0.1/AAASM-5787/fix/adapter_deny_records
Aug 14, 2026
Merged

[AAASM-5787] 🐛 (adapters): Build an audit record on the deny branch of eight adapters#320
Chisanan232 merged 19 commits into
mainfrom
v0.0.1/AAASM-5787/fix/adapter_deny_records

Conversation

@Chisanan232

@Chisanan232 Chisanan232 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Description

AAASM-5750 wired the SDK interceptors' audit sink so a governed call's record reaches the runtime's event channel. In python-sdk that only helped where a record was constructed — and eight of the eleven governing adapters returned or raised at their if status != "allow": branch before reaching their record helper. A denied tool call built nothing, so the sink had nothing to carry however it was configured.

This adds agent_assembly/adapters/_shared/audit_record.py — a leaf module (it imports no adapter, so the _shared.tool_governancecrewai.patch cycle does not apply; _shared.positional_args is the same shape) — and calls it from each of the eight deny branches.

Two things it deliberately does not do:

  • No Observed claim, anywhere. Handing a record to the hook is a handoff. The send is unacknowledged, and AAASM-5783 is open on report_event payloads reaching neither the live stream nor the durable entry. What this PR establishes is narrower and checkable: the deny branch now builds a record and hands it over, which is upstream of the question 5783 asks.
  • No unguarded call. The helper suppresses exceptions internally rather than at each call site. These calls are being inserted where none existed; a raising duck-typed hook would otherwise replace a decided deny with its own exception, and a caller matching on PolicyViolationError / ToolExecutionBlockedError would stop recognising the deny. That question was settled for the openai_agents path under AAASM-4782 and is followed here rather than re-answered.

One extra fix outside the eight

openai_agents was already one of the three that recorded on a deny, but the record it built carried no marker — indistinguishable from a tool that ran and returned the denial text. It now passes denied=True, offered only to hooks that can receive it. Without this the allowed/denied control below could not discriminate for that adapter.

Type of Change

  • 🔧 Bug fix

Breaking Changes

  • No

The public API is unchanged. The denied keyword is offered only to audit hooks whose signature accepts it (an explicit parameter or a **kwargs catch-all), so a caller-supplied hook written against the older call keeps working.

Related Issues

Testing

  • Unit tests added/updated

test/unit/adapters/test_deny_record_conformance.py — a matrix over the existing failopen_conformance driver registry, which already invokes each adapter's real governance wrapper against a fake framework tool. Three cases per adapter:

  1. an authoritative deny builds a record carrying denied=True;
  2. the control that moves with it — an allowed call also records, and its record carries no denial marker. Both paths populate the hook, so "a record exists" decides nothing on its own; a single-sided assertion would pass against an adapter that recorded the allowed outcome twice;
  3. a deny whose audit hook raises still records and still blocks (AAASM-4782's invariant, extended to the eight new branches).

The registry's existing completeness check (test_every_adapter_has_a_driver) already fails when a new adapter package ships without a driver, so a twelfth adapter cannot arrive un-measured here either.

Proven able to fail — one mutation per adapter

Each adapter's record construction was severed in turn (rebinding that module's record function to a no-op), the matrix re-run, and the file restored. Exactly one case reddened each time, and it was that adapter's own:

severed failing case
crewai test_a_policy_deny_builds_a_record_in_that_adapters_own_branch[crewai]
haystack [haystack]
smolagents [smolagents]
llamaindex [llamaindex]
agno [agno]
microsoft_agent_framework [microsoft_agent_framework]
mcp [mcp]
langchain [langchain]
openai_agents [openai_agents]

No cross-talk: severing one adapter never reddened another's case. That is what makes this a per-adapter control rather than one control standing in for eleven.

Documentation claim class — population found 11, population fixed 11

The ticket named six sites. Enumerating the class by what a sentence asserts (rather than by one phrasing) found five more:

# site in ticket?
1 README.md
2 docs/index.md — landing note
3 docs/index.md — "a handoff is not evidence" callout
4 docs/concepts/architecture.md
5 .claude/CLAUDE.md
6 agent_assembly/core/runtime_interceptor.py
7 agent_assembly/core/audit_sink.py ❌ found here
8 agent_assembly/adapters/_shared/tool_governance.py ❌ found here
9 docs/guides/authoring-adapters.md ❌ found here
10 agent_assembly/adapters/haystack/patch.py (module docstring) ❌ found here
11 agent_assembly/adapters/llamaindex/adapter.py (class docstring) ❌ found here

docs/guides/authoring-adapters.md is the one worth calling out: it told a new adapter author that most adapters do not record on the denied path. It now points at the shared helper and says to pass denied=True.

No claim-binding control in this repo referenced any of these sentences — python-sdk has no claim-vocabulary gate, which is AAASM-5784's subject, so there was nothing to move in lockstep. Stating that explicitly rather than leaving it implied.

Also

  • ♻️ commit folds truncate_result_for_audit / accepts_keyword into the leaf module, so the two copies that would otherwise exist stay one (the cross-file duplication SonarCloud flagged on PR [AAASM-4734] 🐛 (adapters): Fail closed on unrecognized verdict and missing interceptor #269 / AAASM-4746 is the precedent).
  • The langchain conformance driver now calls on_tool_end after a permitted start, as LangChain itself does. Without it the driver modelled a framework that never completes a tool, and the allowed path looked like it recorded nothing.
  • test_terminal_pending_blocks_tool_and_does_not_run (crewai) asserted recorded_results == [] to mean "the tool never ran". That held only while the deny branch recorded nothing at all, so it now asserts what was recorded: the rejection message, and no tool output.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic
  • Documentation updated if needed
  • All tests passing

Most adapters return or raise at their `if status != "allow":` branch before
reaching a record helper, so a denied tool call built no record for the sink to
forward (AAASM-5787). This is what those branches will call.

It is a leaf module because `_shared.tool_governance` imports `crewai.patch`,
so an adapter importing the flow module for this would cycle.
`_shared.positional_args` is the same shape.
The deny branch returned the blocked message straight past
`_record_sync_tool_result`, so this adapter built nothing for the sink to
forward (AAASM-5787).

`test_terminal_pending_blocks_tool_and_does_not_run` asserted `recorded_results
== []` to mean "the tool never ran". That held only while the deny branch
recorded nothing at all, so it now asserts what was recorded instead: the
rejection message, and no tool output.
The deny branch returned the blocked message straight past the record call, so this adapter built nothing for the sink to forward (AAASM-5787).
The deny branch returned the blocked message straight past the record call, so this adapter built nothing for the sink to forward (AAASM-5787).
The sync call and async acall wrappers each returned the denied output straight past their record call (AAASM-5787).
execute and aexecute each returned the denied result straight past their record call (AAASM-5787).
The deny branch raised straight past the record call, so this adapter built nothing for the sink to forward (AAASM-5787).
The deny branch raised straight past the record call, so this adapter built nothing for the sink to forward (AAASM-5787).
on_tool_start raised straight out with nothing recorded, and on_tool_end — the hook AAASM-5750 wired to the sink — fires only after a tool has run, so a denied tool never reached it. All three deny paths now route through _deny, which records first and raises unconditionally (AAASM-5787).
This adapter already recorded on the denied path, but the record it built was indistinguishable from a tool that ran and returned the denial text. The flag is offered only to hooks that can receive it (AAASM-5787).
truncate_result_for_audit and accepts_keyword now live in _shared.audit_record, which the deny branches import; tool_governance re-exports them under their private names so its callers and tests are unchanged.
One control per adapter, reaching that adapter's own deny branch — deleting one adapter's record call reddens that adapter's case and no other. The allowed path is the control that moves with it: both populate the hook, so only the denied flag distinguishes them.

The harness gains record capture and run_scenario_capturing_records; the langchain driver now calls on_tool_end after a permitted start, as LangChain does, so its allowed path is comparable (AAASM-5787).
README, the Docs Hub landing note, the architecture page and the adapter-authoring guide all said three of eleven adapters recorded on a deny. Eleven do. The handoff caveat is unchanged: an offer to the hook is still not ADR 0033 §6 Observed while AAASM-5783 is open (AAASM-5787).
Both described the denied path as reached by three adapters. The disposition still says nothing about which branch a record came from — that is what the denial marker is for (AAASM-5787).
tool_governance, haystack and llamaindex each stated their denied calls produce no record (AAASM-5787).
run_id=run_id,
)
if inspect.isawaitable(returned):
await returned
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.03960% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
agent_assembly/adapters/_shared/audit_record.py 91.66% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

Review found the twelfth site: the bolded lead still read "and denied calls are mostly not covered" while both paragraphs it introduces had been corrected. It is the summary a reader takes away, and it contradicted README, the architecture page and .claude/CLAUDE.md. The population in this PR's body was 11 found / 11 fixed; it is 12 / 12 (AAASM-5787).
Several adapters call the hook with just (tool_name, result) on the allowed
path, so a caller-supplied handler written to that contract is entitled to
reject the rest. The deny paths passed four keywords unconditionally, raising
TypeError into a suppression that swallowed it — the record was lost silently,
in exactly the population the deny-record claim is about.

Filtered in all three record paths, not only the new one: the shared async flow
and openai_agents had the same shape and the same defect. openai_agents also
sends `args`, which no other adapter's allowed path carries, so that is
filtered too.

An unreadable signature is not a refusal: a C-implemented callable keeps
receiving agent_id and run_id, and only the newer `denied` flag is withheld
(AAASM-5787).
… record

One case per adapter with a handler accepting only (tool_name, result) and no
**kwargs. It also pins the other half: a hook that cannot receive the denial
marker still receives the record (AAASM-5787).
# so the two copies that would otherwise exist stay one. Re-exported under their
# private names because this module's callers and tests already use those.
_truncate_result_for_audit = truncate_result_for_audit
_accepts_keyword = accepts_keyword
@sonarqubecloud

Copy link
Copy Markdown

@Chisanan232

Copy link
Copy Markdown
Contributor Author

Review round 1 — findings addressed

Independent review found one blocking item and three worth fixing. All addressed; head 84c515d.

The population claim in this PR's body was wrong — it is 12, not 11

F1 (blocking). docs/index.md:78 — the bolded lead of the admonition still read "A handoff is not evidence, and denied calls are mostly not covered." I had rewritten both paragraphs that lead introduces and left the lead itself, twice, in the same admonition. It is the sentence a reader takes away, and it contradicted README.md:18, docs/index.md:66, .claude/CLAUDE.md:42 and docs/concepts/architecture.md:72.

Fixed, and the claim in this body corrected: population found 12, fixed 12. Re-swept after the fix — zero residual.

Worth stating plainly, because it is the defect class this whole programme keeps finding: I enumerated the class by what a sentence asserts, caught eleven sites including five the ticket did not name, and then missed the headline of a block I had edited twice.

F3 — the deny path demanded a wider signature than the allowed path, in three places

Not blocking per the review, but it falsifies the PR's own claim for caller-supplied hooks, so it is fixed.

Several adapters call the audit hook with just (tool_name, result) on the allowed path. A caller-supplied handler written to that contract is entitled to reject agent_id / run_id. The deny path passed them unconditionally → TypeError → swallowed by this module's own suppression → the deny recorded nothing, silently, in exactly the population the "adapters record on deny" claim covers.

optional_audit_kwargs now filters every optional keyword to what the hook can receive, in all three record paths — the new leaf module, _shared/tool_governance, and openai_agents (which also sends args, unique to it, so that is filtered too). The first fix reached only the new module; the two pre-existing paths had the identical shape.

One subtlety the fix had to preserve: an unreadable signature is not a refusal. A C-implemented callable exposes nothing to introspect, and dropping every optional keyword there silently narrows a call that has always carried agent_id and run_id. Those keep flowing; only the newer denied flag is withheld. test_a_hook_with_no_readable_signature_still_receives_the_deny_record caught this on the first attempt.

New control: test_a_hook_written_to_the_allowed_path_contract_still_gets_the_deny_record, one case per adapter with a handler accepting only (tool_name, result) and no **kwargs. On the first run it reddened for google_adk, openai_agents and pydantic_ai — the three that already recorded on deny — which is how the two pre-existing sites surfaced.

F2 — the sync/async mismatch now says what it costs

record_denied_tool_result's .close() path is reachable, and the review measured that the hook body never runs: GeneratorExit at the first suspension point. The docstring said the warning was avoided but not that the record is dropped. It now says so, and names which adapters have an async deny branch to pair such a handler with.

F5 — dead constants removed

_MAX_AUDIT_RESULT_CHARS and _KEYWORD_PARAMETER_KINDS in _shared/tool_governance.py had no consumers left.

F4 — not changed

Narrowing contextlib.suppress(Exception) to the call alone would make a hook-resolution bug observable, which is right in principle. I left it: the suppression's stated job is that a decided deny is final regardless of audit outcome, and narrowing it changes that contract rather than tightening it. Worth its own ticket with the openai_agents AAASM-4782 precedent in view, not a late edit here.

Suite

test/unit + test/integration: 1379 passed, 15 skipped. The two bench-latency cases are excluded here; they fail identically on the unmodified base checkout on this machine.

@Chisanan232
Chisanan232 merged commit 689b589 into main Aug 14, 2026
27 checks passed
@Chisanan232
Chisanan232 deleted the v0.0.1/AAASM-5787/fix/adapter_deny_records branch August 14, 2026 09:22
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