From a82697de7dbf2addb7f85d03e36891591c3a881d Mon Sep 17 00:00:00 2001 From: lycaon Date: Thu, 18 Jun 2026 00:20:24 -0600 Subject: [PATCH] fix(robomp): close lifecycle review gaps --- python/robomp/src/server.py | 20 +++ python/robomp/tests/test_status_contract.py | 11 ++ python/robomp/web/scripts/verify-cards.ts | 153 +++++++++++++--- .../robomp/web/src/components/IssueCard.tsx | 6 +- .../web/src/components/views/Activity.tsx | 24 ++- python/robomp/web/src/styles/index.css | 8 + python/robomp/web/src/types.ts | 1 + python/robomp/web/src/work-items.test.ts | 166 +++++++++++++++++- python/robomp/web/src/work-items.ts | 27 ++- .../web/test/fixtures/status-contract.json | 21 ++- 10 files changed, 390 insertions(+), 47 deletions(-) diff --git a/python/robomp/src/server.py b/python/robomp/src/server.py index 5d64700a6..e0861c527 100644 --- a/python/robomp/src/server.py +++ b/python/robomp/src/server.py @@ -652,6 +652,8 @@ def create_app(settings: Settings | None = None) -> FastAPI: event = db.get_event(delivery_id) if event is None: raise HTTPException(404, f"unknown delivery {delivery_id}") + if event.state != "running": + raise HTTPException(409, f"delivery {delivery_id} is {event.state}; only running deliveries can be cancelled") pool: WorkerPool = bag["pool"] fired = await pool.cancel_event(delivery_id) @@ -739,6 +741,23 @@ def create_app(settings: Settings | None = None) -> FastAPI: } events_rows = db.list_events(limit=25) + + # Recent events can reference issues outside the capped 200-issue + # `issues` window above. Surface each event's current DB issue state + # so the dashboard can suppress failures for issues that have since + # gone terminal (merged/closed/abandoned) without a row to consult. + # Reuse the already-loaded issue states; fall back to a per-key + # lookup (bounded by the 25-event cap) for keys outside the window. + issue_state_by_key: dict[str, str | None] = {r.key: r.state for r in issues_rows} + + def _recent_issue_state(issue_key: str | None) -> str | None: + if not issue_key: + return None + if issue_key not in issue_state_by_key: + row = db.get_issue(issue_key) + issue_state_by_key[issue_key] = row.state if row is not None else None + return issue_state_by_key[issue_key] + return { "event_counts": db.event_state_counts(), "issue_event_counts": db.latest_issue_event_state_counts(), @@ -767,6 +786,7 @@ def create_app(settings: Settings | None = None) -> FastAPI: "attempts": r.attempts, "received_at": r.received_at, "last_error": r.last_error, + "issue_state": _recent_issue_state(r.issue_key), } for r in events_rows ], diff --git a/python/robomp/tests/test_status_contract.py b/python/robomp/tests/test_status_contract.py index 9a4e1aedd..324e6d99d 100644 --- a/python/robomp/tests/test_status_contract.py +++ b/python/robomp/tests/test_status_contract.py @@ -273,6 +273,17 @@ def test_cancel_errors_and_gating(env, monkeypatch: pytest.MonkeyPatch) -> None: ) assert resp.status_code == 404 + # 409 queued/non-running delivery: do not poison WorkerPool._cancelled. + resp = client.post( + "/api/cancel", + json={"delivery_id": "run-cancel-2"}, + headers={"X-Robomp-Replay-Token": token}, + ) + assert resp.status_code == 409 + queued = db.get_event("run-cancel-2") + assert queued is not None + assert queued.state == "queued" + # 401 with a bad token (header present but wrong value) resp = client.post( "/api/cancel", diff --git a/python/robomp/web/scripts/verify-cards.ts b/python/robomp/web/scripts/verify-cards.ts index d7b53bcc1..9524e58b1 100755 --- a/python/robomp/web/scripts/verify-cards.ts +++ b/python/robomp/web/scripts/verify-cards.ts @@ -224,7 +224,7 @@ const populatedStatus: StatusResponse = { number: 10, branch: null, pr_number: null, - state: "done", + state: "merged", classification: "bug", updated_at: "2026-06-18T00:02:00Z", latest_event: { @@ -235,6 +235,24 @@ const populatedStatus: StatusResponse = { received_at: "2026-06-18T00:02:00Z", last_error: null } + }, + { + key: "octo/widget#11", + repo: "octo/widget", + number: 11, + branch: null, + pr_number: null, + state: "reproducing", + classification: "bug", + updated_at: "2026-06-18T00:00:00Z", + latest_event: { + delivery_id: "latest-run-del-11", + event_type: "issue_comment", + state: "running", + attempts: 1, + received_at: "2026-06-18T00:00:00Z", + last_error: null + } } ], recent_events: [ @@ -246,7 +264,8 @@ const populatedStatus: StatusResponse = { state: "failed", attempts: 1, received_at: "2026-06-18T00:00:00Z", - last_error: "old error" + last_error: "old error", + issue_state: "merged" }, { delivery_id: "fail-del-6", @@ -256,7 +275,8 @@ const populatedStatus: StatusResponse = { state: "failed", attempts: 1, received_at: "2026-06-18T00:00:00Z", - last_error: "orphan failed error" + last_error: "orphan failed error", + issue_state: null } ] }; @@ -285,6 +305,9 @@ async function main(): Promise { let statusMock: StatusResponse = baseStatus; let statusDelayMs = 0; let customIndexHtml: string | null = null; + // When set, /api/trigger answers POSTs with an error status + detail body so + // the retry-failure surface (e.g. the Activity status line) can be asserted. + let triggerErrorDetail: string | null = null; // Intercept trigger / cancel requests for asserting later. Bodies are // narrowed to a record (or null) so property checks below stay type-safe @@ -322,7 +345,7 @@ async function main(): Promise { req.respond({ status: 200, contentType: "application/json", - body: JSON.stringify({ issues: [], errors: [] }), + body: JSON.stringify({ issues: [], errors: [], repos: ["octo/widget"], cache: { hit: false, fetched_at: 0 } }), }); } else if (url.pathname === "/api/trigger" && req.method() === "POST") { const parsedBody: Record | null = req.postData() ? (JSON.parse(req.postData()!) as Record) : null; @@ -334,15 +357,23 @@ async function main(): Promise { headers: req.headers() as Record, body: parsedBody, }); - req.respond({ - status: 202, - contentType: "application/json", - body: JSON.stringify({ - delivery: (typeof deliveryId === "string" && deliveryId) ? deliveryId : "manual-trigger", - state: "queued", - mode: (typeof mode === "string" && mode) ? mode : "triage", - }), - }); + if (triggerErrorDetail !== null) { + req.respond({ + status: 500, + contentType: "application/json", + body: JSON.stringify({ detail: triggerErrorDetail }), + }); + } else { + req.respond({ + status: 202, + contentType: "application/json", + body: JSON.stringify({ + delivery: (typeof deliveryId === "string" && deliveryId) ? deliveryId : "manual-trigger", + state: "queued", + mode: (typeof mode === "string" && mode) ? mode : "triage", + }), + }); + } } else if (url.pathname === "/api/cancel" && req.method() === "POST") { const parsedBody: Record | null = req.postData() ? (JSON.parse(req.postData()!) as Record) : null; const deliveryId = parsedBody?.delivery_id; @@ -426,24 +457,28 @@ async function main(): Promise { const errorText = el.querySelector(".rmp-card-error")?.textContent?.trim() ?? ""; const actions = Array.from(el.querySelectorAll(".rmp-card-actions button")).map(b => b.textContent?.trim()); const metaText = el.querySelector(".rmp-card-meta")?.textContent?.trim() ?? ""; + const text = el.textContent ?? ""; const stepper = el.querySelector(".rmp-step") !== null; const currentStep = el.querySelector(".rmp-step-node[data-state='current']") !== null; const liveStep = el.querySelector(".rmp-step-node[data-live='true']") !== null; const failedStep = el.querySelector(".rmp-step-node[data-state='failed']") !== null; - return { key, bucket, errorText, actions, metaText, stepper, currentStep, liveStep, failedStep }; + return { key, bucket, errorText, actions, metaText, text, stepper, currentStep, liveStep, failedStep }; }); }); console.log("All resolved cards:", JSON.stringify(cards)); const fullFailedCard = cards.find(c => c.key === "bugocto/widget#1"); const runningCard = cards.find(c => c.key === "bugocto/widget#2"); - const inflightOnlyCard = cards.find(c => c.key === "octo/wid"); + const inflightOnlyCard = cards.find(c => c.key === "issueocto/widget#7"); const compactOrphanRunning = cards.find(c => c.key === "run-del-"); const compactOrphanFailed = cards.find(c => c.key === "fail-del"); const failedWithoutErr = cards.find(c => c.key === "bugocto/widget#8"); const runningWithInvalidTs = cards.find(c => c.key === "bugocto/widget#9"); const queuedCard = cards.find(c => c.key === "bugocto/widget#3"); const activeCard = cards.find(c => c.key === "bugocto/widget#4"); + // Latest-event-only running: issue.latest_event is running but no matching + // running_events row and not inflight (live === null, inflightOnly === false). + const latestEventOnlyRunning = cards.find(c => c.key === "bugocto/widget#11"); // Check failed full card results.push({ @@ -462,7 +497,7 @@ async function main(): Promise { // Check inflight-only card results.push({ name: "inflight-only-card", - ok: !!inflightOnlyCard && inflightOnlyCard.bucket === "running" && inflightOnlyCard.metaText.includes("held by pool") && inflightOnlyCard.actions.length === 0, + ok: !!inflightOnlyCard && inflightOnlyCard.bucket === "running" && inflightOnlyCard.metaText.includes("held by pool") && inflightOnlyCard.actions.length === 0 && inflightOnlyCard.stepper && inflightOnlyCard.liveStep, detail: `Inflight-only card check: ${JSON.stringify(inflightOnlyCard)}`, }); @@ -508,6 +543,19 @@ async function main(): Promise { detail: `Active full card check: ${JSON.stringify(activeCard)}`, }); + // Latest-event-only running card — renders as running (latest_event state), + // but with NO live row it must not expose cancel (nothing to kill) and must + // not mark a lifecycle step live (no real subprocess heartbeat). + results.push({ + name: "latest-event-only-running-no-cancel-no-live-step", + ok: !!latestEventOnlyRunning && + latestEventOnlyRunning.bucket === "running" && + !latestEventOnlyRunning.actions.includes("cancel") && + latestEventOnlyRunning.stepper && + !latestEventOnlyRunning.liveStep, + detail: `Latest-event-only running card check: ${JSON.stringify(latestEventOnlyRunning)}`, + }); + // Failed cards must sort ahead of every other bucket: the first rendered // card is a failed card (DOM order mirrors buildWorkItems sort order). results.push({ @@ -530,12 +578,21 @@ async function main(): Promise { detail: `Failed card stepper failedStep=${fullFailedCard?.failedStep}`, }); - // Check superseded exclusion - const hasSuperseded = await page.evaluate(() => document.body.textContent?.includes("superseded-del") ?? false); + // Check superseded + terminal exclusion against the rendered lifecycle-card + // model above, not document.body. Activity stays mounted while hidden and may + // legitimately contain historical terminal events. + const terminalLeakKeys = cards + .filter( + (card) => + card.text.includes("superseded-del") || + card.text.includes("done-del") || + card.text.includes("octo/widget#10"), + ) + .map((card) => card.key); results.push({ - name: "superseded-exclusion-visual", - ok: !hasSuperseded, - detail: `Superseded text found: ${hasSuperseded}`, + name: "superseded-terminal-exclusion-visual", + ok: terminalLeakKeys.length === 0, + detail: `Terminal/superseded card keys: ${terminalLeakKeys.join(", ") || "none"}`, }); const debugTextContrast = await page.evaluate(() => { const getLuminance = (str: string): number => { @@ -1051,6 +1108,60 @@ async function main(): Promise { detail: `Matching enter requests: ${enterReqs.length}, token: "${enterReqs[0]?.headers["x-robomp-replay-token"] ?? ""}"`, }); + // G. Activity-view retry failure — switching to Activity and retrying a + // failed event must surface the trigger error WHERE the button was + // clicked (the Trigger bar lives only on Operations/Triage). Force + // /api/trigger to error, click the Activity events-table retry button, + // and assert .rmp-activity-status shows the error with role="alert" and + // that exactly one trigger request fired carrying the replay token. + triggeredRequests.length = 0; + const ACTIVITY_ERROR_DETAIL = "forced retry failure"; + triggerErrorDetail = ACTIVITY_ERROR_DETAIL; + // Switch to the Activity view via the nav rail. + await page.evaluate(() => { + const navBtn = Array.from(document.querySelectorAll(".rmp-nav-item")).find( + (b) => b.querySelector(".rmp-nav-item-label")?.textContent?.trim() === "Activity", + ) as HTMLElement | null; + navBtn?.click(); + }); + await Bun.sleep(100); // let the view flip to display:flex + // Click the first retry button in the Activity events table (Events is the + // only `table.t` consumer, so this is unambiguous). + await page.evaluate(() => { + const btn = Array.from(document.querySelectorAll("table.t button")).find( + (b) => b.textContent?.trim() === "retry", + ) as HTMLElement | null; + btn?.click(); + }); + await Bun.sleep(100); // wait for the failed trigger round-trip + state update + const activityStatus = await page.evaluate(() => { + const el = document.querySelector(".rmp-activity-status"); + if (!el) return null; + return { + text: el.textContent?.trim() ?? "", + role: el.getAttribute("role") ?? "", + // offsetParent is null when an ancestor is display:none — proves the + // status is actually visible inside the active Activity view. + visible: (el as HTMLElement).offsetParent !== null, + }; + }); + const activityRetryReqs = triggeredRequests.filter( + (r) => r.url.endsWith("/api/trigger") && r.body?.mode === "retry", + ); + triggerErrorDetail = null; // reset so later passes see the normal 202 + results.push({ + name: "activity-retry-error-visible", + ok: + !!activityStatus && + activityStatus.visible && + activityStatus.role === "alert" && + activityStatus.text.includes(ACTIVITY_ERROR_DETAIL) && + activityRetryReqs.length === 1 && + activityRetryReqs[0].headers["x-robomp-replay-token"] === REPLAY_TOKEN, + detail: `Activity status: ${JSON.stringify(activityStatus)}. Retry requests: ${activityRetryReqs.length}, token: "${activityRetryReqs[0]?.headers["x-robomp-replay-token"] ?? ""}"`, + }); + await page.screenshot({ path: path.join(outDir, "shots/activity-retry-error.png") }); + customIndexHtml = null; // restore real-index interception // Cleanup browser diff --git a/python/robomp/web/src/components/IssueCard.tsx b/python/robomp/web/src/components/IssueCard.tsx index af7e8308e..97f39a18f 100644 --- a/python/robomp/web/src/components/IssueCard.tsx +++ b/python/robomp/web/src/components/IssueCard.tsx @@ -39,7 +39,7 @@ export function IssueCard(props: IssueCardProps): JSX.Element { state={item().issueState} classification={item().classification} failed={item().bucket === "failed"} - live={item().bucket === "running"} + live={item().live !== null || item().inflightOnly} /> @@ -52,7 +52,7 @@ export function IssueCard(props: IssueCardProps): JSX.Element { when={ CONFIG.replayEnabled && (item().bucket === "failed" || - (item().bucket === "running" && !item().inflightOnly)) + (item().bucket === "running" && item().live !== null)) } >
@@ -64,7 +64,7 @@ export function IssueCard(props: IssueCardProps): JSX.Element { retry - + diff --git a/python/robomp/web/src/components/views/Activity.tsx b/python/robomp/web/src/components/views/Activity.tsx index a928fa3bb..c68c90c9e 100644 --- a/python/robomp/web/src/components/views/Activity.tsx +++ b/python/robomp/web/src/components/views/Activity.tsx @@ -1,11 +1,21 @@ -import { type JSX } from "solid-js"; +import { type JSX, Show } from "solid-js"; -import { runTrigger } from "../../state"; +import { runTrigger, triggerStatus } from "../../state"; import { Events } from "../Events"; import { Logs } from "../Logs"; +const STATUS_TONE = { + idle: "text-ink-400", + pending: "text-ink-200", + ok: "text-ok", + err: "text-err", +} as const; + // Activity (investigate mode). Full history table + full-height log stream. -// Logs finally get real vertical room. +// Logs finally get real vertical room. The shared trigger status renders right +// under the events table so a retry fired from this view surfaces its +// success/error where the button was clicked — the Trigger bar lives only on +// Operations/Triage, so without this the Activity retry feedback is invisible. export function Activity(): JSX.Element { const handleRetry = (deliveryId: string): void => { void runTrigger({ mode: "retry", delivery_id: deliveryId }); @@ -14,6 +24,14 @@ export function Activity(): JSX.Element { return ( <> + + + {triggerStatus().text} + + ); diff --git a/python/robomp/web/src/styles/index.css b/python/robomp/web/src/styles/index.css index 81cbab075..7ab243b31 100644 --- a/python/robomp/web/src/styles/index.css +++ b/python/robomp/web/src/styles/index.css @@ -981,6 +981,14 @@ code, margin-left: auto; } +/* Activity view — shared trigger status surfaced under the events table so a + retry fired from this view shows success/error where the button was clicked. */ +.rmp-activity-status { + font-size: 12px; + color: var(--color-ink-300); + font-variant-numeric: tabular-nums; +} + /* Mobile drawer */ .rmp-mobile-drawer-overlay { position: fixed; diff --git a/python/robomp/web/src/types.ts b/python/robomp/web/src/types.ts index 9f698a03e..b0ffb4b97 100644 --- a/python/robomp/web/src/types.ts +++ b/python/robomp/web/src/types.ts @@ -67,6 +67,7 @@ export interface RecentEvent { attempts: number; received_at: string; last_error: string | null; + issue_state: IssueState | null; } export interface StatusResponse { diff --git a/python/robomp/web/src/work-items.test.ts b/python/robomp/web/src/work-items.test.ts index 60177c0bb..4557449ca 100644 --- a/python/robomp/web/src/work-items.test.ts +++ b/python/robomp/web/src/work-items.test.ts @@ -96,6 +96,7 @@ function recentEvent(overrides: Partial = {}): RecentEvent { attempts: 1, received_at: "2026-06-17T00:00:00Z", last_error: "boom", + issue_state: null, ...overrides, }; } @@ -151,6 +152,65 @@ describe("buildWorkItems", () => { expect(items).toEqual([]); }); + test("keeps live terminal issues visible until the running delivery disappears", () => { + const items = buildWorkItems( + status({ + issues: [ + issue({ + key: "owner/repo#223", + number: 223, + state: "merged", + latest_event: latestEvent({ delivery_id: "done-223", state: "done" }), + }), + ], + running_events: [ + runningEvent({ + delivery_id: "live-terminal-223", + issue_key: "owner/repo#223", + }), + ], + }), + ); + + expect(items).toHaveLength(1); + expect(items[0]).toMatchObject({ + key: "owner/repo#223", + deliveryId: "live-terminal-223", + issueState: "merged", + bucket: "running", + inflightOnly: false, + }); + expect(items[0].live?.delivery_id).toBe("live-terminal-223"); + }); + + test("terminal issues stay excluded from orphan recent-event fallback", () => { + const items = buildWorkItems( + status({ + issues: [ + issue({ + key: "owner/repo#222", + number: 222, + state: "abandoned", + latest_event: latestEvent({ + delivery_id: "failed-terminal", + state: "failed", + last_error: "terminal failure", + }), + }), + ], + recent_events: [ + recentEvent({ + delivery_id: "failed-terminal", + issue_key: "owner/repo#222", + received_at: "2026-06-17T00:07:00Z", + last_error: "terminal failure", + }), + ], + }), + ); + expect(items).toEqual([]); + }); + test("uses issue latest_event as authority for failed issue rows", () => { const items = buildWorkItems( status({ @@ -314,41 +374,71 @@ describe("buildWorkItems", () => { }); }); - test("2. orphan running delivery, issue_key present but no issue row", () => { + test("2. orphan running issue key absent from issues yields ref and keeps delivery/action data", () => { const items = buildWorkItems( status({ running_events: [ runningEvent({ issue_key: "octo/widget#999", delivery_id: "run-x", + last_tool: "edit", + last_tool_ts: "2026-06-17T00:02:00Z", }), ], }), ); expect(items).toHaveLength(1); + // The issue row is absent, but the issue-shaped key recovers the ref so the + // orphan card can still link to octo/widget#999. expect(items[0]).toMatchObject({ - ref: null, + ref: { repo: "octo/widget", number: 999 }, bucket: "running", deliveryId: "run-x", inflightOnly: false, }); - expect(items[0].live).not.toBeNull(); + // Delivery id and live action data survive the orphan path. + expect(items[0].live?.delivery_id).toBe("run-x"); + expect(items[0].live?.last_tool).toBe("edit"); + expect(items[0].live?.model).toBe("test-model"); }); - test("3. orphan inflight key, no issue & no running event", () => { + test("3. orphan inflight-only issue key absent from issues yields ref with no real delivery", () => { const items = buildWorkItems( status({ inflight: ["octo/widget#888"], }), ); expect(items).toHaveLength(1); + // Issue-shaped inflight key recovers the ref even with no issue row. expect(items[0]).toMatchObject({ - ref: null, + ref: { repo: "octo/widget", number: 888 }, bucket: "running", inflightOnly: true, live: null, - deliveryId: "octo/widget#888", }); + // No running event, so there is no real delivery id; it falls back to the key. + expect(items[0].deliveryId).toBe("octo/widget#888"); + }); + + test("3b. non-issue orphan delivery/key keeps ref null", () => { + const items = buildWorkItems( + status({ + running_events: [ + runningEvent({ + issue_key: null, + delivery_id: "run-bare-uuid", + }), + ], + inflight: ["inflight-bare-uuid"], + }), + ); + expect(items).toHaveLength(2); + // Neither key is issue-shaped (no `#`), so both stay ref: null. + const running = items.find((i) => i.deliveryId === "run-bare-uuid"); + const inflight = items.find((i) => i.key === "inflight-bare-uuid"); + expect(running?.ref).toBeNull(); + expect(inflight?.ref).toBeNull(); + expect(inflight?.inflightOnly).toBe(true); }); test("4. failed issue with last_error:null", () => { @@ -684,6 +774,34 @@ describe("buildWorkItems", () => { expect(items).toEqual([]); }); + test("does not let a newer skipped event suppress a retryable orphan failure", () => { + const items = buildWorkItems( + status({ + recent_events: [ + recentEvent({ + delivery_id: "failed-before-skipped", + issue_key: "owner/repo#34", + received_at: "2026-06-17T00:04:00Z", + last_error: "real failure", + }), + recentEvent({ + delivery_id: "skipped-newer-noise", + issue_key: "owner/repo#34", + state: "skipped", + received_at: "2026-06-17T00:06:00Z", + last_error: null, + }), + ], + }), + ); + expect(items).toHaveLength(1); + expect(items[0]).toMatchObject({ + deliveryId: "failed-before-skipped", + bucket: "failed", + error: "real failure", + }); + }); + test("renders an orphan failed recent event that is the newest for an absent issue", () => { const items = buildWorkItems( status({ @@ -713,4 +831,40 @@ describe("buildWorkItems", () => { }); }); + test("suppresses a failed recent event for an absent issue whose issue_state is terminal", () => { + const items = buildWorkItems( + status({ + recent_events: [ + // octo/widget#900 is outside the capped status.issues window, so the + // only authority for its lifecycle is the issue_state /api/status + // attached. It is "merged", so this stale failure must not surface. + recentEvent({ + delivery_id: "failed-terminal-orphan", + issue_key: "octo/widget#900", + issue_state: "merged", + received_at: "2026-06-17T00:06:00Z", + last_error: "stale terminal failure", + }), + // A retryable failure for a different, non-terminal absent issue must + // still render: the terminal skip only drops its own event. + recentEvent({ + delivery_id: "failed-retryable-orphan", + issue_key: "octo/widget#901", + issue_state: "fixing", + received_at: "2026-06-17T00:05:00Z", + last_error: "live failure", + }), + ], + }), + ); + expect(items).toHaveLength(1); + expect(items[0]).toMatchObject({ + key: "octo/widget#901", + deliveryId: "failed-retryable-orphan", + bucket: "failed", + issueState: "fixing", + error: "live failure", + }); + }); + }); diff --git a/python/robomp/web/src/work-items.ts b/python/robomp/web/src/work-items.ts index be9645342..b4e403bee 100644 --- a/python/robomp/web/src/work-items.ts +++ b/python/robomp/web/src/work-items.ts @@ -70,13 +70,14 @@ export function buildWorkItems(status: StatusResponse): WorkItem[] { const items: WorkItem[] = []; for (const issue of status.issues) { - if (TERMINAL_ISSUE_STATES.has(issue.state)) continue; - const key = issue.key; - seen.add(key); - const live = runningByKey.get(key) ?? null; const inflightOnly = !live && inflightSet.has(key); + if (TERMINAL_ISSUE_STATES.has(issue.state) && !live && !inflightOnly) { + seen.add(key); + continue; + } + seen.add(key); const latest = issue.latest_event; // A matching live running_events entry is authoritative over the issue's @@ -144,7 +145,7 @@ export function buildWorkItems(status: StatusResponse): WorkItem[] { const newestRecentByKey = new Map(); for (const event of status.recent_events) { - if (!event.issue_key) continue; + if (!event.issue_key || event.state === "skipped") continue; const current = newestRecentByKey.get(event.issue_key); const eventTs = parseTs(event.received_at); const currentTs = current ? parseTs(current.received_at) : 0; @@ -157,6 +158,13 @@ export function buildWorkItems(status: StatusResponse): WorkItem[] { for (const event of status.recent_events) { if (event.state !== "failed" || !event.delivery_id || seen.has(event.delivery_id)) continue; + // Suppress failures for issues that have since gone terminal + // (merged/closed/abandoned). For issues outside the capped `status.issues` + // window there is no row to consult, so the event's own issue_state — the + // current DB state attached by /api/status — is the only authority. This + // skip only drops the terminal event itself; it never marks the issue_key + // as seen, so a retryable failure for any other issue is untouched. + if (event.issue_state && TERMINAL_ISSUE_STATES.has(event.issue_state)) continue; if (event.issue_key) { const latest = issueByKey.get(event.issue_key)?.latest_event; if (latest && (latest.delivery_id !== event.delivery_id || latest.state !== "failed")) { @@ -177,7 +185,7 @@ export function buildWorkItems(status: StatusResponse): WorkItem[] { key: event.issue_key ?? event.delivery_id, ref: splitRef(event.issue_key), deliveryId: event.delivery_id, - issueState: null, + issueState: event.issue_state, classification: null, branch: null, prNumber: null, @@ -194,10 +202,15 @@ export function buildWorkItems(status: StatusResponse): WorkItem[] { return items; } +// Orphan live/inflight rows have no matching issue row, but their key is still +// the canonical issue key when the row came from an `issue_key` (running event) +// or an issue-shaped inflight entry. Recover the ref via splitRef so the card +// can still link to the issue; splitRef returns null when the `#N` suffix is +// missing or non-numeric (e.g. a bare delivery-id key), keeping ref: null. function orphanLiveItem(key: string, event: RunningEvent | null, inflightOnly: boolean): WorkItem { return { key, - ref: null, + ref: key.includes("#") ? splitRef(key) : null, deliveryId: event?.delivery_id ?? key, issueState: null, classification: null, diff --git a/python/robomp/web/test/fixtures/status-contract.json b/python/robomp/web/test/fixtures/status-contract.json index d853abb99..797dfbaf4 100644 --- a/python/robomp/web/test/fixtures/status-contract.json +++ b/python/robomp/web/test/fixtures/status-contract.json @@ -150,7 +150,8 @@ "state": "queued", "attempts": 0, "received_at": "2024-01-01T00:00:00+00:00", - "last_error": null + "last_error": null, + "issue_state": "new" }, { "delivery_id": "terminal-done", @@ -160,7 +161,8 @@ "state": "done", "attempts": 0, "received_at": "2024-01-01T00:00:00+00:00", - "last_error": null + "last_error": null, + "issue_state": "merged" }, { "delivery_id": "orphan-failed-x", @@ -170,7 +172,8 @@ "state": "failed", "attempts": 0, "received_at": "2024-01-01T00:00:00+00:00", - "last_error": "orphan failed error" + "last_error": "orphan failed error", + "issue_state": null }, { "delivery_id": "new-done", @@ -180,7 +183,8 @@ "state": "done", "attempts": 0, "received_at": "2024-01-01T00:00:00+00:00", - "last_error": null + "last_error": null, + "issue_state": "fixing" }, { "delivery_id": "superseded-failed", @@ -190,7 +194,8 @@ "state": "failed", "attempts": 0, "received_at": "2024-01-01T00:00:00+00:00", - "last_error": "old error" + "last_error": "old error", + "issue_state": "fixing" }, { "delivery_id": "failed-x", @@ -200,7 +205,8 @@ "state": "failed", "attempts": 0, "received_at": "2024-01-01T00:00:00+00:00", - "last_error": "repro diverged" + "last_error": "repro diverged", + "issue_state": "fixing" }, { "delivery_id": "run-x", @@ -210,7 +216,8 @@ "state": "running", "attempts": 1, "received_at": "2024-01-01T00:00:00+00:00", - "last_error": null + "last_error": null, + "issue_state": "reproducing" } ] } \ No newline at end of file