Skip to content

feat: adds a more robust and safe config editor - #184

Merged
ChrisJBurns merged 1 commit into
mainfrom
adds-better-config-layer
Apr 15, 2025
Merged

feat: adds a more robust and safe config editor#184
ChrisJBurns merged 1 commit into
mainfrom
adds-better-config-layer

Conversation

@ChrisJBurns

@ChrisJBurns ChrisJBurns commented Apr 14, 2025

Copy link
Copy Markdown
Collaborator

Currently, due to the way we perform read/write operations on the the client json config files, there are issues when reading visual studio code config files because of the JSON-CC format.
This PR not only resolves this but makes the updating of the client config files a bit better and less invasive, abstracting the complexities of these functions to the low levels.

It does this by:

  • Using hujson to perform patches on the config files, thus allowing use to specifically target certain sections of the files without making large scale updates to the file using json.Marshall
  • Using gjson to specifically retrieve JSON segments, rather than reading the entire file and suffering from the JSON-CC format issues when using json.Marshall.
  • Adding an Upsert and Remove function at the editor layer so we can idempotently add and remove MCP servers.
  • Holding specific information about each MCPClient that matters when updating the config file. This includes the MCPServersPathPrefix and the ClientType, so that we can support the addition of adding specific fields to the mcpServer objects when it concerns each Client. Example, adding type to VS Code types as it requires them.
  • Removed the SaveWithLock and DeleteWithLock functions and just hidden those in the Upsert and Remove functions.
  • Adding quite thorough tests to the editor layer so that we know that upserts and removes work as expected for the varying paths and Clients

Todo

This PR inserts the bulk of the logic that we want, it currently works as expected, however there are some additional TODOs' left:

  • We should clean up and remove the need for the the pathAndEditor
  • We should decouple the concept of the the config file contents and the config file metadata. What I mean by this is, each config file has metadata such as the ClientType, the JSONPath for the MCP Server config, the file path location and the type of Config Updater to use to perform the updates. It does not need to include the data of the config file. This will:
    • Allow the editor layer to make changes to the config files without leaking its operations upwards
    • Allow the calling layers to make decisions based on the metadata of the config files and when to make changes to the files.
  • Rename readConfigFile to retrieveConfigFileMetadata or something similar to express the intent.
  • Refactor getSupportedPaths, as there are probably better ways of achieving the same thing without having to bung the ClientType and MCPServersPathPrefix into the pathEditor object. I think we should be able to just return an array of ConfigFile and then we shouldn't need to loop through the configPaths FindClientConfigs function.
  • Currently there were no unit tests covering the config layer via FindClientConfigs, the next PR should add those tests, and along with the clean up mentioned above, we should have a more decoupled and covered client config code. The config_test.go mainly consisted of tests that were aimed at the editor functions itself.
  • the yaml variant should also be added in future. However it should just be a matter of adding something similar to:
    type YAMLConfigUpdater struct {
    	Path                 string
    	MCPServersPathPrefix string
    }
    
    func (ycu *YAMLConfigUpdater) Upsert(serverName string, data MCPServer) error { ... }
    
    func (ycu *YAMLConfigUpdater) Remove(serverName string) error { ... }

Resolves
#117

@ChrisJBurns
ChrisJBurns marked this pull request as draft April 14, 2025 21:18
@ChrisJBurns
ChrisJBurns marked this pull request as ready for review April 14, 2025 22:02
This commit adds a new editor layer that is more robust and less
invasive when editing client config files. We now use a combination of
json patches and targetting segment retrievals to avoid the failings of
the JSON-CC format causing errors with the standard json marhshalling
functions.

Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com>
@ChrisJBurns
ChrisJBurns force-pushed the adds-better-config-layer branch from 9b327a5 to 05e0f90 Compare April 14, 2025 22:04
@ChrisJBurns ChrisJBurns changed the title DRAFT: Adds better config layer feat: adds a more robust and safe config editor Apr 14, 2025
@ChrisJBurns
ChrisJBurns requested a review from JAORMX April 14, 2025 22:13
@ChrisJBurns
ChrisJBurns merged commit 49d4e89 into main Apr 15, 2025
@ChrisJBurns
ChrisJBurns deleted the adds-better-config-layer branch April 15, 2025 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants