[typist] Typist - Go Type Consistency Analysis #50840
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-08-07T12:18:03.032Z.
|
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.
Warning
Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.
What happened
The threat detection engine failed to produce results.
Review the workflow run logs for details.
🔤 Typist - Go Type Consistency Analysis
Analysis of repository: github/gh-aw
Executive Summary
I analyzed roughly 1,184 non-test
.gofiles underpkg/, focusing on the safe-output config structs inpkg/workflow, the codemod/CLI layer inpkg/cli, and the frontmatter parsing pipeline inpkg/parser. The headline finding is a big one: thefrontmatter map[string]anysignature is repeated 290 times across 128 files — it's the parameter type forUpdateWorkflowFrontmatter, every one of the ~40 codemods inpkg/cli, and most of theCompiler.extract*FromOnhelpers. A single namedFrontmattertype with typed accessor methods would collapse dozens of hand-rolled.(string)/.([]any)type assertions into one well-tested place.On the duplication side, the safe-output config family (
CreateIssuesConfig,CreatePullRequestsConfig,CreateDiscussionsConfig, etc.) already has shared embeddable structs (SafeOutputTargetConfig,SafeOutputFilterConfig) — but 11 structs still redeclareTargetRepoSlug/AllowedReposraw instead of embedding them, and 13 structs redeclare an identicalFooter *stringfield. These are quick, low-risk wins since the shared struct already exists and a couple of structs (CloseJobConfig,ListJobConfig) already show the correct embedding pattern to copy. Fixing these two clusters alone removes ~24 duplicate field declarations with no behavior change.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
Cluster 1:
Footer *stringfield duplicationType: Near duplicate
Occurrences: 13 (verified via grep)
Impact: High — identical field + tag repeated across the entire safe-output config family
Locations (representative):
pkg/workflow/create_pull_request.go:59pkg/workflow/create_issue.go:28pkg/workflow/create_discussion.go:28pkg/workflow/update_issue.go:18pkg/workflow/update_pull_request.go:20pkg/workflow/update_discussion.go:19pkg/workflow/update_release.go:12pkg/workflow/add_comment.go:27pkg/workflow/comment_memory.go:14pkg/workflow/submit_pr_review.go:20pkg/workflow/reply_to_pr_review_comment.go:15yaml:"footer,omitempty"tag)Definition Comparison:
Recommendation:
FooterConfigstruct (field:Footer *stringwith tagyaml:"footer,omitempty") next to the existingSafeOutputTargetConfig/SafeOutputFilterConfigpattern inpkg/workflow/safe_outputs_parser.goFooterConfigwith tagyaml:",inline"in each of the 13 structsCluster 2:
TargetRepoSlug/AllowedReposnot embeddingSafeOutputTargetConfigType: Near duplicate
Occurrences: 11 structs redeclare raw fields (verified via grep) instead of embedding
Impact: High — the shared type already exists, this is a pure migration
Locations:
pkg/workflow/create_pull_request.go:47pkg/workflow/create_issue.go:21pkg/workflow/create_discussion.go:21pkg/workflow/create_agent_session.go:13pkg/workflow/push_to_pull_request_branch.go:24pkg/workflow/create_code_scanning_alert.go:18pkg/workflow/comment_memory.go:11pkg/workflow/update_project.go:29pkg/workflow/add_comment.go:19pkg/workflow/create_pr_review_comment.go:15pkg/workflow/dispatch_workflow.go:15Canonical shared type (already exists):
pkg/workflow/safe_outputs_parser.go:9—SafeOutputTargetConfig { Target, TargetRepoSlug, AllowedRepos }Recommendation:
TargetRepoSlug string/AllowedRepos []stringfield pair by embeddingSafeOutputTargetConfig(tagyaml:",inline") — the patternCloseJobConfig/ListJobConfigalready use correctlyCluster 3:
TitlePrefixreused for two different conceptsType: Semantic duplicate
Occurrences: 8
Impact: Medium — same field name/tag means two different things depending on which struct you're in
Locations:
pkg/workflow/create_pull_request.go:36(default-title-prefix semantics)pkg/workflow/create_issue.go:14(default-title-prefix semantics)pkg/workflow/create_discussion.go:16(default-title-prefix semantics)pkg/workflow/create_project.go:12(default-title-prefix semantics)pkg/workflow/push_to_pull_request_branch.go:17(required-title-prefix validation semantics)pkg/workflow/missing_issue_reporting.go:20(prefix-for-generated-titles semantics)pkg/workflow/update_issue.go:19(deprecated alias forrequired-title-prefix)pkg/workflow/safe_outputs_parser.go:20—SafeOutputFilterConfig.TitlePrefix(deprecated filter alias)Recommendation:
DefaultTitlePrefixto disambiguate from the deprecated filter aliasupdate_issue.goembedSafeOutputFilterConfiginstead of redeclaring its ownTitlePrefixCluster 4:
AllowedLabels []stringfield duplicationType: Near duplicate
Occurrences: 5
Impact: Medium
Locations:
pkg/workflow/create_pull_request.go:39pkg/workflow/create_issue.go:17pkg/workflow/create_discussion.go:20pkg/workflow/update_discussion.go:18pkg/workflow/merge_pull_request.go:14(marked deprecated in favor ofrequired-labels)Recommendation:
AllowedLabels []stringto the existingSafeOutputFilterConfigshared struct (pkg/workflow/safe_outputs_parser.go:17) and embed it in these 5 structsCluster 5:
AuditComparisonIntDelta/AuditComparisonStringDeltaType: Near duplicate
Occurrences: 2 (+1 related slice-shaped variant)
Impact: Medium — same shape, different value type, in a Go 1.18+ codebase that could use generics
Locations:
pkg/cli/audit_comparison.go:45—AuditComparisonIntDelta { Before int; After int; Changed bool }pkg/cli/audit_comparison.go:51—AuditComparisonStringDelta { Before string; After string; Changed bool }Recommendation:
AuditComparisonDelta[T comparable] struct { Before, After T; Changed bool }Cluster 6:
PRInfovsPullRequest(pkg/cli)Type: Semantic duplicate
Occurrences: 2
Impact: Medium — both represent a GitHub PR fetched via
gh pr view, with non-overlapping fieldsLocations:
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 }Recommendation: Consider one shared
PullRequeststruct with the union of fields both call sites need, rather than two overlapping representations of the same GitHub entity in the same package.Cluster 7:
RepositoryFeatures— exact duplicate across build tagsType: Exact duplicate
Occurrences: 2
Impact: Low — fields are byte-identical
Locations:
pkg/workflow/repository_features_validation.go:73(native build)pkg/workflow/repository_features_validation_wasm.go:31(//go:build js || wasm)Recommendation: The struct itself (
HasDiscussions bool; HasIssues bool) has no platform dependency — only the validation function differs between builds. Hoist the type declaration into a shared non-build-tagged file.Clusters flagged but requiring no action
SpinnerWrapper/ProgressBar(native vs. wasm) — deliberate build-tag split with an intentionally slimmer wasm implementation; not consolidatable without pulling terminal-only deps into wasm builds.ActionPin,InputDefinition,ToolCallInfo,SanitizeOptions) — these aretype X = otherpkg.Xre-exports, a deliberate and correctly-applied Go pattern, not real duplication.Worker/fakeOSinpkg/linters/*/testdata— required by thego/analysistestdata convention for custom golangci-lint analyzers; each lives in its own isolated fixture package.Untyped Usages
Summary Statistics
interface{}usages (verified): 17 total in non-testpkg/filesanyusages (verified, rough pattern match): ~3,076 total in non-testpkg/filesfrontmatter map[string]anyspecifically: 290 occurrences across 128 filesCategory 1:
frontmatter map[string]any— the dominant patternImpact: High — single highest-leverage fix in the codebase
Root type:
pkg/parser/frontmatter_content.go:20—FrontmatterResult.Frontmatter map[string]anyThis shape propagates through:
pkg/parser/workflow_update.go:22—UpdateWorkflowFrontmatter(workflowPath string, updateFunc func(frontmatter map[string]any) error, verbose bool) errorpkg/cli/fix_codemods.go:20— every one of the ~40 codemods:Apply func(content string, frontmatter map[string]any) (string, bool, error)pkg/workflow/stop_after.go:19and siblingCompiler.extract*FromOnmethods (extractSkipIfMatchFromOn,extractSkipIfNoMatchFromOn,extractSkipIfCheckFailingFromOn) — same signature repeated per extraction helperSuggested fix:
Benefits: One named type with typed accessor methods replaces hundreds of duplicated
.(string)/.([]any)assertions scattered across 128 files; parsing bugs get fixed in one place instead of wherever they happen to surface.Estimated effort: 4-6 hours for the type + accessors + updating the ~40 codemod signatures (mostly mechanical, but needs a full test run)
Category 2:
sync.Mapcaches storing exactly one concrete typeImpact: Medium — removes a live runtime-failure path, not just a style issue
Examples:
pkg/workflow/behavior_defined_engine.go:632—engineDefinitionCache sync.Mapdocumented asmap[string]EngineDefinition, with defensive code at lines 640-647 to handle a failed type assertion ("cache corruption or concurrent Store with unexpected type")pkg/workflow/repository_features_validation.go:80—repositoryFeaturesCache sync.Mapstoring only*RepositoryFeatures, with a cache.Delete-on-mismatch fallback at line 202Suggested fix: Wrap each in a small typed cache struct (or Go generics) so the type assertion — and its failure-handling branch — disappears entirely, since only one type is ever stored.
Category 3:
engine.go's ~20applyEngineXField(config, engineObj map[string]any)functionsImpact: High — a hand-rolled struct decoder
Location:
pkg/workflow/engine.go:546and ~19 sibling functions (applyEngineEnvField,applyEngineAuthField,applyEngineArgsField,applyEngineMCPField,applyEngineProviderFields,applyEnginePermissionMode,applyEngineTurnFields,applyEngineConcurrencyField,applyEngineStringFields,applyEngineDriverField,applyEngineHarnessField,applyEngineExtensionsField,applyEngineBooleanFields,applyEngineBareField)Suggested fix: Define a struct with yaml tags for the
engine:frontmatter object and unmarshal once viayaml.Unmarshal, instead of ~20 functions each independently re-probing the samemap[string]anyfor fixed, known keys.Estimated effort: 6-8 hours (higher risk — touches core engine parsing, needs thorough test coverage)
Category 4:
[]any → []*WorkflowStep → []anyunneeded round-tripImpact: High — the typed version already exists and is being discarded
Location:
pkg/workflow/workflow_builder.go:790(and 748/770/819)The codebase already has a fully-typed
WorkflowStepstruct (pkg/workflow/step_types.go:18) withSliceToSteps/StepsToSliceconverters.processAndMergeStepsconverts typed steps back to[]any(lines 764, 786, 810) purely to marshal them via amap[string]anywrapper (line 826) — defeating the type safety the conversion functions were built to provide.Suggested fix: Keep
mainSteps/copilotSetupSteps/otherImportedSteps/allStepsas[]*WorkflowStepthroughout; rely onWorkflowStep's existing yaml tags for marshaling.Category 5: String fields that are really enums (
pkg/workflow/engine_definition.go)Impact: Medium — three separate instances of the same pattern in one struct family
SecretStrategy stringengine_definition.go:231"universal-llm-consumer"behavior_defined_engine.go:19ProviderEnvMode stringengine_definition.go:204"universal-llm-consumer"behavior_defined_engine.go:20MergeStrategy stringengine_definition.go:189"json-merge"behavior_defined_engine.go:21Suggested fix: Define named string types (
SecretStrategy,ProviderEnvMode,ConfigMergeStrategy) with typed constants, replacing the untyped string comparisons.Category 6: Discriminated unions currently typed
anyImpact: Medium
pkg/workflow/safe_outputs_config_types.go:109—ReportFailureAsIssue any— doc comment says it's one ofbool | templatable string | []interface{} categories, and the struct already carries two derivedyaml:"-"fields populated by manually parsing thisany. A customUnmarshalYAMLdispatching on shape would remove the derived-field workaround entirely.pkg/workflow/safe_outputs_config_types.go:115—Steps []any— should be[]*WorkflowStep(same established pattern asSliceToSteps/StepsToSliceelsewhere), consumed as GH Actions step objects atcompiler_safe_outputs_job.go:238-240.pkg/workflow/step_types.go:28—ContinueOnError any(bool or expression string) — lowest priority; it's the one untyped field in an otherwise fully-typedWorkflowStep.Category 7: Untyped duration/size constants
Impact: Low-Medium — clarity, not correctness bugs (no confirmed runtime failures)
pkg/workflow/compiler.go:25—MaxLockFileSize,MaxExpressionSize,MaxPromptChunkSizeare untyped byte-count ints, distinguished only by comments. Suggested:type ByteSize int.pkg/cli/mcp_tools_privileged.go:40—defaultMCPAuditTimeoutMinutes,defaultMCPAuditDiffTimeoutMinutes, etc. are untyped int "minutes" constants manually converted at each call site viatime.Duration(x)*time.Minute. Declaring them directly astime.Durationremoves the repeated conversion and the risk of a call site forgetting the multiplier.Recommendations by Priority
Priority 1: Quick wins — safe-output config field consolidation (Clusters 1, 2, 4)
Steps:
FooterConfigand extendSafeOutputFilterConfigwithAllowedLabelsinpkg/workflow/safe_outputs_parser.goTargetRepoSlug/AllowedReposto embedSafeOutputTargetConfiggo test ./pkg/workflow/...) after each mechanical migrationEstimated effort: 4-6 hours total
Impact: High — removes ~29 duplicate field declarations with minimal risk
Priority 2: High-value type safety —
Frontmatternamed typeSteps:
type Frontmatter map[string]anywith typed getters inpkg/parserUpdateWorkflowFrontmatterand the codemodApplysignature to use itCompiler.extract*FromOnfamily inpkg/workflowpkg/parser,pkg/cli(codemods), andpkg/workflowEstimated effort: 4-6 hours
Impact: High — touches 128 files' worth of untyped map assertions, but is additive (map[string]any is still the underlying type, so it's a low-risk rename+method-add, not a breaking change)
Priority 3: Structural cleanups — engine parsing and step round-trips
Steps:
engine:frontmatter object to replace the ~20applyEngineXFieldfunctions inengine.go[]any → []*WorkflowStep → []anyround-trip inworkflow_builder.goto stay typed throughoutSecretStrategy/ProviderEnvMode/MergeStrategyinengine_definition.goEstimated effort: 8-12 hours (higher risk, needs careful test coverage on core engine/workflow compilation)
Impact: High — but higher risk since it touches core compilation logic
Implementation Checklist
FooterConfig+AllowedLabelstoSafeOutputFilterConfig; embed in 13 + 5 structsSafeOutputTargetConfigTitlePrefixintoDefaultTitlePrefixvs. deprecated filter aliasAuditComparisonDelta[T]replacingAuditComparisonIntDelta/StringDeltaRepositoryFeaturesstruct out of build-tagged files into a shared fileFrontmatternamed type with typed accessors; migrateUpdateWorkflowFrontmatter, codemods,Compiler.extract*FromOnengineDefinitionCache/repositoryFeaturesCachesync.Maps in typed accessorsapplyEngineXField(..., map[string]any)family with a single struct +yaml.Unmarshal[]anyround-trip inworkflow_builder.goto stay[]*WorkflowStepSecretStrategy/ProviderEnvMode/MergeStrategyAnalysis Metadata
any/interface{}hits)All reactions