Skip to content

fix(#5716): fail fast on concluded harness workflow failures - #5732

Merged
ifireball merged 2 commits into
mainfrom
agent/5716-harness-fail-fast
Jul 30, 2026
Merged

fix(#5716): fail fast on concluded harness workflow failures#5732
ifireball merged 2 commits into
mainfrom
agent/5716-harness-fail-fast

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

WaitForHarnessAgent now fails fast when a recent workflow run has a terminal failure conclusion (failure, timed_out, startup_failure) instead of polling the full ~12-minute deadline. On true timeout, the error message includes diagnostic information about recent workflow runs (ID, status, conclusion, URL) for faster debugging.

Related Issue

Closes #5716

Changes

  • Add isTerminalFailure helper to identify failure/timed_out/startup_failure conclusions (excluding skipped/cancelled concurrency noise)
  • Add listRecentRunsAfter helper to fetch and filter recent workflow runs by trigger time
  • Add formatRunDiagnostics to build human-readable run summaries for error messages
  • Modify WaitForHarnessAgent to check for terminal failures on each poll iteration and include diagnostics on timeout
  • Add comprehensive tests covering fail-fast on each terminal conclusion, exclusion of skipped/cancelled, time filtering, and diagnostic formatting

Testing

  • go vet passes
  • All 27 package tests pass including 10 new tests
  • Tests cover: fail-fast on failure/timed_out/startup_failure, skipped and cancelled exclusion, runs-before-trigger-time filtering, timeout diagnostics formatting, empty diagnostics

Closes #5716

Post-script verification

  • Branch is not main/master (agent/5716-harness-fail-fast)
  • Secret scan passed (gitleaks — a486edd84e78b51ffa5fa006b624185fdc1c5965..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 29, 2026 16:03
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Jul 29, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:05 PM UTC · Completed 4:18 PM UTC
Commit: 18b23bf · View workflow run →

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.06977% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...iourtest/drivers/ci/githubactions/githubactions.go 79.06% 7 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [test-gap] pkg/behaviourtest/drivers/ci/githubactions/githubactions_test.goFakeClient.ListWorkflowRuns stores at most one WorkflowRun per workflow key (map[string]*WorkflowRun), so listHarnessRunsAfter can only ever return 0 or 1 runs in tests. The fail-fast loop iterates over all returned runs, and production ListWorkflowRuns can return many. The multi-run iteration path is untested — e.g., a scenario where the first run is skipped and the second is failure cannot be constructed with the current FakeClient.
    Remediation: Consider extending FakeClient with a slice-based field (e.g., WorkflowRunsByFile map[string][]WorkflowRun) so ListWorkflowRuns can return multiple runs.

  • [edge-case] pkg/behaviourtest/drivers/ci/githubactions/githubactions.go:459listHarnessRunsAfter silently swallows ListWorkflowRuns errors and returns nil. When the API is consistently unreachable throughout the polling window, the timeout diagnostic reports "no recent workflow runs found after trigger time" — misleading when the actual problem is API unreachability. Pre-existing pattern, not a regression.

Previous run

Review

Findings

Medium

  • [false-positive fail-fast] pkg/behaviourtest/drivers/ci/githubactions/githubactions.go:515 — The fail-fast check in WaitForHarnessAgent iterates over all recent workflow runs from ListRecentWorkflowRuns without filtering by workflow name, agent, or any other scope. ListRecentWorkflowRuns returns runs across all workflows in the repository (it hits the /repos/{owner}/{repo}/actions/runs endpoint). If an unrelated workflow in the same repository (e.g., a CI check, a different agent's harness run) has a terminal failure conclusion, this code will abort WaitForHarnessAgent even though the harness run for the requested agent may still be in progress or may succeed. The time-window filter (after) provides some mitigation, but in repos with active CI, coincidental terminal failures within the window are plausible.
    Remediation: Filter the recent runs to only those associated with the harness workflow before applying the fail-fast check. The forge.WorkflowRun struct has a Name field, and there is a ListWorkflowRuns method that filters by workflow file — either could be used to scope the check.

Low

  • [error-message-consistency] pkg/behaviourtest/drivers/ci/githubactions/githubactions.go:515 — The fail-fast error message uses url: %s (colon separator) while formatRunDiagnostics uses url=%s (equals separator). Minor inconsistency in metadata formatting within the same file.
    Remediation: Align to url=%s to match formatRunDiagnostics.

  • [edge-case] pkg/behaviourtest/drivers/ci/githubactions/githubactions.go:494 — When API calls (ListRepositoryArtifacts, ListRecentWorkflowRuns) fail consistently throughout the polling window, the timeout diagnostic reports "no recent workflow runs found after trigger time" — misleading when the actual problem is API unreachability. Pre-existing pattern, not a regression introduced by this PR.


Labels: PR modifies harness agent wait logic in the behaviour test CI driver

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/harness Agent harness, config, and skills loading labels Jul 29, 2026
@ifireball

Copy link
Copy Markdown
Member

/fs-fix rebase the PR and fix AI review comments

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure · Started 8:12 PM UTC · Completed 8:21 PM UTC
Commit: 18b23bf · 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/30487491366

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.

@ifireball

Copy link
Copy Markdown
Member

/fs-fix redo the last fix, make sure not to include the Signed-off-by trailer

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure · Started 8:25 PM UTC · Completed 8:35 PM UTC
Commit: 18b23bf · View workflow run →

fullsend-ai-coder Bot added a commit that referenced this pull request Jul 29, 2026
The fail-fast check in WaitForHarnessAgent previously used
ListRecentWorkflowRuns, which returns runs across all workflows in the
repository. This could cause false-positive fail-fast when an unrelated
workflow (e.g., a CI check) had a terminal failure within the time
window.

Scope the check to only the harness workflow (fullsend.yaml) by using
ListWorkflowRuns instead of ListRecentWorkflowRuns. Also align the
fail-fast error message to use url= (equals separator) consistently
with formatRunDiagnostics.

Addresses review feedback on #5732
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

⚠️ Post-fix script failed — Structured output processing failed (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/30488267454

Details:
process-fix-result.py failed with exit code 1 (bad input) for PR #5732 in fullsend-ai/fullsend
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

@ifireball

Copy link
Copy Markdown
Member

/fs-fix rebase and fix all remaining review issues, list what you fix or was already fixed in the output

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure · Started 7:13 AM UTC · Completed 7:17 AM UTC
Commit: 49b521e · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

⚠️ Post-fix script failed — Structured output processing failed (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/30522112798

Details:
process-fix-result.py failed with exit code 1 (bad input) for PR #5732 in fullsend-ai/fullsend
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

@ifireball

Copy link
Copy Markdown
Member

/fs-fix rebase

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure · Started 8:15 AM UTC · Completed 8:20 AM UTC
Commit: 49b521e · View workflow run →

WaitForHarnessAgent previously only checked for a success artifact,
polling the full ~12-minute deadline even when the harness workflow
had already concluded with a failure. This wasted CI time and produced
opaque timeout messages.

Add fail-fast detection: on each poll iteration, check recent workflow
runs for terminal failure conclusions (failure, timed_out,
startup_failure). Skipped and cancelled runs are excluded to avoid
false positives from concurrency-group noise. When a terminal failure
is detected, fail immediately with the run ID, conclusion, and URL.

On true timeout (no artifact and no failed runs), the error now
includes diagnostic information about recent workflow runs so the
next CI flake is self-explanatory.

Closes #5716
The fail-fast check in WaitForHarnessAgent previously used
ListRecentWorkflowRuns, which returns runs across all workflows in the
repository. This could cause false-positive fail-fast when an unrelated
workflow (e.g., a CI check) had a terminal failure within the time
window.

Scope the check to only the harness workflow (fullsend.yaml) by using
ListWorkflowRuns instead of ListRecentWorkflowRuns. Also align the
fail-fast error message to use url= (equals separator) consistently
with formatRunDiagnostics.

Addresses review feedback on #5732
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

⚠️ Post-fix script failed — Structured output processing failed (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/30525914229

Details:
process-fix-result.py failed with exit code 1 (bad input) for PR #5732 in fullsend-ai/fullsend
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

@fullsend-ai-coder
fullsend-ai-coder Bot force-pushed the agent/5716-harness-fail-fast branch from 49b521e to a39fca5 Compare July 30, 2026 08:20
@ifireball

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 30, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:37 AM UTC · Completed 8:52 AM UTC
Commit: a39fca5 · View workflow run →

Comment thread pkg/behaviourtest/drivers/ci/githubactions/githubactions.go
@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Jul 30, 2026
@ifireball
ifireball added this pull request to the merge queue Jul 30, 2026
Merged via the queue into main with commit 99a84b9 Jul 30, 2026
23 checks passed
@ifireball
ifireball deleted the agent/5716-harness-fail-fast branch July 30, 2026 10:37
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 30, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 10:39 AM UTC · Completed 10:50 AM UTC
Commit: a39fca5 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5732 — fail fast on concluded harness workflow failures

Overall: The code and review agents performed well on the core task. The review agent correctly identified a real medium-severity scoping bug (fail-fast not filtered to the harness workflow), and the code agent produced a solid initial implementation with comprehensive tests. However, the fix agent loop was entirely broken — all 4 fix attempts failed due to post-script infrastructure issues, not agent logic. The human ultimately had to rebase manually and re-trigger review.

Timeline

  1. Issue test(behaviour): WaitForHarnessAgent should fail fast on failed harness runs #5716 created by @ifireball (Jul 29, 13:53 UTC)
  2. Triage required 3–4 attempts: first two correctly identified existing draft PR test(#5707): fix missing harness agent fixtures and timeout diagnostics #5714 as addressing the issue; a sandbox podman error caused one infrastructure failure; triage succeeded after the draft PR was closed
  3. Code agent created PR #5732 (16:03 UTC) — implementation was solid but status was reported as "Terminated" due to an OIDC token timeout when posting the completion comment (the PR was actually created successfully)
  4. Review agent correctly flagged the fail-fast logic as not scoped to the harness workflow (medium severity) — a genuine bug that would cause false positives from unrelated workflow failures
  5. Fix attempt 1 (20:11 UTC): Agent fixed the scoping issue but used git commit -s, violating the autonomous agent DCO exemption. Post-fix.sh guardrail correctly rejected it.
  6. Fix attempts 2–4 (20:23, 07:11, 08:14 UTC): All failed with identical error — process-fix-result.py could not find fix-result.schema.json at relative path scripts/../schemas/ because the content-addressable resource cache does not preserve sibling directory structure. The agent succeeded in all three runs; the post-script broke.
  7. Human intervention: @ifireball manually rebased, triggered /fs-review. Review agent approved. Human approved. Merged (10:37 UTC, Jul 30).

What went well

  • Review quality was strong. The review agent caught the workflow-scoping bug on first pass — a genuine medium-severity issue. On re-review after the fix, it correctly approved. The human reviewer added no additional findings.
  • Code agent produced good initial implementation with 10 new tests covering fail-fast, exclusion of noise conclusions, time filtering, and diagnostics formatting.
  • Post-fix.sh Signed-off-by guardrail worked correctly — caught the DCO violation on fix attempt 1.

What went wrong

  • 3 fix runs wasted on the same infrastructure bug — the schema path resolution failure in process-fix-result.py blocked all progress and required human workaround.
  • Total time to resolution: ~21 hours — most of the delay was from fix agent failures and overnight wait.

Existing issues with new evidence

Proposals filed

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

Labels

component/harness Agent harness, config, and skills loading ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(behaviour): WaitForHarnessAgent should fail fast on failed harness runs

1 participant