-
Notifications
You must be signed in to change notification settings - Fork 17
feat: allow customization of "View Docs" in the MCP server install page #1405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preview Environment (PR #1405)Preview environment scaled down after 12h of inactivity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devin Review found 3 new potential issues.
🔴 1 issue in files not directly in the diff
🔴 UpsertMetadata SQL query doesn't include external_documentation_text column, preventing persistence (server/internal/mcpmetadata/queries.sql:8-22)
The UpsertMetadata SQL query in queries.sql doesn't include the external_documentation_text column in either the INSERT or UPDATE clauses, so the custom button text can never be saved to the database.
Click to expand
Current query (queries.sql:8-22)
INSERT INTO mcp_metadata (
toolset_id,
project_id,
external_documentation_url,
logo_id,
instructions
-- external_documentation_text is MISSING
) VALUES (...)
ON CONFLICT (toolset_id)
DO UPDATE SET project_id = EXCLUDED.project_id,
external_documentation_url = EXCLUDED.external_documentation_url,
logo_id = EXCLUDED.logo_id,
instructions = EXCLUDED.instructions,
-- external_documentation_text is MISSING from UPDATE
updated_at = clock_timestamp()Impact
Even though the API design (design/mcpmetadata/design.go:79) accepts external_documentation_text as a payload attribute, and the database schema has the column, the value will never be persisted because the SQL query doesn't include it.
Recommendation: Update the UpsertMetadata query to include external_documentation_text in both INSERT and UPDATE clauses:
INSERT INTO mcp_metadata (
toolset_id,
project_id,
external_documentation_url,
external_documentation_text,
logo_id,
instructions
) VALUES (@toolset_id, @project_id, @external_documentation_url, @external_documentation_text, @logo_id, @instructions)
ON CONFLICT (toolset_id)
DO UPDATE SET ...
external_documentation_text = EXCLUDED.external_documentation_text,
...Then regenerate the sqlc code with mise gen:sqlc-server.
View issues and 6 additional flags in Devin Review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devin Review found 1 new potential issue.
🔴 1 issue in files not directly in the diff
🔴 UpsertMetadata SQL query doesn't include external_documentation_text column, preventing persistence (server/internal/mcpmetadata/queries.sql:8-22)
The UpsertMetadata SQL query in queries.sql doesn't include the external_documentation_text column in either the INSERT or UPDATE clauses, so the custom button text can never be saved to the database.
Click to expand
Current query (queries.sql:8-22)
INSERT INTO mcp_metadata (
toolset_id,
project_id,
external_documentation_url,
logo_id,
instructions
-- external_documentation_text is MISSING
) VALUES (...)
ON CONFLICT (toolset_id)
DO UPDATE SET project_id = EXCLUDED.project_id,
external_documentation_url = EXCLUDED.external_documentation_url,
logo_id = EXCLUDED.logo_id,
instructions = EXCLUDED.instructions,
-- external_documentation_text is MISSING from UPDATE
updated_at = clock_timestamp()Impact
Even though the API design (design/mcpmetadata/design.go:79) accepts external_documentation_text as a payload attribute, and the database schema has the column, the value will never be persisted because the SQL query doesn't include it.
Recommendation: Update the UpsertMetadata query to include external_documentation_text in both INSERT and UPDATE clauses:
INSERT INTO mcp_metadata (
toolset_id,
project_id,
external_documentation_url,
external_documentation_text,
logo_id,
instructions
) VALUES (@toolset_id, @project_id, @external_documentation_url, @external_documentation_text, @logo_id, @instructions)
ON CONFLICT (toolset_id)
DO UPDATE SET ...
external_documentation_text = EXCLUDED.external_documentation_text,
...Then regenerate the sqlc code with mise gen:sqlc-server.
View issue and 7 additional flags in Devin Review.
262de33 to
79c53cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devin Review found 1 new potential issue.
🔴 1 issue in files not directly in the diff
🔴 UpsertMetadata SQL query doesn't include external_documentation_text column, preventing persistence (server/internal/mcpmetadata/queries.sql:8-22)
The UpsertMetadata SQL query in queries.sql doesn't include the external_documentation_text column in either the INSERT or UPDATE clauses, so the custom button text can never be saved to the database.
Click to expand
Current query (queries.sql:8-22)
INSERT INTO mcp_metadata (
toolset_id,
project_id,
external_documentation_url,
logo_id,
instructions
-- external_documentation_text is MISSING
) VALUES (...)
ON CONFLICT (toolset_id)
DO UPDATE SET project_id = EXCLUDED.project_id,
external_documentation_url = EXCLUDED.external_documentation_url,
logo_id = EXCLUDED.logo_id,
instructions = EXCLUDED.instructions,
-- external_documentation_text is MISSING from UPDATE
updated_at = clock_timestamp()Impact
Even though the API design (design/mcpmetadata/design.go:79) accepts external_documentation_text as a payload attribute, and the database schema has the column, the value will never be persisted because the SQL query doesn't include it.
Recommendation: Update the UpsertMetadata query to include external_documentation_text in both INSERT and UPDATE clauses:
INSERT INTO mcp_metadata (
toolset_id,
project_id,
external_documentation_url,
external_documentation_text,
logo_id,
instructions
) VALUES (@toolset_id, @project_id, @external_documentation_url, @external_documentation_text, @logo_id, @instructions)
ON CONFLICT (toolset_id)
DO UPDATE SET ...
external_documentation_text = EXCLUDED.external_documentation_text,
...Then regenerate the sqlc code with mise gen:sqlc-server.
View issue and 6 additional flags in Devin Review.
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devin Review found 3 new potential issues.
🔴 2 issues in files not directly in the diff
🔴 SQL query missing external_documentation_text column in SELECT (server/internal/mcpmetadata/queries.sql:1-16)
The GetMetadataForToolset SQL query doesn't select the new external_documentation_text column, so the field will never be populated when reading from the database.
Click to expand
Impact
The code at server/internal/mcpmetadata/impl.go:669 tries to read metadataRecord.ExternalDocumentationText:
if docs := conv.FromPGText[string](metadataRecord.ExternalDocumentationText); docs != nil {
docsBtnText = strings.TrimSpace(*docs)
}But the SQL query in queries.sql only selects these columns:
SELECT id, toolset_id, project_id, external_documentation_url, logo_id,
instructions, header_display_names, default_environment_id,
installation_override_url, created_at, updated_at
FROM mcp_metadataThe external_documentation_text column is missing from the SELECT list. The generated GetMetadataForToolsetRow type in queries.sql.go:72-84 doesn't have this field.
Expected behavior
The query should include external_documentation_text in the SELECT list.
🔴 UpsertMetadata SQL query doesn't include external_documentation_text column, preventing persistence (server/internal/mcpmetadata/queries.sql:8-22)
The UpsertMetadata SQL query in queries.sql doesn't include the external_documentation_text column in either the INSERT or UPDATE clauses, so the custom button text can never be saved to the database.
Click to expand
Current query (queries.sql:8-22)
INSERT INTO mcp_metadata (
toolset_id,
project_id,
external_documentation_url,
logo_id,
instructions
-- external_documentation_text is MISSING
) VALUES (...)
ON CONFLICT (toolset_id)
DO UPDATE SET project_id = EXCLUDED.project_id,
external_documentation_url = EXCLUDED.external_documentation_url,
logo_id = EXCLUDED.logo_id,
instructions = EXCLUDED.instructions,
-- external_documentation_text is MISSING from UPDATE
updated_at = clock_timestamp()Impact
Even though the API design (design/mcpmetadata/design.go:79) accepts external_documentation_text as a payload attribute, and the database schema has the column, the value will never be persisted because the SQL query doesn't include it.
Recommendation: Update the UpsertMetadata query to include external_documentation_text in both INSERT and UPDATE clauses:
INSERT INTO mcp_metadata (
toolset_id,
project_id,
external_documentation_url,
external_documentation_text,
logo_id,
instructions
) VALUES (@toolset_id, @project_id, @external_documentation_url, @external_documentation_text, @logo_id, @instructions)
ON CONFLICT (toolset_id)
DO UPDATE SET ...
external_documentation_text = EXCLUDED.external_documentation_text,
...Then regenerate the sqlc code with mise gen:sqlc-server.
View issues and 7 additional flags in Devin Review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 SetMcpMetadata doesn't handle ExternalDocumentationText payload field
The SetMcpMetadata function doesn't process the payload.ExternalDocumentationText field and doesn't pass it to the UpsertMetadataParams.
Click to expand
Impact
The API design at server/design/mcpmetadata/design.go:178 defines the payload attribute:
Attribute("external_documentation_text", String, "Custom text for the external documentation link button")But SetMcpMetadata in impl.go only handles these fields from the payload:
ExternalDocumentationURL(line 211-214)Instructions(line 216-219)LogoAssetID,DefaultEnvironmentID,InstallationOverrideURL, etc.
The ExternalDocumentationText field is never read from the payload and never passed to UpsertMetadataParams at lines 235-243.
Expected behavior
The function should extract payload.ExternalDocumentationText and include it in the UpsertMetadataParams struct passed to s.repo.UpsertMetadata().
(Refers to lines 235-243)
Was this helpful? React with 👍 or 👎 to provide feedback.
…o ds/feature/customize-additional-header-button-text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 SQL query missing external_documentation_text column in SELECT
The GetMetadataForToolset SQL query doesn't select the new external_documentation_text column, so the field will never be populated when reading from the database.
Click to expand
Impact
The code at server/internal/mcpmetadata/impl.go:669 tries to read metadataRecord.ExternalDocumentationText:
if docs := conv.FromPGText[string](metadataRecord.ExternalDocumentationText); docs != nil {
docsBtnText = strings.TrimSpace(*docs)
}But the SQL query in queries.sql only selects these columns:
SELECT id, toolset_id, project_id, external_documentation_url, logo_id,
instructions, header_display_names, default_environment_id,
installation_override_url, created_at, updated_at
FROM mcp_metadataThe external_documentation_text column is missing from the SELECT list. The generated GetMetadataForToolsetRow type in queries.sql.go:72-84 doesn't have this field.
Expected behavior
The query should include external_documentation_text in the SELECT list.
(Refers to lines 1-16)
Was this helpful? React with 👍 or 👎 to provide feedback.
| -- Modify "mcp_metadata" table | ||
| ALTER TABLE "mcp_metadata" ADD COLUMN "external_documentation_text" text NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Duplicate migrations will fail: both add the same column
Two migration files attempt to add the same column external_documentation_text to the mcp_metadata table. When the second migration runs, it will fail with a "column already exists" error.
Click to expand
Duplicate migrations
20260129185604_add-mcp-metadata-title-slug.sqladdsexternal_documentation_text20260201200255_add-mcp-metadata-title-text.sqladds the same column again
Both contain:
ALTER TABLE "mcp_metadata" ADD COLUMN "external_documentation_text" text NULL;Impact: Database migrations will fail in any environment where the first migration has already run, blocking deployments.
Was this helpful? React with 👍 or 👎 to provide feedback.
The MCP tool install page has this button in it.
Users wish to customize the text slug away from "View Docs"
Here's a rough plan of implementation.