refactor: reduce duplicate code in loggers, validation, and session handling

[Copilot]

Three structural duplication patterns in internal/logger, internal/config, and internal/server accumulated ~500 lines of repetitive boilerplate with divergence risk.

Logger initialization closures → named functions

Each Init*Logger function used anonymous inline closures for setup and error handling. Extracted to named package-level functions per logger type:

// Before: anonymous closures inline in InitFileLogger
logger, err := initLogger(logDir, fileName, os.O_APPEND,
    func(file *os.File, logDir, fileName string) (*FileLogger, error) { ... },
    func(err error, logDir, fileName string) (*FileLogger, error) { ... },
)

// After: named functions
logger, err := initLogger(logDir, fileName, os.O_APPEND, setupFileLogger, handleFileLoggerError)

Applied to FileLogger, MarkdownLogger, JSONLLogger, and ToolsLogger.

Validation log helpers

30+ near-identical logValidation.Printf(...) calls in validation.go standardized via three helpers:

func logValidateServerStart(name, serverType string)
func logValidateServerPassed(name string)
func logValidateServerFailed(name, reason string)

withLock helper on logger types

Replaced repeated mu.Lock() / defer mu.Unlock() preambles with a per-type withLock(fn func() error) error method on FileLogger, JSONLLogger, MarkdownLogger, and ToolsLogger. Updated Close() and write methods accordingly.

session.go double-checked locking → syncutil.GetOrCreate

requireSession previously hand-rolled read→check→write→double-check locking. Replaced with the existing syncutil.GetOrCreate helper (same pattern already used in server_file_logger.go):

isNew := false
if _, err := syncutil.GetOrCreate(&us.sessionMu, us.sessions, sessionID, func() (*Session, error) {
    s := NewSession(sessionID, "")
    isNew = true
    return s, nil
}); err != nil {
    return err
}

Updated common.go documentation to reflect the new withLock and named-function patterns.

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-build758752077/b330/launcher.test /tmp/go-build758752077/b330/launcher.test -test.testlogfile=/tmp/go-build758752077/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go --local 86_64/as user.email (dns block)
  • Triggering command: /tmp/go-build972084089/b334/launcher.test /tmp/go-build972084089/b334/launcher.test -test.testlogfile=/tmp/go-build972084089/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2374296311/b001/vet.cfg ache/go/1.25.8/x64/src/net 64/src/encoding/gob/dec_helpers.go cfg -I . -imultiarch ache/go/1.25.8/x-buildtags -uns�� 752077/b258/_pkg-errorsas /tmp/go-build758-ifaceassert ache/go/1.25.8/x-nilfunc _amd64.s telabs/wazero/in-atomic -D_FORTIFY_SOURC-bool ache/go/1.25.8/x-buildtags (dns block)
  • Triggering command: /tmp/go-build2612040108/b334/launcher.test /tmp/go-build2612040108/b334/launcher.test -test.testlogfile=/tmp/go-build2612040108/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s /usr�� ry=1 ga/m4mMaA4EfUuX_-tests bash /opt/hostedtoolc/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet cfg 64/pkg/tool/linu/tmp/go-build532890590/b158/vet.cfg bash --no�� --noprofile 64/pkg/tool/linux_amd64/vet .13/x64/bash se cfg x_amd64/vet ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet (dns block)
• https://api.github.com/graphql
  • Triggering command: /usr/bin/gh gh issue close 2755 --repo github/gh-aw-mcpg --comment Resolved in PR #2790: refactored logger initialization closures to named functions, added validation log helpers, added withLock helpers to logger types, and refactored requireSession to use syncutil.GetOrCreate. by/734792accd854/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/compile 065 -lang=go1.25 065/init.pid 644 9221a53b1917004b-p runtime-runc/mobgithub.com/github/gh-aw-mcpg/internal/server /systemd-sysctl io.containerd.rugit -c=4 -certificates.crcopilot/duplicate-code-analysis-report 31d/log.json (http block)
  • Triggering command: /usr/bin/gh gh issue close 2756 --repo github/gh-aw-mcpg --comment Resolved in PR #2790: refactored logger initialization closures to named functions, added validation log helpers, added withLock helpers to logger types, and refactored requireSession to use syncutil.GetOrCreate. -errorsas 065 -nilfunc (http block)
  • Triggering command: /usr/bin/gh gh issue close 2757 --repo github/gh-aw-mcpg --comment Resolved in PR #2790: refactored logger initialization closures to named functions, added validation log helpers, added withLock helpers to logger types, and refactored requireSession to use syncutil.GetOrCreate. io.containerd.ru/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/link a8d469f49c8ee67b-o 084089/b330/vet./tmp/go-build1108576084/b001/server.test /usr/bin/runc.or-importcfg init�� 64/pkg/tool/linu-s y ctl ntime.v2.task/mogit -buildtags 64/pkg/tool/linucopilot/duplicate-code-analysis-report ctl (http block)
• https://api.github.com/repos/github/gh-aw-mcpg/issues/2754
  • Triggering command: /usr/bin/gh gh api -X PATCH repos/github/gh-aw-mcpg/issues/2754 -f state=closed "CURL_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt", "REQUESTS_CA_Bcopilot/duplicate-code-analysis-report ntime.v2.task/modocker main -lang=go1.25 table.d/chrony-onoffline /usr�� d -n 10 /var/run/docker/runtime-runc/moby iginal /run/containerd/git --log-format 908/log.json iginal (http block)
• https://api.github.com/repos/github/gh-aw-mcpg/issues/2755
  • Triggering command: /usr/bin/gh gh api -X PATCH repos/github/gh-aw-mcpg/issues/2755 -f state=closed /bin/java io.containerd.ru/usr/libexec/docker/cli-plugins/docker-buildx -ifaceassert -nilfunc /bin/java /usr�� d -n 10 io.containerd.runtime.v2.task/moby/ec75429440bf8--log-format erd-shim-runc-v2 io.containerd.rugit 849bc50063d4bbbaadd json erd-shim-runc-v2-v (http block)
  • Triggering command: /usr/bin/curl curl -s -X PATCH REDACTED -H Authorization: token ****** -H Content-Type: application/json -d {"state":"closed"} aw-mcpg/test/integration/start_gateway_with_pipe.sh --ro�� 115b774795f51db055fc7f996e03d7a54f3702b53ed9bbf2 runtime-runc/moby re-commit-msg io.containerd.rubash functions for l--norc --systemd-cgroup--noprofile re-commit-msg (http block)
  • Triggering command: /usr/bin/curl curl -v -X PATCH REDACTED -H Authorization: token ****** -H Content-Type: application/json -d {"state":"closed"} csi/net-interfac-v 954f�� l/config/validation.go io.containerd.runtime.v2.task/moby/901063bdb288e--log-format bin/bash io.containerd.rubash a361f2191e0cbe7c--norc json docker (http block)
• https://api.github.com/repos/github/gh-aw-mcpg/issues/2756
  • Triggering command: /usr/bin/gh gh api -X PATCH repos/github/gh-aw-mcpg/issues/2756 -f state=closed /bin/java da27db1da389d8d6/usr/libexec/docker/cli-plugins/docker-buildx -ifaceassert df1eeab6e3b61bce271/log.json /bin/java /usr�� /usr/java/packages/lib:/usr/lib64:/lib64:/lib:/usr/lib y bash ntime.v2.task/mogit --log-format df1eeab6e3b61bce. bash (http block)
• https://api.github.com/repos/github/gh-aw-mcpg/issues/2757
  • Triggering command: /usr/bin/gh gh api -X PATCH repos/github/gh-aw-mcpg/issues/2757 -f state=closed d-dispatcher/off.d/chrony-onoffline ntime.v2.task/modocker -buildtags d0d99cb50954aa1dd34960fd0baf0e9b. d-dispatcher/off.d/chrony-onoffline --ro�� d -n 10 y deql ntime.v2.task/mogit df1eeab6e3b61bceadd c9934293d0945041. deql (http block)
• invalid-host-that-does-not-exist-12345.com
  • Triggering command: /tmp/go-build758752077/b315/config.test /tmp/go-build758752077/b315/config.test -test.testlogfile=/tmp/go-build758752077/b315/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go1.25.8 -c=4 -nolocalimports -importcfg /tmp/go-build758752077/b192/importcfg -pack /opt/hostedtoolcache/go/1.25.8/x64/src/net/http/httptest/httptest.go rtcf�� eee754.go W2BDEJWaf ache/go/1.25.8/x-ffile-prefix-map=/opt/hostedtoolcache/go/1.25.8/x64=/_/GOROOT pull.rebase (dns block)
  • Triggering command: /tmp/go-build837462839/b315/config.test /tmp/go-build837462839/b315/config.test -test.testlogfile=/tmp/go-build837462839/b315/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true ache/go/1.25.8/x64/src/runtime/c-p Jxl7/weSoYHfkM6pMRgR2Jxl7 x_amd64/vet (dns block)
  • Triggering command: /tmp/go-build972084089/b319/config.test /tmp/go-build972084089/b319/config.test -test.testlogfile=/tmp/go-build972084089/b319/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1849658217/b202/vet.cfg 9lJ78QKSc /tmp/go-build758752077/b019/vet.cfg ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet (dns block)
• nonexistent.local
  • Triggering command: /tmp/go-build758752077/b330/launcher.test /tmp/go-build758752077/b330/launcher.test -test.testlogfile=/tmp/go-build758752077/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go --local 86_64/as user.email (dns block)
  • Triggering command: /tmp/go-build972084089/b334/launcher.test /tmp/go-build972084089/b334/launcher.test -test.testlogfile=/tmp/go-build972084089/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2374296311/b001/vet.cfg ache/go/1.25.8/x64/src/net 64/src/encoding/gob/dec_helpers.go cfg -I . -imultiarch ache/go/1.25.8/x-buildtags -uns�� 752077/b258/_pkg-errorsas /tmp/go-build758-ifaceassert ache/go/1.25.8/x-nilfunc _amd64.s telabs/wazero/in-atomic -D_FORTIFY_SOURC-bool ache/go/1.25.8/x-buildtags (dns block)
  • Triggering command: /tmp/go-build2612040108/b334/launcher.test /tmp/go-build2612040108/b334/launcher.test -test.testlogfile=/tmp/go-build2612040108/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s /usr�� ry=1 ga/m4mMaA4EfUuX_-tests bash /opt/hostedtoolc/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet cfg 64/pkg/tool/linu/tmp/go-build532890590/b158/vet.cfg bash --no�� --noprofile 64/pkg/tool/linux_amd64/vet .13/x64/bash se cfg x_amd64/vet ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet (dns block)
• slow.example.com
  • Triggering command: /tmp/go-build758752077/b330/launcher.test /tmp/go-build758752077/b330/launcher.test -test.testlogfile=/tmp/go-build758752077/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go --local 86_64/as user.email (dns block)
  • Triggering command: /tmp/go-build972084089/b334/launcher.test /tmp/go-build972084089/b334/launcher.test -test.testlogfile=/tmp/go-build972084089/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2374296311/b001/vet.cfg ache/go/1.25.8/x64/src/net 64/src/encoding/gob/dec_helpers.go cfg -I . -imultiarch ache/go/1.25.8/x-buildtags -uns�� 752077/b258/_pkg-errorsas /tmp/go-build758-ifaceassert ache/go/1.25.8/x-nilfunc _amd64.s telabs/wazero/in-atomic -D_FORTIFY_SOURC-bool ache/go/1.25.8/x-buildtags (dns block)
  • Triggering command: /tmp/go-build2612040108/b334/launcher.test /tmp/go-build2612040108/b334/launcher.test -test.testlogfile=/tmp/go-build2612040108/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s /usr�� ry=1 ga/m4mMaA4EfUuX_-tests bash /opt/hostedtoolc/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet cfg 64/pkg/tool/linu/tmp/go-build532890590/b158/vet.cfg bash --no�� --noprofile 64/pkg/tool/linux_amd64/vet .13/x64/bash se cfg x_amd64/vet ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet (dns block)
• this-host-does-not-exist-12345.com
  • Triggering command: /tmp/go-build758752077/b339/mcp.test /tmp/go-build758752077/b339/mcp.test -test.testlogfile=/tmp/go-build758752077/b339/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go --local ndor/bin/as committer.name (dns block)
  • Triggering command: /tmp/go-build972084089/b343/mcp.test /tmp/go-build972084089/b343/mcp.test -test.testlogfile=/tmp/go-build972084089/b343/testlog.txt -test.paniconexit0 -test.timeout=10m0s -uns�� -unreachable=false /tmp/go-build758752077/b059/vet.cfg ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet -dumpbase _cgo_.c -dumpbase-ext ache/go/1.25.8/x-importcfg -I /opt/hostedtoolc-s -I bash --gdwarf-5 --64 -o /opt/hostedtoolc-extld=gcc (dns block)
  • Triggering command: /tmp/go-build2612040108/b343/mcp.test /tmp/go-build2612040108/b343/mcp.test -test.testlogfile=/tmp/go-build2612040108/b343/testlog.txt -test.paniconexit0 -test.timeout=10m0s --no�� --noprofile pA/uZZaPODy050ia-buildtags docker-buildx /opt/hostedtoolc/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet cfg 64/pkg/tool/linu/tmp/go-build532890590/b139/vet.cfg docker-buildx -ato�� /logger/common.go /logger/file_logger.go 64/pkg/tool/linux_amd64/compile -errorsas -ifaceassert -nilfunc 64/pkg/tool/linux_amd64/compile (dns block)

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

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

---

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

[Copilot]

Pull request overview

This PR refactors several duplicated patterns across internal/logger, internal/config, and internal/server to reduce boilerplate and centralize common concurrency/initialization logic.

Changes:

• Extracts inline logger init closures into named setup*Logger / handle*LoggerError functions, and adds per-logger withLock helpers to reduce repeated mutex boilerplate.
• Adds small helper functions to standardize repeated server-validation log messages.
• Refactors UnifiedServer.requireSession to use syncutil.GetOrCreate instead of hand-rolled double-checked locking.

Reviewed changes

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

Show a summary per file

File	Description
internal/server/session.go	Replaces manual double-checked locking in requireSession with syncutil.GetOrCreate.
internal/logger/tools_logger.go	Extracts init callbacks into named functions; adds withLock for error-returning methods.
internal/logger/markdown_logger.go	Extracts init callbacks into named functions; adds withLock and uses it for Close().
internal/logger/jsonl_logger.go	Extracts init callbacks into named functions; adds withLock and uses it for Close().
internal/logger/file_logger.go	Extracts init callbacks into named functions; adds withLock and uses it for Close().
internal/logger/common.go	Updates package docs to describe the new withLock + named-callback patterns.
internal/config/validation.go	Adds logValidateServer* helpers and replaces repeated Printf calls.

---

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

[Copilot]

The doc comment claims all logger types use a per-type withLock helper for Close(), but several logger types (e.g., ServerFileLogger, ToolsLogger's no-op Close) don’t follow this pattern. Please reword to either scope it to the specific loggers that implement withLock (File/Markdown/JSONL/Tools) or adjust the guidance to cover exceptions accurately.

[Copilot]

This documentation says all logger types use the generic initLogger() helper, but some loggers in this package are initialized differently (e.g., ServerFileLogger doesn’t use initLogger). Suggest narrowing the statement to the file-based global loggers that actually use initLogger, or explicitly call out the exceptions.