[typist] Typist: Go Type Consistency Analysis - Duplicate Types and Untyped Usages

[github-actions[bot]]

🔤 Typist - Go Type Consistency Analysis

Analysis of repository: github/gh-aw

Executive Summary

I swept every non-test .go file under pkg/ — 1,232 top-level struct/interface declarations across 532 files — looking for duplicated types and weakly-typed interface{}/any usage. The good news first: the four biggest "this looks duplicated" candidates I found (a family of security-scanner Finding structs, a cluster of MCP server/tool stats types, a cluster of domain/firewall log-analysis types, and the ~35 per-action *Config structs in pkg/workflow) turned out, on closer reading, to be already well-factored — shared base structs (scanfindings.Finding, MCPServerStatsBase, AnalysisBase, BaseSafeOutputConfig) are consistently embedded where it matters, and the pieces that look similar but are not embedded are usually adapting a genuinely different external wire format (each security tool has its own JSON schema).

That said, I did find two concrete, actionable issues: the ProgressBar/SpinnerWrapper wasm/native build-tag twins in pkg/console have quietly drifted apart (one side has a constructor/method the other lacks), and PolicyAnalysis in pkg/cli/firewall_policy.go duplicates fields that AnalysisBase already provides instead of embedding it. On the typing side, the most valuable fix is in pkg/cli/bootstrap_profile_manifest.go, where a fully-typed struct is populated via a dozen manual map[string]any field lookups instead of a single decode — and there is a real untyped-string-enum (ExperimentsStorage) and an untyped-hours-constant that are cheap, high-value fixes. Full details below.

Full Analysis Report

Duplicated Type Definitions

Summary Statistics

• Total type declarations swept: 1,232 (across 532 files matched by the grep sweep)
• Production types in scope (excluding _test.go and pkg/linters/*/testdata fixtures): ~1,050
• Duplicate/near-duplicate clusters investigated in depth: 4, plus 1 exact-name pair
• Clusters confirmed as real, already-fixed-correctly patterns (false positives): 3 of 4
• Concrete follow-up items found: 3 (API drift, one missing embed, one reflection-based setter)

Cluster A: "Finding" pattern (security/lint tool wrappers) — false positive, no action needed

Type: Apparent duplication, but verified as a well-factored adapter pattern
Occurrences: 7 (scanfindings.Finding, zizmorFinding, poutineFinding, grypeFinding, runnerGuardFinding, grantPackageFinding, AuditFinding)

Locations:

1. pkg/scanfindings/scanfindings.go:107 — Finding (the shared, tool-independent output type: RuleID, Severity SeverityLevel, Message, File, Line, Column, Context)
2. pkg/cli/zizmor.go:25 — zizmorFinding → converted via zizmorFindingsToShared (zizmor.go:282)
3. pkg/cli/poutine.go:25 — poutineFinding → converted via poutineFindingsToShared (poutine.go:475)
4. pkg/cli/grype.go:54 — grypeFinding → converted via grypeFindingsToShared (grype.go:432)
5. pkg/cli/runner_guard.go:23 — runnerGuardFinding → converted via runnerGuardFindingsToShared (runner_guard.go:311)
6. pkg/cli/grant.go:51 — grantPackageFinding → converted via grantFindingsToShared (grant.go:343)
7. pkg/cli/audit_report.go:67 — AuditFinding — a higher-level "audit insight" that reuses only the SeverityLevel enum, structurally unrelated to raw scanner findings

Assessment: Each tool-specific struct is a raw-JSON parse target mirroring the wire format of that external tool (zizmor, poutine, grype, etc. each have their own schema), not a copy of scanfindings.Finding. Every one of them is explicitly converted into the shared Finding type before use. This is exactly the adapter pattern you would want — consolidating further would mean fighting the actual API shape of each external tool for no benefit.

Recommendation: No action needed.

---

Cluster B: MCP server/tool stats pattern — mostly already consolidated

Type: Near-duplicate, partially consolidated
Occurrences: 13 types across pkg/cli/audit_report.go, logs_models.go, gateway_logs_types.go, audit_expanded.go, audit_cross_run.go

What is already fixed: MCPServerStatsBase (pkg/cli/logs_models.go:191: ServerName, ToolCallCount, ErrorCount) is embedded by:

• MCPServerStats (audit_report.go:213)
• MCPServerHealthDetail (audit_expanded.go:90, with a MarshalJSON override for legacy field names)
• MCPServerCrossRunHealth (audit_cross_run.go:82, same pattern)

A comment on MCPServerStatsBase explicitly documents that it replaced four different spellings of the same concept — this consolidation already happened.

Remaining candidate (low priority): GatewayServerMetrics / GatewayToolMetrics (pkg/cli/gateway_logs_types.go:85,97) duplicate the ServerName/ToolCallCount/ErrorCount shape in spirit, but they are untagged, in-memory-only structs used for live gateway-log aggregation (getOrCreateServer/getOrCreateTool) — a different code path than the JSON/console-tagged audit-report family. Worth a future embed of MCPServerStatsBase, but not urgent since the two paths never interact today.

Recommendation: No action needed on the audit-report family (already correct). Optional low-priority follow-up: embed MCPServerStatsBase in GatewayServerMetrics.

---

Cluster C: domain/firewall/access-log analysis pattern — mostly already consolidated, one real gap

Type: Near-duplicate, partially consolidated
Occurrences: ~14 types across pkg/cli/access_log.go, firewall_policy.go, firewall_log.go, gateway_logs_types.go, redacted_domains.go, domain_buckets.go

What is already fixed: AnalysisBase (pkg/cli/domain_buckets.go:41, embedding DomainBuckets + TotalRequests/AllowedRequests/BlockedRequests) is explicitly documented as "the shared base embedded by DomainAnalysis and FirewallAnalysis," and both:

• DomainAnalysis (access_log.go:35) — embeds AnalysisBase (plus legacy-wire-format MarshalJSON/UnmarshalJSON)
• FirewallAnalysis (firewall_log.go:133) — embeds AnalysisBase + RequestsByDomain map[string]DomainRequestStats

...do embed it correctly.

Real gap found: PolicyAnalysis (pkg/cli/firewall_policy.go:79) has TotalRequests, AllowedCount, DeniedCount fields that duplicate the TotalRequests/AllowedRequests/BlockedRequests semantics of AnalysisBase instead of embedding it. It is not a clean drop-in (it also has RuleHits []RuleHitStats and UniqueDomains, no domain-bucket lists), but it is the one spot in this cluster where the existing consolidation pattern was not applied.

False positives correctly ruled out: AccessLogEntry vs FirewallLogEntry vs AuditLogEntry vs GatewayLogEntry are raw parsed-line formats from four genuinely different log sources (squid access log, squid firewall log, AWF audit JSONL, MCP gateway JSONL) — similar-sounding names, different schemas, no consolidation opportunity.

Recommendation: Low-priority follow-up — consider whether PolicyAnalysis can embed AnalysisBase for its request-count fields.

---

Cluster D: safe-outputs per-action *Config structs — false positive, no action needed

Type: Apparent duplication (~35 structs), verified as consistent composition
Occurrences: AddLabelsConfig, CreateIssuesConfig, UpdateIssuesConfig, AssignToUserConfig, MergePullRequestConfig, ReplaceLabelConfig, and ~30 similar structs in pkg/workflow

Assessment: All checked structs embed BaseSafeOutputConfig (pkg/workflow/safe_outputs_config_types.go:22) — directly, or via the shared UpdateEntityConfig indirection used uniformly by the entire "update-*" family — plus a small, consistent set of secondary mixins (SafeOutputTargetConfig, SafeOutputFilterConfig, SafeOutputAllowBlockConfig, SafeOutputAllowedLabelsConfig). This is the "composable mixins" pattern working as intended.

Recommendation: No action needed — this is worth calling out as a positive example, not a problem.

---

Exact-name duplicate: ProgressBar and SpinnerWrapper — intentional split, but API has drifted

Type: Exact duplicate (build-tag platform twins)
Occurrences: 2 pairs

Locations:

1. pkg/console/progress.go:31 ((go/redacted):build !js && !wasm) vs pkg/console/progress_wasm.go:9 ((go/redacted):build js || wasm)
2. pkg/console/spinner.go:92 ((go/redacted):build !js && !wasm) vs pkg/console/spinner_wasm.go:12 ((go/redacted):build js || wasm)

Drift found:

• progress_wasm.go has a NewIndeterminateProgressBar constructor (line 20) with no native equivalent — on native builds, indeterminate is hard-set to false in NewProgressBar and there is no setter, so the native renderIndeterminate path is currently dead code.
• spinner_wasm.go has an IsEnabled() bool method (line 28) with no native equivalent on SpinnerWrapper in spinner.go.
• A repo-wide check confirms nothing outside these two files calls either one today, so this is currently harmless — but the two "platform twins" no longer expose the same public API surface.

Recommendation: Either add NewIndeterminateProgressBar/IsEnabled to the native builds for symmetry, or remove them from the wasm-only files as unused surface, so both build variants expose the same API again. Estimated effort: <1 hour.

---

Untyped Usages

Summary Statistics

• High-value interface{}/any sites reviewed: ~15 (function params, map values, reflection-based dispatch)
• Untyped constants flagged: 2 exported (ExperimentsStorage*, DefaultActionFailureIssueExpiresHours) + 1 internal ratio constant
• Legitimate/intentional any usage excluded: JSON marshal helpers, generic logging, arbitrary-shape YAML/JSON frontmatter parsing (this last one is consistent across pkg/workflow, pkg/cli, and pkg/parser — genuinely external, unknown-shape input, not a smell)

Category 1: map[string]any decoding a fixed, known schema (highest value)

Location: pkg/cli/bootstrap_profile_manifest.go:82, helper stringValue at pkg/cli/add_package_manifest_remote.go:69

func parseManifestBootstrapAction(actionType string, actionMap map[string]any, manifestPath string, index int) (repositoryPackageBootstrapAction, error) {
    // ...12+ manual field lookups like:
    // owner, _ := stringValue(actionMap["owner"])

actionMap is not arbitrary external data — it is the fixed "bootstrap manifest action" schema, and it is already being decoded into a fully-typed struct, repositoryPackageBootstrapAction (string/bool/[]string/map[string]string fields). stringValue (a one-line .(string) assertion helper) is called 20+ times across this file and add_package_manifest_*.go.

Suggested fix: decode actionMap directly into repositoryPackageBootstrapAction via a single mapstructure-style decode instead of 12+ manual assertions — eliminates the stringValue boilerplate at this call site and turns silent zero-value-on-typo into a real decode error.

Category 2: Untyped string-enum constant

Location: pkg/workflow/compiler_experiments.go:25,29; backing field pkg/workflow/workflow_data.go:195

const ExperimentsStorageCache = "cache"
const ExperimentsStorageRepo = "repo"
// ExperimentsStorage string // "cache" or "repo" (default "repo")

Used at compiler_experiments.go:559,814 and compiler_jobs.go:633 via plain string comparison. Nothing stops any code from assigning an arbitrary string to ExperimentsStorage.

Suggested fix:

type ExperimentStorageMode string
const (
    ExperimentsStorageCache ExperimentStorageMode = "cache"
    ExperimentsStorageRepo  ExperimentStorageMode = "repo"
)

and change WorkflowData.ExperimentsStorage to ExperimentStorageMode.

Category 3: Untyped numeric constant for a semantic duration

Location: pkg/workflow/repo_config.go:63

const DefaultActionFailureIssueExpiresHours = 24 * 7

Paired with MaintenanceConfig.ActionFailureIssueExpires int (doc comment: "in hours") — a bare int on both sides gives no protection against a future caller passing minutes or milliseconds by mistake.

Suggested fix: introduce type Hours int (or switch to time.Duration and store 24 * 7 * time.Hour, converting once at the point of use).

Category 4: any immediately type-asserted (should just be the concrete type)

Location: pkg/workflow/step_shell_validator.go:118

func checkStepGHToken(step any, workflowHasGHToken bool) string {
    stepMap, ok := step.(map[string]any)

The caller (step_shell_validator.go:85) already has a []any that came from an earlier rawValue.([]any) assertion, and sibling helpers workflowEnvHasGHToken / stepEnvHasGHToken in the same file already take map[string]any directly — this one function is the odd one out.

Suggested fix: change the signature to checkStepGHToken(step map[string]any, workflowHasGHToken bool) string and have the calling loop skip non-map elements before calling, matching its siblings.

Category 5: Reflection-based generic field setter

Location: pkg/workflow/safe_output_handlers.go:673, used from pkg/workflow/safe_outputs_permissions.go:220

func setSafeOutputField(config *SafeOutputsConfig, fieldName string, value any) bool {
    field, ok := safeOutputPointerFieldValue(config, fieldName)
    // ... reflect.ValueOf(value) / AssignableTo / field.Set(newValue)

The registry entry NewConfig func() any plus a bare StructField string means a mismatch between the registered field name and the actual fields of SafeOutputsConfig is only caught at runtime via reflection, not by the compiler.

Suggested fix: replace the (string fieldName, func() any) pair with a generic Register[T any](field **T, ctor func() T), or explicit per-handler setter functions, so the compiler catches field/type mismatches.

Category 6: Duplicated "string-or-slice" coercion via any (~10 near-identical implementations)

Representative locations:

• pkg/cli/runner_guard_activation_gate.go:221 — jobNeeds(needs any) []string
• pkg/workflow/frontmatter_trigger_helpers.go:37 — normalizeStringOrStringSlice(raw any) []string
• pkg/workflow/role_checks.go:192 — parseOptionalStringSliceField(value any, fieldName string) []string
• pkg/cli/outcome_eval_update.go:200 — mutableStringSlice(raw any) []string

About 10 separate implementations across pkg/workflow and pkg/cli reimplement the same string | []any | []string → []string coercion, each with slightly different trim/sort/dedup behavior.

Suggested fix: consolidate into one shared helper (there is already a precedent — pkg/typeutil.ParseIntValue — for this kind of thing), e.g. typeutil.NormalizeStringSlice(v any) []string.

Category 7: Other lower-priority any round-trips

• pkg/cli/mcp_logs_guardrail.go:21 — const CharsPerToken = 4, an untyped byte/token ratio used in raw division at pkg/cli/logs_episode.go:307. Low priority — consider type ByteCount/type TokenCount wrappers if this arithmetic grows.
• pkg/workflow/daily_aic_workflow.go:26 — extractMaxDailyAICObjectValue(raw any) any round-trips through any on both sides; could resolve directly to a concrete type or return (value any, hadObjectForm bool).
• pkg/workflow/notify_comment.go:305 — parseGroupConcurrencyQueueFeatureValue(value any) bool, the sole consumer of a map[string]any documented as "bool or string only" — a small FeatureValue type with typed accessors would remove the any entirely.
• pkg/workflow/engine_config_parser.go:124,151,203 — three functions doing ~30 combined manual map[string]any → struct field assertions for AuthDefinition/EngineAuthConfig/RequestShape, all pre-existing typed structs. Same fix shape as Category 1: decode once instead of asserting per field.
• pkg/cli/mcp_error.go:11,17 — newMCPError/mcpErrorData taking any for a JSON-RPC error payload. This is a legitimate marshal boundary; flagged only because most call sites pass a plain string, so a newMCPErrorf(code int64, msg, dataMsg string) error convenience overload could cover the common case.

---

Refactoring Recommendations

Priority 1: Fix the ProgressBar / SpinnerWrapper build-tag API drift

Steps: Decide whether NewIndeterminateProgressBar and IsEnabled() are meant to be used; either add them to the native (!js && !wasm) builds for symmetry, or delete them from the wasm-only files as unused surface.
Estimated effort: <1 hour. Impact: Low risk today (nothing calls them), but prevents a future silent platform-specific behavior gap.

Priority 2: Add semantic types to ExperimentsStorage and hour-based durations

Steps: Introduce type ExperimentStorageMode string (pkg/workflow/compiler_experiments.go) and type Hours int or time.Duration (pkg/workflow/repo_config.go); update the two struct fields and their comparison sites.
Estimated effort: 1-2 hours. Impact: Medium — compile-time protection against invalid storage-mode strings and unit confusion in maintenance-window config.

Priority 3: Replace manual map[string]any field-by-field decodes with struct decodes

Steps: In pkg/cli/bootstrap_profile_manifest.go (Category 1) and pkg/workflow/engine_config_parser.go (Category 7), replace the repeated actionMap["key"].(string) / authObj["key"].(string) assertion chains with a single decode into the already-typed target struct.
Estimated effort: 3-4 hours combined. Impact: High — removes ~50 manual assertions total and turns silent typos into real errors.

Priority 4: Consolidate the ~10 "string-or-slice" any coercion helpers

Steps: Add typeutil.NormalizeStringSlice(v any) []string and migrate the call sites listed in Category 6.
Estimated effort: 2-3 hours. Impact: Medium — removes duplicated logic, not just duplicated types.

Priority 5 (optional, low urgency): PolicyAnalysis and GatewayServerMetrics/GatewayToolMetrics

Steps: Consider embedding AnalysisBase in PolicyAnalysis and MCPServerStatsBase in the gateway metrics structs, matching the pattern already used elsewhere.
Estimated effort: 1-2 hours. Impact: Low — cosmetic consistency, not a bug.

Implementation Checklist

• Resolve ProgressBar/SpinnerWrapper native-vs-wasm API drift (add or remove the mismatched methods)
• Add ExperimentStorageMode type for ExperimentsStorage
• Add an Hours/time.Duration-based type for DefaultActionFailureIssueExpiresHours
• Replace manual map-to-struct field assertions in bootstrap_profile_manifest.go and engine_config_parser.go with a single decode
• Consolidate the ~10 string-or-slice any coercion helpers into one typeutil function
• (Optional) Embed AnalysisBase in PolicyAnalysis; embed MCPServerStatsBase in GatewayServerMetrics/GatewayToolMetrics

Analysis Metadata

• Total Go Files Analyzed: 532 files matched by the type-declaration sweep under pkg/ (non-test files were the focus of all cluster verification)
• Total Type Definitions Swept: 1,232
• Duplicate Clusters Investigated: 4 (3 false positives / already-consolidated, 1 with a real gap) + 1 exact-name build-tag pair
• Untyped Usage Locations Reported: ~15
• Detection Method: serena-assisted semantic reading + targeted subagent verification of struct field bodies and call sites
• Analysis Date: 2026-08-26

References:

• Workflow run §32963490489

Generated by ✍️ Typist - Go Type Analysis · claude · agent · 403.1 AIC · ⌖ 11.5 AIC · ⊞ 9.5K · ◷

• expires on Aug 27, 2026, 3:51 AM UTC-08:00