Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkg/actionpins/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ func getCachedActionPins() *actionPinsCache {
})

if cachedPins == nil {
// Build-time invariant: actionPinsOnce.Do above always assigns cachedPins
// unless loadActionPinsData panicked, so this is unreachable in practice.
panic("action pins cache was not initialized")
}
return cachedPins
Expand All @@ -87,6 +89,9 @@ func loadActionPinsData(raw []byte) ActionPinsData {
var data ActionPinsData
if err := json.Unmarshal(raw, &data); err != nil {
actionPinsLog.Printf("Failed to unmarshal action pins JSON: %v", err)
// Build-time invariant: data/action_pins.json is embedded at compile time and
// validated by this package's tests; unmarshal can only fail for corrupted
// release data, never dynamic user input.
panic(fmt.Sprintf("failed to load action pins: %v", err))
}

Expand All @@ -95,6 +100,8 @@ func loadActionPinsData(raw []byte) ActionPinsData {
}

if emptyKeys := collectEntriesWithEmptySHA(data.Entries); len(emptyKeys) > 0 {
// Build-time invariant: an empty SHA in the embedded pin data would produce
// invalid workflow YAML at release time and must be caught before shipping.
panic(fmt.Sprintf("action_pins.json has %d entries with empty SHA %v — these would produce invalid workflow YAML (e.g. 'owner/repo@ # version'); remove or fix these entries before releasing", len(emptyKeys), emptyKeys))
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/workflow/agentic_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,10 @@ func NewEngineRegistry() *EngineRegistry {
}
for _, engine := range builtins {
if err := registry.Register(engine); err != nil {
// Build-time invariant: Register only rejects a negative
// dedicatedLLMGatewayPort, and every built-in engine above is constructed
// with a compile-time constant port, so this is a programming error caught
// by CI (TestBuiltinEnginesRegisterWithoutError), never user input.
panic(fmt.Sprintf("BUG: failed to register built-in engine: %v", err))
}
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/workflow/mcp_setup_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ func writeMCPGatewayStepEnvWithCustomGatewayEnvNames(yaml *strings.Builder, mcpE
if len(customEnvVarNames) > 0 {
customEnvNamesJSON, err := json.Marshal(customEnvVarNames)
if err != nil {
// Build-time invariant: customEnvVarNames is a []string, which json.Marshal
// always serialises successfully; this branch is unreachable in practice.
panic(fmt.Sprintf("BUG: failed to marshal MCP gateway environment variable names: %v", err))
}
yaml.WriteString(formatYAMLEnv(" ", mcpGatewayCustomEnvNamesVar, string(customEnvNamesJSON)))
Expand Down
3 changes: 3 additions & 0 deletions pkg/workflow/mcp_setup_safe_outputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ func generateSafeOutputsSetup(c *Compiler, yaml *strings.Builder, safeOutputConf
}},
})
if err != nil {
// Build-time invariant: fileRenderConfig above is a fixed struct literal of
// strings, which json.Marshal always serialises successfully; this branch is
// unreachable in practice.
panic(fmt.Sprintf("BUG: failed to marshal generated file render config: %v", err))
}
yaml.WriteString(" - name: Generate Safe Outputs Config\n")
Expand Down
8 changes: 7 additions & 1 deletion pkg/workflow/model_aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func loadBuiltinModelAliases() (map[string][]string, error) {
return builtinModelAliasesLoader.Get(func() (map[string][]string, error) {
var data builtinModelAliasesFile
if err := json.Unmarshal(builtinModelAliasesJSON, &data); err != nil {
return nil, fmt.Errorf("BUG: workflow: failed to parse embedded model_aliases.json: %w (try 'make build' to rebuild with the latest data)", err)
return nil, fmt.Errorf("BUG: workflow: could not parse embedded model_aliases.json: %w (expected valid JSON matching the aliases schema; run 'make build' to rebuild with the latest data)", err)
}
return data.Aliases, nil
})
Expand Down Expand Up @@ -88,6 +88,9 @@ func getBuiltinOnlyAliasMap() map[string][]string {
builtinOnlyAliasMapOnce.Do(func() {
data, err := loadBuiltinModelAliases()
if err != nil {
// Build-time invariant: model_aliases.json is embedded at compile time and
// validated by TestBuiltinModelAliases; a real unmarshal failure here can only
// follow a corrupted release build, never dynamic user input.
panic(err)
}
builtinOnlyAliasMap = data
Expand Down Expand Up @@ -145,6 +148,9 @@ func isBuiltinOnlyAliasMap(m map[string][]string) bool {
func BuiltinModelAliases() map[string][]string {
data, err := loadBuiltinModelAliases()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/grill-with-docs] The cross-reference // Build-time invariant: see comment in getBuiltinOnlyAliasMap above. leaves BuiltinModelAliases (a public function) dependent on navigating to another function to understand the invariant. Readers encountering this in search results or generated docs won't have that context.

💡 Suggested inline wording
// Build-time invariant: model_aliases.json is embedded at compile time and
// validated by TestBuiltinModelAliases; unmarshal can only fail for corrupted
// release data, never dynamic user input.
panic(err)

Mirrors the comment in getBuiltinOnlyAliasMap directly, making each site self-contained.

@copilot please address this.

if err != nil {
// Build-time invariant: model_aliases.json is embedded at compile time and
// validated by TestBuiltinModelAliases; a real unmarshal failure here can only
// follow a corrupted release build, never dynamic user input.
panic(err)
}
// Return a fresh deep copy so callers may freely modify map entries and slices.
Expand Down
85 changes: 85 additions & 0 deletions pkg/workflow/panic_invariants_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
//go:build !integration

package workflow

import (
"encoding/json"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// The tests in this file are executable guardrails for the "build-time invariant"
// panic sites documented in this package. Each test exercises the real code path
// that precedes a panic() so that a refactor which makes the panic reachable fails
// the test suite instead of only surfacing at runtime.

// TestEmbeddedModelAliasesAreLoadable guards the panics in getBuiltinOnlyAliasMap
// and BuiltinModelAliases, which fire only when the embedded model_aliases.json
// cannot be unmarshaled.
func TestEmbeddedModelAliasesAreLoadable(t *testing.T) {
aliases, err := loadBuiltinModelAliases()
require.NoError(t, err, "embedded model_aliases.json must unmarshal at build time")
assert.NotEmpty(t, aliases)

assert.NotEmpty(t, getBuiltinOnlyAliasMap())
assert.NotEmpty(t, BuiltinModelAliases())
}

// TestEmbeddedToolsetPermissionsAreLoadable guards the panic in
// getToolsetPermissionsMap, which fires only when the embedded GitHub toolsets
// JSON cannot be unmarshaled.
func TestEmbeddedToolsetPermissionsAreLoadable(t *testing.T) {
assert.NotEmpty(t, getToolsetPermissionsMap())
}

// TestBuiltinEnginesRegisterWithoutError guards the panic in NewEngineRegistry.
// Register only rejects a negative dedicatedLLMGatewayPort, so re-registering every
// built-in engine asserts exactly the failure mode the panic guards against.
func TestBuiltinEnginesRegisterWithoutError(t *testing.T) {
registry := NewEngineRegistry()

for _, id := range registry.GetSupportedEngines() {
engine, err := registry.GetEngine(id)
require.NoError(t, err)
assert.NoError(t, NewEngineRegistry().Register(engine), "built-in engine %q must register without error", id)
}
}

// TestBuildPiModelsJSONMarshalsRuntimeValues guards the panic in buildPiModelsJSON.
// The payload mixes literals with runtime arguments, so this asserts that those
// argument types always serialise.
func TestBuildPiModelsJSONMarshalsRuntimeValues(t *testing.T) {
out := buildPiModelsJSON(4141, "COPILOT_GITHUB_TOKEN", "claude-sonnet-4-20250514")

var decoded map[string]any
require.NoError(t, json.Unmarshal([]byte(out), &decoded))
assert.Contains(t, decoded, "providers")
}

// TestMCPGatewayCustomEnvNamesMarshal guards the panic in
// writeMCPGatewayStepEnvWithCustomGatewayEnvNames, which fires only if the
// []string of env var names fails to marshal.
func TestMCPGatewayCustomEnvNamesMarshal(t *testing.T) {
var stepEnv strings.Builder
gatewayEnv := map[string]string{"CUSTOM_ONE": "value-one", "CUSTOM_TWO": "value-two"}
writeMCPGatewayStepEnvForTest(&stepEnv, nil, nil, gatewayEnv)

assert.Contains(t, stepEnv.String(), mcpGatewayCustomEnvNamesVar)
assert.Contains(t, stepEnv.String(), `[\"CUSTOM_ONE\",\"CUSTOM_TWO\"]`)
}

// TestFileRenderConfigMarshals guards the panic in generateSafeOutputsSetup, which
// fires only if the fixed fileRenderConfig struct fails to marshal.
func TestFileRenderConfigMarshals(t *testing.T) {
out, err := json.Marshal(fileRenderConfig{
Files: []fileRenderItem{{
Path: "safeoutputs/config.json",
ContentEnv: "GH_AW_SAFE_OUTPUTS_CONFIG",
}},
})
require.NoError(t, err)
assert.JSONEq(t, `{"files":[{"path":"safeoutputs/config.json","content_env":"GH_AW_SAFE_OUTPUTS_CONFIG"}]}`, string(out))
}
3 changes: 3 additions & 0 deletions pkg/workflow/permissions_toolset_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ var getToolsetPermissionsMap = sync.OnceValue(func() map[string]GitHubToolsetPer

var data GitHubToolsetsData
if err := json.Unmarshal(githubToolsetsPermissionsJSON, &data); err != nil {
// Build-time invariant: the embedded GitHub toolsets JSON is validated by
// TestToolsetPermissionsLoadedFromJSON; unmarshal can only fail for corrupted
// release data, never dynamic user input.
panic(fmt.Sprintf("BUG: failed to load GitHub toolsets permissions from JSON: %v", err))
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/workflow/pi_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,9 @@ func buildPiModelsJSON(gatewayPort int, secretEnvVarName, modelID string) string
}
b, err := json.Marshal(payload)
if err != nil {
// json.Marshal only fails for non-serialisable types; our map is always
// serialisable, so this branch is unreachable in practice.
// Build-time invariant: the payload holds only strings, maps and slices —
// json.Marshal only errors for non-serialisable types such as channels or
// functions, so no runtime argument value can reach this branch.
panic(fmt.Sprintf("BUG: buildPiModelsJSON failed to marshal JSON: %v", err))
}
return string(b)
Expand Down
Loading