Skip to content

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

Description

@github-actions

Summary

Six production code paths in pkg/cli/ call time.Sleep with hardcoded literal durations instead of the project's established named-constant pattern. This breaks the consistency model that R8 (HTTP timeouts → constants) and the existing APICallCooldown / backupCleanupRetryDelay / timestampDifferentiationDelay named constants set up.

This is #aw_sg17a2, identified in Sergo Run 17.

Evidence

Magic-literal sites (prod code)

File:Line Sleep Purpose (from inline comment)
pkg/cli/trial_repository.go:153 2 * time.Second "Give GitHub a moment to fully initialize the repository"
pkg/cli/run_workflow_execution.go:571 1 * time.Second "Add a small delay between workflows to avoid overwhelming GitHub API"
pkg/cli/mcp_inspect_inspector.go:157 2 * time.Second "Give servers a moment to start up"
pkg/cli/mcp_inspect_inspector.go:201 100 * time.Millisecond "Give each process a chance to clean up"
pkg/cli/mcp_inspect_mcp_scripts_server.go:62 200 * time.Millisecond (startup-pace gate)
pkg/cli/mcp_inspect.go:134 500 * time.Millisecond (startup-pace gate)

Comparison: established named-constant pattern (good citizens)

  • pkg/cli/logs_rate_limit.go:87, 97, 110, 121 — uses APICallCooldown (a typed Duration constant). Four sites, one symbol.
  • pkg/cli/update_extension_check.go:386 — uses backupCleanupRetryDelay.
  • pkg/workflow/compiler_skip_write_test.go:58, 126 — uses timestampDifferentiationDelay.
  • pkg/cli/run_workflow_tracking.go:73 — uses a delay variable derived from caller context (also acceptable: not a magic literal).

The pattern is clear and adopted across pkg/cli/: when a sleep duration encodes a policy decision (rate-limit pacing, GitHub propagation lag, IPC startup wait), it lives in a named time.Duration constant near the code that uses it.

Severity

Medium (consistency / maintainability). No correctness bug today. But:

  1. Tuning these durations means hunting through call-site code instead of editing one symbol.
  2. New contributors learn the wrong pattern by copy-paste.
  3. Tests that mock time (or want to run faster) cannot inject a smaller value.
  4. The "why this number" justification lives in adjacent comments, not the symbol name.

Recommendation

Replace each literal with a package-level const (or var if it should be overridable in tests). Suggested names:

// pkg/cli/trial_repository.go
const trialRepoInitDelay = 2 * time.Second

// pkg/cli/run_workflow_execution.go
const betweenWorkflowsDelay = 1 * time.Second

// pkg/cli/mcp_inspect_inspector.go
const (
    mcpStdioServerStartupDelay = 2 * time.Second
    mcpProcessCleanupDelay     = 100 * time.Millisecond
)

// pkg/cli/mcp_inspect_mcp_scripts_server.go
const mcpScriptsServerStartupDelay = 200 * time.Millisecond

// pkg/cli/mcp_inspect.go
const mcpInspectorStartupDelay = 500 * time.Millisecond

Group related constants (the two mcp_inspect_inspector.go sleeps) into a single const ( ... ) block.

Validation

  • Replace 6 literals with named constants in the 5 files listed.
  • Run go vet ./pkg/cli/... and go test ./pkg/cli/....
  • Verify no other call site exists for any of the new constants (i.e. don't accidentally create shared constants for unrelated waits).
  • Optional: add a comment on each constant explaining the empirical basis (e.g. "GitHub propagation lag observed up to ~1.5s in practice; 2s gives a safety margin").

Related

  • R8 #aw_sg8a2 (HTTP timeout literals → constants, FIXED R12, 4 sites) — exact same refactor shape.
  • R8 #aw_sg8a1 (file-mode octal literals, FIXED R9, 57 sites) — same "magic literal → named constant" theme.
  • Established pattern in pkg/constants/constants.go for Default* symbols.

References:

Generated by 🤖 Sergo - Serena Go Expert · ● opu47 18.5M ·

  • expires on May 31, 2026, 5:13 AM UTC

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions