From 294f1aed731d347fcc8a7e7ae5e4855a6f0b8921 Mon Sep 17 00:00:00 2001 From: LHagfoss Date: Sun, 15 Nov 2026 15:40:52 +0100 Subject: [PATCH] fix(desktop): take a seen failure out of Needs you Opening a thread cleared its failed mark, but Needs you also builds a failed row from a transcript whose last turn failed, so the row stayed after the thread was opened and its banner dismissed. Opening the thread or dismissing its banner now records the failure as seen, the thread on screen has no row for its replayed failure, and a later failure is a row again. The question card's Esc tooltip says Decline, which is what it does. --- apps/desktop/src/state/inbox.test.ts | 14 +++++++++++ apps/desktop/src/state/inbox.ts | 8 +++++- apps/desktop/src/state/inboxRead.ts | 4 ++- apps/desktop/src/state/store.test.ts | 35 ++++++++++++++++++++++++++ apps/desktop/src/state/store.ts | 24 +++++++++++++++++- apps/desktop/src/ui/AttentionCards.tsx | 4 +-- apps/desktop/src/ui/ChatView.tsx | 3 +++ 7 files changed, 87 insertions(+), 5 deletions(-) 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(); }} />