Skip to content

Commit 5fe00a8

Browse files
authored
Support client configs with "." in json keys (#1258)
Signed-off-by: Chris Gernon <[email protected]>
1 parent f03143b commit 5fe00a8

File tree

2 files changed

+75
-2
lines changed

2 files changed

+75
-2
lines changed

pkg/client/config_editor.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,13 @@ func (jcu *JSONConfigUpdater) Remove(serverName string) error {
156156
// This is necessary because the MCP client config file is a JSON object,
157157
// and we need to ensure that the path exists before we can add a new key to it.
158158
func ensurePathExists(content []byte, path string) []byte {
159+
// Special case: if path is root ("/"), just return everything (formatted)
160+
if path == "/" {
161+
v, _ := hujson.Parse(content)
162+
formatted, _ := hujson.Format(v.Pack())
163+
return formatted
164+
}
165+
159166
segments := strings.Split(path, "/")
160167

161168
// Navigate through the JSON structure
@@ -172,12 +179,18 @@ func ensurePathExists(content []byte, path string) []byte {
172179
// The "/" is added to the path for the patch operation because the path
173180
// is a JSON pointer, and JSON pointers are prefixed with "/".
174181
// The "." is added to the path for the retrieval operation.
182+
// - gjson (used for retrieval) treats `.` as a special (traversal) character,
183+
// so any json keys which contain `.` must have the `.` "escaped" with a single
184+
// '\'. In it, key `a.b` would be matched by `a\.b` but not `a.b`.
185+
// - hujson (used for the patch) treats "." and "\" as ordinary characters in a
186+
// json key. In it, key `a.b` would be matched by `a.b` but not `a\.b`.
187+
// So we need to "escape" json keys this way for retrieval, but not for patch.
175188
if len(pathSoFarForPatch) == 0 {
176189
pathSoFarForPatch = "/" + segment
177-
pathSoFarForRetrieval = segment
190+
pathSoFarForRetrieval = strings.ReplaceAll(segment, ".", `\.`)
178191
} else {
179192
pathSoFarForPatch = pathSoFarForPatch + "/" + segment
180-
pathSoFarForRetrieval = pathSoFarForRetrieval + "." + segment
193+
pathSoFarForRetrieval = pathSoFarForRetrieval + "." + strings.ReplaceAll(segment, ".", `\.`)
181194
}
182195

183196
// We retrieve the segment from the content so that we can check if it exists

pkg/client/config_editor_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,3 +224,63 @@ func getMCPServerFromFile(t *testing.T, configPath string, key string) MCPServer
224224

225225
return testMcpServer
226226
}
227+
228+
func TestEnsurePathExists(t *testing.T) {
229+
t.Parallel()
230+
231+
logger.Initialize()
232+
233+
tests := []struct {
234+
name string
235+
description string
236+
content []byte
237+
path string
238+
expectedResult []byte
239+
}{
240+
{
241+
name: "EmptyContent",
242+
description: "Should create path in empty JSON object",
243+
content: []byte("{}"),
244+
path: "/mcp/servers",
245+
expectedResult: []byte("{\"mcp\": {\"servers\": {}}}\n"),
246+
},
247+
{
248+
name: "ExistingPath",
249+
description: "Should return existing path",
250+
content: []byte(`{"mcp": {"servers": {"existing": "value"}}}`),
251+
path: "/mcp/servers",
252+
expectedResult: []byte("{\"mcp\": {\"servers\": {\"existing\": \"value\"}}}\n"),
253+
},
254+
{
255+
name: "PartialExistingPath",
256+
description: "Should create missing nested path when parent exists",
257+
content: []byte(`{"misc": {}}`),
258+
path: "/misc/mcp/servers",
259+
expectedResult: []byte("{\"misc\": {\"mcp\": {\"servers\": {}}}}\n"),
260+
},
261+
{
262+
name: "PathWithDots",
263+
description: "Should handle paths with dots correctly",
264+
content: []byte(`{"agent.support": {"mcp.servers": {"existing": "value"}}}`),
265+
path: "/agent.support/mcp.servers",
266+
expectedResult: []byte("{\"agent.support\": {\"mcp.servers\": {\"existing\": \"value\"}}}\n"),
267+
},
268+
{
269+
name: "RootPath",
270+
description: "Should handle root path",
271+
content: []byte(`{"server1": {"some": "config"}, "server2": {"some": "other_config"}}`),
272+
path: "/",
273+
expectedResult: []byte("{\"server1\": {\"some\": \"config\"}, \"server2\": {\"some\": \"other_config\"}}\n"),
274+
},
275+
}
276+
277+
for _, tt := range tests {
278+
t.Run(tt.name, func(t *testing.T) {
279+
t.Parallel()
280+
281+
result := ensurePathExists(tt.content, tt.path)
282+
283+
assert.DeepEqual(t, tt.expectedResult, result)
284+
})
285+
}
286+
}

0 commit comments

Comments
 (0)