Skip to content

Replace magic time.Sleep literals in pkg/cli with named duration constants#34373

Merged
pelikhan merged 2 commits into
mainfrom
copilot/aw-sg17a2-fix-time-sleep-literals
May 24, 2026
Merged

Replace magic time.Sleep literals in pkg/cli with named duration constants#34373
pelikhan merged 2 commits into
mainfrom
copilot/aw-sg17a2-fix-time-sleep-literals

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 24, 2026

Several production paths in pkg/cli encoded pacing and startup/shutdown waits as inline time.Sleep literals. This change aligns those waits with the existing local named-constant pattern already used elsewhere in the CLI for retry and API cooldown behavior.

  • Replace hardcoded CLI delays with local constants

    • Introduce file-local time.Duration constants for:
      • trial repository initialization delay
      • inter-workflow trigger pacing
      • MCP inspector stdio server startup and cleanup pacing
      • mcp-scripts server readiness polling
      • mcp-scripts shutdown grace period
  • Keep delay policy scoped to each call site

    • Each delay remains defined next to the code it governs rather than being shared across unrelated flows.
    • The two MCP inspector delays are grouped in a single const block to keep related timing policy together.
  • Make intent explicit at the call sites

    • Sleep calls now use named values that describe why the pause exists instead of repeating raw durations.
const betweenWorkflowsDelay = 1 * time.Second

if i < len(workflowNames)-1 {
	time.Sleep(betweenWorkflowsDelay)
}

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix hardcoded time.Sleep literals in production pkg/cli/ Replace magic time.Sleep literals in pkg/cli with named duration constants May 24, 2026
Copilot AI requested a review from gh-aw-bot May 24, 2026 05:41
@pelikhan pelikhan marked this pull request as ready for review May 24, 2026 06:52
Copilot AI review requested due to automatic review settings May 24, 2026 06:52
@pelikhan pelikhan merged commit b2718c2 into main May 24, 2026
@pelikhan pelikhan deleted the copilot/aw-sg17a2-fix-time-sleep-literals branch May 24, 2026 06:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces several inline time.Sleep duration literals in pkg/cli with named time.Duration constants to make the intent of each delay explicit at the call site while keeping the timing policy near the code it governs.

Changes:

  • Introduce named duration constants for repository initialization, workflow trigger pacing, and MCP inspector/server startup/shutdown waits.
  • Replace hardcoded time.Sleep(<literal>) calls with the corresponding named constants across the affected CLI flows.
Show a summary per file
File Description
pkg/cli/trial_repository.go Adds trialRepoInitDelay constant and uses it for post-repo-create initialization wait.
pkg/cli/run_workflow_execution.go Adds betweenWorkflowsDelay constant and uses it to pace sequential workflow triggers.
pkg/cli/mcp_inspect.go Adds mcpScriptsServerShutdownDelay constant and uses it during mcp-scripts shutdown cleanup.
pkg/cli/mcp_inspect_mcp_scripts_server.go Adds a named delay constant for readiness polling sleep while waiting for the server to come up.
pkg/cli/mcp_inspect_inspector.go Adds grouped constants for stdio server startup wait and inter-process cleanup pacing, and uses them in the inspector flow.

Copilot's findings

Tip

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

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

const (
// mcpStdioServerStartupDelay gives stdio MCP servers time to start accepting connections.
mcpStdioServerStartupDelay = 2 * time.Second
// mcpProcessCleanupDelay spaces cleanup signals so each MCP process can exit cleanly.
Comment on lines 21 to 25
mcpScriptsStartPort = 3000
mcpScriptsPortRange = 10
// mcpScriptsServerStartupDelay paces readiness checks while the mcp-scripts server boots.
mcpScriptsServerStartupDelay = 200 * time.Millisecond
)
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.

[sergo] Magic time.Sleep literals in production pkg/cli/ (#aw_sg17a2)

4 participants