Problem
pkg/workflow/frontmatter_parsing.go:195-289 is a 95-line switch scope { ... } with 40 cases, each assigning the same string to a specifically-named field on PermissionsConfig:
for scope, level := range permissions {
if levelStr, ok := level.(string); ok {
switch scope {
case "actions": config.Actions = levelStr
case "checks": config.Checks = levelStr
case "contents": config.Contents = levelStr
// ... 37 more nearly-identical cases ...
case "codespaces-metadata": config.CodespacesMetadata = levelStr
}
}
}
This duplicates — for the fourth time — the permission-scope inventory already maintained in (i) the PermissionXyz const block (permissions.go:135-203), (ii) GetAllPermissionScopes/GetAllGitHubAppOnlyScopes (permissions.go:209-269), and (iii) convertStringToPermissionScope (permissions.go:12-121 — see companion issue #aw_sg7a2).
Impact
- Severity: High (when combined with the companion
permissions.go switch) — adding a new GitHub permission scope already requires three coordinated edits; with this switch it becomes four. A missed case here silently drops the field on parse.
- Bloat: ~95 lines reducible to ~10–15 if driven by reflection or a getter map.
- Discoverability: there is no comment in
frontmatter_parsing.go linking back to permissions.go, so a developer adding PermissionXyz may add the constant and the Get* slice but forget the parser branch — the YAML key will be silently ignored.
Recommended fix
Drive the field assignment from a table indexed by scope key:
// permissions_config_fields.go (new file)
var permissionsConfigFieldByScope = map[PermissionScope]func(*PermissionsConfig, string){
PermissionActions: func(c *PermissionsConfig, v string) { c.Actions = v },
PermissionChecks: func(c *PermissionsConfig, v string) { c.Checks = v },
// ... one row per scope, generated or hand-written
}
// In parsePermissionsConfig:
for scope, level := range permissions {
if levelStr, ok := level.(string); ok {
if setter, ok := permissionsConfigFieldByScope[PermissionScope(scope)]; ok {
setter(config, levelStr)
}
}
}
Alternatively (more invasive, simpler long-term): represent PermissionsConfig itself as map[PermissionScope]string instead of 40 named string fields. Existing JSON marshalling would still produce the same YAML thanks to json:"" tags on a MarshalJSON impl.
If the reflection approach is preferred:
var permissionsConfigFieldName = map[PermissionScope]string{
PermissionActions: "Actions",
PermissionChecks: "Checks",
// ...
}
for scope, level := range permissions {
name, ok := permissionsConfigFieldName[PermissionScope(scope)]
if !ok {
continue
}
if levelStr, ok := level.(string); ok {
reflect.ValueOf(config).Elem().FieldByName(name).SetString(levelStr)
}
}
Coordination with issue #aw_sg7a2
If the convertStringToPermissionScope refactor (issue #aw_sg7a2) lands first, this function can be simplified by validating the key with the same validPermissionScopes map and only then dispatching to the setter table — eliminating the unknown-key fall-through.
Validation
Labels
sergo refactor tech-debt permissions
Sergo Run 7 — strategy: switch-statement-complexity-plus-exported-api-usage-audit
#aw_sg7a3
Generated by Sergo - Serena Go Expert · ● 18.1M · ◷
Problem
pkg/workflow/frontmatter_parsing.go:195-289is a 95-lineswitch scope { ... }with 40 cases, each assigning the same string to a specifically-named field onPermissionsConfig:This duplicates — for the fourth time — the permission-scope inventory already maintained in (i) the
PermissionXyzconst block (permissions.go:135-203), (ii)GetAllPermissionScopes/GetAllGitHubAppOnlyScopes(permissions.go:209-269), and (iii)convertStringToPermissionScope(permissions.go:12-121— see companion issue #aw_sg7a2).Impact
permissions.goswitch) — adding a new GitHub permission scope already requires three coordinated edits; with this switch it becomes four. A missed case here silently drops the field on parse.frontmatter_parsing.golinking back topermissions.go, so a developer addingPermissionXyzmay add the constant and theGet*slice but forget the parser branch — the YAML key will be silently ignored.Recommended fix
Drive the field assignment from a table indexed by scope key:
Alternatively (more invasive, simpler long-term): represent
PermissionsConfigitself asmap[PermissionScope]stringinstead of 40 named string fields. Existing JSON marshalling would still produce the same YAML thanks tojson:""tags on aMarshalJSONimpl.If the reflection approach is preferred:
Coordination with issue #aw_sg7a2
If the
convertStringToPermissionScoperefactor (issue #aw_sg7a2) lands first, this function can be simplified by validating the key with the samevalidPermissionScopesmap and only then dispatching to the setter table — eliminating the unknown-key fall-through.Validation
TestParsePermissionsConfigexercises every scope (or add one if missing) and asserts the matching field is populatedGetAllPermissionScopes() ∪ GetAllGitHubAppOnlyScopes()is parseable through this functionLabels
sergorefactortech-debtpermissionsSergo Run 7 — strategy: switch-statement-complexity-plus-exported-api-usage-audit
#aw_sg7a3