Migrate deprecated timeout constants to time.Duration types

pkg/constants/constants.go

@@ -402,22 +402,6 @@ const DefaultToolTimeout = 60 * time.Second
 // DefaultMCPStartupTimeout is the default timeout for MCP server startup
 const DefaultMCPStartupTimeout = 120 * time.Second
 
-// Legacy timeout constants for backward compatibility (deprecated)
-// These are kept for existing code that expects integer values
-// TODO: Remove these after all call sites are migrated to use time.Duration
-
-// DefaultAgenticWorkflowTimeoutMinutes is the default timeout for agentic workflow execution in minutes
-// Deprecated: Use DefaultAgenticWorkflowTimeout instead
-const DefaultAgenticWorkflowTimeoutMinutes = int(DefaultAgenticWorkflowTimeout / time.Minute)
-
-// DefaultToolTimeoutSeconds is the default timeout for tool/MCP server operations in seconds
-// Deprecated: Use DefaultToolTimeout instead
-const DefaultToolTimeoutSeconds = int(DefaultToolTimeout / time.Second)
-
-// DefaultMCPStartupTimeoutSeconds is the default timeout for MCP server startup in seconds
-// Deprecated: Use DefaultMCPStartupTimeout instead
-const DefaultMCPStartupTimeoutSeconds = int(DefaultMCPStartupTimeout / time.Second)
-
 // DefaultActivationJobRunnerImage is the default runner image for activation and pre-activation jobs
 const DefaultActivationJobRunnerImage = "ubuntu-slim"
 

pkg/constants/constants_test.go

@@ -347,41 +347,6 @@ func TestTimeoutConstants(t *testing.T) {
 			}
 		})
 	}
-
-	// Test backward compatibility with legacy integer constants
-	legacyTests := []struct {
-		name     string
-		value    int
-		minValue int
-	}{
-		{"DefaultAgenticWorkflowTimeoutMinutes", DefaultAgenticWorkflowTimeoutMinutes, 1},
-		{"DefaultToolTimeoutSeconds", DefaultToolTimeoutSeconds, 1},
-		{"DefaultMCPStartupTimeoutSeconds", DefaultMCPStartupTimeoutSeconds, 1},
-	}
-
-	for _, tt := range legacyTests {
-		t.Run(tt.name+"_legacy", func(t *testing.T) {
-			if tt.value < tt.minValue {
-				t.Errorf("%s = %d, should be >= %d", tt.name, tt.value, tt.minValue)
-			}
-		})
-	}
-
-	// Test that legacy constants match the Duration-based values
-	t.Run("legacy_compatibility", func(t *testing.T) {
-		if DefaultAgenticWorkflowTimeoutMinutes != int(DefaultAgenticWorkflowTimeout/time.Minute) {
-			t.Errorf("DefaultAgenticWorkflowTimeoutMinutes (%d) doesn't match DefaultAgenticWorkflowTimeout (%v)",
-				DefaultAgenticWorkflowTimeoutMinutes, DefaultAgenticWorkflowTimeout)
-		}
-		if DefaultToolTimeoutSeconds != int(DefaultToolTimeout/time.Second) {
-			t.Errorf("DefaultToolTimeoutSeconds (%d) doesn't match DefaultToolTimeout (%v)",
-				DefaultToolTimeoutSeconds, DefaultToolTimeout)
-		}
-		if DefaultMCPStartupTimeoutSeconds != int(DefaultMCPStartupTimeout/time.Second) {
-			t.Errorf("DefaultMCPStartupTimeoutSeconds (%d) doesn't match DefaultMCPStartupTimeout (%v)",
-				DefaultMCPStartupTimeoutSeconds, DefaultMCPStartupTimeout)
-		}
-	})
 }
 
 func TestFeatureFlagConstants(t *testing.T) {

pkg/workflow/claude_engine.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"time"
 
 	"github.com/githubnext/gh-aw/pkg/constants"
 	"github.com/githubnext/gh-aw/pkg/logger"
@@ -435,14 +436,14 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str
 	}
 
 	// Set timeout environment variables for Claude Code
-	// Use tools.startup-timeout if specified, otherwise default to DefaultMCPStartupTimeoutSeconds
-	startupTimeoutMs := constants.DefaultMCPStartupTimeoutSeconds * 1000 // convert seconds to milliseconds
+	// Use tools.startup-timeout if specified, otherwise default to DefaultMCPStartupTimeout
+	startupTimeoutMs := int(constants.DefaultMCPStartupTimeout / time.Millisecond)
 	if workflowData.ToolsStartupTimeout > 0 {
 		startupTimeoutMs = workflowData.ToolsStartupTimeout * 1000 // convert seconds to milliseconds
 	}
 
-	// Use tools.timeout if specified, otherwise default to DefaultToolTimeoutSeconds
-	timeoutMs := constants.DefaultToolTimeoutSeconds * 1000 // convert seconds to milliseconds
+	// Use tools.timeout if specified, otherwise default to DefaultToolTimeout
+	timeoutMs := int(constants.DefaultToolTimeout / time.Millisecond)
 	if workflowData.ToolsTimeout > 0 {
 		timeoutMs = workflowData.ToolsTimeout * 1000 // convert seconds to milliseconds
 	}
@@ -531,7 +532,7 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str
 		timeoutValue = strings.TrimPrefix(timeoutValue, "timeout-minutes: ")
 		stepLines = append(stepLines, fmt.Sprintf("        timeout-minutes: %s", timeoutValue))
 	} else {
-		stepLines = append(stepLines, fmt.Sprintf("        timeout-minutes: %d", constants.DefaultAgenticWorkflowTimeoutMinutes)) // Default timeout for agentic workflows
+		stepLines = append(stepLines, fmt.Sprintf("        timeout-minutes: %d", int(constants.DefaultAgenticWorkflowTimeout/time.Minute))) // Default timeout for agentic workflows
 	}
 
 	// Filter environment variables to only include allowed secrets

pkg/workflow/compiler_timeout_default_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/githubnext/gh-aw/pkg/stringutil"
 
@@ -28,7 +29,7 @@ permissions:
   contents: read
 engine: copilot
 ---`,
-			expectedTimeout: constants.DefaultAgenticWorkflowTimeoutMinutes,
+			expectedTimeout: int(constants.DefaultAgenticWorkflowTimeout / time.Minute),
 			description:     "When timeout-minutes is not specified, default should be applied",
 		},
 		{
@@ -139,10 +140,11 @@ func TestDefaultTimeoutMinutesConstantValue(t *testing.T) {
 	// If this test fails, it means the constant was changed and documentation
 	// should be updated accordingly
 	expectedDefault := 20
-	if constants.DefaultAgenticWorkflowTimeoutMinutes != expectedDefault {
-		t.Errorf("DefaultAgenticWorkflowTimeoutMinutes constant is %d, but test expects %d. "+
+	actualDefault := int(constants.DefaultAgenticWorkflowTimeout / time.Minute)
+	if actualDefault != expectedDefault {
+		t.Errorf("DefaultAgenticWorkflowTimeout constant is %d minutes, but test expects %d. "+
 			"If you changed the constant, please update the schema documentation in pkg/parser/schemas/main_workflow_schema.json",
-			constants.DefaultAgenticWorkflowTimeoutMinutes, expectedDefault)
+			actualDefault, expectedDefault)
 	}
 }
 
@@ -159,8 +161,8 @@ func TestSchemaDocumentationMatchesConstant(t *testing.T) {
 	if !strings.Contains(string(schemaContent), expectedText) {
 		t.Errorf("Schema documentation does not mention the correct default timeout.\n"+
 			"Expected to find: %q\n"+
-			"Please update the timeout-minutes description in %s to match DefaultAgenticWorkflowTimeoutMinutes constant (%d minutes)",
-			expectedText, schemaPath, constants.DefaultAgenticWorkflowTimeoutMinutes)
+			"Please update the timeout-minutes description in %s to match DefaultAgenticWorkflowTimeout constant (%d minutes)",
+			expectedText, schemaPath, int(constants.DefaultAgenticWorkflowTimeout/time.Minute))
 	}
 
 	// Count occurrences - should appear at least twice (timeout-minutes and timeout_minutes deprecated)

pkg/workflow/copilot_engine_execution.go

@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"time"
 
 	"github.com/githubnext/gh-aw/pkg/constants"
 	"github.com/githubnext/gh-aw/pkg/logger"
@@ -525,7 +526,7 @@ COPILOT_CLI_INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"
 		timeoutValue = strings.TrimPrefix(timeoutValue, "timeout-minutes: ")
 		stepLines = append(stepLines, fmt.Sprintf("        timeout-minutes: %s", timeoutValue))
 	} else {
-		stepLines = append(stepLines, fmt.Sprintf("        timeout-minutes: %d", constants.DefaultAgenticWorkflowTimeoutMinutes)) // Default timeout for agentic workflows
+		stepLines = append(stepLines, fmt.Sprintf("        timeout-minutes: %d", int(constants.DefaultAgenticWorkflowTimeout/time.Minute))) // Default timeout for agentic workflows
 	}
 
 	// Filter environment variables to only include allowed secrets