Fix CI fix agent's blind context pre-fetch - #1263
Open
ytdb-ci-bot[bot] wants to merge 6 commits into
Open
Conversation
added 6 commits
August 4, 2026 16:38
The fix agent has been starting every run with no failure data at all. Its
job runs on a self-hosted Hetzner image that does not ship the GitHub CLI,
and only jq was installed explicitly, so all six `gh` calls in "Gather CI
failure context" exited 127. Each call redirected stderr to /dev/null and
fell back to a placeholder on failure, so the agent received `run.json` as
`{}`, `jobs.json` with an empty job list, `logs.txt` saying no logs were
available, and four empty arrays -- with nothing in the run log explaining
why. The agent then spent its whole time budget investigating a failure it
could not see.
The pre-fetch step was introduced in #1247, which moved the GitHub reads out
of the agent step (where the agent held a token and could install whatever it
needed on the fly) into a fixed shell step that assumes `gh` is present. The
same commit put `gh pr create` in "Publish fix", so a fresh fix branch would
be pushed and then fail to become a pull request, leaving an orphan branch.
Install `gh` alongside jq and verify both before proceeding. Keep every fetch
non-fatal, but surface the captured stderr as a warning annotation instead of
discarding it, and abort before the agent starts when the run metadata -- the
one datum it cannot work without -- is unavailable. A context digest and a
warning on zero failed jobs make a placeholder context distinguishable from a
genuinely sparse one. The publish step now checks for `gh` before the push
rather than after, so it cannot leave a branch with no pull request. The
agent prompt says what a placeholder file means and where to look instead.
Review of the first commit found the prompt guidance actively harmful. It told the agent to read `[]` as "this datum could not be fetched", but `[]` is also the correct value on a healthy run: no associated PR on a develop push, no annotations on a compile failure, no open fix PR in the repo. That contradicted the file inventory ten lines above, which reads an empty `associated-prs.json` as a direct develop push, so the agent would discard real data. The fallback it offered was worse: `git branch -r --list 'origin/ci-fix/*'` carries no PR number, title, or open/closed state, so the agent could adopt a long-abandoned branch, record `existing_pr` as null, and leave the publish step pushing to a branch whose `gh pr create` then fails. Make the outcome a recorded fact rather than an inference. Each fetch writes its result to `fetch-status.json` in the context dir, keyed by datum, so `ok` means the value is real even when empty and anything else means treat the file as absent. The agent can read a file; it has no token to read this job's log. The prompt now keys off that file, bounds the degraded path to one local suite run, and routes a non-reproducible failure to a new "Insufficient failure context" category that terminates in a `no-fix` sentinel. Also from review, each verified against a stubbed-gh harness covering total failure, partial degradation, a pasted job URL, and hostile stderr: - Extract the run id from the segment after `/runs/`. Trailing-segment extraction turned a pasted `.../runs/<id>/job/<job-id>` URL into the job id, which is numeric and so passed validation before fetching the wrong run. - Report the failing command's real exit code. Reading `$?` after a failed `if` condition yielded the `if` statement's own status, so a missing `gh` was reported as "exit 0". - Fold newlines, drop CR/TAB, and escape `%` in captured stderr so it cannot forge a workflow command or corrupt the status file. - Retry the run metadata three times before aborting, so one transient 5xx cannot kill a job holding a 6-12 hour agent budget. - Clean up the stderr temp file via a trap, which the new abort path skips. - Let an apt failure fall through to the verify loop that names the tool. - Publish now consumes the validated `branch` / `existing_pr` step outputs instead of re-reading the sentinel, which bypassed that validation; it checks `gh` and the App token before the push rather than after. - Clear the fixed-path agent summary at job start, so an abort before the agent runs cannot post a previous job's summary to Zulip.
Review iteration 2 found the status file lying in the one case it was built for. `annotations` and `pr-comments` are produced by loops that enumerate work from an upstream file, and both initialized their status to `ok`. When the `jobs` fetch failed, `jobs.json` became the empty-list placeholder, the annotations loop ran zero times, and the datum was recorded `ok` -- so on a job-wide gh outage the agent was told the run genuinely produced no annotations. It also made the new "Insufficient failure context" category unreachable: a non-ok `annotations` implied `jobs` was ok, so the conjunction could never hold. Seed each derived status from its parent instead, and add a third `partial:` state for a loop that fetched some entries but not all, so the agent uses what is there without reading a missing entry as absence. The Step 1 category now triggers on the data -- no failing test, gate, or compilation error can be named -- rather than on a fetch-status conjunction, and Step 0a scopes the reproduce run to the three failure-detail datums so an unavailable PR-comment list cannot cost a 90-minute integration suite and a spurious no-fix. Also fixed, each verified against the stubbed-gh harness: - One failing per-job annotation call discarded every other job's annotations. gh api writes the error body to stdout as well, so appending unconditionally mixed an object into the array stream; `jq -s add` then exited 5 and the fallback replaced the file with `[]`. Append only on success. - The stale-summary cleanup sat in "Configure Git", after the prerequisite verify that can abort the job. It is now the first step, so nothing can skip it, and uses `rm -rf ... || true` so a directory at that path cannot abort. - Installing gh removed an accidental second layer of the tokenless bracket around the agent step: an absent GH_TOKEN alone does not stop gh, which falls through to hosts.yml and the keyring. Point GH_CONFIG_DIR at an empty dir. - Publish pinned CI_FIX_RESULT_DIR/FILE in its own env. The agent can append to $GITHUB_ENV, and a redirected result dir would make the body-file containment check match any path, publishing the App token into the PR body. - `apt-get update` was unguarded while `install` was, so a mirror blip skipped the verify loop that names the missing tool. - The malformed-URL error echoed the raw dispatch input, handing a smuggled newline the log line the adjacent comment claimed to prevent. - Percent-escaping belongs on the annotation path only; it was corrupting the agent-facing status text. Run-id extraction uses `##*/runs/`. GH_DEBUG moved to job level so it also covers the token-bearing publish step. The trap now covers the TSV, and the status-file jq is guarded like its siblings.
Review iteration 3 found the Step 0a rule drawing a categorical conclusion from a disjunction: "if logs, annotations, or jobs is not marked ok, the failure detail is missing." The three come from independent fetches, so one being unavailable says nothing about the others. In the very scenario the previous commit was verified against -- a failed jobs fetch, which also makes annotations unattainable -- logs.txt is still complete and names the failing test, and the agent was told to ignore it and reproduce locally instead. The rule now says to use whichever datums are usable and to fall back only when none of the three identifies a failing test, gate, or compilation error, which is also how the Step 1 category already states the same condition. Retire the stale-summary problem rather than working around it. The agent now writes its summary to `$CI_FIX_RESULT_DIR/summary.md` instead of a fixed /tmp path, so the per-job temp dir the runner wipes gives the freshness guarantee for free, and the cleanup step added in the previous commit is gone. Its `rm -rf ... || true` was papering over the problem anyway: the comment justified `|| true` with a case that applies to `rm -f`, and its real effect was to hide a permission failure and post the stale summary regardless. Remaining review items, all verified against the stubbed-gh harness: - Both per-job loops fetch without `--paginate`, so they returned only the first 30 entries while the datum was stamped `ok`. The annotation naming the failing test, or the gate comment carrying the coverage numbers, can sit on page 2. - A datum where every per-job call failed reported `partial:` with an empty file. It now downgrades to `failed:`, since nothing collected is real. - "Read fix result" still read an inherited `CI_FIX_RESULT_FILE` while "Publish fix" reads a pinned one, so the agent could point the gating step and the publishing step at different sentinels. Both are pinned now. - Run-id extraction strips the query and fragment before matching `/runs/`; a URL like `...?redirect=/runs/999` otherwise won the match. - `pr-comments` had no status test in Step 1a, so an unavailable comment list read as "the gate posted no comment".
The two loop datums recorded their status before the jq merge that produces the file the agent reads. A merge failure empties that file, so a response that arrives with HTTP 200 but malformed JSON left `annotations` marked `ok` over an empty array -- the same false confidence the status file exists to remove. Merge first, downgrade the status when the merge fails, then record. Also reflows the run-id extraction comment into the order the three steps actually execute, which the incremental edits had scrambled.
Test Count Gate Results✅ No baseline available yet — gate skipped (first run). |
Coverage Gate ResultsThresholds: 85% line, 70% branch ✅ No changed Java files — coverage gate skipped. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
The autonomous CI fix agent has been starting every run with no failure data at all, and nothing in its logs said so. Its job runs on a self-hosted Hetzner image that does not ship the GitHub CLI, and the workflow installs only
jq, so all sixghcalls in the "Gather CI failure context" step exited 127. Each call was written asgh ... 2>/dev/null || echo '<fallback>' > file, so the agent receivedrun.jsonas{},jobs.jsonwith an empty job list,logs.txtsaying no logs were available, and four empty arrays — indistinguishable, from inside the agent, from a run that genuinely had nothing to report.This was the state on run 30921482302, where the pre-fetch produced exactly those seven placeholder files. The agent had a 6-hour budget and no visible failure to investigate.
The same commit that introduced the pre-fetch (#1247, retiring the CI personal access tokens) also moved PR creation into a
gh pr createin the "Publish fix" step. Before that, allghusage sat inside the agent step, where the agent held a token and could install whatever it needed on the fly — which is why the missing CLI went unnoticed for so long. Once the calls moved into fixed shell steps, nobody was left to self-heal them, and the publish path would push a fix branch and then fail to open a pull request, leaving an orphan branch behind.Planned changes:
Current state.
ghis absent on the runner and every fetch failure is silently swallowed. The agent cannot tell a missing datum from an empty one, because[]is both the fallback and the correct value for a develop-push failure, a job with no annotations, and a repo with no open fix PR.What changes.
ghis installed and verified alongsidejq, so both consumers work. Fetch failures stay non-fatal but are no longer silent: each is surfaced as a warning annotation carrying the command's real exit code and stderr, and each datum's outcome is recorded in a newfetch-status.jsonin the context directory. Missing run metadata — the one datum the agent cannot work without — aborts the job before the agent starts rather than handing it an empty object. The publish path checksghand the App token before the push instead of after.How. The status file is the design centre. Placeholders are ambiguous by construction, so the outcome is recorded as a fact rather than left to be inferred from file contents:
okmeans the value is real even when empty,partial:means the entries present are real but incomplete, and anything else means treat the file as absent. Derived datums seed their status from their parent, so a datum whose loop never ran is reported as not attempted rather than asok. The agent prompt keys its degraded path off that file, bounds the fallback to one local reproduce run, and terminates in a new "Insufficient failure context" classification.Key decisions. Installing the distro
ghpackage was chosen over rewriting the six calls ascurlagainst the REST API: the calls are the correct ones and well commented, and rewriting them would mean re-deriving pagination, redirect handling for logs, and the camelCase field names the prompt documents — new bugs for no gain, when the actual defect is a missing prerequisite. Ubuntu 24.04 shipsgh2.45.0, which supports every flag used here, and the file already installsjqthe same way. The stale-summary problem was resolved by moving the agent summary into the per-job temp directory rather than by clearing a fixed/tmppath, so the runner's own per-job wipe provides the guarantee.Out of scope. Two pre-existing exposures surfaced during review and are recorded rather than fixed: the remaining PID-named
/tmpfiles in this workflow are pre-plantable as symlinks, and the agent can write$GITHUB_PATHto influence binaries that later steps invoke. Closing the second properly means moving "Publish fix" into its own job so it does not share a process environment with the agent — a larger change than this fix should carry. TheCI_FIX_RESULT_DIRhalf of that exposure is closed here by pinning the path in the consuming steps'env:.Risks & accepted trade-offs. Making missing run metadata fatal converts a silent, expensive failure into a loud, immediate one; three retries with backoff keep a transient 5xx from tripping it.
ghis installed unpinned from the distro archive, which fails safe: a version too old forrun view --jsonmakes every fetch fail, and the new gate then aborts before the agent starts.Verification approach. The premise was confirmed on the runner image itself, and the new shell was exercised against a stubbed
ghacross eight scenarios rather than reasoned about.Tracks:
N/A (single-track)
Root Cause
.github/workflows/ci-failure-fix-agent.ymlruns entirely on[self-hosted, type-cpx42, image-x86-system-ubuntu-24.04]. That image hasjqonly because the workflow's "Install jq" step put it there;ghwas never installed and is not in the image. Verified directly on a runner of that type:command -v ghfails, whileapt-cache policy ghshows2.45.0-1ubuntu0.3available butInstalled: (none).Every
ghinvocation therefore exited 127 withcommand not found. In the gather step each call redirected stderr to/dev/nulland fell back to a placeholder on failure, so the exit code never reached the log and the agent silently received a complete set of empty files. In the publish stepgh pr createwould fail the same way, but only aftergit pushhad already succeeded.Changes
All in
.github/workflows/ci-failure-fix-agent.yml.apt-getcalls are|| trueso an apt failure reaches that error instead of aborting with only apt's output.fetchhelper captures stderr, emits a::warning::with the real exit code, writes the fallback, and records the outcome. It always returns success, because the step runs underbash -eand a bare non-zero return would abort the pre-fetch and discard everything already gathered.fetch-status.json. Per-datumok/partial:/failed:/not attempted:, built from a TSV accumulator so a successful retry overrides its own earlier failure. Derived datums (annotations,pr-comments) seed from their parent, and both are recorded after their merge so a merge failure cannot leaveokover an empty file./runs/with query and fragment stripped first, then validated as all-digits; a pasted job URL previously yielded the numeric job id and fetched the wrong run. Captured stderr is stripped of CR/TAB, folded to one line, and%-escaped on the annotation path only.gh apiwrites its error body to stdout as well, so appending unconditionally mixed an object into the array stream;jq -s addthen exited 5 and the fallback replaced the file with[], discarding every successful job's annotations. Both loops now append only on success and pass--paginate, since the annotation naming the failing test can sit past the 30-entry first page.ghand the App token are checked before the push;branchandexisting_prcome from the already-validated step outputs rather than a second raw read of the sentinel;CI_FIX_RESULT_DIR/CI_FIX_RESULT_FILEare pinned in the consuming steps'env:, and both sentinel readers now resolve the same file.ghremoved an accidental second layer of it — an absentGH_TOKENalone does not stopgh, which falls through tohosts.ymland the keyring — so the agent step pointsGH_CONFIG_DIRat an empty directory.GH_DEBUGis pinned empty at job level, covering the token-bearing publish step too.no-fixsentinel. The completeness claims that promised the context is always whole now point at the status file.Test plan
command -v ghfails there, andapt-get install -y -qq ghinstalls 2.45.0 cleanly.run view --json/--log-failed,api,pr list --json,pr create --body-file— from the shipped man pages, and the requested JSON field names confirmed present in the binary.bash -nclean on all 13run:blocks.ghacross eight scenarios: total failure (aborts with exit 1 and names 127), partial degradation, parent-failure propagation, partial per-job failure preserving good data, all-per-job-failure downgrading tofailed:, malformed-JSON merge failure, a pasted job URL, and hostile stderr attempting to forge::error::/::stop-commands::.?redirect=/runs/999.$?after a failedif, a numeric job id passing run-id validation, and derived statuses reportingokfor loops that never ran.developwith this change applied — see the note below.No Java changed. The diff is one GitHub Actions workflow file, so the coverage gate has no changed lines to measure and Spotless (Java-only) does not apply.