[typist] Typist Report: Type Duplication and Untyped Usage Audit (pkg/) #52283
Closed
Replies: 1 comment
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #52484. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Executive Summary
This report is an automated type-consistency sweep of the
gh-awGo codebase (primary focus:pkg/, excluding_test.gofiles andpkg/linters/testdata/internals). It looked for two classes of issue: duplicated type definitions (same or near-identical struct/interface shapes defined in more than one place) and untyped usages (interface{}/anyor bare untyped constants standing in for a type that already exists, or should).Headline findings:
SafeOutputTargetConfig,SafeOutputFilterConfig,BaseSafeOutputConfig), but roughly a third of the ~34*Configstructs inpkg/workflowdon't use it — including three structs with an actual field-shadowing bug (GitHubTokenparsed twice, dead field left unread).pkg/cli/logs_models.go:ProcessedRun,DownloadResult, andRunSummaryshare ~13-17 fields with no shared base type.pkg/cli/audit_comparison.go:AuditComparisonIntDelta/AuditComparisonStringDeltadiffer only in value type.GitHubIntegrityLevelenum (pkg/workflow/tools_types.go) is correctly used in one file but re-expressed as raw string literals in two others, and as astring-typed constant in a third.ProgressBar,SpinnerWrapper,RepositoryFeatures) were investigated and confirmed legitimate(go/redacted):buildwasm/native platform splits — no action needed. Flagging them here to show they were checked, not missed.any/interface{}— the vast majority of hits are idiomatic YAML/JSON parsing (map[string]any) or genuine type-switch handling of heterogeneous values. Only one clear "should be concretely typed" function signature was found.No files were modified — this is read-only analysis.
Summary Statistics
.gofiles scanned (pkg/)Full Analysis Report
1. Duplicated Type Definitions
1.1
ProcessedRun/DownloadResult/RunSummary— near-duplicate result structsLocation:
pkg/cli/logs_models.go:97,:252,:225Three structs carry the same ~13-17 field "processed workflow run" shape, hand-duplicated instead of sharing a base:
For contrast, this same file already contains two correctly-factored examples of the pattern this cluster is missing:
ReportProvenance(line 119) is embedded byMissingToolReport,NoopReport,MissingDataReport,MCPFailureReport,SkillActivation.AggregatedSummaryBase(line 171) is embedded byMissingToolSummary,MissingDataSummary.Recommendation: extract a
ProcessedRunBase(or similar) struct carrying the ~13 shared fields, and haveProcessedRun,DownloadResult,RunSummaryembed it plus their own extras. This mirrors theReportProvenance/AggregatedSummaryBasepattern already proven out in the same file.1.2
AuditComparisonIntDelta/AuditComparisonStringDelta— generics candidateLocation:
pkg/cli/audit_comparison.go:45,:51Structurally identical except the value type. Both are composed into
AuditComparisonDeltaalongsideAuditComparisonMCPFailureDelta.Recommendation:
then use
AuditComparisonDelta[int]andAuditComparisonDelta[string]at call sites. Low risk — no external serialization concerns apparent beyond straightforward field access.1.3 Confirmed not duplicates — platform-split types (no action needed)
Three pairs were flagged by initial name-collision scanning and then verified structurally distinct-but-related via
(go/redacted):buildtags:pkg/console/progress.go((go/redacted):build !js && !wasm)pkg/console/progress_wasm.go((go/redacted):build js || wasm)ProgressBarprogress progress.Model,ttyCheck; wasm has a reduced field set (total,current,indeterminate,updateCount).pkg/console/spinner.gopkg/console/spinner_wasm.goSpinnerWrapperprogram *tea.Program,mu sync.Mutex,wg sync.WaitGroup; wasm has onlyenabled boolwith no-op methods.pkg/workflow/repository_features_validation.gopkg/workflow/repository_features_validation_wasm.goRepositoryFeaturesHasDiscussions,HasIssues) are deliberately kept byte-identical per the wasm file's own comment, for cross-platform API compatibility; only method bodies diverge.These are called out explicitly so it's clear they were checked, not overlooked.
1.4 Safe-output
*Configstructs — inconsistent use of an existing DRY layerpkg/workflowalready defines shared sub-structs for common config fields —BaseSafeOutputConfig,SafeOutputTargetConfig(Target,TargetRepoSlug,AllowedRepos),SafeOutputFilterConfig(RequiredLabels,RequiredTitlePrefix,TitlePrefix),SafeOutputAllowBlockConfig,UpdateEntityConfig— and roughly two-thirds of the ~34 top-level safe-output config structs embed them correctly. The remaining third hand-redeclares the same fields instead.Cluster A — actual bug, not just duplication.
GitHubToken string(yaml:"github-token,omitempty") is redeclared alongside an embeddedBaseSafeOutputConfigthat already has this field, in:pkg/workflow/create_project.go:10pkg/workflow/update_project.go:27pkg/workflow/create_project_status_update.go:12Because Go field-shadowing makes the outer field win, and each parser (
create_project.go:24-34,update_project.go:42-52,create_project_status_update.go:23-32) callsparseBaseSafeOutputConfig(which fills the embedded copy) and then re-parses"github-token"into the outer field,BaseSafeOutputConfig.GitHubTokenis set but never read in these three structs. Currently harmless (both copies get the same value), but it's dead code masking a latent divergence risk if either parse path is edited independently in the future.Cluster B —
SafeOutputTargetConfignot used where it should be.TargetRepoSlug/AllowedRepos(and oftenTarget) are hand-declared instead of embeddingSafeOutputTargetConfig, in 8 structs:create_issue.go:21-22,create_discussion.go:21-22,create_pull_request.go:47,51,add_comment.go:18-20,create_code_scanning_alert.go:18-19,push_to_pull_request_branch.go:16,24,29,dispatch_workflow.go:16,18,update_project.go:29-30.Cluster C —
RequiredLabelsnot consolidated intoSafeOutputFilterConfig. 3 structs hand-declareRequiredLabels []string(yamlrequired-labels) with identical semantics to the ~14 structs that already embedSafeOutputFilterConfigfor this field:merge_pull_request.go:12(alsoRequiredTitlePrefixat line 14),push_to_pull_request_branch.go:18,update_issue.go:21.Cluster D — mixed bag, needs judgment, not a pure merge.
TitlePrefix/Labelsappear in two different senses across 6-8 structs: "prefix/labels to apply when creating" (create_issue.go,create_pull_request.go,create_discussion.go,create_project.go) vs. "prefix/labels used to filter/require" (push_to_pull_request_branch.go,update_issue.go— these two should fold into Cluster C/SafeOutputFilterConfiginstead). Recommend a new, separately-named shared struct (e.g.SafeOutputCreateLabelingConfig{TitlePrefix, Labels}) for the "creation" group rather than merging both senses into one field.Minor —
DispatchRepositoryToolConfig(dispatch_repository.go:18-21) hand-copies 4BaseSafeOutputConfigfields (Max,GitHubToken,GitHubApp,Staged) inside a map-value struct rather than embedding. Lowest priority since it's an isolated one-off, not a 3+ cluster.2. Untyped Usages
2.1
anyparameter that should be concretely typedpkg/workflow/args.go:39Both call sites (
pkg/workflow/mcp_renderer_github.go:84,:174) already holdgithubToolasmap[string]anybefore calling this — every sibling accessor inpkg/workflow/mcp_github_config.go(getGitHubType,getGitHubLockdown,getGitHubToolsets,getGitHubAllowedTools,getGitHubDockerImageVersion) already takesmap[string]anydirectly. This one function is the outlier still doing a needless runtime type assertion.2.2
GitHubIntegrityLevelenum re-expressed as raw stringsEnum definition:
pkg/workflow/tools_types.go:287-297(GitHubIntegrityNone,Unapproved,Approved,Merged).Used correctly in
pkg/workflow/tools_validation_github.go:165-169:Duplicated as plain strings in the same package,
pkg/workflow/tools_validation_github_integrity_reactions.go:23-35:Recommendation: retype both maps to
map[GitHubIntegrityLevel]boolusing the existing named constants — removes the risk of the string literals and the enum drifting apart silently.2.3
defaultCacheIntegrityLeveltyped as plainstringinstead ofGitHubIntegrityLevelpkg/workflow/cache_integrity.go:17cacheIntegrityLevel()(line 166) also returns plainstring, forcing an explicitstring(github.MinIntegrity)conversion at line 170 that wouldn't be needed if the return type wereGitHubIntegrityLevelto begin with.2.4 Minor/weaker candidates (lower priority, listed for completeness)
pkg/constants/constants.go:332—DefaultMaxDailyAICredits = "5000"is a string whileDefaultMaxAICredits/DefaultDetectionMaxAICredits(lines 325, 329) areint64. May be intentional (templated/env-var string) — verify before changing.pkg/constants/job_constants.go:260-261andpkg/constants/constants.go:618— untyped numeric constants (DefaultRateLimitMax,DefaultRateLimitWindow,MaxSymlinkDepth) with no shared enum family; single-use magnitudes, not worth a named type.2.5 Explicitly excluded from findings — idiomatic, not a problem
The pervasive use of
map[string]any(300+ occurrences across ~958 files) for YAML/JSON config parsing was reviewed and excluded — this is the idiomatic Go pattern for decoding heterogeneous, schema-flexible data (workflow frontmatter, MCP tool configs) and converting it to strong types would require either a large schema/validation library or hand-written unmarshalers with no clear benefit here. Likewise,console.FormField.Value any(pkg/console/console_types.go:50) is a deliberate generic-form-library field (holds a pointer to whatever the caller wants filled in) and type-switch-based accessors handlingint/float64/string/bool/map/[]anyfrom decoded YAML are correct, idiomatic uses ofany.Refactoring Recommendations
GitHubTokenfield re-declarations, rely on embeddedBaseSafeOutputConfigcreate_project.go,update_project.go,create_project_status_update.goSafeOutputTargetConfiginstead of hand-declaringTarget/TargetRepoSlug/AllowedReposcreate_issue.go,create_discussion.go,create_pull_request.go,add_comment.go,create_code_scanning_alert.go,push_to_pull_request_branch.go,dispatch_workflow.go,update_project.goSafeOutputFilterConfiginstead of hand-declaringRequiredLabels/RequiredTitlePrefixmerge_pull_request.go,push_to_pull_request_branch.go,update_issue.goProcessedRun/DownloadResult/RunSummarypkg/cli/logs_models.goReportProvenance/AggregatedSummaryBasepattern in same fileAuditComparisonDelta[T]to replaceInt/StringDeltapairpkg/cli/audit_comparison.goGitHubIntegrityLevel-shaped maps/constants fromstringto the named enumtools_validation_github_integrity_reactions.go,cache_integrity.gogetGitHubCustomArgsparameter fromanytomap[string]anypkg/workflow/args.goSafeOutputCreateLabelingConfigfor "labels/prefix to apply on create" (distinct from filter semantics)create_issue.go,create_pull_request.go,create_discussion.go,create_project.goBaseSafeOutputConfiginDispatchRepositoryToolConfiginstead of hand-copying 4 fieldsdispatch_repository.goImplementation Checklist
GitHubTokenfield-shadowing bug in 3 project-related configs (rejig docs #1)SafeOutputTargetConfig(Add workflow: githubnext/agentics/weekly-research #2)SafeOutputFilterConfig(Add workflow: githubnext/agentics/weekly-research #3)ProcessedRunBaseforpkg/cli/logs_models.go(Add workflow: githubnext/agentics/weekly-research #4)AuditComparisonDelta[T]generic, update call sites (Add workflow: githubnext/agentics/weekly-research #5)GitHubIntegrityLevel(add cli flag to guard dropping a agentic workflow instructinos file #6)getGitHubCustomArgssignature (Weekly Research Report: AI Workflow Automation Landscape and Market Opportunities - August 2025 #7)SafeOutputCreateLabelingConfig(Add workflow: githubnext/agentics/weekly-research #8, needs design discussion first)BaseSafeOutputConfiginDispatchRepositoryToolConfig(Weekly Research Report: AI Workflow Automation Landscape and Strategic Opportunities - August 2025 #9)ProgressBar,SpinnerWrapper,RepositoryFeatureswasm/native pairs (confirmed legitimate)Analysis Metadata
pkg/Go source files, excluding_test.goandpkg/linters/internals/testdata.gofilespkg/) followed by targeted structural review of name-collision candidates, plus dedicated passes for safe-output config duplication and untyped-usage auditAll reactions