Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 66 additions & 9 deletions scripts/dev-lead-fix-ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ set -euo pipefail
# Env: PR_NUMBER, HEAD_SHA, CHECKS_JSON, REPO, GITHUB_REPOSITORY,
# DEV_LEAD_DRY_RUN, REVIEW_ENGINE, GH_TOKEN
# Optional: PROMPTS_DIR (defaults to prompts/dev-lead relative to CWD)
# Optional: MAX_FAIL_ATTEMPTS — consecutive engine failures before exhaustion (default: 2)

source "$(dirname "$0")/engine.sh"

Expand All @@ -14,19 +15,43 @@ CHECKS_JSON="${CHECKS_JSON:-[]}"
REPO="${REPO:-${GITHUB_REPOSITORY:-}}"
MAX_CI_CYCLES="${MAX_CI_CYCLES:-3}"
LOG_MAX_LINES="${LOG_MAX_LINES:-200}"
MAX_FAIL_ATTEMPTS="${MAX_FAIL_ATTEMPTS:-2}"
MARKER_PREFIX="<!-- dev-lead-fix-ci sha="
EXHAUSTION_MARKER="<!-- dev-lead-fix-ci pr=${PR_NUMBER} status=exhausted -->"
export PROMPTS_DIR="${PROMPTS_DIR:-prompts/dev-lead}"

# check_idempotency: returns 0 (skip) if this exact SHA was already attempted,
# OR if a PR-level exhaustion marker exists (blocks all future SHAs).
check_idempotency() {
local existing
existing=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" \
--jq "[.[] | select(.body | startswith(\"${MARKER_PREFIX}${HEAD_SHA}\"))] | length" 2>/dev/null || echo "0")
[ "$existing" -gt 0 ]
local comments
# Use standalone jq so the mock stub in tests can return raw JSON
comments=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" 2>/dev/null \
| jq -r '.[].body' 2>/dev/null || true)
Comment on lines +28 to +29

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The 'gh api' call for comments defaults to 30 items per page. If a PR has many comments, the exhaustion or idempotency markers might be missed. Adding '--per-page 100' or using '--paginate' would make this more robust. Keeping the error redirection inline ensures the intent to silence errors is clear for this specific tier.

Suggested change
comments=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" 2>/dev/null \
| jq -r '.[].body' 2>/dev/null || true)
comments=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" --per-page 100 2>/dev/null \
| jq -r '.[].body' 2>/dev/null || true)
References
  1. Keep error routing logic inline when tiers have distinct requirements to avoid obscuring intent or introducing false positives.


# PR-level exhaustion — blocks regardless of SHA
if echo "$comments" | grep -qF "${EXHAUSTION_MARKER}"; then
echo "::notice::PR #${PR_NUMBER} is exhausted (repeated engine failures) — skipping all future SHAs"
return 0
fi

# SHA-level idempotency — skip if this exact SHA was already attempted
if echo "$comments" | grep -qF "${MARKER_PREFIX}${HEAD_SHA}"; then
return 0
fi

return 1
}

# count_recent_failures: count status=failed markers on this PR (any SHA)
count_recent_failures() {
local pattern="${MARKER_PREFIX}[a-f0-9A-F]* status=failed"
gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" 2>/dev/null \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Similar to the idempotency check, this API call should handle pagination or at least increase the items per page to ensure all failure markers are counted correctly. Maintaining the inline error routing is preferred here to keep the logic explicit.

Suggested change
gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" 2>/dev/null \
gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" --per-page 100 2>/dev/null \
References
  1. Keep error routing logic inline when tiers have distinct requirements to avoid obscuring intent or introducing false positives.

| jq "[.[] | select(.body | test(\"${pattern}\"))] | length" 2>/dev/null \
|| echo "0"
Comment on lines +47 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Count only consecutive failures before exhausting

This counts every historical status=failed marker on the PR, even if a later status=applied, no-changes, or dry-run marker proves the failure streak was broken. In a PR with one old engine failure, then a successful auto-fix, then one new engine failure, MAX_FAIL_ATTEMPTS=2 will incorrectly post the PR-level exhaustion marker and block all future SHAs. The exhaustion check needs to consider only the latest consecutive failed attempts rather than all failed comments.

Useful? React with 👍 / 👎.

}

collect_logs() {
local check_name="$1" details_url="$2"
# Try to get run ID from details URL
local run_id
run_id=$(echo "$details_url" | grep -oP '(?<=runs/)\d+' || true)
if [ -n "$run_id" ]; then
Expand Down Expand Up @@ -65,21 +90,38 @@ post_summary() {
**PR:** #${PR_NUMBER} | **SHA:** \`${HEAD_SHA}\`
${details}"
if [ "${DEV_LEAD_DRY_RUN:-false}" = "true" ]; then
echo "[dry-run] would post PR comment:"
echo "[dry-run] would post PR comment: $status"
echo "$body"
else
gh pr comment "$PR_NUMBER" --repo "$REPO" --body "$body"
fi
}

post_exhaustion() {
local reason="$1"
local body="${EXHAUSTION_MARKER}
## Dev-Lead Fix CI — exhausted

This PR has had **${MAX_FAIL_ATTEMPTS}** consecutive engine failures (timeouts or errors). Automated CI fixing has been paused to avoid consuming further tokens.

**Reason for last failure:** ${reason}

To re-enable, delete this comment or push a new commit with a substantially different change."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Do not tell users a new commit clears exhaustion

The exhaustion marker is PR-level (check_idempotency skips whenever this comment exists, regardless of HEAD_SHA), so pushing a new commit does not re-enable automated fixing despite this message saying it does. In the exhaustion path users will follow the suggested remediation, push another commit, and still be blocked until the comment is deleted, so the comment should only advertise actions that the script actually honors.

Useful? React with 👍 / 👎.

if [ "${DEV_LEAD_DRY_RUN:-false}" = "true" ]; then
echo "[dry-run] would post exhaustion comment"
else
gh pr comment "$PR_NUMBER" --repo "$REPO" --body "$body"
fi
}

main() {
if [ -z "$PR_NUMBER" ] || [ -z "$HEAD_SHA" ]; then
echo "::error::PR_NUMBER and HEAD_SHA are required"
exit 1
fi

if check_idempotency; then
echo "::notice::Already handled CI failure at SHA $HEAD_SHA — skipping (idempotent)"
echo "::notice::Already handled CI failure at SHA $HEAD_SHA (or PR exhausted) — skipping"
exit 0
fi

Expand All @@ -99,8 +141,23 @@ main() {
while [ "$cycle" -le "$MAX_CI_CYCLES" ]; do
echo " [fix-ci] cycle $cycle/$MAX_CI_CYCLES"

if ! run_writer_with_fallback "$prompt_file"; then
post_summary "failed" "Engine invocation failed after all retries."
local engine_rc=0
run_writer_with_fallback "$prompt_file" || engine_rc=$?

if [ "$engine_rc" -ne 0 ]; then
# Classify the failure: timeout (124) is a hard exhaustion candidate
local reason="Engine invocation failed (exit ${engine_rc})"
[ "$engine_rc" -eq 124 ] && reason="Engine timed out — PR may be too large for automated fixing"
post_summary "failed" "$reason"

# Check if we've hit the consecutive-failure threshold for this PR
local fail_count
fail_count=$(count_recent_failures)
echo " [fix-ci] consecutive failures on this PR: $fail_count (threshold: $MAX_FAIL_ATTEMPTS)"
if [ "$fail_count" -ge "$MAX_FAIL_ATTEMPTS" ]; then
echo "::warning::Exhaustion threshold reached — posting PR-level block to prevent further token spend"
post_exhaustion "$reason"
fi
Comment on lines +151 to +160

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

There is a potential race condition here. 'post_summary' calls the GitHub API to post a comment, and 'count_recent_failures' immediately calls the API to list comments. Due to eventual consistency in GitHub's APIs, the comment just posted might not be returned in the list immediately. It is safer to count the existing failures before posting the new one. Additionally, for LLM-based tasks, ensure the script detects rate limits by checking exit codes and scanning output streams, exiting with code 2 if a limit is hit to allow for engine fallback.

Suggested change
post_summary "failed" "$reason"
# Check if we've hit the consecutive-failure threshold for this PR
local fail_count
fail_count=$(count_recent_failures)
echo " [fix-ci] consecutive failures on this PR: $fail_count (threshold: $MAX_FAIL_ATTEMPTS)"
if [ "$fail_count" -ge "$MAX_FAIL_ATTEMPTS" ]; then
echo "::warning::Exhaustion threshold reached — posting PR-level block to prevent further token spend"
post_exhaustion "$reason"
fi
# Count existing failures before posting the new one to avoid API race conditions
local prev_fail_count
prev_fail_count=$(count_recent_failures)
local total_fail_count=$((prev_fail_count + 1))
post_summary "failed" "$reason"
echo " [fix-ci] consecutive failures on this PR: $total_fail_count (threshold: $MAX_FAIL_ATTEMPTS)"
if [ "$total_fail_count" -ge "$MAX_FAIL_ATTEMPTS" ]; then
echo "::warning::Exhaustion threshold reached — posting PR-level block to prevent further token spend"
post_exhaustion "$reason"
fi
References
  1. In shell scripts implementing retry loops for LLM-based tasks, detect rate limits immediately by checking exit codes and scanning both stdout and stderr for rate-limit indicators before attempting a retry. If a rate limit is detected, exit with a specific code (e.g., 2) to enable engine fallback mechanisms.

exit 1
fi

Expand Down
165 changes: 165 additions & 0 deletions tests/dev-lead/e2e/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
# Dev-Lead Agent — E2E Test Suite

End-to-end tests for the dev-lead agent. These tests exercise the full routing
and intent-classification pipeline, from GitHub event → intent → handler.

## Quick start

```bash
# Run all scenarios (fixture-based ones run locally; live ones require GH_TOKEN)
bash tests/dev-lead/e2e/run-all.sh

# Dry run: see what would execute without doing anything
bash tests/dev-lead/e2e/run-all.sh --dry-run

# Run a single scenario by name fragment
bash tests/dev-lead/e2e/run-all.sh --scenario 05-skip-anti-loop
```

## Environment variables

| Variable | Required | Description |
|---|---|---|
| `GH_TOKEN` or `GH_PAT` | For live scenarios (02–04) | GitHub PAT with `repo` scope |
| `CLAUDE_CODE_OAUTH_TOKEN` | Optional | Claude token; omit to skip response-content assertions |
| `E2E_TARGET_REPO` | No | Override target repo (default: `petry-projects/.github-private`) |
| `E2E_CLEANUP` | No | Set `false` to keep test PRs/branches/issues after run (default: `true`) |

## Scenarios

### 01 — skip-bot-pr (fixture-based, no network)

**What it tests:** A PR opened by `dependabot[bot]` is classified as `skip` with reason `bot-pr`.

**How it works:** Runs `dev-lead-intent.sh` locally against the fixture
`tests/dev-lead/fixtures/events/pr_opened_dependabot.json` and asserts
`INTENT_TYPE=skip` and `INTENT_REASON=bot-pr`.

**Requires:** Nothing (fully local).

---

### 02 — human-at-mention (live)

**What it tests:** A human comment `@dev-lead what does this PR change?` on a
PR triggers the `human` intent and the Dev-Lead Agent workflow runs.

**How it works:**
1. Creates a test branch and PR.
2. Posts an `@dev-lead` comment via `gh pr comment`.
3. Waits for the `Dev-Lead Agent` workflow to complete on the PR's head SHA.
4. Asserts the workflow conclusion is `success` (or `neutral` in dry-run mode).

**Requires:** `GH_TOKEN`. `CLAUDE_CODE_OAUTH_TOKEN` for agent response (otherwise
skips response-content assertion).

---

### 03 — ci-failure-relay (live)

**What it tests:** A failing check_run on a PR triggers the ci-relay job →
`repository_dispatch` event → fix-ci intent → `Run fix-ci` step executes.

**How it works:**
1. Pushes a shell script with a deliberate syntax error to a new branch.
2. Creates a PR — this triggers CI (shellcheck / bash -n will fail).
3. Waits for a `check_run` failure on the PR head SHA.
4. Waits for the `Dev-Lead Agent` workflow triggered by the `check_run` event
(ci-relay job) to complete.
5. Waits for the subsequent `Dev-Lead Agent` workflow triggered by
`repository_dispatch` (fix-ci dispatch) to complete.
6. Asserts both workflows conclude `success`.

**Requires:** `GH_TOKEN`. CI must run on the target repo.

---

### 04 — issue-labeled (live)

**What it tests:** Adding the `dev-lead` label to an issue triggers the `issue`
intent and the `Run issue` step executes.

**How it works:**
1. Creates a test issue.
2. Adds the `dev-lead` label via `gh issue edit`.
3. Waits for the `Dev-Lead Agent` workflow triggered by the `issues` event.
4. Asserts workflow conclusion is `success` or `neutral`.

**Requires:** `GH_TOKEN`. `dev-lead` label must exist on the repo (the test
attempts to create it if missing).

---

### 05 — skip-anti-loop (fixture-based, no network)

**What it tests:**
- Part A: A `pull_request synchronize` event from `donpetry-bot` (BOT_USER)
emits `skip` with reason `dev-lead-own-commit`.
- Part B: The same event from a human user correctly emits `human-pr` (not
skipped by the anti-loop guard).

**How it works:** Runs `dev-lead-intent.sh` twice:
1. Against `tests/dev-lead/fixtures/events/pr_sync_dev_lead_commit.json`
(sender = `donpetry-bot`).
2. Against an inline fixture with sender = `donpetry` (human).

**Requires:** Nothing (fully local). This is the fastest scenario.

---

### 06 — exhaustion-guard (script-based, stub network)

**What it tests:**
- Part A: After `MAX_FAIL_ATTEMPTS` consecutive engine failures, the fix-ci
script posts a PR-level exhaustion marker and the output contains
`status=exhausted`.
- Part B: A pre-existing PR-level exhaustion marker causes the script to exit 0
immediately (blocked, not failed).

**How it works:** Runs `dev-lead-fix-ci.sh` directly with a stub `gh` binary
that returns pre-seeded failure comments, and a stub `claude` binary that exits
with code 1 (simulating an engine failure).

**Requires:** Nothing (fully local).

---

## Results

Each run writes results to `tests/dev-lead/e2e/results/`:

- `results.txt` — one line per scenario: `<timestamp> [PASS|FAIL|SKIP] <scenario>: <details>`
- `summary-<timestamp>.txt` — human-readable summary of the complete run

## Directory layout

```
tests/dev-lead/e2e/
├── run-all.sh Master orchestrator
├── lib/
│ └── helpers.sh Shared helpers (create_test_branch, wait_for_workflow, etc.)
├── scenarios/
│ ├── 01-skip-bot-pr.sh
│ ├── 02-human-at-mention.sh
│ ├── 03-ci-failure-relay.sh
│ ├── 04-issue-labeled.sh
│ ├── 05-skip-anti-loop.sh
│ └── 06-exhaustion-guard.sh
├── results/ Created at runtime — gitignored
└── README.md
```

## Running in CI

The E2E suite is designed to be runnable by the dev-lead agent itself via a
`workflow_dispatch` or as a scheduled job. Fixture-based scenarios (01, 05, 06)
are safe to run in any environment without credentials. Live scenarios (02–04)
require a `GH_TOKEN` secret with `repo` scope.

```yaml
- name: Run E2E fixture scenarios
run: |
bash tests/dev-lead/e2e/run-all.sh --scenario 01-skip-bot-pr
bash tests/dev-lead/e2e/run-all.sh --scenario 05-skip-anti-loop
bash tests/dev-lead/e2e/run-all.sh --scenario 06-exhaustion-guard
```
Loading
Loading