Skip to content

v0.2.0: users, realms, status, syslog tools#1

Open
Miggets7 wants to merge 14 commits into
mainfrom
release/v0.2.0
Open

v0.2.0: users, realms, status, syslog tools#1
Miggets7 wants to merge 14 commits into
mainfrom
release/v0.2.0

Conversation

@Miggets7
Copy link
Copy Markdown
Collaborator

@Miggets7 Miggets7 commented May 20, 2026

Summary

Extends the MCP server beyond the v0.1.0 asset/attribute surface:

  • Users (Keycloak-backed) — CRUD (query_users, get_user, create_user, update_user, delete_user), role management, self-service profile/locale, and the password-reset email flow.
  • Realms — full CRUD (list_realms, list_accessible_realms, get_realm, create_realm, update_realm, delete_realm).
  • System statusget_health_status, get_system_info + matching resources.
  • Syslog — event query/clear and config read/replace.

Tool surface: 13 → 43 tools, 4 → 6 resources. Additive only — no v0.1.x breaking changes.

fixes #2

Miggets7 added 13 commits May 15, 2026 20:31
…t_current_user

Remove tool families that are either security liabilities or low-value for an
LLM-driven workflow:

- users-sessions.ts (both tools): disconnect_user_session uses HTTP GET to
  mutate state, easy for an LLM to fire by accident and log real users out.
  get_user_sessions had marginal debug value on its own.
- update_password / update_current_password: plaintext password flows through
  the LLM context (transcripts, provider logs, cache). Email-only reset flow
  (request_password_reset, request_current_password_reset) stays.
- reset_user_secret: regenerating a service user's OAuth secret can lock out
  integrations and, worst case, the MCP server's own auth.
- get_current_user: returns the service-user identity that the server already
  runs as; marginal value to downstream prompts.

Net: 24 -> 17 user tools. PUT-replace semantics on update_user and the
update_*_roles tools kept as-is (description warnings already present).
Drop the users-sessions import and registerUserSessionTools() call, and
remove the trimmed tool names from the LLM-facing instructions block.
README capability table: tool count 49 -> 43; rows for get_current_user,
update_password, update_current_password, reset_user_secret,
get_user_sessions, disconnect_user_session removed.

CLAUDE.md: add project guidance file; scope-boundaries paragraph documents
that sessions and direct-credential mutation are deferred (footgun +
LLM-context credential leakage); conventions paragraph lists the active
users-* split sub-files.
@Miggets7 Miggets7 self-assigned this May 20, 2026
@Miggets7 Miggets7 added the Enhancement Improvement of an existing feature label May 20, 2026
@Miggets7 Miggets7 requested a review from a team May 20, 2026 10:05
@wborn wborn requested a review from Copilot May 21, 2026 07:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR bumps the MCP server to v0.2.0 and expands the tool/resource surface from asset/attribute management to include Keycloak-backed user management, realm CRUD, system status, and syslog operations—aligning the implementation with the scope described in issue #2.

Changes:

  • Added new tool modules for users (CRUD, roles, self-service, password reset), realms, status, and syslog, and registered them in src/server.ts.
  • Added status resources (openremote://status/health, openremote://status/info) plus Vitest coverage for the new tools/resources.
  • Updated docs and versioning (README.md, CLAUDE.md, package.json, server version string).

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/tools/users-self.test.ts Adds coverage for self-service user profile + locale update tools.
tests/tools/users-roles.test.ts Adds coverage for role catalog + user role assignment tools.
tests/tools/users-passwords.test.ts Adds coverage for password reset email flow tools, including error path.
tests/tools/users-crud.test.ts Adds coverage for user CRUD/query tools, including error path.
tests/tools/syslog.test.ts Adds coverage for syslog event query/clear + config read/replace tools.
tests/tools/status.test.ts Adds coverage for system health/info tools.
tests/tools/realms.test.ts Adds coverage for realm CRUD/list tools.
tests/resources/status.test.ts Adds coverage for new status MCP resources.
src/tools/users-self.ts Implements current-user profile/locale tools.
src/tools/users-roles.ts Implements role catalog and user role assignment tools.
src/tools/users-passwords.ts Implements password reset email flow tools.
src/tools/users-crud.ts Implements user query + CRUD tools.
src/tools/syslog.ts Implements syslog query/clear and config read/replace tools.
src/tools/status.ts Implements health and system info tools.
src/tools/realms.ts Implements realm CRUD and realm listing tools.
src/server.ts Registers the new tools/resources, updates version and instructions text.
src/resources/status.ts Adds openremote://status/health and openremote://status/info resources.
README.md Updates published tool/resource list and adds roadmap section.
package.json Bumps package version to 0.2.0.
CLAUDE.md Adds repo guidance, conventions, and scope boundaries for v0.2.0+.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/tools/syslog.ts
Comment on lines +39 to +46
inputSchema: {
level: syslogLevelSchema.optional().describe("Minimum level filter"),
per_page: z.number().int().min(1).max(1000).optional(),
page: z.number().int().min(0).optional(),
from: z.number().int().optional().describe("Lower bound timestamp (epoch ms)"),
to: z.number().int().optional().describe("Upper bound timestamp (epoch ms)"),
category: z.array(syslogCategorySchema).optional(),
subCategory: z.array(z.string()).optional(),
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction to my earlier reply: keeping per_page as-is. The backend SyslogResource.getEvents query param is literally per_page (snake_case), and we intentionally mirror the OpenRemote wire shape for this tool rather than introducing a camelCase rename plus an internal mapping layer. Not changing this.

Comment thread src/tools/status.ts
"get_health_status",
{
description:
"Get the OpenRemote system health status (database, gateway, etc.). Requires `read:admin`. Returns a map keyed by component.",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this one. getHealthStatus() is typed RestResponse<{ [index: string]: any }> (a flat indexed map), and OpenRemote's /health endpoint returns a map keyed by each HealthStatusProvider name. There is no top-level status field or nested components map — that is the Spring Boot Actuator shape, which OpenRemote does not use. The current wording "Returns a map keyed by component" matches the actual payload. Happy to revisit if you have a live-deployment response showing a different shape.

Comment thread src/server.ts Outdated
- get_client_roles / update_client_roles / update_realm_roles / get_user_realm_roles / update_user_realm_roles / get_user_client_roles / update_user_client_roles / get_current_user_realm_roles / get_current_user_client_roles
- update_current_user / update_current_user_locale

Tools — realms (admin):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed the heading to "Tools — realms". list_accessible_realms is the non-admin variant, so the (admin) qualifier was misleading (server.ts:45).

list_accessible_realms is the non-admin variant, so the "(admin)"
qualifier on the realms instructions heading was inaccurate.
@Ekhorn Ekhorn assigned Ekhorn and unassigned Ekhorn May 28, 2026
@Ekhorn Ekhorn requested review from Ekhorn and removed request for a team May 28, 2026 08:14
Copy link
Copy Markdown

@Ekhorn Ekhorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v0.2.0 Tool Review — Quirks & Issues

I connected Claude (with /effort medium) to a local openremote instance and found the following issues.

I asked it to make some changes and reviewed those. I made a branch (see release/v0.2.0...review/v0.2.0) so you can cherry-pick the commits if you want.

MCP server issues

Request shape issues

  • update_user_client_roles shape mismatchroles expects an array of role name strings (e.g. ["read:assets"]), but get_client_roles returns full role objects with id + name (confirmed by UserResource.java and ManagerKeycloakIdentityProvider.java). The natural next step after listing the role catalog is to pick from it and pass the names to the assignment tool, so the inconsistency causes a validation error. Either accept role objects and extract the name, or document the expected shape prominently. Note: get_user_client_roles already returns plain strings, so the mismatch only arises when a caller starts from get_client_roles.

  • update_realm missing id in schema and description — The backend (ManagerKeycloakIdentityProvider.java:825-827) throws an IllegalStateException if the Realm body has no id, which results in a 500. This applies to every update, not just partial ones. Two problems compound: (1) realmBodySchema does not list id as a field (.passthrough() forwards it if supplied, but an LLM has no signal it is required), and (2) the description says "read with get_realm first if doing a partial update" when a read-first flow is mandatory for all updates to obtain the id. Fix: add id: z.string() to realmBodySchema and state in the description that id is always required and must be obtained from get_realm first.

Self-service tools are useless for service accounts

These two tools remain registered in server.ts (line 74) despite prior trimming commits, and should be removed.

  • update_current_user 405 for service accountsManagerKeycloakIdentityProvider.createUpdateUser (line 334–338) throws NotAllowedException (HTTP 405) when the incoming User body has isServiceAccount() == false (the JSON default) but the stored record is a service account. Since the MCP server always authenticates as a service account, this tool fails for every caller.
  • update_current_user_locale — Succeeds for service accounts but has no observable effect (no UI session, no locale preference to apply). Should be removed alongside update_current_user.

OpenRemote issues

  • update_realm requires id in body — The backend PUT endpoint requires the realm id inside the request body even though name is the route key. Submitting an object without id returns a 500. No standard REST API should require the primary key to be duplicated in the body when it is already in the URL.

Password reset

  • request_password_reset / request_current_password_reset — Both return a 500 with an unhelpful IllegalStateException stack trace when SMTP is not configured, rather than a clear 4xx. No way to distinguish "wrong user ID" from "no mail server" from the error response. The MCP tool descriptions should at minimum warn that a configured mail server is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend MCP server: users, realms, status, syslog

3 participants