[refactor] Semantic Function Clustering Analysis — Refactoring Opportunities

[github-actions]

🔧 Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw-mcpg — run §29779668922

Executive Summary

Analyzed 144 non-test Go source files across internal/. The codebase is generally well-organized with clear package boundaries. This report identifies 6 high-value refactoring opportunities — all concrete, actionable findings based on actual code duplication or semantic misplacement. No trivial or stylistic findings are included.

• Total files analyzed: 144
• Outlier/misplaced functions: 1 cluster
• Duplicate/near-duplicate logic: 3 clusters
• Scattered helper patterns: 2 clusters

Full Report

Function Inventory (by package)

Package	Files	Primary purpose
internal/auth/	2	Auth header parsing, agent ID generation
internal/cmd/	11	CLI wiring, flag registration, startup
internal/config/	16	Config parsing (TOML/JSON), validation, guard policy
internal/difc/	11	DIFC labels, evaluation, pipeline decisions
internal/envutil/	4	Env var access, Docker env args
internal/githubhttp/	4	GitHub API helpers, visibility checks
internal/guard/	9	Guard interface, WASM lifecycle/execution
internal/httputil/	4	Reusable HTTP utilities, TLS config, response helpers
internal/jqutil/	1	jq compiler options
internal/launcher/	5	Process/session lifecycle, health monitoring
internal/logger/	14	File/global/markdown/JSONL loggers
internal/mcp/	6	MCP protocol transport and connection
internal/middleware/	2	jq schema processing
internal/proxy/	7	GitHub API proxying, DIFC filtering
internal/sanitize/	1	Secret/data redaction
internal/server/	22	Gateway HTTP handling, session/auth/guard init
internal/syncutil/	2	Concurrency helpers
internal/sys/	2	Container/Docker detection
internal/tracing/	7	OTLP config resolution, provider setup
internal/util/	6	Generic helpers (strings, random, truncate)

---

Identified Issues

1. 🔴 Duplicate: Workflow-repo visibility / "force public" policy logic split across cmd and server

Priority: High — affects security-sensitive policy enforcement code.

Files involved:

• internal/cmd/proxy.go — proxyForcePublicReposIfNeeded()
• internal/server/guard_visibility.go — shouldForcePublicRepos(), computeForcePublicRepos(), verifySinkVisibilityAtRuntime(), resolveWorkflowRepoVisibility()
• internal/githubhttp/visibility.go — FetchRepoVisibility(), VerifySinkVisibility()

What's duplicated: Both proxy mode and gateway/server mode independently:

• Read GITHUB_REPOSITORY
• Resolve the GitHub auth token
• Call githubhttp.FetchRepoVisibility()
• Enforce "public repo ⇒ force public scope / visibility"
• Fail open on API failure

Code comparison:

// internal/cmd/proxy.go
func proxyForcePublicReposIfNeeded(ctx context.Context, policyJSON, token, apiURL string) string {
    // reads GITHUB_REPOSITORY, calls FetchRepoVisibility, overrides policy JSON
}

// internal/server/guard_visibility.go
func computeForcePublicRepos() bool {
    // reads GITHUB_REPOSITORY, calls FetchRepoVisibility, caches result
}

Recommendation:
Extract shared logic into internal/githubhttp/ or a new internal/policy/ package:

• ResolveWorkflowRepoVisibility(ctx, apiURL, token) (RepoVisibility, bool)
• ShouldForcePublicRepos(visibility RepoVisibility) bool

Let server/guard_visibility.go handle server-specific caching/logging; cmd/proxy.go delegates policy decision and JSON mutation to the shared helpers.

Estimated effort: 3-4 hours | Impact: Reduced duplication in security-sensitive policy path

---

2. 🔴 Duplicate: Request body read/restore in security middleware

Priority: High — correctness-sensitive code in the auth/HMAC path.

Files involved:

• internal/server/http_helpers.go — peekRequestBody() (the canonical helper)
• internal/server/hmac.go — hmacMiddleware() (reimplements the same pattern inline)

Duplicate pattern (both do the same operation):

// http_helpers.go — peekRequestBody
b, err := io.ReadAll(origBody)
// ...
r.Body = io.NopCloser(bytes.NewReader(b))

// hmac.go — hmacMiddleware (inline)
body, err = io.ReadAll(r.Body)
// ...
r.Body = io.NopCloser(bytes.NewReader(body))

Recommendation:

• Use peekRequestBody() from within hmacMiddleware(), or
• Promote peekRequestBody() to internal/httputil as ReadAndRestoreRequestBody(*http.Request) ([]byte, error)

Estimated effort: 1 hour | Impact: Single implementation of body-restore in auth paths

---

3. 🟡 Near-duplicate: HTTP response body reading scattered beyond httputil

Priority: Medium — httputil.ReadResponseBody* exists but is not consistently used.

Files involved:

• internal/httputil/response.go — ReadResponseBody(), ReadResponseBodyStrict() (canonical)
• internal/proxy/handler.go — forwardAndReadBody() (custom inline version)
• internal/config/validation_schema.go — fetchSchema() (inline io.ReadAll + close)
• internal/githubhttp/visibility.go — FetchRepoVisibility() (inline read pattern)

Pattern repeated in non-httputil locations:

defer resp.Body.Close()
body, err := io.ReadAll(resp.Body)
if resp.StatusCode != http.StatusOK { ... }

Recommendation:
Extend internal/httputil/response.go with variants:

• ReadResponseBodyWithLimit(resp, maxBytes) — for bounded reads
• DecodeJSONResponse(resp, target) — for JSON decode + close in one step

Migrate proxy.forwardAndReadBody and config.fetchSchema to use these helpers.

Estimated effort: 2-3 hours | Impact: Centralized HTTP response lifecycle

---

4. 🟡 Outlier: Guard policy resolution/mutation logic split across two server files

Priority: Medium — security-sensitive, high-complexity area.

Files involved:

• internal/server/guard_init.go — resolveGuardPolicy(), resolveWriteSinkPolicy(), ensureGuardInitialized()
• internal/server/guard_visibility.go — overrideToPublicScope(), isServerExemptFromSinkVisibility(), validateSinkVisibilityExemptServers(), verifySinkVisibilityAtRuntime()

Issue: These 8+ functions form a coherent "guard policy resolution and mutation" subsystem, but are split between initialization lifecycle (guard_init.go) and runtime visibility enforcement (guard_visibility.go). The split makes it hard to reason about the complete policy pipeline.

Recommendation:
Create internal/server/guard_policy.go and move all policy resolution/mutation there:

• resolveGuardPolicy()
• resolveWriteSinkPolicy()
• overrideToPublicScope()
• shouldForcePublicRepos()
• verifySinkVisibilityAtRuntime()
• isServerExemptFromSinkVisibility()

Keep guard_init.go focused on guard construction, WASM loading, and registration.

Estimated effort: 2 hours (mechanical moves) | Impact: Improved navigability of security-critical policy code

---

5. 🟢 Scattered: Directory-flag completion registration repeated in cmd

Priority: Low — minor but consistent pattern.

Files involved:

• internal/cmd/flags.go — registerFlagCompletions() marks directory flags for root command
• internal/cmd/proxy.go — newProxyCmd() separately marks --tls-dir

Pattern:
Both manually call cmd.MarkFlagDirname(flagName) across multiple flags. The codebase already centralizes other flag groups (registerGuardsModeFlag, registerTracingFlags), but directory completions are not.

Recommendation:
Add a helper in internal/cmd/flags.go:

func markDirFlags(cmd *cobra.Command, names ...string) {
    for _, name := range names {
        _ = cmd.MarkFlagDirname(name)
    }
}

Estimated effort: 30 minutes | Impact: Consistent with existing flag organization pattern

---

6. 🟢 Scattered: Generic file I/O helpers accumulating inside logger

Priority: Low (future-watch) — not urgent today, but trend is clear.

Files involved:

• internal/logger/fileutil.go — atomicWriteFile(), writeJSONToFile(), initLogFile() (contains a TODO noting the generic nature of atomicWriteFile)
• internal/proxy/tls.go — writePEM() (custom file-write with permissions)
• internal/server/session.go — directory creation + file persistence

Issue: logger/fileutil.go already has a TODO noting atomicWriteFile should migrate when a second consumer appears. The codebase now has multiple independent file-writing patterns in non-logger code.

Recommendation:
When a third file-writing consumer appears, promote generics into a dedicated package:

• internal/fileutil.AtomicWriteFile(path string, data []byte, perm os.FileMode) error
• internal/fileutil.EnsureDir(path string) error

Estimated effort: 1-2 hours when triggered | Impact: Cleaner package boundaries

---

Refactoring Recommendations

Priority 1 — High Impact

1. Extract shared force-public / visibility policy — consolidate cmd/proxy.go + server/guard_visibility.go into githubhttp or internal/policy
2. Centralize request body read/restore — use peekRequestBody in hmac.go, promote to httputil

Priority 2 — Medium Impact

3. Extend httputil.response.go — migrate proxy.forwardAndReadBody and config.fetchSchema
4. Create server/guard_policy.go — consolidate guard policy resolution/mutation from two files

Priority 3 — Low/Future

5. Add markDirFlags helper in cmd
6. Promote atomicWriteFile to internal/fileutil when next consumer appears

Implementation Checklist

• Review Priority 1 findings (visibility policy duplication, body read/restore)
• Create shared ResolveWorkflowRepoVisibility helper
• Use peekRequestBody in hmacMiddleware
• Extend httputil/response.go and migrate callers
• Create server/guard_policy.go via mechanical moves
• Add markDirFlags helper
• Track atomicWriteFile consumers for future extraction

Analysis Metadata

• Total Go Files Analyzed: 144 (non-test, internal/)
• Function Clusters Identified: 13 packages × ~avg 20 functions
• Outliers Found: 1 (guard policy split)
• Duplicates Detected: 3 (visibility policy, body read/restore, HTTP response body)
• Scattered Helpers: 2 (dir-flag completion, file I/O)
• Detection Method: Static function inventory + semantic clustering + targeted code comparison
• Analysis Date: 2026-07-20

References:

• §29779668922

Warning

Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

• awmgmcpg
• proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Semantic Function Refactoring · 84.9 AIC · ⊞ 8.5K · ◷