Skip to content

docs(agents): forbid unsolicited e2e suite timeout bumps - #5527

Merged
rh-hemartin merged 1 commit into
fullsend-ai:mainfrom
ifireball:docs/e2e-suite-timeout-agent-rule
Jul 24, 2026
Merged

docs(agents): forbid unsolicited e2e suite timeout bumps#5527
rh-hemartin merged 1 commit into
fullsend-ai:mainfrom
ifireball:docs/e2e-suite-timeout-agent-rule

Conversation

@ifireball

Copy link
Copy Markdown
Member

Stop agents from raising e2e/behaviour suite ceilings without explicit human authorization; flag unauthorized bumps on review.

Summary

Related Issue

Changes

Testing

  • make lint passes (stage changes first, then run)
  • Tests added/updated for new or modified logic

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • Commits are signed off (DCO) — human and human-directed agent sessions only
  • I wrote this contribution myself and can explain all changes in it

@ifireball
ifireball requested a review from a team as a code owner July 23, 2026 12:34
@ifireball ifireball self-assigned this Jul 23, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Docs: prohibit unauthorized e2e/behaviour suite-timeout increases

📝 Documentation 🕐 Less than 10 minutes

Grey Divider

AI Description

• Document policy forbidding agent-driven e2e/behaviour suite-timeout ceiling increases
• Clarify suite ceilings vs scenario-level wait windows and expected timeout remediation
• Require reviewers to flag unauthorized suite-timeout bumps as important-severity findings
Diagram

graph TD
  A["Agents / reviewers"] --> B["AGENTS.md"] --> C["Go contributing guide"] --> D["Suite-timeout policy"]
  D --> E[".github/workflows/e2e.yml"]
  D --> F["Make targets"]
  D --> G["E2E_LOCK_TIMEOUT"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Automated guardrail (CI check) for suite-timeout bumps
  • ➕ Prevents unauthorized timeout ceiling increases from landing accidentally
  • ➕ Reduces reviewer load and subjective interpretation
  • ➕ Can enforce “requires human authorization” via labels/PR body keywords
  • ➖ Adds maintenance overhead and potential false positives
  • ➖ Needs careful definition of what counts as a suite ceiling change
2. PR template / CODEOWNERS enforcement for timeout-related files
  • ➕ Lightweight process control without custom CI logic
  • ➕ Ensures the right reviewers see timeout changes
  • ➖ Still relies on humans to catch and enforce policy
  • ➖ May increase review friction for legitimate small edits

Recommendation: The documentation-first approach in this PR is a good baseline because it clarifies policy, scope (suite ceilings vs scenario waits), and review expectations. Consider a follow-up to add an automated/ownership-based guardrail for changes to .github/workflows/e2e.yml, Make targets, and any E2E_LOCK_TIMEOUT definition to ensure the policy is consistently enforced.

Files changed (2) +5 / -1

Documentation (2) +5 / -1
AGENTS.mdPoint Go contributors to suite-timeout policy +1/-1

Point Go contributors to suite-timeout policy

• Extends the Go contribution guide entry to explicitly include suite-timeout policy coverage. This makes the new rule discoverable from the agent guidance index.

AGENTS.md

go-code.mdAdd explicit policy on e2e/behaviour suite-timeout ceilings +4/-0

Add explicit policy on e2e/behaviour suite-timeout ceilings

• Documents a strict rule: do not increase e2e/behaviour suite ceilings without explicit human authorization, and clarifies what counts as a suite ceiling versus scenario-level waits. Adds review guidance to flag unauthorized suite-timeout increases as important-severity findings.

docs/contributing/go-code.md

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:36 PM UTC · Completed 12:51 PM UTC
Commit: 2744349 · View workflow run →

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Site preview

Preview: https://9caf63b7-site.fullsend-ai.workers.dev

Commit: 6b4fcd43fc4c5854f353eac282e1b80423f550a4

@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. Authorization rule inconsistent 🐞 Bug ⚙ Maintainability
Description
The new suite-timeout policy text allows authorization via an issue/PR comment but later restricts
the exception to only a linked issue or PR description, creating an ambiguous rule. This ambiguity
can lead to inconsistent enforcement of the “unauthorized bump” policy during reviews.
Code

docs/contributing/go-code.md[R36-38]

+**Do not** increase e2e or behaviour **suite** timeouts without **explicit human authorization** in the current session (or an issue/PR comment that clearly authorizes that bump). Suite ceilings are job `timeout-minutes` in `.github/workflows/e2e.yml` for the `e2e` / `behaviour` jobs, the matching `go test -timeout` values in the `e2e-test` / `behaviour-test` Make targets, and the default `E2E_LOCK_TIMEOUT`. Scenario-level wait or assertion windows (for example dispatch detection) are out of scope when an issue asks for them — those are not suite ceilings. On timeout failures, diagnose and fix the root cause (slow scenarios, unnecessary waits, lock contention); do not open a PR whose primary change is raising the suite timeout.
+
+**When reviewing PRs:** Flag unauthorized suite-timeout increases as an **important-severity** finding (policy violation). Explicit human authorization in the linked issue or PR description is the only exception.
Relevance

⭐⭐⭐ High

Team often accepts fixes that resolve internal documentation inconsistencies/ambiguity, especially
around policy/authorization wording.

PR-#5454
PR-#5457

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added policy text contains two conflicting statements about what constitutes valid human
authorization for increasing suite timeouts: one allows issue/PR comments, while another restricts
the exception to only a linked issue or PR description.

docs/contributing/go-code.md[36-38]

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 suite-timeout policy defines two different sources of acceptable “explicit human authorization” (issue/PR comment vs. linked issue or PR description). This creates an ambiguous rule for reviewers/agents.

## Issue Context
The first paragraph allows authorization via an issue/PR comment, while the review guidance later says the only exception is authorization in the linked issue or PR description. These are not equivalent.

## Fix Focus Areas
- docs/contributing/go-code.md[36-38]

## Suggested fix
Update the review guidance sentence to match the earlier definition (or update the earlier definition to match the later restriction), so there is exactly one clearly stated set of acceptable authorization sources (e.g., “linked issue/PR description or an explicit comment on the issue/PR”).

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


Grey Divider

Qodo Logo

Comment thread docs/contributing/go-code.md
@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

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [protected-path] AGENTS.md — This PR modifies AGENTS.md, a protected governance file, without a linked issue authorizing the change. The modification adds ", and suite-timeout policy" to the Go Code entry in the topic-specific guidance table. Human approval is required for all protected-path changes regardless of change size.
    Remediation: Link a tracking issue that authorizes the AGENTS.md modification, or obtain explicit human approval on this PR.
Previous run

Review

Findings

High

  • [protected-path] AGENTS.md — This PR modifies AGENTS.md, a protected governance file, without a linked issue authorizing the change. The modification adds ", and suite-timeout policy" to the Go Code entry in the topic-specific guidance table. Human approval is required for all protected-path changes regardless of change size.
    Remediation: Link a tracking issue that authorizes the AGENTS.md modification, or obtain explicit human approval on this PR.

Labels: PR modifies contributing documentation and adds e2e suite-timeout policy

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added component/docs User-facing documentation component/e2e End-to-end tests labels Jul 23, 2026
@ifireball
ifireball force-pushed the docs/e2e-suite-timeout-agent-rule branch from 2744349 to afd8db6 Compare July 23, 2026 19:47
Stop agents from raising e2e/behaviour suite ceilings without explicit
human authorization; flag unauthorized bumps on review.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ifireball
ifireball force-pushed the docs/e2e-suite-timeout-agent-rule branch from afd8db6 to 6b4fcd4 Compare July 23, 2026 19:47
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:49 PM UTC · Completed 8:03 PM UTC
Commit: 6b4fcd4 · View workflow run →

@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 AGENTS.md
@rh-hemartin
rh-hemartin enabled auto-merge July 24, 2026 06:39
@rh-hemartin
rh-hemartin added this pull request to the merge queue Jul 24, 2026
Merged via the queue into fullsend-ai:main with commit 331d749 Jul 24, 2026
18 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 24, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 6:52 AM UTC · Completed 7:01 AM UTC
Commit: 6b4fcd4 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5527 — docs(agents): forbid unsolicited e2e suite timeout bumps

PR: Small docs-only change (+5/−1, 2 files) by ifireball adding a policy that agents must not increase e2e/behaviour suite timeouts without explicit human authorization.

Timeline

  1. PR opened (Jul 23 12:34 UTC). qodo-code-review flagged an inconsistency in the new policy text: paragraph 1 accepts authorization from "an issue/PR comment" while paragraph 2 restricts it to "the linked issue or PR description" — materially different scopes.
  2. fullsend-ai-review (run 30007575353) posted a [high] protected-path finding on AGENTS.md and requested changes.
  3. Author force-pushed at 19:47 UTC (rebase adding new sections to go-code.md, but the policy text was unchanged).
  4. fullsend-ai-review re-ran (run 30039442799), posting an identical duplicate [high] protected-path inline comment and a second CHANGES_REQUESTED review.
  5. rh-hemartin approved (Jul 24 06:38 UTC), overriding the bot. PR merged at 06:49 UTC.

What went well

  • The review agent correctly identified the protected-path change to AGENTS.md and deferred judgment to a human reviewer.
  • The sticky comment pattern worked well — the summary comment was edited in place with the previous run folded into a collapsible section.
  • Label assignment (component/docs, component/e2e) was appropriate.
  • Human reviewer approved and merged, correctly overriding the procedural bot finding on this trusted human-authored docs PR.

What could go better

1. Policy text inconsistency merged unfixed. qodo flagged a valid inconsistency between two paragraphs in the new policy text — one allows authorization via any "issue/PR comment" while the other restricts it to "linked issue or PR description." Neither the author nor the approving reviewer addressed this finding, and the inconsistency was merged as-is. The fullsend review agent did not flag this inconsistency either. See proposal below.

2. Duplicate inline comments on force-push (existing issues). The review agent re-posted the same [high] protected-path inline comment verbatim after the force-push. This is well-tracked by existing issues: #5007 ("Deduplicate inline review comments across force-push re-reviews"), #1285, #2959, #1013, #1500. This retro provides additional evidence: on PR #5527, the second review run consumed ~14 minutes of Opus compute to produce zero new findings.

3. Review agent content inconsistency detection (existing issues). The review agent's docs-review skill focuses on staleness detection (code changed, docs didn't follow) rather than internal text consistency within the same document. The gap where qodo catches text-level contradictions that fullsend-ai-review misses is tracked by #3901 ("Review agent should detect internal contradictions in rule/guidance documents") and fullsend-ai/agents#265 ("Review correctness sub-agent should cross-reference verified facts to detect misleading documentation narratives"). This retro provides a clean example of the gap.

4. Protected-path severity calibration (existing issues). The [high] protected-path finding on a 5-word table description change to AGENTS.md, authored by a human with commit access, was correct per policy but noisy. This is tracked by #1551 ("Downgrade protected-path severity for human-authored PRs") and related issues #1392, #2745, #3487.

Proposals filed

ggallen pushed a commit to ggallen/fullsend that referenced this pull request Jul 24, 2026
The suite-timeout policy added in PR fullsend-ai#5527 used inconsistent
authorization-source language: paragraph 1 accepted "an issue/PR
comment" while paragraph 2 restricted to "the linked issue or PR
description". These are distinct scopes in GitHub's data model.

Align paragraph 2 to match paragraph 1's broader scope by changing
"linked issue or PR description" to "linked issue or a PR comment".
This preserves the original intent and is more practical since
humans naturally authorize via comments.

Closes fullsend-ai#5560
@github-actions
github-actions Bot deleted the docs/e2e-suite-timeout-agent-rule branch August 23, 2026 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/docs User-facing documentation component/e2e End-to-end tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants