Skip to content

ci(e2e): raise behaviour job timeout from 30m to 45m - #5483

Closed
ifireball wants to merge 1 commit into
fullsend-ai:mainfrom
ifireball:ci/raise-behaviour-timeout-45m
Closed

ci(e2e): raise behaviour job timeout from 30m to 45m#5483
ifireball wants to merge 1 commit into
fullsend-ai:mainfrom
ifireball:ci/raise-behaviour-timeout-45m

Conversation

@ifireball

@ifireball ifireball commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

  • Raise the behaviour job timeout-minutes from 30 → 45 in .github/workflows/e2e.yml
  • Raise make behaviour-test go test -timeout from 30m → 45m in Makefile
  • Leaves the e2e job timeout at 30m unchanged

Why

The behaviour suite already takes ~29 minutes on a good day on main, leaving ~1 minute of margin. Pool latency and non-fast-forward commit retries push runs past 30m with no assertion failures — GitHub Actions cancels the job mid-scenario (The operation was canceled.).

Evidence

This blocks #5428: its config encapsulation changes correctly trigger behaviour, but pull_request_target loads the workflow from main, so a timeout bump on that PR branch cannot take effect until this lands.

Test plan

The suite already takes ~29m on a good day, leaving almost no margin
against the 30m job/go-test timeouts. Pool latency and non-fast-forward
retries push runs past 30m with no assertion failures.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

CI(e2e): increase behaviour suite timeout to 45 minutes

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Increase GitHub Actions behaviour job timeout from 30m to 45m to prevent mid-run cancellations.
• Align local/CI make behaviour-test Go test timeout to 45m.
• Leave the e2e job and make e2e-test timeout unchanged at 30m.
Diagram

graph TD
  A["GitHub Actions"] --> B["e2e.yml workflow"] --> C["behaviour job (45m)"] --> D["make behaviour-test"] --> E["go test -timeout 45m"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Split/parallelize behaviour suite
  • ➕ Reduces wall-clock time instead of extending it
  • ➕ Improves feedback cycle for PRs
  • ➕ Less risk of hitting future timeouts as suite grows
  • ➖ More invasive changes (test grouping, reporting, potential flake surface)
  • ➖ Higher maintenance complexity in CI orchestration
2. Optimize or prune slow scenarios
  • ➕ Keeps timeouts tight while improving runtime
  • ➕ Helps long-term CI cost and stability
  • ➖ Requires profiling and ongoing performance work
  • ➖ Doesn’t immediately unblock current timeout-driven cancellations

Recommendation: Proceed with the current timeout bump: it’s the smallest change that directly addresses non-failing CI cancellations and keeps CI/go test timeouts consistent. The alternatives are worthwhile longer-term, but are significantly more invasive than needed to unblock runs now.

Files changed (2) +2 / -2

Other (2) +2 / -2
e2e.ymlIncrease behaviour job timeout to 45 minutes +1/-1

Increase behaviour job timeout to 45 minutes

• Updates the 'behaviour' job 'timeout-minutes' from 30 to 45 to avoid GitHub Actions canceling long-running behaviour runs mid-scenario.

.github/workflows/e2e.yml

MakefileAlign behaviour-test Go timeout to 45 minutes +1/-1

Align behaviour-test Go timeout to 45 minutes

• Updates the 'behaviour-test' target to run 'go test' with '-timeout 45m', matching the CI job timeout increase.

Makefile

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:22 PM UTC · Completed 10:31 PM UTC
Commit: 78eb2e5 · View workflow run →

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://080607b0-site.fullsend-ai.workers.dev

Commit: 78eb2e5806932ba0165f16b2b95432d698b6d41b

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] .github/workflows/e2e.yml — This file is under the .github/ protected path. Human approval is always required for changes to governance and infrastructure files, regardless of the change's nature. The PR provides clear rationale (behaviour suite timing out at ~29min with a 30m limit, blocking PR refactor(#5080): remove DirConfig, unexport raw config structs #5428) and the change is limited to a single timeout value.

Low

  • [scope-authorization-missing] No formal issue linked authorizing this timeout increase. PR body references blocking PR refactor(#5080): remove DirConfig, unexport raw config structs #5428 as motivation but does not link to a dedicated issue documenting the timeout exhaustion pattern.

  • [intent-justification-weak] PR body claims the behaviour suite takes ~29min with ~1min margin but provides no supporting evidence (run links, timing data). Including a link to a recent CI run showing the ~29min runtime or a timeout failure would strengthen the justification.


Labels: PR modifies e2e workflow and Makefile test targets

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

Comment thread .github/workflows/e2e.yml
(github.event_name != 'pull_request_target' || needs.gate.outputs.authorized == 'true')
runs-on: ubuntu-24.04
timeout-minutes: 30
timeout-minutes: 45

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] protected-path

This file is under the .github/ protected path. Human approval is always required for changes to governance and infrastructure files, regardless of the change's nature. The PR provides clear rationale (behaviour suite timing out at ~29min with a 30m limit, blocking PR #5428) and the change is limited to a single timeout value.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/e2e End-to-end tests component/ci CI pipelines and checks labels Jul 22, 2026
@ifireball ifireball self-assigned this Jul 23, 2026
@ifireball

Copy link
Copy Markdown
Member Author

already done elsewhere

@ifireball ifireball closed this Jul 23, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 1:05 PM UTC · Completed 1:17 PM UTC
Commit: 78eb2e5 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5483 — ci(e2e): raise behaviour job timeout from 30m to 45m

A 2-line config change (timeout 30m → 45m in .github/workflows/e2e.yml and Makefile) authored by ifireball. The review agent ran once (run 29962540964), dispatched 5 sub-agents (correctness, security, style-conventions, intent-coherence, challenger), and produced 3 findings. The PR was approved by a human reviewer and then closed without merging ("already done elsewhere").

Review quality

  • Medium — protected-path: Valid. .github/ changes correctly require human approval.
  • Low — scope-authorization-missing: Debatable. The PR references refactor(#5080): remove DirConfig, unexport raw config structs #5428 as the blocking motivation, which provides authorization context. This is covered by agents#150 (suppress compliance findings on trivial config-only PRs from repo members).
  • Low — intent-justification-weak: False positive. The review agent stated the PR body "provides no supporting evidence (run links, timing data)" when the PR body contains an explicit "Evidence" section with two CI run links and specific timing data (1731s / ~28.9m). Root cause: the orchestrator compressed the PR body into a single-paragraph summary before passing it to the intent-coherence sub-agent, stripping the structured Evidence section, hyperlinks, and test plan.

Existing issue evidence

  • agents#301 (trivial change fast path): This retro provides additional evidence. The review agent dispatched 4 sub-agents + challenger for a 2-line value-only config change, costing $2.02 and ~10 minutes. Only the deterministic protected-path check was actionable.
  • agents#150 (suppress compliance findings on trivial config PRs from members): The scope-authorization-missing finding here is the same class of low-value compliance finding on a trivial config change from a repo member.

What went well

The protected-path detection worked correctly, the human approved promptly, and the challenger sub-agent did improve the initial findings (removing speculative claims about "masking underlying performance issues" and noting the "50% increase" framing was misleading). The review agent's overall verdict (comment-only, not request-changes) was appropriate for the severity of findings.

Proposals filed

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

Labels

component/ci CI pipelines and checks component/e2e End-to-end tests requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants