Skip to content

Fix broken ${PWD} volume mount, misleading ${VAR} note, and undocumented tools field behaviour in TOML config#2121

Merged
lpcox merged 3 commits intomainfrom
copilot/fix-config-example-toml-discrepancy
Mar 19, 2026
Merged

Fix broken ${PWD} volume mount, misleading ${VAR} note, and undocumented tools field behaviour in TOML config#2121
lpcox merged 3 commits intomainfrom
copilot/fix-config-example-toml-discrepancy

Conversation

Copy link
Contributor

Copilot AI commented Mar 18, 2026

Three discrepancies between docs/examples and actual runtime behaviour, found by the nightly reconciliation check.

config.example.toml Example 4: broken ${PWD} volume mount

TOML configs have no variable expansion — ${PWD} was passed verbatim to Docker, silently producing a broken volume mount. Replaced with an absolute path placeholder and added an explanatory NOTE comment.

Misleading ${VAR_NAME} note in Docker Best Practices

The notes section implied ${VAR} syntax works in TOML. Clarified it applies to JSON stdin format only; TOML configs require literal absolute paths.

tools field behaviour clarified in documentation

docs/CONFIGURATION.md previously stated the tools field controlled which tools were exposed, implying non-listed tools would be filtered out. In practice, registerToolsFromBackend does not read this field — all backend tools are always registered. The documentation now explicitly states the field is stored but not currently enforced at runtime.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…toml

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix discrepancy in config.example.toml example 4 Fix tools filter enforcement, broken ${PWD} volume mount, and misleading ${VAR} note in TOML config Mar 18, 2026
Copilot AI requested a review from lpcox March 18, 2026 23:54
@lpcox lpcox marked this pull request as ready for review March 18, 2026 23:56
Copilot AI review requested due to automatic review settings March 18, 2026 23:56
@lpcox
Copy link
Collaborator

lpcox commented Mar 18, 2026

@copilot for registerToolsFromBackend don't change the implementation. instead change the documentation

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns runtime behavior and documentation/examples around tool exposure and Docker/TOML configuration by enforcing the per-server tools allow-list at registration time and clarifying that TOML configs do not perform ${VAR} expansion.

Changes:

  • Enforce ServerConfig.Tools as a runtime allow-list when registering backend tools (empty/nil continues to mean “allow all”).
  • Add test coverage for the tools filter behavior (allow-listed, no filter, and no matches).
  • Fix/clarify example TOML configs to avoid broken ${PWD} mounts and misleading ${VAR_NAME} notes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
internal/server/unified.go Filters backend tool registrations using serverCfg.Tools allow-list.
internal/server/register_tools_from_backend_test.go Adds new tests + a mock backend for tools-filter enforcement.
config.example.toml Replaces ${PWD} mount with absolute-path placeholder; clarifies ${VAR} expansion limitations in TOML.
Comments suppressed due to low confidence (2)

internal/server/register_tools_from_backend_test.go:884

  • Same as above: without a dummy Headers entry, NewHTTPConnection will attempt streamable/SSE transports before plain JSON-RPC. Adding a header here keeps this test deterministic and avoids potential transport-handshake timeouts against the minimal mock server.
	cfg := &config.Config{
		Servers: map[string]*config.ServerConfig{
			"nofilter-backend": {
				Type:  "http",
				URL:   backend.URL,
				Tools: nil, // no filter — all tools should be exposed
			},
		},
	}

internal/server/register_tools_from_backend_test.go:918

  • Same issue here: consider adding a dummy Headers map entry to force the plain JSON-RPC transport path in tests (avoids streamable/SSE attempts against this httptest server).
	cfg := &config.Config{
		Servers: map[string]*config.ServerConfig{
			"nomatch-backend": {
				Type:  "http",
				URL:   backend.URL,
				Tools: []string{"nonexistent_tool"},
			},
		},
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +355 to +359
filteredTools := make([]struct {
Name string `json:"name"`
Description string `json:"description"`
InputSchema map[string]interface{} `json:"inputSchema"`
}, 0, len(listResult.Tools))
Comment on lines +842 to +850
cfg := &config.Config{
Servers: map[string]*config.ServerConfig{
"filter-backend": {
Type: "http",
URL: backend.URL,
Tools: []string{"read_file"}, // only allow read_file
},
},
}
Comment on lines +800 to +830
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req map[string]interface{}
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
w.WriteHeader(http.StatusBadRequest)
return
}
method, _ := req["method"].(string)
switch method {
case "initialize":
json.NewEncoder(w).Encode(map[string]interface{}{
"jsonrpc": "2.0",
"id": req["id"],
"result": map[string]interface{}{
"protocolVersion": "2024-11-05",
"capabilities": map[string]interface{}{},
"serverInfo": map[string]interface{}{"name": "test", "version": "1.0.0"},
},
})
case "tools/list":
json.NewEncoder(w).Encode(map[string]interface{}{
"jsonrpc": "2.0",
"id": req["id"],
"result": map[string]interface{}{
"tools": []map[string]interface{}{
{"name": "read_file", "description": "Read a file", "inputSchema": map[string]interface{}{"type": "object"}},
{"name": "write_file", "description": "Write a file", "inputSchema": map[string]interface{}{"type": "object"}},
},
},
})
}
}))
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 19, 2026

@copilot for registerToolsFromBackend don't change the implementation. instead change the documentation

Done. Reverted the implementation change and updated docs/CONFIGURATION.md to clarify that the tools field is stored but not currently enforced at runtime — all backend tools are always exposed. (eef008c)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build3631937006/b332/launcher.test /tmp/go-build3631937006/b332/launcher.test -test.testlogfile=/tmp/go-build3631937006/b332/testlog.txt -test.paniconexit0 -test.timeout=10m0s conf�� 64/src/runtime/cgo --local x_amd64/asm pull.rebase l/buffer (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build3631937006/b317/config.test /tmp/go-build3631937006/b317/config.test -test.testlogfile=/tmp/go-build3631937006/b317/testlog.txt -test.paniconexit0 -test.timeout=10m0s conf�� ternal/engine/interpreter/compiler.go ternal/engine/interpreter/format.go x_amd64/compile user.email 64/src/runtime/c-unsafeptr=false (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build3631937006/b332/launcher.test /tmp/go-build3631937006/b332/launcher.test -test.testlogfile=/tmp/go-build3631937006/b332/testlog.txt -test.paniconexit0 -test.timeout=10m0s conf�� 64/src/runtime/cgo --local x_amd64/asm pull.rebase l/buffer (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build3631937006/b332/launcher.test /tmp/go-build3631937006/b332/launcher.test -test.testlogfile=/tmp/go-build3631937006/b332/testlog.txt -test.paniconexit0 -test.timeout=10m0s conf�� 64/src/runtime/cgo --local x_amd64/asm pull.rebase l/buffer (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build3631937006/b341/mcp.test /tmp/go-build3631937006/b341/mcp.test -test.testlogfile=/tmp/go-build3631937006/b341/testlog.txt -test.paniconexit0 -test.timeout=10m0s go1.25.8 -c=4 -nolocalimports -importcfg /tmp/go-build3631937006/b343/importcfg -pack /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/middleware/jqschema.go /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/middleware/jqschema_bench_test.go swit�� 64/src/net ternal/hpke/hpke-ifaceassert x_amd64/vet -p internal/goexper--version -lang=go1.25 x_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Fix tools filter enforcement, broken ${PWD} volume mount, and misleading ${VAR} note in TOML config Fix broken ${PWD} volume mount, misleading ${VAR} note, and undocumented tools field behaviour in TOML config Mar 19, 2026
@lpcox lpcox merged commit 9e54e59 into main Mar 19, 2026
3 checks passed
@lpcox lpcox deleted the copilot/fix-config-example-toml-discrepancy branch March 19, 2026 00:06
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.

📚 Documentation Reconciliation Report - 2026-03-18

3 participants