Skip to content

Follow-ups from PR #2392: ControlTools refactor, integration tests, input validation, UI colour consistency #2420

@rocketstack-matt

Description

@rocketstack-matt

Feature Proposal

Follow-up items raised during review of #2392 (Add MCP Controls and AI Governance Seeded Data). Captured here so the PR can land and these can be addressed as separate, smaller changes.

Target Project:

calm-hub (MCP server + Java unit/integration tests) and calm-hub-ui.

Description of Feature:

Four discrete cleanup / hardening items, all surfaced while reviewing #2392:

  1. Unify error-handling style in ControlTools.java. The read methods (listControls, getControl, listControlVersions) use the modern McpValidationHelper.firstError(...) pattern, but createControlRequirement (ControlTools.java:130-153) and createControlConfiguration (ControlTools.java:171-190) use the older sequential String error = …; if (error != null) return ToolResponse.error(error); chain. Refactor the create methods onto firstError for consistency with the rest of the file and with NamespaceTools / DomainTools.

  2. Add integration test coverage for the new create paths. MongoMcpIntegration.java and NitriteMcpIntegration.java inject controlTools but only exercise the read paths and only error/empty branches. Add end-to-end tests for createControlRequirement and createControlConfiguration against the real Mongo and Nitrite stores, mirroring what mcp_create_decorator / mcp_get_decorator do for decorators (e.g. create → list → get → assert content).

  3. Add max-length validation on MCP create inputs. McpValidationHelper.validateMaxLength and validateDescriptionLength already exist but aren't applied to name / description in createControlRequirement, and there is no size cap on requirementJson / configurationJson. Combined with the README note that the secure profile doesn't yet enforce per-tool scope checks on @Tool invocations, an authenticated client could store unbounded strings or unbounded JSON. Apply the existing helpers and pick a sensible JSON-size cap.

  4. UI: hardcoded Tailwind palette colours mixed with DaisyUI semantic classes. calm-hub-ui/src/hub/components/control-detail-section/ControlDetailSection.tsx:118,125,141 uses !bg-blue-700 !text-white for the requirement section while the configuration section uses !bg-accent !text-white. Mixing the raw Tailwind palette with DaisyUI's themed accent / primary means a theme change won't recolour the requirement section uniformly. Pick one (likely bg-primary) for consistency.

Current Limitations:

None of these block functionality — they are consistency, coverage, and hardening improvements identified during code review.

Proposed Implementation:

  • Items 1, 3 — small, local edits in calm-hub/src/main/java/org/finos/calm/mcp/tools/ControlTools.java. Item 3 also touches the existing unit tests to cover the new error paths.
  • Item 2 — extend MongoMcpIntegration.java and NitriteMcpIntegration.java with create-path test cases.
  • Item 4 — UI-only change in ControlDetailSection.tsx. Verify visually under the default DaisyUI theme.

Alternatives Considered:

Splitting these into four separate issues — opted for a single tracking issue since they all originate from the same PR follow-up.

Testing Strategy:

  • Unit tests stay green for items 1, 3, 4.
  • Item 2 is itself a test addition; verify with mvn -P integration verify from calm-hub.
  • Item 4 is a visual change; eyeball under the default theme and any non-default theme to confirm consistency.

Documentation Requirements:

None expected unless item 3 introduces user-visible limits worth calling out in the README MCP section.

Implementation Checklist:

  • (1) ControlTools create methods refactored onto firstError
  • (2) Integration tests added for createControlRequirement / createControlConfiguration (Mongo + Nitrite)
  • (3) Max-length validation applied to MCP create inputs (name, description, JSON payloads)
  • (4) ControlDetailSection.tsx colour classes unified onto DaisyUI semantic tokens
  • Tests passing
  • Documentation updated (if needed)

Additional Context:

See review thread on #2392 for the original context.

Metadata

Metadata

Assignees

Labels

calm-hubAffects `calm-hub`

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions