Add per-user Chat Profiles (MCP tool allowlist + prompt override) for the browser AI Assistant - #94
Add per-user Chat Profiles (MCP tool allowlist + prompt override) for the browser AI Assistant#94IzBrain67 wants to merge 6 commits into
Conversation
Let each user choose which MCP tools the browser chat may use and optionally override its system prompt, saved as named profiles that can be switched from the chat header. Backend (app.chat): - ChatProfile model (user FK, name, allowed_tools, system_prompt) with migration 0002 and CRUD at /api/chat/profiles/ (owner-scoped). - /api/chat/stream/ accepts profile_id; the orchestrator filters the MCP tools by the allowlist, skips MCP discovery entirely when the allowlist is empty, enforces the allowlist at execution time, and appends a disabled/restricted note to the system prompt. Prompt precedence: profile > conversation > default. Frontend: - Profile selector in the chat header (selection remembered per user in localStorage and sent with every message). - Settings > Chat Profiles page with a category-grouped tool picker and a Back to Workflow button. - "Generate report" is disabled when the selected profile lacks the report tools. The per-user localStorage key falls back to preferred_username / email because the local Keycloak access tokens carry no `sub` claim. Tests: tests/test_chat_profiles.py (CRUD ownership, validation, tool filtering, MCP skipped when tools are off, prompt precedence). Docs: docs/CHAT_PROFILES.md, BRAIN_VIEWER_CHAT.md, CLAUDE.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UX7PJ4DQADdAPRE39jtBtS
There was a problem hiding this comment.
🟢 Approval recommended
The changes are cohesive end-to-end (model/API/orchestrator/UI), include targeted backend tests for key behaviors (ownership, allowlist enforcement, prompt precedence), and the reviewed logic appears consistent with the PR’s stated requirements.
Pull request overview
Adds per-user Chat Profiles to the browser AI Assistant, enabling users to (1) explicitly allowlist which MCP tools the assistant may use, and (2) optionally override the assistant system prompt—while preserving the existing “Default” behavior when no profile is selected.
Changes:
- Backend: introduce
ChatProfilemodel + CRUD API, and plumbprofile_idthrough/api/chat/stream/into the chat orchestrator (tool filtering + prompt precedence). - Frontend: add profile selection in the chat header and a Settings page to create/edit/delete profiles (with category-grouped tool picker).
- Docs/tests: document behavior and add Django tests covering ownership, validation, tool allowlisting/disable behavior, and prompt precedence.
File summaries
| File | Description |
|---|---|
| gui/workflow_frontend/src/views/home/components/chatToolCategories.ts | Defines category mapping and grouping logic for MCP tool picker UI. |
| gui/workflow_frontend/src/views/home/components/ChatProfileSelector.tsx | Adds header dropdown to switch between Default and saved chat profiles. |
| gui/workflow_frontend/src/views/home/components/ChatProfileModal.tsx | Implements create/edit modal with tool allowlist picker + prompt override. |
| gui/workflow_frontend/src/views/home/components/ChatProfileManager.tsx | Adds Settings page to list/create/edit/delete chat profiles. |
| gui/workflow_frontend/src/views/home/components/chatbotView.tsx | Initializes profiles per user, sends profile_id with messages, disables report when required tools aren’t allowed. |
| gui/workflow_frontend/src/stores/chatProfileStore.ts | Adds zustand store for loading profiles and persisting selected profile per user in localStorage. |
| gui/workflow_frontend/src/shared/header/header.tsx | Adds navigation entry to “Chat Profiles” under Settings menu. |
| gui/workflow_frontend/src/components/tabs/TabManager.tsx | Registers route for /settings/chat-profiles. |
| gui/workflow_frontend/src/api/chatProfileApi.ts | Adds frontend API client for profile CRUD + MCP tool catalog fetch. |
| gui/workflow_frontend/src/api/chatApi.ts | Extends chat stream payload to include optional profile_id. |
| gui/workflow_backend/django-project/tests/test_chat_profiles.py | Adds backend test coverage for profile CRUD scoping, validation, tool filtering/disable, and prompt precedence. |
| gui/workflow_backend/django-project/app/chat/views.py | Adds profile list/create + detail views; resolves profile_id before conversation creation; passes profile to orchestrator. |
| gui/workflow_backend/django-project/app/chat/urls.py | Wires new /api/chat/profiles/ and /api/chat/profiles/<uuid>/ routes. |
| gui/workflow_backend/django-project/app/chat/services/mcp_client.py | Adds allowlist filtering to mcp_tools_to_openai_functions(..., allowed=). |
| gui/workflow_backend/django-project/app/chat/services/chat_orchestrator.py | Adds profile-aware system prompt precedence and MCP tool allowlist/disable enforcement. |
| gui/workflow_backend/django-project/app/chat/serializers.py | Adds SendMessageSerializer.profile_id and ChatProfileSerializer validations. |
| gui/workflow_backend/django-project/app/chat/models.py | Introduces ChatProfile model with unique-per-user name constraint. |
| gui/workflow_backend/django-project/app/chat/migrations/0002_chatprofile.py | Adds DB migration for ChatProfile. |
| gui/workflow_backend/django-project/app/chat/admin.py | Registers ChatProfile in Django admin for visibility/support. |
| docs/CHAT_PROFILES.md | Documents concepts, UI usage, API, and code map for Chat Profiles. |
| docs/BRAIN_VIEWER_CHAT.md | Notes that viewer tool availability may be restricted by selected Chat Profile. |
| CLAUDE.md | Updates repo documentation to include the new chat profiles API endpoint and behavior. |
| .gitignore | Ignores .claude/settings.local.json (personal Claude Code overrides). |
Review details
- Files reviewed: 22/23 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Cap system_prompt, reject empty tool names, and return 400 on unique-name clashes so a race cannot 500. Prove off-allowlist tool calls never reach MCP, and start new profiles with all listed tools checked so Save does not silently disable them.
|
Audit on this branch (not a rewrite): the feature is complete and reasonable. Added prompt/tool validation, unique-name 400, an execution-deny test (off-allowlist |
deployment/pr94-chat-profiles-audit-progress.log was a personal working log (local worktree paths, throwaway container names, live-site checks) that does not belong next to the deployment assets. Its conclusions are already in the PR comment and the commit messages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZHJW8gnn4CfducyvfQuW5
Chat Profiles were per-user. Once the MCP server also advertises tools that manage user secrets (PR #92), "Default (all tools)" would hand them to every chat, and each user could only protect themselves. Profiles are now shared presets that only staff (Django is_staff, as for custom databases) create, edit or delete; every signed-in user can list and select them. - ChatProfile: drop the user FK (created_by for attribution), name unique globally, is_default with a partial unique constraint so at most one profile is the default. Migration 0002 is rewritten in place because the branch is unmerged; a dev DB that applied the old 0002 needs `migrate chat 0001` with the old file first. - Views: SAFE_METHODS -> IsAuthenticated, writes -> IsAdminUser; save() wrapped in transaction.atomic() so the IntegrityError -> 400 path cannot poison the connection. /api/chat/stream/ applies the default profile when a non-staff request carries no profile_id; staff keep all tools. - /api/profile/ returns is_staff so the frontend knows what to show. - Frontend: the store loads canManage; the header selector hides "Default (all tools)" from non-staff while a default profile exists and shows "Manage profiles" to staff only; the Settings page is read-only for non-staff and gains Set/Clear default; the modal no longer flashes the "tools disabled" warning while the catalog loads. - Docs: CHAT_PROFILES.md describes the admin model, the default profile, how to grant is_staff, and the API permissions; PR-process notes removed. Tests: 20 passed (shared read / staff-only write, global unique name, single default, default applied to non-staff, unknown profile 404). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZHJW8gnn4CfducyvfQuW5
|
@kirillmitrofanov thanks for the audit pass — the validation caps, the unique-name 400, the execution-deny test and the create-mode default are all kept. Two follow-ups pushed on top (
The Written with the help of Claude Code. |
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate frontend issues remain, along with documentation and API contract nits.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (5)
docs/CHAT_PROFILES.md:45
- The documented shell command instantiates the user model with
U()and then tries to access.objectson that instance, which raisesAttributeError. Useget_user_model().objects.get(...)(or bind the class without calling it) so the documented procedure can actually grant staff access.
"from django.contrib.auth import get_user_model as U; u = U().objects.get(email='alice@example.com'); u.is_staff = True; u.save()"
gui/workflow_frontend/src/api/chatApi.ts:64
- The payload comment says null/omitted always means all tools and the default prompt, but the backend applies the configured global default profile for non-staff users when no
profile_idis supplied. Correct this API contract comment so frontend callers do not assume unrestricted behavior.
// Selected chat profile (MCP tool allowlist + system prompt override).
// Null/omitted means the default: all tools, default prompt.
profile_id?: string | null;
gui/workflow_frontend/src/api/chatProfileApi.ts:10
- This API comment contradicts the behavior implemented by
ChatStreamView: it says no profile always means all tools, then notes that non-staff users receive the default profile instead. State the staff/non-staff resolution explicitly so callers do not rely on the wrong contract.
// Empty allowed_tools disables tools entirely; "no profile selected" means all
// tools + default prompt. At most one profile is the default, which non-staff
// users get instead of "no profile".
gui/workflow_frontend/src/views/home/components/chatbotView.tsx:93
selectedProfileis null while the profile store is loading (and also remains null if loading fails), so this enables Generate report even when the backend will apply a restricted non-staff default or the stored profile selection. A report request can therefore be sent without its required tools; track profile-resolution state and disable the action until the selected profile is known.
const reportToolsEnabled =
!selectedProfile ||
(selectedProfile.allowed_tools.includes('get_workflow_facts') &&
selectedProfile.allowed_tools.includes('save_report'));
gui/workflow_frontend/src/views/home/components/chatbotView.tsx:198
- The selected profile ID is persisted and sent on every message, but it is only validated during the initial profile load. If an administrator deletes the selected profile in another session, the next request gets the backend's 404 before streaming and every subsequent message keeps retrying the stale ID until a reload or profile-manager refresh. Handle a profile-not-found response by reloading profiles and clearing or replacing the selection.
profile_id: selectedProfileId,
- Files reviewed: 24/25 changed files
- Comments generated: 2
- Review effort level: Lite
- Disable Create in the profile modal until the tool catalog has loaded (and drop a stale catalog on open), so a fast click or a catalog failure can no longer save an empty allowlist by accident. - Gate the Generate report button on the profiles having loaded, so it is not enabled while the effective profile is still unknown. - Recover from a deleted selected profile: a 404 "Chat profile not found" from /api/chat/stream/ now reloads the profiles and drops the stale selection instead of failing on every following message. - Refresh the profile_id contract comments (serializer, chatApi, chatProfileApi): an explicit id wins; omitted means all tools for staff and the admin default profile for non-staff. - Clarify the staff-granting shell command in docs/CHAT_PROFILES.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UX7PJ4DQADdAPRE39jtBtS
|
Copilot review (1f0e319) addressed in ae30153 — the two inline threads are answered in place; the five suppressed comments:
Checks: 20 backend tests pass, Written with the help of Claude Code. |
There was a problem hiding this comment.
🔵 Needs a closer look
Moderate issues remain in admin validation/default handling, cross-tab navigation, and persisted default selection.
Review details
Suppressed comments (7)
Previously missed (2) — in code that hasn't changed since the last review.
gui/workflow_backend/django-project/app/chat/models.py:84
- The 16,000-character limit is enforced only by ChatProfileSerializer. Because this new model is also registered in Django admin, an administrator can save an overlong system prompt through /admin despite the documented limit; put max_length=16000 on the model field (and generate the corresponding migration) or add equivalent admin-form validation.
gui/workflow_backend/django-project/app/chat/views.py:172 - This catches every
IntegrityErroras a duplicate-name conflict, but thechat_profile_single_defaultconstraint can also fail when two admins set defaults concurrently. That turns a default-setting race into a misleading name error; distinguish the violated constraint or retry/report the default conflict separately.
gui/workflow_backend/django-project/app/chat/admin.py:32
- The web API clears other defaults in ChatProfileSerializer, but this ModelAdmin saves ChatProfile through Django's model form directly. Once one profile is default, checking is_default on a different profile fails the single-default validation/constraint instead of switching the default, so the registered admin path cannot manage defaults reliably. Reuse the clear-other-defaults logic in the model/admin save path.
@admin.register(ChatProfile)
class ChatProfileAdmin(admin.ModelAdmin):
list_display = ["name", "is_default", "created_by", "updated_at"]
search_fields = ["name"]
gui/workflow_backend/django-project/app/chat/views.py:132
- This catches every
IntegrityErroras a duplicate-name conflict, but thechat_profile_single_defaultconstraint can also fail when two admins set defaults concurrently. That turns a default-setting race into a misleading name error; distinguish the violated constraint or retry/report the default conflict separately.
try:
with transaction.atomic():
profile = serializer.save(created_by=request.user)
except IntegrityError:
return Response(
DUPLICATE_PROFILE_NAME, status=status.HTTP_400_BAD_REQUEST
gui/workflow_frontend/src/shared/header/header.tsx:197
Headeris rendered outsideTabManager, while this route is mounted only inside the workflow tab (TabManager.tsx:228-249). When a Jupyter or viewer tab is active, clicking this new Settings item changes the URL but leaves the visible iframe active, so Chat Profiles never becomes visible. Activate the workflow tab before navigating or expose this route outside the tab-specific routes.
to="/settings/chat-profiles"
gui/workflow_frontend/src/stores/chatProfileStore.ts:66
- This automatic default is written through
selectProfile, which persists the id in localStorage and makes it indistinguishable from an explicit user choice. If an administrator later changes or clears the default, this browser reloads the old id and sends it explicitly, so the new default is never applied. Keep automatic-default state separate from the persisted explicit selection.
if (selectedProfileId === null && !canManage && defaultProfile) {
get().selectProfile(defaultProfile.id);
gui/workflow_frontend/src/views/home/components/ChatProfileSelector.tsx:98
- The chat overlay is rendered at the TabManager level for every tab, but this action only calls
navigate. From a Jupyter or viewer tab,Manage profiles…therefore navigates to a route rendered in the hidden workflow tab and the user still sees the iframe. Switch to the workflow tab before navigating, or move the manager route to a global route.
onClick={() => navigate("/settings/chat-profiles")}
- Files reviewed: 24/25 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Adds Chat Profiles to the browser AI Assistant: named presets that decide which MCP tools the assistant may use and optionally override its system prompt. Profiles are shared presets managed by administrators (Django
is_staff, the same flag that already gates custom-database management); every user picks one from the chat header.profile_id). This is what keeps sensitive MCP tools — e.g. the secret-store tools from Add owner-only encrypted user secret store #92 — away from ordinary chats once they exist.allowed_toolsis an explicit allowlist.[]disables tools entirely (MCP discovery is skipped, no MCP round-trips).Conversation.system_prompt>DEFAULT_SYSTEM_PROMPT, plus an automatic note when tools are disabled or restricted.Backend (
app.chat)ChatProfilemodel + migration0002_chatprofile(globally uniquename,allowed_tools,system_prompt≤ 16000 chars,is_defaultwith a partial unique constraint,created_by).GET /api/chat/profiles/,GET /api/chat/profiles/<uuid>/for any signed-in user;POST/PUT/DELETEfor staff only (403 otherwise). Duplicate names return 400, including the race that only the DB constraint catches.is_default: trueclears the flag on every other profile.POST /api/chat/stream/acceptsprofile_id(resolved before any conversation is created; unknown id → 404). Without it, non-staff users get the default profile if one is set.GET /api/profile/now returnsis_staff.mcp_tools_to_openai_functions(..., allowed=)filters the tools; the orchestrator also rejects tool calls outside the allowlist at execution time (covered by a test that provesmcp.call_toolis never reached).Frontend
ChatProfileSelectorin the chat header; selection is remembered per user inlocalStorageand sent with every message (switchable mid-conversation). Non-staff users start on the default profile and do not see "Default (all tools)" while one is set./settings/chat-profiles): staff get list, create/edit modal with a category-grouped tool picker (category bulk toggle + per-tool checkboxes, Select all / none), Set/Clear default, delete, and a Back to Workflow button. Non-staff see the same list read-only. New profiles start with every listed tool checked.get_workflow_facts/save_report.Docs
docs/CHAT_PROFILES.md(new; includes how to grantis_staff),docs/BRAIN_VIEWER_CHAT.md,CLAUDE.md.Notes for reviewers
subclaim, souseAuth().user.idis"". The per-user localStorage key therefore falls back topreferred_username/ email (same order the backend maps users by).homeView.tsxalso readsuser?.idand is likely affected — out of scope here.0002_chatprofilewas rewritten in place when the model changed from per-user to shared (the branch was unmerged). A dev DB that had applied the earlier 0002 must runmigrate chat 0001with the old file before pulling; fresh DBs are unaffected.makemigrationsonmainalso emits unrelatedworkflow/0005_alter_flowproject_workflow_context.pyandbox/0006_alter_pythonfile_category.py(pre-existing model/migration drift); they are intentionally not included.chatToolCategories.ts.header.tsxandTabManager.tsx(both add a Settings item + route); Add node catalog drawer and fix README install links #93 touchesheader.tsxin a different hunk..claude/settings.local.jsonadded to.gitignore.Test plan
pytest django-project/tests/test_chat_profiles.py django-project/tests/test_notebook_agent.py— 20 passed (shared read / staff-only write, global unique name, single default, default applied to non-staff, unknown profile 404, allowlist filtering, MCP skipped when tools are off, off-allowlist call never reaches MCP, prompt precedence).makemigrations --checkemits nothing forchat.tsc -b && vite buildpasses; ESLint clean on all changed files.🤖 Generated with Claude Code
https://claude.ai/code/session_01KZHJW8gnn4CfducyvfQuW5