agents: update workflow, Python, and CI skill guidelines (#4462) Workflow rules and CI skills lacked guidance for `download-tool` caching, `/.github/actions/*` Dependabot globs, HTTP gateway timeouts, and draft PR webhook behavior, causing noisy reviews and misclassified CI flakes. Document `download-tool`, Dependabot glob, `asyncio`, and docstring conventions in the workflow and Python rules. Teach CI failure analysis to accept ANSI escape codes from `gh api` and classify HTTP 502/503/504 gateway errors and Read the Docs `exit-code: 2` failures as flakes. Update CI monitoring and merge skills to cover timer polling, outage backoff, targeted job retries, and `gh pr ready` webhook sequencing.
diff --git a/.agents/rules/github_actions_workflows.md b/.agents/rules/github_actions_workflows.md index 02607e5..f009b19 100644 --- a/.agents/rules/github_actions_workflows.md +++ b/.agents/rules/github_actions_workflows.md
@@ -10,10 +10,9 @@ `.github/workflows/` and `.github/actions/` (check via `gh release view --repo <owner>/<repo>`), targeting supported Node.js runtimes (`node24`, not `node20`). -* **Dependabot Coverage for Composite Actions**: When adding a composite action - directory under `.github/actions/<name>`, add `"/.github/actions/<name>"` to - the `github-actions` `directories` list in `.github/dependabot.yml` (since - `directory: "/"` only scans `.github/workflows/`). +* **Dependabot Coverage**: Keep `directories: ["/", "/.github/actions/*"]` in + `.github/dependabot.yml` to cover workflows and composite actions; do not list + individual action directories or brace globs. * **Verifying Secret-Gated Workflow Steps**: GitHub Actions does not expose repository secrets (e.g. `secrets.BUILDBUDDY_CAS_ONLY_KEY`) to `pull_request` runs from forks (`origin`). To test secret-gated workflow or composite action @@ -25,6 +24,10 @@ like `bazel-contrib/setup-bazel` directly in `.github/workflows/`. Update `.github/actions/setup-bazel/action.yaml` if project defaults or inputs need adjustment. +* **Verified Tool Downloads (`./.github/actions/download-tool`)**: Use + `./.github/actions/download-tool` (or `dev/verified_download.py`) to fetch, + SHA-256-verify, and cache tool binaries; key caches by `<name>-<sha256>` + without OS or version prefixes. * Preserve `suppress-no-jobs-ran-error` fallback jobs in conditional workflows. * Pass inputs (e.g. `${{ inputs.issue }}`) directly to commands without redundant shell stripping. @@ -110,8 +113,9 @@ `needs.parse_step.result == 'success' && contains(needs.*.result, 'failure')` without adding redundant `outputs.command != ''` predicates. * **Single-Line `$GITHUB_OUTPUT` and `$GITHUB_ENV` Writes**: Reject `\n` and - `\r` in keys and values before writing `key=value` lines to `$GITHUB_OUTPUT` - or `$GITHUB_ENV`. In line-anchored regexes over comment or PR bodies, match + `\r` in untrusted keys and values before writing `key=value` lines to + `$GITHUB_OUTPUT` or `$GITHUB_ENV`; omit these guards on trusted workflow + constants. In line-anchored regexes over comment or PR bodies, match horizontal whitespace (`[ \t]`) and non-linebreak characters (`[^\r\n]`) rather than `\s` or `.`. * **Suppress Untrusted Stdout in Privileged Jobs**: Never echo raw PR bodies, @@ -133,9 +137,11 @@ * **Test Location**: Place workflow tests under `tests/workflows/`. * **Platform Constraints**: Set `target_compatible_with = ["@platforms//os:linux"]` on tests for Linux-only - workflow scripts instead of adding macOS/BSD workarounds. -* **Imports over `sys.path`**: Never alter `sys.path`. Define a `py_library` - with `imports = ["../../.github/workflows"]` in `tests/workflows/BUILD.bazel`. + workflow scripts instead of adding macOS/BSD workarounds. Scope CI-only + scripts to the job's runner OS and omit unused platform or cache fallbacks. +* **Imports over `sys.path`**: Never alter `sys.path`. Co-locate shared helper + modules in `dev/` or set `PYTHONPATH` in step `env:`, and define `py_library` + targets with `imports` in `tests/workflows/BUILD.bazel`. * **Functional Design & Fixtures**: Keep scripts functional with one public entry point (e.g. `process_comment()`) and private helpers (`_` prefix). Write outputs directly when matching actions. Test end-to-end via autouse fixtures
diff --git a/.agents/rules/python.md b/.agents/rules/python.md index 789d3c8..02ec22f 100644 --- a/.agents/rules/python.md +++ b/.agents/rules/python.md
@@ -27,6 +27,10 @@ displays defaults automatically. * Include a `Usage:` example and input file format description in CLI module docstrings. +* State what a module or function does in docstrings; omit execution adverbs + like `"Asynchronously"`. +* Split simple comma- or newline-delimited CLI inputs with `splitlines()` and + `.split(",")` instead of `csv`. ## API Minimalism & Module State * Pass dependencies explicitly; avoid implicit optionals that auto-load state or @@ -71,6 +75,7 @@ * Demarcate captured stdout/stderr in exceptions with 20 `=` characters: `==================== STDOUT BEGIN ====================` / `==================== STDOUT END ====================` (same for STDERR). +* Use `asyncio` for concurrent subprocesses and retried network I/O. ## Data Classes & TypedDict * In dataclasses (`@dataclasses.dataclass`), add a comment above each field
diff --git a/.agents/skills/analyze-ci-failure/scripts/analyze_ci_failure.py b/.agents/skills/analyze-ci-failure/scripts/analyze_ci_failure.py index a447394..7906ae6 100755 --- a/.agents/skills/analyze-ci-failure/scripts/analyze_ci_failure.py +++ b/.agents/skills/analyze-ci-failure/scripts/analyze_ci_failure.py
@@ -8,6 +8,46 @@ import urllib.parse import urllib.request +_ANSI_ESCAPE = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])") + +_HTTP_GATEWAY_ERRORS = ( + # keep-sorted start + "502 bad gateway", + "503 service unavailable", + "504 gateway time-out", + # keep-sorted end +) + +_ERROR_KEYWORDS = _HTTP_GATEWAY_ERRORS + ( + # keep-sorted start + "##[error]", + "connection reset by peer", + "critical path", + "curl: (", + "errno 5", + "error during connect:", + "error waiting for container", + "error:", + "error]", + "exception", + "exit code", + "exit-code", + "failed:", + "fatal", + "fatal:", + "filenotfounderror", + "input/output error", + "no such package", + "no such target", + "status 125", + "traceback", + "unable to find bazelisk version", + "user command error:", + "would be reformatted", + "would reformat:", + # keep-sorted end +) + def fetch_log(job_name, build_id, job_id, output_path): if ( @@ -55,7 +95,14 @@ gh_repo = "bazel-contrib/rules_python" gh_job_id = gh_match.group(1) print(f"📥 Fetching GitHub Action log for job {gh_job_id} using gh CLI...") - api_cmd = ["gh", "api", f"repos/{gh_repo}/actions/jobs/{gh_job_id}/logs"] + api_cmd = [ + "gh", + "api", + # Raw job logs contain ANSI color codes that gh api rejects by + # default; parse_log() strips them. + "--allow-escape-sequences", + f"repos/{gh_repo}/actions/jobs/{gh_job_id}/logs", + ] try: res = subprocess.run(api_cmd, capture_output=True, text=True, check=True) if res.stdout.strip(): @@ -107,7 +154,12 @@ return False -ANSI_ESCAPE = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])") +def _contains_any(lines, substrings): + for line in lines: + lower_line = line.lower() + if any(sub in lower_line for sub in substrings): + return True + return False def parse_log(log_path): @@ -119,41 +171,12 @@ errors = [] for line in lines: - clean_line = ANSI_ESCAPE.sub("", line).strip() + clean_line = _ANSI_ESCAPE.sub("", line).strip() # Clean buildkite timestamp prefix: _bk;t=... clean_line = re.sub(r"^_bk;t=\d+\s*", "", clean_line) - if any( - keyword.lower() in clean_line.lower() - for keyword in [ - "error:", - "failed:", - "critical path", - "traceback", - "exception", - "filenotfounderror", - "no such package", - "no such target", - "exit code", - "exit-code", - "status 125", - "fatal:", - "fatal", - "##[error]", - "would reformat:", - "would be reformatted", - "error]", - "error waiting for container", - "error during connect:", - "user command error:", - "curl: (", - "connection reset by peer", - "input/output error", - "errno 5", - "unable to find bazelisk version", - ] - ): - if clean_line: - errors.append(clean_line) + lower_line = clean_line.lower() + if clean_line and any(kw in lower_line for kw in _ERROR_KEYWORDS): + errors.append(clean_line) return errors[:30] @@ -165,6 +188,10 @@ else "No obvious keyword error lines matched. Please inspect the raw log file." ) + lower_job_name = job_name.lower() + is_docs_job = "docs" in lower_job_name or "readthedocs" in lower_job_name + has_exit_code_2 = _contains_any(errors, ("exit code 2", "exit-code: 2")) + is_flake = False flake_reason = "" if any( @@ -173,9 +200,7 @@ ): is_flake = True flake_reason = "ReadTheDocs workspace checkout race / dirty container environment where target directory is not empty (`fatal: destination path '.' already exists`). This is an infrastructure flake, not a codebase failure." - elif any("exit code 2" in e.lower() for e in errors) and ( - "docs" in job_name.lower() or "readthedocs" in job_name.lower() - ): + elif is_docs_job and has_exit_code_2: is_flake = True flake_reason = "Known docs build flake with exit code 2." elif any( @@ -187,6 +212,12 @@ ): is_flake = True flake_reason = "Buildkite agent / Docker runner infrastructure failure (dockerd disconnection / grpc context canceled / exit status 125). This is an infrastructure flake, not a codebase bug." + elif _contains_any(errors, _HTTP_GATEWAY_ERRORS): + is_flake = True + flake_reason = ( + "Upstream HTTP 502/503/504 gateway timeout during download " + "(infrastructure flake)." + ) elif any( "curl:" in e.lower() or "recv failure: connection reset" in e.lower()
diff --git a/.agents/skills/merge-pr/SKILL.md b/.agents/skills/merge-pr/SKILL.md index 7c18194..231d790 100644 --- a/.agents/skills/merge-pr/SKILL.md +++ b/.agents/skills/merge-pr/SKILL.md
@@ -12,7 +12,10 @@ - **Draft PRs**: If the PR is a draft (`isDraft: true`), mark it ready for review first via `gh pr ready <pr_number> --repo bazel-contrib/rules_python` (otherwise `gh pr merge` fails with - `Pull Request is still a draft`). + `Pull Request is still a draft`). Run `gh pr ready` before pushing final + commits so external `pull_request` webhooks trigger; if external checks do + not register on head, run `gh pr close <pr_number> && gh pr reopen + <pr_number>`. - **Force / Admin Merge**: **CRITICAL**: Only pass `--admin` (e.g., `gh pr merge <pr_number> --repo bazel-contrib/rules_python --admin --squash`) with explicit user consent ("force merge", "admin merge", or
diff --git a/.agents/skills/monitor-ci-results/SKILL.md b/.agents/skills/monitor-ci-results/SKILL.md index f0b6922..633ae86 100644 --- a/.agents/skills/monitor-ci-results/SKILL.md +++ b/.agents/skills/monitor-ci-results/SKILL.md
@@ -31,7 +31,9 @@ ### ✨ Autonomous Subagent Orchestration 1. **Background Polling**: `monitor_remote_ci.py` continuously polls both GitHub PR checks and Buildkite workflow executions in the background across - new commits and CI re-runs. + new commits and CI re-runs. Because it sends messages only on failures or + blocked steps, set periodic `schedule` timers (`TimerCondition: "any"`) to + detect passing completion. 2. **Blocked Jobs (`.bazelci/presubmit.yml` Approval)**: When a Buildkite build is blocked on a manual approval step (e.g., when `.bazelci/presubmit.yml` is modified), `monitor_remote_ci.py` automatically approves and unblocks it via @@ -58,13 +60,19 @@ `analyze-ci-failure` skill on the reported failure (instructing the subagent to pass its own subagent conversation ID to `analyze_ci_failure.py` so only its completion message wakes the parent). + When many Buildkite jobs fail together, inspect 1–3 representative jobs + instead of spawning an analyzer per job. 5. **Autonomous Flake Retry**: If the CI Failure Analyzer subagent confirms that the failure is a transient infrastructure or network flake (e.g. disk I/O error, 504 gateway, sandbox initialization failure, or `setup-bazel` failing with `Unable to find Bazelisk version`), **immediately and autonomously** launch a separate subagent using `invoke_subagent` with the role "CI Job Retrier" and the prompt template in - `.agents/skills/monitor-ci-results/retry-job-prompt.md` to run the - `retry-ci-job` skill for `<pr_number>`. Do not execute the script - directly; use the subagent and skill orchestration instead. Continue - monitoring without pausing to ask the user. + `.agents/skills/monitor-ci-results/retry-job-prompt.md`, passing + `--buildkite-build bazel/rules-python-python/<n> --rebuild` for mass + Buildkite failures, `--buildkite-job-id <uuid>` for isolated Buildkite + failures, `--github-job-id <id>` for isolated GitHub Actions failures, or + `--pr <pr_number>` as `{retry_target}`. Do not execute the script directly; + use the subagent and skill orchestration instead. During sustained `504` + outages, verify HTTP `GET` liveness with `schedule` timer backoff before + retrying. Continue monitoring without pausing to ask the user.
diff --git a/.agents/skills/monitor-ci-results/retry-job-prompt.md b/.agents/skills/monitor-ci-results/retry-job-prompt.md index f7c3382..fc161c8 100644 --- a/.agents/skills/monitor-ci-results/retry-job-prompt.md +++ b/.agents/skills/monitor-ci-results/retry-job-prompt.md
@@ -1,5 +1,5 @@ Use the `retry-ci-job` skill to retry the failed Buildkite or GitHub Actions -job(s) for PR {pr_number}. +job(s) ({retry_target}). Follow the instructions in `.agents/skills/retry-ci-job/SKILL.md` to retry the failing jobs using the skill. Once the retry request completes, report back to the parent conversation.
diff --git a/.agents/skills/review-code/review-workflows-prompt.md b/.agents/skills/review-code/review-workflows-prompt.md index 72b0ce2..dd37d70 100644 --- a/.agents/skills/review-code/review-workflows-prompt.md +++ b/.agents/skills/review-code/review-workflows-prompt.md
@@ -11,8 +11,9 @@ 3. Verify action and tool versions and Dependabot coverage: ensure `.github/workflows/**` and `.github/actions/**` use the latest release of referenced actions and downloaded tools (check via - `gh release view --repo <owner>/<repo>`), and verify every - `.github/actions/<name>` directory is in `.github/dependabot.yml`. + `gh release view --repo <owner>/<repo>`), and verify + `.github/dependabot.yml` keeps `directories: ["/", "/.github/actions/*"]` + without listing individual action directories or brace globs. 4. Verify fallback jobs: ensure `suppress-no-jobs-ran-error` / `noop` fallback jobs exist in conditional workflows. 5. Check event payload handling: for steps that are a single Python script, @@ -20,8 +21,9 @@ suggestion otherwise), rather than passing payload fields via CLI args or environment variables. 6. Verify workflow tests: check that tests are located in `tests/workflows/`, - use `imports = ["../../.github/workflows"]` in `BUILD.bazel`, and do not - modify `sys.path`. + co-locate shared helpers in `dev/` or set `PYTHONPATH` in step `env:`, + define `py_library` targets with `imports` in `tests/workflows/BUILD.bazel`, + and do not modify `sys.path`. 7. Verify console messaging: ensure scripts use GHA workflow command syntax (e.g. `::error::`, `::warning::`, `::notice::`, `::group::`) and include identifying context (such as issue/PR numbers) in error messages. @@ -37,12 +39,11 @@ invocations via heredocs (`<< 'EOF'`) or `-c` flags (`bash -c`, `python3 -c`). 10. Verify local composite actions: ensure workflows use local composite action - wrappers (specifically `uses: ./.github/actions/setup-bazel`) rather than - invoking `bazel-contrib/setup-bazel` directly. If a workflow calls - `bazel-contrib/setup-bazel` directly, replace it with - `uses: ./.github/actions/setup-bazel` (updating - `.github/actions/setup-bazel/action.yaml` if new defaults or inputs are - needed). + wrappers (`uses: ./.github/actions/setup-bazel` rather than invoking + `bazel-contrib/setup-bazel` directly, and + `uses: ./.github/actions/download-tool` or `dev/verified_download.py` to + fetch, SHA-256-verify, and cache tool binaries keyed by `<name>-<sha256>` + without OS or version prefixes). 11. Audit for injection and security vulnerabilities: - Require explicit top-level `permissions:` (`{}` or minimal scopes) in all workflows, and explicit job-level permissions for reusable workflow calls @@ -62,8 +63,9 @@ - Verify comment and event handlers check actor authorization before writing to `$GITHUB_OUTPUT` or `$GITHUB_ENV`, reacting to comments, or triggering failure-reporting jobs. - - Verify helpers writing `key=value` to `$GITHUB_OUTPUT` or `$GITHUB_ENV` - reject `\n` and `\r`, and ensure untrusted PR/comment text is never - echoed to stdout where `::...::` workflow commands could execute. + - Verify writes of untrusted `key=value` entries to `$GITHUB_OUTPUT` or + `$GITHUB_ENV` reject `\n` and `\r` (omitting guards on trusted workflow + constants), and ensure untrusted PR/comment text is never echoed to + stdout where `::...::` workflow commands could execute. @.agents/skills/review-code/review-report-format.md