Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 27, 2026

Removes deprecated integer timeout constants (DefaultAgenticWorkflowTimeoutMinutes, DefaultToolTimeoutSeconds, DefaultMCPStartupTimeoutSeconds) in favor of their time.Duration equivalents.

Changes

Constants removed (pkg/constants/constants.go):

  • 3 deprecated integer constants and their TODO comments (16 lines)

Call sites updated (7 locations across 4 files):

  • pkg/workflow/tools.go - Workflow timeout: int(constants.DefaultAgenticWorkflowTimeout / time.Minute)
  • pkg/workflow/copilot_engine_execution.go - Step timeout: int(constants.DefaultAgenticWorkflowTimeout / time.Minute)
  • pkg/workflow/claude_engine.go - MCP/tool timeouts: int(constants.DefaultMCPStartupTimeout / time.Millisecond), int(constants.DefaultToolTimeout / time.Millisecond)
  • pkg/workflow/mcp_renderer.go - MCP/tool timeouts: int(constants.DefaultMCPStartupTimeout / time.Second), int(constants.DefaultToolTimeout / time.Second)

Tests updated:

  • Removed legacy constant validation tests from pkg/constants/constants_test.go (35 lines)
  • Updated pkg/workflow/compiler_timeout_default_test.go to use time.Duration conversions

Example

// Before
timeout := constants.DefaultAgenticWorkflowTimeoutMinutes

// After  
timeout := int(constants.DefaultAgenticWorkflowTimeout / time.Minute)

Improves type safety by making duration units explicit at conversion points rather than hiding them in constant definitions.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Code Quality] Migrate deprecated timeout constants to time.Duration types</issue_title>
<issue_description>## Description

The codebase contains deprecated integer timeout constants that should be migrated to time.Duration types for better type safety and consistency.

Current State

Deprecated constants in pkg/constants/constants.go:

  • DefaultAgenticWorkflowTimeoutMinutes (int)
  • DefaultToolTimeoutSeconds (int)
  • DefaultMCPStartupTimeoutSeconds (int)

Modern replacements already exist:

  • DefaultAgenticWorkflowTimeout (time.Duration)
  • DefaultToolTimeout (time.Duration)
  • DefaultMCPStartupTimeout (time.Duration)

Affected Files

Still using deprecated constants:

  • pkg/workflow/tools.go:145
  • pkg/workflow/copilot_engine_execution.go:503
  • pkg/workflow/claude_engine.go:423-424,429-430,519
  • pkg/workflow/compiler_timeout_default_test.go (multiple lines)
  • pkg/workflow/mcp_renderer.go:367-368,374-375

Suggested Changes

  1. Update call sites to use time.Duration constants:

    // Before:
    timeout := constants.DefaultAgenticWorkflowTimeoutMinutes
    
    // After:
    timeout := int(constants.DefaultAgenticWorkflowTimeout / time.Minute)
  2. Remove deprecated constants from pkg/constants/constants.go (lines 407-419)

  3. Update tests in pkg/constants/constants_test.go to remove deprecated constant tests

Success Criteria

  • ✅ All 6+ call sites migrated to use time.Duration constants
  • ✅ Deprecated constants removed from pkg/constants/constants.go
  • ✅ All tests pass (make test-unit)
  • ✅ Code formatted and linted (make fmt && make lint)

Benefits

  • Type safety: Using time.Duration provides compile-time type checking
  • Consistency: Aligns with modern Go best practices
  • Maintainability: Reduces technical debt
  • Clarity: Makes duration units explicit (no confusion about seconds vs minutes)

Source

Extracted from codebase analysis - TODO comment at pkg/constants/constants.go:407

Priority

Medium - Technical debt cleanup that improves code quality

AI generated by Discussion Task Miner - Code Quality Improvement Agent

  • expires on Feb 10, 2026, 5:12 AM UTC

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Migrate deprecated timeout constants to time.Duration types Migrate deprecated timeout constants to time.Duration types Jan 27, 2026
Copilot AI requested a review from pelikhan January 27, 2026 19:37
@pelikhan pelikhan marked this pull request as ready for review January 27, 2026 20:04
@pelikhan pelikhan merged commit d82ce32 into main Jan 27, 2026
158 checks passed
@pelikhan pelikhan deleted the copilot/migrate-timeout-constants branch January 27, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code Quality] Migrate deprecated timeout constants to time.Duration types

2 participants