fix(deep-research): source-routing guard recognizes sandbox #306
Conversation
…al key Under a sandbox provider, CompositeBackend strips the /shared/ route before StateBackend, so source_routing.json is stored at /source_routing.json. The guard only checked /shared/source_routing.json, so _routing_complete never became true and the orchestrator was blocked on every tool call in sandboxed runs. Accept both key forms; add a regression test for the route-local key. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
🧰 Additional context used📓 Path-based instructions (5)**/*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*test*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
src/aiq_agent/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/aiq_agent/agents/**/*⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
WalkthroughThe SourceRoutingGuardMiddleware's routing-completion check is extended to accept a sandbox-local source-routing file path ( ChangesSandbox source-routing key support
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant Handler
participant SourceRoutingGuardMiddleware
participant AgentState
Handler->>SourceRoutingGuardMiddleware: tool call request
SourceRoutingGuardMiddleware->>AgentState: check files for _SOURCE_ROUTING_STATE_KEYS
AgentState-->>SourceRoutingGuardMiddleware: /shared/source_routing.json or /source_routing.json present
SourceRoutingGuardMiddleware->>Handler: allow normal tool call
Handler-->>SourceRoutingGuardMiddleware: ToolMessage
Related issues: None provided. Related PRs: None provided. Suggested labels: None provided. Suggested reviewers: None provided. Poem: A rabbit hopped through paths anew, one shared, one sandboxed, both checked through. Two keys now open up the gate, no longer must the shared file wait. Hop, hop, test passed — celebrate! 🐇 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
soumilinandi
left a comment
There was a problem hiding this comment.
tested PR with this, LGTM!
Overview
Sandboxed deep-research runs could get stuck: the orchestrator was rejected on every tool call with "Source routing is required before any other tool call," so it never made progress.
Root cause seems to be a key-name mismatch:
state["files"]for
/shared/source_routing.json.CompositeBackenddrops the/shared/prefix before storing the file, so it actually lands under
/source_routing.json.keep the
/shared/prefix, so they were fine — this only hit sandbox runs.)Fix: the guard now accepts both key forms (
/shared/source_routing.jsonand thesandbox
/source_routing.json). Small, no behavior change for non-sandbox runs.Validation
pytest test_custom_middleware.py test_factory.py→ 52 passed, including a newtest that seeds the sandbox key
/source_routing.jsonand confirms the guard opens.ruff checkon changed files → clean.I ran the relevant local checks or explained why they are not applicable.
I added or updated tests for behavior changes.
I updated documentation for user-facing or contributor-facing changes.
I confirmed this PR does not include secrets, credentials, or internal-only data.
I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with
git commit -sor an equivalent sign-off.Where should reviewers start?
'custom_middleware.py
→SourceRoutingGuardMiddleware._routing_complete. Context for why the key gets stripped is indeepagents_runtime.py::_normalize_state_files`.Related Issues
Summary by CodeRabbit