fix issue in review

Author: KylinMountain

frontend/src/App.tsx

@@ -31,16 +31,6 @@ function KbDetailRoute() {
   return <KbDetail key={id} />
 }
 
-/** Remount ChatSession per session id so its restore effect (which guards on a
- *  module-instance restoredRef and only depends on [id]) re-runs on a genuine
- *  navigation between sessions — otherwise back/forward or a hash edit between
- *  /chat/A and /chat/B changes the id without a remount and shows the stale
- *  session. */
-function ChatSessionRoute() {
-  const { id = "" } = useParams()
-  return <ChatSession key={id} />
-}
-
 const inputCls =
   "mt-1.5 w-full h-9 rounded-md border border-input bg-transparent px-3 text-[13px] font-mono2 outline-none focus-visible:ring-2 focus-visible:ring-ring focus:border-accent-brand"
 
@@ -175,7 +165,14 @@ export default function App() {
             </div>
             <Routes>
               <Route path="/" element={<Home />} />
-              <Route path="/chat/:id" element={<ChatSessionRoute />} />
+              {/* No key={id}: ChatSession must NOT remount when runTurn adopts a
+                  real session id mid-turn (the new→/chat/<sid> self-navigate) —
+                  a remount there would abort the live stream and drop the
+                  just-finished turn's artifact cards. Its restore effect instead
+                  re-runs on an `id` change and reloads only when navigating to a
+                  DIFFERENT saved session, so switching /chat/A→/chat/B still
+                  shows the right one. */}
+              <Route path="/chat/:id" element={<ChatSession />} />
               <Route path="/kb" element={<KbList />} />
               <Route path="/kb/:id" element={<KbDetailRoute />} />
               <Route path="/settings" element={<Settings />} />

frontend/src/api/artifacts.ts

@@ -6,13 +6,18 @@ import { apiStream, fetchAsBlobUrl } from "./client"
  * event's data is `{ name, status, path }` (see `_iter_deck`/`_iter_skill` in
  * `openkb/api_helpers.py`). NOTE: this `final` shape differs from chat/query's
  * `final` — do not fold these through `mapSseEvent`/`foldSseEvent`.
+ *
+ * Pass an optional `signal` (threaded to the underlying `apiStream` fetch) to
+ * make the stream cancellable: aborting it disconnects the client, which the
+ * backend `_stream_deck`/`_stream_skill` handlers now honor. Backward-compatible
+ * — callers that omit it get an uncancellable stream exactly as before.
  */
-export function runDeckCommand(kb: string, name: string, intent: string) {
-  return apiStream("/api/v1/deck", { kb, name, intent, stream: true })
+export function runDeckCommand(kb: string, name: string, intent: string, signal?: AbortSignal) {
+  return apiStream("/api/v1/deck", { kb, name, intent, stream: true }, signal)
 }
 
-export function runSkillCommand(kb: string, name: string, intent: string) {
-  return apiStream("/api/v1/skill", { kb, name, intent, stream: true })
+export function runSkillCommand(kb: string, name: string, intent: string, signal?: AbortSignal) {
+  return apiStream("/api/v1/skill", { kb, name, intent, stream: true }, signal)
 }
 
 /**

frontend/src/api/client.ts

@@ -55,6 +55,12 @@ export function onUnauthorized(cb: () => void): void {
 
 export class ApiError extends Error {
   status: number
+  /** Structured `detail` payload when the backend returns an object rather than
+   *  a plain string — e.g. the 409 multiple-match `{ message, candidates }` from
+   *  `POST /api/v1/remove`. `message` is lifted onto `.message`; the whole object
+   *  is kept here so callers can read the structured fields (candidates, etc.).
+   *  `undefined` for string-detail errors (404) and network failures. */
+  detail?: unknown
   constructor(status: number, message: string) {
     super(message)
     this.status = status
@@ -82,13 +88,28 @@ export async function apiFetch<T>(path: string, opts: FetchOpts = {}): Promise<T
   if (res.status === 401) unauthorizedHandler?.()
   if (!res.ok) {
     let detail = `${res.status} ${res.statusText}`
+    let structured: unknown
     try {
       const j = await res.json()
-      detail = typeof j.detail === "string" ? j.detail : JSON.stringify(j.detail ?? j)
+      const d = j.detail
+      if (typeof d === "string") {
+        // Plain-string detail (e.g. 404 "Document not found.") — unchanged path.
+        detail = d
+      } else if (d && typeof d === "object" && typeof (d as { message?: unknown }).message === "string") {
+        // Structured detail (e.g. 409 multiple-match `{ message, candidates }`):
+        // surface the human message, and keep the object so callers can read the
+        // structured fields instead of seeing a raw JSON blob.
+        detail = (d as { message: string }).message
+        structured = d
+      } else {
+        detail = JSON.stringify(d ?? j)
+      }
     } catch {
       // keep default message
     }
-    throw new ApiError(res.status, detail)
+    const err = new ApiError(res.status, detail)
+    err.detail = structured
+    throw err
   }
 
   const ct = res.headers.get("content-type") || ""

frontend/src/api/maintenance.ts

@@ -46,8 +46,8 @@ export interface WatchStatus {
  * per-file UI signal, so they are collapsed away here.
  */
 export type UploadEvent =
-  | { type: "file_start"; original_name: string }
-  | { type: "file_done"; file: AddFileItem }
+  | { type: "file_start"; index: number; original_name: string }
+  | { type: "file_done"; index: number; file: AddFileItem }
   | { type: "final"; result: AddResult }
   | { type: "error"; message: string }
 
@@ -63,11 +63,22 @@ export type UploadEvent =
  *   `file_done`  → that file's `AddFileItem` (status added/skipped/failed),
  *   `final`      → the summary `AddResponse`,
  *   `error`      → a stream-level failure message.
+ *
+ * The backend emits per-file events strictly in upload order (one `file_start`
+ * then one `file_done` per file, `saved_uploads` order == `files` order), so
+ * `file_start`/`file_done` carry a running `index` the caller uses to correlate
+ * an event to its seeded row by position — robust to duplicate basenames, which
+ * name-matching would collide.
+ *
+ * Pass an optional `signal` to make the upload cancellable — aborting it rejects
+ * the in-flight `fetch`/`reader.read()` with an `AbortError` that propagates out
+ * (mirrors `apiStream`/`runRecompile`), so navigating away mid-upload cancels it.
  */
 export async function streamUpload(
   kb: string,
   files: File[],
   onEvent: (ev: UploadEvent) => void,
+  signal?: AbortSignal,
 ): Promise<void> {
   const form = new FormData()
   form.append("kb", kb)
@@ -78,6 +89,7 @@ export async function streamUpload(
     method: "POST",
     headers: token ? { Authorization: `Bearer ${token}` } : {},
     body: form,
+    signal,
   })
   if (!res.ok || !res.body) {
     let detail = `${res.status} ${res.statusText}`
@@ -93,6 +105,9 @@ export async function streamUpload(
   const reader = res.body.getReader()
   const decoder = new TextDecoder()
   let buf = ""
+  // Running cursor over the seeded rows: `file_start` advances it, the matching
+  // `file_done` reuses it. See the ordering note above.
+  let fileCursor = -1
   while (true) {
     const { done, value } = await reader.read()
     if (done) break
@@ -105,13 +120,21 @@ export async function streamUpload(
       const dataLine = lines.find((l) => l.startsWith("data: "))
       if (!eventLine || !dataLine) continue
       const event = eventLine.slice("event: ".length)
-      const data = JSON.parse(dataLine.slice("data: ".length))
+      let data: any
+      try {
+        data = JSON.parse(dataLine.slice("data: ".length))
+      } catch {
+        // Malformed / non-JSON data frame — skip it like a robust SSE reader
+        // instead of throwing out of the whole read loop.
+        continue
+      }
       switch (event) {
         case "file_start":
-          onEvent({ type: "file_start", original_name: data.original_name })
+          fileCursor += 1
+          onEvent({ type: "file_start", index: fileCursor, original_name: data.original_name })
           break
         case "file_done":
-          onEvent({ type: "file_done", file: data as AddFileItem })
+          onEvent({ type: "file_done", index: fileCursor, file: data as AddFileItem })
           break
         case "final":
           onEvent({ type: "final", result: data as AddResult })
@@ -125,11 +148,15 @@ export async function streamUpload(
   }
 }
 
-/** `/api/v1/remove` response (`RemoveResponse`) — the subset the UI reads. */
+/** `/api/v1/remove` response (`RemoveResponse`) — the subset the UI reads.
+ *  `status` is `removed` on full success or `partial` when the local wiki files
+ *  were removed but PageIndex cleanup failed (both are HTTP 200); on `partial`,
+ *  `pageindex_error` carries why the remote cleanup failed. */
 export interface RemoveResult {
   status: string
   name?: string | null
   doc_name?: string | null
+  pageindex_error?: string | null
   message?: string | null
 }
 

frontend/src/components/MarkdownView.tsx

@@ -5,21 +5,37 @@ import { useTheme } from '@/lib/theme'
 
 /** Guard for [text](url) links: only render a real anchor for http(s) or
  *  scheme-less (relative) URLs. Anything with another scheme (javascript:,
- *  data:, vbscript:, …) is treated as literal text, never an anchor. */
-const isSafeUrl = (u: string) => /^https?:\/\//i.test(u) || !/^[a-z][\w+.-]*:/i.test(u)
+ *  data:, vbscript:, …) is treated as literal text, never an anchor.
+ *
+ *  Browsers strip leading/embedded control chars & whitespace from a URL before
+ *  resolving it, so ` javascript:x` and `java\tscript:x` both execute. We mirror
+ *  that: collapse ALL whitespace/control chars, then only permit an explicit
+ *  `scheme:` when it is http(s). The caller also trims the raw URL for the href. */
+const isSafeUrl = (u: string) => {
+  const s = u.replace(/[\u0000-\u0020\u007f-\u009f]/g, '')
+  const scheme = /^[a-z][a-z0-9+.-]*:/i.exec(s)
+  return scheme ? /^https?:/i.test(scheme[0]) : true
+}
 
 /**
  * Minimal inline Markdown renderer. Tokens (in match-priority order):
  *   [[wikilink|alias]] · \( math \) · **bold** · ~~strike~~ · `code` ·
  *   [text](url) · *italic* / _italic_.
  * ORDER IS LOAD-BEARING inside the alternation: wikilink must precede the link
  * pattern (so `[[x]]` is not parsed as a link), and bold must precede italic
- * (so `**x**` is not parsed as `*`-italic-`*`). Every alternative uses a bounded
- * `[^…]+` class (no nested quantifiers) to avoid catastrophic backtracking.
+ * (so `**x**` is not parsed as `*`-italic-`*`). Alternatives use bounded `[^…]+`
+ * classes or a single non-greedy `.+?` (bold/math) — no nested quantifiers — so
+ * there is no catastrophic backtracking. The inner text of bold/italic/strike
+ * and a link's LABEL are re-run through inline() so nested markup resolves; the
+ * recursed slice is always strictly shorter, which bounds the recursion (the
+ * wikilink target is NOT recursed). Single `*`/`_` obey a minimal left/right
+ * flanking rule so intraword runs (`a_b_c`, `2*3*4`) stay literal.
  */
 function inline(text: string, onWikiLink?: (target: string) => void): React.ReactNode[] {
   const parts: React.ReactNode[] = []
-  const re = /(\[\[[^\]]+\]\]|\\\([\s\S]+?\\\)|\*\*[^*]+\*\*|~~[^~]+~~|`[^`]+`|\[[^\]]+\]\([^)]+\)|\*[^*]+\*|_[^_]+_)/g
+  // Bold is non-greedy (`.+?`) so it tolerates an inner opposite-emphasis
+  // (`**a *b* c**`); italic keeps a bounded `[^*]+`/`[^_]+` class.
+  const re = /(\[\[[^\]]+\]\]|\\\([\s\S]+?\\\)|\*\*.+?\*\*|~~[^~]+~~|`[^`]+`|\[[^\]]+\]\([^)]+\)|\*[^*]+\*|_[^_]+_)/g
   let last = 0, m: RegExpExecArray | null, k = 0
   while ((m = re.exec(text))) {
     if (m.index > last) parts.push(text.slice(last, m.index))
@@ -68,9 +84,10 @@ function inline(text: string, onWikiLink?: (target: string) => void): React.Reac
         ),
       )
     } else if (tok.startsWith('**')) {
-      parts.push(<strong key={k++} className="font-semibold text-foreground">{tok.slice(2, -2)}</strong>)
+      // Recurse so nested markup (e.g. `**[docs](url)**`) resolves. slice is strictly shorter.
+      parts.push(<strong key={k++} className="font-semibold text-foreground">{inline(tok.slice(2, -2), onWikiLink)}</strong>)
     } else if (tok.startsWith('~~')) {
-      parts.push(<del key={k++} className="line-through">{tok.slice(2, -2)}</del>)
+      parts.push(<del key={k++} className="line-through">{inline(tok.slice(2, -2), onWikiLink)}</del>)
     } else if (tok.startsWith('`')) {
       parts.push(<code key={k++} className="font-mono2 text-[12px] bg-muted rounded px-1 py-px">{tok.slice(1, -1)}</code>)
     } else if (tok.startsWith('[')) {
@@ -79,7 +96,10 @@ function inline(text: string, onWikiLink?: (target: string) => void): React.Reac
       // passes the scheme guard; otherwise render the raw token as literal text.
       const lm = /^\[([^\]]+)\]\(([^)]+)\)$/.exec(tok)
       const label = lm ? lm[1] : tok
-      const url = lm ? lm[2] : ''
+      // Trim the captured URL BEFORE the guard so ` javascript:x` can't slip
+      // past the scheme test (browsers trim & would run it). Same trimmed
+      // value is used for href. The label is recursed so `[**x**](url)` works.
+      const url = lm ? lm[2].trim() : ''
       parts.push(
         lm && isSafeUrl(url) ? (
           <a
@@ -89,15 +109,33 @@ function inline(text: string, onWikiLink?: (target: string) => void): React.Reac
             rel="noopener noreferrer"
             className="text-accent-brand hover:underline"
           >
-            {label}
+            {inline(label, onWikiLink)}
           </a>
         ) : (
           <span key={k++}>{tok}</span>
         ),
       )
     } else {
-      // *italic* or _italic_ (bold `**…**` was consumed above).
-      parts.push(<em key={k++} className="italic">{tok.slice(1, -1)}</em>)
+      // *italic* / _italic_ (bold `**…**` was consumed above). Enforce a
+      // minimal CommonMark-ish flanking rule so intraword runs stay literal:
+      // the delimiter may only OPEN when the preceding char isn't a word char
+      // (and the run doesn't start with whitespace), and only CLOSE when the
+      // following char isn't a word char (and the run doesn't end with
+      // whitespace). This keeps `a_b_c` / `2*3*4` as plain text.
+      const before = m.index > 0 ? text[m.index - 1] : ''
+      const after = text[m.index + tok.length] ?? ''
+      const innerEm = tok.slice(1, -1)
+      const openable = !/\w/.test(before) && !/^\s/.test(innerEm)
+      const closable = !/\w/.test(after) && !/\s$/.test(innerEm)
+      if (!openable || !closable) {
+        // Not a real emphasis run: emit the opening delimiter literally and
+        // resume scanning right after it (inner text may still hold tokens).
+        parts.push(tok[0])
+        last = m.index + 1
+        re.lastIndex = m.index + 1
+        continue
+      }
+      parts.push(<em key={k++} className="italic">{inline(innerEm, onWikiLink)}</em>)
     }
     last = m.index + tok.length
   }
@@ -191,6 +229,8 @@ export default function MarkdownView({
   const out: React.ReactNode[] = []
   let list: string[] = []
   let olist: string[] = []
+  // Source number of the first `<ol>` item, so a list starting at N≠1 keeps it.
+  let olistStart = 1
   let key = 0
 
   const flushList = () => {
@@ -211,7 +251,7 @@ export default function MarkdownView({
   const flushOList = () => {
     if (!olist.length) return
     out.push(
-      <ol key={key++} className="my-2.5 space-y-1.5 pl-6 list-decimal">
+      <ol key={key++} start={olistStart} className="my-2.5 space-y-1.5 pl-6 list-decimal">
         {olist.map((li, i) => (
           <li key={i} className="pl-1 text-[14px] leading-relaxed text-muted-foreground marker:text-muted-foreground">
             <span>{inline(li, onWikiLink)}</span>
@@ -220,6 +260,7 @@ export default function MarkdownView({
       </ol>,
     )
     olist = []
+    olistStart = 1
   }
 
   // Block boundary: any non-list line closes BOTH pending lists.
@@ -279,10 +320,23 @@ export default function MarkdownView({
       /^\s*\|?\s*:?-{1,}:?\s*(\|\s*:?-{1,}:?\s*)+\|?\s*$/.test(tblNext)
     ) {
       flushBlocks()
-      // Split a row into trimmed cells, dropping the empty cell an optional
-      // leading/trailing edge pipe produces (`| a | b |` → ['a','b']).
+      // Split a row into trimmed cells on unescaped, non-code `|`. A naive
+      // `split('|')` tears apart an escaped `\|` or a `|` inside an inline code
+      // span; we walk the string so `` `a|b` `` and `a \| b` survive as one
+      // cell. Drops the empty cell an optional leading/trailing edge pipe makes.
       const cells = (row: string) => {
-        const c = row.trim().split('|').map((s) => s.trim())
+        const t = row.trim()
+        const c: string[] = []
+        let cur = ''
+        let inCode = false
+        for (let j = 0; j < t.length; j++) {
+          const ch = t[j]
+          if (ch === '\\' && t[j + 1] === '|') { cur += '|'; j++; continue }
+          if (ch === '`') { inCode = !inCode; cur += ch; continue }
+          if (ch === '|' && !inCode) { c.push(cur.trim()); cur = ''; continue }
+          cur += ch
+        }
+        c.push(cur.trim())
         if (c.length && c[0] === '') c.shift()
         if (c.length && c[c.length - 1] === '') c.pop()
         return c
@@ -295,10 +349,25 @@ export default function MarkdownView({
         return l && r ? 'text-center' : r ? 'text-right' : l ? 'text-left' : ''
       })
       const alignOf = (col: number) => aligns[col] ?? ''
-      // Consume header + delimiter, then all contiguous non-blank `|` rows.
+      // A continuation line is a body row only when it is table-SHAPED, not
+      // merely a line that happens to contain a `|`. We key off the header's
+      // edge-pipe style: if the header had a leading/trailing `|`, body rows
+      // must match — so a trailing prose/list line with a stray `|` (and no
+      // blank separator) is NOT swallowed as a phantom row.
+      const hTrim = line.trim()
+      const edgeLead = hTrim.startsWith('|')
+      const edgeTrail = hTrim.endsWith('|')
+      const isRow = (l: string) => {
+        const t = l.trim()
+        if (!t.includes('|')) return false
+        if (edgeLead && !t.startsWith('|')) return false
+        if (edgeTrail && !t.endsWith('|')) return false
+        return true
+      }
+      // Consume header + delimiter, then all contiguous non-blank table rows.
       const body: string[][] = []
       i += 2
-      while (i < lines.length && lines[i].trim() && lines[i].includes('|')) {
+      while (i < lines.length && lines[i].trim() && isRow(lines[i])) {
         body.push(cells(lines[i]))
         i++
       }
@@ -341,9 +410,16 @@ export default function MarkdownView({
     // Unordered item: close any open ordered list, keep accumulating the ul.
     if (line.startsWith('- ')) { flushOList(); list.push(line.slice(2)); continue }
     // Ordered item `N. …`: close any open unordered list, strip the marker, and
-    // accumulate into a real <ol> (mirrors the ul buffer/flush pattern).
-    const om = /^\d+\.\s+/.exec(line)
-    if (om) { flushList(); olist.push(line.slice(om[0].length)); continue }
+    // accumulate into a real <ol> (mirrors the ul buffer/flush pattern). The
+    // first item's source number seeds `<ol start>` so a list beginning at N≠1
+    // keeps its numbering (a blank line still ends the list).
+    const om = /^(\d+)\.\s+/.exec(line)
+    if (om) {
+      flushList()
+      if (!olist.length) olistStart = parseInt(om[1], 10)
+      olist.push(line.slice(om[0].length))
+      continue
+    }
     flushBlocks()
     if (!line.trim()) { out.push(<div key={key++} className="h-2" />); continue }
     if (line.startsWith('### ')) out.push(<h3 key={key++} className="mt-4 mb-1.5 text-[14px] font-semibold text-foreground">{inline(line.slice(4), onWikiLink)}</h3>)