[typist] Typist - Go Type Consistency Analysis

[github-actions[bot]]

Analysis of repository: github/gh-aw — workflow run §36708587651

Executive Summary

I scanned 1,393 non-test .go files under pkg/ (roughly 2,466 files total including tests), split across pkg/cli (~557 files), pkg/workflow (~540 files), and 34 smaller packages (~396 files), using Serena's semantic tooling plus targeted interface{}/any/constant pattern searches. In total I found around 700 top-level struct/interface declarations.

The good news: this codebase already practices several of the strong-typing patterns I'd normally recommend from scratch — pkg/types.BaseMCPServerConfig is explicitly embedded by both parser and workflow to avoid duplication, pkg/constants.StepID/JobName and pkg/stringutil.PATType are clean typed-string enums with String()/IsValid() methods, and literal interface{} has been almost entirely replaced by any. The remaining issues are concentrated in a few clear buckets: near-identical structs re-declared under different names (TrialArtifacts/WorkflowTrialResult being the cleanest example — three identical fields, down to an identical commented-out line), a dual representation of workflow tool config (WorkflowData.Tools map[string]any living alongside an already-parsed *Tools struct), and untyped string/int constants sitting right next to well-typed siblings in the same file (pkg/constants/job_constants.go has a typed StepID block immediately followed by a dozen untyped output-name string constants). None of this is urgent, but each cluster below is a small, mechanical, low-risk cleanup.

Full Analysis Report

Duplicated Type Definitions

Summary Statistics

• Files analyzed: 1,393 non-test .go files (pkg/cli, pkg/workflow, 34 other packages)
• Type declarations found: ~700 struct/interface declarations (~600 in pkg/cli, ~525 in pkg/workflow, ~90-100 elsewhere)
• Structural note: pkg/cli and pkg/workflow are each a single Go package, so identical identifiers cannot literally collide within them (the compiler forbids it) — real duplication in those packages shows up as near-identical structs under different names, not exact redeclarations. Exact same-name duplication is only possible across packages.

Cluster 1: TrialArtifacts / WorkflowTrialResult — near-exact field duplication

Type: Near-duplicate (verified)
Impact: Medium — same three fields hand-copied between two files, already drifting in comments

Locations:

1. pkg/cli/trial_support.go:19-24 — TrialArtifacts
2. pkg/cli/trial_types.go:6-21 — WorkflowTrialResult

Definition Comparison (verified by direct read):

// trial_support.go
type TrialArtifacts struct {
	SafeOutputs map[string]any `json:"safe_outputs"`
	(agentstdiologs/redacted)      []string               `json:"agent_stdio_logs,omitempty"`
	AgenticRunInfo      map[string]any `json:"agentic_run_info,omitempty"`
	AdditionalArtifacts map[string]any `json:"additional_artifacts,omitempty"`
}

// trial_types.go
type WorkflowTrialResult struct {
	WorkflowName string         `json:"workflow_name"`
	RunID        string         `json:"run_id"`
	SafeOutputs  map[string]any `json:"safe_outputs"`
	(agentstdiologs/redacted)      []string               `json:"agent_stdio_logs,omitempty"`
	AgenticRunInfo      map[string]any `json:"agentic_run_info,omitempty"`
	AdditionalArtifacts map[string]any `json:"additional_artifacts,omitempty"`
	Timestamp           time.Time      `json:"timestamp"`
	Success             bool           `json:"success"`
	SafeOutputErrors    []string       `json:"safe_output_errors,omitempty"`
}

Same JSON tags, same field order, even the same commented-out AgentStdioLogs line copy-pasted between files — a strong signal one was cloned from the other.

Recommendation: Have WorkflowTrialResult embed TrialArtifacts instead of re-declaring the three shared fields:

type WorkflowTrialResult struct {
	WorkflowName string `json:"workflow_name"`
	RunID        string `json:"run_id"`
	TrialArtifacts
	Timestamp        time.Time `json:"timestamp"`
	Success          bool      `json:"success"`
	SafeOutputErrors []string  `json:"safe_output_errors,omitempty"`
}

• Estimated effort: <1 hour
• Benefits: One place to add/rename artifact fields; removes the stale duplicated comment

---

Cluster 2: "Wire" schema-mirror structs in pkg/cli/mcp_schema.go

Type: Near-duplicate (deliberate but risky)
Impact: Medium-high — five hand-maintained structs can silently drift from the real MarshalJSON output they're supposed to mirror

Locations:

• mcp_schema.go:239 toolUsageSummaryWire mirrors logs_report_tools.go:46 ToolUsageSummary
• mcp_schema.go:247 mcpServerHealthDetailWire mirrors audit_expanded.go:90 MCPServerHealthDetail
• mcp_schema.go:258 mcpServerCrossRunHealthWire mirrors audit_cross_run.go:85 MCPServerCrossRunHealth
• mcp_schema.go:268 mcpFailureSummaryWire mirrors logs_models.go:237 MCPFailureSummary
• mcp_schema.go:275 domainAnalysisWireSchema mirrors access_log.go:35 DomainAnalysis (which already has its own hand-rolled sibling access_log.go:44 domainAnalysisWire)

Recommendation: The mcp_schema.go:238 comment explains these exist because jsonschema-go reflection can't infer custom MarshalJSON output — a legitimate constraint. Rather than removing the pattern, add a round-trip test per pair (marshal the real type, unmarshal into the Wire type, assert field-for-field equality) so schema drift fails CI instead of shipping silently.

• Estimated effort: 2-3 hours (one test helper + 5 call sites)
• Benefits: Turns a "trust me" duplication into a verified one

---

Cluster 3: Dual tool-config representation in pkg/workflow.WorkflowData

Type: Near-duplicate / stale untyped field (verified)
Impact: High — this is the compiler's central data structure

Location: pkg/workflow/workflow_data.go:90-92

Tools                          map[string]any
LSP                            map[string]LSPServerConfig
ParsedTools                    *Tools                     // Structured tools configuration (NEW: parsed from Tools map)

A fully-typed *Tools struct already exists and is populated from Tools, but the untyped map[string]any remains the field threaded through much of the compiler (per the sub-analysis, Tools is read far more widely than ParsedTools).

Recommendation: Audit remaining WorkflowData.Tools reads, migrate them to ParsedTools, and once nothing depends on the raw map, rename ParsedTools to Tools and drop the untyped field entirely (or keep the raw map only as a private parse-time intermediate, not a public field).

• Estimated effort: 4-6 hours (touches many call sites, needs careful test coverage)
• Benefits: Eliminates the single largest "trust the map key spelling" risk in the compiler

---

Cluster 4: Near-duplicate "hosted web policy" / "network config" chains in pkg/workflow

Type: Near-duplicate (layered, partially intentional)
Impact: Medium — high confusion risk even where layering is deliberate

Locations:

• HostedWebPolicy (engine.go:210) vs. AWFHostedWebConfig/AWFHostedWebPolicy (awf_config.go:219,225) — same enabled/allowed/blocked/max-uses shape, divergent field names (Allowed vs AllowedDomains) across the frontmatter → AWF-runtime boundary
• EngineNetworkConfig (engine.go:263) / EngineNetworkDefinition (engine_definition.go:189) / AWFNetworkConfig (awf_config.go:120) — three shapes across the frontmatter→definition→runtime pipeline
• EngineInstallConfig (engine_helpers.go:56) vs. EngineInstallationDefinition (engine_definition.go:198) — two parallel install-step systems (hardcoded vs. declarative engines)

Recommendation: Where a working converter already exists (as with EngineCapabilities → EngineCapabilitiesDefinition's ToRuntimeCapabilities()), no change needed — that's the right pattern. Where it doesn't (the three clusters above), add one and standardize field names (Allowed vs AllowedDomains) so the same concept doesn't require re-reading two structs to map fields.

• Estimated effort: 3-5 hours across the three chains
• Benefits: Fewer copy/paste field-name mismatches when the frontmatter schema evolves

---

Cluster 5: "Per-domain stats" sprawl in pkg/cli (lower confidence)

Type: Near-duplicate concept, 8 occurrences
Impact: Low-medium — each may be justified by a different reporting context

Locations: DomainItem (domains_command.go:35), DomainRequestStats (firewall_log.go:157), DomainBreakdown (outcome_domain_breakdown.go:14), DomainInventoryEntry/DomainRunStatus (audit_cross_run.go:152,162), DomainDiffEntry (audit_diff.go:31), DomainBuckets (domain_buckets.go:13), RedactedDomainsAnalysis (redacted_domains.go:21), domainAggregation (logs_report_firewall.go:33)

Recommendation: This package already has a proven fix for exactly this shape of problem — the existing AggregatedSummaryBase/MCPServerStatsBase/ToolUsageStatsBase/FirewallSummaryBase/DiffEntryBase/AnalysisBase embedding pattern. Apply the same base-struct-plus-embedding approach to the Domain-* family rather than introducing a new abstraction.

• Estimated effort: 3-4 hours, worth a design discussion first given lower confidence
• Benefits: Consistency with the pattern already established elsewhere in the same package

---

Untyped Usages

Summary Statistics

• Literal interface{} is essentially gone from production code — the codebase has already standardized on any.
• The overwhelming majority of any/map[string]any usage (thousands of occurrences) is intentional: arbitrary YAML/JSON frontmatter, MCP tool config, and JSON-RPC passthrough where the schema is genuinely user-defined or spec-mandated (e.g. JSON-RPC 2.0's polymorphic id field). These were explicitly excluded from findings below.
• ~15 high-value untyped-usage locations and ~10 untyped-constant clusters were identified as real refactor candidates (selection below; full lists available on request from the sub-analyses).

Category 1: interface{}/any in struct fields (real candidates)

Location	Current	Likely concrete type	Note
pkg/cli/copilot_events_jsonl.go:76	Usage map[string]any	{PromptTokens, CompletionTokens, TotalTokens int}	Sits beside typed InputTokens int/OutputTokens int on the same struct
pkg/cli/audit_report.go:161-171	CreatedItemReport.ID any, .BeforeState/.AfterState map[string]any	ID: string (GitHub node ID); before/after likely map[string]string per label-diff usage	Worth checking each safe-output handler's actual writes
pkg/cli/bootstrap_profile_helpers.go:264,283	manifest map[string]any (GitHub App manifest)	typed GitHubAppManifest struct	Code already does a defensive type-switch on manifest["default_permissions"] at line 285-289 — exactly the churn a concrete type removes
pkg/workflow/workflow_data.go:90	Tools map[string]any	already-typed *Tools (see Cluster 3 above)	Highest-impact item in this list
pkg/workflow/safe_output_handlers.go (~35 entries)	NewConfig func() any { return &XConfig{} }	shared SafeOutputConfig marker interface	Every entry is immediately type-asserted back to a concrete type
pkg/console/console_types.go:49	FormField.Value any	—	Appears effectively dead outside the wasm stub/tests; consider removing
pkg/parser/import_observability.go:19	observabilityImportEndpoint.Headers any	string | map[string]string	Comment says kept as any deliberately for later normalization by pkg/workflow — flagged, not a clean skip

Category 2: Untyped Constants

The strongest pattern here is inconsistency within a single file — a typed enum sits right next to an untyped one for the same kind of value:

// pkg/constants/job_constants.go — the good pattern, already in the file:
type StepID string
const CheckMembershipStepID StepID = "check_membership"
func (s StepID) String() string { return string(s) }
func (s StepID) IsValid() bool  { return s != "" }

// ...a few dozen lines later, same file, same kind of value, untyped:
const IsTeamMemberOutput = "is_team_member"
const StopTimeOkOutput = "stop_time_ok"
// ...10 more output-name constants, all bare strings

Suggested fix: introduce type OutputName string mirroring StepID and retype the output-name block — this is a copy-paste of a pattern the file already demonstrates correctly.

Other verified/reported untyped-constant clusters:

Location	Current	Suggested
pkg/workflow/run_phase.go:8-10	runPhaseAgent = "agent", etc.	type RunPhase string
pkg/workflow/sandbox_agent_images.go:32-42	11 untyped image-role strings	type AWFImageRole string
pkg/workflow/enclaves.go:46-53	enclaveMCPConnectTimeout = 120, enclaveMCPReadinessTimeoutMS = 120000	time.Duration-typed consts (name mixes seconds/ms today)
pkg/workflow/ledger.go:19-21 vs pkg/workflow/repo_memory.go:28	KB constants vs. a raw-byte constant, same "size" domain	shared sized-unit type across both
pkg/cli/add_interactive_engine.go:238-239	authMethodCopilotRequests/authMethodPAT bare strings, compared with == at 3 call sites	type authMethod string
pkg/cli/run_workflow_execution.go:25	workflowCompletionWaitTimeoutMinutes = 6 * 60 (bare int named "Minutes" storing 360)	const workflowCompletionWaitTimeout = 6 * time.Hour
pkg/cli/token_usage_types.go:149-150	modelMismatchReasonTokenUsageMissing/ModelNotObserved bare strings assigned into a plain-string field	type ModelMismatchReason string
pkg/agentdrain/anomaly.go	AnomalyWeightNew/Low/Rare/MaxScore bare float64	type AnomalyWeight float64
pkg/cli/logs_cached_json.go:24, audit_cache.go:16, add_package_ownership.go:24	three unrelated "schema version" int constants, each compared against an unrelated struct field	per-domain named types (a value from one can't accidentally satisfy another)

What's already good (cite as the target pattern)

• pkg/types.BaseMCPServerConfig — explicitly embedded by both pkg/parser and pkg/workflow "to eliminate duplication"; exactly the shared-type-package pattern to extend to the clusters above.
• pkg/stringutil.PATType and pkg/constants.StepID/JobName — typed string enums with String()/IsValid() methods.
• pkg/parser.ValidationError embedding pkg/validationerror.Payload, detectable via errors.As — a clean shared-interface pattern other ad hoc error structs could imitate.

---

Refactoring Recommendations

Priority 1 — WorkflowData.Tools dual representation (Cluster 3): highest blast radius since it's the compiler's core struct; migrate remaining Tools map reads to the existing typed ParsedTools, then retire the map. ~4-6 hours.

Priority 2 — TrialArtifacts/WorkflowTrialResult duplication (Cluster 1): mechanical, low-risk, embed instead of re-declare. <1 hour.

Priority 3 — job_constants.go output-name constants: copy the file's own StepID pattern onto the output-name block. ~1 hour.

Priority 4 — MCP schema "Wire" struct drift risk (Cluster 2): add round-trip tests rather than removing the pattern (the underlying constraint is real). 2-3 hours.

Priority 5 (lower confidence, discuss first) — Domain-* stats sprawl (Cluster 5) and the hosted-web/network-config layering (Cluster 4): apply the codebase's own existing *Base embedding pattern / add missing converters.

Implementation Checklist

• Embed TrialArtifacts into WorkflowTrialResult
• Add round-trip tests for the 5 *Wire structs in mcp_schema.go
• Migrate WorkflowData.Tools readers to ParsedTools, then drop the untyped map field
• Introduce type OutputName string in pkg/constants/job_constants.go
• Introduce type RunPhase string / type AWFImageRole string in pkg/workflow
• Review Domain-* stats structs against the existing *Base embedding pattern
• Review HostedWebPolicy/EngineNetworkConfig chain field-name consistency

Analysis Metadata

• Total Go files analyzed: 1,393 non-test files (pkg/cli, pkg/workflow, 34 other packages)
• Total type declarations: ~700
• Duplicate/near-duplicate clusters reported: 5 (plus cross-package good-pattern citations)
• Untyped usage locations reported: ~15 struct/field/param examples, ~10 constant clusters
• Detection method: Serena semantic analysis (activate_project, symbol/reference search) + targeted grep pattern matching, with direct-read verification of the top clusters
• Analysis date: 2026-09-30

References:

• §36708587651

Generated by ✍️ Typist - Go Type Analysis · claude · agent · 765.8 AIC · ⌖ 7.39 AIC · ⊞ 6.3K · ◷

• expires on Oct 1, 2026, 3:44 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 #64752.