[typist] Typist: Go Type Consistency Analysis (duplicated types & untyped usages) #49330
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-08-01T12:13:41.381Z.
|
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
🔤 Typist - Go Type Consistency Analysis
Analysis of repository: github/gh-aw
Executive Summary
I scanned every non-test
.gofile underpkg/(1,061 files, ~974 top-level struct/interface definitions) looking for duplicated type definitions and untyped (any/interface{}) usage that could be tightened up. The good news: theinterface{}→anymodernization is essentially done (only 2 leftover mentions, both in comments) and there are very few exact same-name struct duplicates outside of intentional per-linter test fixtures and wasm/native build-tag pairs.The more interesting findings are structural: several option/config/error structs across
pkg/cliandpkg/workflowindependently re-invent the same field clusters (a "repo target" ofOwner/Repo/Hostname, a "name+error" failure record, a DIFC content-descriptor block repeated three times ingateway_logs_types.go). And on the untyped side, there's a real, high-value pattern inpkg/workflow/mcp_github_config.goandpkg/parser/mcp.go: dozens of functions manually type-assert fixed, well-known keys out ofmap[string]any, even though matching strongly-typed destinations (GitHubToolConfig,types.BaseMCPServerConfig) already exist in the codebase and just aren't being used at those call sites. Wiring those up would delete a lot of.(string)/.(map[string]any)assertion noise for free.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
Cluster 1:
updateFailurevsactionUpdateFailureType: Exact duplicate (same shape, same file)
Locations:
pkg/cli/update_types.go:11—type updateFailure struct { Name string; Error string }pkg/cli/update_types.go:17—type actionUpdateFailure struct { name string; err string }Recommendation: unify into one exported
updateFailure{Name, Error string}reused by both call sites — they're declared 6 lines apart in the same file.Estimated effort: <1 hour
Cluster 2:
CopilotWorkflowStepvsworkflow.WorkflowStepType: Exact/near duplicate (subset relationship)
Locations:
pkg/cli/copilot_setup.go:208—CopilotWorkflowStep{Name, Uses, Run string; With, Env map[string]any}pkg/workflow/step_types.go:18—WorkflowStep{Name, ID, If, Uses, Run, WorkingDirectory, Shell string; With map[string]any; Env map[string]string; ContinueOnError any; TimeoutMinutes int}CopilotWorkflowStep's 5 fields are a strict subset ofWorkflowStep, reusing identical names/semantics.pkg/cli/copilot_setup.goalready importspkg/workflow(usesworkflow.ActionMode), so there's no import-cycle reason for the parallel type.Recommendation: reuse
workflow.WorkflowStepdirectly instead of a local minimal copy.Estimated effort: 1-2 hours
Cluster 3: Repeated "repo target" fields (
Owner/Repo/Hostname)Type: Near duplicate, 4+ occurrences
Locations:
pkg/cli/audit.go:25-40—AuditOptions{Owner, Repo, Hostname, OutputDir string; ...}pkg/cli/view_command.go:109-115—ViewOptions{Owner, Repo, Hostname, OutputDir string; Verbose bool}(field-for-field prefix ofAuditOptions)pkg/parser/github_urls.go:32-33—GitHubURLComponents{Host, Owner, Repo string, ...}pkg/parser/import_remote.go:20-21—remoteImportOrigin(owner/repo, doc-commented)ViewOptionsis literally a field-for-field prefix ofAuditOptions.pkg/repoutil/already exists as the natural package for repo-spec utilities but doesn't export a canonicalOwnerRepo-style value type these could converge on.Recommendation: extract a shared
RepoTarget{Owner, Repo, Hostname string}(candidate home:pkg/repoutil) and embed it inAuditOptions/ViewOptions; consider it for the parser-side URL/import types too.Estimated effort: 2-3 hours
Cluster 4:
LogsDownloadOptionsvsStdinLogsOptionsType: Near duplicate (17 of ~20 fields identical)
Locations:
pkg/cli/logs_orchestrator_types.go:8-36and:39-62Both share
OutputDir, Engine, RepoOverride, Verbose, ToolGraph, NoStaged, FirewallOnly, NoFirewall, Parse, JSONOutput, SummaryFile, SafeOutputType, FilteredIntegrity, EvalsOnly, Train, Format, ArtifactSets, ReportFileverbatim; only download-specific (WorkflowName/Count/date range/run-ID fields) vs stdin-specific (RunURLs/Timeout) fields differ.Recommendation: extract
LogsCommonOptionsand embed in both.Estimated effort: 2-3 hours
Cluster 5: Parallel error struct family with duplicated
Error()logicType: Near duplicate
Locations:
pkg/workflow/workflow_errors.go—WorkflowValidationError(38-50),OperationError(98-105),ConfigurationError(154-160)All three share a
{Reason/Value, Suggestion string, Timestamp time.Time}shape and near-identicalError()string-building logic (timestamp prefix, optional truncated value, reason line, suggestion line). The file already definesFieldLocation = console.ErrorPositionas an alias for{File, Line, Column}, butWorkflowValidationErrorre-declaresFile/Line/Columninline instead of embedding it — an inconsistency within the very file that defines the alias.Recommendation: shared
baseWorkflowError{Suggestion string; Timestamp time.Time}embed + commonformatError(...)helper; embedFieldLocationinWorkflowValidationErrorinstead of restating its fields.Estimated effort: 3-4 hours
Cluster 6:
permissionEntry/repoEntrycache entriesType: Near duplicate (same file, same shape)
Locations:
pkg/cli/mcp_server_cache.go:22-25and:27-30— both{value/permission/repository string; timestamp time.Time}Recommendation: since the repo already uses Go generics elsewhere, a
cacheEntry[T any]{value T; timestamp time.Time}removes this duplication cleanly.Estimated effort: 1 hour
Cluster 7: DIFC content-descriptor fields repeated 3x
Type: Semantic duplicate
Locations:
pkg/cli/gateway_logs_types.go—GatewayLogEntry(18-41),DifcFilteredEvent(45-57),RPCMessageEntry(127-143)All three independently repeat a byte-for-byte identical 9-field block (
ToolName, Description, Reason, SecrecyTags []string, IntegrityTags []string, AuthorAssociation, AuthorLogin, HTMLURL, Number stringwith matching JSON tags), because each type also carries different envelope fields (timestamp/level vs RPC direction).Recommendation: extract
DifcContentFieldsand embed in all three.Estimated effort: 2 hours
Cluster 8:
TokenCoreMetricsembedding inconsistencyType: Semantic/documentation inconsistency
Locations:
pkg/cli/token_usage_types.goTokenCoreMetrics{InputTokens, OutputTokens, CacheReadTokens, CacheWriteTokens, ReasoningTokens, EffectiveTokens int}(12-19) is documented as "the single source of truth for the token-usage quartet" and correctly embedded inTokenUsageEntry/ModelTokenUsage. ButModelTokenUsageRow(76-86) re-declares the same four fields by hand instead of embedding it.Recommendation: embed
TokenCoreMetricsinModelTokenUsageRow, or update the doc comment to acknowledge the exception.Estimated effort: <1 hour
Cluster 9 (informational): wasm/native build-tag pairs
RepositoryFeatures(pkg/workflow/repository_features_validation.go:73/repository_features_validation_wasm.go:31) is an exact verbatim duplicate (HasDiscussions bool; HasIssues bool) gated by//go:buildtags — this one is pure duplication with no behavioral reason to differ, so it's the easiest win here: move the struct into a shared, tag-free file both variants can import.SpinnerWrapper(pkg/console/spinner.go:92/spinner_wasm.go:10) andProgressBar(pkg/console/progress.go:31/progress_wasm.go:7) share names but have genuinely different fields (real vs no-op TTY implementations) — same-name, not same-shape, so no consolidation needed there.Untyped Usages
Summary Statistics
interface{}usages (non-test, non-comment): 0 — migration toanyis completeanykeyword occurrences: ~691 files touchanymap[string]any/map[string]interface{}occurrences: ~2,364 (expected — this is a YAML/JSON frontmatter compiler, most of this is legitimately dynamic)Category 1:
interface{}cleanupOnly 2 real hits outside test fixtures (15 more are intentional linter-testdata fixtures that must keep
interface{}as the input corpus for lint rules), and both are comment text, not live declarations:pkg/workflow/behavior_defined_engine.go:595— comment says "interface{}", should say "any"pkg/workflow/safe_outputs_config_types.go:109— struct-tag comment says[]interface{}; the field itself (ReportFailureAsIssue any) is already modernizedImpact: cosmetic only. No functional risk.
Category 2:
map[string]anymasquerading as fixed structsExample 1: GitHub tool-config extraction cluster (highest value in this report)
pkg/workflow/mcp_github_config.go—hasGitHubApp(86),isGitHubCLIModeEnabled(96),getGitHubType(137),getGitHubToken(160),getGitHubLockdown/hasGitHubLockdownExplicitlySet(177),getGitHubToolsets(194),getGitHubAllowedTools(255),getGitHubDockerImageVersion(599),getGitHubFeatures(630)githubTool map[string]anyand pull one fixed key via type assertion:GitHubToolConfig(pkg/workflow/tools_types.go:306, fieldsMode,Type,GitHubToken,Lockdown,Toolset,Allowed) and a parserparseGitHubTool(val any) *GitHubToolConfig(pkg/workflow/tools_parser.go:190) — these accessor functions just need to callparseGitHubToolonce and read struct fields instead of re-deriving from the raw map each time.Example 2: MCP stdio/http config parsing —
pkg/parser/mcp.goparseMCPStdioTypeConfig(520),configureMCPStdioContainer(535),configureMCPStdioCommand(551),appendContainerEnv(579),appendContainerMounts(593),parseMCPHTTPTypeConfig(641)mcpConfig map[string]anypullingcontainer/entrypoint/command/args/env/mounts/network/urlone key at a time with repeated.(string)/.(map[string]any)assertions.types.BaseMCPServerConfig(pkg/types/mcp.go:6-26) hasCommand,Args,Env map[string]string,URL,Headers map[string]string,Container,Entrypoint,EntrypointArgs,Mounts []string— a field-for-field match, already embedded byRegistryMCPServerConfig.mcpConfigstraight intoBaseMCPServerConfiginstead of ~15 hand-written extractor functions.Example 3:
getGitHubGuardPolicies—mcp_github_config.go:315-355Builds a fixed
{"allow-only": {"repos", "min-integrity", "tool-call-limits", "blocked-users", "trusted-users", "approval-labels"}}shape every time. Candidate:GuardAllowOnlyPolicy{Repos any; MinIntegrity string; ToolCallLimits map[string]int; BlockedUsers, TrustedUsers, ApprovalLabels string}.Example 4: write-sink policy —
mcp_github_config.go:454-596deriveSafeOutputsGuardPolicyFromGitHub/deriveWriteSinkGuardPolicyFromWorkflowbuild then immediately re-assert (policy["write-sink"].(map[string]any)at line 572) the same{"accept": [...], "sink-visibility": ...}shape. Candidate:WriteSinkPolicy{Accept []string; SinkVisibility string}.Example 5:
MCPRegistryServerForProcessing.Config—pkg/cli/mcp_registry.go:28Config map[string]anyis populated at lines 188-226 as exactly one of two shapes ({"env": ...}for stdio,{"url", "headers": ...}for remote), never anything else. Candidate:MCPRuntimeConfig{Env map[string]string; URL string; Headers map[string]string}.Example 6: trigger event maps —
pkg/workflow/tools.go:229-353buildCommandTriggerEventsMap/buildLabelCommandEventsMapalways build{"types": [...]}then re-assert.([]any)at lines 251, 335-339. Candidate:TriggerEventConfig{Types []string}.Example 7:
ensureWorkflowDispatchItemNumberInput—pkg/workflow/tools.go:356-398Builds the identical GH Actions
workflow_dispatch.inputs.item_numberschema (description/required/default/typekeys) inline twice. Candidate:WorkflowDispatchInput{Description, Required, Default, Type string}.Example 8 (architectural note, not a point fix): reverse
ToMap()conversions —pkg/workflow/frontmatter_serialization.go:41-65, 249-327FrontmatterConfig.ToMap(),runtimeConfigToMap(),permissionsConfigToMap()take already-typed structs and flatten them back tomap[string]any"for backward compatibility," per their own doc comments. This signals the typed-struct migration is mid-flight rather than a bug to fix directly — worth tracking as its own follow-up rather than bundling into the above.Lower priority:
pkg/workflow/tools.go:502-631(applyDefaultTools) legitimately needstools map[string]any(arbitrary user-defined MCP server names as keys) — but the known"bash"key specifically gets a long assertion chain (lines 597, 612, 621, 626) that a smallBashToolConfigaccessor could clean up without fighting the genuinely dynamic outer map.Category 3: Untyped constants
Network port constants —
pkg/constants/constants.go:100-140Inconsistent with the file's own precedent two lines above, where
MaxExpressionLineLength LineLength = 120uses a dedicated named type.MinNetworkPort/MaxNetworkPortimply range validation — exactly the case for atype Port intwith aValid()method to prevent accidental mixing with unrelated ints.Log level —
pkg/constants/constants.go:235(AWFDefaultLogLevel = "info")One member of a real enum validated elsewhere against a bare
[]string{"debug", "info", "warn", "error"}literal (pkg/workflow/firewall_validation.go:54). The codebase already has the idiomatic pattern next door —GitHubMCPMode(pkg/workflow/tools_types.go) is a propertype GitHubMCPMode stringwith named constants.LogLevelshould get the same treatment, withFirewallConfig.LogLevel(currently plainstring,pkg/workflow/firewall.go:19) updated to match.StatusActive/StatusInactive—pkg/cli/mcp_registry_types.go:107-110Untyped string constants compared at
pkg/cli/mcp_registry.go:135. Smaller blast radius than log level (one comparison site) but same fix:type MCPServerStatus string.ArgumentTypePositional/ArgumentTypeNamed—pkg/cli/mcp_registry_types.go:113-116Same shape, minor.
DefaultMaxRuns/DefaultMaxTurnCacheMisses—pkg/constants/constants.go:336-339Plain untyped int constants for domain concepts (run-count limit, cache-miss threshold) sitting next to properly
time.Duration-typed timeout constants in the same file. Low urgency, but flagged for consistency with the file's otherwise disciplined typing.Refactoring Recommendations
Priority 1 — High-value untyped-map cleanup (do this first)
Recommendation: Route
pkg/workflow/mcp_github_config.go's ~10 GitHub-tool accessor functions through the already-existingparseGitHubTool/GitHubToolConfig, and decodepkg/parser/mcp.go's MCP stdio/http config parsing straight into the already-existingtypes.BaseMCPServerConfig.Why first: unlike most findings here, the destination types are not proposals — they already exist in the codebase and are unused at these specific call sites. This is the lowest-risk, highest-payoff item.
Estimated effort: 4-6 hours
Impact: High — removes ~15-20 duplicated type-assertion sites for config shapes that are already fully modeled elsewhere
Priority 2 — Consolidate the
RepositoryFeaturesexact duplicateRecommendation: move
RepositoryFeatures{HasDiscussions, HasIssues bool}out of the two build-tag-gated files into a shared, untagged file bothrepository_features_validation.goandrepository_features_validation_wasm.gocan import.Estimated effort: <1 hour
Impact: Medium — this is the one true byte-for-byte duplicate in the whole scan; trivial to fix
Priority 3 — Extract shared option/error base types
Recommendation: work through clusters 1-8 above (
RepoTarget,LogsCommonOptions,baseWorkflowError,cacheEntry[T],DifcContentFields,TokenCoreMetricsembedding) roughly in the order listed — each is small and independent, so they can land as separate small PRs.Estimated effort: 12-18 hours total across all clusters
Impact: Medium — reduces field-drift risk (e.g.
ViewOptionssilently diverging fromAuditOptionsover time) more than it fixes an active bugPriority 4 — Named types for enum-like constants
Recommendation: introduce
type LogLevel string(mirroring the existingGitHubMCPModepattern),type Port int,type MCPServerStatus stringand update the handful of comparison/validation sites to match.Estimated effort: 3-5 hours
Impact: Low-Medium — mostly clarity and future-proofing rather than an active defect
Implementation Checklist
mcp_github_config.goaccessors throughparseGitHubTool/GitHubToolConfigpkg/parser/mcp.gostdio/http configs intotypes.BaseMCPServerConfigRepositoryFeaturesto a shared untagged fileRepoTarget,LogsCommonOptions,baseWorkflowError,cacheEntry[T],DifcContentFieldsTokenCoreMetricsinModelTokenUsageRowLogLevel,Port,MCPServerStatusnamed typesAnalysis Metadata
All reactions