Skip to content

fix(impl-generate): keep infrastructure failures out of the 3-attempt cap - #11199

Merged
MarkusNeusinger merged 7 commits into
mainfrom
fix/impl-generate-cap-excludes-infra-failures
Sep 2, 2026
Merged

fix(impl-generate): keep infrastructure failures out of the 3-attempt cap#11199
MarkusNeusinger merged 7 commits into
mainfrom
fix/impl-generate-cap-excludes-infra-failures

Conversation

@MarkusNeusinger

Copy link
Copy Markdown
Owner

Summary

  • The retry cap in impl-generate.yml counted every failed run alike. During the Claude outage of 2026-09-02 (03:15–03:45 UTC) every run ended in is_error:true with an "Internal error"; 27 pairs burned all three attempts in twenty minutes, and for the rest of the 12-hour window each re-dispatch ran without any auto-retry.
  • The failure handler now classifies the run from step outcomes (steps.<id>.outcome, plus the Claude execution log for the provider-side signature): both Claude runs dying with Internal error/overloaded/rate limit/ECONNRESET, a Google Cloud auth/SDK/upload failure, or a GitHub API failure at PR creation or review dispatch is an infrastructure failure. Authenticate to GCP, Set up Cloud SDK and Trigger review workflow get ids for that.
  • Infrastructure failures are recorded with an extra <!-- impl-fail-cause:infra --> tag in the same marker comment, excluded from the genuine count (one paginated query returns both numbers), and retried on a separate cap of 5 per window; at that cap the pair is paused with a comment and without impl:<lib>:failed, because an incident is not a capability verdict. "Implementation file not found" and a missing theme render keep counting as before.
  • The retry dispatch is factored into dispatch_retry() and used by both branches; docs (docs/workflows/overview.md, agentic/docs/project-guide.md) describe the label semantics.

Plan

N/A

Test plan

  • Workflow change has no local verification loop (CLAUDE.md known gap): the run script parses as YAML and passes bash -n; the new jq counting filter was dry-run against a synthetic comment list (genuine=1, infra=1).
  • After merge, a run that fails at Authenticate to GCP posts the :cloud: infrastructure comment, retries, and the next run's notice reads Previous failures ...: N (plus 1 infrastructure failures, not counted).
  • A run that fails with "Implementation file not found" still posts Attempt N/3 and counts.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu

… cap

The cap exists to stop re-running a pair the model cannot solve. It counted
every failed run alike, so the Claude outage of 2026-09-02 (03:15-03:45 UTC,
every run ending in `is_error:true` with an "Internal error") burned all
three attempts of 27 pairs in twenty minutes, and for the rest of the
12-hour window each re-dispatch ran without any auto-retry.

The failure handler now classifies the run from step outcomes: both Claude
runs dying with a provider-side signature in the execution log, a Google
Cloud auth/SDK/upload failure, or a GitHub API failure at PR creation or
review dispatch is an infrastructure failure. Those markers carry an extra
`<!-- impl-fail-cause:infra -->` tag, are excluded from the genuine count,
and are retried on a separate cap of 5 per window, after which the pair is
paused without `impl:<lib>:failed`. "Implementation file not found" and a
missing theme render keep counting as before.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu
Copilot AI lite review requested due to automatic review settings September 2, 2026 16:19
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The workflow’s infra-cause extraction can produce an empty/low-signal “Cause” message (and one inline comment is misleading), which should be corrected to avoid confusing incident diagnostics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates the impl-generate workflow to distinguish “infrastructure” failures (provider incident, cloud auth/upload, GitHub API) from genuine generation failures, so outages don’t consume the 3-attempt cap for a (spec, library) pair. It also documents the updated retry/label semantics.

Changes:

  • Classify failures using step outcomes plus the Claude execution log signature, and track infra failures separately from genuine failures.
  • Add an infra-only retry cap (5 per 12-hour window) that pauses the pair without setting impl:{library}:failed.
  • Update workflow documentation and changelog to reflect the new label and retry behavior.
File summaries
File Description
.github/workflows/impl-generate.yml Adds infra-failure classification, separate counting/tagging, and a separate infra retry cap with pause behavior.
docs/workflows/overview.md Updates the meaning of impl:{library}:failed to reflect campaign-window semantics and infra exclusions.
agentic/docs/project-guide.md Mirrors the updated impl:{library}:failed semantics for agent-focused docs.
CHANGELOG.md Records the infra-failure retry behavior change under Fixed.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/impl-generate.yml
Comment thread .github/workflows/impl-generate.yml
Copilot AI review requested due to automatic review settings September 2, 2026 16:25
… address review

- INFRA_CAUSE always carries a snippet (falls back to a generic
  provider-side note when only the HTTP status matched)
- comment states that only the retry step's outcome is inspected

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The new infra-classification path can inadvertently fail under Actions’ default bash -eo pipefail due to a grep pattern mismatch in the INFRA_CAUSE extraction, which can prevent retries/parking from running.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/impl-generate.yml:1064

  • The infra classification builds INFRA_CAUSE via a command substitution that can fail under Actions' default bash -eo pipefail: the detection grep includes the "status": *5xx alternative, but the extraction grep does not. If the only match is the status code, grep -o returns 1 and can abort the whole failure-handler step, preventing infra retries/parking from running.
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 2, 2026 16:28
…-excludes-infra-failures

# Conflicts:
#	CHANGELOG.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The updated docs describe impl:{library}:failed as only a generation-attempt cap, but the same label is also applied by impl-review.yml for post-review failure, so the documentation is currently misleading.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread agentic/docs/project-guide.md Outdated
Comment thread docs/workflows/overview.md Outdated
Copilot AI review requested due to automatic review settings September 2, 2026 16:32
MarkusNeusinger and others added 2 commits September 2, 2026 18:35
…repairs

Review feedback: the label table and the project guide described only the
generation path.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The workflow uses a comma-separated --remove-label that can cause gh issue edit to fail to remove either label when one is missing, which undermines the new pause/retry behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .github/workflows/impl-generate.yml Outdated
Copilot AI review requested due to automatic review settings September 2, 2026 16:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes core GitHub Actions retry/labeling behavior with operational impact and can’t be fully validated here without executing real workflow runs across failure modes.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…andler

Review feedback: a comma-separated --remove-label fails as a whole when
one name cannot be resolved, which could leave a pair marked pending.
Applies to the new infrastructure-pause branch and the existing cap branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu
Copilot AI review requested due to automatic review settings September 2, 2026 16:40
@MarkusNeusinger
MarkusNeusinger merged commit a870820 into main Sep 2, 2026
8 checks passed
@MarkusNeusinger
MarkusNeusinger deleted the fix/impl-generate-cap-excludes-infra-failures branch September 2, 2026 16:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The new infra-classification logic currently treats any GCS upload step failure as “infrastructure,” which can misclassify missing-theme renders and contradict the intended attempt-counting behavior.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

.github/workflows/impl-generate.yml:1070

  • GCS_OUTCOME == failure is treated as an infrastructure failure, but the Upload to GCS Staging step can fail due to missing plot renders (plot-light.png/plot-dark.png), which the PR description says should still count toward the 3-attempt cap. As written, a missing-theme render would be misclassified as infra and not spend the budget.
    agentic/docs/project-guide.md:792
  • This bullet refers to the workflows as plain text ("impl-generate", "impl-review"), while other docs in this PR use the canonical workflow filenames (e.g., impl-generate.yml). Using the exact filenames in backticks makes it unambiguous which workflow sets the label.
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants