feat: Configurable MCP Server URL in Admin Settings (#76) - #161
Conversation
vybe
left a comment
There was a problem hiding this comment.
This PR requires the following changes before merge:
- Add requirement entry to
docs/memory/requirements.md(e.g., under section 5 Platform Settings or section 7 MCP Integration) - Add
GET/PUT/DELETE /api/settings/mcp-urlendpoints todocs/memory/architecture.mdsettings API table
Code and tests look good — just need the documentation updates per Trinity methodology (new feature = changelog + requirements + architecture).
vybe
left a comment
There was a problem hiding this comment.
PR Validation: APPROVED (with rebase required)
Code review passed — the implementation is clean, well-tested, and fully documented. However, the branch has 101 merge conflicts with main due to fork divergence and can't be merged in its current state.
Required before merge
- Rebase onto current
mainto resolve conflicts
How to rebase
git remote add upstream https://github.com/abilityai/trinity.git
git fetch upstream
git rebase upstream/main
# Resolve any conflicts, then:
git push --force-with-leaseThe actual feature changes (settings.py, ApiKeys.vue, Settings.vue, test_settings.py) are small and focused — most conflicts will be in files your branch didn't touch, so accepting the upstream version (git checkout --theirs <file>) should work for those.
Once rebased, this is ready to merge. The code, tests, docs, and security all look good.
vybe
left a comment
There was a problem hiding this comment.
Please rebase this branch onto main to resolve any conflicts and incorporate recent changes.
Once rebased, request re-review.
vybe
left a comment
There was a problem hiding this comment.
Validated via /validate-pr. Complete documentation (requirements, architecture, feature flows, changelog), 9 comprehensive tests, clean implementation following existing patterns.
274d981 to
fb9a0ab
Compare
|
Rebased onto Settings.vue — 4 conflict regions, all "both sides added adjacent new content" patterns (User Management section from ROLE-001 + my MCP URL section; usersList state + mcpUrlConfig state; serial loads + my parallel load; role/user functions + MCP URL functions). Union-merged: kept both sides with correct div closure for the template conflict. The settings.py — single import-line conflict (upstream added docs/memory/feature-flows.md — single row conflict on the Recent Updates table. Inserted the #76 row at its chronological position. docs/archive/memory/changelog.md — resolved to upstream. The changelog was deprecated on main ( Post-rebase doc fix ( Final branch: 5 commits, no production behavior change vs the pre-rebase branch. Re-running |
The MCP server URL on the API Keys page was hardcoded to
http://{hostname}:8080/mcp, which is wrong for production deployments
proxied through nginx. Admins can now override it via Settings.
- Backend: GET/PUT/DELETE /api/settings/mcp-url (GET: any auth user,
PUT/DELETE: admin-only, URL validation for http(s):// + /mcp suffix)
- Settings.vue: MCP URL config section with custom/auto-detect badge
- ApiKeys.vue: Fetches configured URL on mount, falls back to auto-detect
- Tests: 9 new tests in TestMcpUrlSettings class
Closes Abilityai#76
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…axios - Move urlparse import to module level (PEP 8) - Extract _submitMcpUrl helper to eliminate save/reset duplication - Use dynamic success message (save vs reset) - Parallelize independent API calls in Settings.vue and ApiKeys.vue - Switch fetchMcpUrl to axios for consistency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- platform-settings.md: added MCP URL endpoints, Settings.vue section, user story - api-keys-page.md: updated onMounted flow with fetchMcpUrl + Promise.all - feature-flows.md: added recent update entry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add requirement entry (7.4 MCP-URL-001) and Platform Settings API table per PR review feedback — completing Trinity methodology docs (changelog + requirements + architecture). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Settings.vue line numbers shifted as new sections (User Management, etc.) landed upstream. Update the platform-settings.md table to point at the current location (1089-1149) for the MCP Server URL section added by Abilityai#76. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fb9a0ab to
eee8613
Compare
|
Rebased again onto 7 new upstream commits since the last rebase. One conflict on Five commits preserved; no code changes, docs-only tweaks. Ready for re-review. |
Summary
/mcpChanges
src/backend/routers/settings.py— GET/PUT/DELETE/api/settings/mcp-urlendpointssrc/frontend/src/views/Settings.vue— MCP URL config section with badge, save/resetsrc/frontend/src/views/ApiKeys.vue— Fetch configured URL on mounttests/test_settings.py— 9 new tests (TestMcpUrlSettings)docs/memory/changelog.md— Change entrydocs/memory/feature-flows/— Updated platform-settings.md and api-keys-page.mdTest Plan
pytest tests/test_settings.py::TestMcpUrlSettings -vpytest tests/test_settings.py -vCloses #76
Generated with Claude Code