fix: disable global timeout for --server mode

[simonbaird]

Summary

• The default 5-minute global timeout was silently killing the persistent HTTP server started by ec validate input --server
• The timeout is now automatically disabled when --server is detected, so the server runs indefinitely until interrupted
• Added a unit test verifying the behavior

Ref: EC-2029

Test plan

• Existing unit tests pass (go test -tags unit ./cmd/root/... ./cmd/validate/...)
• New TestGlobalTimeoutDisabledInServerMode test verifies globalTimeout is set to 0 when --server flag is present
• Manual: run ec validate input --server --policy <policy> and confirm it stays alive past 5 minutes

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: b7ef1a19-e173-4617-9483-b881d4751bfe

📥 Commits

Reviewing files that changed from the base of the PR and between 94c6b77 and e801b01.

📒 Files selected for processing (2)

• cmd/root/root_cmd.go
• cmd/root/root_cmd_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• cmd/root/root_cmd.go

---

📝 Walkthrough

Walkthrough

Server mode now computes an effective timeout of zero for validate input --server, causing root command setup to skip context.WithTimeout. A unit test verifies that the configured global timeout remains unchanged and the command context has no deadline.

Changes

Server-mode timeout handling

Layer / File(s)	Summary
Effective timeout and validation cmd/root/root_cmd.go, cmd/root/root_cmd_test.go	effectiveTimeout disables timeout application for validate input --server; PersistentPreRun uses the computed value, and a Cobra-based unit test verifies the unchanged global timeout and absence of a context deadline.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description has the needed context, but it does not follow the required What/Why/Tickets template headings.	Rewrite the PR description using the template sections What, Why, and Tickets, and keep the test plan as supporting detail.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: disabling the global timeout in --server mode.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:55 PM UTC · Completed 4:10 PM UTC
Commit: 87c4a29 · View workflow run →

[qodo-for-conforma]

PR Summary by Qodo

Disable global timeout for persistent server mode

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Disable the global execution timeout when the active command enables --server.
• Keep validation HTTP servers alive until explicitly interrupted.
• Add unit coverage for server-mode timeout behavior.

Diagram

graph TD
  A["Server Invocation"] --> B["Root Pre-Run"] --> C{"Server Enabled?"}
  C -->|Yes| D["Clear Timeout"] --> E["No-Deadline Context"] --> F["HTTP Server"]
  C -->|No| G["Timed Context"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Derive an effective timeout locally

• ➕ Avoids mutating package-global timeout state
• ➕ Keeps parsed flag values intact for diagnostics and repeated command execution
• ➕ Separates configured timeout from execution policy
• ➖ Requires a small refactor to use a local timeout variable
• ➖ Existing tests that inspect globalTimeout would need adjustment

Recommendation: The server-flag detection is appropriately placed after Cobra parses the active command's flags. However, deriving a local effectiveTimeout and setting it to zero for server mode would be safer than permanently mutating globalTimeout, especially when commands are reused in tests or embedded callers.

Files changed (2) +25 / -0

Bug fix (1) +4 / -0

root_cmd.goBypass the global deadline for server commands +4/-0

Bypass the global deadline for server commands

• Detects an enabled '--server' flag during root pre-run and clears the global timeout before context creation. Persistent server commands therefore run without the default five-minute deadline.

cmd/root/root_cmd.go

Tests (1) +21 / -0

root_cmd_test.goVerify server mode disables the global timeout +21/-0

Verify server mode disables the global timeout

• Adds a Cobra subcommand with a server flag and executes it in server mode. The test confirms successful execution and verifies that the global timeout becomes zero.

cmd/root/root_cmd_test.go

[qodo-for-conforma]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 27 rules

1. Server mode leaks timeout state ✓ Resolved 🐞 Bug ☼ Reliability

Description

Server mode permanently assigns zero to the package-level globalTimeout, so a later NewRootCmd
in the same process registers --timeout with a zero default and one-shot commands lose their
five-minute timeout. This can leave later in-process operations running indefinitely instead of
timing out.

Code

cmd/root/root_cmd.go[121]

+				globalTimeout = 0

Relevance

⭐⭐⭐ High

Deterministic cross-execution state leak; team accepts server-mode shared-state reliability fixes.

PR-#3386
PR-#2557

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

globalTimeout is package-level state initialized to five minutes, while every new root command
binds its persistent --timeout flag to that same variable using its current value as the default.
The repository also exposes an in-process execution path that constructs a fresh root command for
every call, so executing server mode first can cause later one-shot executions to inherit a zero
timeout.

cmd/root/root_cmd.go[40-45]
cmd/root/root_cmd.go[202-216]
benchmark/internal/suite/suite.go[26-31]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Server mode mutates the package-level `globalTimeout` to zero. The mutation persists after execution and changes the default timeout of subsequently constructed root commands in the same process.

## Issue Context
Keep the configured/default timeout unchanged and derive an execution-local timeout value. Set only that local value to zero when the active command is in server mode, then use it when creating the context.

## Fix Focus Areas
- cmd/root/root_cmd.go[117-127]
- cmd/root/root_cmd_test.go[83-101]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. root_cmd_test.go lacks unit tag ✓ Resolved 📘 Rule violation ▣ Testability

Description

The newly added TestGlobalTimeoutDisabledInServerMode is a unit test, but its Go test file has no
//go:build unit constraint. Consequently, the test is not properly gated as required for unit
tests.

Code

cmd/root/root_cmd_test.go[83]

+func TestGlobalTimeoutDisabledInServerMode(t *testing.T) {

Relevance

⭐⭐⭐ High

Explicit active compliance rule makes adding the unit build tag a straightforward fix.

PR-#2557
PR-#2660

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 110 requires every new or modified Go unit test file to declare //go:build unit.
The file begins with its license header followed directly by package root at line 17, while the PR
adds a unit test at lines 83-101.

Rule 110: Unit tests must use the unit build tag
cmd/root/root_cmd_test.go[1-17]
cmd/root/root_cmd_test.go[83-101]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The modified Go unit test file lacks the required `//go:build unit` constraint.

## Issue Context
`TestGlobalTimeoutDisabledInServerMode` is an example-based unit test and should only be selected when the `unit` build tag is enabled.

## Fix Focus Areas
- cmd/root/root_cmd_test.go[1-17]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/root/root_cmd.go`:
- Around line 119-122: The server-mode override in cmd/root/root_cmd.go should
avoid permanently mutating the package-level globalTimeout by using an
invocation-local effective timeout or restoring the prior value after execution.
In cmd/root/root_cmd_test.go, capture globalTimeout before the test and register
t.Cleanup to restore it afterward; apply the change at both listed sites.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 9334cf2d-b8de-4017-ab88-f26f762e7053

📥 Commits

Reviewing files that changed from the base of the PR and between 30894bf and 94c6b77.

📒 Files selected for processing (2)

• cmd/root/root_cmd.go
• cmd/root/root_cmd_test.go

[fullsend-ai-review]

Review

Findings

Low

• [edge-case] cmd/root/root_cmd_test.go — No direct unit test for effectiveTimeout when validate input is invoked without --server. The existing TestGlobalTimeout tests use NewRootCmd() without a validate/input subtree, so they never enter the cmd.Name()=="input" branch. A regression that drops the server-flag check could silently disable timeouts for all validate input invocations.
  Remediation: Add a test that invokes validate input without --server and asserts the context carries a deadline.

• [format-verb-consistency] cmd/root/root_cmd.go:141 — The two adjacent debug log lines use different format verbs for the same Duration type: %s (line 139) vs %d (line 141, prints raw nanoseconds). The time.Duration() cast on line 139 is also a no-op since timeout is already time.Duration.
  Remediation: Use %s in both branches and remove the redundant cast.

• [architectural-coupling] cmd/root/root_cmd.go:72 — effectiveTimeout hard-codes knowledge of the validate input --server leaf subcommand by matching command names and flag values. The author's comment acknowledges the coupling and the cobra lifecycle constraint. A generic opt-out mechanism (e.g., cobra annotations) would scale better if future long-running subcommands are added.
  Remediation: Consider a cobra annotation-based approach so any subcommand can opt out of the global timeout without modifying root_cmd.go.

• [missing-doc] cmd/validate/input.go:370 — The --server flag description does not mention that the global --timeout is automatically disabled in server mode. Since .adoc docs are auto-generated from Cobra flag descriptions, this omission propagates to the reference documentation.
  Remediation: Append something like "The global --timeout is automatically disabled in this mode" to the --server flag usage string.

• [missing-doc] cmd/validate/input.go:92 — The Synopsis's "Server limits" line mentions the per-request evaluation timeout but does not clarify that the global --timeout is separately disabled in server mode.
  Remediation: Add a sentence clarifying that the global --timeout is ignored in server mode.

• [missing-doc] cmd/root/root_cmd.go:228 — The --timeout flag usage string ("max overall execution duration") does not note the server-mode exception.
  Remediation: Optionally add a note like "disabled automatically in server mode."

Previous run

Review

Findings

Medium

• [scope-coherence] cmd/root/root_cmd.go:119 — The root command's PersistentPreRun dynamically looks up a --server flag via cmd.Flags().Lookup("server") that is defined on a leaf subcommand (validate input). This inverts the dependency direction: the root command now has implicit knowledge of a subcommand-specific flag. Any future command that defines a --server flag will also silently have its global timeout disabled.
  Remediation: Consider moving the timeout-override logic into the validate input subcommand's PreRunE (which already exists), or have the subcommand explicitly set --timeout=0. This keeps the root command decoupled from leaf-command flags.

Low

• [error-handling-idiom] cmd/root/root_cmd.go:119 — The flag value check uses serverFlag.Value.String() == "true". While functionally correct (pflag normalizes bool values to "true"/"false"), the more idiomatic cobra pattern would combine serverFlag.Changed with a value check. Note: using serverFlag.Changed alone would be incorrect — it would disable the timeout even for --server=false. See also: [scope-coherence] finding at this location.
• [test-inadequate] cmd/root/root_cmd_test.go:82 — TestGlobalTimeoutDisabledInServerMode mutates the package-level globalTimeout to 0 but does not restore it after the test completes. Future tests added after this one that don't explicitly reset globalTimeout would see 0 instead of the default 5 minutes.
  Remediation: Add t.Cleanup(func() { globalTimeout = 5 * time.Minute }) at the top of the test.
• [test-conventions] cmd/root/root_cmd_test.go:82 — The test constructs a synthetic cobra.Command with its own --server flag rather than exercising the real validate input subcommand. The test would pass even if the actual flag name were changed in cmd/validate/input.go. See also: [test-inadequate] finding at this location.
• [naming-convention] cmd/root/root_cmd.go:120 — The new debug log uses log.Debug while the surrounding timeout-related messages use log.Debugf. Minor inconsistency in logging style within the same block.
• [undocumented-behavioral-change] docs/modules/ROOT/pages/ec_validate_input.adoc:98 — The --server flag description does not mention that it now disables the global --timeout. Users combining --timeout with --server will see no effect from the timeout flag without explanation. Since docs are auto-generated from cobra flag descriptions, update the --server flag usage text in cmd/validate/input.go.

---

Labels: PR fixes a bug in CLI timeout behavior for server mode

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
acceptance	54.35% <100.00%> (+0.02%)	⬆️
generative	16.39% <0.00%> (-0.40%)	⬇️
integration	27.55% <0.00%> (-0.40%)	⬇️
unit	71.94% <72.72%> (+0.17%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cmd/root/root_cmd.go	74.56% <100.00%> (+2.59%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[simonbaird]

Not sure why the build task failed, but perhaps it was spurious.

[fullsend-ai-review]

🤖 Finished Review · ❌ Failure · Started 4:47 PM UTC · Completed 5:02 PM UTC
Commit: 87c4a29 · View workflow run →

[simonbaird]

Fullsend failed, but the other bots are happy. Ready for (human) review.