feat: Implement startup requirement checks and dynamic tool registration #96
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… requiments while starting the server
MacondoExpress
requested review from
keremgocen and
mjfwebb
and removed request for
mjfwebb
keremgocen
reviewed
Nov 18, 2025
| analytics "github.com/neo4j/mcp/internal/analytics/mocks" | ||
| "github.com/neo4j/mcp/internal/config" | ||
| db_mock "github.com/neo4j/mcp/internal/database/mocks" | ||
| db "github.com/neo4j/mcp/internal/database/mocks" |
keremgocen
reviewed
Nov 18, 2025
|
|
||
| "github.com/neo4j/mcp/internal/analytics" | ||
| analytics_mock "github.com/neo4j/mcp/internal/analytics/mocks" | ||
| amock "github.com/neo4j/mcp/internal/analytics/mocks" |
Contributor
There was a problem hiding this comment.
I think we are not using the real analytics here, so there should not be a conflict if we used analytics instead of amock to be a bit more specific.
keremgocen
reviewed
Nov 18, 2025
| Service database.Service | ||
| Deps *tools.ToolDependencies | ||
| createdLabels map[string]bool | ||
| AnalyticsService *analytics_mocks.MockService |
Contributor
There was a problem hiding this comment.
Same here, I guess it was done in an earlier PR, but I think we should use analytics here as well. At least name it something other than snake case, please:)
keremgocen
reviewed
Nov 18, 2025
| } | ||
|
|
||
| // VerifyConnectivity checks the driver can establish a valid connection with a Neo4j instance; | ||
| func (s *Neo4jService) VerifyConnectivity(ctx context.Context) error { |
Contributor
There was a problem hiding this comment.
this looks neat as part of the service now
keremgocen
reviewed
Nov 18, 2025
| dbService: dbService, | ||
| version: version, | ||
| anService: anService, | ||
| gdsInstalled: false, |
Contributor
There was a problem hiding this comment.
nit: no need to provide it, as it defaults to false
Counterargument: it's more visible to be explicit
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This update introduces a comprehensive pre-flight checks during the startup sequence.
We've implemented a new
verifyRequirementsfunction to ensure that the server's environment meets all necessary prerequisites before it begins its main operations.The verification process now confirms the following:
To perform these, this PR includes also other improvements such as the ability to filter registered tools by their specific categories, for instance: "read-only", "cypher", "gds".
The
registerToolsmethod is no longer exposed but instead is been moved to be part of the.Startmethod and implicitly invoked.Integration test helpers are slightly changed to accomodates testing initial part of the Server initialisation.