Skip to content

Commit c674b9d

Browse files
ambient-code[bot]claude
authored andcommitted
fix: address review feedback
- Fix onSubmitAnswer type signature to return Promise<void> for proper error handling - Add null safety guard for tool call function name access - Add type="button" to tab navigation to prevent form submission - Make AskUserQuestion handleSubmit async with loading state - Include missing/empty agentStatus in 5s polling tier - Re-throw errors in sendToolAnswer for component error handling - Fix namespace collision in event store by using namespace-qualified session IDs Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 957bc96 commit c674b9d

8 files changed

Lines changed: 38 additions & 19 deletions

File tree

components/backend/handlers/sessions.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ var (
4343
DeriveRepoFolderFromURL func(string) string
4444
// DeriveAgentStatusFromEvents derives agentStatus from the persisted event log.
4545
// Set by the websocket package at init to avoid circular imports.
46+
// sessionID should be namespace-qualified (e.g., "namespace/sessionName") to avoid cross-project collisions.
4647
DeriveAgentStatusFromEvents func(sessionID string) string
4748
// LEGACY: SendMessageToSession removed - AG-UI server uses HTTP/SSE instead of WebSocket
4849
)
@@ -375,10 +376,13 @@ func enrichAgentStatus(session *types.AgenticSession) {
375376
return
376377
}
377378
name, _ := session.Metadata["name"].(string)
378-
if name == "" {
379+
namespace, _ := session.Metadata["namespace"].(string)
380+
if name == "" || namespace == "" {
379381
return
380382
}
381-
if derived := DeriveAgentStatusFromEvents(name); derived != "" {
383+
// Use namespace-qualified key to avoid cross-project collisions in the event store
384+
sessionID := namespace + "/" + name
385+
if derived := DeriveAgentStatusFromEvents(sessionID); derived != "" {
382386
session.Status.AgentStatus = types.StringPtr(derived)
383387
}
384388
}

components/backend/websocket/agui_proxy.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -257,10 +257,13 @@ func HandleAGUIRunProxy(c *gin.Context) {
257257

258258
log.Printf("AGUI Proxy: run=%s session=%s/%s msgs=%d", truncID(runID), projectName, sessionName, len(rawMessages))
259259

260+
// Use namespace-qualified session ID to avoid cross-project collisions
261+
namespacedSessionID := projectName + "/" + sessionName
262+
260263
sessionLastSeen.Store(sessionName, time.Now())
261264

262265
// Store project→session mapping for activity tracking in persistStreamedEvent
263-
sessionProjectMap.Store(sessionName, projectName)
266+
sessionProjectMap.Store(namespacedSessionID, projectName)
264267

265268
// Resolve and cache the runner port for this session from the registry.
266269
cacheSessionPort(projectName, sessionName)
@@ -297,7 +300,7 @@ func HandleAGUIRunProxy(c *gin.Context) {
297300
runnerURL := getRunnerEndpoint(projectName, sessionName)
298301

299302
// Start background goroutine to proxy runner SSE → persist + broadcast
300-
go proxyRunnerStream(runnerURL, bodyBytes, sessionName, runID, threadID)
303+
go proxyRunnerStream(runnerURL, bodyBytes, sessionName, namespacedSessionID, runID, threadID)
301304

302305
// Return metadata immediately — events arrive via GET /agui/events
303306
c.JSON(http.StatusOK, gin.H{
@@ -309,21 +312,22 @@ func HandleAGUIRunProxy(c *gin.Context) {
309312
// proxyRunnerStream connects to the runner's SSE endpoint, reads events,
310313
// persists them, and publishes them to the live broadcast pipe. Runs in
311314
// a background goroutine so the POST /agui/run handler can return immediately.
312-
func proxyRunnerStream(runnerURL string, bodyBytes []byte, sessionName, runID, threadID string) {
315+
// namespacedSessionID is the namespace-qualified session ID (e.g., "namespace/sessionName") for event persistence.
316+
func proxyRunnerStream(runnerURL string, bodyBytes []byte, sessionName, namespacedSessionID, runID, threadID string) {
313317
log.Printf("AGUI Proxy: connecting to runner at %s", runnerURL)
314318
resp, err := connectToRunner(runnerURL, bodyBytes)
315319
if err != nil {
316320
log.Printf("AGUI Proxy: runner unavailable for %s: %v", sessionName, err)
317321
// Publish error events so GET /agui/events subscribers see the failure
318-
publishAndPersistErrorEvents(sessionName, runID, threadID, "Runner is not available")
322+
publishAndPersistErrorEvents(sessionName, namespacedSessionID, runID, threadID, "Runner is not available")
319323
return
320324
}
321325
defer resp.Body.Close()
322326

323327
if resp.StatusCode != http.StatusOK {
324328
body, _ := io.ReadAll(resp.Body)
325329
log.Printf("AGUI Proxy: runner returned %d: %s", resp.StatusCode, string(body))
326-
publishAndPersistErrorEvents(sessionName, runID, threadID, fmt.Sprintf("Runner error: HTTP %d", resp.StatusCode))
330+
publishAndPersistErrorEvents(sessionName, namespacedSessionID, runID, threadID, fmt.Sprintf("Runner error: HTTP %d", resp.StatusCode))
327331
return
328332
}
329333

@@ -343,7 +347,7 @@ func proxyRunnerStream(runnerURL string, bodyBytes []byte, sessionName, runID, t
343347
// Persist every data event to JSONL
344348
if strings.HasPrefix(trimmed, "data: ") {
345349
jsonData := strings.TrimPrefix(trimmed, "data: ")
346-
persistStreamedEvent(sessionName, runID, threadID, jsonData)
350+
persistStreamedEvent(namespacedSessionID, runID, threadID, jsonData)
347351
}
348352

349353
// Publish raw SSE line to all GET /agui/events subscribers
@@ -356,14 +360,15 @@ func proxyRunnerStream(runnerURL string, bodyBytes []byte, sessionName, runID, t
356360
// publishAndPersistErrorEvents generates RUN_STARTED + RUN_ERROR events,
357361
// persists them, and publishes to the live broadcast so subscribers get
358362
// notified of runner failures.
359-
func publishAndPersistErrorEvents(sessionName, runID, threadID, message string) {
363+
// sessionName is used for broadcasting; namespacedSessionID is used for persistence.
364+
func publishAndPersistErrorEvents(sessionName, namespacedSessionID, runID, threadID, message string) {
360365
// RUN_STARTED
361366
startEvt := map[string]interface{}{
362367
"type": "RUN_STARTED",
363368
"threadId": threadID,
364369
"runId": runID,
365370
}
366-
persistEvent(sessionName, startEvt)
371+
persistEvent(namespacedSessionID, startEvt)
367372
startData, _ := json.Marshal(startEvt)
368373
publishLine(sessionName, fmt.Sprintf("data: %s\n\n", startData))
369374

@@ -374,7 +379,7 @@ func publishAndPersistErrorEvents(sessionName, runID, threadID, message string)
374379
"threadId": threadID,
375380
"runId": runID,
376381
}
377-
persistEvent(sessionName, errEvt)
382+
persistEvent(namespacedSessionID, errEvt)
378383
errData, _ := json.Marshal(errEvt)
379384
publishLine(sessionName, fmt.Sprintf("data: %s\n\n", errData))
380385
}

components/backend/websocket/agui_store.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,10 @@ func loadEvents(sessionID string) []map[string]interface{} {
198198
// DeriveAgentStatus reads a session's event log and returns the agent
199199
// status derived from the last significant events.
200200
//
201+
// sessionID should be namespace-qualified (e.g., "namespace/sessionName") to avoid cross-project collisions.
201202
// Returns "" if the status cannot be determined (no events, file missing, etc.).
202203
func DeriveAgentStatus(sessionID string) string {
204+
// sessionID is now namespace-qualified, e.g., "default/session-123"
203205
path := fmt.Sprintf("%s/sessions/%s/agui-events.jsonl", StateBaseDir, sessionID)
204206

205207
// Read only the tail of the file to avoid loading entire event log into memory.

components/frontend/src/app/projects/[name]/sessions/[sessionName]/page.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,6 +1406,7 @@ export default function ProjectSessionDetailPage({
14061406
});
14071407
} catch (err) {
14081408
toast.error(err instanceof Error ? err.message : "Failed to send answer");
1409+
throw err;
14091410
}
14101411
};
14111412

components/frontend/src/components/session/ask-user-question.tsx

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export type AskUserQuestionMessageProps = {
1919
toolUseBlock: ToolUseBlock;
2020
resultBlock?: ToolResultBlock;
2121
timestamp?: string;
22-
onSubmitAnswer?: (formattedAnswer: string) => void;
22+
onSubmitAnswer?: (formattedAnswer: string) => Promise<void>;
2323
isNewest?: boolean;
2424
};
2525

@@ -56,7 +56,8 @@ export const AskUserQuestionMessage: React.FC<AskUserQuestionMessageProps> = ({
5656

5757
// Only interactive when newest message AND not already answered/submitted
5858
const [submitted, setSubmitted] = useState(false);
59-
const disabled = alreadyAnswered || submitted || !isNewest;
59+
const [isSubmitting, setIsSubmitting] = useState(false);
60+
const disabled = alreadyAnswered || submitted || isSubmitting || !isNewest;
6061

6162
// Active tab index (for multi-question tabbed view)
6263
const [activeTab, setActiveTab] = useState(0);
@@ -108,7 +109,7 @@ export const AskUserQuestionMessage: React.FC<AskUserQuestionMessageProps> = ({
108109

109110
const allQuestionsAnswered = questions.every(isQuestionAnswered);
110111

111-
const handleSubmit = () => {
112+
const handleSubmit = async () => {
112113
if (!onSubmitAnswer || !allQuestionsAnswered || disabled) return;
113114

114115
// Build the structured response matching Claude SDK format
@@ -120,8 +121,13 @@ export const AskUserQuestionMessage: React.FC<AskUserQuestionMessageProps> = ({
120121

121122
// Send as JSON matching the AskUserQuestion response format
122123
const response = JSON.stringify({ questions, answers });
123-
onSubmitAnswer(response);
124-
setSubmitted(true);
124+
try {
125+
setIsSubmitting(true);
126+
await onSubmitAnswer(response);
127+
setSubmitted(true);
128+
} finally {
129+
setIsSubmitting(false);
130+
}
125131
};
126132

127133
if (questions.length === 0) return null;
@@ -276,6 +282,7 @@ export const AskUserQuestionMessage: React.FC<AskUserQuestionMessageProps> = ({
276282
const active = idx === activeTab;
277283
return (
278284
<button
285+
type="button"
279286
key={idx}
280287
onClick={() => setActiveTab(idx)}
281288
className={cn(

components/frontend/src/components/ui/stream-message.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { FeedbackButtons } from "@/components/feedback";
1313
export type StreamMessageProps = {
1414
message: (MessageObject | ToolUseMessages | HierarchicalToolMessage) & { streaming?: boolean };
1515
onGoToResults?: () => void;
16-
onSubmitAnswer?: (formattedAnswer: string) => void;
16+
onSubmitAnswer?: (formattedAnswer: string) => Promise<void>;
1717
plainCard?: boolean;
1818
isNewest?: boolean;
1919
agentName?: string;

components/frontend/src/hooks/use-agent-status.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export function useAgentStatus(
3838

3939
// Check the last tool call on this message
4040
const lastTc = msg.toolCalls[msg.toolCalls.length - 1];
41-
if (isAskUserQuestionTool(lastTc.function.name)) {
41+
if (lastTc.function?.name && isAskUserQuestionTool(lastTc.function.name)) {
4242
const hasResult =
4343
lastTc.result !== undefined &&
4444
lastTc.result !== null &&

components/frontend/src/services/queries/use-sessions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export function useSessionsPaginated(projectName: string, params: PaginationPara
5555

5656
// Tier 2: Any session with agent actively working → moderate (5s)
5757
const hasWorking = items.some((s) => {
58-
return s.status?.phase === 'Running' && s.status?.agentStatus === 'working';
58+
return s.status?.phase === 'Running' && (!s.status?.agentStatus || s.status?.agentStatus === 'working');
5959
});
6060
if (hasWorking) return 5000;
6161

0 commit comments

Comments
 (0)