Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions .github/workflows/nightly-mcp-stress-test.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ mcp-servers:
filesystem:
type: stdio
container: "mcp/filesystem"
env:
ALLOWED_PATHS: "/tmp,/workspace"
entrypointArgs: ["/workspace"]
mounts:
- "/tmp/mcp-test-fs:/workspace:rw"
memory:
Expand Down Expand Up @@ -95,7 +94,7 @@ mcp-servers:
type: stdio
container: "mcr.microsoft.com/playwright:v1.49.1-noble"
args: ["--init", "--network", "host"]
entrypointArgs: ["--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com", "--allowed-origins", "localhost;localhost:*;127.0.0.1;127.0.0.1:*;github.com"]
entrypointArgs: ["--output-dir", "/tmp/gh-aw/mcp-logs/playwright", "--allowed-hosts", "localhost:*;127.0.0.1:*;github.com", "--allowed-origins", "localhost:*;127.0.0.1:*;github.com"]
# env:
# PLAYWRIGHT_BROWSERS_PATH: "/ms-playwright"
# Launch options to prevent ERR_BLOCKED_BY_CLIENT errors in CI testing
Expand Down
152 changes: 152 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1243,3 +1243,155 @@ args = ["run", "--rm", "-i", "test/container:latest"]
// Error should mention the duplicate key
assert.Contains(t, err.Error(), "line", "Error should mention line number")
}

// TestLoadFromStdin_FilesystemServerConfig tests that filesystem server configuration
// correctly passes directories as entrypointArgs instead of environment variables.
func TestLoadFromStdin_FilesystemServerConfig(t *testing.T) {
jsonConfig := `{
"mcpServers": {
"filesystem": {
"type": "stdio",
"container": "mcp/filesystem",
"entrypointArgs": ["/workspace"],
"mounts": ["/tmp/mcp-test-fs:/workspace:rw"]
}
},
"gateway": {
"port": 8080,
"domain": "localhost",
"apiKey": "test-key"
}
}`

r, w, _ := os.Pipe()
oldStdin := os.Stdin
os.Stdin = r
go func() {
w.Write([]byte(jsonConfig))
w.Close()
}()

cfg, err := LoadFromStdin()
os.Stdin = oldStdin

require.NoError(t, err, "LoadFromStdin() failed")
server, ok := cfg.Servers["filesystem"]
require.True(t, ok, "Server 'filesystem' not found")

assert.Equal(t, "docker", server.Command)
assert.True(t, contains(server.Args, "mcp/filesystem"), "Container name not found")

// Check that mount is present
hasMount := false
for i := 0; i < len(server.Args)-1; i++ {
if server.Args[i] == "-v" && server.Args[i+1] == "/tmp/mcp-test-fs:/workspace:rw" {
hasMount = true
break
}
}
assert.True(t, hasMount, "Mount not found in args")

// Check that /workspace is passed as an entrypoint arg (after the container name)
containerIdx := -1
for i, arg := range server.Args {
if arg == "mcp/filesystem" {
containerIdx = i
break
}
}
require.NotEqual(t, -1, containerIdx, "Container name not found in args")

// Verify that /workspace appears after the container name as an entrypoint arg
hasWorkspaceArg := false
for i := containerIdx + 1; i < len(server.Args); i++ {
if server.Args[i] == "/workspace" {
hasWorkspaceArg = true
break
}
}
assert.True(t, hasWorkspaceArg, "Expected /workspace as entrypoint arg after container name")
Comment on lines +1294 to +1312
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

The filesystem test verifies that "/workspace" appears after the container name, but it would still pass if "/workspace" were accidentally included both before and after the image (e.g., if it were incorrectly added as a Docker runtime arg too). Consider adding a check that "/workspace" does not appear anywhere before the container name to make the placement guarantee stricter.

This issue also appears on line 1393 of the same file.

Copilot uses AI. Check for mistakes.
}

// TestLoadFromStdin_PlaywrightServerConfig tests that playwright server configuration
// correctly separates Docker runtime flags (args) from playwright binary arguments (entrypointArgs).
func TestLoadFromStdin_PlaywrightServerConfig(t *testing.T) {
jsonConfig := `{
"mcpServers": {
"playwright": {
"type": "stdio",
"container": "mcr.microsoft.com/playwright:v1.49.1-noble",
"args": ["--init", "--network", "host"],
"entrypointArgs": [
"--output-dir", "/tmp/gh-aw/mcp-logs/playwright",
"--allowed-hosts", "localhost:*;127.0.0.1:*",
"--allowed-origins", "localhost:*;127.0.0.1:*"
]
}
},
"gateway": {
"port": 8080,
"domain": "localhost",
"apiKey": "test-key"
}
}`

r, w, _ := os.Pipe()
oldStdin := os.Stdin
os.Stdin = r
go func() {
w.Write([]byte(jsonConfig))
w.Close()
}()

cfg, err := LoadFromStdin()
os.Stdin = oldStdin

require.NoError(t, err, "LoadFromStdin() failed")
server, ok := cfg.Servers["playwright"]
require.True(t, ok, "Server 'playwright' not found")

assert.Equal(t, "docker", server.Command)

// Find the container name position in args
containerIdx := -1
for i, arg := range server.Args {
if arg == "mcr.microsoft.com/playwright:v1.49.1-noble" {
containerIdx = i
break
}
}
require.NotEqual(t, -1, containerIdx, "Container name not found in args")

// Verify Docker flags (--init, --network host) appear BEFORE the container name
hasInitBeforeContainer := false
hasNetworkBeforeContainer := false
for i := 0; i < containerIdx; i++ {
if server.Args[i] == "--init" {
hasInitBeforeContainer = true
}
if server.Args[i] == "--network" && i+1 < containerIdx && server.Args[i+1] == "host" {
hasNetworkBeforeContainer = true
}
}
assert.True(t, hasInitBeforeContainer, "Docker flag --init should appear before container name")
assert.True(t, hasNetworkBeforeContainer, "Docker flag --network host should appear before container name")

// Verify playwright binary args appear AFTER the container name
hasOutputDirAfterContainer := false
hasAllowedHostsAfterContainer := false
for i := containerIdx + 1; i < len(server.Args); i++ {
if server.Args[i] == "--output-dir" && i+1 < len(server.Args) && server.Args[i+1] == "/tmp/gh-aw/mcp-logs/playwright" {
hasOutputDirAfterContainer = true
}
if server.Args[i] == "--allowed-hosts" {
hasAllowedHostsAfterContainer = true
}
}
assert.True(t, hasOutputDirAfterContainer, "Playwright arg --output-dir should appear after container name")
assert.True(t, hasAllowedHostsAfterContainer, "Playwright arg --allowed-hosts should appear after container name")

// Verify Docker flags do NOT appear as duplicate entrypoint args after container
for i := containerIdx + 1; i < len(server.Args); i++ {
assert.NotEqual(t, "--init", server.Args[i], "Docker flag --init should not appear after container name")
}
}
Loading