[typist] Typist: Go Type Consistency Analysis — duplicate types & untyped usages in pkg/

[github-actions[bot]]

🔤 Typist - Go Type Consistency Analysis

Analysis of repository: github/gh-aw

Executive Summary

I scanned all non-test .go files under pkg/ (1,326 files, ~1,150 struct/interface type definitions), using Serena for semantic lookups plus targeted structural analysis of the two largest packages, pkg/workflow (~700 files) and pkg/cli (~440 files).

The good news first: this codebase already practices strong typing well in the places it matters most. There is essentially zero raw interface{} left in production code (one stray mention survives only in a comment), map[string]any is used consistently and deliberately for the one place it's actually appropriate — passing through arbitrary user-authored YAML/JSON — and a shared pkg/types.BaseMCPServerConfig already prevents what would otherwise be MCP-config duplication across parser and workflow. The team has clearly already run consolidation passes in a few areas (e.g. ToolUsageStatsBase, DiffEntryBase in pkg/cli).

That said, I found 11 duplicate/near-duplicate type clusters and about a dozen untyped-usage spots worth a look. The standout: SafeOutputTargetConfig (the target/target-repo/allowed-repos trio used by every safe-output config) is already correctly embedded in 13 configs — but 11 other safe-output configs hand-copied the same three fields instead of embedding it, so the shared type exists and is just half-adopted. That's a low-risk, high-value, mechanical fix. There's also live drift already visible in pkg/cli: AuditData and RunAnalysis duplicate the same ~12-field analysis payload, and one of them renamed a shared field (FirewallTokenUsage vs TokenUsage) in the process.

Full Analysis Report

Duplicated Type Definitions

Summary Statistics

• Total non-test Go files analyzed: 1,326 (primary focus: pkg/workflow, pkg/cli, plus a full-repo sweep of the rest)
• Total type definitions surveyed: ~1,150 (structs + interfaces)
• Duplicate clusters found: 11
• Exact duplicates (identical field sets under different names): 2
• Near duplicates (majority field overlap, some divergence): 7
• Semantic duplicates (same concept, different structure/naming conventions): 2

Cluster 1: SafeOutputTargetConfig — half-adopted shared type

Type: Exact duplicate (11 manual copies) alongside 13 correct embeddings
Impact: High — the fix is purely mechanical and the shared type already exists

The canonical type is pkg/workflow/safe_outputs_parser.go:9:

type SafeOutputTargetConfig struct {
    Target         string   `yaml:"target,omitempty"`
    TargetRepoSlug string   `yaml:"target-repo,omitempty"`
    AllowedRepos   []string `yaml:"allowed-repos,omitempty"`
}

Correctly embedded (SafeOutputTargetConfig yaml:",inline"``) in 13 configs: assign_milestone.go:12, `mark_pull_request_as_ready_for_review.go:12`, `hide_comment.go:12`, `set_issue_field.go:10`, `add_reviewer.go:12`, `dismiss_pull_request_review.go:10`, `replace_label.go:21`, `set_issue_type.go:12`, `update_entity_helpers.go:95`, `submit_pr_review.go:18`, `resolve_pr_review_thread.go:14`, `link_sub_issue.go:12`, `reply_to_pr_review_comment.go:13`.

Manually re-declared (same 3 fields, same yaml tags, copy-pasted instead of embedded) in 11 more: create_issue.go:21-22, add_comment.go:18-20, dispatch_workflow.go:17, create_pr_review_comment.go:15, create_agent_session.go:13, update_project.go:28, create_pull_request.go:96, push_to_pull_request_branch.go:24, create_code_scanning_alert.go:18, create_discussion.go:21, comment_memory.go:10-12.

Recommendation: Swap the 11 manual declarations for the embedded form. YAML output shape is unchanged (inline embedding serializes identically), so this is a safe, mechanical refactor.
Estimated effort: 1-2 hours
Benefits: Removes ~33 duplicated lines, closes the gap so future fields on SafeOutputTargetConfig (e.g. a new target mode) propagate everywhere automatically.

---

Cluster 2: AuditData vs RunAnalysis — parallel single-run analysis graphs, with live drift

Type: Near duplicate
Impact: High — a field has already drifted apart under two names

pkg/cli/audit_report.go:29 (AuditData) and pkg/cli/logs_models.go:247 (RunAnalysis, embedded into RunSummary at logs_models.go:282 and DownloadResult at logs_models.go:292) both wrap the same ~12 nested analysis types with identical field name/type/json-tag: TaskDomain, BehaviorFingerprint, AgenticAssessments, MissingTools, MissingData, Noops, MCPFailures, SkillActivations, MCPToolUsage, GitHubRateLimitUsage, FirewallAnalysis, RedactedDomainsAnalysis.

They diverge only in the "header" (AuditData.Overview OverviewData vs RunAnalysis.Run WorkflowRun) and in report-only extras. Evidence of drift: AuditData.FirewallTokenUsage *TokenUsageSummary (json firewall_token_usage) is the same type as RunAnalysis.TokenUsage *TokenUsageSummary (json token_usage_summary) but under a different field/JSON name.

Recommendation: Extract the 12 shared fields into a RunAnalysisCore struct embedded by both, or have AuditData embed RunAnalysis directly and add only its report-specific fields.
Estimated effort: 2-3 hours
Benefits: Single source of truth for "what a run's analysis contains", eliminates the risk of the two views silently diverging further.

---

Cluster 3: "Workflow run" modeled 4 different ways

Type: Mixed (1 exact, rest semantic view-projections)
Impact: Medium

• pkg/cli/logs_models.go:60 WorkflowRun — the full internal model (DatabaseID, Number, URL, Status, Conclusion, WorkflowName, WorkflowPath, timestamps, Event, HeadBranch, HeadSha, ...).
• pkg/cli/run_workflow_tracking.go:19 WorkflowRunInfo — URL, DatabaseID, Status, Conclusion, CreatedAt: a strict, 100%-matching 5-field subset of WorkflowRun, used only for post-trigger polling.
• pkg/cli/audit_report.go:94 OverviewData and pkg/cli/logs_report.go:116 RunData are legitimate view projections with different JSON key casing for different commands (audit vs logs) — not recommended for full unification, but they duplicate ~6 field declarations that a shared RunIdentity struct could absorb.

Recommendation: Delete WorkflowRunInfo and have run_workflow_tracking.go use *WorkflowRun directly (exact duplicate, zero-risk removal). Leave OverviewData/RunData as separate output schemas, optionally sharing a small RunIdentity{RunID, WorkflowName, WorkflowPath, Status, Conclusion, CreatedAt} embed.
Estimated effort: 1 hour for WorkflowRunInfo removal
Benefits: One less type to keep in sync with WorkflowRun's evolution.

---

Cluster 4: Memory-entry family — CacheMemoryEntry / RepoMemoryEntry / DriveMemoryEntry

Type: Near duplicate
Impact: Medium

All three (pkg/workflow/cache_config.go:106, pkg/workflow/repo_memory.go:47, pkg/workflow/drive_memory_config.go:38) share an identical core — ID, Description, RestoreOnly, AllowedExtensions []string, Validation *MemoryValidationConfig — then add their own storage-specific fields.

Recommendation: Extract the 5 shared fields into an embedded MemoryEntryBase (yaml:",inline").
Estimated effort: 1-2 hours
Benefits: One place to change shared memory-entry semantics instead of three.

---

Cluster 5: GitHubMCPDockerOptions vs GitHubMCPRemoteOptions

Type: Near duplicate
Impact: Medium

pkg/workflow/mcp_renderer_types.go:67 and :101 share 8 of ~12 fields (ReadOnly, Lockdown, LockdownFromStep, GuardPoliciesFromStep, Toolsets, Features, AllowedTools, GuardPolicies), diverging only on transport-specific fields.

Recommendation: Extract the 8 shared fields into an embedded GitHubMCPCommonOptions.
Estimated effort: 1 hour
Benefits: Docker/Remote transport options stay in lockstep for shared behavior (read-only mode, lockdown, tool filtering).

---

Cluster 6: AwInfo vs AuditEngineConfig — engine metadata reimplemented

Type: Near duplicate
Impact: Medium

pkg/cli/logs_models.go:373 (AwInfo, the raw aw_info.json schema) and pkg/cli/audit_expanded.go:23 (AuditEngineConfig) share 7 of 9 fields (EngineID, EngineName, Model, Version, CLIVersion, FirewallVersion, Repository) verbatim. AuditEngineConfig is a hand-built projection that already missed propagating AwInfo's AgentRuntime field.

Recommendation: Build AuditEngineConfig via an explicit constructor from *AwInfo, or embed the shared subset.
Estimated effort: 1 hour
Benefits: Prevents future AwInfo fields from silently failing to reach audit reports (as AgentRuntime already has).

---

Cluster 7: ToolUsageInfo never migrated to the package's own established base type

Type: Near duplicate
Impact: Low-medium (easy fix, package already has the right pattern)

pkg/cli/audit_report.go:159 ToolUsageInfo duplicates 3 of 4 fields (CallCount, MaxOutputSize, MaxDuration) already extracted into ToolUsageStatsBase (pkg/cli/logs_report_tools.go:14) and reused by its siblings ToolUsageSummary/MCPToolSummary.

Recommendation: Embed ToolUsageStatsBase in ToolUsageInfo too, for consistency with the rest of the family.
Estimated effort: 30 minutes
Benefits: Closes the one gap in an otherwise-completed consolidation.

---

Cluster 8: graderManifestEntry — defined independently in producer and consumer packages

Type: Near duplicate (cross-package)
Impact: Medium-high — this is a write/read pair that can silently drift

pkg/workflow/compiler_yaml_graders.go:67 (the compiler's version, written into the manifest JSON consumed by the JS grader runtime) and pkg/cli/audit_report_graders.go:67 (the CLI's version, used when reading grader results back for the audit report) both declare a graderManifestEntry struct with overlapping ID, Name, Unit, Direction, Threshold fields — but the workflow-side struct has 8 more fields (Description, Source, Enabled, Max, Min, Digest, Run, Config) that the cli-side reader silently ignores.

Recommendation: Move the shared read/write schema into pkg/types (following the existing BaseMCPServerConfig pattern) so both packages import one canonical shape instead of maintaining two independently-evolving JSON contracts for the same file.
Estimated effort: 2 hours
Benefits: A future compiler-side field addition (e.g. a new grader property) won't have to be separately re-added on the reading side to show up in audit reports.

---

Cluster 9: runtimeImportReference — duplicated struct and duplicated regex

Type: Exact duplicate (cross-package)
Impact: Medium

pkg/parser/frontmatter_hash.go:572 and pkg/workflow/runtime_import_validation.go:53 each independently define:

type runtimeImportReference struct {
    path/importPath string
    startLine       int
    endLine         int
}

and each package separately compiles the same regex literal (\{\{#(?:runtime-import|import)\??(?:[ \t]+|[ \t]*:[ \t]*)([^{}]+?)\}\}) to populate it — runtimeImportReferenceRe in parser, runtimeImportMacroRe in workflow.

Recommendation: Move the struct and the regex into a shared location (pkg/parser already being imported by pkg/workflow, or a new small shared package) so a future change to the {{#runtime-import}} macro syntax can't be applied to one copy and missed in the other.
Estimated effort: 1-2 hours
Benefits: Single source of truth for runtime-import macro syntax.

---

Clusters 10-11: Lower priority, noted for awareness only

• Single-field Raw any tool-config wrappers: CacheMemoryToolConfig, DriveMemoryToolConfig, CommentMemoryToolConfig, RepoMemoryToolConfig (pkg/workflow/tools_types.go and repo_memory.go:64) are all exactly struct { Raw any yaml:"-" }. Trivial to consolidate into one RawPassthroughToolConfig, but low impact.
• AgentAPIProxyTargetConfig (pkg/workflow/sandbox.go:120) vs AWFAPITargetConfig (pkg/workflow/awf_config.go:217) share 4 fields (AuthHeader, ExtraHeaders, ExtraBodyFields, SessionId) but are a legitimate two-layer split (user-facing frontmatter schema vs. external AWF tool schema) — leave as two structs, optionally share the 4 common fields via an embed to avoid name drift.

Left alone (verified, not duplicates): pkg/console's ProgressBar/SpinnerWrapper appear twice each, but it's a (go/redacted):build wasm vs !wasm platform split with genuinely different implementations — correct Go idiom, not a smell. SRTNetworkConfig/AWFNetworkConfig and similar sandbox pairs map to different, non-interchangeable external JSON schemas (legacy SRT is being migrated away from, see migrateSRTToAWF). pkg/types.BaseMCPServerConfig is already a working example of the fix pattern recommended above — it's explicitly documented as being embedded by both parser.RegistryMCPServerConfig and workflow.MCPServerConfig to avoid exactly this kind of duplication.

---

Untyped Usages

Summary Statistics

• interface{} usages in production code: 0 (one stray mention survives only inside a comment)
• any usages that cross a real API boundary (vs. legitimate passthrough): ~7 flagged below
• Untyped constants worth a semantic type: ~8 flagged below
• map[string]any/[]any passthrough usages: thousands, and correctly left alone (see note below)

Category 1: any fields hiding an implicit sum type

Impact: High — each is type-switched independently at multiple call sites, so a missed case fails silently

PrivateToPublicFlows any — pkg/workflow/tools_types.go:419, in GitHubToolConfig. Holds either the literal string "allow" or a []string of MCP server IDs, and is re-type-switched independently in three separate files: mcp_gateway_config.go:175, mcp_github_config.go:565, strict_mode_network_validation.go:235,256.

// current
PrivateToPublicFlows any `yaml:"-"`

Suggested fix:

type PrivateToPublicFlowsPolicy struct {
    AllowAll  bool
    ServerIDs []string
}

with one parse function and IsAllowAll()/ServerIDs() accessors replacing the 4 scattered assertions.

Data any — pkg/workflow/safe_outputs_config_types.go:96, in SafeOutputsConfig. A 4-shape union (bool false/omitted, bool true, inline schema object, or GH Actions expression string), type-switched in pkg/workflow/safe_outputs_data_schema.go:76. Lower urgency (single call site), but a named sum type would let the switch's exhaustiveness be checked once.

---

Category 2: Adjacent same-typed parameters (real swap risk)

Impact: High — this is a genuine "wrong argument order compiles fine" hazard

Location: pkg/cli/mcp_tools_privileged.go:133 and :118

func effectiveMCPLogsToolTimeoutMinutes(requestedTimeout, count int, workflowName, engine string) int
func defaultMCPLogsToolTimeoutMinutesForCount(count int) int

requestedTimeout (minutes) and count (number of runs) are both bare int — nothing stops a caller from swapping them. The related constants (mcpLogsRunsPerDefaultTimeoutMinute, defaultMCPLogsTimeoutMinutes, lines 20-22) are bare int too.

Suggested fix:

type timeoutMinutes int
func effectiveMCPLogsToolTimeoutMinutes(requestedTimeout timeoutMinutes, count int, workflowName, engine string) timeoutMinutes

---

Category 3: Untyped duration/size constants (semantic clarity)

Impact: Medium — internal-only, so safe to change; the package is already inconsistent about it (some sibling files already do this correctly)

// current — units only in a comment, inconsistent with sibling files that already
// write "3 * time.Second" / "5 * time.Minute" (docker_validation.go:60, repository_features_validation.go:62,69)
const defaultCodeCoverageWaitForProcessingTimeout = 160          // pkg/workflow/publish_code_coverage.go:18 (seconds)
const enclaveGitHubProxyMaxTTL = 600                             // pkg/workflow/enclave_github_proxy.go:22 (seconds; JSON tag says "_seconds" but Go type doesn't)
const defaultMemoryValidationTimeoutMinutes = 1                 // pkg/workflow/memory_validation_config.go:15 (minutes)
const maxMemoryValidationTimeoutMinutes = 5                      // pkg/workflow/memory_validation_config.go:16
const DefaultActionFailureIssueExpiresHours = 24 * 7             // pkg/workflow/repo_config.go:63 (hours)
const defaultNoOpIssueExpirationHours = 24 * 30                  // pkg/workflow/maintenance_workflow.go:128 (hours, same concept as above, different constant)

Suggested fix: multiply into time.Duration at the declaration site (160 * time.Second, 600 * time.Second, 5 * time.Minute), matching the style the package already uses elsewhere — this is exactly the class of bug that forced MaxCapabilityTTL's JSON tag to spell out _seconds in its name because the Go type alone doesn't carry the unit.

Lower priority — bare byte-size constants (defaultRepoMemoryMaxFileSize = 102400 // 100KB, defaultRepoMemoryMaxPatchSize = 10240 // 10KB, maxRepoMemoryPatchSize = 1048576 // 1MB in pkg/workflow/repo_memory.go:27-31; maxOperationalValueEvaluatorSize = 64 * 1024 in pkg/workflow/graders_operational_value.go:16) are internal-only and low-risk, but a type ByteSize int with KB/MB constants would make the mixed style (some already multiplied, some not) consistent.

---

Category 4: Legitimate any usage — no action needed

The following are correct, idiomatic uses of any and are called out only so they aren't mistaken for gaps in a future pass:

• JSON/YAML passthrough for arbitrary user content: map[string]any/[]any across both packages (thousands of occurrences) — this is how the compiler represents arbitrary frontmatter, generated GitHub Actions with: blocks, and tool schemas. Changing this would require modeling the entirety of GitHub Actions YAML as Go types, which is not worth the cost.
• YAML union fields mirroring GitHub Actions' own flexible schema: On any, RunsOn any, Imports any, Checkout any, Engine any (pkg/workflow/workflow_file.go, frontmatter_types.go, runs_on_unmarshal.go) and On any in pkg/cli/jsonworkflow_to_markdown.go:39 / list_workflows_command.go:27 — these fields can legitimately be a string, array, or object in upstream YAML; Go has no union types, so any + a type-switch during parsing is the correct idiom.
• Wire-format fields with a spec'd polymorphic type: RunID/RunIDOrURL any (pkg/cli/mcp_tools_privileged.go:410-411, JSON-RPC tool args accepting string or number) and ID any (pkg/cli/gateway_logs_types.go:179,190, JSON-RPC 2.0's spec'd id: string|number|null).
• Generic conversion utilities: pkg/typeutil/convert.go (ParseIntValue(value any), ConvertToInt(val any), etc.) — this package's entire purpose is safe conversion of heterogeneous values; any parameters are the point, not a gap.
• Decode-into targets: DoWithContext(..., response any) error (pkg/cli/update_check.go:201) mirrors encoding/json.Unmarshal's own signature — standard Go idiom.
• Untyped numeric thresholds used once, next to their declaration: mcpErrorRateThreshold, mcpConnectionRateThreshold, spikeDetectionMultiplier (pkg/cli/audit_cross_run.go:15-23) and similar single-use, well-commented constants — idiomatic Go, not worth wrapping.

---

Refactoring Recommendations

Priority 1: Critical — Finish the SafeOutputTargetConfig rollout

Recommendation: Replace the 11 hand-copied Target/TargetRepoSlug/AllowedRepos field trios with an embedded SafeOutputTargetConfig.

Steps:

1. In each of the 11 files listed in Cluster 1, replace the three literal fields with SafeOutputTargetConfig yaml:",inline"``.
2. Update any direct field-literal construction sites (Target: "..." → nested or via the embedded promoted field, which works unchanged in Go).
3. Run the existing compiler/config tests to confirm YAML round-trips are unaffected (inline embedding preserves the flat YAML shape).

Estimated effort: 1-2 hours
Impact: High — a shared type already exists; this closes the half-finished adoption gap.

---

Priority 2: High — Fix live drift in pkg/cli run-analysis types

Recommendation: Extract RunAnalysisCore shared by AuditData and RunAnalysis; rename AuditData.FirewallTokenUsage to match RunAnalysis.TokenUsage (or vice versa) as part of the same pass.

Steps:

1. Define RunAnalysisCore with the 12 shared analysis fields.
2. Embed it in both AuditData and RunAnalysis.
3. Reconcile the FirewallTokenUsage/TokenUsage naming so both structs expose the same field/JSON name.
4. Run pkg/cli tests, particularly anything snapshotting audit-report JSON shape.

Estimated effort: 2-3 hours
Impact: High — removes an active source of silent divergence between the audit and logs code paths.

---

Priority 3: Medium — Delete WorkflowRunInfo, add PrivateToPublicFlowsPolicy, fix the timeoutMinutes/count swap risk

Recommendation: Three independent, small, high-value fixes:

1. Delete pkg/cli/run_workflow_tracking.go's WorkflowRunInfo and use *WorkflowRun directly.
2. Introduce PrivateToPublicFlowsPolicy in pkg/workflow and replace the 4 scattered type assertions on PrivateToPublicFlows any.
3. Give pkg/cli/mcp_tools_privileged.go's timeout/count parameters distinct types so they can't be swapped.

Estimated effort: 3-4 hours combined
Impact: Medium — each is small in isolation but removes a real footgun.

---

Priority 4: Low — Remaining consolidations

Memory-entry family, GitHubMCPDockerOptions/RemoteOptions, AwInfo/AuditEngineConfig, ToolUsageInfo, the cross-package graderManifestEntry and runtimeImportReference pairs, and the duration/byte-size constant typing — all detailed above, roughly 1-2 hours each, do independently as time allows.

---

Implementation Checklist

• Embed SafeOutputTargetConfig in the 11 manually-duplicated safe-output configs
• Extract RunAnalysisCore shared by AuditData and RunAnalysis; reconcile the FirewallTokenUsage/TokenUsage naming
• Delete WorkflowRunInfo, use *WorkflowRun directly in run_workflow_tracking.go
• Replace PrivateToPublicFlows any with a typed PrivateToPublicFlowsPolicy
• Give the MCP-logs timeout/count parameters distinct types in mcp_tools_privileged.go
• Extract MemoryEntryBase for CacheMemoryEntry/RepoMemoryEntry/DriveMemoryEntry
• Extract GitHubMCPCommonOptions for the Docker/Remote MCP option structs
• Build AuditEngineConfig from AwInfo via a constructor (fixes the missing AgentRuntime propagation)
• Embed ToolUsageStatsBase in ToolUsageInfo
• Move graderManifestEntry's shared fields into pkg/types
• Move runtimeImportReference and its regex into one shared location
• Convert the flagged bare duration/size constants to time.Duration/typed units
• Run the full test suite after each change

Analysis Metadata

• Total Go Files Analyzed: 1,326 (non-test, under pkg/)
• Total Type Definitions: ~1,150 (structs + interfaces)
• Duplicate Clusters: 11 (9 detailed, 2 noted for awareness)
• Untyped Usage Locations: ~15 flagged, ~10 explicitly verified as legitimate and left alone
• Detection Method: Serena semantic analysis, targeted grep-based structural clustering, and parallel deep-dive review of pkg/workflow and pkg/cli
• Analysis Date: 2026-08-28

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

• expires on Aug 29, 2026, 7:43 AM UTC-08:00

[github-actions[bot]]

This discussion was automatically closed because it expired on 2026-08-29T15:43:36.107Z.

Closed by Workflow

[github-actions[bot]]

🤖 Smoke test bot stopping by! This typist report is looking sharp — keep those types consistent. Beep boop, all systems nominal! 🎉

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

• accounts.google.com
• android.clients.google.com
• clients2.google.com
• contentautofill.googleapis.com
• www.google.com
• www.gstatic.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "www.google.com"
    - "www.gstatic.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot · copilot · auto · 50 AIC · ⌖ 2.53 AIC · ⊞ 9.9K · ◷
_{Comment /smoke-copilot to run again
Add label smoke to run again}