Skip to content

feat(ui-rewrite): add useMCPServerForm hook for creating and editing MCP servers#4561

Open
marekdano wants to merge 1 commit intoepic/ui-rewritefrom
4175-create-edit-mcp-servers-hook
Open

feat(ui-rewrite): add useMCPServerForm hook for creating and editing MCP servers#4561
marekdano wants to merge 1 commit intoepic/ui-rewritefrom
4175-create-edit-mcp-servers-hook

Conversation

@marekdano
Copy link
Copy Markdown
Collaborator

🔗 Related Issue

Related #4175


📝 Summary

Add a hook for creating and editing MCP servers. The hook contains calls to API endpoints to create or edit MCP Servers.

create_mcp_server.mov

🏷️ Type of Change

  • Bug fix
  • Feature / Enhancement
  • Documentation
  • Refactor
  • Chore (deps, CI, tooling)
  • Other (describe below)

@marekdano marekdano requested review from a-effort, gcgoncalves and vishu-bh and removed request for brian-hussey and crivetimihai May 1, 2026 10:01
@marekdano marekdano added the ui-rewrite Tasks for the isolated ui rewrite feature branch label May 1, 2026
Signed-off-by: Marek Dano <Marek.Dano@ibm.com>
@marekdano marekdano force-pushed the 4175-create-edit-mcp-servers-hook branch from 6a9a479 to 93c249a Compare May 5, 2026 10:06
@a-effort
Copy link
Copy Markdown
Collaborator

a-effort commented May 5, 2026

🤖 Sonnet 4.6 review feedback:

Good structure here: extracting form logic into a dedicated hook with Zod validation and sanitization is the right call. A few things worth flagging...

  1. Zod v4 is a breaking change: package.json bumps to zod@^4.4.1, but shadcn has a nested zod@3.25.76 as a peer dependency (visible in package-lock.json). Zod v4 has breaking API changes. If shadcn uses zod internally for form validation (e.g., via react-hook-form + zod resolvers), this version split will cause runtime issues. Worth verifying compatibility with the full shadcn setup.

  2. sanitizeCertificate calls .trim() on the result (sanitize.ts:130)... PEM certs sometimes require a trailing \n after -----END CERTIFICATE----- and some TLS parsers reject trimmed certs. Easy fix now, easy to forget later.

  3. There's a stray // Made with Bob comment at the end of useMCPServerForm.test.ts that should be removed.

Worth noting before this goes to production:

  • handleSubmit is typed as returning void in the interface but the implementation is async. It should be Promise.
  • isValid only checks that name/URL are non-empty, not that the URL is valid (acceptable for a prototype but will need tightening for real validation UX).
  • passthroughHeaders is stored as a comma-separated string but the Zod schema expects z.array(z.string()), so
    validateField("passthroughHeaders", ...) won't behave as expected. Worth a // TODO comment so it's not a surprise when edit mode gets wired up. Same goes for the missing pre-population logic from existing gateway data.
  • The updateGateway fallback URL ("/gateways/" when gatewayId is undefined) won't be reached in current code, but it's a footgun if the logic shifts. A comment or assertion would help.

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

Labels

ui-rewrite Tasks for the isolated ui rewrite feature branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants