payload-snapshot: fix silent data loss on unauthenticated gcloud (supersedes #641)

.claude-plugin/marketplace.json

@@ -88,7 +88,7 @@
       "name": "ci",
       "source": "./plugins/ci",
       "description": "A plugin to work with OpenShift CI and analyze Prow job results",
-      "version": "0.0.71",
+      "version": "0.0.72",
       "category": "ci",
       "keywords": [
         "prow",

docs/index.html

@@ -541,7 +541,7 @@ <h1>ai-helpers</h1>
     {
       "name": "ci",
       "description": "Tools for working with OpenShift CI and analyzing Prow job results",
-      "version": "0.0.71",
+      "version": "0.0.72",
       "has_readme": true,
       "commands": [
         {

plugins/ci/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "ci",
   "description": "Tools for working with OpenShift CI and analyzing Prow job results",
-  "version": "0.0.71",
+  "version": "0.0.72",
   "author": {
     "name": "github.com/openshift-eng"
   }

plugins/ci/skills/payload-analysis/SKILL.md

@@ -106,6 +106,24 @@ From `summary.json` top-level fields:
 
 **Record `phase` verbatim** from the `summary.json` metadata (`Accepted`, `Rejected`, or `Ready`). Never infer the phase from the job results or from whether failures exist — a payload can be `Accepted` *with* blocking failures (force-accepted) or `Ready` while jobs are still running. The stored phase drives the force-accept decision (Step 6.4) and the executive summary (Step 7.1), so an inferred phase silently corrupts both.
 
+#### 3.1b: Verify the Snapshot Is Complete
+
+Check `summary.json` → `data_complete`. When it is `false`, read
+`collection_errors[]` and treat every affected job's data as **unknown, not
+empty**.
+
+A job entry with `junit_collection_failed: true` has **no** `test_failure_count`
+because its JUnit could not be read. An absent `test_failure_count` means
+*unknown*; only an explicit `0` means *verified clean*. Never conclude "this job
+had no test failures" — and therefore "it must have failed for some other
+reason" — from missing data. That inference sends the whole analysis down a
+false path.
+
+If the target payload's test data is incomplete, say so explicitly in the report
+and treat any streak or root-cause conclusion that depends on it as
+provisional. Prefer re-running the snapshot (see `payload-snapshot`) over
+analyzing an incomplete one.
+
 #### 3.2: Failed Blocking Jobs
 
 From `summary.json` → `blocking_jobs.failed_jobs[]`, each entry contains:
@@ -291,6 +309,17 @@ Compare the subagent's root cause analysis for the target payload against previo
 
 If a job fails in two consecutive payloads but for **different reasons**, treat each as a separate streak=1 failure with its own originating payload and candidate PRs. Re-split the streak and re-assign originating payloads before proceeding to scoring.
 
+**The job-level streak is not the originating payload of a failure mode.** `blocking_jobs.failed_jobs[].streak.originating_payload` tracks when the *job* started failing, which silently merges unrelated modes — an infrastructure blip, a flaky test, and a real regression all read as one streak. Scoring candidate PRs from a job-level originating payload that predates the actual regression guarantees misattribution: the causal PR is not even in the candidate set, so the analysis confidently blames whatever else happened to land there.
+
+Do this mechanically, per failure mode:
+
+1. Take the **specific failing test** identified in Step 4, and read `test_failures.blocking[]` → the matching entry's **`first_failed_in`** and `payloads_failing`. That is the originating payload for *this* mode.
+2. When `first_failed_in` is **later** than the job-level `streak.originating_payload`, the job's earlier failures are a **different mode**. Score candidates from `first_failed_in`, not from the job streak.
+3. Verify the boundary. Confirm the test **passed** in the payload immediately before `first_failed_in`. If per-test data is unavailable there (e.g. the aggregation produced none), do **not** assume the test was failing — check the underlying child runs. A job that failed before the upgrade phase even ran cannot have failed this test.
+4. An aggregated job can fail with **no per-test signal at all** (too few runs completed, or infrastructure killed the runs). Such a payload is **unclassified**, not a member of the regression streak. Read the underlying child runs' step results before assigning it to any mode.
+
+Only after the per-mode originating payload is established should you enumerate candidate PRs (Step 6.1). Record the job-level streak and the per-mode onset separately in the report when they differ, and state which one drove scoring.
+
 ### Step 5b: Adjudicate Conflicting Root Causes
 
 When two or more investigations reach **contradictory root causes for the same failure signature** (same test, same operation, or same error class — across jobs, across retries, or between a subagent and a previous analysis), the analysis is **UNRESOLVED**. It is *not* a tie to be broken by whichever explanation feels more plausible. Resolve it only with discriminating evidence, applying these rules:

plugins/ci/skills/payload-snapshot/SKILL.md

@@ -183,6 +183,26 @@ Comprehensive stream-level triage data — start here. Contains:
 - `test_failures.blocking[]` — `test_name`, `jobs`, `first_failed_in`, `payloads_failing`, `failure_message`, `failure_text` (full, not truncated)
 - `payloads[]` — per-payload entries with `tag`, `phase`, relative file paths, `prs[]` with component/diff/comments paths, and `rhcos_changes[]` with RPM diffs per RHCOS variant
 - `rhcos_rpms[]` — RPMDB metadata for the target payload's RHCOS variants: `tag`, `name`, `pullspec`, `rpmdb` (relative path to rpmdb.sqlite)
+- `data_complete` — `true` when every collection step succeeded. `false` means part of the snapshot could not be read.
+- `collection_errors[]` — present only when `data_complete` is `false`. Each entry has `reason` (`auth`, `timeout`, `gcloud_missing`, `command_failed`, `junit_unavailable`, `build_log_unavailable`), `command`, and optionally `detail`, `stage`, `job`.
+
+#### Missing data is absent, never empty
+
+When a collection step fails, the affected file is **not written** and the
+corresponding summary field is **omitted** — it is never emitted as an empty
+list or a zero count. Specifically:
+
+- If JUnit could not be read for a job, `results.json` is not created, the
+  job entry has **no** `test_failure_count` and `junit_results`, and instead
+  carries `junit_collection_failed: true`.
+- An absent `test_failure_count` therefore means *unknown*, whereas `0` means
+  *verified clean*.
+
+Consumers **must** distinguish these. Treating unreadable data as "no test
+failures" makes a broken job look like it failed for some other reason, which
+misdirects root-cause analysis. Check `data_complete` before drawing any
+conclusion from an absence of test failures. Use `--fail-on-incomplete` in
+automation to exit non-zero rather than emit a partial snapshot.
 
 ### `AGENTS.md` / `CLAUDE.md`