[dead-code] chore: remove dead functions — 4 functions removed by github-actions[bot] · Pull Request #23882 · github/gh-aw

github-actions · 2026-04-01T12:18:02Z

Dead Code Removal

This PR removes unreachable Go functions identified by the deadcode static analyzer.

Functions Removed

Function	File
`Compiler.buildInlineDetectionSteps`	`pkg/workflow/threat_detection.go`
`Compiler.formatSafeOutputsRunsOn`	`pkg/workflow/safe_outputs_runtime.go`
`extractSafeScriptsFromFrontmatter`	`pkg/workflow/safe_scripts.go`
`ErrorCollector.HasErrors`	`pkg/workflow/workflow_errors.go`

Notes

buildInlineDetectionSteps was a one-line deprecated wrapper around buildDetectionJobSteps; test callers were updated to call buildDetectionJobSteps directly.
formatSafeOutputsRunsOn was superseded by the more general formatFrameworkJobRunsOn.
extractSafeScriptsFromFrontmatter was an internal helper with no production callers.
ErrorCollector.HasErrors was redundant — Count() > 0 is equivalent; test assertions updated.

Tests Removed

Test	File
`TestBuildInlineDetectionSteps`	`pkg/workflow/threat_detection_test.go`
`TestFormatSafeOutputsRunsOn`	`pkg/workflow/safe_outputs_test.go`
`TestFormatSafeOutputsRunsOnEdgeCases`	`pkg/workflow/safe_outputs_runs_on_test.go`
`TestExtractSafeScriptsFromFrontmatter`	`pkg/workflow/safe_scripts_test.go`
`TestExtractSafeScriptsFromFrontmatterEmpty`	`pkg/workflow/safe_scripts_test.go`

Verification

✅ go build ./... — passes
✅ go vet ./... — passes
✅ go vet -tags=integration ./... — passes
✅ make fmt — no changes needed
✅ go test ./pkg/workflow/... — all tests pass

Dead Function Count

Before this batch: ~19 functions
Removed in this PR: 4 functions
Remaining: ~15 functions

Automated by Dead Code Removal workflow — https://github.com/github/gh-aw/actions/runs/23847489282

Generated by Dead Code Removal Agent · ◷

expires on Apr 4, 2026, 12:17 PM UTC

✨ PR Review Safe Output Test - Run 23848854083

💥 [THE END] — Illustrated by Smoke Claude · ◷

Remove unreachable Go functions identified by the deadcode static analyzer: - Compiler.buildInlineDetectionSteps (deprecated wrapper around buildDetectionJobSteps) - Compiler.formatSafeOutputsRunsOn (superseded by formatFrameworkJobRunsOn) - extractSafeScriptsFromFrontmatter (unused internal helper) - ErrorCollector.HasErrors (redundant with Count() > 0) Update test callers: - Replace buildInlineDetectionSteps with buildDetectionJobSteps in threat detection tests - Remove exclusive TestBuildInlineDetectionSteps, TestFormatSafeOutputsRunsOn, TestFormatSafeOutputsRunsOnEdgeCases, TestExtractSafeScriptsFromFrontmatter, and TestExtractSafeScriptsFromFrontmatterEmpty test functions - Replace HasErrors() assertions with Count() equivalents Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Removes unreachable Go code (and associated tests) flagged by the deadcode analyzer to reduce maintenance surface area in the workflow compiler.

Changes:

Removed 4 dead/unreachable functions from the workflow package.
Updated remaining tests to call the non-deprecated implementations directly (e.g., buildDetectionJobSteps, Count()).
Deleted tests that only covered the removed dead functions and cleaned up related imports.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/workflow/workflow_errors.go	Removes redundant `ErrorCollector.HasErrors` helper.
pkg/workflow/error_aggregation_test.go	Updates assertions to use `Count()` instead of `HasErrors()`.
pkg/workflow/threat_detection.go	Removes deprecated wrapper `buildInlineDetectionSteps`.
pkg/workflow/threat_detection_test.go	Removes wrapper-specific test and updates callers to `buildDetectionJobSteps`.
pkg/workflow/threat_detection_file_access_test.go	Updates callers to `buildDetectionJobSteps` and trims now-stale commentary.
pkg/workflow/safe_outputs_runtime.go	Removes superseded `formatSafeOutputsRunsOn` helper.
pkg/workflow/safe_outputs_test.go	Removes test that only covered removed `formatSafeOutputsRunsOn` and drops unused import.
pkg/workflow/safe_outputs_runs_on_test.go	Removes edge-case test that only covered removed `formatSafeOutputsRunsOn`.
pkg/workflow/safe_scripts.go	Removes unused internal helper `extractSafeScriptsFromFrontmatter`.
pkg/workflow/safe_scripts_test.go	Removes tests that only covered removed `extractSafeScriptsFromFrontmatter`.

Comments suppressed due to low confidence (2)

pkg/workflow/error_aggregation_test.go:83

These back-to-back assertions duplicate the same collector.Count() == 0 check with slightly different messages. Consider removing one to keep the test focused and avoid redundant failures.

	assert.Equal(t, 0, collector.Count(), "Should not have errors")
	assert.Equal(t, 0, collector.Count(), "Should have zero count")

pkg/workflow/threat_detection_test.go:213

This test (and its comments/error messages) still refer to “inline detection steps”, but the code now calls buildDetectionJobSteps, and the PR description notes detection runs in a separate detection job. Updating the test name/comments/messages to match the current behavior would reduce confusion for future maintainers.

func TestThreatDetectionInlineStepsDependencies(t *testing.T) {
	// Test that inline detection steps are generated when threat detection is enabled
	// and that safe-output jobs can check detection results via agent job outputs
	compiler := NewCompiler()

	data := &WorkflowData{
		SafeOutputs: &SafeOutputsConfig{
			ThreatDetection: &ThreatDetectionConfig{},
		},
	}

	// Build inline detection steps
	steps := compiler.buildDetectionJobSteps(data)
	if steps == nil {
		t.Fatal("Expected inline detection steps to be created")
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T12:38:21Z

pkg/workflow/error_aggregation_test.go

 			assert.Equal(t, tt.failFast, collector.failFast, "Fail-fast setting should match")
-			assert.False(t, collector.HasErrors(), "New collector should have no errors")
+			assert.Equal(t, 0, collector.Count(), "New collector should have no errors")
 			assert.Equal(t, 0, collector.Count(), "New collector should have zero count")


These two assertions both call collector.Count() with essentially the same expectation/message. Consider keeping a single assertion here to avoid redundant checks and reduce noise in the test output when it fails.

This issue also appears on line 82 of the same file.

Suggested change

assert.Equal(t, 0, collector.Count(), "New collector should have zero count")

Copilot · 2026-04-01T12:38:22Z

pkg/workflow/safe_scripts_test.go

 // TestExtractSafeScriptsFromFrontmatter verifies extraction from frontmatter
-func TestExtractSafeScriptsFromFrontmatter(t *testing.T) {
-	frontmatter := map[string]any{
-		"safe-outputs": map[string]any{
-			"scripts": map[string]any{
-				"my-handler": map[string]any{
-					"description": "A custom handler",
-					// Users write only the body — no module.exports or main declaration needed
-					"script": "return async (m) => ({ success: true });",
-				},
-			},
-		},
-	}
-
-	result := extractSafeScriptsFromFrontmatter(frontmatter)
-
-	require.Len(t, result, 1, "Should have one script")
-	script, exists := result["my-handler"]
-	require.True(t, exists, "Should have my-handler script")
-	assert.Equal(t, "A custom handler", script.Description, "Description should match")
-}
-
-// TestExtractSafeScriptsFromFrontmatterEmpty verifies empty result when no scripts
-func TestExtractSafeScriptsFromFrontmatterEmpty(t *testing.T) {
-	frontmatter := map[string]any{
-		"safe-outputs": map[string]any{
-			"create-issue": map[string]any{},
-		},
-	}
-
-	result := extractSafeScriptsFromFrontmatter(frontmatter)
-	assert.Empty(t, result, "Should return empty map when no scripts")
-}
-
 // TestBuildCustomSafeOutputScriptsJSON verifies JSON generation for script env var
 func TestBuildCustomSafeOutputScriptsJSON(t *testing.T) {


This comment says “TestExtractSafeScriptsFromFrontmatter verifies extraction from frontmatter”, but the TestExtractSafeScriptsFromFrontmatter* tests were removed. Please remove or update the comment so it doesn’t suggest a missing test.

github-actions

💥 Automated smoke test review - all systems nominal!

💥 [THE END] — Illustrated by Smoke Claude

github-actions · 2026-04-01T12:46:24Z

pkg/workflow/error_aggregation_test.go

 			require.NotNil(t, collector, "Collector should be created")
 			assert.Equal(t, tt.failFast, collector.failFast, "Fail-fast setting should match")
-			assert.False(t, collector.HasErrors(), "New collector should have no errors")
+			assert.Equal(t, 0, collector.Count(), "New collector should have no errors")


Good change — using collector.Count() directly instead of the removed HasErrors() method makes the intent clearer and eliminates the need for a separate predicate method.

github-actions · 2026-04-01T12:46:25Z

pkg/workflow/safe_scripts.go

-	return make(map[string]*SafeScriptConfig)
-}
-
 // isSafeScriptName returns true if the script name is safe for use as a filename component.


Removing extractSafeScriptsFromFrontmatter keeps the package surface clean. The parseSafeScriptsConfig helper below is the right entry point for callers that already have the scripts map.

github-actions bot added chore dead-code labels Apr 1, 2026

pelikhan marked this pull request as ready for review April 1, 2026 12:35

Copilot AI review requested due to automatic review settings April 1, 2026 12:35

Copilot started reviewing on behalf of pelikhan April 1, 2026 12:36 View session

Copilot AI approved these changes Apr 1, 2026

View reviewed changes

pelikhan merged commit fb402ad into main Apr 1, 2026
11 checks passed

pelikhan deleted the dead-code-removal-2026-04-01-3bfe306df8c2f8a3 branch April 1, 2026 12:43

github-actions bot commented Apr 1, 2026

View reviewed changes

github-actions bot mentioned this pull request Apr 1, 2026

Smoke Test: Copilot - 23849538374 #23893

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dead-code] chore: remove dead functions — 4 functions removed#23882

[dead-code] chore: remove dead functions — 4 functions removed#23882
pelikhan merged 1 commit intomainfrom
dead-code-removal-2026-04-01-3bfe306df8c2f8a3

github-actions bot commented Apr 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot Apr 1, 2026

Uh oh!

github-actions bot Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

github-actions bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dead Code Removal

Functions Removed

Notes

Tests Removed

Verification

Dead Function Count

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Apr 1, 2026 •

edited

Loading