Skip to content

ci(behaviour): raise behaviour job timeout 30→60m - #5522

Closed
ifireball wants to merge 0 commit into
mainfrom
ci/behaviour-timeout-60m-main
Closed

ci(behaviour): raise behaviour job timeout 30→60m#5522
ifireball wants to merge 0 commit into
mainfrom
ci/behaviour-timeout-60m-main

Conversation

@ifireball

Copy link
Copy Markdown
Member

Summary

Test plan

  • Confirm behaviour job on a PR run shows timeout-minutes 60 in the workflow file from main after merge
  • No change to e2e job timeout

Made with Cursor

@ifireball
ifireball requested a review from a team as a code owner July 23, 2026 10:08
@ifireball
ifireball enabled auto-merge July 23, 2026 10:09
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

CI: increase behaviour job/test timeout from 30→60 minutes

⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Increase GitHub Actions behaviour job timeout-minutes from 30 to 60.
• Raise make behaviour-test Go test timeout to match the CI budget.
• Add context comments for longer runtime under lazy create+install suites.
Diagram

graph TD
gha["GitHub Actions"] --> wf[".github/workflows/e2e.yml"] --> job["behaviour job (60m)"] --> runner["ubuntu-24.04 runner"] --> mk["Makefile: behaviour-test"] --> gt["go test -timeout 60m"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Parallelize/split behaviour suites into multiple jobs
  • ➕ Reduces wall-clock time and flakiness due to long single-job runtime
  • ➕ Keeps per-job timeouts lower and failures more isolated
  • ➖ Requires workflow restructuring and potential test isolation work
  • ➖ May increase total CI resource usage and complexity
2. Optimize lazy create+install path (caching, fewer leased repos)
  • ➕ Addresses root cause by reducing suite runtime rather than expanding budget
  • ➕ Improves developer feedback time across all runs
  • ➖ Likely more involved than a timeout change
  • ➖ May require deeper changes across tests and provisioning logic

Recommendation: Proceed with the timeout increase as a pragmatic, low-risk fix to unblock pull_request_target runs that must honor the main-branch workflow configuration. Consider suite parallelization or runtime optimization separately if 60 minutes becomes insufficient or CI feedback time regresses.

Files changed (2) +4 / -2

Other (2) +4 / -2
e2e.ymlRaise behaviour job timeout-minutes to 60 with rationale comment +3/-1

Raise behaviour job timeout-minutes to 60 with rationale comment

• Increases the 'behaviour' job timeout from 30 to 60 minutes. Adds comments explaining the increased runtime due to lazy create+install and serial suite execution.

.github/workflows/e2e.yml

MakefileAlign behaviour-test Go test timeout with CI budget (60m) +1/-1

Align behaviour-test Go test timeout with CI budget (60m)

• Updates the 'behaviour-test' target to run 'go test' with '-timeout 60m' instead of '30m', matching the extended CI job timeout.

Makefile

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:10 AM UTC · Completed 10:20 AM UTC
Commit: beef020 · View workflow run →

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://f077aa86-site.fullsend-ai.workers.dev

Commit: beef0201a29d6552d6449dbfa16a6c039b01877f

@rh-hemartin rh-hemartin left a comment

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.

Stop this madness.

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 54 rules

Grey Divider


Remediation recommended

1. No timeout headroom 🐞 Bug ☼ Reliability
Description
The behaviour job sets timeout-minutes: 60 while it runs make behaviour-test whose go test
also uses -timeout 60m, so any time spent in checkout/auth/setup reduces the effective test
runtime below 60 minutes. When runs approach the limit, GitHub Actions can terminate the job first,
yielding a generic cancellation instead of a Go test timeout with actionable diagnostics.
Code

.github/workflows/e2e.yml[R192-194]

+    # Lazy create+install (#5439) pays per leased test-repo-NN; serial suites
+    # need more than the old shared-test-repo 30m budget.
+    timeout-minutes: 60
Relevance

⭐⭐ Medium

Timeout headroom seems reasonable, but similar “add buffer between tool/ctx timeouts” was previously
rejected.

PR-#761
PR-#2106

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow sets a 60-minute cap for the entire behaviour job and then runs `make
behaviour-test. The Makefile target runs go test ... -timeout 60m`, so there is no buffer for
non-test steps, making it possible for the job to be killed by the runner timeout before Go can emit
its own timeout error.

.github/workflows/e2e.yml[185-195]
.github/workflows/e2e.yml[257-260]
Makefile[178-183]

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 workflow job timeout and the Go test timeout are set to the same duration (60m). Because the job timeout includes all setup steps, the runner can cancel the job before `go test` reaches its own timeout, producing less-informative failures.

## Issue Context
The behaviour job runs `make behaviour-test`, and that Makefile target sets `go test -timeout 60m`.

## Fix Focus Areas
- Increase the job-level timeout above the test timeout (e.g., job 70m, test 60m), **or** reduce the Go test timeout below the job timeout (e.g., test 55m, job 60m).
- Keep the two timeouts intentionally staggered to preserve clear timeout diagnostics.

### References
- .github/workflows/e2e.yml[185-195]
- .github/workflows/e2e.yml[257-260]
- Makefile[178-183]

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


Grey Divider

Qodo Logo

Comment thread .github/workflows/e2e.yml Outdated
Comment on lines +192 to +194
# Lazy create+install (#5439) pays per leased test-repo-NN; serial suites
# need more than the old shared-test-repo 30m budget.
timeout-minutes: 60

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. No timeout headroom 🐞 Bug ☼ Reliability

The behaviour job sets timeout-minutes: 60 while it runs make behaviour-test whose go test
also uses -timeout 60m, so any time spent in checkout/auth/setup reduces the effective test
runtime below 60 minutes. When runs approach the limit, GitHub Actions can terminate the job first,
yielding a generic cancellation instead of a Go test timeout with actionable diagnostics.
Agent Prompt
## Issue description
The workflow job timeout and the Go test timeout are set to the same duration (60m). Because the job timeout includes all setup steps, the runner can cancel the job before `go test` reaches its own timeout, producing less-informative failures.

## Issue Context
The behaviour job runs `make behaviour-test`, and that Makefile target sets `go test -timeout 60m`.

## Fix Focus Areas
- Increase the job-level timeout above the test timeout (e.g., job 70m, test 60m), **or** reduce the Go test timeout below the job timeout (e.g., test 55m, job 60m).
- Keep the two timeouts intentionally staggered to preserve clear timeout diagnostics.

### References
- .github/workflows/e2e.yml[185-195]
- .github/workflows/e2e.yml[257-260]
- Makefile[178-183]

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

@codecov

codecov Bot commented Jul 23, 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 PR modifies a file under .github/, which is a protected path requiring human approval. The change is well-justified by linked issue feat(behaviour): lazy create+install on Given the enrolled test repository #5439 (lazy create+install requires per-leased-repo setup that needs more than the old 30m budget), but human review is always required for governance and infrastructure file changes, regardless of context.

Low

  • [edge-case] .github/workflows/e2e.yml — The workflow timeout-minutes (60) exactly equals the go test -timeout (60m) in the Makefile behaviour-test target. If the Go test timeout fires near the 60m mark, process cleanup may push the job past the GitHub Actions timeout, causing a hard kill mid-cleanup. This is a pre-existing pattern (the e2e job already uses identical timeout-minutes: 30 with -timeout 30m), so this PR simply maintains the existing convention. Consider adding a small buffer (e.g., workflow 65m / test 60m) as a repo-wide follow-up.

Labels: PR modifies CI workflow (.github/workflows/e2e.yml) and build config (Makefile) for the behaviour test job

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/ci CI pipelines and checks component/e2e End-to-end tests labels Jul 23, 2026
@ifireball ifireball closed this Jul 23, 2026
auto-merge was automatically disabled July 23, 2026 10:21

Pull request was closed

@ifireball
ifireball force-pushed the ci/behaviour-timeout-60m-main branch from beef020 to 4ef60e7 Compare July 23, 2026 10:21
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:23 AM UTC · Completed 10:27 AM UTC
Commit: 4ef60e7 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review skipped — this PR is already closed.

The /fs-review command only reviews open pull requests.

Posted by fullsend pre-review check

@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 10:24 AM UTC · Completed 10:36 AM UTC
Commit: 4ef60e7 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review skipped — this PR is already closed.

The /fs-review command only reviews open pull requests.

Posted by fullsend post-review check

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5522ci(behaviour): raise behaviour job timeout 30→60m

Timeline

PR fullsend-ai/fullsend#5522 was a human-authored 4-line CI config change by @ifireball to raise the behaviour test job timeout from 30 to 60 minutes (.github/workflows/e2e.yml and Makefile). This was the 4th PR in a series attempting to increase this timeout (#5432, #5483, #5519, #5522).

Time (UTC) Event
10:08:34 PR opened, auto-merge enabled
10:08:52 Review agent run 29998185575 started
10:10:45 Human (@rh-hemartin) APPROVED: "Stop this madness."
10:11:25 Qodo bot posted: timeout headroom bug (job timeout = test timeout)
10:19:57 Review agent posted: 1 MEDIUM (protected-path), 1 LOW (timeout headroom)
10:20:02 Review agent applied requires-manual-review, component/ci, component/e2e
10:21:38 Author closed PR without merging

The review agent took ~11 minutes and cost $1.75 (28 turns, claude-opus-4-6) for a change it correctly triaged as "trivial/mechanical." It dispatched 3 sub-agents (correctness, style-conventions, intent-coherence) plus a challenger. The human approved in 2 minutes. The author closed the PR, keeping the open #5483 (45m version) instead.

What worked well

  • The review agent correctly identified the timeout headroom issue (workflow timeout = Go test timeout, no buffer for setup/cleanup) and appropriately rated it LOW, noting it's a pre-existing pattern.
  • The agent correctly flagged the protected path and chose comment action (non-blocking), not request-changes.
  • The post-close guard worked: a second review triggered by branch force-push was correctly skipped ("Review skipped — this PR is already closed").
  • The agent's technical analysis was more thorough than the human review, which approved without flagging the timeout headroom issue.

Evidence for existing issues

Autonomy note

The review agent's technical analysis exceeded the human's on this PR (caught the timeout headroom issue the human missed). However, the primary value of the review on this CI change was the governance gate (protected-path), which is a policy decision requiring human judgment by design. For CI config changes, the review agent is adding real technical value but the governance requirement correctly keeps the human in the loop.

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