Skip to content

Commit dc555cc

Browse files
Copilotpelikhan
andcommitted
Fix: compiler drops blocked constraints from safe-outputs configs inconsistently
- Add blocked field to add_labels config.json generation (safe_outputs_config_generation.go) - Add blocked field to assign_to_user handler config (compiler_safe_outputs_config.go) - Add blocked field to unassign_from_user handler config (compiler_safe_outputs_config.go) - Add tests for all three fixes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent ae12ad7 commit dc555cc

File tree

4 files changed

+142
-0
lines changed

4 files changed

+142
-0
lines changed

pkg/workflow/compiler_safe_outputs_config.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,7 @@ var handlerRegistry = map[string]handlerBuilder{
618618
return newHandlerConfigBuilder().
619619
AddTemplatableInt("max", c.Max).
620620
AddStringSlice("allowed", c.Allowed).
621+
AddStringSlice("blocked", c.Blocked).
621622
AddIfNotEmpty("target", c.Target).
622623
AddIfNotEmpty("target-repo", c.TargetRepoSlug).
623624
AddStringSlice("allowed_repos", c.AllowedRepos).
@@ -632,6 +633,7 @@ var handlerRegistry = map[string]handlerBuilder{
632633
return newHandlerConfigBuilder().
633634
AddTemplatableInt("max", c.Max).
634635
AddStringSlice("allowed", c.Allowed).
636+
AddStringSlice("blocked", c.Blocked).
635637
AddIfNotEmpty("target", c.Target).
636638
AddIfNotEmpty("target-repo", c.TargetRepoSlug).
637639
AddStringSlice("allowed_repos", c.AllowedRepos).

pkg/workflow/compiler_safe_outputs_config_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,3 +1084,103 @@ func TestHandlerConfigUnassignFromUser(t *testing.T) {
10841084
}
10851085
}
10861086
}
1087+
1088+
// TestHandlerConfigAssignToUserWithBlocked tests that blocked patterns are included in assign_to_user handler config
1089+
func TestHandlerConfigAssignToUserWithBlocked(t *testing.T) {
1090+
compiler := NewCompiler()
1091+
1092+
workflowData := &WorkflowData{
1093+
Name: "Test Workflow",
1094+
SafeOutputs: &SafeOutputsConfig{
1095+
AssignToUser: &AssignToUserConfig{
1096+
BaseSafeOutputConfig: BaseSafeOutputConfig{
1097+
Max: strPtr("1"),
1098+
},
1099+
SafeOutputTargetConfig: SafeOutputTargetConfig{
1100+
Target: "*",
1101+
TargetRepoSlug: "microsoft/vscode",
1102+
},
1103+
Blocked: []string{"copilot", "*[bot]"},
1104+
},
1105+
},
1106+
}
1107+
1108+
var steps []string
1109+
compiler.addHandlerManagerConfigEnvVar(&steps, workflowData)
1110+
1111+
for _, step := range steps {
1112+
if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") {
1113+
parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ")
1114+
if len(parts) == 2 {
1115+
jsonStr := strings.TrimSpace(parts[1])
1116+
jsonStr = strings.Trim(jsonStr, "\"")
1117+
jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"")
1118+
1119+
var config map[string]map[string]any
1120+
err := json.Unmarshal([]byte(jsonStr), &config)
1121+
require.NoError(t, err, "Handler config JSON should be valid")
1122+
1123+
assignConfig, ok := config["assign_to_user"]
1124+
require.True(t, ok, "Should have assign_to_user handler")
1125+
1126+
blocked, ok := assignConfig["blocked"]
1127+
require.True(t, ok, "Should have blocked field")
1128+
blockedSlice, ok := blocked.([]any)
1129+
require.True(t, ok, "Blocked should be an array")
1130+
assert.Len(t, blockedSlice, 2, "Should have 2 blocked patterns")
1131+
assert.Equal(t, "copilot", blockedSlice[0], "First blocked pattern should be copilot")
1132+
assert.Equal(t, "*[bot]", blockedSlice[1], "Second blocked pattern should be *[bot]")
1133+
}
1134+
}
1135+
}
1136+
}
1137+
1138+
// TestHandlerConfigUnassignFromUserWithBlocked tests that blocked patterns are included in unassign_from_user handler config
1139+
func TestHandlerConfigUnassignFromUserWithBlocked(t *testing.T) {
1140+
compiler := NewCompiler()
1141+
1142+
workflowData := &WorkflowData{
1143+
Name: "Test Workflow",
1144+
SafeOutputs: &SafeOutputsConfig{
1145+
UnassignFromUser: &UnassignFromUserConfig{
1146+
BaseSafeOutputConfig: BaseSafeOutputConfig{
1147+
Max: strPtr("2"),
1148+
},
1149+
SafeOutputTargetConfig: SafeOutputTargetConfig{
1150+
Target: "*",
1151+
TargetRepoSlug: "microsoft/vscode",
1152+
},
1153+
Blocked: []string{"copilot", "*[bot]"},
1154+
},
1155+
},
1156+
}
1157+
1158+
var steps []string
1159+
compiler.addHandlerManagerConfigEnvVar(&steps, workflowData)
1160+
1161+
for _, step := range steps {
1162+
if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") {
1163+
parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ")
1164+
if len(parts) == 2 {
1165+
jsonStr := strings.TrimSpace(parts[1])
1166+
jsonStr = strings.Trim(jsonStr, "\"")
1167+
jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"")
1168+
1169+
var config map[string]map[string]any
1170+
err := json.Unmarshal([]byte(jsonStr), &config)
1171+
require.NoError(t, err, "Handler config JSON should be valid")
1172+
1173+
unassignConfig, ok := config["unassign_from_user"]
1174+
require.True(t, ok, "Should have unassign_from_user handler")
1175+
1176+
blocked, ok := unassignConfig["blocked"]
1177+
require.True(t, ok, "Should have blocked field")
1178+
blockedSlice, ok := blocked.([]any)
1179+
require.True(t, ok, "Blocked should be an array")
1180+
assert.Len(t, blockedSlice, 2, "Should have 2 blocked patterns")
1181+
assert.Equal(t, "copilot", blockedSlice[0], "First blocked pattern should be copilot")
1182+
assert.Equal(t, "*[bot]", blockedSlice[1], "Second blocked pattern should be *[bot]")
1183+
}
1184+
}
1185+
}
1186+
}

pkg/workflow/safe_outputs_config_generation.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,9 @@ func generateSafeOutputsConfig(data *WorkflowData) string {
274274
if len(data.SafeOutputs.AddLabels.Allowed) > 0 {
275275
additionalFields["allowed"] = data.SafeOutputs.AddLabels.Allowed
276276
}
277+
if len(data.SafeOutputs.AddLabels.Blocked) > 0 {
278+
additionalFields["blocked"] = data.SafeOutputs.AddLabels.Blocked
279+
}
277280
safeOutputsConfig["add_labels"] = generateTargetConfigWithRepos(
278281
data.SafeOutputs.AddLabels.SafeOutputTargetConfig,
279282
data.SafeOutputs.AddLabels.Max,

pkg/workflow/safe_outputs_config_generation_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,3 +328,40 @@ func TestGenerateCustomJobToolDefinitionJSONSerializable(t *testing.T) {
328328
require.NoError(t, json.Unmarshal(data, &parsed), "JSON should be parseable back")
329329
assert.Equal(t, "deploy", parsed["name"], "name should round-trip through JSON")
330330
}
331+
332+
// TestGenerateSafeOutputsConfigAddLabelsBlocked tests that the blocked field is included
333+
// in config.json for add_labels.
334+
func TestGenerateSafeOutputsConfigAddLabelsBlocked(t *testing.T) {
335+
data := &WorkflowData{
336+
SafeOutputs: &SafeOutputsConfig{
337+
AddLabels: &AddLabelsConfig{
338+
BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("5")},
339+
SafeOutputTargetConfig: SafeOutputTargetConfig{
340+
Target: "*",
341+
TargetRepoSlug: "microsoft/vscode",
342+
},
343+
Allowed: []string{"bug", "enhancement"},
344+
Blocked: []string{"[*]*", "~spam", "stale", "triage-needed"},
345+
},
346+
},
347+
}
348+
349+
result := generateSafeOutputsConfig(data)
350+
require.NotEmpty(t, result, "Expected non-empty config")
351+
352+
var parsed map[string]any
353+
require.NoError(t, json.Unmarshal([]byte(result), &parsed), "Result must be valid JSON")
354+
355+
addLabelsConfig, ok := parsed["add_labels"].(map[string]any)
356+
require.True(t, ok, "Expected add_labels key in config")
357+
358+
blocked, ok := addLabelsConfig["blocked"]
359+
require.True(t, ok, "Expected blocked field in add_labels config")
360+
blockedSlice, ok := blocked.([]any)
361+
require.True(t, ok, "Blocked should be an array")
362+
assert.Len(t, blockedSlice, 4, "Should have 4 blocked patterns")
363+
assert.Equal(t, "[*]*", blockedSlice[0], "First blocked pattern should match")
364+
assert.Equal(t, "~spam", blockedSlice[1], "Second blocked pattern should match")
365+
assert.Equal(t, "stale", blockedSlice[2], "Third blocked pattern should match")
366+
assert.Equal(t, "triage-needed", blockedSlice[3], "Fourth blocked pattern should match")
367+
}

0 commit comments

Comments
 (0)