Pass-3 review deferrals from PR #6049: ten small MCP gateway findings #6942

CodeRabbit's third full review of PR #6049 raised 16 actionable comments. Five were real defects on that branch's own surface and were fixed there. One was not a defect. These are the remaining ten: each is real, each is small or latent, and none blocks the release. They are collected here so the pull request gets one reply instead of eleven.

Each was graded against the code at 9202f628c7, not against the claim. Two of them arrived marked Major and verify as P3 for the reasons given below.

The ten

1. web/packages/agenta-chat/src/components/McpServerNoticeCard.tsx — canConnect checks the endpoint's id and slug but not its base URL, which is optional in the type. A missing value becomes an empty url that reconnect can carry and manual credential submission can persist. Guard on a non-blank base URL.

2. web/packages/agenta-entities/src/mcpEndpoint/core/agentReference.ts — toolPrefixFromName("!!!") returns "___" where the function's own docstring says a punctuation-only name has no valid prefix and should return null. Confirmed by running it. Require at least one letter or digit after normalization.

3. web/packages/agenta-entities/src/mcpEndpoint/core/connectJourney.ts — sameAddress lowercases the entire URL rather than only its case-insensitive parts, so /MCP and /mcp compare equal. Marked Major on review; graded P3 because adoption also requires the normalized names to match, so it needs two same-named endpoints whose URLs differ only in path case.

4. web/packages/agenta-entities/src/mcpEndpoint/core/refusal.ts — isOurUnauthorised identifies our middleware by the presence of string message and operation_id fields alone, so an upstream body carrying both would be misread as ours. Real as code and unlikely in practice: our own middleware answers 401 with a plain string detail and builds the object form at 500, so at 401 or 403 that branch matches our shape only when a caller passed an operation id through. Any fix should be based on what the middleware actually sends rather than on a guessed marker.

5. web/packages/agenta-entities/src/mcpEndpoint/core/toolPolicy.ts — readMcpPolicy validates that the policy is an object but casts its interior unchecked, so a non-array tools.names reaches .includes() and throws while the policy is evaluated. Use Array.isArray rather than ?? [], which only substitutes for null and undefined.

6. web/packages/agenta-entity-ui/src/mcpEndpoint/McpConnectJourney.tsx — isProbeableUrl accepts plain HTTP, but the gateway egress guard rejects it unless an operator has deliberately turned the insecure setting on, which is off by default. The attempt fails with a clear message, so this is a mismatch between what the field accepts and what the route allows rather than a silent failure. Either reject it in the field or say in the copy that it needs the operator's setting.

7. web/packages/agenta-entity-ui/src/mcpEndpoint/McpConnectionDetail.tsx and McpPermissionDrawer.tsx — both bind a tool-list response to the connection's slug, so two overlapping requests for the same slug can resolve out of order and the older one wins. Marked Major on review; graded P3 because the existing slug guard already prevents one connection's tools appearing under another's, and what is left is a stale list for the same server. A per-request generation token closes it.

8. web/packages/agenta-entity-ui/src/secret/SecretForm/SecretForm.tsx — the control and label ids are fixed module-level strings, so two instances in one document would associate a label with the wrong form. No path was found that mounts two at once, so this is hygiene rather than a live defect. useId() removes the question.

9. web/packages/agenta-settings-ui/src/mcp/McpServersSection.tsx — the Remove confirmation passes neither a named confirm label nor danger styling, while the less destructive Disconnect passes both. The body text does state the consequence, so the gap is in the button rather than in the warning.

10. web/packages/agenta-ui/src/components/ui/inline-confirm.tsx — the props spread is placed after the internal key handler, so a caller supplying onKeyDown would silently replace the documented Escape behaviour. No caller does today, so this is latent. Compose the handlers, or move the spread above.

Not included

Five findings were fixed on the branch: a retry stating the previous failure's reason, a server notice counted as a turn's answer by type while its reader rejects it by content, a refused-key screen calling an authentication scheme a header, two credential controls left enabled during submission, and a tool filter that survives a connection switch while its input hides.

One was graded as not a defect: the mobile blank-session hook catches a rejected navigation deliberately, so the guard clears and the person can act again, which a test pins by name.