Skip to content

[dead-code] chore: remove dead functions — 5 functions removed #20835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 1 commit into from
Mar 13, 2026
Merged

[dead-code] chore: remove dead functions — 5 functions removed #20835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
102 changes: 0 additions & 102 deletions pkg/workflow/safe_output_helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,95 +295,6 @@ func TestApplySafeOutputEnvToMap(t *testing.T) {
}

// TestApplySafeOutputEnvToSlice verifies the helper function for YAML string slices
func TestApplySafeOutputEnvToSlice(t *testing.T) {
tests := []struct {
name string
workflowData *WorkflowData
expected []string
}{
{
name: "nil SafeOutputs",
workflowData: &WorkflowData{
SafeOutputs: nil,
},
expected: []string{},
},
{
name: "basic safe outputs",
workflowData: &WorkflowData{
SafeOutputs: &SafeOutputsConfig{},
},
expected: []string{
" GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }}",
},
},
{
name: "safe outputs with staged flag",
workflowData: &WorkflowData{
SafeOutputs: &SafeOutputsConfig{
Staged: true,
},
},
expected: []string{
" GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }}",
" GH_AW_SAFE_OUTPUTS_STAGED: \"true\"",
},
},
{
name: "trial mode",
workflowData: &WorkflowData{
TrialMode: true,
TrialLogicalRepo: "owner/repo",
SafeOutputs: &SafeOutputsConfig{},
},
expected: []string{
" GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }}",
" GH_AW_SAFE_OUTPUTS_STAGED: \"true\"",
" GH_AW_TARGET_REPO_SLUG: \"owner/repo\"",
},
},
{
name: "upload assets config",
workflowData: &WorkflowData{
SafeOutputs: &SafeOutputsConfig{
UploadAssets: &UploadAssetsConfig{
BranchName: "gh-aw-assets",
MaxSizeKB: 10240,
AllowedExts: []string{".png", ".jpg", ".jpeg"},
},
},
},
expected: []string{
" GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }}",
" GH_AW_ASSETS_BRANCH: \"gh-aw-assets\"",
" GH_AW_ASSETS_MAX_SIZE_KB: 10240",
" GH_AW_ASSETS_ALLOWED_EXTS: \".png,.jpg,.jpeg\"",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var stepLines []string
applySafeOutputEnvToSlice(&stepLines, tt.workflowData)

if len(stepLines) != len(tt.expected) {
t.Errorf("Expected %d lines, got %d", len(tt.expected), len(stepLines))
}

for i, expectedLine := range tt.expected {
if i >= len(stepLines) {
t.Errorf("Missing expected line %d: %q", i, expectedLine)
continue
}
if stepLines[i] != expectedLine {
t.Errorf("Line %d: expected %q, got %q", i, expectedLine, stepLines[i])
}
}
})
}
}

// TestBuildWorkflowMetadataEnvVars verifies the helper function for workflow metadata env vars
func TestBuildWorkflowMetadataEnvVars(t *testing.T) {
tests := []struct {
Expand Down Expand Up @@ -663,11 +574,6 @@ func TestEnginesUseSameHelperLogic(t *testing.T) {
envMap := make(map[string]string)
applySafeOutputEnvToMap(envMap, workflowData)

// Test slice-based helper (used by claude)
var stepLines []string
applySafeOutputEnvToSlice(&stepLines, workflowData)

// Verify both approaches produce the same env vars
expectedKeys := []string{
"GH_AW_SAFE_OUTPUTS",
"GH_AW_SAFE_OUTPUTS_STAGED",
Comment on lines 574 to 579
Expand All @@ -683,14 +589,6 @@ func TestEnginesUseSameHelperLogic(t *testing.T) {
t.Errorf("envMap missing expected key: %s", key)
}
}

// Check slice (should contain all keys)
sliceContent := strings.Join(stepLines, "\n")
for _, key := range expectedKeys {
if !strings.Contains(sliceContent, key) {
t.Errorf("stepLines missing expected key: %s", key)
}
}
}

// TestBuildAgentOutputDownloadSteps verifies the agent output download steps
Expand Down
25 changes: 0 additions & 25 deletions pkg/workflow/safe_outputs_jobs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,31 +436,6 @@ func applySafeOutputEnvToMap(env map[string]string, data *WorkflowData) {
}
}

// applySafeOutputEnvToSlice adds safe-output related environment variables to a YAML string slice
// This is for engines that build YAML line-by-line (like Claude)
func applySafeOutputEnvToSlice(stepLines *[]string, workflowData *WorkflowData) {
if workflowData.SafeOutputs == nil {
return
}

*stepLines = append(*stepLines, " GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }}")

// Add staged flag if specified
if workflowData.TrialMode || workflowData.SafeOutputs.Staged {
*stepLines = append(*stepLines, " GH_AW_SAFE_OUTPUTS_STAGED: \"true\"")
}
if workflowData.TrialMode && workflowData.TrialLogicalRepo != "" {
*stepLines = append(*stepLines, fmt.Sprintf(" GH_AW_TARGET_REPO_SLUG: %q", workflowData.TrialLogicalRepo))
}

// Add branch name if upload assets is configured
if workflowData.SafeOutputs.UploadAssets != nil {
*stepLines = append(*stepLines, fmt.Sprintf(" GH_AW_ASSETS_BRANCH: %q", workflowData.SafeOutputs.UploadAssets.BranchName))
*stepLines = append(*stepLines, fmt.Sprintf(" GH_AW_ASSETS_MAX_SIZE_KB: %d", workflowData.SafeOutputs.UploadAssets.MaxSizeKB))
*stepLines = append(*stepLines, fmt.Sprintf(" GH_AW_ASSETS_ALLOWED_EXTS: %q", strings.Join(workflowData.SafeOutputs.UploadAssets.AllowedExts, ",")))
}
}

// buildWorkflowMetadataEnvVars builds workflow name and source environment variables
// This extracts the duplicated workflow metadata setup logic from safe-output job builders
func buildWorkflowMetadataEnvVars(workflowName string, workflowSource string) []string {
Expand Down
10 changes: 0 additions & 10 deletions pkg/workflow/safe_outputs_runtime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,6 @@ func (c *Compiler) formatSafeOutputsRunsOn(safeOutputs *SafeOutputsConfig) strin
return "runs-on: " + safeOutputs.RunsOn
}

// formatDetectionRunsOn resolves the runner for the detection job using the following priority:
// 1. safe-outputs.detection.runs-on (detection-specific override)
// 2. agentRunsOn (the agent job's runner, passed by the caller)
func (c *Compiler) formatDetectionRunsOn(safeOutputs *SafeOutputsConfig, agentRunsOn string) string {
if safeOutputs != nil && safeOutputs.ThreatDetection != nil && safeOutputs.ThreatDetection.RunsOn != "" {
return "runs-on: " + safeOutputs.ThreatDetection.RunsOn
}
return agentRunsOn
}

// usesPatchesAndCheckouts checks if the workflow uses safe outputs that require
// git patches and checkouts (create-pull-request or push-to-pull-request-branch)
func usesPatchesAndCheckouts(safeOutputs *SafeOutputsConfig) bool {
Expand Down
50 changes: 0 additions & 50 deletions pkg/workflow/safe_outputs_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,42 +91,6 @@ func hasAnySafeOutputEnabled(safeOutputs *SafeOutputsConfig) bool {
return false
}

// getEnabledSafeOutputToolNamesReflection uses reflection to get enabled tool names.
// Results are sorted for deterministic compilation output.
//
// NOTE: Reflection is used here to avoid a large switch statement that would need
// updating every time a new safe-output type is added. The cost is acceptable
// because this function is called infrequently (once per compilation).
func getEnabledSafeOutputToolNamesReflection(safeOutputs *SafeOutputsConfig) []string {
if safeOutputs == nil {
return nil
}

safeOutputReflectionLog.Print("Getting enabled safe output tool names using reflection")
var tools []string

// Use reflection to check all pointer fields
val := reflect.ValueOf(safeOutputs).Elem()
for fieldName, toolName := range safeOutputFieldMapping {
field := val.FieldByName(fieldName)
if field.IsValid() && !field.IsNil() {
tools = append(tools, toolName)
}
}

// Add custom job tools
for jobName := range safeOutputs.Jobs {
tools = append(tools, jobName)
safeOutputReflectionLog.Printf("Added custom job tool: %s", jobName)
}

// Sort tools to ensure deterministic compilation
sort.Strings(tools)

safeOutputReflectionLog.Printf("Found %d enabled safe output tools", len(tools))
return tools
}

// builtinSafeOutputFields contains the struct field names for the built-in safe output types
// that are excluded from the "non-builtin" check. These are: noop, missing-data, missing-tool.
var builtinSafeOutputFields = map[string]bool{
Expand Down Expand Up @@ -183,20 +147,6 @@ func HasSafeOutputsEnabled(safeOutputs *SafeOutputsConfig) bool {
return enabled
}

// GetEnabledSafeOutputToolNames returns a list of enabled safe output tool names.
// NOTE: Tool names should NOT be included in agent prompts. The agent should query
// the MCP server to discover available tools. This function is used for generating
// the tools.json file that the MCP server provides, and for diagnostic logging.
func GetEnabledSafeOutputToolNames(safeOutputs *SafeOutputsConfig) []string {
tools := getEnabledSafeOutputToolNamesReflection(safeOutputs)

if safeOutputsConfigLog.Enabled() {
safeOutputsConfigLog.Printf("Enabled safe output tools: %v", tools)
}

return tools
}

// checkAllEnabledToolsPresent verifies that every tool in enabledTools has a matching entry
// in filteredTools. This is a compiler error check: if a safe-output type is registered in
// Go code but its definition is missing from safe-output-tools.json, it will not appear in
Expand Down
115 changes: 0 additions & 115 deletions pkg/workflow/safe_outputs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1057,118 +1057,3 @@ func TestBuildStandardSafeOutputEnvVars(t *testing.T) {
})
}
}

// ========================================
// GetEnabledSafeOutputToolNames Tests
// ========================================

// TestGetEnabledSafeOutputToolNames tests that tool names are returned in sorted order
func TestGetEnabledSafeOutputToolNames(t *testing.T) {
tests := []struct {
name string
safeOutputs *SafeOutputsConfig
expected []string
}{
{
name: "nil safe outputs returns nil",
safeOutputs: nil,
expected: nil,
},
{
name: "empty safe outputs returns empty slice",
safeOutputs: &SafeOutputsConfig{},
expected: nil,
},
{
name: "single tool",
safeOutputs: &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{},
},
expected: []string{"create_issue"},
},
{
name: "multiple tools in alphabetical order",
safeOutputs: &SafeOutputsConfig{
AddComments: &AddCommentsConfig{},
CreateIssues: &CreateIssuesConfig{},
UpdateIssues: &UpdateIssuesConfig{},
},
expected: []string{"add_comment", "create_issue", "update_issue"},
},
{
name: "custom jobs are sorted with standard tools",
safeOutputs: &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{},
UpdateIssues: &UpdateIssuesConfig{},
Jobs: map[string]*SafeJobConfig{
"zzz_custom": {},
"aaa_custom": {},
"middle_custom": {},
},
},
expected: []string{"aaa_custom", "create_issue", "middle_custom", "update_issue", "zzz_custom"},
},
{
name: "all standard tools are sorted",
safeOutputs: &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{},
CreateAgentSessions: &CreateAgentSessionConfig{},
CreateDiscussions: &CreateDiscussionsConfig{},
CloseDiscussions: &CloseDiscussionsConfig{},
CloseIssues: &CloseIssuesConfig{},
ClosePullRequests: &ClosePullRequestsConfig{},
AddComments: &AddCommentsConfig{},
CreatePullRequests: &CreatePullRequestsConfig{},
AddLabels: &AddLabelsConfig{},
AddReviewer: &AddReviewerConfig{},
AssignMilestone: &AssignMilestoneConfig{},
UpdateIssues: &UpdateIssuesConfig{},
UpdatePullRequests: &UpdatePullRequestsConfig{},
SubmitPullRequestReview: &SubmitPullRequestReviewConfig{},
NoOp: &NoOpConfig{},
},
// Expected order is alphabetical
expected: []string{
"add_comment",
"add_labels",
"add_reviewer",
"assign_milestone",
"close_discussion",
"close_issue",
"close_pull_request",
"create_agent_session",
"create_discussion",
"create_issue",
"create_pull_request",
"noop",
"submit_pull_request_review",
"update_issue",
"update_pull_request",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := GetEnabledSafeOutputToolNames(tt.safeOutputs)

if len(result) != len(tt.expected) {
t.Errorf("Expected %d tools, got %d: %v", len(tt.expected), len(result), result)
return
}

for i, tool := range result {
if tool != tt.expected[i] {
t.Errorf("Tool at index %d: expected %q, got %q", i, tt.expected[i], tool)
}
}

// Verify the list is sorted
for i := 1; i < len(result); i++ {
if result[i-1] > result[i] {
t.Errorf("Tools not in sorted order: %q comes after %q", result[i-1], result[i])
}
}
})
}
}
14 changes: 0 additions & 14 deletions pkg/workflow/step_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"maps"

"github.com/github/gh-aw/pkg/logger"
"github.com/goccy/go-yaml"
)

var stepTypesLog = logger.New("workflow:step_types")
Expand Down Expand Up @@ -162,19 +161,6 @@ func (s *WorkflowStep) Clone() *WorkflowStep {
return clone
}

// ToYAML converts the WorkflowStep to YAML string
func (s *WorkflowStep) ToYAML() (string, error) {
stepTypesLog.Printf("Converting step to YAML: name=%s", s.Name)
stepMap := s.ToMap()
yamlBytes, err := yaml.Marshal(stepMap)
if err != nil {
stepTypesLog.Printf("Failed to marshal step to YAML: %v", err)
return "", fmt.Errorf("failed to marshal step to YAML: %w", err)
}
stepTypesLog.Printf("Successfully converted step to YAML: size=%d bytes", len(yamlBytes))
return string(yamlBytes), nil
}

// SliceToSteps converts a slice of any (typically []map[string]any from YAML parsing)
// to a typed slice of WorkflowStep pointers for type-safe manipulation
func SliceToSteps(steps []any) ([]*WorkflowStep, error) {
Expand Down
Loading
Loading