[typist] Typist Go Type Consistency Analysis - duplicate types and untyped usages in pkg

[github-actions[bot]]

🔤 Typist - Go Type Consistency Analysis

Analysis of repository: github/gh-aw

Executive Summary

I scanned roughly 1,375 non-test .go files under pkg/ (about 950-1,000 top-level type definitions, concentrated in pkg/workflow and pkg/cli) for duplicated types and weakly-typed (any) usage. The good news first: the codebase already practices a lot of good hygiene here — interface{} is essentially extinct in production code (everything uses any), and several type families (BaseMCPServerConfig, MCPServerStatsBase, the validationerror.Payload embed) show the team has already done real consolidation work. The remaining opportunities cluster tightly in one area: the pkg/cli audit/logs reporting subsystem, which has grown several near-identical "compare two runs" and "domain analysis" struct families independently instead of sharing one base type.

On the untyped-usage side, the vast majority of the ~2,800+ map[string]any/[]any occurrences are legitimate — they represent not-yet-validated YAML/JSON frontmatter, which is exactly what any is for. But I found about a dozen high-value spots where any (or a bare int) is standing in for a small, known set of concrete types, including one exported function (WaitForWorkflowCompletion) whose timeoutMinutes int parameter has no compiler-enforced unit, and five sibling Azure DevOps config structs that each declare their own Target any field with no validation logic anywhere behind them.

Full Analysis Report

Duplicated Type Definitions

Summary Statistics

• Total types analyzed: ~950-1,000 (non-test, under pkg/)
• Duplicate clusters found: 11
• Exact duplicates: 2
• Near duplicates: 5
• Semantic duplicates: 4

Cluster 1: AuditComparison*Delta / diff-entry family — Semantic + Exact

Type: Exact duplicate (the two Delta types) nested inside a broader semantic duplicate (two independent "diff two runs" frameworks)
Impact: High — both live in pkg/cli, both power the audit command's compare/diff output

Locations:

1. pkg/cli/audit_comparison.go:45 — AuditComparisonIntDelta{Before int; After int; Changed bool}
2. pkg/cli/audit_comparison.go:51 — AuditComparisonStringDelta{Before string; After string; Changed bool} (identical to rejig docs #1 except field type)
3. pkg/cli/audit_diff.go:24 — DiffEntryBase{Status; IsAnomaly; AnomalyNote}, embedded by DomainDiffEntry:31
4. pkg/cli/audit_diff.go:246,302 — MCPToolDiffEntry, ToolCallDiffEntry use a differently-shaped Run1X/Run2X pattern for the same "compare a metric across two runs" concept

Definition Comparison:

type AuditComparisonIntDelta struct {
    Before  int  `json:"before"`
    After   int  `json:"after"`
    Changed bool `json:"changed"`
}

type AuditComparisonStringDelta struct {
    Before  string `json:"before"`
    After   string `json:"after"`
    Changed bool   `json:"changed"`
}

Recommendation:

• Replace the two Delta types with one generic: type Delta[T comparable] struct { Before, After T; Changed bool }
• Reconcile DiffEntryBase-style entries with the Run1X/Run2X-style entries onto one shared shape
• Estimated effort: 3-4 hours
• Benefits: Removes ~10 near-identical struct definitions from the audit/diff subsystem

---

Cluster 2: RedactedDomainsAnalysis / RedactedDomainsLogSummary — Exact

Impact: Medium — inconsistent with the codebase's own established pattern for this exact scenario

Locations:

1. pkg/cli/redacted_domains.go:21 — RedactedDomainsAnalysis{TotalDomains int; Domains []string}
2. pkg/cli/redacted_domains.go:29 — RedactedDomainsLogSummary{TotalDomains int; Domains []string; ByWorkflow map[string]*RedactedDomainsAnalysis}

Definition Comparison:

type RedactedDomainsAnalysis struct {
    TotalDomains int      `json:"total_domains" console:"header:Total Domains"`
    Domains      []string `json:"domains" console:"title:Redacted Domains,omitempty"`
}

type RedactedDomainsLogSummary struct {
    TotalDomains int                                  `json:"total_domains" console:"header:Total Domains"`
    Domains      []string                             `json:"domains" console:"title:Redacted Domains,omitempty"`
    ByWorkflow   map[string]*RedactedDomainsAnalysis  `json:"by_workflow,omitempty" console:"-"`
}

The summary type repeats the analysis type's two fields verbatim instead of embedding it — notably, the sibling pattern AccessLogSummary/FirewallSummaryBase (pkg/cli/logs_report_firewall.go:11-26) already does this correctly via embedding.

Recommendation:

• Extract DomainCountBase{TotalDomains int; Domains []string} and embed it in both types, matching the firewall summary pattern
• Estimated effort: 1 hour
• Benefits: Single source of truth; matches an existing, proven pattern elsewhere in the same package

---

Cluster 3: Domain/firewall "analysis" family — Semantic

Impact: Medium — 3 files model the same "aggregate domain traffic from a log" concept differently

Locations:

1. pkg/cli/domain_buckets.go:41 — AnalysisBase (embeds DomainBuckets)
2. pkg/cli/access_log.go:35 — DomainAnalysis (embeds AnalysisBase)
3. pkg/cli/firewall_log.go:133 — FirewallAnalysis (embeds AnalysisBase)
4. pkg/cli/redacted_domains.go:21 — RedactedDomainsAnalysis (bespoke, non-embedding shape — see Cluster 2)

Recommendation: Bring RedactedDomainsAnalysis onto the same AnalysisBase embed as its two siblings once Cluster 2 is fixed.

---

Cluster 4: Rate-limit quota types — Near

Impact: Medium — two authors must hand-sync the same GitHub REST quota fields

Locations:

1. pkg/cli/logs_rate_limit.go:113 — GitHubAPIRateLimitState{Limit int; Remaining int; Reset int64; Used int}
2. pkg/cli/logs_github_rate_limit_usage.go:33 — GitHubRateLimitEntry{..., Limit int; Remaining int; Used int; Reset string, ...}

Recommendation:

• Extract RateLimitQuota{Limit, Remaining, Used int} and embed it in both; keep Reset (differently typed: int64 snapshot vs. string log field) on each specific type
• Estimated effort: 1-2 hours

---

Cluster 5: safeOutputTargetConfig vs SafeOutputTargetConfig — Naming collision

Type: Not a true field duplicate, but a same-package naming hazard
Impact: High risk, low volume — case-only difference invites autocomplete/typo bugs

Locations:

1. pkg/workflow/safe_outputs_validation.go:79 — unexported safeOutputTargetConfig{name string; target string}
2. pkg/workflow/safe_outputs_parser.go:9 — exported SafeOutputTargetConfig{Target string; TargetRepoSlug string; AllowedRepos []string}

Recommendation: Rename the unexported validation-only struct (e.g. targetValidationEntry) to eliminate the case-only collision within pkg/workflow.
Estimated effort: 30 minutes

---

Cluster 6: ValidationError family — Semantic (mostly already unified)

Locations:

1. pkg/validationerror/validationerror.go:25,49 — shared Payload + ValidationError interface
2. pkg/parser/validation_error.go:12 — ValidationError{validationerror.Payload}
3. pkg/workflow/workflow_errors.go:41 — WorkflowValidationError{validationerror.Payload; Severity; Category; Timestamp; File; Line; Column}

Already consolidated via the shared Payload embed. Flagged only because workflow independently adds File/Line/Column location fields that other validation-error consumers might also want promoted into the shared package. Not an action item on its own — worth a quick confirm, not a rewrite.

---

Cluster 7: Safe-output TitlePrefix field — Near, low impact

Locations: CreateIssuesConfig (pkg/workflow/create_issue.go:15), CreateDiscussionsConfig (pkg/workflow/create_discussion.go:17), CreateProjectsConfig (pkg/workflow/create_project.go:11) each redeclare TitlePrefix string instead of embedding a shared field.

Recommendation: Add a TitlePrefixConfig{TitlePrefix string} embed to match the existing BaseSafeOutputConfig/CloseOlderConfig pattern already used by these same structs. Estimated effort: 30 minutes.

---

Cluster 8: Azure DevOps work-item config family — Near, single file

Locations: pkg/workflow/safe_outputs_azure_devops.go — CreateWorkItemConfig:19, UpdateWorkItemConfig:32, CommentOnWorkItemConfig:50, AssignWorkItemConfig:55, LinkWorkItemsConfig:62, UploadWorkItemAttachmentConfig:68 each embed BaseSafeOutputConfig and independently redeclare Target any (lines 40, 52, 57, 64, 70).

Recommendation: Shared TargetableWorkItemConfig{Target any} embed (see also the untyped-usage finding on this same field below). Estimated effort: 1 hour.

---

Already-consolidated patterns worth pointing to as precedent

• pkg/types/mcp.go:6 BaseMCPServerConfig, embedded by pkg/parser/mcp.go:47 and pkg/workflow/tools_types.go:502 — a deliberate, well-commented consolidation.
• pkg/cli/logs_models.go:211 MCPServerStatsBase, embedded by MCPServerStats, MCPServerHealthDetail, and MCPServerCrossRunHealth — the struct fields are unified, though each embedder still hand-rolls its own MarshalJSON/UnmarshalJSON to preserve legacy wire keys (audit_expanded.go:101-150, audit_cross_run.go:95-140); that boilerplate could become one shared helper.
• WorkflowRun (pkg/cli/logs_models.go:68), GitHubWorkflow (pkg/cli/workflows.go:64), JSONWorkflow (pkg/cli/jsonworkflow_to_markdown.go:23), WorkflowFile (pkg/workflow/workflow_file.go:8) — four different representations of "a workflow" at different layers (run instance / API definition / export format / compiled artifact). Field overlap is minimal; this reads as legitimate layering, not copy-paste — informational only.

---

Untyped Usages

Summary Statistics

• interface{} usages in production code: ~0 (only appears in linter testdata fixtures, out of scope)
• any in function params: ~180
• any in return types: ~15-20
• any in struct fields: ~45
• map[string]any / []any: 2,800+ (the large majority are legitimate — see exclusions below)
• Untyped semantic constants worth flagging: ~10-15 of ~250 total const declarations

Category 1: Exported function with an unenforced time unit

Impact: High — public API, unit encoded only in the parameter name

• Location: pkg/cli/pr_automerge.go:110
• Current signature:

  func WaitForWorkflowCompletion(ctx context.Context, repoSlug, runID string, timeoutMinutes int, verbose bool) error

• Actual usage: called once, at pkg/cli/run_workflow_execution.go:468, with workflowCompletionWaitTimeoutMinutes — itself defined as const workflowCompletionWaitTimeoutMinutes = 6 * 60 (run_workflow_execution.go:25), where the * 60 is a giveaway this is really a duration in disguise
• Suggested fix:

  func WaitForWorkflowCompletion(ctx context.Context, repoSlug, runID string, timeout time.Duration, verbose bool) error
  // call site: WaitForWorkflowCompletion(ctx, targetRepo, runIDStr, 6*time.Hour, opts.Verbose)

• Benefits: A caller passing seconds instead of minutes currently fails silently; time.Duration makes the unit part of the type system

---

Category 2: any struct fields with no validation behind them

Impact: High — polymorphism that's structurally allowed but never actually parsed

• Location: pkg/workflow/safe_outputs_azure_devops.go:40,52,57,64,70

  Target any `yaml:"target,omitempty"`

  repeated across CreateWorkItemConfig, UpdateWorkItemConfig, CommentOnWorkItemConfig, AssignWorkItemConfig, LinkWorkItemsConfig.
• Context: Every other safe-output config's equivalent field is a plain string (add_comment.go:18, push_to_pull_request_branch.go:16, safe_outputs_parser.go:10, 10+ sites total). The JSON schema genuinely allows integer | "*" | []integer | string for Azure DevOps targets, so any isn't arbitrary — but the only code that touches these fields (addAzureDevOpsTarget / appendAzureDevOpsTargetConstraint, both in the same file) passes the value straight through into a map[string]any or a %v format string with no type switch or validation anywhere.
• Suggested fix: a small tagged type, e.g. type WorkItemTarget struct { ID *int; IDs []int; Wildcard bool; Path string } with a custom UnmarshalYAML, replacing all 5 fields — turns an undocumented, unvalidated shape into one validated type used everywhere.
• Estimated effort: 2-3 hours (touches 5 struct fields + 2 helper functions in one file)

---

Category 3: Untyped duration constants

Impact: Medium — the unit is only documented in a comment, not enforced

• pkg/constants/job_constants.go:309-310:

  const DefaultRateLimitMax = 5     // Default maximum runs per time window
  const DefaultRateLimitWindow = 60 // Default time window in minutes (1 hour)

  DefaultRateLimitWindow should be time.Duration (e.g. time.Hour); DefaultRateLimitMax is a plain count and is fine as-is.
• pkg/workflow/publish_code_coverage.go:18:

  const defaultCodeCoverageWaitForProcessingTimeout = 160

  paired with WaitForProcessingTimeout int (yaml tag wait-for-processing-timeout,omitempty, commented // seconds) at line 40 — same int-seconds-by-convention pattern.
• Suggested fix: follow the pattern already used elsewhere in pkg/constants/constants.go (e.g. DefaultToolTimeout = 60 * time.Second) — 160 * time.Second, field retyped to time.Duration.
• Estimated effort: 2-3 hours across both files (includes updating consumers)

---

Category 4: any standing in for a small closed set of concrete types

Impact: Medium

• pkg/console/console_types.go:50 — FormField.Value any (commented "pointer to the value to store the result"). The adjacent Type string discriminator ("input"/"password"/"confirm"/"select") implies callers always pass one of a small set of pointer kinds. Suggested fix: a generic FormField[T any] per field kind, or at minimum document/enforce the Type→pointer-kind contract with a switch.
• pkg/workflow/safe_output_handlers.go:13-19 — registry table where every entry's NewConfig func() any returns a different concrete *XConfig, immediately type-asserted back at the single call site. Suggested fix: a SafeOutputConfig marker interface (e.g. requiring Validate() error) instead of bare any, so the registry is self-documenting.
• pkg/cli/gateway_logs_types.go:179,190 — ID any (json tag id) in two separate JSON-RPC structs. Protocol-mandated polymorphism (string/number/null) makes any defensible here, but the same pattern is duplicated across two structs; a shared RPCID wrapper type would avoid two independent sets of ad-hoc type assertions downstream.

---

Category 5: Numeric limit stored as a string constant

• Location: pkg/constants/constants.go:417

  const DefaultMaxDailyAICredits = "5000"

  Interpolated into a GitHub Actions expression string at compilerenv/manager.go:238. Defensible, but storing it as const DefaultMaxDailyAICredits = 5000 (int) and calling strconv.Itoa at the interpolation site keeps the "this is a number" invariant machine-checked from the point of definition.

---

What was excluded as legitimate any usage (and why)

• map[string]any / []any for YAML/JSON frontmatter parsing — the large majority of all hits, concentrated in pkg/parser/* and pkg/workflow/frontmatter_*. This is pre-validation, arbitrary user-authored YAML; any here correctly represents "not yet validated," and the values are checked against a JSON Schema before becoming typed structs.
• pkg/typeutil/convert.go (ParseIntValue, ConvertToInt, ConvertToFloat) — genuinely heterogeneous numeric coercion across 30+ call sites where the source's underlying Go type varies by decoder (int vs float64 vs string); a well-documented, textbook-legitimate generic utility.
• pkg/console/render.go:31 RenderStruct(v any) — reflection-based generic pretty-printer, correctly generic by design.
• JSON-RPC/REST client wrappers (Get/Post/Patch(path string, response any) error in pkg/parser/remote_resolve_sha.go, pkg/cli/update_check.go, pkg/cli/bootstrap_profile_actions_repo.go) and thin encoding/json wrappers (pkg/cli/preconditions.go:159, pkg/cli/trial_helpers.go:361, pkg/cli/setup_repository.go:441) — standard Go idiom for unmarshal/marshal targets; a generics rewrite is a stylistic option, not a safety fix.
• Raw any fields tagged yaml:"-" (pkg/workflow/repo_memory.go:68, pkg/workflow/tools_types.go:482,488,495) — intentionally preserve the original decoded value for round-tripping/debugging, excluded from serialization by design.
• pkg/workflow/frontmatter_types.go (Engine, RunsOn, imports, checkout, evals, graders — 6 fields) — genuinely polymorphic per YAML author ergonomics (string-or-array-or-object, matching GitHub Actions' own runs-on/on schemas). A full sum-type rewrite here would be large and risky for marginal safety gain — lowest priority, informational only.

---

Refactoring Recommendations

Priority 1: pkg/cli audit/diff/logs reporting subsystem (Clusters 1-4)

Recommendation: Consolidate the diff-delta types, fix the RedactedDomains* embedding inconsistency, and unify the rate-limit quota fields.

Steps:

1. Introduce Delta[T comparable] and migrate AuditComparisonIntDelta/AuditComparisonStringDelta onto it
2. Extract DomainCountBase and embed it in RedactedDomainsAnalysis/RedactedDomainsLogSummary, matching the existing AccessLogSummary/FirewallSummaryBase pattern
3. Extract RateLimitQuota and embed it in GitHubAPIRateLimitState/GitHubRateLimitEntry
4. Run the pkg/cli test suite to verify no JSON-tag/serialization breakage

Estimated effort: 6-8 hours combined
Impact: High — removes the bulk of the codebase's real type duplication in one pass

---

Priority 2: Exported timeout API + Azure DevOps Target fields

Recommendation: Retype WaitForWorkflowCompletion's timeout parameter to time.Duration, and introduce a validated WorkItemTarget type for the 5 Azure DevOps config structs.

Steps:

1. Change WaitForWorkflowCompletion signature and its one call site
2. Define WorkItemTarget with UnmarshalYAML, replace the 5 Target any fields, add validation in addAzureDevOpsTarget
3. Update tests and schema docs if needed

Estimated effort: 4-6 hours
Impact: High — closes an actual unit-safety gap in an exported function, and turns an unvalidated any into a real contract

---

Priority 3: Duration constants and small cleanups

Recommendation: Retype DefaultRateLimitWindow and defaultCodeCoverageWaitForProcessingTimeout to time.Duration; rename safeOutputTargetConfig to remove the case-collision with SafeOutputTargetConfig; add the TitlePrefixConfig embed.

Estimated effort: 3-4 hours combined
Impact: Medium — clarity and readability wins, low risk

---

Implementation Checklist

• Introduce generic Delta[T] and migrate the two AuditComparison*Delta types
• Fix RedactedDomainsAnalysis/RedactedDomainsLogSummary to embed a shared base
• Extract RateLimitQuota base for the two rate-limit types
• Rename safeOutputTargetConfig to remove the naming collision with SafeOutputTargetConfig
• Retype WaitForWorkflowCompletion's timeoutMinutes int to time.Duration
• Introduce WorkItemTarget and replace the 5 Target any fields in safe_outputs_azure_devops.go
• Retype DefaultRateLimitWindow and the code-coverage timeout constant to time.Duration
• Add shared TitlePrefixConfig embed to the three safe-output config structs
• Run the full test suite after each change group

Analysis Metadata

• Total Go Files Analyzed: ~1,375 non-test files under pkg/
• Total Type Definitions Reviewed: ~950-1,000
• Duplicate Clusters: 11 (2 exact, 5 near, 4 semantic)
• Untyped Usage Locations Flagged: ~12 high-value findings out of ~180 any-in-params, ~45 any-in-fields, and 2,800+ map[string]any/[]any occurrences (the vast majority of the latter are legitimate pre-validation frontmatter parsing)
• Detection Method: Ripgrep-based structural scan + manual verification of top candidates
• Analysis Date: 2026-09-22

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 ✍️ Typist - Go Type Analysis · claude · agent · 426.5 AIC · ⌖ 9.07 AIC · ⊞ 10.2K · ◷

• expires on Sep 23, 2026, 3:40 AM UTC-08:00

[github-actions[bot]]

This discussion has been marked as outdated by Typist - Go Type Analysis.

A newer discussion is available at Discussion #62931.