diff --git a/apps/desktop/src/state/inbox.test.ts b/apps/desktop/src/state/inbox.test.ts index 143a2ffe6..5d020939d 100644 --- a/apps/desktop/src/state/inbox.test.ts +++ b/apps/desktop/src/state/inbox.test.ts @@ -117,6 +117,20 @@ describe("the needs-you inbox", () => { ]); }); + test("the thread on screen has no row for its replayed failure", () => { + const items = deriveInbox( + { + sessions, + threads: { two: { permissions: [], items: [{ kind: "turnEnd", outcome: "failed", at: NOW - 500 }] } }, + failed: {}, + links: [], + viewing: "two", + }, + NOW, + ); + expect(items.filter((item) => item.kind === "failed")).toEqual([]); + }); + test("a settled thread's failed turn is no row, from either source", () => { const settled = sessions.map((s) => (s.id === "one" ? { ...s, status: "done" as const } : s)); const items = deriveInbox( diff --git a/apps/desktop/src/state/inbox.ts b/apps/desktop/src/state/inbox.ts index 8972d7a55..ee5c2c708 100644 --- a/apps/desktop/src/state/inbox.ts +++ b/apps/desktop/src/state/inbox.ts @@ -65,6 +65,12 @@ export interface InboxRead { snoozed?: Readonly>; /** Rows put away by hand, which stay away until their thread asks again. */ dismissed?: InboxDismissals; + /** + * The thread on screen. Its replayed failure is no row: the user is looking at it, and its + * banner already says it (a failure that lands while the window is elsewhere still comes in + * through `failed`). + */ + viewing?: string | null; } /** The title a thread is known by, when the app has one. */ @@ -160,7 +166,7 @@ export function deriveInbox(read: InboxRead, now = Date.now()): InboxItem[] { // A replayed thread whose last turn failed is a failure too, even when this app // sent no prompt: the mock seeds one, and a thread opened after a restart has one. for (const [threadId, thread] of Object.entries(read.threads)) { - if (failedThreads.has(threadId)) continue; + if (failedThreads.has(threadId) || threadId === read.viewing) continue; const tail = thread.items?.[thread.items.length - 1]; if (tail?.kind !== "turnEnd" || tail.outcome !== "failed") continue; items.push({ diff --git a/apps/desktop/src/state/inboxRead.ts b/apps/desktop/src/state/inboxRead.ts index 0e8425e9f..7d56e0f30 100644 --- a/apps/desktop/src/state/inboxRead.ts +++ b/apps/desktop/src/state/inboxRead.ts @@ -31,6 +31,7 @@ export function useInboxItems(): InboxItem[] { const archived = useApp((s) => s.archived); const dismissed = useApp((s) => s.inboxDismissed); const snoozed = useApp((s) => s.snoozed); + const viewing = useApp((s) => s.selectedId); const signature = useApp((s) => threadSignature(s.threads)); // biome-ignore lint/correctness/useExhaustiveDependencies: the signature stands for the threads it was built from return useMemo( @@ -42,9 +43,10 @@ export function useInboxItems(): InboxItem[] { archived, dismissed, snoozed, + viewing, threads: useApp.getState().threads, }), - [sessions, failed, links, archived, dismissed, snoozed, signature], + [sessions, failed, links, archived, dismissed, snoozed, viewing, signature], ); } diff --git a/apps/desktop/src/state/store.test.ts b/apps/desktop/src/state/store.test.ts index bbf789c82..66ad19a80 100644 --- a/apps/desktop/src/state/store.test.ts +++ b/apps/desktop/src/state/store.test.ts @@ -4038,6 +4038,41 @@ describe("finished threads", () => { markUnread("t1"); expect(Object.keys(useApp.getState().finished)).toEqual(["t1"]); }); + + test("opening a thread whose replayed last turn failed takes it out of Needs you, until it fails again", async () => { + const { clearFinished, seeFailure } = await import("./store"); + const { deriveInbox } = await import("./inbox"); + const failedAt = Date.now() - 5_000; + const thread = (at: number) => + ({ permissions: [], queue: [], items: [{ kind: "turnEnd", id: "e", outcome: "failed", at }] }) as never; + useApp.setState({ + selectedId: "other", + sessions: [{ id: "t1", title: "T1", cwd: "/tmp" }] as never, + threads: { t1: thread(failedAt) }, + failed: {}, + inboxDismissed: {}, + }); + const rows = () => { + const s = useApp.getState(); + return deriveInbox({ + sessions: s.sessions, + threads: s.threads, + failed: s.failed, + links: [], + dismissed: s.inboxDismissed, + }).filter((row) => row.threadId === "t1"); + }; + expect(rows()).toHaveLength(1); + clearFinished("t1"); + expect(rows()).toHaveLength(0); + // Seeing it again records nothing new. + const before = useApp.getState().inboxDismissed; + seeFailure("t1"); + expect(useApp.getState().inboxDismissed).toBe(before); + // The next failure arrives later, and is a row again. + useApp.setState({ threads: { t1: thread(Date.now() + 60_000) } }); + expect(rows()).toHaveLength(1); + }); }); test("a mode chosen in one thread leaves the default and the other threads alone", async () => { diff --git a/apps/desktop/src/state/store.ts b/apps/desktop/src/state/store.ts index 46b8399c4..8c5a7ed9b 100644 --- a/apps/desktop/src/state/store.ts +++ b/apps/desktop/src/state/store.ts @@ -2856,10 +2856,32 @@ function without(map: Record, id: string): Record ({ finished: without(s.finished, id), failed: without(s.failed, id) })); } +/** + * The thread's failure was seen: opened, or its banner dismissed. Its "Needs you" row is put away + * quietly, from both of its sources — the store's `failed` map and a replayed transcript that ends + * in a failed turn — and a later failure, which arrives after this moment, is a row again + * (state/inboxDismissed.ts `isDismissed`). A thread with no failure records nothing. + */ +export function seeFailure(id: string): void { + const s = get(); + const items = s.threads[id]?.items; + const tail = items?.[items.length - 1]; + const failedTail = tail?.kind === "turnEnd" && tail.outcome === "failed"; + if (!(id in s.failed) && !failedTail) return; + const key = `${id}:failed`; + const at = Math.max(s.failed[id] ?? 0, failedTail ? (tail.at ?? 0) : 0); + if ((s.inboxDismissed[key] ?? -1) >= at) return; + const next = dismissRows(s.inboxDismissed, [{ key, kind: "failed", at }]); + set({ inboxDismissed: next }); + store(STORAGE.inboxDismissed, JSON.stringify(next)); +} + /** The shortest gap between two window-focus list refreshes, see below. */ const FOCUS_LIST_GAP_MS = 5_000; let lastFocusListAt = 0; diff --git a/apps/desktop/src/ui/AttentionCards.tsx b/apps/desktop/src/ui/AttentionCards.tsx index fcb534e6e..3fcc43f47 100644 --- a/apps/desktop/src/ui/AttentionCards.tsx +++ b/apps/desktop/src/ui/AttentionCards.tsx @@ -517,7 +517,7 @@ function focusComposerField() { * draft, so the thread's context and the user's own words are where they were (issue 3696). It * says which of a set it is, the question (scrolling past `QUESTION_TEXT_MAX`), its answers with * their descriptions, and a field for the user's own answer. ↑/↓ move between answers, 1–9 pick - * one, esc dismisses when no field has focus. + * one, esc declines when no field has focus. * * A question that takes the user's own answer also takes files (issue 3756): Attach files, or a * paste or drop on the form, under the composer's own limits (`ui/Attachments.tsx`). A file goes @@ -825,7 +825,7 @@ export function QuestionCard({ Send {reject && ( - + answerPermission(request, reject)}>{reject.name} )} diff --git a/apps/desktop/src/ui/ChatView.tsx b/apps/desktop/src/ui/ChatView.tsx index f64e007dc..5b2b871f4 100644 --- a/apps/desktop/src/ui/ChatView.tsx +++ b/apps/desktop/src/ui/ChatView.tsx @@ -47,6 +47,7 @@ import { refreshSessions, repairThread, retryPendingSetup, + seeFailure, setAutoCompactForThread, setPanel, setTerminalOpen, @@ -1008,6 +1009,8 @@ export function ChatView() { } onDismiss={() => { setDismissed(error); + // Dismissed here is seen: its "Needs you" row goes with it. + if (thread) seeFailure(thread.id); void refreshSessions(); }} />