[typist] Typist: Go type consistency findings — duplicate types and untyped usage in pkg/

[github-actions[bot]]

🔤 Typist - Go Type Consistency Analysis

Analysis of repository: github/gh-aw

Executive Summary

I scanned all 1,317 non-test .go files under pkg/ (1,135 exported/unexported type definitions) for duplicated types and untyped (interface{}/any) usage. The headline finding is good news: this codebase is already disciplined about typing. interface{} is essentially extinct in production code (1 real occurrence, everything else is linter test fixtures), and the team has already built explicit semantic-type conventions (pkg/constants) and a typed replacement for the biggest map[string]any offender (workflow.ToolsConfig). The real opportunities are narrower than a first grep suggests: a handful of genuine same-name type collisions to double-check, one deliberate but shrinkable near-duplicate config family (AWF/SRT/bounded-queries), and a batch of untyped file-path/artifact-name string constants that don't yet follow the semantic-type pattern already established two files away.

Full Analysis Report

Duplicated Type Definitions

Summary Statistics

• Total files analyzed: 1,317 non-test .go files under pkg/
• Total type definitions found: 1,135
• Same-name types (count > 1): 7 names, 16 declarations
• Confirmed false positives (build-tag platform variants): 2 names (4 declarations)
• Naming collisions worth a rename: 1
• Deliberate shared-interface pattern (no action needed): 1
• Near-duplicate config clusters (semantic duplicates): 2 families, 8 types

Cluster 1: ProgressBar / SpinnerWrapper — false positive, verified clean

Type: Same name, different files
Locations:

• pkg/console/progress.go:31 ((go/redacted):build !js && !wasm)
• pkg/console/progress_wasm.go:9 ((go/redacted):build js || wasm)
• pkg/console/spinner.go:92 ((go/redacted):build !js && !wasm, inferred)
• pkg/console/spinner_wasm.go:12 ((go/redacted):build js || wasm)

Verdict: Not a duplicate. These are intentional per-platform implementations selected by build tags (native TTY vs. WASM/JS). No action needed — flagging only so the team knows this pattern was checked, not missed.

---

Cluster 2: Finding — same name, different concepts

Type: Naming collision (not a structural duplicate)
Occurrences: 2
Impact: Low-medium — no functional bug, but the same word means two different things in two packages that both deal with "problems found in a workflow."

Locations:

1. pkg/scanfindings/scanfindings.go:107 — shared scanner finding format (RuleID, Severity, Message, File, Line, Column, Context)
2. pkg/cli/audit_report.go:67 — an audit insight (Category, Severity, Title, Description, Impact)

Recommendation: Rename the audit-report one to AuditFinding (or similar) to avoid ambiguity when both packages are imported side by side, e.g. in pkg/cli. Low effort (single type + its constructors), no behavior change.

---

Cluster 3: ValidationError — good pattern, not a duplicate

Locations:

1. pkg/validationerror/validationerror.go:49 — shared ValidationError interface (error + ValidationField/Value/Reason/Suggestion)
2. pkg/parser/validation_error.go:12 — parser.ValidationError struct embedding validationerror.Payload, implementing the shared interface
3. (referenced but not re-checked) workflow.WorkflowValidationError follows the same embedding pattern

Verdict: This is exactly the "consolidate common behavior, let each package keep its own concrete type" pattern the rest of this report recommends elsewhere. Worth calling out as a positive example other clusters (see below) could follow, not a problem to fix.

---

Cluster 4: AWF / frontmatter bounded-query config — deliberate near-duplicate, shrinkable

Type: Near duplicate (same fields, different serialization tags)
Impact: Medium — two structs to keep in sync by hand whenever the schema changes

Locations:

1. pkg/workflow/tools_types.go:414 BoundedQueriesConfig / :454 BoundedQueryPrivateRepo — YAML frontmatter shape (yaml:"..." tags)
2. pkg/workflow/awf_config.go:111 AWFBoundedQueriesConfig / :145 AWFBoundedQueryPrivateRepo — AWF config-file JSON shape (json:"..." tags)

Both carry the identical field set (PrivateRepos, Runtime, Timeout, MemoryLimit, Interpreter, MaxInvocations), and the AWF-side doc comments even say things like "A pointer mirrors BoundedQueriesConfig.Timeout so nil-vs-zero semantics stay in sync" — the duplication is already acknowledged in comments as something that has to be kept in sync manually.

Recommendation:

• Define one canonical struct with both yaml and json tags per field (Go supports multiple tags on one field), or
• Generate the AWF-shape struct from the frontmatter struct with a small mapping function, so the field list only exists once.
• Estimated effort: 2-3 hours plus test updates.
• Benefit: schema drift between the frontmatter surface and the AWF binary contract becomes a compile error instead of a manual-sync convention.

Cluster 5: Network/filesystem sandbox config — same package, overlapping responsibility

Locations:

1. pkg/workflow/awf_config.go:168 AWFNetworkConfig (AllowDomains, BlockDomains, Isolation, TopologyAttach) / :188 AWFFilesystemConfig (AllowWrite)
2. pkg/workflow/sandbox.go:167 SRTNetworkConfig (AllowedDomains, BlockedDomains, AllowUnixSockets, AllowLocalBinding, AllowAllUnixSockets) / :176 SRTFilesystemConfig (DenyRead, AllowWrite, DenyWrite)

Verdict: These are not structurally identical (SRT has finer-grained unix-socket and read/write-deny controls AWF doesn't), so this is a semantic overlap rather than a copy-paste duplicate — both packages model "what can this sandbox touch on the network/filesystem" for two different sandbox backends (AWF vs. SRT) living in the same pkg/workflow package.

Recommendation: Lower priority than Cluster 4. Worth a follow-up design question for the team: do AWF and SRT genuinely need independent shapes, or could a shared NetworkPolicy/FilesystemPolicy base with backend-specific extension fields serve both? Not urgent — flagging for awareness, not immediate action.

---

Untyped Usages

Summary Statistics

• interface{} usages (non-test): 1 in production code (pkg/workflow/behavior_defined_engine.go), 17 more in linter testdata/golden fixtures — effectively already eliminated.
• any usages (non-test): ~2,700 across ~450 files, dominated by map[string]any (~2,715 hits, 448 files, some overlap with plain any).
• Untyped string/numeric constants (non-test): 392 across 182 files, concentrated in pkg/constants/*.go (90 of the 392).

Category 1: any usage is mostly legitimate — the codebase already knows this

gh-aw compiles arbitrary YAML/JSON workflow frontmatter, JSON Schema documents, GitHub Actions step with: blocks, and MCP registry payloads — domains that are inherently dynamic before validation. Most map[string]any/[]any usage sits in parser/schema/YAML-conversion code (pkg/parser/import_field_extractor.go, pkg/parser/schema_suggestions.go, pkg/parser/mcp.go, pkg/workflow/frontmatter_serialization.go, etc.), where a dynamic container is the correct representation, not a shortcut.

The team is already aware of this: pkg/workflow/tools_types.go:14 documents ToolsConfig explicitly as "a structured alternative to the pervasive map[string]any pattern", with ParseToolsConfig/ToMap bridging functions for legacy callers. That's the right shape for future reductions — I'd extend the same treatment to other map[string]any bridge points rather than treating the raw count as a problem to zero out.

Impact: Low as a whole (working as intended), but worth targeted follow-up on the items below.

Category 2: Concrete, worth-fixing any fields

Example 1: GitHubReposScope

• Location: pkg/workflow/tools_types.go:306
• Current: type GitHubReposScope any // string or []any (YAML-parsed arrays are []any)
• Issue: every caller has to type-switch on string vs []any to use this value.
• Suggested fix: give it a real type with a custom UnmarshalYAML that normalizes both YAML shapes into []string internally:

  type GitHubReposScope []string
  
  func (s *GitHubReposScope) UnmarshalYAML(unmarshal func(any) error) error {
      var single string
      if err := unmarshal(&single); err == nil {
          *s = GitHubReposScope{single}
          return nil
      }
      var many []string
      if err := unmarshal(&many); err != nil {
          return err
      }
      *s = GitHubReposScope(many)
      return nil
  }

• Benefit: removes the type assertion from every call site permanently.

Example 2: Raw any fields

• Locations: pkg/workflow/tools_types.go:509, 515, 522
• Current: three separate Raw any \yaml:"-"`` fields.
• Suggested fix: now that ToolsConfig exists as the typed replacement for the map-based representation, check whether each Raw field's actual runtime type is narrow enough (e.g., always map[string]any from a specific parse step) to replace with a concrete struct or a shared RawToolPayload type. This needs a quick audit of assignment sites before committing to a type, so treat as a "look, then fix" task rather than a blind rename.

Category 3: Untyped constants — the pattern to fix is right there in the same package

pkg/constants already defines and documents semantic string types for exactly this purpose (JobName, StepID, MCPServerID, CommandPrefix, WorkflowID, LineLength), with a comment block explicitly explaining the convention:

// Semantic types for measurements and identifiers
// ...
// These type aliases provide meaningful names for primitive types, improving code clarity
// and type safety.

But in the same files, dozens of related constants are still bare untyped strings, e.g.:

// pkg/constants/job_constants.go
const SafeOutputArtifactName = "safe-output"
const AgentOutputArtifactName = "agent-output"
const AgentArtifactName = "agent"
const EvalsArtifactName = "evals"
const AgentOutputFilename = "agent_output.json"
const SafeOutputsFilename = "safeoutputs.jsonl"
const GraderManifestFilename = "grader_manifest.json"

// pkg/constants/constants.go
const AWFDefaultCommand = "awf"
const AWFProxyLogsDir = "/tmp/gh-aw/sandbox/firewall/logs"
const AWFConfigFilePath = "/tmp/gh-aw/awf-config.json"

Suggested fix: introduce ArtifactName, Filename, and FilePath semantic types alongside the existing ones and retype these constants:

type ArtifactName string
type Filename string
type FilePath string

const SafeOutputArtifactName ArtifactName = "safe-output"
const AgentOutputFilename    Filename     = "agent_output.json"
const AWFProxyLogsDir        FilePath     = "/tmp/gh-aw/sandbox/firewall/logs"

Impact: Medium — this isn't introducing a new idea to the codebase, it's finishing one the team already started. It would also make it a compile error to accidentally pass a Filename where a FilePath or ArtifactName was expected, which today are all interchangeable bare strings.
Estimated effort: 3-4 hours (mostly mechanical retyping + fixing call sites that build paths via string concatenation).

Everywhere else, the 392 untyped constants are spread thinly (1-6 per file) across CLI/parser/workflow code and are mostly one-off sentinel strings or magic numbers local to a single file — lower priority than the concentrated pkg/constants cluster above.

---

Refactoring Recommendations

Priority 1: Extend the existing semantic-type convention to remaining pkg/constants strings

Steps:

1. Add ArtifactName, Filename, FilePath types to pkg/constants/constants.go next to the existing semantic types.
2. Retype the ~90 untyped constants in pkg/constants/job_constants.go and constants.go that are clearly artifact names, filenames, or paths.
3. Fix compile errors at call sites (mostly string conversions or comparisons).
4. Run the full test suite.

Estimated effort: 3-4 hours
Impact: Medium — closes a gap in a convention the team already committed to.

Priority 2: Consolidate the AWF/frontmatter bounded-query config duplication (Cluster 4)

Steps:

1. Decide between dual-tagged single struct vs. generator function.
2. Update pkg/workflow/tools_types.go and pkg/workflow/awf_config.go accordingly.
3. Update tests asserting on both shapes.

Estimated effort: 2-3 hours
Impact: Medium — removes a manual-sync burden the code comments already flag as fragile.

Priority 3: Rename cli.Finding to avoid collision with scanfindings.Finding (Cluster 2)

Steps:

1. Rename the type and its constructors in pkg/cli/audit_report.go.
2. Update references within pkg/cli.

Estimated effort: <1 hour
Impact: Low — readability only, no functional risk.

Priority 4 (optional, needs an audit first): Type GitHubReposScope and review the three Raw any fields

Estimated effort: 2-3 hours (mostly the audit of Raw call sites)
Impact: Low-medium — removes a handful of type assertions.

---

Implementation Checklist

• Add ArtifactName/Filename/FilePath semantic types to pkg/constants and retype existing constants
• Consolidate BoundedQueriesConfig/AWFBoundedQueriesConfig field definitions
• Rename cli.Finding → cli.AuditFinding
• Give GitHubReposScope a real type with custom YAML unmarshaling
• Audit the three Raw any fields in pkg/workflow/tools_types.go for narrowing
• Optional: discuss whether AWFNetworkConfig/SRTNetworkConfig and their filesystem counterparts should share a base type
• Run full test suite after each change

Analysis Metadata

• Total Go Files Analyzed: 1,317 (non-test, under pkg/)
• Total Type Definitions: 1,135
• Same-name duplicate clusters: 7 (2 false positives, 1 collision, 1 confirmed-good pattern, 2 near-duplicate families worth consolidating, 1 low-priority testdata artifact not detailed above)
• Untyped Usage Locations: ~450 files with any/map[string]any, 392 untyped constants across 182 files
• Detection Method: Serena project activation + structural grep-based clustering and targeted file reads (heavier scripted text-processing was unavailable in this sandbox, so sampling favored high-signal locations — pkg/constants, pkg/workflow config types — over exhaustive per-file review)
• Analysis Date: 2026-08-25

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

• expires on Aug 26, 2026, 3:39 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 #56016.