Skip to content

[refactor] pkg/stringutil sanitize.go bundles four unrelated concerns #60153

Description

@github-actions

Executive Summary

Analyzed 10 non-test Go files across the precomputed package slice: pkg/stats (1 file) and pkg/stringutil (9 files). Overall organization is good — 8 of the 9 stringutil files and the single stats file are single-purpose and cleanly scoped. One file, sanitize.go, bundles four distinct concerns under a shared Sanitize* naming convention and is the primary actionable finding. A secondary, low-priority duplication was found in pat_validation.go.

Note on tooling: Serena's Go language server (gopls) could not initialize in this sandbox (Go toolchain not installed on the analysis host), so symbol-level analysis fell back to Serena's non-LSP search_for_pattern plus direct file reads rather than get_symbols_overview/find_referencing_symbols. Findings below were manually cross-checked against full file contents.

Function Inventory

pkg/stats (1 file)

  • statvar.goStatVar type: Add, Count, Min, Max, Mean, SampleVariance, SampleStdDev, Median. Single cohesive purpose (Welford's online statistics). No issues.

pkg/stringutil (9 files)

File Functions Purpose
ansi.go StripANSI + 4 private helpers ANSI escape sequence stripping
fuzzy_match.go FindClosestMatches, LevenshteinDistance Fuzzy string matching
identifiers.go NormalizeWorkflowName, NormalizeSafeOutputIdentifier, NormalizeIdentifierToHyphens, MarkdownToLockFile, LockFileToMarkdown Workflow identifier/path normalization
pat_validation.go PATType + ClassifyPAT, ValidateCopilotPAT, GetPATTypeDescription GitHub PAT classification
sanitize.go 7 exported + 4 private functions (see below) Mixed concerns — see finding #1
stringutil.go Truncate, FormatList, IsPositiveInteger Misc small string utilities (acceptable catch-all)
urls.go NormalizeGitHubHostURL, ExtractDomainFromURL + private fallback URL/domain handling
version.go ParseVersionValue Version value coercion
whitespace.go NormalizeWhitespace, NormalizeLeadingWhitespace Whitespace normalization

Clustering results

Naming clusters (Normalize*, Sanitize*) map cleanly onto topic-scoped files in every case except sanitize.go, where the shared Sanitize prefix hides four unrelated responsibilities in one 343-line file.

Identified Issues

1. sanitize.go bundles four unrelated concerns (Priority 1)

File: pkg/stringutil/sanitize.go (343 lines — the largest file in the package)

The file groups functions purely by the Sanitize name prefix, not by responsibility:

  • General name/filename sanitization: SanitizeName, SanitizeOptions, SanitizeForFilename (+ helpers normalizeSanitizeSeparators, buildSanitizePreservePattern, applySanitizePattern, logSanitizeInput)
  • Secret redaction in log/error text (security concern, not identifier formatting): SanitizeErrorMessage (+ secretNamePattern, pascalCaseSecretPattern, commonWorkflowKeywords)
  • Code-identifier generation for generated scripts: SanitizeIdentifierName, SanitizeParameterName, SanitizePythonVariableName (+ isASCIIAlphanumeric)
  • MCP tool ID cleanup: SanitizeToolID

SanitizeErrorMessage in particular has nothing in common with the other three clusters — it redacts secret-shaped substrings from log messages, whereas the rest reshape identifiers/filenames into valid character sets. Co-locating a security-redaction utility with generic string cleanup makes both harder to find and to reason about independently.

Recommendation: Split by responsibility:

  • Keep SanitizeName, SanitizeOptions, SanitizeForFilename in sanitize.go
  • Move SanitizeErrorMessage + its patterns/keyword table to a new secret_redaction.go
  • Move SanitizeIdentifierName, SanitizeParameterName, SanitizePythonVariableName, isASCIIAlphanumeric to a new code_identifiers.go
  • Move SanitizeToolID next to identifiers.go's identifier-normalization functions (same spirit as NormalizeSafeOutputIdentifier)

Estimated effort: 1-2 hours (pure move, no logic changes, same package so no import updates needed).

2. Minor: repeated PATType dispatch logic (Priority 3)

File: pkg/stringutil/pat_validation.go

ValidateCopilotPAT (lines 77-91) and GetPATTypeDescription (lines 93-107) both switch over the same four PATType cases (PATTypeFineGrained/PATTypeClassic/PATTypeOAuth/default) to produce a per-type string. The case structure is duplicated even though the strings differ (error text vs. plain description).

Recommendation: Extract a map[PATType]string (or a private helper returning both a description and a "supported" bool) that both functions consult, so the four-way classification lives in one place. Low impact — flagging for awareness rather than urgent action.

Estimated effort: <1 hour.

What's already well-organized

  • ansi.go, fuzzy_match.go, identifiers.go, urls.go, whitespace.go, and pkg/stats/statvar.go each have a single, clear responsibility with private helpers colocated correctly — no moves recommended.
  • No exact or near-duplicate (>80% similarity) implementations were found between separate files in this slice.
  • stringutil.go's three-function grab-bag (Truncate, FormatList, IsPositiveInteger) is an acceptable "misc utilities" file matching Go idiom for small, unrelated one-off helpers — not flagged.

Implementation Checklist

  • Split sanitize.go into sanitize.go / secret_redaction.go / code_identifiers.go, moving SanitizeToolID to sit with identifiers.go's normalization functions
  • Update any doc comments that cross-reference the moved functions' file location
  • (Optional, low priority) Extract shared PATType → description mapping used by both ValidateCopilotPAT and GetPATTypeDescription
  • Run go build ./pkg/stringutil/... and existing tests after the split (pure move within the same package, so no import changes expected)
Analysis Metadata
  • Total Go files analyzed: 10 (1 in pkg/stats, 9 in pkg/stringutil)
  • Total functions cataloged: ~40 (exported + private)
  • Function clusters identified: 9 (one per file) + 4 sub-clusters within sanitize.go
  • Outliers found: 0 (no function is in a clearly wrong file among files analyzed)
  • File-organization violations found: 1 (sanitize.go mixing 4 concerns)
  • Duplicates detected: 1 minor (repeated PATType switch dispatch)
  • Detection method: Direct file reads + Serena search_for_pattern (Go LSP/gopls unavailable in this sandbox — see tooling note above)
  • Analysis date: 2026-09-11

References:

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • api.anthropic.com

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

network:
  allowed:
    - defaults
    - "api.anthropic.com"

See Network Configuration for more information.

Generated by 🔧 Semantic Function Refactoring · claude · sonnet50 · 223.2 AIC · ⌖ 41.2 AIC · ⊞ 10.8K ·

  • expires on Sep 12, 2026, 6:44 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions