Skip to content

feat: add list_clients MCP tool and client query surface (PR-LC3)#44

Merged
HumanBean17 merged 3 commits into
masterfrom
feat/list-clients-lc3-mcp-surface
May 6, 2026
Merged

feat: add list_clients MCP tool and client query surface (PR-LC3)#44
HumanBean17 merged 3 commits into
masterfrom
feat/list-clients-lc3-mcp-surface

Conversation

@HumanBean17
Copy link
Copy Markdown
Owner

Summary

  • Add first-class outbound client discovery with KuzuGraph.list_clients and deterministic filterable query semantics (microservice, client_kind, target_service, path_prefix, method, limit).
  • Register a new MCP tool list_clients in server.py with ClientRowDto / ClientsListOutput, method normalization, and bounded limit behavior (100 default, clamped to 1..500).
  • Document inbound vs outbound discovery split in README.md and add LC3 test coverage (tests/test_list_clients.py plus tool-registration smoke in tests/test_mcp_tools.py).

Test plan

  • ruff check .
  • pytest tests/test_list_clients.py -v
  • pytest tests/test_mcp_tools.py -v
  • pytest tests -v

Made with Cursor

Co-authored-by: Cursor <cursoragent@cursor.com>
@HumanBean17
Copy link
Copy Markdown
Owner Author

Review: PR-LC3 — list_clients MCP tool + query surface + docs

Verdict: Approved ✅

Final slice of the LC plan lands cleanly. Read-only query surface (no schema or matcher edits), DTO/tool registration symmetric with list_routes, all 8 plan-mandated filter/limit tests + tool-registration smoke present. Tests green at 311/4 locally (+9 over PR-LC2's 302). Manual end-to-end list_clients query reproduces against bank-chat-system and returns 2 rest_template Client rows with full payload fidelity. One pre-existing warning surfaces in the build output that's worth a follow-up PR but does not block this one.

Scope discipline (out-of-scope checks)

Sentinel Status
ONTOLOGY_VERSION bump ✅ 0 hits — correctly untouched
CREATE NODE TABLE / CREATE REL TABLE ✅ 0 hits — no schema delta
pass5 / pass6 modifications ✅ 2+2 hits, all in the new test fixture builder (calling existing pass functions, no behavioural edits)
ClientRow / DeclaresClientRow dataclass touches ✅ 0 hits
_client_id (LC1 deterministic-id contract) ✅ 0 hits — id contract preserved
find_route_callers ✅ 2 hits, both in README context lines describing existing tool table

Real-code touch is exactly the LC3 plan §165-189 set:

  • kuzu_queries.py:1377-1442list_clients helper + _CLIENT_RETURN projection + _row_to_client_dict
  • server.py:251-272ClientRowDto, ClientsListOutput Pydantic models
  • server.py:1057-1090@mcp.tool(name="list_clients") registration with method normalization and limit clamping
  • README.md — tool table row + directional callout + reindex requirement
  • tests/test_list_clients.py — 8 plan-mandated tests
  • tests/test_mcp_tools.py:53-58test_list_clients_tool_is_registered registration smoke

Plan compliance

# Plan item (PR-LC3 §) Verified
1 kuzu_queries.py query helper with optional filters: microservice, client_kind, target_service, path_prefix, method, limit ✅ all six on KuzuGraph.list_clients(); path_prefix correctly translates to c.path STARTS WITH $path_prefix Cypher predicate
2 Deterministic ordering and limit enforcement ORDER BY c.microservice, c.client_kind, c.path, c.method, c.id LIMIT $lim — five-key tuple; lim = max(1, min(int(limit), 500)) clamps below the query parameter bind
3 server.py adds ClientRowDto and ClientsListOutput ✅ both defined at server.py:252-272; field set matches Client schema from PR-LC1 minus source_layer (intentional — see observation 4 below)
4 Register @mcp.tool(name="list_clients", ...) ✅ at server.py:1057-1090; description names all five filter axes
5 Method normalization normalized_method = (method or "").strip().upper() at the tool boundary (server.py:1085) — test test_list_clients_filter_method proves "get" and "GET" produce identical results
6 Safe default limit=100, bounded 1..500 ✅ both in the tool signature (limit: int = Field(default=100, ...)) and re-clamped at the helper layer (max(1, min(int(limit), 500))) — defense in depth
7 Empty results return success with empty list (not an error) test_list_clients_empty_result_is_success_with_empty_clients asserts success=True, clients=[] for a triple-AND-impossible filter
8 README documents new tool + outbound/inbound split + reindex callout ✅ tool table row added at README.md:170; directional sentence at README.md:175 ("list_routes for inbound service exposures and list_clients for outbound HTTP declarations"); ontology v10 prerequisite called out inline
9 All 8 plan-mandated tests ✅ all present with exact names: test_list_clients_returns_rows, test_list_clients_filter_microservice, test_list_clients_filter_client_kind, test_list_clients_filter_target_service, test_list_clients_filter_path_prefix, test_list_clients_filter_method, test_list_clients_empty_result_is_success_with_empty_clients, test_list_clients_limit_bounds_and_clamping_behavior
10 Existing tool-suite smoke updated tests/test_mcp_tools.py::test_list_clients_tool_is_registered added

Tests

311 passed, 4 skipped in 79.87s

Exactly +9 over PR-LC2's 302 baseline (8 filter/limit tests + 1 registration smoke). All green locally.

Manual evidence reproduced

Built graph against tests/bank-chat-system and queried via list_clients:

total: 2
by_kind: {'rest_template': 2}
by_micro: {'chat-assign': 1, 'chat-core': 1}
sample row: {'id': 'c:0d35b35467cebb06', 'client_kind': 'rest_template', 'target_service': '',
             'method': 'POST', 'path': '', ...
             'microservice': 'chat-assign', 'module': 'chat-assign', ...
             'member_fqn': 'com.bank.chat.assign.integration.ChatCoreJoinClient#joinOperator(...)',
             'member_id': 'fd7df239339e3c95ee1e810e9fb596a189336308',
             'resolved': False}
filter client_kind=feign_method → 0   (correct, no Feign synthesized in this fixture)
filter path_prefix=/chat       → 0   (correct, paths empty for these imperative rows)
limit=0   → 1 row  (correctly clamped to 1)
limit=600 → 2 rows (correctly clamped to 500, but only 2 exist)

c: id prefix carries through from PR-LC1's deterministic-id contract ✅. member_id linkage round-trips through the persisted graph ✅. Limit clamping verified in both directions (lower and upper) ✅.

Notes that earned my trust

  • Deterministic ordering tuple (5 keys: microservice, client_kind, path, method, id) — explicitly tie-breaks on id, the SHA1-derived stable identifier from LC1. Two rows with identical (microservice, client_kind, path, method) would otherwise have non-deterministic order; the id tie-breaker prevents flakey golden-file tests downstream.
  • Limit clamping is defence-in-depth — clamped at the Pydantic boundary in the tool signature and at the helper's parameter bind (lim = max(1, min(int(limit), 500))). Caller bypassing the MCP shell still gets the same bound.
  • path_prefix uses STARTS WITH $param parameter binding, not string interpolation — Cypher injection-safe.
  • Method normalization at the tool boundary, not the helper — keeps the helper's contract honest (caller-side normalization in tests can bypass it; the test suite's _lower vs _upper comparison is what guarantees normalization is wired in the production path).
  • _row_to_client_dict is defensive on every field: str(row.get("id") or ""), int(row.get("start_line") or 0), bool(row.get("resolved", True)). Kuzu rarely returns None for non-nullable columns but the dict-coercion idiom matches _row_to_route_dict (consistency with the existing route-side code).
  • ClientsListOutput failure mode returns success=False, message=msg when _require_graph() fails — symmetric with RoutesListOutput. Empty-but-graph-present case correctly still returns success=True, clients=[].
  • Test fixture builder builds the graph from scratch (full pass1-6 + write_kuzu) rather than synthesising rows directly — true integration coverage including pass5/pass6 idempotency.
  • Test fixture creates both @CodebaseClient source-layer rows AND a @FeignClient interface so feign_method and rest_template / web_client filter paths are both exercisable. The bank-chat-system manual run only had rest_template; this fixture is broader.

Observations (non-blocking)

  1. Pre-existing warning surfaces during graph build: unknown client source strategy 'rest_template', falling back to builtin (printed twice during bank-chat-system rebuild). I confirmed this also reproduces on master (02a4c0c), so it's not introduced by this PR. It's the same _client_source_layer whitelist-vs-strategy issue from PR-LC1's observation Add per-PR Cursor task prompts for Tier 1 completion #4: when pass5_imperative_edges resolves a strategy of rest_template (a client_kind, not a source_layer), the LC1 whitelist {layer_a_meta, layer_b_ann, layer_b_fqn, layer_c_source, builtin} doesn't match and the row gets builtin. Worth a follow-up PR to either (a) extend the whitelist with the client_kind-as-strategy synonyms or (b) silence the warning when the strategy is a known client_kind. Out of scope for LC3.

  2. Client.source_layer is not exposed through ClientRowDto — present on the underlying schema (LC1) but not in _CLIENT_RETURN projection or the DTO. Possibly intentional to keep the public surface narrow, but agents using list_clients to debug brownfield-override behaviour will find it useful. Consider adding in a follow-up PR alongside confidence (currently on DECLARES_CLIENT edge but not surfaced anywhere). Note that the test test_client_source_layer_reflects_winning_override_layer (LC1) still locks the persisted value, so this is purely a public-API gap.

  3. Limit clamping returns min=1 for limit=0 instead of 0. The plan says "bounded 1..500" so this matches spec. But agents that pass limit=0 to mean "metadata-only / count" will get one row back unexpectedly. The semantics are documented in the field description ("normalized to 1..500") so caller can adapt; mentioning here only for visibility.

  4. description text on the tool says "Feign methods and annotated RestTemplate/WebClient call declarations" — this matches PR-LC1's CodebaseClientKind enum {feign_method, rest_template, web_client}. If a future kind lands, this string drifts. Trivially fixable; flag for whoever extends the enum.

  5. KuzuGraph._instance reset in fixture teardown is good (prevents singleton bleed) but doesn't wrap in try/finally — if a test panics mid-fixture-yield, env vars stay polluted. Same pattern as existing tests/test_* files in this repo, so consistent rather than regressive.

Plan deltas needed

None. PR-LC3 done-criteria all met:

  • list_clients tool registered and callable
  • ✅ Filter behaviour test-covered and stable
  • ✅ Docs updated for outbound discovery entry point and v10 reindex requirement
  • ✅ Full test suite green (311 passed, 4 skipped)

The whole-plan done definition (plans/PLAN-LIST-CLIENTS-MCP-TOOL.md §233) is also now satisfied:

  • ✅ PR-LC1/LC2/LC3 ready to merge in order
  • ✅ Graph contains Client nodes and DECLARES_CLIENT edges with deterministic ids
  • ✅ pass6 hint recovery reads Client declaration metadata (LC2)
  • list_clients tool available with full filter coverage
  • README.md and ontology/reindex callouts updated consistently
  • pytest tests -v passes

Ready to merge. The list_clients plan is done after this lands. After merge, mark plans/PLAN-LIST-CLIENTS-MCP-TOOL.md §242 "Tracking" block with the three PR-LCx merge commits and move the file to plans/completed/ per the plan's own §250 directive. Suggested follow-up PRs (not blockers):

  1. _client_source_layer whitelist extension to silence the unknown client source strategy 'rest_template' warning (or a separate strategy-synonym map).
  2. Surface Client.source_layer through ClientRowDto for brownfield-debug agents.
  3. The four companion tools and Producer projection sketched in propose/LIST-CLIENTS-MCP-TOOL-PROPOSE.md (still punted to follow-up proposals per plan §227-229).

HumanBean17 and others added 2 commits May 6, 2026 18:59
Co-authored-by: Cursor <cursoragent@cursor.com>
@HumanBean17 HumanBean17 merged commit 0d6c7e1 into master May 6, 2026
@HumanBean17 HumanBean17 deleted the feat/list-clients-lc3-mcp-surface branch May 10, 2026 21:17
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