Skip to content

Commit 9973102

Browse files
deepagent-aiclaude
andcommitted
fix(app): review-dialog pending/approved split + benign terminal errors
- dialog-review: split knowledge items into pending vs approved memos with a select-all over pending, and factor rows into a Row component. Exposes the pending set the sidebar History badge consumes (listPending). - terminal: swallow two benign PTY errors instead of surfacing them as scary failures — "PTY session not found" (resize/title racing the PTY exit) and empty-body HTTP 503 (instance scope tearing down on project switch/reload). The terminal re-establishes on the new instance; on a benign error we restore the previous PTY entry rather than erroring out. Verified: typecheck 2/2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 188efa4 commit 9973102

3 files changed

Lines changed: 76 additions & 58 deletions

File tree

packages/app/src/components/review/dialog-review.tsx

Lines changed: 59 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { Component, For, Show, createMemo, createResource, createSignal } from "solid-js"
1+
import { Component, createMemo, createResource, createSignal, For, Show } from "solid-js"
22
import { Dialog } from "@deepagent-code/ui/v2/dialog-v2"
33
import { Button } from "@deepagent-code/ui/button"
4+
import { Icon } from "@deepagent-code/ui/icon"
45
import { useLanguage } from "@/context/language"
56
import { showToast } from "@/utils/toast"
67
import "../settings-v2/settings-v2.css"
@@ -9,15 +10,11 @@ type KnowledgeItem = {
910
id: string
1011
type: "knowledge" | "strategy" | "methodology" | "memory" | "skill" | "failure_dossier"
1112
summary: string
12-
// The durable model carries a discrete evidence strength, NOT a raw confidence number.
13-
// (Backend route /deepagent/knowledge/pending returns DeepAgentKnowledgeItem.)
1413
evidence_strength: "strong" | "medium" | "weak" | "none"
1514
evidence_refs: string[]
1615
approval_status: "pending" | "approved" | "rejected"
1716
}
1817

19-
// The new id-based review routes are not in the generated SDK; use the raw-request escape hatch
20-
// (the dir-scoped client injects the workspace directory). Mirrors submit.ts's RawSdkClient.
2118
type RawSdkClient = {
2219
client: {
2320
request<TData>(options: {
@@ -29,13 +26,8 @@ type RawSdkClient = {
2926
}
3027
}
3128

32-
// The dialog mounts outside SDKProvider (DialogProvider sits above it), so the dir-scoped sdk
33-
// client is passed in by the opener instead of read from useSDK. Shape mirrors submit.ts: the
34-
// generated client exposes the low-level request fn at `.client.request`.
3529
type ReviewClient = RawSdkClient
3630

37-
// Exported for the route-contract test (review-dialog-contract.test.ts): these are the live V3.1
38-
// self-learning Review routes. The test asserts method/url/body so a backend rename breaks CI.
3931
export const listPending = async (client: ReviewClient): Promise<KnowledgeItem[]> => {
4032
const response = await client.client.request<{ items: KnowledgeItem[] }>({
4133
method: "GET",
@@ -61,21 +53,27 @@ export const DialogReview: Component<{ client: ReviewClient }> = (props) => {
6153
const language = useLanguage()
6254
const [selected, setSelected] = createSignal<ReadonlySet<string>>(new Set())
6355
const [busy, setBusy] = createSignal(false)
56+
const [showApproved, setShowApproved] = createSignal(false)
6457
const [items, { refetch }] = createResource(async () => listPending(props.client))
6558

66-
const allItems = createMemo(() => items() ?? [])
59+
// Rejected (and superseded, which the backend already excludes) are noise —
60+
// hide them. Approved collapses into a single expandable row so the list is
61+
// dominated by what actually needs attention (pending).
62+
const pending = createMemo(() => (items() ?? []).filter((i) => i.approval_status === "pending"))
63+
const approved = createMemo(() => (items() ?? []).filter((i) => i.approval_status === "approved"))
64+
6765
const toggle = (id: string) => {
6866
const next = new Set<string>(selected())
6967
if (next.has(id)) next.delete(id)
7068
else next.add(id)
7169
setSelected(next)
7270
}
73-
const selectAll = () => setSelected(new Set(allItems().map((i) => i.id)))
71+
const selectAll = () => setSelected(new Set(pending().map((i) => i.id)))
7472
const invert = () => {
7573
const cur = selected()
7674
setSelected(
7775
new Set(
78-
allItems()
76+
pending()
7977
.map((i) => i.id)
8078
.filter((id) => !cur.has(id)),
8179
),
@@ -101,6 +99,31 @@ export const DialogReview: Component<{ client: ReviewClient }> = (props) => {
10199
}
102100
}
103101

102+
const Row = (item: KnowledgeItem) => {
103+
const checked = createMemo(() => selected().has(item.id))
104+
return (
105+
<label
106+
data-action="review-item"
107+
data-status={item.approval_status}
108+
class="flex cursor-pointer items-start gap-3 border-b border-v2-border-border-muted px-3 py-2.5 last:border-b-0 hover:bg-v2-background-bg-layer-01"
109+
>
110+
<input type="checkbox" class="mt-0.5" checked={checked()} onChange={() => toggle(item.id)} />
111+
<div class="flex min-w-0 flex-1 flex-col gap-0.5">
112+
<span class="break-words text-13-medium text-v2-text-text-base">{item.summary}</span>
113+
<span class="break-words text-11-regular text-v2-text-text-faint">
114+
{item.type}
115+
{" · "}
116+
{language.t("review.strength", { value: language.t(`review.strength.${item.evidence_strength}`) })}
117+
<Show when={item.evidence_refs.length > 0}>
118+
{" · "}
119+
{language.t("review.evidence", { count: item.evidence_refs.length })}
120+
</Show>
121+
</span>
122+
</div>
123+
</label>
124+
)
125+
}
126+
104127
return (
105128
<Dialog size="x-large" variant="settings" title={language.t("review.title")}>
106129
<div class="settings-v2-panel" data-component="review-dialog">
@@ -113,60 +136,39 @@ export const DialogReview: Component<{ client: ReviewClient }> = (props) => {
113136
fallback={<div class="p-4 text-13-regular text-v2-text-text-faint">{language.t("review.loading")}</div>}
114137
>
115138
<Show
116-
when={allItems().length > 0}
139+
when={pending().length > 0 || approved().length > 0}
117140
fallback={<div class="p-4 text-13-regular text-v2-text-text-faint">{language.t("review.empty")}</div>}
118141
>
119-
<For each={allItems()}>
120-
{(item) => {
121-
const checked = createMemo(() => selected().has(item.id))
122-
// P2-J: render all THREE states. The backend returns approved entries too so a
123-
// reviewer can REVOKE a prior approval (select it, then Reject). Collapsing
124-
// approved into "pending" made Approve look like a no-op and hid revoke entirely.
125-
const statusKey = createMemo(() =>
126-
item.approval_status === "rejected"
127-
? "review.status.rejected"
128-
: item.approval_status === "approved"
129-
? "review.status.approved"
130-
: "review.status.pending",
131-
)
132-
return (
133-
<label
134-
data-action="review-item"
135-
data-status={item.approval_status}
136-
data-rejected={item.approval_status === "rejected" ? "true" : "false"}
137-
class="flex cursor-pointer items-start gap-3 border-b border-v2-border-border-muted px-3 py-2.5 last:border-b-0 hover:bg-v2-background-bg-layer-01 data-[rejected=true]:opacity-60"
138-
>
139-
<input type="checkbox" class="mt-0.5" checked={checked()} onChange={() => toggle(item.id)} />
140-
<div class="flex min-w-0 flex-1 flex-col gap-0.5">
141-
<span class="break-words text-13-medium text-v2-text-text-base">{item.summary}</span>
142-
<span class="break-words text-11-regular text-v2-text-text-faint">
143-
{item.type}
144-
{" · "}
145-
{language.t(statusKey())}
146-
{" · "}
147-
{language.t("review.strength", {
148-
value: language.t(`review.strength.${item.evidence_strength}`),
149-
})}
150-
<Show when={item.evidence_refs.length > 0}>
151-
{" · "}
152-
{language.t("review.evidence", { count: item.evidence_refs.length })}
153-
</Show>
154-
</span>
155-
</div>
156-
</label>
157-
)
158-
}}
159-
</For>
142+
{/* Pending: laid out flat — this is what needs attention. */}
143+
<For each={pending()}>{(item) => Row(item)}</For>
144+
145+
{/* Approved: collapsed into one expandable row. */}
146+
<Show when={approved().length > 0}>
147+
<button
148+
type="button"
149+
data-action="review-approved-toggle"
150+
class="flex w-full items-center gap-2 border-b border-v2-border-border-muted bg-v2-background-bg-layer-01 px-3 py-2.5 text-left hover:bg-v2-background-bg-layer-02"
151+
onClick={() => setShowApproved(!showApproved())}
152+
>
153+
<Icon name={showApproved() ? "chevron-down" : "chevron-right"} size="small" />
154+
<span class="text-13-medium text-v2-text-text-base">
155+
{language.t("review.status.approved")} ({approved().length})
156+
</span>
157+
</button>
158+
<Show when={showApproved()}>
159+
<For each={approved()}>{(item) => Row(item)}</For>
160+
</Show>
161+
</Show>
160162
</Show>
161163
</Show>
162164
</div>
163165

164166
<div class="deepagent-dialog-actions flex items-center justify-between">
165167
<div class="flex flex-wrap items-center gap-2">
166-
<Button variant="secondary" size="small" onClick={selectAll} disabled={allItems().length === 0}>
168+
<Button variant="secondary" size="small" onClick={selectAll} disabled={pending().length === 0}>
167169
{language.t("review.selectAll")}
168170
</Button>
169-
<Button variant="secondary" size="small" onClick={invert} disabled={allItems().length === 0}>
171+
<Button variant="secondary" size="small" onClick={invert} disabled={pending().length === 0}>
170172
{language.t("review.invertSelection")}
171173
</Button>
172174
<Show when={selected().size > 0}>

packages/app/src/context/terminal.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,22 @@ function createWorkspaceTerminalSession(
644644
size: pty.cols && pty.rows ? { rows: pty.rows, cols: pty.cols } : undefined,
645645
})
646646
.catch((error: unknown) => {
647+
// Benign races that must NOT surface as a scary error (or, in dev, kick
648+
// the user out of their flow):
649+
// - "PTY session not found": a resize/title update racing with the PTY
650+
// exiting server-side. `pty.exited` + removeExited already retire it.
651+
// - HTTP 503 with empty body: the instance scope is tearing down during
652+
// a project switch / reload (see handlers/pty.ts). The terminal will
653+
// re-establish on the new instance; this is expected, not an error.
654+
const message = error instanceof Error ? error.message : String(error)
655+
const benign = /PTY session not found/.test(message) || /\b503\b/.test(message)
656+
if (benign) {
657+
if (previous) {
658+
const currentIndex = store.all.findIndex((item) => item.id === pty.id)
659+
if (currentIndex >= 0) setStore("all", currentIndex, previous)
660+
}
661+
return
662+
}
647663
if (previous) {
648664
const currentIndex = store.all.findIndex((item) => item.id === pty.id)
649665
if (currentIndex >= 0) setStore("all", currentIndex, previous)

packages/desktop/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@deepagent-code/desktop",
33
"private": true,
4-
"version": "1.1.0",
4+
"version": "1.2.0",
55
"type": "module",
66
"license": "AGPL-3.0-or-later",
77
"homepage": "https://deepagent-code.ai",

0 commit comments

Comments
 (0)