fix(mcp): remove shadowed tool classes and make product/contract tools read the DB

[ferdinand-van-butzelaar]

What

src/backend/src/tools/data_products.py and src/backend/src/tools/data_contracts.py each define the same tool class twice. 6de81fd1 ("Add LLM CRUD tools for app objects") added a CRUD block at the top of each module and another at the bottom:

data_products.py     GetDataProductTool @17,  DeleteDataProductTool @101
                     GetDataProductTool @520, DeleteDataProductTool @669   ← these run
data_contracts.py    SearchDataContractsTool @16,  GetDataContractTool @139,  DeleteDataContractTool @209
                     SearchDataContractsTool @479, GetDataContractTool @564, DeleteDataContractTool @689   ← these run

Python keeps the last definition. Every maintenance commit since has landed on the copy at the top of the file — the dead one — while the copy at the bottom is what actually executes.

Why it matters

1. Five registered tools silently lost their scope

35c0ea6f ("Add MCP endpoint") introduced required_scope and added it to the shadowed copies only. On development today:

>>> r = create_default_registry()
search_data_products   -> 'data-products:read'
get_data_product       -> '*'     # declared data-products:read on the dead copy
delete_data_product    -> '*'     # declared data-products:write
search_data_contracts  -> '*'     # declared contracts:read
get_data_contract      -> '*'     # declared contracts:read
delete_data_contract   -> '*'     # declared contracts:write

MCPProtocolHandler._has_scope only passes "*" when the token literally carries the wildcard, and tools/list filters by the same check (mcp_routes.py:233). So those five tools are invisible in tools/list and rejected by tools/call for every least-privilege MCP token — a token minted with contracts:read cannot use search_data_contracts at all.

2. The data-contract tool surface is non-functional

The running copies call DataContractsManager.list_contracts() / get_contract() / delete_contract() — the legacy in-memory _contracts dict, not the database. Nothing populates it at runtime: create_contract has no callers, load_from_yaml is never invoked for this manager, and those four tool call sites are the store's only callers in the entire backend.

Verified against a session with a real DataContractDb row present:

search_data_contracts -> {'contracts': [], 'total_found': 0}
get_data_contract     -> "Data contract '<id>' not found"

3. Descriptions and output ports were always empty

DataProductDb.description is a relationship to DescriptionDb and output_ports a relationship to OutputPortDb — not JSON columns. json.loads(...) if isinstance(str) falls through to the ORM object, .get('purpose') is never reached and the isinstance(port, dict) branch never matches. So search_data_products never matched on description text and output_tables was always [].

DataContractDb has no description attribute at all (the ODCS description is split across description_purpose / description_usage / description_limitations), so the shadowed contract tools raised AttributeError: 'DataContractDb' object has no attribute 'description' against real rows. Both copies were broken — just differently.

What this PR does

• Removes the duplicate class definitions, keeping the maintained copies: the ones carrying the #520 junction-table domain handling and the declared required_scope.
• Adds _description_purpose() and _output_port_names() to both modules, accepting the ORM, dict and JSON-string shapes so the tools work regardless of which layer hands them the object.
• Routes delete_data_contract through delete_contract_from_db instead of the in-memory store.
• Fixes and registers list_data_products / list_data_contracts. Both now query the DB the way their search_* siblings do, with any-of domain matching over entity_domain_associations and status/limit filters.

Net: 170 insertions, 447 deletions.

Fixes #660

list_data_products passed domain= / status= to DataProductsManager.list_products, which accepts neither. Worth noting for the issue thread: the tool was never registered (create_default_registry doesn't import it), so the TypeError was unreachable — same for list_data_contracts. Simply dropping the kwargs would not have been enough either: list_products fail-closes to an empty list without caller scope, and ToolContext carries no caller identity, so the tool would have gone from raising to silently returning nothing. Querying the DB directly matches what search_data_products already does and sidesteps that.

Tests

New src/backend/src/tests/unit/test_tool_module_integrity.py (35 tests):

• AST guard — fails if any module in src/tools/ defines a class more than once. This is the guard that would have caught the original mistake.
• Scope guard — fails if any registered tool inherits BaseTool.required_scope instead of declaring its own, plus explicit assertions on the eight product/contract tool scopes.
• DB-backed behaviour tests — insert real DataContractDb / DataProductDb rows and assert the tools find them, match on purpose text, filter by status, and return descriptions and output ports.

Results:

New suite on this branch	35 passed
New suite with only the source changes stashed	18 failed, 17 passed
Full backend suite (backend/src/tests/unit + backend/tests)	1520 passed, 1 skipped

Note on ruff

ruff already flags this as F811 redefined-while-unused — 5 hits in these two files on development, 0 after this change. It went unnoticed because ruff check has been unrunnable since v0.5.4 (Feb 2026):

$ ruff check backend
Cause: TOML parse error at line 128, column 18
128 | target-version = "1.0.1"
unknown variant `1.0.1`, expected one of `py37`, `py38`, ...

scripts/bump_version.py matches (version\s*=\s*")[^"]+(") unanchored, so every release rewrites [tool.ruff] target-version to the app version. It was py310 when the tooling landed in 53fce85b and has been clobbered by every bump since. That also disables the ruff and ruff-format pre-commit hooks. Filed separately rather than bundled here, because restoring it surfaces ~750 pre-existing violations that are yours to sequence.

[CLAassistant]

All committers have signed the CLA.

[mvkonchits-db]

Thanks for a genuinely thorough writeup — I verified every diagnostic claim against development source rather than taking the description at face value, and it all checks out. Approving.

Diagnosis confirmed:

• Duplicate class defs are real (GetDataProductTool @17 & @520, DeleteDataProductTool @101 & @669; contracts @16/@479, @139/@564, @209/@689). Python keeps the last, so maintenance edits landed on the dead top copies.
• The running (bottom) copies carry no required_scope, so they inherit BaseTool's "*" default. _has_scope only passes the wildcard when the token literally carries "*", so those five tools were invisible in tools/list and rejected by tools/call for every least-privilege token. Confirmed.
• Contract tools called the legacy in-memory _contracts store (list_contracts()/get_contract()/delete_contract()), which has no runtime populator — so they returned zero against real DB rows. Confirmed.
• DataContractDb has no description attribute (only description_purpose/usage/limitations), and DataProductDb.description/output_ports are relationships, not JSON — so the old .get('purpose') / isinstance(port, dict) paths never matched. Confirmed.

Fix verified:

• No duplicate classes remain; the AST guard would catch any regression — a good structural guard, not busywork.
• All eight tools now declare their own required_scope; the newly-registered list_* tools carry the read scope.
• The rewritten list_* tools query the DB exactly like the working search_* siblings; get_domains_for_entities(db, *, entity_type, entity_ids) matches the repo signature and the entity_type values (data_contract/data_product) match the existing tools.
• delete_data_contract correctly routes through delete_contract_from_db with ValueError -> not-found handling.
• The DB-backed tests insert real rows with matching column names, so they genuinely exercise the fix.

Two things worth a conscious maintainer sign-off (non-blocking):

1. Access-surface widening, by design. Today these five tools are effectively admin-only (inherited wildcard). After this merge a contracts:read / data-products:read token can call them. That is the intended fix, but it is a real broadening of the MCP surface — flagging so it's a deliberate decision, not a side effect nobody noticed.
2. .limit(500).all() then Python-side domain filter then break at limit (default 50): matches beyond the first 500 rows would be missed. Same pattern as the existing search_* tools, so it's consistent — minor scalability caveat only.

One note for whoever merges: CI hasn't run here (fork PR from a first-time contributor needs workflow approval). My review is a static verification against source plus a read of the test suite; worth letting the 35 tests actually execute before merge.