smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(robomp): close lifecycle review gaps

Browse Source
This commit is contained in:
lycaon
2026-06-18 00:20:24 -06:00
parent feb04e8a37
commit a82697de7d
10 changed files with 390 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
python/robomp/web/src/components/IssueCard.tsx
+3 -3
View File
@@ -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}
/>
</Show>
<MetaRow item={item()} />
@@ -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))
}
>
<div class="rmp-card-actions">
@@ -64,7 +64,7 @@ export function IssueCard(props: IssueCardProps): JSX.Element {
retry
</button>
</Show>
<Show when={item().bucket === "running" && item().deliveryId}>
<Show when={item().bucket === "running" && item().live !== null && item().deliveryId}>
<button class="tiny danger" onClick={() => void cancel(item().deliveryId)}>
cancel
</button>
python/robomp/web/src/components/views/Activity.tsx
+21 -3
View File
@@ -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 (
<>
<Events onRetry={handleRetry} />
<Show when={triggerStatus().text}>
<span
class={`rmp-activity-status ${STATUS_TONE[triggerStatus().kind]}`}
role={triggerStatus().kind === "err" ? "alert" : "status"}
>
{triggerStatus().text}
</span>
</Show>
<Logs />
</>
);
python/robomp/web/src/styles/index.css
+8
View File
@@ -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;
python/robomp/web/src/types.ts
+1
View File
@@ -67,6 +67,7 @@ export interface RecentEvent {
attempts: number;
received_at: string;
last_error: string | null;
issue_state: IssueState | null;
}
export interface StatusResponse {
python/robomp/web/src/work-items.test.ts
+160 -6
View File
@@ -96,6 +96,7 @@ function recentEvent(overrides: Partial<RecentEvent> = {}): 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",
});
});
});
python/robomp/web/src/work-items.ts
+20 -7
View File
@@ -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<string, RecentEvent>();
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,