From 2febcbed6a8be61451396cb26e6349b38685efce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:19:48 +0000 Subject: [PATCH 1/3] Initial plan From 35ba1f48dc16310a6a5cf97d5874c65655298a67 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:28:46 +0000 Subject: [PATCH 2/3] Document panic() contract for embed-guarded lazy-load panic sites Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/actionpins/data.go | 7 +++++++ pkg/workflow/agentic_engine.go | 3 +++ pkg/workflow/mcp_setup_gateway.go | 2 ++ pkg/workflow/mcp_setup_safe_outputs.go | 3 +++ pkg/workflow/model_aliases.go | 4 ++++ pkg/workflow/permissions_toolset_data.go | 3 +++ pkg/workflow/pi_engine.go | 5 +++-- 7 files changed, 25 insertions(+), 2 deletions(-) diff --git a/pkg/actionpins/data.go b/pkg/actionpins/data.go index e5751dc356e..6a3a9ec5ffb 100644 --- a/pkg/actionpins/data.go +++ b/pkg/actionpins/data.go @@ -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 @@ -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)) } @@ -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)) } diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index d53781b5ca6..766196a4ce8 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -577,6 +577,9 @@ func NewEngineRegistry() *EngineRegistry { } for _, engine := range builtins { if err := registry.Register(engine); err != nil { + // Build-time invariant: the built-in engines list above has fixed, non-empty + // names, so registration can only fail if a future refactor introduces a + // duplicate or empty name — a programming error caught by CI, not user input. panic(fmt.Sprintf("BUG: failed to register built-in engine: %v", err)) } } diff --git a/pkg/workflow/mcp_setup_gateway.go b/pkg/workflow/mcp_setup_gateway.go index 64577d52159..6fcfb3115c8 100644 --- a/pkg/workflow/mcp_setup_gateway.go +++ b/pkg/workflow/mcp_setup_gateway.go @@ -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))) diff --git a/pkg/workflow/mcp_setup_safe_outputs.go b/pkg/workflow/mcp_setup_safe_outputs.go index 502590d4eb6..33346f1ce38 100644 --- a/pkg/workflow/mcp_setup_safe_outputs.go +++ b/pkg/workflow/mcp_setup_safe_outputs.go @@ -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") diff --git a/pkg/workflow/model_aliases.go b/pkg/workflow/model_aliases.go index 201fa071d09..46edd5b9428 100644 --- a/pkg/workflow/model_aliases.go +++ b/pkg/workflow/model_aliases.go @@ -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 @@ -145,6 +148,7 @@ func isBuiltinOnlyAliasMap(m map[string][]string) bool { func BuiltinModelAliases() map[string][]string { data, err := loadBuiltinModelAliases() if err != nil { + // Build-time invariant: see comment in getBuiltinOnlyAliasMap above. panic(err) } // Return a fresh deep copy so callers may freely modify map entries and slices. diff --git a/pkg/workflow/permissions_toolset_data.go b/pkg/workflow/permissions_toolset_data.go index d99eb78560d..90f5a4ca294 100644 --- a/pkg/workflow/permissions_toolset_data.go +++ b/pkg/workflow/permissions_toolset_data.go @@ -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)) } diff --git a/pkg/workflow/pi_engine.go b/pkg/workflow/pi_engine.go index 7ece31d33b1..d4e8de1fee6 100644 --- a/pkg/workflow/pi_engine.go +++ b/pkg/workflow/pi_engine.go @@ -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: payload above is built entirely from string/map + // literals, so Marshal cannot fail; json.Marshal only errors for + // non-serialisable types such as channels or functions. panic(fmt.Sprintf("BUG: buildPiModelsJSON failed to marshal JSON: %v", err)) } return string(b) From 0617d8909d986ec78d81d749dcf71b58beb6af1b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Aug 2026 21:03:21 +0000 Subject: [PATCH 3/3] Add executable guardrail tests for documented panic invariants and fix error message lint Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/agentic_engine.go | 7 ++- pkg/workflow/model_aliases.go | 6 +- pkg/workflow/panic_invariants_test.go | 85 +++++++++++++++++++++++++++ pkg/workflow/pi_engine.go | 6 +- 4 files changed, 96 insertions(+), 8 deletions(-) create mode 100644 pkg/workflow/panic_invariants_test.go diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index 766196a4ce8..924c381aff3 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -577,9 +577,10 @@ func NewEngineRegistry() *EngineRegistry { } for _, engine := range builtins { if err := registry.Register(engine); err != nil { - // Build-time invariant: the built-in engines list above has fixed, non-empty - // names, so registration can only fail if a future refactor introduces a - // duplicate or empty name — a programming error caught by CI, not user input. + // 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)) } } diff --git a/pkg/workflow/model_aliases.go b/pkg/workflow/model_aliases.go index 46edd5b9428..f4027d7daa9 100644 --- a/pkg/workflow/model_aliases.go +++ b/pkg/workflow/model_aliases.go @@ -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 }) @@ -148,7 +148,9 @@ func isBuiltinOnlyAliasMap(m map[string][]string) bool { func BuiltinModelAliases() map[string][]string { data, err := loadBuiltinModelAliases() if err != nil { - // Build-time invariant: see comment in getBuiltinOnlyAliasMap above. + // 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. diff --git a/pkg/workflow/panic_invariants_test.go b/pkg/workflow/panic_invariants_test.go new file mode 100644 index 00000000000..bb6bc0f5d9a --- /dev/null +++ b/pkg/workflow/panic_invariants_test.go @@ -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)) +} diff --git a/pkg/workflow/pi_engine.go b/pkg/workflow/pi_engine.go index d4e8de1fee6..45429f89c42 100644 --- a/pkg/workflow/pi_engine.go +++ b/pkg/workflow/pi_engine.go @@ -144,9 +144,9 @@ func buildPiModelsJSON(gatewayPort int, secretEnvVarName, modelID string) string } b, err := json.Marshal(payload) if err != nil { - // Build-time invariant: payload above is built entirely from string/map - // literals, so Marshal cannot fail; json.Marshal only errors for - // non-serialisable types such as channels or functions. + // 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)