From 1961b830ba94b3287eac4de4a32bc641ec9446cf Mon Sep 17 00:00:00 2001 From: lycaon Date: Wed, 17 Jun 2026 22:21:45 -0600 Subject: [PATCH] fix(robomp): harden lifecycle contract review cases --- python/robomp/tests/test_status_contract.py | 26 ++++++++---- python/robomp/web/src/types.ts | 4 +- python/robomp/web/src/work-items.test.ts | 44 +++++++++++++++++++++ python/robomp/web/src/work-items.ts | 10 +++-- 4 files changed, 71 insertions(+), 13 deletions(-) diff --git a/python/robomp/tests/test_status_contract.py b/python/robomp/tests/test_status_contract.py index 8075bc42b..9a4e1aedd 100644 --- a/python/robomp/tests/test_status_contract.py +++ b/python/robomp/tests/test_status_contract.py @@ -1,8 +1,13 @@ -"""Contract test for the robomp server API response, producing status-contract.json.""" +"""Contract test for the robomp server API response vs status-contract.json. + +Regenerate the committed fixture with: + ROBOMP_UPDATE_STATUS_CONTRACT=1 pytest tests/test_status_contract.py +""" from __future__ import annotations import json +import os from pathlib import Path from typing import Any @@ -14,10 +19,11 @@ from robomp.db import get_database from robomp.server import create_app -# Runtime/timestamp fields vary every run; normalize them so an ordinary pytest -# run regenerates a byte-identical committed fixture instead of dirtying it. +# Runtime/timestamp fields vary every run; normalize them so the live payload +# can be compared against (or regenerated into) a byte-stable committed fixture. _VOLATILE_TS_KEYS = {"received_at", "started_at", "last_tool_ts", "updated_at"} _FIXED_TS = "2024-01-01T00:00:00+00:00" +_UPDATE_ENV = "ROBOMP_UPDATE_STATUS_CONTRACT" def _normalize_for_fixture(value: Any) -> Any: @@ -191,12 +197,16 @@ def test_status_contract(settings: Settings) -> None: # check runtime: assert data["runtime"]["repo_allowlist"] == ["octo/widget"] - # Write fixture: + # Compare the normalized payload against the committed fixture. Normal + # pytest runs assert equality; regenerate only when ROBOMP_UPDATE_STATUS_CONTRACT=1. fixture_path = Path(__file__).parent.parent / "web/test/fixtures/status-contract.json" - fixture_path.parent.mkdir(parents=True, exist_ok=True) - fixture_path.write_text( - json.dumps(_normalize_for_fixture(data), indent=2), encoding="utf-8" - ) + actual = _normalize_for_fixture(data) + if os.environ.get(_UPDATE_ENV) == "1": + fixture_path.parent.mkdir(parents=True, exist_ok=True) + fixture_path.write_text(json.dumps(actual, indent=2), encoding="utf-8") + else: + expected = json.loads(fixture_path.read_text(encoding="utf-8")) + assert actual == expected def _enable_replay(monkeypatch: pytest.MonkeyPatch) -> str: diff --git a/python/robomp/web/src/types.ts b/python/robomp/web/src/types.ts index fc9bc2f49..9f698a03e 100644 --- a/python/robomp/web/src/types.ts +++ b/python/robomp/web/src/types.ts @@ -8,9 +8,11 @@ export type IssueState = | "new" | "reproducing" | "fixing" + | "reviewing" | "opened" | "merged" | "closed" + | "needs_info" | "abandoned"; export interface RuntimeInfo { @@ -37,7 +39,7 @@ export interface IssueRow { number: number; branch: string | null; pr_number: number | null; - state: IssueState | string; + state: IssueState; classification: string | null; updated_at: string; latest_event: LatestEvent | null; diff --git a/python/robomp/web/src/work-items.test.ts b/python/robomp/web/src/work-items.test.ts index 49f4ce026..aaa1034f4 100644 --- a/python/robomp/web/src/work-items.test.ts +++ b/python/robomp/web/src/work-items.test.ts @@ -470,8 +470,15 @@ describe("buildWorkItems", () => { expect(stageOrdinal("nonsense")).toBe(0); expect(stageOrdinal("new")).toBe(0); expect(stageOrdinal("reproducing")).toBe(1); + // needs_info is set by mark_unable_to_reproduce during reproduction and + // cleared back to reproducing by repro_record in host_tools; it stays in + // the reproduction phase (ordinal 1), not advanced past fixing. + expect(stageOrdinal("needs_info")).toBe(1); expect(stageOrdinal("fixing")).toBe(2); + // reviewing is PR review work (tasks.py sets it on PR-opened issues); it + // lives on the PR step, same stage as opened (ordinal 3). expect(stageOrdinal("opened")).toBe(3); + expect(stageOrdinal("reviewing")).toBe(3); expect(stageOrdinal("merged")).toBe(4); }); @@ -570,4 +577,41 @@ describe("buildWorkItems", () => { expect(items[0].latestEvent?.state).toBe("running"); }); + test("live running event outranks a newer queued latest_event for the same issue", () => { + const items = buildWorkItems( + status({ + issues: [ + issue({ + key: "owner/repo#22", + number: 22, + latest_event: latestEvent({ + delivery_id: "queued-newer", + state: "queued", + received_at: "2026-06-17T00:09:00Z", + }), + }), + ], + running_events: [ + runningEvent({ + delivery_id: "live-older-3", + issue_key: "owner/repo#22", + received_at: "2026-06-17T00:01:00Z", + started_at: "2026-06-17T00:02:00Z", + }), + ], + }), + ); + expect(items).toHaveLength(1); + expect(items[0]).toMatchObject({ + key: "owner/repo#22", + bucket: "running", + deliveryId: "live-older-3", + inflightOnly: false, + error: null, + }); + expect(items[0].live?.delivery_id).toBe("live-older-3"); + expect(items[0].latestEvent?.state).toBe("running"); + expect(items[0].latestEvent?.delivery_id).toBe("live-older-3"); + }); + }); diff --git a/python/robomp/web/src/work-items.ts b/python/robomp/web/src/work-items.ts index b23e2fde3..e2851ff2f 100644 --- a/python/robomp/web/src/work-items.ts +++ b/python/robomp/web/src/work-items.ts @@ -38,8 +38,10 @@ export const SIMPLE_CLASSIFICATIONS: ReadonlySet = new Set([ const STATE_ORDINAL: Record = { new: 0, reproducing: 1, + needs_info: 1, fixing: 2, opened: 3, + reviewing: 3, merged: 4, closed: 4, abandoned: 4, @@ -78,10 +80,10 @@ export function buildWorkItems(status: StatusResponse): WorkItem[] { const latest = issue.latest_event; // A matching live running_events entry is authoritative over the issue's - // own latest_event, which may be a newer failed/done row that the live run - // has not yet superseded. Render the live run so the card stays running, - // cancel-capable (deliveryId from the live delivery), and free of the - // stale failure. Non-live rows keep latest_event authority below. + // own latest_event. That summary row can be a newer queued retry/comment, + // or a failed/done row the live run has not superseded yet. Render the live + // run so the card stays running, cancel-capable (deliveryId from the live + // delivery), and free of stale latest-event state. if (live) { items.push({ key,