Repository navigation
estc-pr-buildkite-detective: pre-fetch logs via setup-commands - #764
Conversation
- Replace remote mcp.buildkite.com HTTP MCP with buildkite/mcp-server container - Allow api.buildkite.com for REST API egress; drop mcp.buildkite.com - Update estc-pr-buildkite-detective README Made-with: Cursor
- Expose BUILDKITE_API_TOKEN in the setup-commands env so callers can pre-fetch Buildkite failure data before the agent runs - Add Step 2 shortcut: if /tmp/gh-aw/buildkite-failures.txt exists (summary) with per-job logs under /tmp/gh-aw/buildkite-logs/, skip MCP and public-page steps entirely - Improve MCP guidance from real-world dry-run findings: - Filter builds by commit SHA not branch (fork PRs use fork:branch format) - Distinguish failed (ran, exit != 0) from broken (never ran, no logs) - Use tail_logs first; list_annotations is infra metrics, not failures Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gs only The workflow now relies entirely on data pre-fetched by setup-commands into /tmp/gh-aw/buildkite-failures.txt and /tmp/gh-aw/buildkite-logs/. The agent never needs direct Buildkite API access. - Remove mcp-servers.buildkite and api.buildkite.com network allowlist - Remove buildkite-org and buildkite-pipeline inputs (no longer needed) - Make BUILDKITE_API_TOKEN required (used by setup-commands to fetch data) - Remove mcp-pagination import - Simplify Step 2 to just read the pre-fetched files - Remove all MCP/public-page fallback instructions from prompt Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughRemoved ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/gh-aw-estc-pr-buildkite-detective.md (1)
148-149:⚠️ Potential issue | 🟠 MajorBranch-based PR filtering can drop valid fork PRs
The branch match gate can discard correct PRs for forked branches (
owner:branchformat mismatch). Since you already resolve bycommit_sha, keep commit association as the primary filter and avoid hard branch equality here.Suggested adjustment
- - Otherwise, use `bash` + `gh api repos/${{ github.repository }}/commits/{commit_sha}/pulls` to find PRs containing the commit SHA. Filter the results to keep only PRs whose `state` is `"open"` and, when `branches` is non-empty, whose `head.ref` matches one of the listed branches. If no candidates remain, also try searching open PRs whose head branch matches one of the branches. + - Otherwise, use `bash` + `gh api repos/${{ github.repository }}/commits/{commit_sha}/pulls` to find PRs containing the commit SHA. Filter the results to keep only PRs whose `state` is `"open"`. Do not require branch-name equality for fork PRs; use branch matching only as an optional tiebreaker when multiple open PRs remain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/gh-aw-estc-pr-buildkite-detective.md around lines 148 - 149, When filtering PRs returned by gh api repos/${{ github.repository }}/commits/{commit_sha}/pulls, do not drop PRs solely because head.ref does not exactly equal an entry in branches (this breaks fork PRs with owner:branch formats); instead keep all PRs associated with the commit_sha as primary candidates and only apply relaxed branch matching: consider a branch match if head.ref === branch OR head.ref endsWith(`/${branch}`) OR head.ref.split(':').pop() === branch. Update the filter logic around checking state === "open" and branches to use this relaxed comparison, and only if no candidates remain then run the fallback search of open PRs by head branch using the same relaxed match before calling noop with "No pull request associated with failed commit status; nothing to do".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/gh-aw-estc-pr-buildkite-detective.md:
- Around line 46-47: The BUILDKITE_API_TOKEN requirement is currently
unconditional; change the workflow so BUILDKITE_API_TOKEN is required only when
the setup-prefetch path is used (i.e., when the workflow input like
inputs.setup-commands or a boolean input enabling setup prefetch is true).
Update the job/input validation logic that declares BUILDKITE_API_TOKEN to make
it conditional based on inputs.setup-commands (or the corresponding
setup-prefetch input) so callers that don't use setup commands are not forced to
provide the token; ensure the requirement is evaluated at workflow
dispatch/usage time and keep the environment variable name BUILDKITE_API_TOKEN
unchanged.
---
Outside diff comments:
In @.github/workflows/gh-aw-estc-pr-buildkite-detective.md:
- Around line 148-149: When filtering PRs returned by gh api repos/${{
github.repository }}/commits/{commit_sha}/pulls, do not drop PRs solely because
head.ref does not exactly equal an entry in branches (this breaks fork PRs with
owner:branch formats); instead keep all PRs associated with the commit_sha as
primary candidates and only apply relaxed branch matching: consider a branch
match if head.ref === branch OR head.ref endsWith(`/${branch}`) OR
head.ref.split(':').pop() === branch. Update the filter logic around checking
state === "open" and branches to use this relaxed comparison, and only if no
candidates remain then run the fallback search of open PRs by head branch using
the same relaxed match before calling noop with "No pull request associated with
failed commit status; nothing to do".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5681a3ca-e1aa-40f3-a600-d7d7629d3377
📒 Files selected for processing (1)
.github/workflows/gh-aw-estc-pr-buildkite-detective.md
| BUILDKITE_API_TOKEN: | ||
| required: false | ||
| required: true |
There was a problem hiding this comment.
Scope BUILDKITE_API_TOKEN requirement to setup-command usage
Making BUILDKITE_API_TOKEN globally required breaks callers that don’t use inputs.setup-commands. This should be conditionally enforced only when setup prefetch is enabled.
Suggested adjustment
secrets:
COPILOT_GITHUB_TOKEN:
required: true
BUILDKITE_API_TOKEN:
- required: true
+ required: false - name: Repo-specific setup
if: ${{ inputs.setup-commands != '' }}
env:
SETUP_COMMANDS: ${{ inputs.setup-commands }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BUILDKITE_API_TOKEN: ${{ secrets.BUILDKITE_API_TOKEN }}
- run: eval "$SETUP_COMMANDS"
+ run: |
+ set -euo pipefail
+ if [ -z "${BUILDKITE_API_TOKEN:-}" ]; then
+ echo "BUILDKITE_API_TOKEN is required when setup-commands is provided" >&2
+ exit 1
+ fi
+ eval "$SETUP_COMMANDS"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/gh-aw-estc-pr-buildkite-detective.md around lines 46 - 47,
The BUILDKITE_API_TOKEN requirement is currently unconditional; change the
workflow so BUILDKITE_API_TOKEN is required only when the setup-prefetch path is
used (i.e., when the workflow input like inputs.setup-commands or a boolean
input enabling setup prefetch is true). Update the job/input validation logic
that declares BUILDKITE_API_TOKEN to make it conditional based on
inputs.setup-commands (or the corresponding setup-prefetch input) so callers
that don't use setup commands are not forced to provide the token; ensure the
requirement is evaluated at workflow dispatch/usage time and keep the
environment variable name BUILDKITE_API_TOKEN unchanged.
github-actions
Bot
added
the
medium_boom
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ## How it works | ||
|
|
||
| Triggered automatically when a commit status or check run reports a failure. Looks up the related Buildkite build via MCP, analyzes failed jobs/logs/annotations, and posts a comment with root cause and recommended fixes. Read-only — never pushes changes. | ||
| Triggered automatically when a commit status or check run reports a failure. Looks up the related Buildkite build via the [Buildkite MCP server](https://buildkite.com/docs/apis/mcp-server/local/installing) (`buildkite/mcp-server`, stdio), analyzes failed jobs/logs/annotations, and posts a comment with root cause and recommended fixes. Read-only — never pushes changes. |
There was a problem hiding this comment.
The README now conflicts with the workflow contract introduced in this PR.
This line says the detective queries Buildkite via MCP, but the workflow now expects pre-fetched local artifacts (/tmp/gh-aw/buildkite-failures.txt and /tmp/gh-aw/buildkite-logs/*) and noops when they are missing.
Please update this section to describe the setup-commands prefetch flow. Also align the Inputs/Secrets sections with the new contract (removed buildkite-org/buildkite-pipeline, and BUILDKITE_API_TOKEN now required).
Move the pre-fetch script into the workflow itself as a dedicated step that runs between event context resolution and the agent. BUILDKITE_API_TOKEN is used here directly — callers no longer need setup-commands for this. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| pass | ||
|
|
||
| if not pr_number: | ||
| print('No PR number found in event context; skipping Buildkite data fetch') |
There was a problem hiding this comment.
The new prefetch step exits early when pr_numbers is empty, but Resolve event context always writes pr_numbers: empty for status events (line 86). That means status-triggered failures skip Buildkite prefetch entirely, /tmp/gh-aw/buildkite-failures.txt is never produced, and Step 2 later noops.
Please add a fallback here to resolve PRs from commit_sha (e.g., gh api repos/\$\{REPO}/commits/{sha}/pulls and filter open PRs), so status events still fetch logs.
- Remove buildkite-org and buildkite-pipeline from example.yml (inputs no longer exist) - Update README: remove MCP server mention, remove stale inputs, make BUILDKITE_API_TOKEN required - Recompile to regenerate trigger-estc-pr-buildkite-detective.yml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| for job in failed: | ||
| total_failed += 1 | ||
| slug = slugify(job.get('name', f'job-{job["id"]}')) | ||
| log_file = f'/tmp/gh-aw/buildkite-logs/{pipeline}-{slug}.txt' |
There was a problem hiding this comment.
Log filenames can collide across builds and overwrite earlier failure evidence.
log_file is currently keyed by only pipeline + slugified job name, so two failed jobs with the same name (common with retries/matrix jobs or multiple failed builds in one run) write to the same path. That can make the summary point multiple entries at one overwritten file.
Consider including a unique identifier like build number and/or job id in the filename, for example:
| log_file = f'/tmp/gh-aw/buildkite-logs/{pipeline}-{slug}.txt' | |
| log_file = f'/tmp/gh-aw/buildkite-logs/{pipeline}-{number}-{job["id"]}-{slug}.txt' |
Summary
This refactor removes runtime Buildkite MCP usage from PR Buildkite Detective and switches to local-file analysis.
Buildkite failure data is now pre-fetched by the workflow itself (before the agent runs), and the agent reads only local files under
/tmp/gh-aw/.Workflow changes
mcp-servers.buildkiteconfiguration.mcp.buildkite.comfrom the workflow network allowlist.buildkite-organdbuildkite-pipelineinputs.BUILDKITE_API_TOKENrequired.gh pr checks/tmp/gh-aw/buildkite-failures.txt/tmp/gh-aw/buildkite-logs/setup-commandsas an optional repo-specific step after the pre-fetch step.mcp-paginationfragment import./tmp/gh-aw/buildkite-failures.txtand/tmp/gh-aw/buildkite-logs/.noopbehavior when/tmp/gh-aw/buildkite-failures.txtis absent.See
elastic/beats#49589for a related usage context.Test plan
make compile)BUILDKITE_API_TOKENis available to the built-in fetch step/tmp/gh-aw/buildkite-failures.txtand/tmp/gh-aw/buildkite-logs/*noopis called when/tmp/gh-aw/buildkite-failures.txtis absentThe body of this PR is automatically managed by the Trigger Update PR Body workflow.