fix(migration): stop the report claiming PASSED when the red step is outside every phase (#1141)

.github/workflows/migration-test.yml

@@ -219,12 +219,19 @@ jobs:
       # opened an issue titled "Failed" whose body said PASSED for exactly that
       # reason — `test_05_execute_flow_ui` died on a Playwright timeout before it
       # could write its own verdict.
+      #
+      # `JOB_STATUS` closes the other half (#1141): only these three steps carry an
+      # `id`, so a failure in the steps BETWEEN them — resolving/installing/booting
+      # the nightly, or the alembic migration timing out — left phases 2 and 3
+      # unrun, their outcomes empty, and the report saying PASSED on a red job. The
+      # job's own status covers those steps, and any step added here later.
       - name: Generate report
         if: always()
         env:
           PHASE_OUTCOME_latest: ${{ steps.phase_latest.outcome }}
           PHASE_OUTCOME_nightly_api: ${{ steps.phase_nightly_api.outcome }}
           PHASE_OUTCOME_nightly_ui: ${{ steps.phase_nightly_ui.outcome }}
+          JOB_STATUS: ${{ job.status }}
         run: python tests/github-workflows/migration/generate_report.py
 
       - name: Print report

.gitignore

@@ -44,3 +44,9 @@ all-liveness/
 # run's commit id and the machine's system data, so a committed copy is stale on
 # arrival. Same category as playwright-report/ and blob-report/ above.
 flakiness-report/
+
+# Python bytecode from the migration tests (tests/github-workflows/migration/).
+# Those tests arrived with #1139's python-units lane; running them locally leaves
+# __pycache__ behind, which showed up as untracked noise on every subsequent diff.
+__pycache__/
+*.pyc

tests/github-workflows/migration/generate_report.py

@@ -23,6 +23,22 @@
 recorded no failure, is reported as a crash — never as a pass. A phase that was
 supposed to run and is missing from the state entirely is reported as incomplete.
 Nothing here can conclude "passed" from missing data.
+
+## Why the job's own status is reconciled too (#1141)
+
+Only three steps carry an `id`, so `PHASE_OUTCOME_*` covers only the three
+verification phases. The steps **between** them — resolving, installing, booting
+the nightly, and waiting out the alembic migration — carry none, and a failure
+there reproduced the #1120 symptom by a different route: the job goes red, phases 2
+and 3 never run (so their outcomes arrive empty and are not declared), the state
+file holds a fully-passing `latest` phase, and the report printed `Result: PASSED`
+into an issue titled *"Failed"*. A migration that never completes is precisely what
+this workflow exists to catch, and it landed in that gap.
+
+So the workflow also hands over `JOB_STATUS` (`job.status`). A red job whose report
+found nothing wrong is itself an integrity problem: the failure is real and lives
+outside every phase this report can see. Reconciling the job status covers the
+steps that exist today *and* any added later, which per-step `id`s would not.
 """
 
 import json
@@ -47,6 +63,13 @@
 # `steps.<id>.outcome` vocabulary, passed straight through.
 PHASE_OUTCOME_PREFIX = "PHASE_OUTCOME_"
 
+# The job's own status at the time the report is generated, from `job.status`
+# (`success|failure|cancelled`). The report step runs under `if: always()`, so this
+# is the runner's verdict on everything that happened before it — including the
+# steps no `PHASE_OUTCOME_*` covers.
+JOB_STATUS_VAR = "JOB_STATUS"
+JOB_STATUS_NOT_OK = {"failure", "cancelled"}
+
 RESULT_FAILED = "FAILED"
 RESULT_PASSED = "PASSED"
 RESULT_PASSED_WARN = "PASSED (with warnings)"
@@ -81,13 +104,24 @@ def declared_outcomes(environ=None) -> dict:
     return out
 
 
-def assess(state: dict, declared: dict) -> dict:
+def job_status(environ=None) -> Optional[str]:
+    """The runner's verdict on the job so far, from `JOB_STATUS`. `None` if unset."""
+    env = os.environ if environ is None else environ
+    value = (env.get(JOB_STATUS_VAR) or "").strip().lower()
+    return value or None
+
+
+def assess(state: dict, declared: dict, job: Optional[str] = None) -> dict:
     """Decide the overall result, and surface anything that makes the report untrustworthy.
 
     Returns `{"result", "failures", "warnings", "integrity"}`. `integrity` holds
     mismatches between what the runner observed and what the phase recorded — the
     #1120 class. Those count as failures: a report that cannot account for a phase
     must not claim that phase passed.
+
+    `job` is the runner's verdict on the whole job (`job.status`). It catches the
+    #1141 case: a failure in a step no phase covers, which otherwise left the report
+    saying `PASSED` on a red run.
     """
     phases = state.get("phases", {}) or {}
 
@@ -132,6 +166,18 @@ def assess(state: dict, declared: dict) -> dict:
             "verified nothing. Treated as a failure rather than an empty pass."
         )
 
+    # #1141: the job is red but nothing above accounts for it — the failing step is
+    # one this report does not cover. Only reported when there is no attribution yet;
+    # a phase that already owns the failure says it better than this can.
+    if job in JOB_STATUS_NOT_OK and not failures and not integrity:
+        integrity.append(
+            f"- **(outside every phase)**: the runner reports this job `{job}`, but no "
+            f"verification phase recorded or was declared a failure — so the cause is a "
+            f"step this report does not cover (resolving, installing or booting the "
+            f"nightly, or the alembic migration timing out). Read the job log; this "
+            f"report cannot attribute it."
+        )
+
     if failures or integrity:
         result = RESULT_FAILED
     elif warnings:
@@ -160,21 +206,31 @@ def format_step(name: str, step: dict) -> str:
     return line
 
 
-def generate_report(state: dict, declared: Optional[dict] = None) -> str:
+def generate_report(
+    state: dict, declared: Optional[dict] = None, job: Optional[str] = None
+) -> str:
     if declared is None:
         declared = declared_outcomes()
+    if job is None:
+        job = job_status()
     now = datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M UTC")
     latest_digest = load_digest("/tmp/latest-digest.txt")
     nightly_digest = load_digest("/tmp/nightly-digest.txt")
 
-    verdict = assess(state, declared)
+    verdict = assess(state, declared, job)
 
     lines = [
         "# Langflow Migration Test Report",
         f"**Date:** {now}",
         f"**Flow:** {state.get('flow_name', 'N/A')} (`{state.get('flow_id', 'N/A')}`)",
         f"**Latest digest:** `{latest_digest[-20:]}`",
         f"**Nightly digest:** `{nightly_digest[-20:]}`",
+    ]
+    # Stated in the header so a reader comparing the issue title to the body can see
+    # both verdicts at once — the pair that contradicted each other in #1120/#1141.
+    if job:
+        lines.append(f"**Job status (runner):** `{job}`")
+    lines += [
         "",
         f"## Result: {verdict['result']}",
         "",
@@ -184,7 +240,7 @@ def generate_report(state: dict, declared: Optional[dict] = None) -> str:
     # trusted at face value — burying it below the per-step tables is how run #115
     # read as a pass.
     if verdict["integrity"]:
-        lines.append("## Unaccounted phases")
+        lines.append("## Unaccounted failures")
         lines.append("")
         lines.extend(verdict["integrity"])
         lines.append("")

tests/github-workflows/migration/test_generate_report.py

@@ -163,12 +163,98 @@ def test_the_rendered_report_leads_with_the_unaccounted_phase():
     )
 
     assert "## Result: FAILED" in body
-    assert "## Unaccounted phases" in body
-    assert body.index("## Unaccounted phases") < body.index("### Phase: latest")
+    assert "## Unaccounted failures" in body
+    assert body.index("## Unaccounted failures") < body.index("### Phase: latest")
     # The runner's own verdict is shown next to the phase it belongs to.
     assert "runner outcome: `failure`" in body
 
 
+# ── The job-status half of the same defect (#1141) ────────────────────────────
+#
+# The exact shape of a run whose nightly never came up: `latest` fully recorded and
+# passing, phases 2 and 3 never reached, so the runner hands over empty outcomes for
+# them. Before the fix this rendered `Result: PASSED` into an issue titled "Failed".
+NIGHTLY_NEVER_BOOTED_STATE = {
+    "phases": {
+        "latest": {
+            "steps": {"auth": {"status": "pass"}, "execute_flow": {"status": "pass"}}
+        }
+    }
+}
+
+
+def test_a_red_job_with_no_phase_failure_is_not_a_pass():
+    """The #1141 regression: the failing step is one no phase covers."""
+    verdict = gr.assess(
+        NIGHTLY_NEVER_BOOTED_STATE, {"latest": "success"}, job="failure"
+    )
+
+    assert verdict["result"] == gr.RESULT_FAILED, (
+        "a red job whose report found nothing wrong must not print PASSED — the "
+        "failure is real and lives outside every phase this report can see"
+    )
+    assert len(verdict["integrity"]) == 1
+    assert "outside every phase" in verdict["integrity"][0]
+    assert "Read the job log" in verdict["integrity"][0]
+
+
+def test_the_same_state_still_passes_when_the_job_is_green():
+    """Guards against the fix reddening healthy runs: only the job status differs."""
+    for job in ("success", None):
+        verdict = gr.assess(
+            NIGHTLY_NEVER_BOOTED_STATE, {"latest": "success"}, job=job
+        )
+        assert verdict["result"] == gr.RESULT_PASSED, job
+        assert verdict["integrity"] == []
+
+
+def test_a_cancelled_job_is_not_a_pass():
+    verdict = gr.assess(
+        NIGHTLY_NEVER_BOOTED_STATE, {"latest": "success"}, job="cancelled"
+    )
+
+    assert verdict["result"] == gr.RESULT_FAILED
+    assert "`cancelled`" in verdict["integrity"][0]
+
+
+def test_a_red_job_is_not_reported_twice_when_a_phase_already_owns_the_failure():
+    """The phase-level message attributes the failure; this one cannot. Don't add noise."""
+    recorded = {"phases": {"nightly_api": {"steps": {"flow": {"status": "fail"}}}}}
+    verdict = gr.assess(recorded, {"nightly_api": "failure"}, job="failure")
+
+    assert verdict["result"] == gr.RESULT_FAILED
+    assert verdict["integrity"] == []
+    assert len(verdict["failures"]) == 1
+
+    # Same for an unaccounted phase: run #115's job was red too.
+    verdict = gr.assess(
+        RUN_115_STATE,
+        {"latest": "success", "nightly_api": "success", "nightly_ui": "failure"},
+        job="failure",
+    )
+    assert len(verdict["integrity"]) == 1
+    assert "nightly_ui" in verdict["integrity"][0]
+
+
+def test_job_status_is_parsed_from_the_env():
+    assert gr.job_status({"JOB_STATUS": "Failure"}) == "failure"  # GitHub casing varies
+    assert gr.job_status({"JOB_STATUS": "  success  "}) == "success"
+    # An unset or blank value must read as "unknown", never as a status.
+    assert gr.job_status({}) is None
+    assert gr.job_status({"JOB_STATUS": "   "}) is None
+
+
+def test_the_rendered_report_states_both_verdicts():
+    """The pair that contradicted each other in the issue body, side by side."""
+    body = gr.generate_report(
+        NIGHTLY_NEVER_BOOTED_STATE, {"latest": "success"}, job="failure"
+    )
+
+    assert "**Job status (runner):** `failure`" in body
+    assert "## Result: FAILED" in body
+    assert "## Unaccounted failures" in body
+
+
 def test_the_rendered_report_still_lists_every_step():
     body = gr.generate_report(RUN_115_STATE, {})