You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I scanned the non-test Go files under pkg/ (~1,250 files across pkg/workflow, pkg/cli, pkg/parser, pkg/types, and ~25 smaller utility packages) for two things: types that are defined more than once, and code that's using interface{}/any/untyped constants where a concrete type would catch bugs at compile time instead of runtime.
The good news first: this codebase already shows real anti-duplication discipline. Several structs that would have been flagged (MCPServerConfig variants, MCPServerStats variants, missing-tool/data summaries) have already been refactored to share embedded base types, with comments explicitly calling out "eliminate duplication." Likewise, production interface{}/any usage is dominated by legitimate dynamic YAML/JSON frontmatter parsing (map[string]any) rather than laziness -- most of it is exactly as narrow as the underlying data model allows.
That said, I did find 5 duplicate type clusters (1 exact, 2 near, 2 semantic) and a handful of genuinely fixable untyped spots -- most interestingly a time.Duration-shaped constant that's stored as a bare int of "minutes" while every sibling timeout in pkg/constants is a typed time.Duration, and two any fields (RunID, RunNumber) in pkg/cli/logs_models.go that are int64 everywhere else in the same package. Both are small, low-risk, high-clarity fixes.
Interface declarations scanned: ~29 (no cross-package name collisions found)
Named/alias type declarations scanned: ~80 (no cross-package collisions found)
Duplicate clusters found: 5
Exact duplicates: 1
Near duplicates: 2
Semantic duplicates: 2
Method: grepped for ^type \w+ struct, ^type \w+ interface, and ^type \w+ (string|int|bool|...) across pkg/**/*.go (excluding _test.go), grouped by name across packages, then read field lists for every 2+ occurrence cluster. Sampled most heavily in pkg/cli (~450 struct decls) and pkg/workflow (~400 struct decls).
Byte-identical, gated by mutually exclusive build tags so they never compile together -- a legitimate WASM-stub pattern, but nothing forces the two copies to stay in sync if a field is ever added.
Recommendation: Move the struct definition itself to a small shared, build-tag-free file (e.g. repository_features_types.go) that both build variants import, leaving only the behavior (the validation logic) behind the build tags. Estimated effort: 15 minutes | Benefits: Removes one manual-sync footgun.
Cluster 2: ProgressBar -- Near duplicate (build-tag mirror)
Occurrences: 2 | Impact: Low -- same risk pattern as Cluster 1
Locations:
pkg/console/progress.go:31 (native)
pkg/console/progress_wasm.go:9 (WASM stub)
Native carries progress progress.Model, total/current int64, indeterminate bool, updateCount int64, ttyCheck func() bool; the WASM stub drops the bubbletea-specific progress/ttyCheck fields but keeps the same 4 core fields and public API (NewProgressBar, Update).
Recommendation: Extract the 4 shared fields into an embedded progressState struct used by both variants, so the "core" state can't drift. Estimated effort: 20 minutes | Benefits: Single source of truth for shared fields.
Cluster 3: SpinnerWrapper -- Near duplicate (build-tag mirror)
Both are ad-hoc partial models of a gh pr list --json ... result, parsed independently for two different commands in the same package, with overlapping fields (Number, Title).
Recommendation:
Define one PullRequest type in pkg/cli (or pkg/types) as the superset of both field sets
Update pr_command.go and pr_automerge.go to populate/consume the shared type
Estimated effort: 1-2 hours | Benefits: One source of truth for "what a PR looks like" in this package, easier to extend later (e.g. adding a field once instead of twice)
Same purpose (per-tool aggregated call stats for console/report rendering), overlapping fields (MaxOutputSize, MaxDuration, a call-count field). MCPToolSummary reads like an evolved superset of ToolUsageSummary for the MCP-aware path that never got merged back.
Recommendation: Consolidate onto MCPToolSummary's shape (or a shared embedded base, following the pattern already used for MCPServerStatsBase/AggregatedSummaryBase elsewhere in this package) and have both report paths use it. Estimated effort: 2-3 hours | Benefits: One tool-stats model instead of two independently-evolving ones.
Already resolved -- not flagged: MCPServerConfig/RegistryMCPServerConfig/BaseMCPServerConfig and MCPServerStats/MCPServerHealthDetail/MCPServerCrossRunHealth already share embedded base types (types.BaseMCPServerConfig, MCPServerStatsBase) with comments confirming this was a deliberate de-duplication. Good precedent to follow for Clusters 4-5.
Untyped Usages
Summary Statistics
Production interface{} usages (old syntax): effectively 0 -- 15 of 16 raw hits are in linter testdata fixtures, not real logic (codebase has already migrated to any)
any in map[string]any form: 250+ files, dominant pattern for YAML/JSON frontmatter parsing
Bare any struct fields (not map-shaped): ~30, mostly legitimately polymorphic per YAML union types
Bare any in function params/returns: ~150+, overwhelmingly map[string]any config-parsing helpers
Untyped numeric constants: ~55 found, ~6 recommended for change
Untyped string constants: 100+ found, ~2 recommended for change (rest are legitimate single-value literals like image refs/paths)
Total items recommended for change: ~18
Method: grepped for interface{}/any in signatures, struct fields, and map values, then verified actual call-site types before recommending a fix (not guessing). Focused on pkg/workflow and pkg/cli (the two largest packages), plus pkg/constants, pkg/types, pkg/parser.
Category 1: any fields that are always a concrete type in practice
Impact: High -- these are the clearest "should just be a real type" cases, since call-site evidence shows they're never actually polymorphic.
Example 1: AwInfo.RunID / RunNumber
Location: pkg/cli/logs_models.go:358-359
Current: RunID any (json:"run_id,omitempty") / RunNumber any (json:"run_number,omitempty")
Actual usage: Every other RunID field in the same package (pkg/cli/logs_models.go:123,280, pkg/cli/audit_cross_run.go, pkg/cli/logs_episode.go) is int64 -- GitHub Actions run IDs/numbers are always numeric.
Benefits: Consistent with the rest of the package; removes an unnecessary type assertion wherever these fields are read.
Example 2: ServerStatus / ArgumentType enums hiding as bare strings
Location: pkg/cli/mcp_registry_types.go:63,71,100 (struct Type string fields), constants at :108-109 (StatusActive/StatusInactive) and :114-115 (ArgumentTypePositional/ArgumentTypeNamed)
Current: plain string fields compared against untyped string constants
Suggested fix: introduce type ServerStatus string and type ArgumentType string, mirroring the existing pkg/constants.EngineName pattern already used for engine identifiers, and retype both the constants and the struct fields.
Benefits: arg.Type == ArgumentTypePositional (used at pkg/cli/mcp_registry.go:171,180) becomes compile-time checked against a closed enum instead of arbitrary strings.
Example 3: service_ports.go field narrower than its only use
Location: pkg/workflow/service_ports.go:86
Current: Ports any, immediately type-asserted to []any at every call site (pkg/workflow/service_ports.go:144)
Suggested fix: declare the field as []any directly, removing one runtime assertion + ok check.
Benefits: Minor but free -- no behavior change, one less assertion.
Category 2: Untyped constants that should share a type with their consumer
Impact: Medium-high -- these create a type mismatch between a constant and the field/parameter it feeds, which is exactly where "wrong unit" or "wrong enum value" bugs hide.
Example 1: Timeout stored as bare minutes instead of time.Duration
Current (pkg/cli/run_workflow_execution.go:25):
constworkflowCompletionWaitTimeoutMinutes=6*60// bare int, ambiguous unitfuncWaitForWorkflowCompletion(ctx, repoSlug, runIDstring, timeoutMinutesint, verbosebool) error
Suggested -- matches every sibling timeout in pkg/constants/constants.go (DefaultAgenticWorkflowTimeout, DefaultToolTimeout, DefaultHTTPClientTimeout are all time.Duration):
Locations: pkg/cli/run_workflow_execution.go:25, called from pkg/cli/pr_automerge.go:120 Benefits: Matches the established time.Duration convention used everywhere else in pkg/constants; eliminates "is this seconds or minutes?" ambiguity for future readers.
Example 2: Cache integrity default untyped vs. its typed consumer
Current (pkg/workflow/cache_integrity.go:17):
constdefaultCacheIntegrityLevel="none"// bare stringfunccacheIntegrityLevel() string { ... } // returns bare string
Suggested -- GitHubIntegrityLevel already exists at pkg/workflow/tools_types.go:287:
Locations: pkg/workflow/cache_integrity.go:17,166, sibling field at pkg/workflow/tools_types.go:287,325 Benefits: Closes a type gap between a default constant and the exact enum-typed field (GitHubToolConfig.MinIntegrity) it exists to default.
Not recommended for change (ruled out with reasoning): defaultSafeOutputsTimeoutMinutes = 45 in pkg/workflow/compiler_safe_outputs_job.go:513 feeds a GitHub Actions YAML timeout-minutes: field, which is inherently minutes-denominated in the Actions schema itself -- an untyped int is the right call there. Most pkg/constants sizing constants (DefaultMaxRuns, DefaultMCPGatewayPayloadSizeThreshold, per-function maxRetries/maxDepth) and the 100+ single-value string constants (image refs, file paths, command names) have no closed value set or enum-like semantics, so typing them would add ceremony without safety.
Category 3: map[string]any frontmatter/config parsing -- largely fine as-is
The overwhelming majority of any usage (250+ files) is func parseXConfig(m map[string]any) *XConfig-style helpers pulling one key out of already-yaml.Unmarshal'd frontmatter, plus MCP/JSON-RPC fields (like rpcRequestPayload.ID any) that are legitimately polymorphic per protocol spec, plus documented multi-form YAML fields (pkg/workflow/frontmatter_types.go's Headers, Endpoint, RunsOn, Imports, etc., each already parsed into a narrower type downstream). Retyping these would require a full frontmatter schema type system Go doesn't make easy (no discriminated unions) for marginal benefit -- not recommended.
Recommendations, Prioritized
Priority 1: RunID/RunNumber and time.Duration timeout fixes
Small, mechanical, zero ambiguity about the right type -- the surrounding code already establishes the pattern (see pkg/cli/logs_models.go and pkg/constants/constants.go). Estimated effort: ~1 hour total | Impact: High confidence, low risk
Mirrors the existing EngineName enum pattern already in the codebase -- a template to copy, not a new idiom to invent. Estimated effort: 2-3 hours | Impact: Medium -- prevents invalid string comparisons for status/arg-kind/integrity checks
Priority 3: Consolidate PRInfo/PullRequest and ToolUsageSummary/MCPToolSummary
Follow the precedent already set by BaseMCPServerConfig/MCPServerStatsBase in the same package. Estimated effort: 3-5 hours combined | Impact: Medium -- one model per concept instead of two
Low urgency since they're not currently divergent, but worth fixing opportunistically next time one of these files is touched. Estimated effort: ~1 hour combined | Impact: Low -- removes manual-sync risk
Implementation Checklist
Retype AwInfo.RunID/RunNumber to int64 (pkg/cli/logs_models.go)
Retype workflowCompletionWaitTimeoutMinutes to a time.Duration constant and update WaitForWorkflowCompletion's signature
Retype defaultCacheIntegrityLevel and cacheIntegrityLevel() to GitHubIntegrityLevel
Introduce ServerStatus/ArgumentType named types in pkg/cli/mcp_registry_types.go
Consolidate PRInfo/PullRequest in pkg/cli
Consolidate ToolUsageSummary/MCPToolSummary in pkg/cli
Opportunistically de-duplicate the 3 build-tag mirror structs when next touched
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
Executive Summary
I scanned the non-test Go files under
pkg/(~1,250 files acrosspkg/workflow,pkg/cli,pkg/parser,pkg/types, and ~25 smaller utility packages) for two things: types that are defined more than once, and code that's usinginterface{}/any/untyped constants where a concrete type would catch bugs at compile time instead of runtime.The good news first: this codebase already shows real anti-duplication discipline. Several structs that would have been flagged (
MCPServerConfigvariants,MCPServerStatsvariants, missing-tool/data summaries) have already been refactored to share embedded base types, with comments explicitly calling out "eliminate duplication." Likewise, productioninterface{}/anyusage is dominated by legitimate dynamic YAML/JSON frontmatter parsing (map[string]any) rather than laziness -- most of it is exactly as narrow as the underlying data model allows.That said, I did find 5 duplicate type clusters (1 exact, 2 near, 2 semantic) and a handful of genuinely fixable untyped spots -- most interestingly a
time.Duration-shaped constant that's stored as a bareintof "minutes" while every sibling timeout inpkg/constantsis a typedtime.Duration, and twoanyfields (RunID,RunNumber) inpkg/cli/logs_models.gothat areint64everywhere else in the same package. Both are small, low-risk, high-clarity fixes.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
Method: grepped for
^type \w+ struct,^type \w+ interface, and^type \w+ (string|int|bool|...)acrosspkg/**/*.go(excluding_test.go), grouped by name across packages, then read field lists for every 2+ occurrence cluster. Sampled most heavily inpkg/cli(~450 struct decls) andpkg/workflow(~400 struct decls).Cluster 1:
RepositoryFeatures-- Exact duplicate (build-tag mirror)Occurrences: 2 | Impact: Low-medium -- manual-sync risk, not currently divergent
Locations:
pkg/workflow/repository_features_validation.go:83(build tag!js && !wasm)pkg/workflow/repository_features_validation_wasm.go:31(build tagjs || wasm)Byte-identical, gated by mutually exclusive build tags so they never compile together -- a legitimate WASM-stub pattern, but nothing forces the two copies to stay in sync if a field is ever added.
Recommendation: Move the struct definition itself to a small shared, build-tag-free file (e.g.
repository_features_types.go) that both build variants import, leaving only the behavior (the validation logic) behind the build tags.Estimated effort: 15 minutes | Benefits: Removes one manual-sync footgun.
Cluster 2:
ProgressBar-- Near duplicate (build-tag mirror)Occurrences: 2 | Impact: Low -- same risk pattern as Cluster 1
Locations:
pkg/console/progress.go:31(native)pkg/console/progress_wasm.go:9(WASM stub)Native carries
progress progress.Model,total/current int64,indeterminate bool,updateCount int64,ttyCheck func() bool; the WASM stub drops the bubbletea-specificprogress/ttyCheckfields but keeps the same 4 core fields and public API (NewProgressBar,Update).Recommendation: Extract the 4 shared fields into an embedded
progressStatestruct used by both variants, so the "core" state can't drift.Estimated effort: 20 minutes | Benefits: Single source of truth for shared fields.
Cluster 3:
SpinnerWrapper-- Near duplicate (build-tag mirror)Occurrences: 2 | Impact: Low
Locations:
pkg/console/spinner.go:92(wrapstea.Program+spinnerModel)pkg/console/spinner_wasm.go:12(one-fieldenabled boolstub)Same exported method set (
Start,Stop,StopWithMessage,UpdateMessage,IsEnabled) on both -- deliberate mirror, same pattern/risk as Clusters 1-2.Recommendation: No urgent action needed beyond awareness; if touched again, consider the same shared-state extraction as Cluster 2.
Cluster 4:
PRInfo/PullRequest-- Semantic duplicateType: Semantic duplicate | Occurrences: 2 | Impact: Medium -- same package, overlapping GitHub PR model
Locations:
pkg/cli/pr_command.go:28--PRInfo{Number, Title, Body, State, HeadSHA, BaseBranch, HeadBranch, SourceRepo, TargetRepo, AuthorLogin}pkg/cli/pr_automerge.go:21--PullRequest{Number, Title, IsDraft, Mergeable, CreatedAt, UpdatedAt}Both are ad-hoc partial models of a
gh pr list --json ...result, parsed independently for two different commands in the same package, with overlapping fields (Number,Title).Recommendation:
PullRequesttype inpkg/cli(orpkg/types) as the superset of both field setspr_command.goandpr_automerge.goto populate/consume the shared typeCluster 5:
ToolUsageSummary/MCPToolSummary-- Near/semantic duplicateType: Near duplicate | Occurrences: 2 | Impact: Medium
Locations:
pkg/cli/logs_report_tools.go:13--ToolUsageSummary{Name, TotalCalls, Runs, MaxOutputSize, MaxDuration}pkg/cli/audit_report.go:174--MCPToolSummary{ServerName, ToolName, CallCount, TotalInputSize, TotalOutputSize, MaxInputSize, MaxOutputSize, AvgDuration, MaxDuration, ErrorCount}Same purpose (per-tool aggregated call stats for console/report rendering), overlapping fields (
MaxOutputSize,MaxDuration, a call-count field).MCPToolSummaryreads like an evolved superset ofToolUsageSummaryfor the MCP-aware path that never got merged back.Recommendation: Consolidate onto
MCPToolSummary's shape (or a shared embedded base, following the pattern already used forMCPServerStatsBase/AggregatedSummaryBaseelsewhere in this package) and have both report paths use it.Estimated effort: 2-3 hours | Benefits: One tool-stats model instead of two independently-evolving ones.
Already resolved -- not flagged:
MCPServerConfig/RegistryMCPServerConfig/BaseMCPServerConfigandMCPServerStats/MCPServerHealthDetail/MCPServerCrossRunHealthalready share embedded base types (types.BaseMCPServerConfig,MCPServerStatsBase) with comments confirming this was a deliberate de-duplication. Good precedent to follow for Clusters 4-5.Untyped Usages
Summary Statistics
interface{}usages (old syntax): effectively 0 -- 15 of 16 raw hits are in linter testdata fixtures, not real logic (codebase has already migrated toany)anyinmap[string]anyform: 250+ files, dominant pattern for YAML/JSON frontmatter parsinganystruct fields (not map-shaped): ~30, mostly legitimately polymorphic per YAML union typesanyin function params/returns: ~150+, overwhelminglymap[string]anyconfig-parsing helpersMethod: grepped for
interface{}/anyin signatures, struct fields, and map values, then verified actual call-site types before recommending a fix (not guessing). Focused onpkg/workflowandpkg/cli(the two largest packages), pluspkg/constants,pkg/types,pkg/parser.Category 1:
anyfields that are always a concrete type in practiceImpact: High -- these are the clearest "should just be a real type" cases, since call-site evidence shows they're never actually polymorphic.
Example 1:
AwInfo.RunID/RunNumberpkg/cli/logs_models.go:358-359RunID any(json:"run_id,omitempty") /RunNumber any(json:"run_number,omitempty")RunIDfield in the same package (pkg/cli/logs_models.go:123,280,pkg/cli/audit_cross_run.go,pkg/cli/logs_episode.go) isint64-- GitHub Actions run IDs/numbers are always numeric.Example 2:
ServerStatus/ArgumentTypeenums hiding as bare stringspkg/cli/mcp_registry_types.go:63,71,100(structType stringfields), constants at:108-109(StatusActive/StatusInactive) and:114-115(ArgumentTypePositional/ArgumentTypeNamed)stringfields compared against untyped string constantstype ServerStatus stringandtype ArgumentType string, mirroring the existingpkg/constants.EngineNamepattern already used for engine identifiers, and retype both the constants and the struct fields.arg.Type == ArgumentTypePositional(used atpkg/cli/mcp_registry.go:171,180) becomes compile-time checked against a closed enum instead of arbitrary strings.Example 3:
service_ports.gofield narrower than its only usepkg/workflow/service_ports.go:86Ports any, immediately type-asserted to[]anyat every call site (pkg/workflow/service_ports.go:144)[]anydirectly, removing one runtime assertion +okcheck.Category 2: Untyped constants that should share a type with their consumer
Impact: Medium-high -- these create a type mismatch between a constant and the field/parameter it feeds, which is exactly where "wrong unit" or "wrong enum value" bugs hide.
Example 1: Timeout stored as bare minutes instead of
time.DurationCurrent (
pkg/cli/run_workflow_execution.go:25):Suggested -- matches every sibling timeout in
pkg/constants/constants.go(DefaultAgenticWorkflowTimeout,DefaultToolTimeout,DefaultHTTPClientTimeoutare alltime.Duration):Locations:
pkg/cli/run_workflow_execution.go:25, called frompkg/cli/pr_automerge.go:120Benefits: Matches the established
time.Durationconvention used everywhere else inpkg/constants; eliminates "is this seconds or minutes?" ambiguity for future readers.Example 2: Cache integrity default untyped vs. its typed consumer
Current (
pkg/workflow/cache_integrity.go:17):Suggested --
GitHubIntegrityLevelalready exists atpkg/workflow/tools_types.go:287:Locations:
pkg/workflow/cache_integrity.go:17,166, sibling field atpkg/workflow/tools_types.go:287,325Benefits: Closes a type gap between a default constant and the exact enum-typed field (
GitHubToolConfig.MinIntegrity) it exists to default.Not recommended for change (ruled out with reasoning):
defaultSafeOutputsTimeoutMinutes = 45inpkg/workflow/compiler_safe_outputs_job.go:513feeds a GitHub Actions YAMLtimeout-minutes:field, which is inherently minutes-denominated in the Actions schema itself -- an untyped int is the right call there. Mostpkg/constantssizing constants (DefaultMaxRuns,DefaultMCPGatewayPayloadSizeThreshold, per-functionmaxRetries/maxDepth) and the 100+ single-value string constants (image refs, file paths, command names) have no closed value set or enum-like semantics, so typing them would add ceremony without safety.Category 3:
map[string]anyfrontmatter/config parsing -- largely fine as-isThe overwhelming majority of
anyusage (250+ files) isfunc parseXConfig(m map[string]any) *XConfig-style helpers pulling one key out of already-yaml.Unmarshal'd frontmatter, plus MCP/JSON-RPC fields (likerpcRequestPayload.ID any) that are legitimately polymorphic per protocol spec, plus documented multi-form YAML fields (pkg/workflow/frontmatter_types.go'sHeaders,Endpoint,RunsOn,Imports, etc., each already parsed into a narrower type downstream). Retyping these would require a full frontmatter schema type system Go doesn't make easy (no discriminated unions) for marginal benefit -- not recommended.Recommendations, Prioritized
Priority 1:
RunID/RunNumberandtime.Durationtimeout fixesSmall, mechanical, zero ambiguity about the right type -- the surrounding code already establishes the pattern (see
pkg/cli/logs_models.goandpkg/constants/constants.go).Estimated effort: ~1 hour total | Impact: High confidence, low risk
Priority 2:
ServerStatus/ArgumentType/GitHubIntegrityLevelconstant typingMirrors the existing
EngineNameenum pattern already in the codebase -- a template to copy, not a new idiom to invent.Estimated effort: 2-3 hours | Impact: Medium -- prevents invalid string comparisons for status/arg-kind/integrity checks
Priority 3: Consolidate
PRInfo/PullRequestandToolUsageSummary/MCPToolSummaryFollow the precedent already set by
BaseMCPServerConfig/MCPServerStatsBasein the same package.Estimated effort: 3-5 hours combined | Impact: Medium -- one model per concept instead of two
Priority 4: Build-tag mirror structs (
RepositoryFeatures,ProgressBar,SpinnerWrapper)Low urgency since they're not currently divergent, but worth fixing opportunistically next time one of these files is touched.
Estimated effort: ~1 hour combined | Impact: Low -- removes manual-sync risk
Implementation Checklist
AwInfo.RunID/RunNumbertoint64(pkg/cli/logs_models.go)workflowCompletionWaitTimeoutMinutesto atime.Durationconstant and updateWaitForWorkflowCompletion's signaturedefaultCacheIntegrityLevelandcacheIntegrityLevel()toGitHubIntegrityLevelServerStatus/ArgumentTypenamed types inpkg/cli/mcp_registry_types.goPRInfo/PullRequestinpkg/cliToolUsageSummary/MCPToolSummaryinpkg/cliAnalysis Metadata
pkg/workflow,pkg/cli,pkg/console,pkg/constants,pkg/types,pkg/parserAll reactions