Skip to content

test(#5493): fold PR-to-issue negative into PR-label positive scenario - #5498

Merged
ifireball merged 2 commits into
mainfrom
agent/5493-fold-pr-issue-negative
Jul 23, 2026
Merged

test(#5493): fold PR-to-issue negative into PR-label positive scenario#5498
ifireball merged 2 commits into
mainfrom
agent/5493-fold-pr-issue-negative

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Fold the standalone "PR does not trigger issue-only harness" negative scenario into the existing "PR label dispatches PR-only harness" positive scenario, eliminating a separate ~90s settle wait and GHA round-trip. The merged scenario registers both a PR-only harness (pr-ping) and an issue-only harness (issue-only-ping) with the same label, then asserts that pr-ping dispatched exactly once while issue-only-ping did not run.

Related Issue

Fixes #5493 (part of #5490)

Changes

  • Removed standalone "PR does not trigger issue-only harness" scenario from dispatch.feature
  • Extended the PR-label positive scenario to also register an issue-only harness (issue-only-ping) with work_item entity kind and the same label
  • Added assertion: pr-ping was dispatched exactly 1 time
  • Added assertion: issue-only-ping agent did not run
  • Follows the enabled/disabled pattern already used by the "Disabled harness" scenario

Testing

  • Behaviour dispatch feature stays green (make e2e-test or behaviour subset)
  • PR-label positive scenario proves pr-ping ran and issue-only harness did not

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • No secrets committed
  • Change is scoped to issue acceptance criteria only

Closes #5493

Post-script verification

  • Branch is not main/master (agent/5493-fold-pr-issue-negative)
  • Secret scan passed (gitleaks — c9c7540c2995d9a294ede00c2444d2eb600e4852..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 23, 2026 04:53
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Jul 23, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:54 AM UTC · Completed 5:04 AM UTC
Commit: a238b8e · View workflow run →

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Site preview

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

Commit: 034f8468177a41a9180a2b1f18f950a67e97cb2a

@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

Medium

  • [protected-path] .github/workflows/e2e.yml:192 — PR modifies a protected path (.github/). The timeout bump from 30 to 45 minutes supports the consolidated test scenarios and is mechanically necessary. Human approval is required for protected-path changes.

Low

  • [scope-implicit-infrastructure] .github/workflows/e2e.yml:192 — Timeout increase from 30 to 45 minutes (both e2e.yml and Makefile) is mechanically necessary for the consolidated test scenarios but was not called out in the acceptance criteria.
Previous run

Review

Findings

Medium

  • [protected-path] .github/workflows/e2e.yml:192, AGENTS.md — PR modifies protected paths. The e2e.yml timeout bump (30→45 min) is a mechanical consequence of consolidating behaviour test scenarios. The AGENTS.md mintcore documentation originated from main (PR docs(#5458): document full mintcore file lifecycle in AGENTS.md #5459) and appears in this diff via branch merge, not new work in this PR. Human approval is required for protected-path changes.

Low

Previous run (2)

Looks good to me


Labels: PR modifies e2e behaviour dispatch feature tests

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge component/e2e End-to-end tests component/dispatch Workflow dispatch and triggers Review effort 2/5 labels Jul 23, 2026
@ifireball ifireball added ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) and removed ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) labels Jul 23, 2026
@ifireball ifireball added ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) and removed ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) labels Jul 23, 2026
@ifireball

Copy link
Copy Markdown
Member

/fs-fix

Behaviour CI is flaking against the 30m job ceiling; the folded dispatch scenario itself is fine.

Evidence

Fix (in this PR)

  1. In .github/workflows/e2e.yml, bump the behaviour job timeout-minutes from 30 → 45 so a healthy ~28–29m suite has headroom for transient GHA/harness delays.
  2. If make behaviour-test / go test ... -timeout 30m is also capped at 30m, raise that to match (e.g. 40m or 45m) so the Go test timeout is not the next cliff after the job timeout bump.
  3. Do not change the folded dispatch.feature scenario logic — it already passes when infra cooperates.

Out of scope / note

Transient e2e admin failure (connection reset by peer fetching agents scripts) and triage issue-open dispatch flakes are infrastructure noise; the timeout bump is the actionable reliability fix for this PR's babysit.

@ifireball

Copy link
Copy Markdown
Member

/fs-fix

Please rebase this branch onto current upstream/fullsend-ai main (branch is ~11 commits behind; sibling consolidation PRs #5497/#5499/#5500/#5501 already landed), resolve any conflicts, and push the rebased branch.

Then apply the prior timeout reliability fix if not already present after rebase:

  1. In .github/workflows/e2e.yml, bump the behaviour job timeout-minutes from 30 → 45 (healthy suite is ~24–29m; 30m is too tight).
  2. If make behaviour-test / the Go test -timeout is also 30m, raise it to match (40–45m).
  3. Do not change the folded dispatch.feature scenario logic.

After push, leave the branch based on up-to-date main so behaviour CI can re-run cleanly with ok-to-test.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure · Started 7:54 AM UTC · Completed 8:05 AM UTC
Commit: 7ada4e0 · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

⚠️ Post-fix script failed — Push rejected — workflows permission (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Environmental limitation: the GitHub App lacks workflows write permission on this repository. The agent's patch is not necessarily wrong — update repo or app permissions (or avoid .github/workflows/ changes) and retry.

Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/29989530182

Details:
To https://github.com/fullsend-ai/fullsend.git
! [rejected] agent/5493-fold-pr-issue-negative -> agent/5493-fold-pr-issue-negative (non-fast-forward)
error: failed to push some refs to 'https://github.com/fullsend-ai/fullsend.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. If you want to integrate the remote changes,
hint: use 'git pull' before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
To https://github.com/fullsend-ai/fullsend.git
! [remote rejected] agent/5493-fold-pr-issue-negative -> agent/5493-fold-pr-issue-negative (refusing to allow a GitHub App to create or update workflow .github/workflows/e2e.yml without workflows permission)
error: failed to push some refs to 'https://github.com/fullsend-ai/fullsend.git'
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

@ifireball

Copy link
Copy Markdown
Member

/fs-fix

Previous fix run failed at push: the GitHub App cannot update .github/workflows/e2e.yml (missing workflows permission). Do not touch workflow files in this round.

Do only

  1. Rebase agent/5493-fold-pr-issue-negative onto current fullsend-ai/fullsend main (branch is still ~11 commits behind; feat(#5492): skip negative harness settle when positive wait elapsed #5497/test(#5491): merge triage sufficient + sandbox URL-block scenarios #5499/test(#5495): fold wrong-label negative into issue-label positive scenario #5500/test(#5494): fold fork-PR issue-harness negative into fork-label positive #5501 landed).
  2. Resolve any conflicts.
  3. Force-push the rebased branch (--force-with-lease is fine for this rebase).
  4. Do not change .github/workflows/**, Makefile timeouts, or the folded dispatch.feature scenario logic.

A maintainer will apply the behaviour timeout-minutes 30→45 bump in a follow-up human push once the rebase lands.

@rh-hemartin

Copy link
Copy Markdown
Member

/fs-retro This fix agent comment regarding failure within the sandbox regarding workflow permissions is not properly formatted: #5498 (comment), open an issue

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:12 AM UTC · Completed 8:20 AM UTC
Commit: 7ada4e0 · View workflow run →

@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:13 AM UTC · Completed 8:34 AM UTC
Commit: 7ada4e0 · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Rebased branch onto current main (0dcbe1a). Rebase resolved cleanly — no conflicts. Only dispatch.feature changed (the PR's intended change). No workflow files, Makefile timeouts, or scenario logic were modified. The post-script needs to force-push since history was rewritten by the rebase.

Fixed (2):

  1. Rebase branch onto current main: Rebased agent/5493-fold-pr-issue-negative onto current origin/main (0dcbe1a). The rebase resolved cleanly with no conflicts. The branch was ~11 commits behind main (PRs feat(#5492): skip negative harness settle when positive wait elapsed #5497, test(#5491): merge triage sufficient + sandbox URL-block scenarios #5499, test(#5495): fold wrong-label negative into issue-label positive scenario #5500, test(#5494): fold fork-PR issue-harness negative into fork-label positive #5501 and others). The rebased commit (870d8aa) replays the original PR changes on top of the current main, preserving the folded dispatch.feature scenario logic. No workflow files (.github/workflows/**), Makefile timeouts, or scenario logic were modified.
  2. Do not touch workflow files: Verified that no .github/workflows/** files are in the diff. The only changed file is e2e/behaviour/features/dispatch/dispatch.feature, which is the intended PR change.

Tests: passed

Updated by fullsend fix agent

@ifireball

Copy link
Copy Markdown
Member

/fs-review

Branch is rebased onto current main. Human follow-up commit bumps behaviour job timeout-minutes 30→45 and matching make behaviour-test Go -timeout to 45m (fix agent cannot push workflow files). Folded dispatch.feature logic unchanged.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 8:24 AM UTC · Ended 8:40 AM UTC
Commit: 7ada4e0 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5498 — fold PR-to-issue negative into PR-label positive scenario

Workflow overview

PR #5498 was opened by the code agent to resolve issue #5493 (part of #5490). It folds two e2e dispatch scenarios into one, eliminating a separate ~90s settle wait. The change is small (+10/−12 lines, single file dispatch.feature).

Timeline:

  • 04:34 — Issue test(behaviour): fold PR↛issue-harness negative into PR-label positive scenario #5493 triaged (run 29979911759)
  • 04:45 — Code agent dispatched (run 29980334797), PR opened at 04:53
  • 04:53–05:04 — Review agent approved (run 29980695180)
  • 05:27–07:17 — Human toggled ok-to-test multiple times; behaviour CI repeatedly hit the 30m job timeout
  • 07:50–07:52 — Human (ifireball) issued /fs-fix to rebase and bump e2e.yml timeout from 30→45m
  • 07:54–08:05 — Fix agent run 1 (29989530182) failed: rebased and modified e2e.yml, but push rejected — GitHub App lacks workflows permission
  • 08:05 — Fix agent posted a failure comment with raw git error output
  • 08:10 — Human (rh-hemartin) approved the PR; human (ifireball) issued third /fs-fix scoped to rebase-only
  • 08:11 — Human (rh-hemartin) requested /fs-retro about the failure comment formatting
  • 08:12–08:20 — Fix agent run 2 (29990446816) succeeded: rebased without touching workflow files

What went well

  • Code quality: Clean, focused change that correctly merges two e2e scenarios. Both the review agent and human reviewer approved without requesting changes.
  • Recovery: Human correctly diagnosed the workflows permission issue from the failure comment and scoped the retry to avoid workflow files. Fix agent run 2 succeeded.
  • Pipeline efficiency: Triage → code → review → approval took ~35 minutes for a straightforward change.

Existing issue corroboration

The fix agent's attempt to modify .github/workflows/e2e.yml without the required workflows permission is well-tracked across multiple existing issues: #3627 (code agent should treat workflows as unpushable), #3814 (triage should avoid recommending workflow changes), #2822 (epic: least-privilege path to workflow file changes), agents#139 (post-script should detect permission rejections). This PR provides another data point for those issues — the fix agent was explicitly asked to modify the workflow file by a human, so the agent-side guardrails (detecting unpushable paths before attempting the change) would have saved one failed run (~11 minutes of compute).

New proposal

One net-new issue identified: the post-failure comment's Details section uses 4-space indented code blocks instead of fenced code blocks, causing rendering issues on GitHub. See proposal below.

Proposals filed

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:24 AM UTC · Completed 8:40 AM UTC
Commit: 7ada4e0 · View workflow run →

@fullsend-ai-coder
fullsend-ai-coder Bot force-pushed the agent/5493-fold-pr-issue-negative branch from a238b8e to 870d8aa Compare July 23, 2026 08:43
github-merge-queue Bot pushed a commit that referenced this pull request Jul 23, 2026
The behaviour test dispatch detection polling loop used
dispatchMaxTry=24 × dispatchPoll=5s = 120s (2 minutes) to
detect whether GitHub Actions dispatched a workflow in
response to an issues event. When GitHub webhook delivery
is under load, this 2-minute window is exceeded, causing
false test failures that require human intervention to
retry (observed on PRs #5497 and #5498 on the same day
with unrelated code changes).

Raise dispatchMaxTry from 24 to 48, extending the
detection window to 48 × 5s = 240s (4 minutes). This stays
well within the existing dispatchWait of 12 minutes for
workflow completion. Add a regression test that asserts the
dispatch detection window is at least 4 minutes.

Note: pre-commit could not run (network 403 in sandbox).
Manual linting verification is required.

Closes #5503
@ifireball

Copy link
Copy Markdown
Member

/fs-fix

Branch drifted again — now ~6 commits behind main (including #5506 dispatch window bump and renovate merges). Behaviour run on 1e1f61bc was cancelled mid-suite (~30m in, during triage scenario), not a timeout.

Do only

  1. Rebase onto current fullsend-ai/fullsend main.
  2. Resolve conflicts if any.
  3. Force-push with --force-with-lease.
  4. Do not change .github/workflows/**, Makefile, or folded dispatch.feature logic (timeout bump commit 1e1f61bc must be preserved/replayed).

After push, maintainer will re-apply ok-to-test for a clean behaviour run.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure · Started 9:05 AM UTC · Completed 9:10 AM UTC
Commit: 7ada4e0 · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

⚠️ Post-fix script failed — Signed-off-by rejected (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/29993761849

Details:
Agent commit contains a Signed-off-by trailer. Agents must not use 'git commit -s' or append Signed-off-by trailers.
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

fullsend-ai-coder Bot and others added 2 commits July 23, 2026 12:15
Remove the standalone "PR does not trigger issue-only harness" scenario
and fold its negative assertion into the "PR label dispatches PR-only
harness" scenario. The positive scenario already opens a PR, labels it,
and waits for pr-ping to complete — the negative check piggy-backs on
the same dispatch round-trip without a separate 90s settle wait.

The merged scenario now:
- Registers both pr-ping (change_proposal) and issue-only-ping
  (work_item) with the same label, verifying entity kind filtering
- Asserts pr-ping workflow completes and dummy proves execution
- Asserts pr-ping was dispatched exactly 1 time
- Asserts issue-only-ping agent did not run

This follows the enabled/disabled pattern already established by the
"Disabled harness is not dispatched while enabled one triggers" scenario.

Closes #5493

Signed-off-by: Barak Korren <bkorren@redhat.com>
Healthy behaviour suites run ~24–29m and were flaking against the 30m
job ceiling after the folded dispatch scenario already passed. Align the
Makefile go test -timeout with the workflow budget.

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

Copy link
Copy Markdown
Member

/fs-review

Behaviour is green on 034f8468 (21m11s) after rebase onto main. Please re-review current head.

Note for findings triage:

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:52 AM UTC · Completed 10:02 AM UTC
Commit: 7ada4e0 · 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 .github/workflows/e2e.yml
Comment thread .github/workflows/e2e.yml
@ifireball
ifireball requested a review from rh-hemartin July 23, 2026 10:05
@ifireball

Copy link
Copy Markdown
Member

Ready for CODEOWNERS re-approval

All CI green on 034f8468 including behaviour 21m11s / e2e / test.

Latest review findings (≤ done bar):

  • Medium [protected-path] .github/workflows/e2e.yml — intentional timeout 30→45 (and matching Makefile). Needs human CODEOWNERS approval on current head (prior @rh-hemartin approve was on a238b8e8 before this commit).
  • Low [scope-implicit-infrastructure] — same timeout; intentional, not a code defect.
  • No concurrency / shared-World / GODOG_CONCURRENCY findings.

Sibling timeout-only PR: #5519 (same bump for landing on main so pull_request_target base YAML picks it up for future runs).

@rh-hemartin could you re-approve current head for the .github/ protected path?

@ifireball
ifireball added this pull request to the merge queue Jul 23, 2026
Merged via the queue into main with commit 4ef60e7 Jul 23, 2026
20 checks passed
@ifireball
ifireball deleted the agent/5493-fold-pr-issue-negative branch July 23, 2026 10:15
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 10:17 AM UTC · Completed 10:35 AM UTC
Commit: 034f846 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5498 — fold PR-to-issue negative into PR-label positive scenario

Workflow overview

PR #5498 was a small e2e test consolidation (+12/−14 lines, 3 files) opened by the code agent at 04:53 UTC to resolve issue #5493. It merged at 10:15 UTC (~5h22m total). The code change itself was clean and correct, but the PR required substantial human intervention due to two known infrastructure bugs that caused fix agent failures.

Timeline

  • 04:34 — Issue test(behaviour): fold PR↛issue-harness negative into PR-label positive scenario #5493 triaged
  • 04:45–04:53 — Code agent dispatched and opened PR with a focused dispatch.feature change
  • 04:54–05:04 — Review agent approved: "Looks good to me" (commit a238b8e)
  • 05:27–07:17 — Human toggled ok-to-test multiple times; behaviour CI repeatedly hit 30m job timeout
  • 07:50–07:52 — Human (ifireball) issued /fs-fix to rebase and bump e2e.yml timeout 30→45m
  • 07:54–08:05Fix run 1 FAILED: Agent rebased and modified e2e.yml, but push rejected — GitHub App lacks workflows permission (run 29989530182)
  • 08:10 — Human scoped next /fs-fix to rebase-only (avoid workflow files)
  • 08:12–08:20Fix run 2 SUCCEEDED: Rebased without touching workflow files (run 29990446816)
  • 08:22 — Human pushed timeout bump commit manually, requested /fs-review
  • 08:24–08:40 — Review agent correctly detected [protected-path] and [scope-implicit-infrastructure], applied requires-manual-review
  • 08:34 — First retro filed agents#390 for post-failure comment formatting
  • 09:04 — Human requested /fs-fix to rebase again (branch drifted ~6 commits behind)
  • 09:05–09:10Fix run 3 FAILED: Post-fix script rejected with Signed-off-by false positive after rebase (run 29993761849)
  • 09:15 — Human manually force-pushed the rebase
  • 09:50–10:02 — Behaviour green; review agent confirmed same findings
  • 10:05–10:15 — Merge queue processed and merged

What went well

  • Code agent quality: Clean, focused change produced in ~18 minutes. No rework on the code itself.
  • Review agent calibration: Correctly approved the initial narrow change (dispatch.feature only), then correctly escalated to requires-manual-review after the human-added timeout commit expanded scope to include .github/workflows/e2e.yml.
  • First retro quality: Correctly identified known issues, filed one net-new proposal (agents#390), avoided duplicates.

Evidence for existing issues (no new proposals needed)

  • agents#318 / fullsend#5419: Fix run 3 (09:05–09:10 UTC) hit the same SCAN_RANGE false positive — after rebasing, the post-fix script scanned upstream human commits with legitimate Signed-off-by trailers and attributed them to the agent. This is the second occurrence in 3 days (first was PR refactor(#5390): add polling-support methods to GitLab client #5391 on Jul 21). Human had to manually force-push the rebase, wasting ~10 minutes.
  • fullsend#3627: Fix run 1 (07:54–08:05 UTC) modified .github/workflows/e2e.yml as instructed by the human, but the GitHub App token lacks workflows permission. The agent's work was correct but unpushable. Pre-flight pushability detection would have saved ~11 minutes of compute.
  • agents#390: Already filed by the first retro on this PR for post-failure comment formatting (indented vs fenced code blocks).

Agents repo

Agent definitions resolved from fullsend-ai/agents at commit ca518d9353f5a8363c6ba499d7a2070a1b0e7c5d.

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

Labels

component/dispatch Workflow dispatch and triggers component/e2e End-to-end tests ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) ready-for-review Triggers review agent dispatch requires-manual-review Review requires human judgment Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(behaviour): fold PR↛issue-harness negative into PR-label positive scenario

2 participants