[refactor] Semantic Function Clustering: Outliers, Duplication, and Organization Improvements

[github-actions]

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

Overview

Analysis of 78 non-test Go files across internal/ (plus main.go) identified five actionable refactoring opportunities: four carried over unaddressed from the previous run (§23267034906) and one new finding from the latest commit (9322ba8 feat: Surface DIFC-filtered items in tool responses).

The new internal/server/difc_log.go added by this commit is well-structured. The one new issue is a minor unused struct field (FilterReason) that adds maintenance surface without benefit. The four pre-existing issues remain low-effort, high-clarity changes (~60 minutes total).

Full Report

Function Inventory Summary

Package	Files	Functions (approx)
internal/difc	7	~80
internal/logger	11	~65
internal/config	9	~60
internal/server	10	~60
internal/guard	6	~40
internal/mcp	6	~35
internal/cmd	7	~30
internal/launcher	3	~25
Other packages	9	~30
Root (main.go)	1	~5
Total	79	~430

---

Identified Issues

1. Redundant Private Wrapper Function (carried over)

File: internal/mcp/connection.go:40–42

// REDUNDANT — unexported wrapper that only delegates to the exported function
func getAgentTagsSnapshotFromContext(ctx context.Context) (*AgentTagsSnapshot, bool) {
    return GetAgentTagsSnapshotFromContext(ctx)
}

This unexported function is called exactly once at line 296. It provides zero abstraction — it solely calls its exported counterpart GetAgentTagsSnapshotFromContext. The call site can call the exported function directly.

Recommendation: Remove the private wrapper. Update line 296 to call GetAgentTagsSnapshotFromContext directly.

Estimated effort: 5 minutes
Impact: Eliminates unnecessary indirection; removes confusion about why two functions have identical signatures.

---

2. Missing Named Constant for "backend-id" Context Key (carried over)

File: internal/server/http_helpers.go:105

// INCONSISTENT — magic string cast, no named constant
ctx = context.WithValue(ctx, mcp.ContextKey("backend-id"), backendID)

The other context keys in the mcp package have proper named constants:

• mcp.SessionIDContextKey = "awmg-session-id" (mcp/connection.go:29)
• mcp.AgentTagsSnapshotContextKey = "awmg-agent-tags-snapshot" (mcp/connection.go:32)

But "backend-id" has no named constant — the string is cast inline with no namespace prefix (awmg-), creating inconsistency.

Recommendation: Add a named constant in internal/mcp/connection.go:

// BackendIDContextKey stores the routed server backend ID in context
const BackendIDContextKey ContextKey = "awmg-backend-id"

Then update http_helpers.go:105 to use mcp.BackendIDContextKey.

Estimated effort: 10 minutes
Impact: Consistent with the existing context key pattern; improves discoverability.

---

3. authMiddleware Bypasses auth.ValidateAPIKey() (carried over)

File: internal/server/auth.go:57

The authMiddleware docstring explicitly references auth.ValidateAPIKey() as the validation function to use, but the implementation performs a raw string comparison instead:

// Documented as using auth.ValidateAPIKey() for key validation
if authHeader != apiKey {   // ← bypasses the dedicated function

The auth.ValidateAPIKey() function in internal/auth/header.go:97 encodes the "auth disabled when expected=empty" semantic. While applyAuthIfConfigured guards against an empty key, the raw comparison bypasses the package's intended API and creates an inconsistency between documentation and implementation.

Recommendation: Replace the direct comparison in authMiddleware:

if !auth.ValidateAPIKey(authHeader, apiKey) {
    // ... error handling
}

Estimated effort: 15 minutes
Impact: Aligns implementation with documentation; uses the auth package's intended API.

---

4. RUNNING_IN_CONTAINER Env Var Not Checked in config/validation_env.go (carried over)

Files:

• internal/tty/container.go:30 — checks RUNNING_IN_CONTAINER
• internal/config/validation_env.go:152 — detectContainerized() does not check it

Check	tty/container.go	config/validation_env.go:detectContainerized()
/.dockerenv file	✓	✓
/proc/*/cgroup for docker/containerd	✓	✓
RUNNING_IN_CONTAINER env var	✓	✗ missing

RUNNING_IN_CONTAINER=true is documented as the override to force container detection when file-based signals are unavailable. However, detectContainerized() ignores this env var, so --validate-env fails to detect containerized execution in environments where the documented override is used.

Recommendation: Add the env var check to detectContainerized() in config/validation_env.go:

if os.Getenv("RUNNING_IN_CONTAINER") == "true" {
    return true, ""
}

Estimated effort: 20 minutes
Impact: Fixes a behavioral gap in --validate-env; aligns the two container detectors.

---

5. FilteredCollectionLabeledData.FilterReason Field Is Never Read in Production Code (new)

File: internal/difc/resource.go:143 and internal/difc/evaluator.go:365

The FilteredCollectionLabeledData struct introduced in the latest commit has a FilterReason field that is hardcoded to "DIFC policy" on construction:

// internal/difc/evaluator.go:365
filtered := &FilteredCollectionLabeledData{
    ...
    FilterReason: "DIFC policy",  // ← always this string, never read
}

FilterReason is never accessed in any non-test production code — only test assertions verify the hardcoded string. The per-item reason is already available via FilteredItemDetail.Reason (which is set from result.Reason and actively used in difc_log.go and buildDIFCFilteredNotice).

Recommendation: Either remove FilterReason from the struct (since item-level reasons are sufficient) or promote it to a typed constant if a collection-level reason label is genuinely needed:

// Option A: Remove unused field entirely
type FilteredCollectionLabeledData struct {
    Accessible []LabeledItem
    Filtered   []FilteredItemDetail
    TotalCount int
    // FilterReason removed
}

// Option B: Named constant (if collection-level reason is needed in future)
const filterReasonDIFCPolicy = "DIFC policy"

Estimated effort: 10 minutes
Impact: Removes dead code; eliminates the implicit contract that FilterReason must always be "DIFC policy".

---

Semantic Clusters (Well-Organized Areas)

The following areas are well-organized and require no changes:

✓ New difc_log.go — logFilteredItems, buildDIFCFilteredNotice, and three private helpers (getStringField, extractAuthorLogin, extractNumberField) are cleanly self-contained in the new file with clear single responsibility.

✓ FilteredItemLogEntry / JSONLFilteredItem placement — Correctly located in internal/logger/jsonl_logger.go; embedding pattern avoids parallel struct maintenance.

✓ String utilities — internal/strutil/truncate.go is a clean central location; all consumers correctly delegate to it.

✓ Logger generics — internal/logger/global_helpers.go uses Go generics to eliminate duplication across logger types.

✓ Config validation split — validation.go, validation_env.go, validation_schema.go have clear single responsibilities.

✓ Docker utilities — internal/mcp/dockerenv.go and internal/config/docker_helpers.go serve distinct purposes.

---

Refactoring Recommendations

Priority 1 — Low effort, high clarity (< 30 min total)

1. Remove getAgentTagsSnapshotFromContext private wrapper (mcp/connection.go:40–42, update call at line 296)
2. Add BackendIDContextKey constant to mcp/connection.go, update server/http_helpers.go:105
3. Use auth.ValidateAPIKey() in authMiddleware instead of raw authHeader != apiKey

Priority 2 — Behavioral fix and dead code (< 30 min total)

4. Add RUNNING_IN_CONTAINER env var check to config/validation_env.go:detectContainerized()
5. Remove or promote FilterReason field from difc/resource.go FilteredCollectionLabeledData

---

Implementation Checklist

• Remove getAgentTagsSnapshotFromContext private wrapper (mcp/connection.go:40–42, update call at line 296)
• Add BackendIDContextKey constant to mcp/connection.go and update server/http_helpers.go:105
• Replace authHeader != apiKey with !auth.ValidateAPIKey(authHeader, apiKey) in server/auth.go:57
• Add RUNNING_IN_CONTAINER env var check in config/validation_env.go:detectContainerized()
• Remove or promote FilterReason field in difc/resource.go (update evaluator.go and evaluator_test.go)
• Run make agent-finished to verify all changes pass lint, build, and tests

---

Analysis Metadata

• Total Go files analyzed: 78 (excluding _test.go)
• Total functions cataloged: ~430
• Outliers/issues found: 5 (4 carried over, 1 new)
• New issues from latest commit: 1 (FilterReason unused field)
• Detection method: Naming pattern analysis + cross-package function comparison + implementation review
• Analysis date: 2026-03-19

References:

• §23316910004
• §23267034906 (previous run)

Generated by Semantic Function Refactoring · ◷

[github-actions]

Closing this issue as a new semantic function refactoring analysis is being performed for workflow run §23362442784.