Skip to content

fix(workflow:watch): increase poll interval to 10s and add 30min default timeout#2809

Open
JGrubb wants to merge 1 commit into4.xfrom
fix/workflow-watch-poll-interval-and-timeout
Open

fix(workflow:watch): increase poll interval to 10s and add 30min default timeout#2809
JGrubb wants to merge 1 commit into4.xfrom
fix/workflow-watch-poll-interval-and-timeout

Conversation

@JGrubb
Copy link
Copy Markdown

@JGrubb JGrubb commented Mar 31, 2026

Summary

terminus workflow:watch previously polled at a fixed 5-second interval with no default timeout, which could result in extremely high API call volumes when left running unattended in CI/CD pipelines.

Changes

Two-phase polling

Replaces the fixed 5s interval with a simple two-phase schedule:

  • Warmup (first 30 polls, ~5 minutes): 10s — responsive for fast workflows
  • Steady-state (after warmup): 30s — backs off for long-running operations

--timeout replaces --checks

The new --timeout option accepts a duration in minutes (default: 15). The command exits with a warning when the timeout is reached. Pass --timeout=0 for unlimited (previous default behavior).

[warning] Workflow watch timed out after 15 minutes. Use --timeout=0 for no limit.

Unit tests

Added tests/Unit/Commands/Workflow/WatchCommandTest.php covering:

  • Fast interval during warmup, slow interval after
  • Timeout fires and logs the correct message
  • --timeout=0 never triggers the timeout guard

@JGrubb JGrubb requested a review from a team as a code owner March 31, 2026 14:28
@JGrubb JGrubb force-pushed the fix/workflow-watch-poll-interval-and-timeout branch 5 times, most recently from 50e9cbe to cc3d691 Compare March 31, 2026 15:43
@JGrubb
Copy link
Copy Markdown
Author

JGrubb commented Mar 31, 2026

The failing checks (Functional testing matrix) are pre-existing failures on 4.x unrelated to this PR — specifically SiteTeamCommandsTest hitting a live API error. The two most recent merges to 4.x have the same failure.

@JGrubb JGrubb force-pushed the fix/workflow-watch-poll-interval-and-timeout branch from cc3d691 to fe7ceac Compare March 31, 2026 16:54
- Replace fixed 5s interval with two-phase polling: 10s for first 30 polls
  (~5 min warmup), then 30s steady-state
- Replace --checks with --timeout (minutes); default 15 min, 0 = unlimited
- Emit a warning on timeout explaining how to override
- Extract sleep() to a protected method for testability
- Add unit tests covering interval switching, timeout, and unlimited mode

Prevents runaway API call volumes when workflow:watch is left running
unattended in CI/CD pipelines with no timeout.
@JGrubb JGrubb force-pushed the fix/workflow-watch-poll-interval-and-timeout branch from fe7ceac to 47c7503 Compare March 31, 2026 16:55
@kaylimepy
Copy link
Copy Markdown
Contributor

I noticed --checks got replaced with --timeout, which makes way more sense from a UX perspective!

I'm wondering if we should consider this a breaking change though? I'm not sure how widely --checks is used, but if maybe customer have it in CI scripts, those would break on upgrade?

A few options we could consider:

  1. keep it as is (breaking change, document it clearly)
  2. keep --checks as a deprecated alias for one release with a warning
  3. something else?

*
* @command workflow:watch
*
* @option integer $checks Times to query
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't remove the "checks" option, as that would be a breaking change.

Copy link
Copy Markdown
Contributor

@namespacebrian namespacebrian Apr 21, 2026

Choose a reason for hiding this comment

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

I agree with the change in principle. This is actually a good example of something that would go on a 5.x branch.

@pwtyler pwtyler self-requested a review April 21, 2026 17:26
Copy link
Copy Markdown
Member

@greg-1-anderson greg-1-anderson left a comment

Choose a reason for hiding this comment

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

We need to ensure that existing CI scripts that call Terminus with existing options e.g. --checks should continue to work, even if the removed option is ignored. Maybe convert "checks" into an roughly equivalent timeout period?

@namespacebrian
Copy link
Copy Markdown
Contributor

namespacebrian commented Apr 21, 2026

Agree, @JGrubb if --timeout is passed it should work as it does here. --checks should not be removed, instead, if passed it should be converted to an equivalent timeout value, and that value used. If both options are passed --checks is silently ignored.

We could also flag --checks as deprecated in terminus's help, right? To declare that it would be removed in 5.x

@namespacebrian
Copy link
Copy Markdown
Contributor

Whether it's part of this PR or another one, the tests will need to work again for terminus to release again. Working backwards from there, there's little reason to merge a PR while the tests are failing if you can't release the changes anyway until the tests are working.

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.

5 participants