[typist] 🔤 Typist: Go Type Consistency Analysis #67180

Duplicated Type Definitions

Summary Statistics

• Total types analyzed: ~1,436 (approximate; top-level struct/interface declarations in non-test .go files under pkg/)
• Duplicate clusters found: 5 actionable + 1 false positive (intentional WASM build-tag pattern)
• Exact duplicates: 1 (false positive — see Cluster 6)
• Near duplicates: 2
• Semantic duplicates: 3

Cluster 1: RunOptions vs RunWorkflowOptions

Type: Near duplicate
Occurrences: 2
Impact: High — same package, 80% field overlap

Locations:

1. pkg/cli/run_workflow_execution.go:31 — type RunOptions struct {...}
2. pkg/cli/run_interactive.go:366 — type RunWorkflowOptions struct {...}

Definition Comparison:

type RunOptions struct {
    Enable            bool
    EngineOverride    string
    RepoOverride      string
    RefOverride       string
    AutoMergePRs      bool
    Push              bool
    WaitForCompletion bool
    RepeatCount       int
    Inputs            []string
    Verbose           bool
    DryRun            bool
    JSON              bool
    Approve           bool
}

type RunWorkflowOptions struct {
    WorkflowName       string
    Verbose            bool
    EngineOverride     string
    RepoOverride       string
    RefOverride        string
    AutoMergePRs       bool
    Push               bool
    DryRun             bool
    Approve            bool
    requiredInputsOnly bool
}

8 of RunWorkflowOptions' 10 fields (Verbose, EngineOverride, RepoOverride, RefOverride, AutoMergePRs, Push, DryRun, Approve) are identical name+type matches with RunOptions.

Recommendation:

• Extract a shared WorkflowRunFlags struct holding the 8 common fields and embed it in both RunOptions and RunWorkflowOptions.
• Estimated effort: 1-2 hours
• Benefits: One source of truth for the shared run flags; adding a new shared flag (e.g. a future --timeout) only requires touching one struct instead of two.

Cluster 2: MCP/Gateway server & tool call metrics

Type: Near duplicate
Occurrences: 5 related types
Impact: High — the codebase already solved this once and left one case unconsolidated

Locations:

1. pkg/cli/gateway_logs_types.go:85 — type GatewayServerMetrics struct {...}
2. pkg/cli/gateway_logs_types.go:97 — type GatewayToolMetrics struct {...}
3. pkg/cli/logs_models.go:218 — type MCPServerStatsBase struct {...} (the shared base others already embed)
4. pkg/cli/audit_report.go:254 — type MCPServerStats struct { MCPServerStatsBase; ... }
5. pkg/cli/audit_expanded.go:94 — type MCPServerHealthDetail struct { MCPServerStatsBase; ... }

Definition Comparison:

// logs_models.go:218 — the existing comment documents the exact problem this cluster re-introduces:
// "Those types previously spelled the same concepts four different ways
//  (TotalCalls/ToolCalls/ToolCallCount and TotalErrors/ErrorCount)"
type MCPServerStatsBase struct {
    ServerName    string `json:"server_name" console:"header:Server"`
    ToolCallCount int    `json:"tool_call_count" console:"header:Tool Calls"`
    ErrorCount    int    `json:"error_count,omitempty" console:"header:Errors,omitempty"`
}

// gateway_logs_types.go:85 — re-spells the same ServerName/ToolCallCount/ErrorCount concept
// instead of embedding MCPServerStatsBase:
type GatewayServerMetrics struct {
    ServerName         string
    RequestCount       int
    ToolCallCount      int
    TotalDuration      float64
    ErrorCount         int
    FilteredCount      int
    GuardPolicyBlocked int
    Tools              map[string]*GatewayToolMetrics
}

Recommendation:

• MCPServerStats and MCPServerHealthDetail already embed MCPServerStatsBase to avoid exactly this drift — GatewayServerMetrics (live gateway-log parsing path) is the one straggler left outside that hierarchy.
• Embed MCPServerStatsBase in GatewayServerMetrics (keeping its gateway-specific fields like TotalDuration/FilteredCount/GuardPolicyBlocked alongside it).
• Estimated effort: 2-3 hours (plus updating any field-rename call sites)
• Benefits: Finishes the consolidation the team already started; one name per concept across the whole MCP metrics surface.

Cluster 3: WorkQueue Assignment/AssignmentClaim vs CLI "receipt" types

Type: Semantic duplicate
Occurrences: 4
Impact: Medium

Locations:

1. pkg/workqueue/types.go:218 — type Assignment struct {DispatchID, RequestID, CommitID, PolicyEpoch, Pool, WorkerProfile string; Claims []AssignmentClaim}
2. pkg/workqueue/types.go:229 — type AssignmentClaim struct {Handle, ClaimID, WorkID, Work, ResultRefs}
3. pkg/cli/logs_work_queue_current.go:30 — type WorkQueueAssignmentReceipt struct {DispatchID, RequestID, CommitID, State string; Released bool; Claims []WorkQueueClaimReceipt}
4. pkg/cli/logs_work_queue_current.go:39 — type WorkQueueClaimReceipt struct {Handle, ClaimID, WorkID, State, Barrier}

pkg/cli's "receipt" types hand-duplicate ~60-70% of the field names from pkg/workqueue's core Assignment/AssignmentClaim types instead of projecting from them.

Recommendation:

• Add a ToReceipt() method on workqueue.Assignment/AssignmentClaim, or factor out a shared identity sub-struct (DispatchID/RequestID/CommitID/Handle/ClaimID/WorkID), instead of re-declaring the field names by hand in pkg/cli.
• Estimated effort: 2-3 hours
• Benefits: Changes to the core work-queue identity fields propagate automatically to the CLI reporting view instead of needing a second hand-edit.

Cluster 4: Domain allow/block list duplication outside DomainBuckets

Type: Semantic duplicate
Occurrences: 3
Impact: Medium

Locations:

1. pkg/cli/domain_buckets.go:13 — type DomainBuckets struct {AllowedDomains []string; BlockedDomains []string} (designed to be embedded)
2. pkg/cli/domains_command.go:27 — type WorkflowDomainsDetail struct {Workflow, Engine string; AllowedDomains, BlockedDomains []string}
3. pkg/cli/access_log.go:44 — type domainAnalysisWire struct {...; AllowedDomains, BlockedDomains []string}

Recommendation:

• DomainBuckets already exists for exactly this purpose and is embedded elsewhere via AnalysisBase. Embed it in WorkflowDomainsDetail too.
• The domainAnalysisWire case is a JSON-compat shim that must preserve specific legacy keys — that one is a reasonable, already-understood exception and lower priority.
• Estimated effort: 30-60 minutes
• Benefits: One definition of "what an allow/block domain pair looks like" across CLI reporting types.

Cluster 5: Per-tool "Finding" wrapper structs (scanner integrations)

Type: Semantic duplicate
Occurrences: 5
Impact: Low-Medium — largely justified, worth a lighter-touch fix

Locations:

1. pkg/scanfindings/scanfindings.go:107 — type Finding struct {RuleID, Severity, Message, File, Line, Column, Context} (the shared normalized target)
2. pkg/cli/zizmor.go:27 — type zizmorFinding struct {...}
3. pkg/cli/poutine.go:26 — type poutineFinding struct {...}
4. pkg/cli/grype.go:54 — type grypeFinding struct {...}
5. pkg/cli/runner_guard.go:23 — type runnerGuardFinding struct {...}

Each *Finding struct mirrors a distinct third-party tool's own JSON output schema (zizmor/poutine/grype/runner-guard), so merging them into one type isn't appropriate — but all four get hand-converted into scanfindings.Finding with near-identical RuleID/Severity/File/Line mapping code.

Recommendation:

• Keep the tool-specific wire structs, but introduce a small shared conversion helper (or a rawScanFinding interface) so the four conversions to scanfindings.Finding stop being copy-pasted independently.
• Note: AuditFinding (pkg/cli/audit_report.go:79) and SecurityFinding (pkg/workflow/markdown_security_scanner.go:63) are further semantic cousins of the same "severity + message + location" shape worth folding in if this is tackled.
• Estimated effort: 2-3 hours
• Benefits: New scanners only need to write a mapping function, not a parallel finding struct plus conversion.

Cluster 6: ProgressBar / SpinnerWrapper (native vs. WASM) — not actionable

Type: Exact duplicate (name only)
Occurrences: 4
Impact: None — false positive

Locations: pkg/console/progress.go:31, pkg/console/progress_wasm.go:9, pkg/console/spinner.go:92, pkg/console/spinner_wasm.go:12

These are same-named types split across Go build tags (native vs. GOOS=js WASM stub) — the standard, intentional pattern already documented in this repo's developer-code-organization skill. No change needed.

Untyped Usages

Summary Statistics

• interface{} usages in production pkg/ code: 0 (9 raw matches, all inside pkg/linters/*/testdata fixture files that intentionally demonstrate the anti-pattern for the custom linters' own golden tests)
• any usages: ~2,900 across 565 files — the large majority are legitimate (Go generics constraints, and dynamic YAML/JSON frontmatter fields the authors have explicitly commented as "too dynamic to type")
• Untyped constants: ~596 raw const X = <literal> declarations (mostly local, test-scoped constants); a handful of production, semantically-significant numeric constants stood out
• Total flagged untyped locations: ~15 concrete call-outs across 3 categories below

Category 1: any fields that are actually always one concrete type

Impact: High — these aren't "genuinely dynamic," they're just inconsistent with identically-named fields elsewhere in the same codebase

Example 1: Target any in Azure DevOps safe-output configs

• Location: pkg/workflow/safe_outputs_azure_devops.go:40,52,57,64,70 (UpdateWorkItemConfig, CommentOnWorkItemConfig, AssignWorkItemConfig, LinkWorkItemsConfig, UploadWorkItemAttachmentConfig)
• Current: Target any with tag yaml:"target,omitempty", checked only via target != nil and consumed with fmt.Sprintf("%v", target)
• Actual usage: Every other Target field in the same package is a plain string — e.g. CreateCheckRunConfig.Target, UpdateDiscussions.Target, assign_milestone.go's config.Target — and safe_outputs_azure_devops_test.go:74 asserts config.UpdateWorkItems.Target equals the string "42".
• Suggested fix:

  Target string `yaml:"target,omitempty"`

  with the existing != nil guards changed to != "".
• Benefits: Matches the convention already used for every other Target field in pkg/workflow; removes a pointless any→string round trip through %v.

Example 2: Headers any vs. Headers map[string]string

• Locations: pkg/workflow/frontmatter_types.go:252,289, pkg/parser/import_observability.go:18
• Current: Headers any with tag json:"headers,omitempty"
• Actual usage: pkg/workflow/tools_parser.go:717 builds config.Headers = make(map[string]string) for the equivalent MCP-server headers concept, and tools_types.go/observability_otlp.go consume headers as either map[string]string or string elsewhere in the same package — never as arbitrary nested data.
• Suggested fix: Headers map[string]string (matching pkg/workflow/tools_types.go's existing header config).
• Benefits: One shape for "HTTP headers" across the whole config surface; callers stop needing a type assertion before use.

Category 2: Same untyped shape copy-pasted across structs (should be a named type)

Impact: Medium — not wrong, but duplicated map[string]any/[]any literals hide that these are the same concept

Example: GuardPolicies map[string]any

• Locations: pkg/workflow/mcp_renderer_types.go:21,94,131, pkg/workflow/mcp_config_types.go:54, pkg/workflow/tools_types.go:530
• Current: five separate struct fields each independently declared as map[string]any
• Suggested fix:

  type GuardPolicyMap map[string]any

  and use GuardPolicies GuardPolicyMap everywhere instead of re-typing the map literal.
• Benefits: A single named type documents intent once; if this ever grows real structure, there's one declaration to change instead of five.

Example: []any for GitHub Actions step passthrough

• Locations: pkg/workflow/frontmatter_types.go:413-416 (PreSteps, Steps, PreAgentSteps, PostSteps), pkg/workflow/threat_detection_config.go:12-13, pkg/workflow/safe_jobs.go:25, pkg/workflow/safe_outputs_config_types.go:137
• Note: these genuinely need to stay dynamic — they're raw passthrough of arbitrary GitHub Actions step YAML (uses:/run:/with:/etc.), confirmed by compiler_main_job_helpers.go's extractRunScriptsFromSectionYAML(data.PreSteps, ...), which re-serializes them back to YAML rather than inspecting fields.
• Suggested fix: not a structural retyping, just a shared alias for clarity: type RawActionSteps []any, used consistently instead of eight independent []any fields.
• Benefits: Makes the "this is intentionally untyped YAML passthrough" decision visible at the type level instead of only in a comment on one of the eight fields.

Category 3: Untyped numeric/semantic constants

Impact: Low-Medium — mostly clear from context already, but a few lack units at a glance

Examples:

// pkg/workqueue/codec.go:26-27 — byte sizes with no unit in the name
const maxParseBytes = 80 << 20
const maxLineBytes = 8 << 20

// pkg/workqueue/branch.go:22 — retry count
const maxRetries = 5

// pkg/workqueue/types.go:10 — count
const MaxClaimsPerDispatch = 16

// pkg/linters/largefunc/largefunc.go:19 — line count
const DefaultMaxLines = 60

// pkg/linters/excessivefuncparams/excessivefuncparams.go:19 — param count
const DefaultMaxParams = 8

// pkg/validationerror/validationerror.go:21 — string length
const MaxValueLength = 100

Most of these already have self-documenting names (MaxParseBytes, DefaultMaxLines), so the risk is low — but introducing small semantic types would remove any residual ambiguity and is a very cheap, mechanical change if the team wants it:

type ByteSize int64
const maxParseBytes ByteSize = 80 << 20
const maxLineBytes  ByteSize = 8 << 20

Recommendation: Low priority relative to Categories 1-2. Only worth doing opportunistically (e.g. next time one of these files is touched for another reason), not as a dedicated pass.

🎯 What Should We Do About This?

Here's a suggested action plan, prioritized by impact and how cheap each fix is:

Priority 1: Quick, high-confidence fixes

1. Target any → Target string in pkg/workflow/safe_outputs_azure_devops.go (5 fields, 1 file, tests already prove it's always a string)
2. Headers any → Headers map[string]string in pkg/workflow/frontmatter_types.go and pkg/parser/import_observability.go
3. Extract shared WorkflowRunFlags struct for RunOptions/RunWorkflowOptions in pkg/cli

Estimated effort: ~3-4 hours total
Impact: High — removes real type-assertion risk and finishes work the codebase already started elsewhere

Priority 2: Finish an existing consolidation

4. Embed MCPServerStatsBase in GatewayServerMetrics (pkg/cli/gateway_logs_types.go) to close the gap the team's own comment already flags

Estimated effort: 2-3 hours
Impact: High — architectural consistency across the MCP metrics surface

Priority 3: Medium-value cleanups

5. Add ToReceipt() on workqueue.Assignment/AssignmentClaim instead of hand-duplicating fields in pkg/cli's work-queue receipt types
6. Embed DomainBuckets in WorkflowDomainsDetail
7. Introduce GuardPolicyMap and RawActionSteps named type aliases for the repeated map[string]any/[]any shapes

Estimated effort: ~5-6 hours total
Impact: Medium — clarity and drift-prevention, not correctness

Priority 4: Opportunistic only

8. Semantic types for byte-size/count constants (ByteSize, etc.) — only worth doing when already touching those files for another reason.

Implementation Checklist

• Fix Target any → string in Azure DevOps safe-output configs
• Fix Headers any → map[string]string in frontmatter/observability types
• Extract shared WorkflowRunFlags struct for RunOptions/RunWorkflowOptions
• Embed MCPServerStatsBase in GatewayServerMetrics
• Add ToReceipt() projection on workqueue Assignment/AssignmentClaim
• Embed DomainBuckets in WorkflowDomainsDetail
• Introduce GuardPolicyMap / RawActionSteps named type aliases
• Run full test suite after each change
• Leave the WASM-stub ProgressBar/SpinnerWrapper duplication alone (intentional)

Analysis Metadata

• Total Go Files Analyzed: ~1,493 non-test .go files under pkg/
• Total Type Definitions: ~1,436 (approximate)
• Duplicate Clusters: 5 actionable, 1 false positive
• Untyped Usage Locations Flagged: ~15 across 3 categories
• Detection Method: Serena semantic analysis + targeted grep/ripgrep pattern matching, with manual verification of flagged usage sites and surrounding call sites
• Analysis Date: 2026-10-09