[typist] Typist: Go type consistency findings in pkg/

[github-actions[bot]]

🔤 Typist — Go Type Consistency Analysis

Analysis of repository: github/gh-aw · scope: non-test .go files under pkg/ (~1,374 files) · run §34963240581

Executive Summary

I scanned roughly 1,230 top-level struct/interface definitions and ~2,900 weakly-typed usages (interface{}/any in signatures, struct fields, and maps, plus untyped constants) across pkg/. The good news: most of the "duplication" I found is either intentional (platform build-tag variants, a well-factored BaseMCPServerConfig embedding pattern) or low-risk. The most actionable finding is a family of hand-copied "wire" schema structs in pkg/cli/mcp_schema.go that duplicate the shape of MarshalJSON/UnmarshalJSON anonymous structs living in four other files — including one byte-for-byte exact duplicate (domainAnalysisWire / domainAnalysisWireSchema). These will silently drift the next time someone edits a marshal method without remembering to update the schema twin.

On the untyped-usage side, the standout is pkg/workflow's step-handling code: []any step slices get round-tripped through SliceToSteps/StepsToSlice at 8 separate call sites in workflow_builder_steps.go, even though a typed *WorkflowStep already exists and is the type actually operated on in between. There's also a 3-level nested map[string]any walk for model pricing (providers.<provider>.models.<model>.cost.*) whose exact shape is already documented in a comment — a clear candidate for concrete types.

Full Analysis Report

Duplicated Type Definitions

Summary Statistics

• Total type definitions analyzed: ~1,230 (struct/interface, non-test, under pkg/)
• Duplicate clusters found: 10
• Exact duplicates: 3 (one genuine bug-risk, two intentional build-tag variants)
• Near duplicates: 5
• Semantic duplicates: 2

---

Cluster 1: domainAnalysisWire exact duplicate

Kind: Exact · Impact: High

Locations:

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

Comparison: Byte-for-byte identical. domainAnalysisWire is the real MarshalJSON shadow struct for DomainAnalysis; domainAnalysisWireSchema is a hand-copied twin created only so jsonschema-go reflection produces a matching shape.

Recommendation: Delete domainAnalysisWireSchema and generate the schema from domainAnalysisWire directly. Effort: ~30 min. Benefit: removes a silent-drift trap.

---

Cluster 2: Wire-schema shadow structs mirroring MarshalJSON anonymous types

Kind: Near · Impact: High

Locations:

• pkg/cli/mcp_schema.go:242 — mcpServerHealthDetailWire mirrors the anonymous struct in MCPServerHealthDetail.MarshalJSON/UnmarshalJSON (pkg/cli/audit_expanded.go:101,125)
• pkg/cli/mcp_schema.go:253 — mcpServerCrossRunHealthWire mirrors MCPServerCrossRunHealth.MarshalJSON/UnmarshalJSON (pkg/cli/audit_cross_run.go:95,117)
• pkg/cli/mcp_schema.go:263 — mcpFailureSummaryWire mirrors MCPFailureSummary.MarshalJSON (pkg/cli/logs_models.go:226)
• pkg/cli/mcp_schema.go:234 — toolUsageSummaryWire mirrors ToolUsageSummary.MarshalJSON (pkg/cli/logs_report_tools.go:56)

Comparison: Four named "Wire" structs in mcp_schema.go, each hand-duplicating the field set of an anonymous struct literal embedded in a MarshalJSON/UnmarshalJSON method on an unrelated type in a different file. All four pairs match field-for-field today, but nothing enforces that they stay in sync.

Recommendation: Factor each anonymous marshal struct into a named, exported wire type in its home file; have mcp_schema.go reference that single definition instead of re-declaring it. Effort: ~2-3 hours for all four. Benefit: one source of truth per wire shape.

---

Cluster 3: MCPToolCall vs cachedLogsJSONLMCPToolCall

Kind: Near · Impact: Medium

Locations:

1. pkg/cli/audit_report.go:227 — MCPToolCall (11 fields incl. Error)
2. pkg/cli/logs_cached_json.go:72 — cachedLogsJSONLMCPToolCall (same 10 fields, no Error)

Recommendation: Have cachedLogsJSONLMCPToolCall embed/convert from MCPToolCall instead of re-declaring the shared fields.

---

Cluster 4: MCPToolUsageData vs MCPToolUsageSummary

Kind: Near · Impact: Medium

Locations:

1. pkg/cli/audit_report.go:194 — used in AuditData.MCPToolUsage, adds GuardPolicySummary
2. pkg/cli/logs_models.go:254 — used in LogsData.MCPToolUsage

Recommendation: Merge into one type (add GuardPolicySummary as optional on MCPToolUsageSummary) and reuse it in both containers.

---

Cluster 5: RepeatOptions vs PollOptions

Kind: Semantic · Impact: Medium

Locations: pkg/cli/retry.go:20, pkg/cli/signal_aware_poll.go:35

Comparison: Two independently-implemented "run a function repeatedly with context/signal awareness + progress messaging" option structs — one count-based, one interval/timeout-based.

Recommendation: Consider a shared loop-runner abstraction with count vs. interval/timeout modes, or document why both exist.

---

Cluster 6: Parallel comparison/diff subsystems (audit_comparison.go vs audit_diff.go)

Kind: Semantic · Impact: Medium

Locations: AuditComparisonData/Baseline/Delta (audit_comparison.go) vs AuditDiff/FirewallDiff/MCPToolsDiff/ToolCallsDiff/RunMetricsDiff/GitHubRateLimitDiff (audit_diff.go)

Comparison: Both model deltas between two runs (domains, MCP tools, tool calls, rate limits), for two different UX flows (implicit rolling-baseline comparison vs. explicit two-run diff), but each declares its own IntDelta/StringDelta primitives.

Recommendation: Extract one shared generic delta helper (NumericDelta[T], StringDelta) used by both; keep the feature-level containers separate since the UX flows genuinely differ.

---

Lower-priority / informational clusters

• ValidationError name collision (pkg/parser, pkg/validationerror, pkg/workflow.WorkflowValidationError) — all three deliberately embed the shared validationerror.Payload base; this is well-factored, just confusing to search for. No fix needed beyond a doc comment.
• BaseMCPServerConfig family (pkg/types/mcp.go, 126 references across 22 files) — MCPServerConfig and RegistryMCPServerConfig already correctly embed the shared base. cli.VSCodeMCPServer/MCPConfig model an external file format and are reasonably kept separate.
• ToolUsageInfo vs ToolUsageStatsBase/ToolUsageSummary (pkg/cli) — overlapping stats fields, not currently embedded; low priority.
• SpinnerWrapper/ProgressBar build-tag duplicates (pkg/console) — same name declared under mutually-exclusive Go build tags (default vs. wasm). Intentional platform abstraction, no action needed.

---

Untyped Usages

Summary Statistics (approximate, non-test files under pkg/)

• interface{}/any in function signatures: ~125
• interface{}/any in struct fields: ~90
• map[string]interface{}/map[string]any: ~2,771
• Untyped constants flagged: 9

The map[string]any count is large mostly because of legitimate dynamic-JSON handling (GitHub API responses, MCP tool arguments); the findings below are the subset where a concrete type is realistically feasible.

Category 1: []any step round-tripping (pkg/workflow)

Impact: High

• pkg/workflow/step_types.go:234,261 — SliceToSteps(steps []any) ([]*WorkflowStep, error) / StepsToSlice(steps []*WorkflowStep) []any. Called in matched pairs 8 times in pkg/workflow/workflow_builder_steps.go (e.g. lines 23/33, 45/55, 69/79, 121/129, 142/150, 182/190, 202/210, 242/250, 263/271) — each site converts []any → []*WorkflowStep → []any just to mutate steps in place, even though the typed WorkflowStep is what's actually operated on in between.
  • Fix: Keep step collections as []*WorkflowStep once parsed from frontmatter; serialize to []any only at the final YAML-emission boundary.
• pkg/workflow/frontmatter_types.go:412-415 — PreSteps, Steps, PreAgentSteps, PostSteps are all []any, immediately fed through SliceToSteps at nearly every consumer.
  • Fix: Type these fields as []*WorkflowStep directly; decode map[string]any → WorkflowStep once during frontmatter parsing.
• pkg/workflow/known_action_credentials.go:120 — DetectKnownCredentialLeakingActions(steps []any) hand-rolls step.(map[string]any) then stepMap["uses"].(string), duplicating what WorkflowStep.Uses/.With already provide.
  • Fix: func DetectKnownCredentialLeakingActions(steps []*WorkflowStep) map[string]struct{}; call SliceToSteps once at the call site.

Category 2: Nested map[string]any for model pricing

Impact: High

• pkg/workflow/compiler_model_pricing.go:124-207 and pkg/workflow/awf_config.go:198 (Providers map[string]any) — both walk a 3-level nested shape (providers.<provider>.models.<model>.cost.{input,output,cache_read,cache_write,reasoning}) via chained .(map[string]any) assertions. The shape is already documented in a comment on awf_config.go:196-197, and the same chain repeats in awf_config_build.go:519-521 and several tests.
  • Fix: Define ProviderPricing, ModelPricing, CostEntry structs and drop the assertion chains.

Category 3: Untyped constants (units hidden in comments, not types)

Impact: Low–Medium

• pkg/workflow/cache_integrity.go:17 — const defaultCacheIntegrityLevel = "none" duplicates the already-typed enum value GitHubIntegrityNone GitHubIntegrityLevel = "none" (pkg/workflow/tools_types.go:300) as a separate untyped string. Fix: reference GitHubIntegrityNone directly.
• pkg/constants/job_constants.go:310-311 — DefaultRateLimitMax = 5, DefaultRateLimitWindow = 60 (minutes, per comment only).
• pkg/cli/run_workflow_execution.go:25 — workflowCompletionWaitTimeoutMinutes = 6 * 60, an untyped int passed as minutes next to another constant in the same file that correctly uses time.Duration.
• pkg/workflow/publish_code_coverage.go:18 — defaultCodeCoverageWaitForProcessingTimeout = 160 (seconds, per comment only), where sibling timeouts elsewhere in the package already use time.Duration.
  • Fix pattern for all four: express as time.Duration where the value is a time span, or introduce a named unit type (type Minutes int, type RetryCount int) so units live in the type system instead of a comment.

Category 4: Union-typed MCP tool arguments

Impact: Low

• pkg/cli/mcp_tools_privileged.go:458-459 — RunID any, RunIDOrURL any intentionally accept either a JSON string or number.
  • Fix: A small custom type implementing json.Unmarshaler (e.g. type RunIDOrURL string with custom unmarshal) would keep the union explicit without any.

---

Refactoring Recommendations

Priority 1 — Fix the wire-schema duplication (Clusters 1 & 2)

1. Delete domainAnalysisWireSchema; generate its schema from domainAnalysisWire.
2. Move mcpServerHealthDetailWire, mcpServerCrossRunHealthWire, mcpFailureSummaryWire, toolUsageSummaryWire to live next to (and be reused by) the MarshalJSON methods they mirror.

Effort: ~3-4 hours · Benefit: eliminates the single highest-risk drift trap found.

Priority 2 — Type the workflow step pipeline ([]any → *WorkflowStep)

1. Change FrontmatterSteps-family fields to []*WorkflowStep.
2. Remove the 8 SliceToSteps/StepsToSlice round-trips in workflow_builder_steps.go; operate on []*WorkflowStep throughout.
3. Update DetectKnownCredentialLeakingActions to take []*WorkflowStep.

Effort: ~4-6 hours (touches core workflow compilation — needs full test suite run) · Benefit: removes ~10 type-assertion sites and a class of possible panics.

Priority 3 — Type the model-pricing map and fix unit-less constants

1. Introduce ProviderPricing/ModelPricing/CostEntry for compiler_model_pricing.go and awf_config.go.
2. Replace the 4 untyped duration/count constants with time.Duration or named unit types.

Effort: ~2-3 hours · Benefit: clearer intent, no more comment-only units.

---

Implementation Checklist

• Delete domainAnalysisWireSchema, point schema generation at domainAnalysisWire
• Consolidate the four mcp_schema.go "Wire" structs into their source files
• Type PreSteps/Steps/PreAgentSteps/PostSteps as []*WorkflowStep
• Remove redundant SliceToSteps/StepsToSlice round-trips in workflow_builder_steps.go
• Add ProviderPricing/ModelPricing/CostEntry types for model pricing maps
• Convert the 4 flagged untyped duration/count constants to time.Duration or named unit types
• Run full test suite after each change (go test ./pkg/...)

Analysis Metadata

• Scope: non-test .go files under pkg/ (~1,374 files)
• Total type definitions found: ~1,230
• Duplicate clusters: 10 (3 exact, 5 near, 2 semantic)
• Untyped usage findings: ~2,986 total occurrences scanned; 10 high-confidence, actionable findings detailed above
• Detection method: grep/rg pattern search + manual verification via file reads (no code execution)
• Analysis date: 2026-09-15

References:

• §34963240581

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 · 596.1 AIC · ⌖ 11.6 AIC · ⊞ 10K · ◷

• expires on Sep 16, 2026, 3:43 AM UTC-08:00

[github-actions[bot]]

This discussion was automatically closed because it expired on 2026-09-16T11:43:26.209Z.

Closed by Workflow