From 8e69ac185ea7d3b5964ea25a4c3104e0e35b3aef Mon Sep 17 00:00:00 2001
From: Ken <ken@kenotron.com>
Date: Tue, 26 May 2026 19:46:38 +0000
Subject: [PATCH] fix: address code review findings from Phase 2 final review

- ChatPanel: add 'chat-message ${role}' CSS classes to message wrappers
  so .chat-message.user and .chat-message.assistant rules apply correctly
- LoginPage: rename className 'login-error' -> 'error' to match the
  .login-card .error selector in globals.css
- MCPAppRenderer: verify event.source matches iframe contentWindow before
  processing postMessage to close cross-window injection vector
- App: remove redundant await refresh() after createNew() since
  createNew() already calls refresh() internally
- useArtifacts: wrap listArtifacts call in try/catch with console.error,
  matching the error-handling pattern already used in useSessions
---
 frontend/src/App.tsx                       | 1 -
 frontend/src/components/ChatPanel.tsx      | 2 +-
 frontend/src/components/LoginPage.tsx      | 2 +-
 frontend/src/components/MCPAppRenderer.tsx | 1 +
 frontend/src/hooks/useArtifacts.ts         | 8 ++++++--
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx
index fb5dd5f..99bdf0a 100644
--- a/frontend/src/App.tsx
+++ b/frontend/src/App.tsx
@@ -31,7 +31,6 @@ function App() {
         onSelect={(id) => switchTo(id)}
         onCreate={async () => {
           await createNew()
-          await refresh()
         }}
         onLogout={logout}
       />
diff --git a/frontend/src/components/ChatPanel.tsx b/frontend/src/components/ChatPanel.tsx
index d44d597..16e256e 100644
--- a/frontend/src/components/ChatPanel.tsx
+++ b/frontend/src/components/ChatPanel.tsx
@@ -58,7 +58,7 @@ export function ChatPanel({ messages, isStreaming, onSend, sessionId }: ChatPane
     <div className='chat-panel'>
       <div className='chat-messages'>
         {messages.map(message => (
-          <div key={message.id}>
+          <div key={message.id} className={`chat-message ${message.role}`}>
             {message.role === 'assistant' ? (
               <div>
                 <ReactMarkdown>{message.content}</ReactMarkdown>
diff --git a/frontend/src/components/LoginPage.tsx b/frontend/src/components/LoginPage.tsx
index a5ea3c9..a5d38a3 100644
--- a/frontend/src/components/LoginPage.tsx
+++ b/frontend/src/components/LoginPage.tsx
@@ -28,7 +28,7 @@ export function LoginPage({ onLogin }: LoginPageProps) {
     <div className="login-page">
       <form className="login-card" onSubmit={handleSubmit}>
         <h2>Research Workbench</h2>
-        {error && <div className="login-error">{error}</div>}
+        {error && <div className="error">{error}</div>}
         <label htmlFor="email">Email</label>
         <input
           id="email"
diff --git a/frontend/src/components/MCPAppRenderer.tsx b/frontend/src/components/MCPAppRenderer.tsx
index d6fee2e..0708b66 100644
--- a/frontend/src/components/MCPAppRenderer.tsx
+++ b/frontend/src/components/MCPAppRenderer.tsx
@@ -53,6 +53,7 @@ export function MCPAppRenderer({
   const handleMessage = useCallback(
     (event: MessageEvent) => {
       if (!iframeRef.current?.contentWindow) return
+      if (event.source !== iframeRef.current.contentWindow) return
 
       const msg = event.data
       if (!msg || msg.jsonrpc !== '2.0') return
diff --git a/frontend/src/hooks/useArtifacts.ts b/frontend/src/hooks/useArtifacts.ts
index 10f8fcf..b7dac7d 100644
--- a/frontend/src/hooks/useArtifacts.ts
+++ b/frontend/src/hooks/useArtifacts.ts
@@ -8,8 +8,12 @@ export function useArtifacts(sessionId: string | null) {
 
   async function refresh() {
     if (!sessionId) return
-    const data = await api.listArtifacts(sessionId)
-    setArtifacts(data)
+    try {
+      const data = await api.listArtifacts(sessionId)
+      setArtifacts(data)
+    } catch (err) {
+      console.error('Failed to list artifacts:', err)
+    }
   }
 
   async function loadArtifact(name: string) {