release: v2.5.0#208
Merged
Merged
Conversation
- Use host_name from site config if available - Force https:// when host_name is configured - Fixes http -> https mismatch for production deployments - Fixes #156
- Add _get_public_base_url() helper that respects host_name and force_https - Replace get_server_url() at all 5 call sites in oauth_discovery.py - Override Frappe-inherited URL keys in both openid_configuration() and authorization_server_metadata() to ensure issuer consistency - Drop trailing slash on issuer per RFC 8414 - Fixes #156
get_doctype_info previously returned only top-level fields, so callers hitting create_document for a parent doctype (Sales Order, Purchase Order, etc.) had no way to discover the child DocType's own fields and ended up creating the parent with empty child tables. create_document's own description already instructs the model to call get_doctype_info first, so the gap left the workflow broken. Add a child_tables list to the response — one entry per Table / Table MultiSelect field — including each child DocType's full field metadata (same shape as the parent fields array). Callers now have everything they need in a single tool call. Also fix is_single, which was reading meta.istable (child-table flag) instead of meta.issingle, and add a separate is_child_table key so the two concepts stop being conflated. Tests: regression guard asserting `User` exposes its `roles` child table with recursive field metadata, plus a single-vs-child-table distinguishing test against `System Settings` and `Has Role`.
The MCP endpoint used a single module-level MCPServer instance and rebuilt its shared _tool_registry on every request (clear() + re-register). With concurrent tools/call requests in the same worker, one request could clear or overwrite the registry while another was validating or executing a tool against it, causing intermittent "Tool 'X' not found. Available tools: []" errors and only one of several concurrent executions being audited. Build the tool registry per request on the call stack and thread it through handle() / _handle_tools_list() / _handle_tools_call() instead of mutating shared state: - Add build_tool_dict() in tool_adapter to produce a tool dict without touching server/global state; register_base_tool() now reuses it. - handle() and the tools/* handlers accept an optional tool_registry and route against it, falling back to self._tool_registry for direct add_tool usage. The register() wrapper passes through the dict returned by the endpoint. - fac_endpoint builds a fresh per-user OrderedDict via _build_tool_registry() and returns it; no per-request mutation of the shared singleton. Add test_mcp_concurrency regression tests: per-request registry isolation and N overlapping threaded tools/call requests against one shared MCPServer. Tool execution is kept in memory so the threaded test needs no DB connection and stays within the test transaction. The tests fail against the old shared- registry code with the exact reported "tool not found" symptom and pass with the fix. Fixes #197
The search tools query records with frappe.get_all, which bypasses DocType and user/row-level permissions. Because the registered BaseTool search tools (search_documents, search_doctype) delegate to SearchTools.global_search and SearchTools.search_doctype, a restricted user could get back records they cannot read — the same permission-leak class reported in #189 for list_documents, via a different tool. - global_search and search_doctype now use frappe.get_list(..., ignore_permissions=False) instead of frappe.get_all. The existing has_permission(doctype, "read") checks only gate DocType-level access and do not apply user permissions / row-level filtering; get_list does. - search_link already uses frappe.desk.search.search_link (permission-aware) and is unchanged. Also remove the dead legacy DocumentTools class (document_tools.py). It still used frappe.get_all / frappe.db.count, but is not registered by any plugin, referenced by hooks, or a BaseTool subclass, so it was unreachable — deleting it prevents the leaky implementation from being wired back up later. The live list_documents tool (DocumentList) was already fixed and is unaffected. Add regression guards in test_search_tools that patch frappe.get_all to raise and assert global_search / search_doctype query via frappe.get_list with ignore_permissions=False. The tests fail against the old get_all code and pass with the fix. Fixes #189
…#196) The MCP endpoint's 401 WWW-Authenticate header advertises the OAuth resource_metadata URL using frappe.oauth.get_server_url(). That helper reconstructs the host/port from the request URL or the Social Login Key base_url and does not honor a configured host_name's non-standard port, so the advertised URL dropped the port (e.g. :8000). Behind a port-restricted network where the public URL is only reachable on a non-standard port, the client then tries the implied :443 and the OAuth handshake fails with "couldn't reach the MCP server". The discovery endpoints already build URLs from _get_public_base_url(), which uses host_name verbatim (scheme + port) — only the WWW-Authenticate header was still on get_server_url(). - Expose get_public_base_url() in oauth_discovery as the shared accessor for the canonical public base URL. - fac_endpoint now builds all six resource_metadata URLs (bearer-not-found, token-validation-error, API-auth-error, API-other-error, no-auth, and the HEAD probe) from get_public_base_url() instead of get_server_url(), so the configured port is preserved. Add test_oauth_metadata_port: get_public_base_url returns a host_name with a non-standard port verbatim (and does not consult get_server_url), and the MCP 401 WWW-Authenticate resource_metadata URL keeps the port on both the unauthenticated and invalid-bearer paths. The header tests fail against the old get_server_url code and pass with the fix. Fixes #196
MCP clients (e.g. Claude Desktop) group tools and choose default approval
behavior from the annotation hints in the tools/list response. FAC emitted no
annotations, so every tool landed in a single "Other tools" bucket needing
approval, while servers that send hints (Google Drive, etc.) show proper
Read-only vs Write/delete groups.
Derive the hints from the existing FAC tool category — the same value shown
and overridable on the FAC admin page (FAC Tool Configuration.tool_category) —
so the client grouping stays in sync with the admin page (one source of truth):
read_only -> {readOnlyHint: true} (Read-only group)
write -> {readOnlyHint: false} (Write/delete group)
read_write -> {readOnlyHint: false} (Write/delete group)
privileged -> {readOnlyHint: false, destructiveHint: true}
- Add category_to_annotations() in tool_category_detector (co-located with the
category logic; accepts the legacy "dangerous" alias; unknown -> no hints).
- _build_tool_registry resolves each tool's category once via
_resolve_tool_categories (stored config/override -> auto-detect ->
read_write, batch-fetched) and merges the derived hints onto each tool dict.
The MCP server already forwards a tool dict's annotations in tools/list.
The transport already supported annotations; only the data source was empty.
Add test_tool_annotations: the category->hint mapping, category resolution
(stored override wins over auto-detect; fallbacks), tools/list emitting the
hints, and an end-to-end check that every built tool is classified.
Closes the "FAC tools show under Other tools" categorization gap.
FAC pinned beautifulsoup4~=4.12.2, which locks to the 4.12.* series. Frappe v15 requires ~=4.12.2 but Frappe v16 requires ~=4.13.5, so installing FAC on a v16 bench resolved bs4 down to 4.12.x — downgrading a dependency the rest of the bench expected at 4.13.x. Relax to beautifulsoup4>=4.12,<5 so the constraint spans both Frappe v15 (4.12.x) and v16 (4.13.x) without forcing a downgrade. FAC uses bs4 only as a transitive convenience and does not depend on any 4.12-vs-4.13 behavior. Fixes #198 (Observation 1). Observation 2 (optional analytics extras) is a larger packaging change tracked separately.
…#203) report_requirements returned empty filter definitions for custom Script Reports whose filters live in the .js file, and failed silently — agents and users had no way to see why. Reproduced two real triggers against a custom-app Script Report: - JSON-style quoted keys ("fieldname": "x"): the parser regex only matched bare keys (fieldname:), so every filter object was skipped -> 0 filters. - Filters built programmatically (filters: get_filters()): there is no literal array to anchor on, so parsing produced nothing with no explanation. Changes: - Add Report.filters child table as the first (structured, no-regex) discovery source, then JS. - Resolve the .js path via Frappe's own get_module_path + scrub (guarded by Module Def.custom) instead of reconstructing it by looping over installed apps, and fall back to the Report.javascript DB field for custom/DB-only modules with no on-disk file. - Make the filter-object regexes tolerate quoted keys and template-literal labels. - Surface discovery_diagnostics in the response: what each source attempted, the resolved path / file_exists / file_readable, filters_found, and a note when a source could not be parsed (e.g. programmatically-built filters), so an empty result is debuggable instead of silent. Native reports (e.g. Sales Analytics, 8 filters) and existing report tests are unaffected. Add test_report_requirements_filters covering quoted/bare/template-literal/ programmatic JS, child-table conversion, and discovery orchestration. The quoted-keys case fails against the old regex and passes with the fix. Fixes #203
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release v2.5.0 — merges the current
developcycle intomain. On push tomain, semantic-release auto-bumps the version, tags, and publishes the GitHub Release.Version bump: one
feat:commit since v2.4.3 (#202) → MINOR → 2.5.0.Change log:
frappe_assistant_core/change_log/v2/v2_5_0.md(added in this cycle).Included since v2.4.3
New Features
Security
list_documents/search_documentspermission leak fixed ([Bug]: list_documents leaks records by using frappe.get_all, which bypasses DocType permissions #189)Fixes
report_requirementsfilter discovery robust and diagnosable ([Bug] report_requirements fails to discover JS filters for custom Script Reports #203)get_doctype_infoincludes child table metadata ([Feature]: get doc info Child table support #192)Improvements
_get_public_base_url()Post-merge
mainback intodevelop