Conversation
…EnvFile to envutil Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/37d9c674-0805-410f-b63e-8d16a101a0e8 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors a few small utilities without changing their intended behavior: it inlines jq-schema query ID generation, reuses strutil for secret truncation, and moves .env loading into internal/envutil so the logic lives with other environment helpers.
Changes:
- Removed the one-line
generateRandomIDwrapper and inlined thestrutil.RandomHexWithFallbackcall in jq-schema middleware. - Updated
sanitize.TruncateSecretto delegate truncation tostrutil.TruncateWithSuffixwhile preserving current behavior. - Moved
.envfile loading frominternal/cmd/root.gointo a new exportedenvutil.LoadEnvFile, and relocated its tests.
Show a summary per file
| File | Description |
|---|---|
internal/middleware/jqschema.go |
Replaces the local random-ID helper with a constant plus direct strutil call. |
internal/middleware/jqschema_test.go |
Removes the test for the deleted wrapper function. |
internal/logger/sanitize/sanitize.go |
Refactors secret truncation to reuse strutil.TruncateWithSuffix. |
internal/envutil/envfile.go |
Adds the new exported .env loading helper under envutil. |
internal/envutil/envfile_test.go |
Moves and preserves test coverage for .env parsing behavior. |
internal/cmd/root.go |
Switches the CLI startup path to call envutil.LoadEnvFile. |
internal/cmd/root_test.go |
Removes the tests that were moved into internal/envutil. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 2
| func generateRandomID() string { | ||
| return strutil.RandomHexWithFallback(16) | ||
| } | ||
| // queryIDHexLen is the number of hex characters used for random query IDs. |
| } | ||
| defer file.Close() | ||
|
|
||
| log.Printf("Loading environment from %s...", path) |
|
@copilot merge main and address the review feedback |
…ntic-function-clustering-analysis Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/0a073261-820f-44e3-b7eb-6148cb3ca914 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Done in 325a2fb. Merged main and addressed both review items:
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:
If you need me to access, download, or install something from one of these locations, you can either:
|
Static analysis identified three refactoring opportunities: a redundant one-line wrapper, reimplemented truncation logic that bypasses
strutil, and an env-file utility misplaced in the CLI root command.Changes
Inline
generateRandomID(middleware/jqschema.go): Removed wrapper aroundstrutil.RandomHexWithFallback(16); introducedconst queryIDHexLen = 16and call the utility directly at the use site.sanitize.TruncateSecret→ delegates tostrutil.TruncateWithSuffix(logger/sanitize/sanitize.go): Was reimplementing byte-slice truncation inline; now callsstrutil.TruncateWithSuffix(input, 4, "...")for the truncation path. Semantics preserved (≤4-char inputs still return"...", empty returns"").Move
loadEnvFile→envutil.LoadEnvFile(envutil/envfile.go): Extracted 44-line.envparser fromcmd/root.gointo its natural home inenvutil; exported asLoadEnvFile. Removed now-unusedbufioandsanitizeimports fromroot.go. Tests relocated fromcmd/root_test.gotoenvutil/envfile_test.go.rpc_helpers.truncateAndSanitize: Already delegates tostrutil.Truncate— no change required.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/tmp/go-build3751646541/b509/launcher.test /tmp/go-build3751646541/b509/launcher.test -test.testlogfile=/tmp/go-build3751646541/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-errorsas 3823591/b314/ /home/REDACTED/.do-nilfunc . --gdwarf2 --64 04.o 3823�� 64/src/runtime/c-errorsas -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build623460966/b513/launcher.test /tmp/go-build623460966/b513/launcher.test -test.testlogfile=/tmp/go-build623460966/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build623460966/b494/vet.cfg Y2BynxhhJ -o/tmp/go-build3751646541/b169//_cgo_.o x_amd64/vet -c -I /tmp/go-build3751646541/b169/ x_amd64/vet ortc�� o t.go x_amd64/compile -pthread -Wl,--no-gc-sect-test.testlogfile=/tmp/go-build623460966/b001/testlog.txt -fmessage-length-test.paniconexit0 x_amd64/compile(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build3751646541/b491/config.test /tmp/go-build3751646541/b491/config.test -test.testlogfile=/tmp/go-build3751646541/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go elemetry.io/otel@v1.43.0/semconv-imultiarch x_amd64/vet 3823591/b150/ --64 -o x_amd64/vet -W -MOQeqnD5 /tmp/go-build287-ifaceassert 3823591/b300=> . .io/otel/metric --64 ortcfg(dns block)/tmp/go-build623460966/b495/config.test /tmp/go-build623460966/b495/config.test -test.testlogfile=/tmp/go-build623460966/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s ner_�� se 1646541/b071/vet.cfg ache/go/1.25.9/x64/pkg/tool/linux_amd64/vet . -imultiarch x86_64-linux-gnu ache/go/1.25.9/x64/pkg/tool/linux_amd64/vet -W -I /tmp/go-build3751646541/b169/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet(dns block)nonexistent.local/tmp/go-build3751646541/b509/launcher.test /tmp/go-build3751646541/b509/launcher.test -test.testlogfile=/tmp/go-build3751646541/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-errorsas 3823591/b314/ /home/REDACTED/.do-nilfunc . --gdwarf2 --64 04.o 3823�� 64/src/runtime/c-errorsas -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build623460966/b513/launcher.test /tmp/go-build623460966/b513/launcher.test -test.testlogfile=/tmp/go-build623460966/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build623460966/b494/vet.cfg Y2BynxhhJ -o/tmp/go-build3751646541/b169//_cgo_.o x_amd64/vet -c -I /tmp/go-build3751646541/b169/ x_amd64/vet ortc�� o t.go x_amd64/compile -pthread -Wl,--no-gc-sect-test.testlogfile=/tmp/go-build623460966/b001/testlog.txt -fmessage-length-test.paniconexit0 x_amd64/compile(dns block)slow.example.com/tmp/go-build3751646541/b509/launcher.test /tmp/go-build3751646541/b509/launcher.test -test.testlogfile=/tmp/go-build3751646541/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-errorsas 3823591/b314/ /home/REDACTED/.do-nilfunc . --gdwarf2 --64 04.o 3823�� 64/src/runtime/c-errorsas -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build623460966/b513/launcher.test /tmp/go-build623460966/b513/launcher.test -test.testlogfile=/tmp/go-build623460966/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build623460966/b494/vet.cfg Y2BynxhhJ -o/tmp/go-build3751646541/b169//_cgo_.o x_amd64/vet -c -I /tmp/go-build3751646541/b169/ x_amd64/vet ortc�� o t.go x_amd64/compile -pthread -Wl,--no-gc-sect-test.testlogfile=/tmp/go-build623460966/b001/testlog.txt -fmessage-length-test.paniconexit0 x_amd64/compile(dns block)this-host-does-not-exist-12345.com/tmp/go-build3751646541/b518/mcp.test /tmp/go-build3751646541/b518/mcp.test -test.testlogfile=/tmp/go-build3751646541/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-errorsas om/spf13/pflag@v-ifaceassert x_amd64/vet --gdwarf-5 g/protobuf/inter--version -o x_amd64/vet -p DELC/KeoNfyou8G7ZaqPWDELC -trimpath x_amd64/vet -I 3823591/b469/ -I x_amd64/vet(dns block)/tmp/go-build623460966/b522/mcp.test /tmp/go-build623460966/b522/mcp.test -test.testlogfile=/tmp/go-build623460966/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s /home/REDACTED/.cache/go-build/77/7710171347be097cb0663f048e8f5ad858c953b3f355354b9ed9239a70adf7ca-d /testutil/mcptest/example_test.g-p /testutil/mcptest/gateway_integrgithub.com/github/gh-aw-mcpg/internal/oidc x_amd64/vet 64/src/net(dns block)If you need me to access, download, or install something from one of these locations, you can either: