Fix build: inline removed getDefault* helpers, rename duplicate tests#2394
Fix build: inline removed getDefault* helpers, rename duplicate tests#2394
Conversation
PR #2383 removed getDefaultGuardPolicyJSON, getDefaultAllowOnlyScopeOwner, getDefaultAllowOnlyScopeRepo, getDefaultAllowOnlyMinIntegrity, and getDefaultDIFCSinkServerIDs from flags_difc.go but missed callers in root.go, proxy.go, and flags_difc_test.go. Inline the os.Getenv calls. PR #2381 added config_core_test.go and docker_helpers_and_env_test.go with test names that collide with existing tests in config_test.go and validation_env_test.go. Rename the new tests to avoid redeclaration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a broken main build by removing remaining references to deleted getDefault* env helper functions and resolves Go test compilation failures caused by duplicate test function names in the internal/config package.
Changes:
- Inline
os.Getenv(...)at the remaining call sites ininternal/cmdand update affected tests accordingly. - Rename colliding test functions in
internal/configto unique names to eliminate redeclaration errors. - Minor formatting cleanup in
internal/cmd/root.go.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/config/docker_helpers_and_env_test.go | Renames test functions to avoid collisions with existing tests in the same package. |
| internal/config/config_core_test.go | Renames a test to avoid colliding with an existing LoadFromFile multi-server test. |
| internal/cmd/root.go | Replaces removed getDefaultAllowOnly* callers with direct os.Getenv reads in env override handling. |
| internal/cmd/proxy.go | Replaces removed getDefaultGuardPolicyJSON() with direct env var default for --policy. |
| internal/cmd/flags_difc_test.go | Updates tests to assert against direct env var reads instead of removed helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| os.Unsetenv("MCP_GATEWAY_GUARDS_SINK_SERVER_IDS") | ||
| assert.Equal(t, "", getDefaultDIFCSinkServerIDs()) | ||
| assert.Equal(t, "", os.Getenv("MCP_GATEWAY_GUARDS_SINK_SERVER_IDS")) | ||
|
|
||
| os.Setenv("MCP_GATEWAY_GUARDS_SINK_SERVER_IDS", "safeoutputs,github") | ||
| assert.Equal(t, "safeoutputs,github", getDefaultDIFCSinkServerIDs()) | ||
| assert.Equal(t, "safeoutputs,github", os.Getenv("MCP_GATEWAY_GUARDS_SINK_SERVER_IDS")) |
There was a problem hiding this comment.
These assertions now directly validate os.Getenv("MCP_GATEWAY_GUARDS_SINK_SERVER_IDS") rather than a getDefault* helper. To keep the test intent clear, consider renaming TestGetDefaultDIFCSinkServerIDs (and/or the sub-assert messages) to reflect that it’s testing env-var passthrough behavior, not a dedicated defaulting helper.
| @@ -120,7 +120,7 @@ func TestValidateContainerID(t *testing.T) { | |||
| } | |||
|
|
|||
| // TestGetGatewayPortFromEnv tests the env-based gateway port parsing. | |||
There was a problem hiding this comment.
The comment says “TestGetGatewayPortFromEnv tests …” but the test function was renamed to TestGetGatewayPortFromEnv_Comprehensive. Update the comment to match the new function name so grepping/IDE navigation stays consistent.
| // TestGetGatewayPortFromEnv tests the env-based gateway port parsing. | |
| // TestGetGatewayPortFromEnv_Comprehensive tests the env-based gateway port parsing. |
Problem
Main is broken — two recent Repo Assist PRs introduced compilation errors:
PR [Repo Assist] refactor(cmd): inline trivial os.Getenv wrappers in flags_difc.go #2383 removed
getDefaultGuardPolicyJSON,getDefaultAllowOnlyScopeOwner,getDefaultAllowOnlyScopeRepo,getDefaultAllowOnlyMinIntegrity, andgetDefaultDIFCSinkServerIDsfromflags_difc.gobut missed callers inroot.go,proxy.go, andflags_difc_test.go. The PR couldn't build locally becauseproxy.golang.orgwas blocked by the firewall.PR [test] Add tests for config.LoadFromFile and related helpers #2381 added
config_core_test.goanddocker_helpers_and_env_test.gowith test function names (TestLoadFromFile_MultipleServers,TestValidateContainerID,TestGetGatewayPortFromEnv) that collide with existing tests inconfig_test.goandvalidation_env_test.go.Fix
os.Getenv()calls at the 5 remaining call sites (matching the pattern PR [Repo Assist] refactor(cmd): inline trivial os.Getenv wrappers in flags_difc.go #2383 applied inflags_difc.go)Verification
make agent-finishedpasses (build, lint, all tests).