-
Notifications
You must be signed in to change notification settings - Fork 14
Description
Overview
Analysis of 71 non-test Go files in internal/ catalogued 410 functions/methods across 14 packages. Overall, the codebase is well-organized with clear modular patterns and good use of Go generics (e.g., initGlobalLogger[T] in internal/logger/). The identified issues are limited but actionable: three micro-files that fragment trivially small pieces of code, Docker-specific helpers that have grown beyond the scope of internal/dockerutil/, and a test file without a matching implementation file.
Cluster Summary
| Package | Files | Functions | Assessment |
|---|---|---|---|
internal/logger/ |
11 | ~80 | ✅ Well-organized; quad-function pattern is intentional and documented |
internal/config/ |
10 | ~65 | |
internal/server/ |
10 | ~55 | errors.go) |
internal/difc/ |
5 | ~55 | ✅ Excellent per-responsibility split |
internal/mcp/ |
4 | ~38 | ✅ Clean protocol separation |
internal/cmd/ |
7 | ~18 | ✅ Modular flag-registration pattern works well |
internal/launcher/ |
3 | ~22 | ✅ Clean pool + log-helpers split |
internal/guard/ |
4 | ~16 | ✅ Clean interface/implementation/registry |
| Remaining packages | 17 | ~60 | ✅ Small, focused, appropriately scoped |
Full Report
Identified Issues
1. Micro-file: internal/config/config_logging.go — single constant (6 lines)
File content:
const DefaultLogDir = "/tmp/gh-aw/mcp-logs"Issue: A 6-line file containing one constant sits alongside internal/config/config_payload.go (27 lines), which already holds the analogous DefaultPayloadDir and DefaultPayloadSizeThreshold constants. The naming pattern config_logging.go / config_payload.go suggests both should live together as operational defaults, not in separate files.
Recommendation: Merge config_logging.go into config_payload.go (or rename the merged file to config_operational_defaults.go / config_defaults.go).
Estimated effort: 5 minutes
Impact: Removes a confusing micro-file; makes it easier to locate all gateway operational defaults in one place.
2. Micro-file: internal/logger/constants.go — two constants (12 lines)
File content:
const (
EnvDebug = "DEBUG"
EnvDebugColors = "DEBUG_COLORS"
)Issue: Two constants that directly control the Logger type's behavior live in a separate file from logger.go where Logger, New(), computeEnabled(), and matchPattern() are defined. The constants are tightly coupled to those functions.
Recommendation: Move EnvDebug and EnvDebugColors into logger.go.
Estimated effort: 5 minutes
Impact: Eliminates a 12-line file; keeps related constants co-located with the code that uses them.
3. Single-function file: internal/server/errors.go — one unexported function (31 lines)
File:
func logRuntimeError(errorType, detail string, r *http.Request, serverName *string) { ... }Call sites: Only called from internal/server/auth.go (2 call sites):
logRuntimeError("authentication_failed", "missing_auth_header", r, nil)
logRuntimeError("authentication_failed", "invalid_api_key", r, nil)Issue: An unexported helper function used exclusively by auth.go lives in its own file. This makes errors.go appear to be a package-level concern when it's actually an auth-only helper.
Recommendation: Move logRuntimeError into auth.go (where it is used) or into http_helpers.go if the intent is to make it available to other server handlers in the future.
Estimated effort: 10 minutes
Impact: Reduces file count without losing clarity; co-locates the function with its only callers.
4. Docker-inspection helpers in internal/config/validation_env.go could be in internal/dockerutil/
Functions in validation_env.go that are Docker utilities:
runDockerInspect(containerID, formatTemplate string) (string, error)checkDockerAccessible() boolcheckPortMapping(containerID, port string) (bool, error)checkStdinInteractive(containerID string) boolcheckLogDirMounted(containerID, logDir string) bool
Current internal/dockerutil/:
// Only one function today:
func ExpandEnvArgs(args []string) []string { ... }Issue: internal/dockerutil/ is the natural home for Docker-specific utilities, but it only has ExpandEnvArgs. The five inspection helpers in validation_env.go perform Docker operations (exec docker inspect, parse output) and are conceptually Docker utilities, not config-validation logic.
Note: These helpers are called exclusively within validation_env.go, so the coupling is currently tight. Moving them would require either:
- Moving them to
dockerutil/and importing that package fromconfig/, or - Extracting them into a new
internal/config/docker_helpers.gofile to at least separate Docker I/O from validation logic within the config package.
Recommendation: Create internal/config/docker_helpers.go to house runDockerInspect, checkDockerAccessible, checkPortMapping, checkStdinInteractive, and checkLogDirMounted. This separates Docker I/O concerns from the validation-result logic in validation_env.go without changing the package boundary.
Estimated effort: 30 minutes
Impact: validation_env.go becomes focused on interpreting results; Docker I/O is isolated for easier mocking/testing.
5. internal/config/config_guardpolicies_test.go has no corresponding implementation file
Test file: internal/config/config_guardpolicies_test.go (13 test cases, 14 KB)
Implementation: Guard-policy fields and handling are spread across config_core.go (GuardPolicies field definition) and config_stdin.go (JSON parsing, conversion logic).
Issue: The pattern throughout internal/config/ uses config_feature.go to describe the modular feature-registration framework. Guard policies are a discrete feature with their own test file, yet have no corresponding config_guardpolicies.go. Developers looking for guard-policy logic have no clear home file to navigate to.
Recommendation: Create internal/config/config_guardpolicies.go that consolidates the guard-policy related code (currently scattered in config_core.go and config_stdin.go) into a single file that is clearly the authoritative location for guard-policy configuration logic.
Estimated effort: 1–2 hours
Impact: Improves discoverability; makes the config_guardpolicies_test.go / config_guardpolicies.go pairing explicit and consistent with the rest of the package.
Well-Organized Patterns (No Action Needed)
internal/logger/global_helpers.go— The genericinitGlobalLogger[T]/closeGlobalLogger[T]pattern with aclosableLoggerconstraint is an excellent use of Go generics that eliminates five pairs of boilerplate functions. TheLog*/Log*Md/Log*WithServerquad-function sets are intentionally kept separate (different signatures, different callers) and are documented incommon.go.internal/difc/—agent.go,capabilities.go,evaluator.go,labels.go,resource.goeach have a single, clear responsibility. The pre-lock debug logging pattern is consistent.internal/mcp/http_transport.go—trySDKTransportproperly extracts the shared connection logic;tryStreamableHTTPTransport,trySSETransportare clean one-level wrappers.internal/cmd/flags_*.go— TheRegisterFlag/init()modular registration pattern is clean and consistent across all flag files.internal/config/config_feature.go— TheRegisterDefaults/RegisterStdinConverterregistration framework is idiomatic and well-documented.
Implementation Checklist
- Issue 1 — Merge
config_logging.gointoconfig_payload.go(or newconfig_defaults.go) - Issue 2 — Move
EnvDebug/EnvDebugColorsfromconstants.gointologger.go - Issue 3 — Move
logRuntimeErrorfromerrors.gointoauth.goorhttp_helpers.go - Issue 4 — Extract Docker-inspection helpers into
internal/config/docker_helpers.go - Issue 5 — Create
internal/config/config_guardpolicies.goto match the existing test file
Analysis Metadata
| Metric | Value |
|---|---|
| Go source files analyzed | 71 (excluding test files) |
| Functions/methods catalogued | ~410 |
| Semantic clusters identified | 14 packages × 2–5 sub-clusters |
| Micro-files found | 2 (config_logging.go, logger/constants.go) |
| Single-function outlier files | 1 (server/errors.go) |
| Misplaced helper groups | 1 (Docker helpers in validation_env.go) |
| Test/impl mismatches | 1 (config_guardpolicies_test.go without impl) |
| Detection method | Static AST grep + naming-pattern analysis + call-site verification |
| Analysis date | 2026-03-06 |
References: §22781813270
Generated by Semantic Function Refactoring