Skip to content

[AAASM-6415] 📝 (readme): Document required LangChain callback wiring - #387

Open
Chisanan232 wants to merge 7 commits into
mainfrom
v0.0.1/AAASM-6415/fix/quickstart_denied_tool_wiring
Open

Chisanan232 wants to merge 7 commits into
mainfrom
v0.0.1/AAASM-6415/fix/quickstart_denied_tool_wiring

Conversation

@Chisanan232

@Chisanan232 Chisanan232 commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Outcome

The README Quick Start said init_assembly() sends every LangChain tool call through the policy gate. That is false: init_assembly() activates the LangChain adapter and creates one AssemblyCallbackHandler, but installs no hook into LangChain. A denied tool still runs unless the handler is passed in callbacks. This PR corrects the docs and adds a test that pins the behaviour. No runtime or public API change.

Evidence

  • Source: runtime.py:auto_inject_callback_handler only stores the handler singleton; no LangChain hook is registered anywhere in the SDK (QA finding D5, AAASM-6415).
  • New test test/unit/test_quickstart_langchain_denied_tool_wiring.py runs the real init_assembly (real RuntimeQueryInterceptor, repo's fake native runtime as decision source), takes the handler from the langchain adapter on the returned context, and invokes a real langchain_core StructuredTool whose body writes a file:
    • test_denied_tool_without_wiring_still_executes (negative control, documents the gap): deny, handler not passed, file written.
    • test_denied_tool_with_wiring_has_no_side_effect: deny, handler in config={"callbacks": [handler]}, ToolExecutionBlockedError, no file.
    • test_allowed_tool_with_wiring_executes: positive control.
  • Local: full suite 1448 passed, 22 skipped; ruff check/format clean; pre-commit all hooks pass; mypy agent_assembly reports 5 pre-existing errors in files this PR does not touch (4 are missing native _core stub, not run against a built extension here).
  • Docs changed: README Quick Start snippet and explanation, README Public API sentence, docs/index.md "What it wraps".

Not verified / limits

  • The README snippet itself (AgentExecutor + create_react_agent) was not executed: langchain, langchain-classic, langchain-community are not in this environment. The mechanism is verified with a langchain_core tool via the callback manager; passing config={"callbacks": [handler]} to AgentExecutor.invoke relies on LangChain's standard config propagation to tools.
  • The test places a bare langchain module in sys.modules only to satisfy FrameworkAdapter.is_available(); the adapter, handler and invocation path are real.
  • The fake native runtime is the decision source; no real gateway/runtime or published core binary was exercised. Core pin in native/aa-ffi-python/Cargo.toml: aa-core rev 0044d515d020. No Rust build was done.
  • Only the LangChain fact is stated; for other frameworks the docs now say to check that framework's adapter page. No per-adapter claim is made (LangGraph, MCP etc. were not checked for a similar wiring gap).
  • The test uses a bare StructuredTool, not AgentExecutor; propagation of config={"callbacks": [...]} through an agent executor is not verified.
  • This does not by itself resolve D5; it makes the docs true and pins the gap. Whether init_assembly() should auto-wire LangChain (e.g. via a configure hook) is a separate design question.

Needs owner decision (no code here)

Silent fail-open: init_assembly() succeeds and reports the LangChain adapter active, yet tools are ungoverned unless callbacks is passed. Options: (a) docs-only plus a follow-up ticket (this PR), (b) auto-wire via LangChain register_configure_hook (runtime contract change), (c) a runtime/doctor warning when the handler has not been seen on a tool run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TLpw696KjUDUg4QjH83En4

Chisanan232 and others added 2 commits November 1, 2026 18:11
init_assembly() activates the LangChain adapter and creates its
AssemblyCallbackHandler but installs no hook into LangChain, so a denied
tool still runs unless the handler is passed in callbacks. Correct the
Quick Start snippet and the claims that said otherwise.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TLpw696KjUDUg4QjH83En4
@codecov

codecov Bot commented Nov 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sonarqubecloud

sonarqubecloud Bot commented Nov 1, 2026

Copy link
Copy Markdown

@Chisanan232

Copy link
Copy Markdown
Contributor Author

OWNER-MERGE-REVIEW head=60c5bbe56ed67399812c148b256a6e779e62b39d verdict=PASS blocking=0 reviewer=claude-independent-agent

Independent exact-head review (separate read-only agent) of the docs-only correction for D5 / AAASM-6415, over four iterations (60aae12 -> ea9b261 -> 47b8cf4 -> f3a39cf -> 60c5bbe): final verdict PASS, no Critical/High/Medium. PR = README.md, docs/index.md, docs/concepts/architecture.md, docs/concepts/index.md, docs/examples/framework-support.md, docs/examples/langchain-basic-agent.md and one test file; no runtime change. Claims now limited to what was verified against source (init_assembly() installs no hook into LangChain; handler must be passed via callbacks; denied tool with wiring raises ToolExecutionBlockedError and has no side effect, negative control proves the gap). Not verified, and stated in the docs: the AgentExecutor config={callbacks} propagation (langchain-classic not installed in the verification env); decisions come from the repo's fake native runtime. Open product question (silent fail-open on LangChain without callbacks: docs-only vs auto-wire vs runtime warning) is recorded in the PR body for the owner and is NOT resolved by this PR. D5 is not claimed resolved by this merge. Merging requires the owner's formal GitHub approval on this head (repo rule).

🤖 Generated with Claude Code

https://claude.ai/code/session_01TLpw696KjUDUg4QjH83En4

This branch has not been deployed

No deployments
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