Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions services/studio/src/nmp/studio/coding_agents.py
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,7 @@ async def _request_permission(session_id: str, args: dict[str, Any]) -> dict[str
try:
decision = await asyncio.wait_for(future, timeout=300)
except asyncio.TimeoutError:
await queue.put(("permission_expired", json.dumps({"request_id": request_id})))
return {"behavior": "deny", "message": "permission request timed out"}
finally:
_pending_permissions.pop(request_id, None)
Expand Down Expand Up @@ -745,6 +746,7 @@ async def _request_agent_input(session_id: str, kind: str, args: dict[str, Any])
try:
decision = await asyncio.wait_for(future, timeout=300)
except asyncio.TimeoutError:
await queue.put(("input_expired", json.dumps({"request_id": request_id})))
return {"status": "error", "message": "input request timed out"}
finally:
_pending_agent_inputs.pop(request_id, None)
Expand Down Expand Up @@ -864,6 +866,10 @@ async def _stream_claude(
yield _sse(payload, event="permission_request")
elif event_type == "input_request":
yield _sse(payload, event="input_request")
elif event_type == "permission_expired":
yield _sse(payload, event="permission_expired")
elif event_type == "input_expired":
yield _sse(payload, event="input_expired")

returncode = await proc.wait()
if stderr_task is not None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,15 @@ export const FilesetFileBlockingInput: FC<FilesetFileBlockingInputBaseProps> = (
control,
handleSubmit,
setError,
watch,
formState: { errors },
} = useForm<FilesetFileFormData>({
defaultValues: { datasetFile: null },
resolver: zodResolver(schema),
disabled: isSubmitting,
});
const acceptedFileTypes = getAcceptedFileTypes(input, defaultAcceptedFileTypes);
const datasetFile = watch('datasetFile');

const submit = handleSubmit((data) => {
const parsed =
Expand All @@ -98,6 +100,7 @@ export const FilesetFileBlockingInput: FC<FilesetFileBlockingInputBaseProps> = (
request={request}
secondaryActions={secondaryActions}
secondaryActionLabel={secondaryActionLabel}
submitDisabled={!datasetFile}
submitLabel={submitLabel}
>
<Stack className="max-h-[45vh] overflow-y-auto pr-density-sm">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe('AgentDecisionInput', () => {
);
});

it('shows a send button for the text choice and submits the entered text', async () => {
it('shows a send button that is disabled while a text choice awaits input', async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();

Expand All @@ -129,13 +129,13 @@ describe('AgentDecisionInput', () => {
/>
);

expect(screen.queryByRole('button', { name: /Send alternative instruction/i })).toBeNull();
const sendButton = screen.getByRole('button', { name: /Send/i });
expect(sendButton).not.toBeDisabled();

await user.click(
screen.getByRole('option', { name: /3\.\s+Tell the Agent what to do differently/i })
);

const sendButton = screen.getByRole('button', { name: /Send alternative instruction/i });
expect(sendButton).toBeDisabled();

await user.type(
Expand Down Expand Up @@ -276,6 +276,39 @@ describe('AgentDecisionInput', () => {
expect(onSubmit).toHaveBeenCalledWith({ id: 'no', label: 'No' });
});

it('resets to the default choice when request.id changes', async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();

const { rerender } = render(
<AgentDecisionInput
request={request}
choices={choices}
defaultChoiceId="yes"
onSubmit={onSubmit}
/>
);

// Navigate to the text-input choice on the first request
await user.click(
screen.getByRole('option', { name: /3\.\s+Tell the Agent what to do differently/i })
);
expect(screen.getByRole('button', { name: /Send/i })).toBeDisabled();

// Simulate a new request arriving (different id) — as if a queued permission dequeued
rerender(
<AgentDecisionInput
request={{ ...request, id: 'permission-2', title: 'New request' }}
choices={choices}
defaultChoiceId="yes"
onSubmit={onSubmit}
/>
);

// Send button must be enabled immediately — no stale "alternative" selection
expect(screen.getByRole('button', { name: /Send/i })).not.toBeDisabled();
});

it('calls skip from the bottom action', async () => {
const user = userEvent.setup();
const onSkip = vi.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,19 +367,17 @@ export const AgentDecisionInput = ({
<Text kind="label/regular/sm">{skipLabel}</Text>
</Button>
) : null}
{selectedChoiceInput ? (
<Button
aria-label="Send alternative instruction"
type="button"
color="brand"
size="small"
disabled={isSubmitting || !trimmedChoiceInputValue}
className={subduedButtonFocusClassName}
onClick={() => void submitSelectedChoice()}
>
<Send size={16} />
</Button>
) : null}
<Button
aria-label="Send"
type="button"
color="brand"
size="small"
disabled={isSubmitting || (!!selectedChoiceInput && !trimmedChoiceInputValue)}
className={subduedButtonFocusClassName}
onClick={() => void submitSelectedChoice()}
>
<Send size={16} />
</Button>
</Flex>
</Flex>
</Flex>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export const ClaudeCodeChatThread: FC<ClaudeCodeChatThreadProps> = ({
composerOverride={
studioNavigationRequest ? (
<AgentDecisionInput
key={studioNavigationRequest.id}
request={{
id: studioNavigationRequest.id,
title: 'Studio UI available',
Expand All @@ -183,6 +184,7 @@ export const ClaudeCodeChatThread: FC<ClaudeCodeChatThreadProps> = ({
/>
) : decisionRequest ? (
<AgentDecisionInput
key={decisionRequest.id}
request={decisionRequest}
choices={decisionChoices}
defaultChoiceId={decisionChoices[0]?.id}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,80 @@ describe('Claude Code API helpers', () => {
);
});

it('calls onPermissionExpired when a permission_expired event arrives', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue(
new Response(
[
'event: permission_expired',
'data: {"request_id":"request-1"}',
'',
'event: done',
'data: ',
'',
].join('\n'),
{ status: 200 }
)
);
vi.stubGlobal('fetch', fetchMock);

const onPermissionExpired = vi.fn();

await streamClaudeCodeMessage({
sessionId: 'session-1',
message: 'list files',
signal: new AbortController().signal,
handlers: {
onClaudeEvent: vi.fn(),
onInputRequest: vi.fn(),
onPermissionRequest: vi.fn(),
onPermissionExpired,
onDone: vi.fn(),
onError: vi.fn(),
},
});

expect(onPermissionExpired).toHaveBeenCalledWith('request-1');
});

it('calls onInputExpired when an input_expired event arrives', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue(
new Response(
[
'event: input_expired',
'data: {"request_id":"request-2"}',
'',
'event: done',
'data: ',
'',
].join('\n'),
{ status: 200 }
)
);
vi.stubGlobal('fetch', fetchMock);

const onInputExpired = vi.fn();

await streamClaudeCodeMessage({
sessionId: 'session-1',
message: 'list files',
signal: new AbortController().signal,
handlers: {
onClaudeEvent: vi.fn(),
onInputRequest: vi.fn(),
onPermissionRequest: vi.fn(),
onInputExpired,
onDone: vi.fn(),
onError: vi.fn(),
},
});

expect(onInputExpired).toHaveBeenCalledWith('request-2');
});

it('posts approval decisions using the backend permission shape', async () => {
const fetchMock = vi.fn().mockResolvedValue(new Response('{}', { status: 200 }));
vi.stubGlobal('fetch', fetchMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,22 @@ const handleSseEvent = (
return true;
}

if (event.event === 'permission_expired') {
const payload = parseJsonObject(event.data);
if (isRecord(payload) && typeof payload.request_id === 'string') {
handlers.onPermissionExpired?.(payload.request_id);
}
return true;
}

if (event.event === 'input_expired') {
const payload = parseJsonObject(event.data);
if (isRecord(payload) && typeof payload.request_id === 'string') {
handlers.onInputExpired?.(payload.request_id);
}
return true;
}
Comment on lines +392 to +406

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fail closed on malformed expiration events.

If request_id is missing here, the event is silently dropped and the blocking UI never clears. Match the permission_request / input_request branches and surface onError instead of returning success.

Suggested fix
   if (event.event === 'permission_expired') {
     const payload = parseJsonObject(event.data);
-    if (isRecord(payload) && typeof payload.request_id === 'string') {
-      handlers.onPermissionExpired?.(payload.request_id);
-    }
-    return true;
+    if (!isRecord(payload) || typeof payload.request_id !== 'string') {
+      handlers.onError(new Error('Claude Code permission expiration was malformed'));
+      return false;
+    }
+    handlers.onPermissionExpired?.(payload.request_id);
+    return true;
   }

   if (event.event === 'input_expired') {
     const payload = parseJsonObject(event.data);
-    if (isRecord(payload) && typeof payload.request_id === 'string') {
-      handlers.onInputExpired?.(payload.request_id);
-    }
-    return true;
+    if (!isRecord(payload) || typeof payload.request_id !== 'string') {
+      handlers.onError(new Error('Claude Code input expiration was malformed'));
+      return false;
+    }
+    handlers.onInputExpired?.(payload.request_id);
+    return true;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (event.event === 'permission_expired') {
const payload = parseJsonObject(event.data);
if (isRecord(payload) && typeof payload.request_id === 'string') {
handlers.onPermissionExpired?.(payload.request_id);
}
return true;
}
if (event.event === 'input_expired') {
const payload = parseJsonObject(event.data);
if (isRecord(payload) && typeof payload.request_id === 'string') {
handlers.onInputExpired?.(payload.request_id);
}
return true;
}
if (event.event === 'permission_expired') {
const payload = parseJsonObject(event.data);
if (!isRecord(payload) || typeof payload.request_id !== 'string') {
handlers.onError(new Error('Claude Code permission expiration was malformed'));
return false;
}
handlers.onPermissionExpired?.(payload.request_id);
return true;
}
if (event.event === 'input_expired') {
const payload = parseJsonObject(event.data);
if (!isRecord(payload) || typeof payload.request_id !== 'string') {
handlers.onError(new Error('Claude Code input expiration was malformed'));
return false;
}
handlers.onInputExpired?.(payload.request_id);
return true;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.ts` around
lines 392 - 406, The permission_expired and input_expired handlers in
ClaudeCodeChatRoute/api.ts currently return success even when payload.request_id
is missing or invalid, which lets malformed expiration events get dropped
silently. Update the event handling in the same switch/if chain that processes
permission_request and input_request so these branches fail closed: parse the
payload, validate request_id, and if it is absent or not a string, call
handlers.onError with a clear message instead of returning true. Keep the
existing onPermissionExpired/onInputExpired calls only for valid request_id
values.


handlers.onClaudeEvent(parseJsonObject(event.data));
return true;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ export interface ClaudeCodeStreamHandlers {
onClaudeEvent: (event: unknown) => void;
onInputRequest: (request: ClaudeCodeInputRequest) => void;
onPermissionRequest: (request: ClaudeCodePermissionRequest) => void;
onInputExpired?: (requestId: string) => void;
onPermissionExpired?: (requestId: string) => void;
onDone: () => void;
onError: (error: Error) => void;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,14 @@ export const useClaudeCodeChatRuntime = (options?: UseClaudeCodeChatRuntimeOptio
prepareForUserInput();
handleInputRequest(request);
},
onPermissionExpired: (requestId) => {
if (signal.aborted || !isCurrentRun()) return;
dispatchBlocking({ type: 'clear_permission', requestId, dequeueNext: true });
},
onInputExpired: (requestId) => {
if (signal.aborted || !isCurrentRun()) return;
dispatchBlocking({ type: 'clear_input', requestId, dequeueNext: true });
},
onDone: () => {
doneReceived = true;
},
Expand Down