Skip to content

Refactor: 8 parallel dispatch tables keyed by safe-output type cause shotgun surgery #31631

Description

@github-actions

Problem

pkg/workflow/safe_outputs_state.go:16-21 explicitly documents this hazard:

NOTE: When adding a new pointer field to SafeOutputsConfig that represents a user-facing safe output action, add it to ALL of the following locations:

  1. safeOutputFieldMapping (below) — drives imports, prompt generation, etc.
  2. hasAnySafeOutputEnabled — performance-critical hot path
  3. hasNonBuiltinSafeOutputsEnabled — if it is NOT a builtin (noop/missing-*)
  4. hasSafeOutputType in imports.go — used for conflict detection

The comment lists 4 locations, but Sergo Run 7 (2026-05-12) located at least 8 parallel dispatch tables keyed on the same safe-output handler name axis. Adding a new safe-output requires editing every one of them.

Inventory of duplicate dispatch sites (Run 7)

# Function File Dispatch shape
1 safeOutputFieldMapping (map literal) pkg/workflow/safe_outputs_state.go:27-71 StructField → key
2 hasAnySafeOutputEnabled pkg/workflow/safe_outputs_state.go:79+ nil-check cascade
3 hasNonBuiltinSafeOutputsEnabled pkg/workflow/safe_outputs_state.go nil-check cascade
4 hasSafeOutputType pkg/workflow/imports.go:321-419 45-case switch (key → bool)
5 SafeOutputsConfigFromKeys pkg/workflow/safe_outputs_permissions.go:313+ 45-case switch (key → struct setter)
6 ComputePermissionsForSafeOutputs pkg/workflow/safe_outputs_permissions.go:79-307 30+ if-branches per pointer
7 mergeSafeOutputConfig pkg/workflow/imports.go:423-670 45+ if result.X == nil && imported.X != nil
8 extractSafeOutputsConfig pkg/workflow/compiler.go (already-tracked) 637-line / 45-branch parser (#31298)

Impact

Recommended approach

Define one canonical descriptor per handler type and derive every dispatch site from it.

type safeOutputHandlerDesc struct {
    Key        string                              // e.g. "create-issue"
    StructField string                             // e.g. "CreateIssues"
    Builtin    bool                                // noop, missing-*, etc.
    NewConfig  func() any                          // builds the zero-value pointer
    Permissions func(*SafeOutputsConfig) *Permissions // returns merged perms or nil
}

var safeOutputHandlers = []safeOutputHandlerDesc{
    {Key: "create-issue", StructField: "CreateIssues", NewConfig: func() any { return &CreateIssuesConfig{} },
     Permissions: func(c *SafeOutputsConfig) *Permissions { return NewPermissionsContentsReadIssuesWrite() }},
    // ... one row per handler
}

Then rewrite each dispatch site as a table walk using reflect to read/write the struct field. Specifically:

  • hasSafeOutputType(cfg, key) → look up desc by key, reflect.ValueOf(cfg).Elem().FieldByName(desc.StructField).IsNil() == false
  • SafeOutputsConfigFromKeys → assign desc.NewConfig() into the named field
  • mergeSafeOutputConfig → loop descriptors; if result.field == nil, copy from imported
  • ComputePermissionsForSafeOutputs → loop and merge desc.Permissions(safeOutputs) for every non-nil, non-staged field
  • hasAnySafeOutputEnabled — keep direct nil-checks (hot path), but generate it via go:generate from the descriptor table

Validation

  • Existing tests in safe_outputs_*_test.go cover all four refactored dispatch sites; they must remain green
  • TestHasSafeOutputTypeNewKeys validates parity between descriptor table and old switch
  • Snapshot tests on generated workflow YAML detect permission drift
  • Benchmark hasAnySafeOutputEnabled to confirm hot path is unchanged

Labels

sergo refactor tech-debt workflow-compiler

Sergo Run 7 — strategy: switch-statement-complexity-plus-exported-api-usage-audit

#aw_sg7a1

Generated by Sergo - Serena Go Expert · ● 18.1M ·

  • expires on May 19, 2026, 4:59 AM UTC

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions