[ghazy2001]

Summary of Changes

This PR hardens backend/utils/retrieval/tools/app_tools.py against edge-case crashes when handling dynamically configured app tools and manifest schemas:

1. Config & UID Extraction Resilience (_call_tool_endpoint):

   • Validates that config is a dictionary before accessing .get(), returning a structured error message (Error: Configuration not available for ...) instead of raising AttributeError.
   • Supports fallback extraction of user ID from top-level config['user_id'], alias configurable['uid'], or top-level config['uid'], ensuring seamless compatibility across different agent execution contexts.

2. Schema & Parameters Validation (_create_pydantic_model_from_schema & create_app_tool):

   • Ensures parameters is a dict before attempting .get('properties'), generating an empty input model cleanly if omitted or malformed.
   • Guards properties and required parsing against non-dict or non-iterable values (raw_req is None).
   • Handles malformed individual parameter schemas gracefully without breaking tool model registration.

3. HTTP Method & Request Payload Normalization (_call_tool_endpoint):

   • Normalizes app_tool.method with fallback to 'POST', stripping whitespace and handling lowercase method names safely.
   • For DELETE and non-POST/PUT/PATCH methods, passes request payload as query parameters (params=payload) rather than dropping them.

4. MCP Tool Argument & Transport Safety (create_app_tool):

   • Validates mcp_oauth_tokens as a dictionary before calling .get('access_token').
   • Normalizes app_tool.transport with a default of "sse".
   • Hardens _supplied_arguments(kwargs) to return an empty dict when kwargs is non-dict.

Tests Added

• Created backend/tests/unit/test_app_tools_resilience.py with 9 unit tests covering:
  • Non-dict / malformed parameters and properties in schema model creation.
  • Non-dict kwargs in _supplied_arguments.
  • Non-dict parameters and transport defaults in create_app_tool.
  • Non-dict config, top-level user ID fallback, and alias fallbacks in _call_tool_endpoint.
  • DELETE method payload routing and method normalization.
• Verified all 16 app_tools tests pass hermetically:
  • test_app_tools_resilience.py (9 passed)
  • test_app_tools_omitted_arguments.py (5 passed)
  • test_app_tools_status_cap.py (2 passed)

[Git-on-my-level]

Verdict: good hardening change, correctly scoped, with tests — leaning positive; not formally approving (first-time contributor; maintainer sign-off before merge).

Verified at your head (63d179d):

• backend/utils/retrieval/tools/app_tools.py — _create_pydantic_model_from_schema: the isinstance guards for parameters/properties/required and the skip of empty/non-string param names are correct, and typing non-dict param schemas as Optional[str] (rather than crashing registration) is a sensible degradation. _supplied_arguments' non-dict guard is harmless. _call_tool_endpoint's non-dict config rejection and (app_tool.method or 'POST').upper().strip() normalization fix real AttributeError paths (ChatTool.method defaults to 'POST' but tool manifests come from untrusted app definitions). The mcp_oauth_tokens isinstance guard and (app_tool.transport or 'sse') fallback are fine; note ChatTool.transport's model default is 'streamable_http', so the 'sse' fallback only fires for explicitly-empty values.
• backend/tests/unit/test_app_tools_resilience.py — all 9 tests pass at head; httpx/circuit-breaker/db paths are fully mocked so the file is hermetic, and it is picked up by the unit-test discovery manifest (0 orphans). Sibling test_app_tools_omitted_arguments / test_app_tool_method_null / test_app_tools_status_cap still pass — no regression.

Behavior notes (not blocking):

1. uid resolution now accepts configurable['user_id'/'uid'] and top-level config['user_id'/'uid']. That is a compatibility widening beyond crash hardening and is tested, but the accepted alias set is now an implicit contract — worth a one-line comment at the resolution site naming which contexts each alias serves.
2. Every non-POST/PUT/PATCH method now sends the payload (including uid/app_id/tool_name) as query params — pre-existing for GET, newly extended to DELETE et al. Query strings commonly land in access logs; acceptable, just be aware.

USER IMPACT: Apps with malformed tool definitions no longer crash tool loading or chat tool calls — those tools now register cleanly or return a readable error instead of failing the request.

What I'd still want: a CI run of the backend unit suite on this head (the PR checks only show the neutral AI-reviewer; I ran the new + sibling app_tools tests locally, all green), and ideally one test pinning the priority order of the uid alias chain so a future reorder can't silently change which key wins.

Leaving for human maintainer review before merge.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.