[typist] Typist: Go Type Consistency Analysis

[github-actions[bot]]

🔤 Typist - Go Type Consistency Analysis

Analysis of repository: github/gh-aw

Executive Summary

I scanned 1,531 non-test .go files under pkg/ (1,629 top-level type declarations) for duplicated type definitions and untyped (interface{}/any/bare-literal-constant) usage. The headline finding: this codebase already has unusually strong type discipline. There's a documented internal style guide (scratchpad/go-type-patterns.md) covering semantic type aliases, and a dedicated pkg/constants package already wraps job names, step IDs, artifact names, filenames, and engine names in purpose-built string types. interface{} has effectively been eliminated from production code — the only two hits in all of pkg/ are inside comments describing the concept, not actual type usage.

That said, I found 7 real duplication clusters worth consolidating and a handful of untyped constants that break the project's own semantic-typing convention. The most actionable one: pkg/cli/mcp_schema.go hand-maintains four to five "Wire" struct twins that mirror custom MarshalJSON output on the real domain structs — a documented but fragile pattern where adding a field to the real struct silently doesn't reach the JSON-schema generator unless someone remembers to update the twin by hand. There's also one byte-for-byte exact duplicate (domainAnalysisWireSchema / domainAnalysisWire) sitting in the same package, which is a pure one-line fix.

Full Analysis Report

Duplicated Type Definitions

Summary Statistics

• Total types analyzed: 1,629
• Duplicate clusters found: 7
• Exact duplicates: 1
• Near duplicates: 3
• Semantic duplicates: 3

Cluster 1: domainAnalysisWireSchema / domainAnalysisWire

Type: Exact duplicate
Occurrences: 2
Impact: High — identical struct defined twice in the same package

Locations:

1. pkg/cli/mcp_schema.go:275 — type domainAnalysisWireSchema struct { TotalRequests int; AllowedCount int; BlockedCount int; AllowedDomains []string; BlockedDomains []string }
2. pkg/cli/access_log.go:44 — type domainAnalysisWire struct { TotalRequests int; AllowedCount int; BlockedCount int; AllowedDomains []string; BlockedDomains []string }

Recommendation: Delete domainAnalysisWireSchema and have mcp_schema.go reference domainAnalysisWire directly for schema generation.
Estimated effort: <30 minutes.

---

Cluster 2: "Wire" shadow structs mirroring custom MarshalJSON output

Type: Near duplicate
Occurrences: 4 pairs
Impact: High — documented pattern, but each pair is hand-synced and will silently drift

pkg/cli/mcp_schema.go keeps unexported *Wire twins of several domain structs purely so the JSON-schema generator (which can't see custom MarshalJSON output) has something to reflect over:

Domain struct	Wire twin
ToolUsageSummary (pkg/cli/logs_report_tools.go:46)	toolUsageSummaryWire (pkg/cli/mcp_schema.go:239)
MCPServerHealthDetail (pkg/cli/audit_expanded.go:107)	mcpServerHealthDetailWire (pkg/cli/mcp_schema.go:247)
MCPServerCrossRunHealth (pkg/cli/audit_cross_run.go:86)	mcpServerCrossRunHealthWire (pkg/cli/mcp_schema.go:258)
MCPFailureSummary (pkg/cli/logs_models.go:243)	mcpFailureSummaryWire (pkg/cli/mcp_schema.go:268)

Recommendation: Instead of four independent hand-maintained twins, add a single helper (e.g. a SchemaShape() any method on each domain type, or a shared struct-tag-driven reflector) so the wire shape is derived from — and fails to compile when it diverges from — the domain type's actual MarshalJSON output.
Estimated effort: 3-4 hours.

---

Cluster 3: graderManifestEntry (writer vs. reader)

Type: Near duplicate
Occurrences: 2
Impact: Medium

Locations:

1. pkg/workflow/compiler_yaml_graders.go:84 — full writer-side struct (ID, Name, Description, Source, Enabled, Unit, Direction, Threshold, Max, Min, Digest, Run, Inline, Config)
2. pkg/cli/audit_report_graders.go:67 — reader-side subset (ID, Name, Unit, Direction, Threshold)

Both read/write the same grader_manifest.json file (see pkg/constants.GraderManifestFilename). A field added on the write side won't be visible to the cli reader unless someone manually mirrors it.

Recommendation: Move the full struct to a shared location (e.g. pkg/workflow already owns it — export it, or move to a small shared package) and have pkg/cli import it instead of redefining a subset.
Estimated effort: 1-2 hours.

---

Cluster 4: AuditOptions / auditCommandOptions / auditRunConfig

Type: Near duplicate
Occurrences: 3
Impact: Medium

Locations:

1. pkg/cli/audit.go:14 — AuditOptions (public, ~15 fields: owner/repo/hostname/outputDir/verbose/parse/jsonOutput/jobID/stepNumber/format/artifactSets/experimentFilter/...)
2. pkg/cli/audit.go:34 — auditRunConfig (internal, same concepts, lower-cased fields)
3. pkg/cli/audit_command.go:59 — auditCommandOptions (CLI-flag-facing, same concepts again)

Three structs carry the same audit-run configuration through three pipeline stages (raw flags → public options → internal run config) with >70% field overlap and only casing/naming differences.

Recommendation: Collapse to one struct passed through the pipeline, or make the later stages embed the earlier one and add only the fields that differ.
Estimated effort: 2-3 hours.

---

Cluster 5: FirewallLogEntry / AuditLogEntry

Type: Semantic duplicate
Occurrences: 2
Impact: Medium

Locations:

1. pkg/cli/firewall_log.go:118 — FirewallLogEntry (parsed from the squid-style text log: Timestamp, ClientIPPort, Domain, DestIPPort, Proto, Method, Status, Decision, URL, UserAgent)
2. pkg/cli/firewall_policy.go:50 — AuditLogEntry (parsed from the JSON audit log: Schema, Timestamp, Client, Host, Dest, Method, Status, Decision, URL)

Both represent the same conceptual entity — one proxied network request observed by the sandbox firewall — just parsed from two different log formats, with ~6 of ~9-10 fields overlapping (Timestamp, host/domain, Status, Method, Decision, URL).

Recommendation: Introduce a shared normalized ProxiedRequest type that both format-specific parsers populate, so downstream reporting code only needs to know one shape.
Estimated effort: 2-3 hours.

---

Cluster 6: Diagnostic-with-source-position structs

Type: Semantic duplicate
Occurrences: 3 (spanning 2 packages)
Impact: Medium

Locations:

1. pkg/console/console_types.go:4 — ErrorPosition{File, Line, Column} + CompilerError{Position, Type, Message, Context, Hint} (console-facing error/warning rendering)
2. pkg/parser/json_path_locator.go:23 — JSONPathLocation{Line, Column, Found} (schema-validation error locations)
3. pkg/scanfindings/scanfindings.go:107 — Finding{RuleID, Severity, Message, File, Line, Column, Context} / NewFindingParams{..., FileLines} (linter/scanner findings)

All three independently reinvent "a {File, Line, Column} position attached to a message with optional source-line context" — once for compiler errors, once for JSON-path schema errors, once for scanner findings. None of them are wrong individually, but a shared SourcePosition{Line, Column int} (optionally +File string) value type would let all three compose it instead of each redeclaring the same two int fields.
Estimated effort: 2-3 hours (mostly mechanical, touches 3 packages).

---

Cluster 7: Wire/Display one-struct-per-presentation-layer pattern (broader)

Type: Semantic duplicate
Occurrences: 2+ (same shape as Cluster 2, different layer)
Impact: Low

pkg/cli/logs_report.go:933 (mcpFailureSummaryDisplay) adds a third representation of the MCP-failure-summary shape (console/table-formatted) alongside the domain struct and its Cluster-2 Wire twin — confirming this "one struct per presentation layer" pattern is systemic across pkg/cli's audit/logs reporting code rather than an isolated case. Worth keeping in mind when addressing Cluster 2, since the same consolidation approach would likely cover this too.

---

Investigated and ruled out (intentional, already well-factored — not duplicates): types.BaseMCPServerConfig embedded by both pkg/parser.RegistryMCPServerConfig and pkg/workflow.MCPServerConfig; pkg/parser.ValidationError deliberately wrapping pkg/validationerror.Payload; pkg/cli.DependencyInfoWithIndirect embedding DependencyInfo.

---

Untyped Usages

Summary Statistics

• interface{} usages in production code: 0 (two apparent hits are both inside comments describing the concept, not real type declarations)
• any usages in production code: present, but every instance found is already documented as an intentional dynamic-YAML/JSON case per the project's own scratchpad/go-type-patterns.md
• Untyped constants flagged: 3 locations (4 constants)

This codebase is already unusually disciplined here. scratchpad/go-type-patterns.md codifies "always use any instead of interface{}", documents when map[string]any is appropriate (dynamic YAML/JSON frontmatter), and gives worked examples of converting []any to semantic typed slices (GitHubAllowedTools, GitHubToolsets). The pkg/constants package already wraps job names, step IDs, MCP server IDs, artifact names, filenames, versions, and engine names in dedicated string types with String()/IsValid() methods.

Category 1: any struct fields — reviewed, all justified

Location	Field	Why it's any
pkg/types/input_definition.go:18	InputDefinition.Default any	Workflow-dispatch input default can be string, number, or boolean per GitHub Actions schema — documented in the adjacent comment
pkg/workflow/service_ports.go:85-86	serviceContainerConfig.Ports any	GitHub Actions services: YAML allows the ports list to be strings, bare ints, or omitted — explained in the doc comment above the field
pkg/parser/import_observability.go:16-18	observabilityImportEndpoint.Headers any	OTLP endpoint headers can be a string or a map depending on import source format — explained in the doc comment
pkg/console/console_types.go:52-59	FormField.Value any	Generic interactive-prompt field; holds a pointer to whatever concrete type the specific form field needs

No changes recommended here — these match the "valid uses of any" criteria in the team's own style guide.

Category 2: Untyped numeric/string constants

Impact: Low-medium — mostly a missed opportunity for the semantic-type pattern the codebase already uses everywhere else, one real inconsistency

Examples:

// pkg/constants/job_constants.go:309-310 — no semantic type, unlike neighboring
// JobName/StepID/ArtifactName constants in the same file
const DefaultRateLimitMax = 5     // Default maximum runs per time window
const DefaultRateLimitWindow = 60 // Default time window in minutes (1 hour)

Used as plain int at the call site: pkg/workflow/role_checks.go:80 (max := constants.DefaultRateLimitMax) and :87 (window := constants.DefaultRateLimitWindow).

// pkg/constants/constants.go:738 — same pattern
const MaxSymlinkDepth = 5

Used as plain int at pkg/parser/remote_download_file.go:92.

Suggested fix (following the existing LineLength/Version pattern documented in scratchpad/go-type-patterns.md):

type RetryWindow int // minutes
type RateLimitCount int

const DefaultRateLimitMax RateLimitCount = 5
const DefaultRateLimitWindow RetryWindow = 60

One concrete inconsistency worth a quick fix: every other filename constant in pkg/constants/constants.go is typed Filename (AgentOutputFilename, SafeOutputsFilename, TokenUsageFilename, SarifFileName, ...) except:

// pkg/constants/constants.go:228 — missing the `Filename` type its siblings all have
const SafeOutputErrorsFilename = "safe-output-errors.json"

Recommendation: add Filename to match its siblings — a one-line fix.

---

What Should We Do About This?

Prioritized by effort-to-impact ratio:

1. Quick win (today): delete the exact-duplicate domainAnalysisWireSchema (Cluster 1) and type SafeOutputErrorsFilename Filename (Category 2). Both are single-line changes with zero behavioral risk.
2. Next: type DefaultRateLimitMax/DefaultRateLimitWindow/MaxSymlinkDepth with semantic types, following the existing LineLength/Version pattern already in pkg/constants.
3. When touching pkg/cli/mcp_schema.go next: tackle the Wire-struct consolidation (Clusters 2 and 7) together, since they share the same root cause (schema generator can't see MarshalJSON output).
4. Lower priority, opportunistic: graderManifestEntry dedup (Cluster 3), AuditOptions pipeline collapse (Cluster 4), FirewallLogEntry/AuditLogEntry unification (Cluster 5), and the shared SourcePosition type (Cluster 6) — all genuine but none urgent; good candidates for a future refactor PR rather than one-off fixes.

Implementation Checklist

• Remove domainAnalysisWireSchema, reuse domainAnalysisWire in mcp_schema.go
• Add Filename type to SafeOutputErrorsFilename
• Add semantic types for DefaultRateLimitMax, DefaultRateLimitWindow, MaxSymlinkDepth
• Consolidate the four Wire/domain struct pairs in pkg/cli/mcp_schema.go
• Share the graderManifestEntry struct between writer and reader
• Collapse the AuditOptions/auditCommandOptions/auditRunConfig trio
• Consider a shared SourcePosition type for console, parser, and scanfindings
• Run full test suite after each change

Analysis Metadata

• Total Go Files Analyzed: 1,531 (non-test, under pkg/)
• Total Type Definitions: 1,629
• Duplicate Clusters: 7 (1 exact, 3 near, 3 semantic)
• Untyped Usage Locations Flagged: 3 (4 constants); 0 real interface{} usages found
• Detection Method: ripgrep-based enumeration + field-level comparison, cross-checked by reading source (not name-matching alone)
• Analysis Date: 2026-10-12

References: §38243234272

Generated by ✍️ Typist - Go Type Analysis · claude · agent · 420.8 AIC · ⊞ 6.6K · ◷

• expires on Oct 13, 2026, 3:43 AM UTC-08:00

[github-actions[bot]]

This discussion was automatically closed because it expired on 2026-10-13T11:43:02.293Z.

Closed by Workflow