Fix mcp validation

[sarah-inkeep]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9b25513

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
agents-api	Ready	Preview, Comment	Jan 24, 2026 1:51am
agents-manage-ui	Ready	Preview, Comment	Jan 24, 2026 1:51am

1 Skipped Deployment

Project	Deployment	Review	Updated (UTC)
agents-docs	Skipped		Jan 24, 2026 1:51am

[claude]

Claude finished @sarah-inkeep's task —— View job

---

PR Review: Fix MCP Validation

Todo List:

• Read repository setup and guidelines (AGENTS.md, CLAUDE.md)
• Analyze the changed validation file and understand the modification
• Research existing MCP server patterns and validation approaches
• Examine broader system context and related validation logic
• Provide architect-level feedback on the changes
• Check for consistency with existing patterns and potential gotchas
• Assess customer impact and future extensibility considerations

---

Review Summary

This PR fixes a simple error message typo by removing the "000" suffix from 'Name is required 000.' → 'Name is required.'. While the change itself is trivial, the commit history reveals some concerning patterns that warrant deeper architectural discussion.

Technical Analysis

The Change

File: agents-manage-ui/src/components/mcp-servers/form/validation.ts:53

- name: z.string().min(1, 'Name is required 000.'),
+ name: z.string().min(1, 'Name is required.'),

Historical Context

The git history shows this is part of a validation relaxation pattern:

1. Previous commit (33b53af0): "Revert strict regex on mcp names" - removed comprehensive name validation including length limits and character restrictions
2. This commit (9b25513c): "Fix error message" - cleaning up a debug artifact

Critical Architectural Concerns

🚨 High Priority: Validation Consistency Gap

Issue: The codebase shows inconsistent validation patterns for names across different entity types:

1. MCP Tools (this file): Minimal validation - only requires non-empty string
2. Other entities consistently use comprehensive validation:
   • Tool overrides (same file, line 84-87): Strict regex /^[a-zA-Z0-9_-]+$/ + length limits
   • Project names: Length + character validation
   • Branch names: Complex regex validation
   • Resource IDs: URL-safe pattern validation

Risk: This creates an inconsistent user experience and potential security/data integrity issues.

🔍 Architecture Pattern Violation

Problem: The validation inconsistency violates the "principle of least surprise" - users will expect similar validation behavior across similar inputs.

Evidence from codebase:

// Tool overrides (same file) - strict validation
displayName: z.string()
  .max(100, 'Display name must be less than 100 characters.')
  .regex(/^[a-zA-Z0-9_-]+$/, 'Display name can only contain letters, numbers, hyphens, and underscores.')

// MCP tool name (after this PR) - minimal validation  
name: z.string().min(1, 'Name is required.')

🛡️ Security & Data Quality Considerations

Concern: Without input sanitization, MCP tool names could contain:

• Special characters that break UI rendering
• Characters that cause issues in logging/debugging
• Non-URL-safe characters that complicate API operations
• Extremely long strings that impact database performance

Current risk level: Medium - Names are likely escaped in UI contexts, but could cause issues in logging, debugging, or API operations.

Recommendations

Immediate Actions

1. Standardize validation approach - Add back reasonable validation for MCP tool names:

name: z.string()
  .min(1, 'Name is required.')
  .max(100, 'Name must be less than 100 characters.')
  .regex(/^[a-zA-Z0-9\s\-_.]+$/, 'Name can only contain letters, numbers, spaces, hyphens, underscores, and dots.')

2. Document validation rationale - If lenient validation is intentional, document why MCP tools have different requirements than other entities.

System-Level Improvements

1. Create validation pattern library - Extract common validation patterns (like name validation) into reusable schemas to ensure consistency
2. Audit validation patterns - Review all form validations for consistency across entity types
3. Testing coverage - Add tests for edge cases in name validation

Future Extensibility

Consider creating a validation hierarchy:

const baseNameSchema = z.string().min(1).max(100);
const strictNameSchema = baseNameSchema.regex(/^[a-zA-Z0-9_-]+$/);
const relaxedNameSchema = baseNameSchema.regex(/^[a-zA-Z0-9\s\-_.]+$/);

Customer Impact Assessment

Current impact: Low - The typo fix improves UX without breaking functionality.

Potential impact of validation inconsistency: Medium - Users may be confused by different validation rules for similar inputs across the platform.

Conclusion

While this PR successfully fixes a typo, it highlights a broader architectural concern about validation consistency. The immediate change is safe to merge, but I recommend addressing the validation standardization as a follow-up task.

Approval: ✅ Safe to merge with recommendation for follow-up work.