feat: add erp to iag-mcp-demo - #111
Conversation
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Secrets |  |
Aug 31, 2026 10:36p.m. | Review ↗ |
| Python | |
Aug 31, 2026 10:36p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
There was a problem hiding this comment.
Pull request overview
Adds an optional ERP “invoices” capability to the a2a/iag-mcp-demo demo by introducing an ERP MCP server backed by Postgres, wiring it into the agent chain via an orchestrator tool, and extending both insurance and canbank datasets/policies so invoice rows are pre-filtered per caller via AuthZEN search/resource.
Changes:
- Extend insurance/canbank instant-stack datasets with
Invoicenodes andHAS_INVOICE/SERVESrelationships, plus KBAC policies for invoice visibility. - Add an
erpdocker-compose profile (erp-db + erp-mcp + erp-mcp-iag), seed data, and build target; document/demo-script updates for the new “same prompt, different rows” ERP beat. - Add orchestrator routing (
query_erp) and analyst prompt guidance for ERP invoice questions; UI CSS to make markdown tables scroll within chat bubbles.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| instant-stack/data/insurance/relationships.json | Adds invoice/workflow/department relationships (billing + ERP workflow chains). |
| instant-stack/data/insurance/nodes.json | Adds Invoice, Workflow, and Agent nodes for the ERP scenario. |
| instant-stack/data/insurance/manifest.json | Adds invoice-view KBAC policies for staff and customers. |
| instant-stack/data/canbank/relationships.json | Adds invoice relationships and ERP workflow chain wiring for canbank. |
| instant-stack/data/canbank/nodes.json | Adds Invoice, Workflow, and Agent nodes for canbank ERP. |
| instant-stack/data/canbank/manifest.json | Adds staff invoice-view KBAC policy for canbank. |
| a2a/iag-mcp-demo/usecases/insurance/skills/retriever/insurance-authz/SKILL.md | Documents CAN_VIEW Invoice authorization shape for insurance. |
| a2a/iag-mcp-demo/usecases/insurance/skills/orchestrator/query-erp/SKILL.md | New orchestrator skill directing invoice/billing queries to query_erp. |
| a2a/iag-mcp-demo/usecases/insurance/skills/analyst/insurance-authz/SKILL.md | Updates analyst authz documentation for invoice viewing. |
| a2a/iag-mcp-demo/usecases/insurance/skills/analyst/erp-invoices/SKILL.md | New analyst skill for erp_* invoice tools and rules. |
| a2a/iag-mcp-demo/usecases/insurance/DEMO_SCRIPT.md | Adds ERP invoice beats and setup steps to the insurance demo script. |
| a2a/iag-mcp-demo/usecases/canbank/skills/retriever/canbank-authz/SKILL.md | Documents CAN_VIEW Invoice authorization shape for canbank. |
| a2a/iag-mcp-demo/usecases/canbank/skills/orchestrator/query-erp/SKILL.md | New orchestrator skill directing invoice/billing queries to query_erp. |
| a2a/iag-mcp-demo/usecases/canbank/skills/analyst/erp-invoices/SKILL.md | New analyst skill for canbank ERP invoice tools. |
| a2a/iag-mcp-demo/usecases/canbank/skills/analyst/canbank-authz/SKILL.md | Updates analyst authz documentation for canbank invoices. |
| a2a/iag-mcp-demo/usecases/canbank/DEMO_SCRIPT.md | Adds ERP invoice beats and setup steps to the canbank demo script. |
| a2a/iag-mcp-demo/README.md | Documents the erp profile architecture, setup, and troubleshooting. |
| a2a/iag-mcp-demo/orchestrator_agent/orchestrator_agent.py | Adds query_erp tool, routing prompt guidance, and enablement flag. |
| a2a/iag-mcp-demo/makefile | Adds new-erp-mcp build target. |
| a2a/iag-mcp-demo/erp_mcp/server.py | New ERP MCP server implementing AuthZEN-filtered invoice retrieval. |
| a2a/iag-mcp-demo/erp_mcp/requirements.txt | Python dependencies for the ERP MCP server. |
| a2a/iag-mcp-demo/erp_mcp/init.sql | Seeds Postgres invoices table for both usecases. |
| a2a/iag-mcp-demo/erp_mcp/Dockerfile | Containerizes ERP MCP server with non-root user + healthcheck. |
| a2a/iag-mcp-demo/docker-compose.yaml | Adds erp profile services/networks/volumes; wires networks into chatbot/analyst. |
| a2a/iag-mcp-demo/chatbot/static/styles.css | Adds styling so markdown tables scroll within message bubbles. |
| a2a/iag-mcp-demo/analyst_agent/analyst_agent.py | Updates prompt to prefer erp_* for invoices and avoid speculation. |
| a2a/iag-mcp-demo/.example.env | Documents and adds env vars needed to enable the ERP profile/tools. |
Suppressed comments (1)
a2a/iag-mcp-demo/orchestrator_agent/orchestrator_agent.py:809
- The system prompt becomes internally inconsistent: it adds "For invoices… use query_erp" but still says to ALWAYS use query_retriever for "enterprise data". Since invoices can reasonably be interpreted as enterprise data, this conflict can cause the orchestrator to route invoice requests to the retriever instead of ERP despite the new guidance. Clarify the exception (or remove the ambiguous term) in the ALWAYS list.
+ _ERP_PROMPT
+ "ALWAYS use the query_retriever tool for: MCP, MCP tools or resources, data retrieval, "
"internal documents, real-time stock prices, "
"questions about employees, enterprise data, authorization (AuthZEN), knowledge queries, or any "
"internal/knowledge-graph data. Forward the user's question to the retriever and return its response. "
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Trust boundary: the server sits behind its IAG gateway (MCP proxy mode) and | ||
| is not published to the host. The gateway introspects the caller and replaces | ||
| the Authorization header with its own minted delegation token, so the ``sub`` | ||
| claim decoded here (unverified - the gateway already verified it) is the | ||
| authenticated person. Do not expose this server directly. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
a2a/iag-mcp-demo/orchestrator_agent/orchestrator_agent.py:582
_call_analystis defined asasync def _call_analyst(text: str), butquery_erpcurrently passes two positional string arguments due to the comma-separated f-strings. This will raiseTypeError: _call_analyst() takes 1 positional argument but 2 were giventhe first timequery_erpis invoked. Combine the two f-strings into a single argument (implicit concatenation) so only onetextvalue is passed.
return await _call_analyst(
f"ERP invoice request - answer ONLY with the erp_* tools (e.g. erp_list_invoices), "
f"never with knowledge-graph, CIQ or drive tools: {query}",
)
| """Read the allowed invoice rows from Postgres (empty ids -> no query).""" | ||
| if not invoice_ids: | ||
| return [] | ||
| with psycopg.connect(ERP_DB_DSN) as conn, conn.cursor() as cur: |
There was a problem hiding this comment.
psycopg.connect failures aren't caught.
There was a problem hiding this comment.
It is corrected: just not on line 127 itself. psycopg.connect is deliberately left to raise, and the failure is caught one frame up, at the only places that can turn it into a proper answer.
So a connect failure on line 127 no longer escapes as a raw traceback therefore both MCP tools now return the clean {"error": "ERP database unavailable: …"} JSON. That addresses exactly what you flagged ("psycopg.connect failures aren't caught").
Why the catch isn't inside _fetch_rows: the helper returns rows, so on failure it could only (a) return [] which the tools can't tell apart from "this user has no visible invoices", silently converting an outage
into a wrong answer or (b) re-raise, which changes nothing. Catching at the call sites keeps the two outcomes distinguishable. It's the same structure already used for the AuthZEN call (_allowed_invoice_ids raises, the tools catch httpx.HTTPError).
implement [ENG-8488]
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (1)
a2a/iag-mcp-demo/orchestrator_agent/orchestrator_agent.py:582
_call_analystis defined to take a singletext: strargument, butquery_erpcurrently passes two positional arguments because of the comma-separated f-strings. This will raise aTypeErrorthe first timequery_erpis invoked. Combine the prompt pieces into a single string argument (implicit concatenation without a comma, or a single f-string).
return await _call_analyst(
f"ERP invoice request - answer ONLY with the erp_* tools (e.g. erp_list_invoices), "
f"never with knowledge-graph, CIQ or drive tools: {query}",
)
implement [ENG-8488
add erp to iag-mcp-demo