[typist] π€ Typist β Go Type Consistency Analysis (2026-06-29) #42244
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Typist - Go Type Analysis. A newer discussion is available at Discussion #42475. |
Beta Was this translation helpful? Give feedback.
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.
-
π€ Typist β Go Type Consistency Analysis
Analysis of repository: github/gh-aw β’ 2026-06-29
Executive Summary
Good news up front:
gh-awis already a strongly-typed codebase. I swept every non-test.gofile underpkg/β 974 files, 709 type definitions (612 structs) β and the usual type-safety smells are mostly absent.interface{}has been all but eliminated from source (just 1 occurrence, inside a doc comment). Theanykeyword appears a lot, but the overwhelming majority are legitimate dynamic boundaries β JSON/YAML frontmatter, MCP payloads, and a purpose-builttypeutilconverter β exactly whereanybelongs. The team already embeds shared base structs (BaseSafeOutputConfig,UpdateEntityConfig) and uses named string enums (ActionMode,ShellType), so the idioms for closing the few remaining gaps are already in the codebase.So this is less "we have a problem" and more "a handful of small, high-confidence hardening wins." The two best: name two untyped constant groups (
SafeOutputsURLsPolicy,GuardPolicyErrorCode), and one low-effort struct consolidation (the repeatedAllowed/Blockedpair across six safe-output configs). None of the 13 duplicate name collisions were real β every one is an intentional alias, build-tag platform variant, or linter testdata. Net: type safety is in great shape; below is polish, not rescue.Full Analysis Report
Duplicated Type Definitions
Stats: 709 types analyzed (612 structs) β’ 4 genuine clusters β’ 1 exact, 2 near, 1 semantic β’ 13 name-collisions ruled out (all
type X = Yre-export aliases,js/wasmbuild variants, ortestdata/srcfixtures β correct by design).Cluster 1 β Safe-output allow/deny configs β best win β Near
6 occurrences β’ Impact: Medium (low-effort, mechanical)
Six configs each embed the same base trio and then repeat an
Allowed []string/Blocked []stringpair:pkg/workflow/add_labels.go:10AddLabelsConfig(Allowed+Blocked)pkg/workflow/remove_labels.go:10RemoveLabelsConfig(identical to AddLabels)pkg/workflow/assign_to_user.go:10AssignToUserConfig(+UnassignFirst *string)pkg/workflow/unassign_from_user.go:10UnassignFromUserConfigpkg/workflow/set_issue_type.go:10SetIssueTypeConfig(Allowed only)pkg/workflow/assign_milestone.go:10AssignMilestoneConfig(+AutoCreate bool)Fix: extract
SafeOutputAllowBlockConfig { Allowed, Blocked []string }and embed it inline (matching the existing base-embedding pattern); per-type extras stay local. ~1β2 h.Cluster 2 β
TransportvsRemote(MCP registry) β Exactpkg/cli/mcp_registry_types.go:62&:99have identical field sets (only anomitemptytag differs). These mirror the upstream MCP Registry OpenAPI spec β flag only, collapse totype Remote = Transportonly if the spec unifies them.Cluster 3 β Update-entity configs β Near (low)
pkg/workflow/update_issue.go:13UpdateIssuesConfigandupdate_pull_request.go:13UpdatePullRequestsConfigalready shareUpdateEntityConfig; only theTitle *bool/Body *bool/Footer *stringtrio repeats. Optional extract; field divergence is real β small payoff.Cluster 4 β Position/location triples β Semantic (weak)
pkg/workflow/workflow_errors.go:38WorkflowValidationErrorredeclares inlineFile/Line/Column(lines 47β49) despite already referencingFieldLocation(=console.ErrorPosition). Could embedFieldLocation;parser/import_error.go:15ImportErrorlikewise. Optional cleanup.Untyped Usages
Stats:
interface{}in source = 1 (doc comment) β’anyβ350+ (vast majority idiomatic) β’ untyped-constantβnamed-type opportunities β5. Theinterface{}/anyaxis is clean; the real surface is enum-like constants.Category 1 β
any/interface{}β mostly acceptable β (don't "fix")Correctly generic, leave as-is:
pkg/typeutil/convert.go(heterogeneous JSON/YAML converter),pkg/parser/tools_merger.goMergeTools(map[string]any),pkg/workflow/compiler_types.goWorkflowData.{Tools,Features,RawFrontmatter,...} map[string]any(open-schema YAML),pkg/cli/gateway_logs_types.go:148,159JSON-RPCID any(spec: string|number|null). Only note:pkg/cli/mcp_tools_privileged.go:259-260RunID anydeliberately accepts string-or-number β defensible. No high-impactanyβconcrete conversions exist.Category 2 β Untyped constants β named enums β the real wins
Pattern already in repo (
type ActionMode string), so these are low-risk:2.1 β SafeOutputs URL policy (HIGH)
pkg/workflow/safe_outputs_validation.go:12-15, fieldcompiler_types.go:728Closed two-value enum with a dedicated
validateSafeOutputsURLsswitch β ideal candidate.2.2 β Guard-policy JSON-RPC error codes (HIGH)
pkg/cli/gateway_logs_types.go:61-68Untyped
const guardPolicyErrorCode... = -32001 ... -32006, consumed byisGuardPolicyErrorCode(int)and stored asGuardPolicyEvent.ErrorCode int. βtype GuardPolicyErrorCode intwith named consts. A closed protocol-code set; prevents mixing with arbitrary ints.2.3 β Guard-policy reason strings (MEDIUM)
gateway_logs_types.go:191-209βguardPolicyReasonFromCodereturns bare strings ("access_denied", ...) βtype GuardPolicyReason string. Mostly display-facing.2.4 β Domain cross-run status (MEDIUM)
pkg/cli/audit_cross_run.go:106,113βOverallStatus/Status string // "allowed","denied","mixed","absent"is a comment-documented enum βtype DomainStatus stringenables exhaustive handling.2.5 β MCP transport type (LOW/MED)
pkg/cli/mcp_registry_types.go:63,100+transportTypeflag (mcp_add.go:22) β small known set ("stdio"/"http"/"sse");type MCPTransportType string, keep open-string fallback for third-party registry JSON.Already strong β
Timeout/retry magic numbers are typed (
time.Durationvia* time.Second, or named ints likemaxRetries) inpkg/constants/constants.go:270-304,mcp_inspect_mcp.go:27-29,run_workflow_tracking.go:31-33. Metrics structs fully concretely typed.π― Recommendations (impact-to-effort order)
Priority 1 β name two constant groups (HIGH). Convert 2.1 + 2.2. Add
type X string/int, type the const block + consuming field + helpers,go build ./...finds every call site, run tests. ~1β2 h each.Priority 2 β consolidate Cluster 1 (MED). Extract & embed
SafeOutputAllowBlockConfigacross the six configs. ~1β2 h mechanical.Priority 3 β optional polish. Findings 2.3/2.4/2.5 (more enums) and Clusters 3/4 (embedding cleanups) β do opportunistically.
Checklist
SafeOutputsURLsPolicynamed type (2.1)GuardPolicyErrorCodenamed type (2.2)SafeOutputAllowBlockConfig(Cluster 1)GuardPolicyReason,DomainStatus,MCPTransportType; embedFieldLocation(Cluster 4)go build ./...+ full test suite after each changeMetadata: 974 files β’ 709 types (612 structs) β’ 4 genuine clusters (9 collisions ruled out) β’ ~5 untyped opportunities β’ Method: Serena semantic analysis + ripgrep, every finding verified against source.
Beta Was this translation helpful? Give feedback.
All reactions