OpenClaw streaming, gateway auth, and UI improvements#50
Conversation
Overhaul the OpenClaw runtime to track agent lifecycle phases and prevent premature finalization on intermediate chat:final events. Add gateway device-auth with token caching, session bridging, and v3 protocol handshake. Improve history reconciliation with a wider optimistic dedup window and stable ID assignment. Introduce tool result plugin injection, turn activity box, detached document-scroll chat mode for iOS Safari, and numerous chat input and message rendering refinements.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Sorry @wende, your pull request is larger than the review limit of 150000 diff characters
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates to the OpenClaw gateway integration, including protocol v3 support with device-auth signature alignment between TypeScript and Swift. It implements a gateway-auth cache backed by Keychain on iOS and localStorage on web to persist device tokens across reconnects. A major architectural shift adds a 'document-scroll' mode to improve the iOS Safari experience, moving away from fixed-height viewport shells. The UI is enhanced with a new TurnActivityBox for consolidated thinking and tool-call display, alongside improved handling of accumulated text deltas to prevent visual jitter. Feedback focuses on critical performance concerns regarding expensive DOM measurements within the scroll loop and stability issues caused by using non-deterministic IDs for React keys. Additionally, the reviewer identified a missing cleanup mechanism for timeouts in the activity feed and redundant styling logic in the chat input component.
|
|
||
| return { | ||
| type: "plugin", | ||
| pluginType: "flow_list_card", |
There was a problem hiding this comment.
Using Date.now() for partId makes the ID unstable across renders. This causes React to treat the plugin as a new component on every history refresh or state update (e.g., during polling or after a run), leading to unnecessary unmounting/remounting, flickering, and loss of local component state. This issue also exists at lines 125 and 184. Consider passing and using the toolCallId from the source tool_call part to ensure a stable ID.
| const getDocumentChromeOffset = useCallback(() => { | ||
| if (!useDocumentScroll || typeof document === "undefined" || typeof window === "undefined") return 0; | ||
| const probe = document.createElement("div"); | ||
| probe.style.cssText = "position:fixed;left:-9999px;top:0;height:100vh;width:0;pointer-events:none;"; | ||
| document.body.appendChild(probe); | ||
| const layoutViewportHeight = probe.getBoundingClientRect().height; | ||
| probe.remove(); | ||
| return Math.max(0, layoutViewportHeight - window.innerHeight); | ||
| }, [useDocumentScroll]); |
There was a problem hiding this comment.
getDocumentChromeOffset performs expensive DOM operations (creating, appending, and removing an element) to measure the viewport. Since this function is called within the scroll event handler and the requestAnimationFrame loop, it will cause significant layout thrashing and performance degradation during scrolling. This value should be calculated once or cached in a useRef and only updated when the window is resized.
| const composerBackground = isPill | ||
| ? "oklch(from var(--background) l c h / 0.30)" | ||
| : "oklch(from var(--background) l c h / 0.30)"; |
There was a problem hiding this comment.
| useEffect(() => { | ||
| parts.forEach((part, i) => { | ||
| const prev = prevStatusRef.current[i]; | ||
| const curr = isToolCallPart(part) ? part.status : undefined; | ||
| if (curr === "running" && prev !== "running") { | ||
| setRowOpen((r) => { const n = [...r]; n[i] = true; return n; }); | ||
| } | ||
| if ((curr === "success" || curr === "error") && prev === "running") { | ||
| const idx = i; | ||
| setTimeout(() => setRowOpen((r) => { const n = [...r]; n[idx] = false; return n; }), 400); | ||
| } | ||
| }); | ||
| prevStatusRef.current = parts.map((p) => isToolCallPart(p) ? p.status : undefined); | ||
| }, [parts]); |
There was a problem hiding this comment.
The setTimeout used for auto-collapsing tool rows lacks a cleanup mechanism. If the component unmounts or the parts array changes before the timeout fires, it may attempt to update the state of an unmounted component or modify the wrong index in the rowOpen array. Consider tracking the timeout IDs and clearing them in the effect's cleanup function.
| useEffect(() => { | |
| parts.forEach((part, i) => { | |
| const prev = prevStatusRef.current[i]; | |
| const curr = isToolCallPart(part) ? part.status : undefined; | |
| if (curr === "running" && prev !== "running") { | |
| setRowOpen((r) => { const n = [...r]; n[i] = true; return n; }); | |
| } | |
| if ((curr === "success" || curr === "error") && prev === "running") { | |
| const idx = i; | |
| setTimeout(() => setRowOpen((r) => { const n = [...r]; n[idx] = false; return n; }), 400); | |
| } | |
| }); | |
| prevStatusRef.current = parts.map((p) => isToolCallPart(p) ? p.status : undefined); | |
| }, [parts]); | |
| useEffect(() => { | |
| const timers: number[] = []; | |
| parts.forEach((part, i) => { | |
| const prev = prevStatusRef.current[i]; | |
| const curr = isToolCallPart(part) ? part.status : undefined; | |
| if (curr === "running" && prev !== "running") { | |
| setRowOpen((r) => { const n = [...r]; n[i] = true; return n; }); | |
| } | |
| if ((curr === "success" || curr === "error") && prev === "running") { | |
| const idx = i; | |
| timers.push(window.setTimeout(() => setRowOpen((r) => { const n = [...r]; n[idx] = false; return n; }), 400)); | |
| } | |
| }); | |
| prevStatusRef.current = parts.map((p) => isToolCallPart(p) ? p.status : undefined); | |
| return () => timers.forEach(window.clearTimeout); | |
| }, [parts]); |
Summary
chat:finaleventsNote: App-specific plugin cards (
plugins/app/) are excluded from this PR — only the generic plugin infrastructure (lib/plugins/,lib/chat/toolResultPlugins.ts) is included.Test plan
pnpm test— all 317 tests pass