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
4 changes: 4 additions & 0 deletions DEADCODE.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ make fmt
- `compiler.ParseWorkflowString`
- `compiler.CompileToYAML`

**`pkg/console/console_wasm.go`** — this file provides WASM-specific stub implementations of many `pkg/console` functions (gated with `//go:build js || wasm`). Before deleting any function from `pkg/console/`, `grep` for it in `console_wasm.go`. If the function is called there, either inline the logic in `console_wasm.go` or delete the call. Batch 10 mistake: deleted `renderTreeSimple` from `render.go` but `console_wasm.go`'s `RenderTree` still called it, breaking the WASM build. Fix: replaced the `RenderTree` body in `console_wasm.go` with an inlined closure that no longer calls the deleted helper.

Comment on lines +39 to +40

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

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

PR title/description focus on removing dead code from the parser package, but this PR also removes dead functions/tests across several other packages (workflow, stringutil, sliceutil, repoutil, logger, fileutil). Please update the PR title/description to reflect the broader scope so reviewers understand the impact.

Copilot uses AI. Check for mistakes.
**`compiler_test_helpers.go`** — shows 3 dead functions but serves as shared test infrastructure for ≥15 test files. Do not delete.

**Constant/embed rescue** — Some otherwise-dead files contain live constants or `//go:embed` directives. Extract them before deleting the file.
Expand Down Expand Up @@ -163,7 +165,9 @@ For each batch:
- [ ] Delete the function
- [ ] Delete test functions that exclusively call the deleted function (not shared helpers)
- [ ] Check for now-unused imports in edited files
- [ ] If editing `pkg/console/`, check `pkg/console/console_wasm.go` for calls to the deleted functions
- [ ] `go build ./...`
- [ ] `GOARCH=wasm GOOS=js go build ./pkg/console/...` (if `pkg/console/` was touched)
- [ ] `go vet ./...`
- [ ] `go vet -tags=integration ./...`
- [ ] `make fmt`
Expand Down
71 changes: 0 additions & 71 deletions pkg/cli/audit_report_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,81 +10,10 @@ import (
"testing"
"time"

"github.com/github/gh-aw/pkg/fileutil"
"github.com/github/gh-aw/pkg/stringutil"
"github.com/github/gh-aw/pkg/testutil"
)

func TestCalculateDirectorySize(t *testing.T) {
tests := []struct {
name string
setup func(t *testing.T) string
expected int64
}{
{
name: "empty directory",
setup: func(t *testing.T) string {
dir := testutil.TempDir(t, "test-*")
return dir
},
expected: 0,
},
{
name: "single file",
setup: func(t *testing.T) string {
dir := testutil.TempDir(t, "test-*")
err := os.WriteFile(filepath.Join(dir, "test.txt"), []byte("hello"), 0644)
if err != nil {
t.Fatal(err)
}
return dir
},
expected: 5,
},
{
name: "multiple files in nested directories",
setup: func(t *testing.T) string {
dir := testutil.TempDir(t, "test-*")
// File 1: 10 bytes
err := os.WriteFile(filepath.Join(dir, "file1.txt"), []byte("0123456789"), 0644)
if err != nil {
t.Fatal(err)
}
// Create subdirectory
subdir := filepath.Join(dir, "subdir")
err = os.Mkdir(subdir, 0755)
if err != nil {
t.Fatal(err)
}
// File 2: 5 bytes
err = os.WriteFile(filepath.Join(subdir, "file2.txt"), []byte("hello"), 0644)
if err != nil {
t.Fatal(err)
}
return dir
},
expected: 15,
},
{
name: "nonexistent directory",
setup: func(t *testing.T) string {
return "/nonexistent/path/that/does/not/exist"
},
expected: 0,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dir := tt.setup(t)
got := fileutil.CalculateDirectorySize(dir)
if got != tt.expected {
t.Errorf("fileutil.CalculateDirectorySize() = %d, want %d", got, tt.expected)
}
})
}
}

func TestParseDurationString(t *testing.T) {
tests := []struct {
name string
Expand Down
13 changes: 6 additions & 7 deletions pkg/cli/docker_images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ package cli

import (
"context"
"strings"
"testing"
"time"

"github.com/github/gh-aw/pkg/sliceutil"
)

func TestCheckAndPrepareDockerImages_NoToolsRequested(t *testing.T) {
Expand Down Expand Up @@ -39,7 +38,7 @@ func TestCheckAndPrepareDockerImages_ImageAlreadyDownloading(t *testing.T) {
// Error message should mention downloading and retry
if err != nil {
errMsg := err.Error()
if !sliceutil.ContainsAny(errMsg, "downloading", "retry") {
if !strings.Contains(errMsg, "downloading") && !strings.Contains(errMsg, "retry") {
t.Errorf("Expected error to mention downloading and retry, got: %s", errMsg)
}
}
Expand Down Expand Up @@ -110,7 +109,7 @@ func TestDockerImageConstants(t *testing.T) {
}

for name, image := range expectedImages {
if !sliceutil.ContainsAny(image, "/", ":") {
if !strings.Contains(image, "/") && !strings.Contains(image, ":") {
t.Errorf("%s image %s does not look like a Docker image reference", name, image)
}
}
Expand Down Expand Up @@ -138,7 +137,7 @@ func TestCheckAndPrepareDockerImages_MultipleImages(t *testing.T) {
// Error should mention downloading images
if err != nil {
errMsg := err.Error()
if !sliceutil.ContainsAny(errMsg, "downloading", "retry") {
if !strings.Contains(errMsg, "downloading") && !strings.Contains(errMsg, "retry") {
t.Errorf("Expected error to mention downloading and retry, got: %s", errMsg)
}
}
Expand Down Expand Up @@ -172,7 +171,7 @@ func TestCheckAndPrepareDockerImages_RetryMessageFormat(t *testing.T) {
}

for _, expected := range expectations {
if !sliceutil.ContainsAny(errMsg, expected) {
if !strings.Contains(errMsg, expected) {
t.Errorf("Expected error message to contain '%s', got: %s", expected, errMsg)
}
}
Expand All @@ -199,7 +198,7 @@ func TestCheckAndPrepareDockerImages_StartedDownloadingMessage(t *testing.T) {
errMsg := err.Error()

// Should contain zizmor since it's downloading
if !sliceutil.ContainsAny(errMsg, "zizmor") {
if !strings.Contains(errMsg, "zizmor") {
t.Errorf("Expected error message to mention zizmor, got: %s", errMsg)
}

Expand Down
19 changes: 0 additions & 19 deletions pkg/fileutil/fileutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,22 +101,3 @@ func CopyFile(src, dst string) error {
log.Printf("File copied successfully: src=%s, dst=%s", src, dst)
return out.Sync()
}

// CalculateDirectorySize recursively calculates the total size of files in a directory.
func CalculateDirectorySize(dirPath string) int64 {
log.Printf("Calculating directory size: %s", dirPath)
var totalSize int64

_ = filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error {
if err != nil {
return nil
}
if !info.IsDir() {
totalSize += info.Size()
}
return nil
})

log.Printf("Directory size: path=%s, size=%d bytes", dirPath, totalSize)
return totalSize
}
14 changes: 0 additions & 14 deletions pkg/logger/slog_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package logger
import (
"context"
"fmt"
"io"
"log/slog"
"strings"
)
Expand Down Expand Up @@ -91,21 +90,8 @@ func formatSlogValue(v any) string {
return slog.AnyValue(v).String()
}

// NewSlogLogger creates a new slog.Logger that uses gh-aw's logger package
// This allows integration with libraries that expect slog.Logger
func NewSlogLogger(namespace string) *slog.Logger {
logger := New(namespace)
handler := NewSlogHandler(logger)
return slog.New(handler)
}

// NewSlogLoggerWithHandler creates a new slog.Logger using an existing Logger instance
func NewSlogLoggerWithHandler(logger *Logger) *slog.Logger {
handler := NewSlogHandler(logger)
return slog.New(handler)
}

// Discard returns a slog.Logger that discards all output
func Discard() *slog.Logger {
return slog.New(slog.NewTextHandler(io.Discard, nil))
}
85 changes: 0 additions & 85 deletions pkg/logger/slog_adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,91 +10,6 @@ import (
"testing"
)

func TestSlogAdapter(t *testing.T) {
// Only run if DEBUG is enabled
if os.Getenv("DEBUG") == "" {
t.Skip("Skipping test: DEBUG environment variable not set")
}

// Capture stderr output
oldStderr := os.Stderr
r, w, _ := os.Pipe()
os.Stderr = w

// Create slog logger using our adapter
slogLogger := NewSlogLogger("test:slog")

// Test different log levels
slogLogger.Info("info message", "key", "value")
slogLogger.Debug("debug message", "count", 42)
slogLogger.Warn("warning message")
slogLogger.Error("error message", "error", "something went wrong")

// Close write end and read output
w.Close()
var buf bytes.Buffer
io.Copy(&buf, r)
output := buf.String()

// Restore stderr
os.Stderr = oldStderr

// Verify output contains expected messages
if !strings.Contains(output, "[INFO] info message") {
t.Errorf("Expected info message in output, got: %s", output)
}
if !strings.Contains(output, "[DEBUG] debug message") {
t.Errorf("Expected debug message in output, got: %s", output)
}
if !strings.Contains(output, "[WARN] warning message") {
t.Errorf("Expected warn message in output, got: %s", output)
}
if !strings.Contains(output, "[ERROR] error message") {
t.Errorf("Expected error message in output, got: %s", output)
}

// Verify attributes are included
if !strings.Contains(output, "key=value") {
t.Errorf("Expected 'key=value' in output, got: %s", output)
}
if !strings.Contains(output, "count=42") {
t.Errorf("Expected 'count=42' in output, got: %s", output)
}
}

func TestSlogAdapterDisabled(t *testing.T) {
// Only run if DEBUG is not set
if os.Getenv("DEBUG") != "" {
t.Skip("Skipping test: DEBUG environment variable is set")
}

// Capture stderr output
oldStderr := os.Stderr
r, w, _ := os.Pipe()
os.Stderr = w

// Create slog logger using our adapter
slogLogger := NewSlogLogger("test:slog")

// Test logging (should be disabled)
slogLogger.Info("info message", "key", "value")
slogLogger.Debug("debug message")

// Close write end and read output
w.Close()
var buf bytes.Buffer
io.Copy(&buf, r)
output := buf.String()

// Restore stderr
os.Stderr = oldStderr

// Verify no output
if output != "" {
t.Errorf("Expected no output when logger is disabled, got: %s", output)
}
}

func TestNewSlogLoggerWithHandler(t *testing.T) {
// Only run if DEBUG is enabled
if os.Getenv("DEBUG") == "" {
Expand Down
88 changes: 0 additions & 88 deletions pkg/parser/frontmatter_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,93 +170,5 @@ Workflow demonstrating array handling in frontmatter.
}

// BenchmarkValidateSchema benchmarks schema validation
func BenchmarkValidateSchema(b *testing.B) {
frontmatter := map[string]any{
"on": "push",
"permissions": map[string]any{
"contents": "read",
"issues": "write",
"pull-requests": "read",
},
"engine": "claude",
"tools": map[string]any{
"github": map[string]any{
"allowed": []any{"issue_read", "add_issue_comment"},
},
"bash": []any{"echo", "ls"},
},
"timeout-minutes": 10,
}

for b.Loop() {
_ = ValidateMainWorkflowFrontmatterWithSchema(frontmatter)
}
}

// BenchmarkValidateSchema_Complex benchmarks schema validation with complex data
func BenchmarkValidateSchema_Complex(b *testing.B) {
frontmatter := map[string]any{
"on": map[string]any{
"pull_request": map[string]any{
"types": []any{"opened", "synchronize", "reopened"},
"forks": []any{"org/*", "user/repo"},
},
},
"permissions": map[string]any{
"contents": "read",
"issues": "write",
"pull-requests": "write",
"actions": "read",
},
"engine": map[string]any{
"id": "copilot",
"max-turns": 5,
"max-concurrency": 3,
"model": "gpt-5",
},
"mcp-servers": map[string]any{
"github": map[string]any{
"mode": "remote",
"toolsets": []any{"default", "actions", "discussions"},
"read-only": false,
},
"playwright": map[string]any{
"container": "mcr.microsoft.com/playwright:v1.41.0",
"allowed-domains": []any{"github.com", "*.github.io"},
},
},
"network": map[string]any{
"allowed": []any{"defaults", "python", "node"},
"firewall": map[string]any{
"version": "v1.0.0",
"log-level": "debug",
},
},
"tools": map[string]any{
"edit": true,
"web-fetch": true,
"web-search": true,
"bash": []any{"git status", "git diff", "npm test"},
},
"safe-outputs": map[string]any{
"create-pull-requests": map[string]any{
"title-prefix": "[ai] ",
"labels": []any{"automation", "ai-generated"},
"draft": true,
},
"add-comments": map[string]any{
"max": 3,
"target": "*",
},
},
"timeout-minutes": 30,
"concurrency": map[string]any{
"group": "workflow-123",
"cancel-in-progress": true,
},
}

for b.Loop() {
_ = ValidateMainWorkflowFrontmatterWithSchema(frontmatter)
}
}
Loading
Loading