fix(codex): Handle mcpServer/elicitation/request#381
fix(codex): Handle mcpServer/elicitation/request#38191ac0m0 wants to merge 10 commits intotiann:mainfrom
Conversation
There was a problem hiding this comment.
Findings
- [Major] MCP elicitation parsing currently reads a flattened payload, but the new request type declares the actual app-server shape under
params.request. That means realmcpServer/elicitation/requestcalls will hitInvalid MCP elicitation request: missing modebefore the UI/RPC bridge is reached. Evidencecli/src/codex/codexRemoteLauncher.ts:258,cli/src/codex/appServerTypes.ts:161,cli/src/codex/codexRemoteLauncher.test.ts:222.
Suggested fix:const requestRecord = asRecord(params.request) ?? {} const mode = asString(requestRecord.mode) const message = asString(requestRecord.message) ?? '' const requestedSchema = asRecord(requestRecord.requestedSchema) const url = asString(requestRecord.url) const elicitationId = asString(requestRecord.elicitationId)
- [Major] The web flow cannot satisfy
formelicitations yet: accepting always posts{}, while the UI only renders the schema instead of collecting values. Any MCP that expects actual user input will still fail after approval. Evidenceweb/src/components/ToolCard/CodexMcpElicitationFooter.tsx:81,web/src/components/ToolCard/views/CodexMcpElicitationView.tsx:27.
Suggested fix:if (parsed.mode === 'form') { setError('This MCP request requires form input before it can be accepted') return }
Summary
Review mode: initial.
The new bridge wiring is incomplete in two places: the CLI side does not read the nested request payload it just typed, and the web side accepts form elicitations without any way to provide schema-defined values.
Testing
- Not run (automation environment lacks
bunon PATH). - Coverage gap: not found in repo/tests for the real nested
params.requestpayload or an end-to-endformelicitation that submits non-empty content.
HAPI Bot
cli/src/codex/codexRemoteLauncher.ts
Outdated
| }); | ||
|
|
||
| const parseMcpElicitationRequest = (params: McpServerElicitationRequestParams) => { | ||
| const paramsRecord = asRecord(params) ?? {}; |
There was a problem hiding this comment.
[MAJOR] McpServerElicitationRequestParams puts the actual elicitation payload under params.request, but this parser reads mode/message/requestedSchema/url from the top level. With the typed app-server shape, mode stays null here and the request is rejected before it ever reaches the hub/web bridge.
Suggested fix:
const requestRecord = asRecord(params.request) ?? {}
const mode = asString(requestRecord.mode)
const message = asString(requestRecord.message) ?? ''
const requestedSchema = asRecord(requestRecord.requestedSchema)
const url = asString(requestRecord.url)
const elicitationId = asString(requestRecord.elicitationId)| if (loading) return | ||
| setLoading('accept') | ||
|
|
||
| let content: unknown | null = null |
There was a problem hiding this comment.
[MAJOR] Accepting form elicitations always sends {}. The dialog shows requestedSchema, but there is no way to enter schema-defined values, so any MCP that needs real user input will still fail after approval.
Suggested fix:
if (parsed.mode === 'form') {
setError('This MCP request requires form input before it can be accepted')
return
}There was a problem hiding this comment.
Findings
- [Major] In-progress MCP form entries are reset on live refresh because the footer reinitializes local state whenever
formSchemagets a fresh object identity.SessionChatre-normalizes messages onprops.messageschanges andensureToolBlockreplacestool.input, so a normal refresh can wipe partially entered values mid-approval. Evidenceweb/src/components/ToolCard/CodexMcpElicitationFooter.tsx:66,web/src/components/SessionChat.tsx:171,web/src/chat/reducerTools.ts:88.
Suggested fix:useEffect(() => { setLoading(null) setError(null) setFormState(formSchema ? createCodexMcpElicitationFormState(formSchema) : {}) }, [props.tool.id])
- [Major] URL elicitations can be accepted without ever opening the supplied URL. The footer labels the primary action
Open and continue, but the handler only postsaccept; the actual link is only exposed in the expanded detail view. That lets sign-in/OAuth elicitations complete without the required browser step. Evidenceweb/src/components/ToolCard/CodexMcpElicitationFooter.tsx:120,web/src/components/ToolCard/CodexMcpElicitationFooter.tsx:240,web/src/components/ToolCard/views/CodexMcpElicitationView.tsx:36.
Suggested fix:if (parsed.mode === 'url') { window.open(parsed.url, '_blank', 'noopener,noreferrer') } await run(() => props.api.respondToMcpElicitation(props.sessionId, parsed.requestId, { action: 'accept', content }))
Summary
Review mode: follow-up after new commits.
The earlier nested-request parsing and form submission gaps look addressed. Two follow-up regressions remain in the new web footer: live updates clear partially entered MCP form values, and the URL-mode primary action approves without opening the URL.
Testing
- Not run (automation environment lacks
bunon PATH). - Coverage gap: not found in repo/tests for preserving form state across rerenders or for actually opening the URL flow before accepting.
HAPI Bot
| useEffect(() => { | ||
| setLoading(null) | ||
| setError(null) | ||
| setFormState(formSchema ? createCodexMcpElicitationFormState(formSchema) : {}) |
There was a problem hiding this comment.
[MAJOR] This reset effect depends on formSchema, so the footer clears formState any time the same request is re-parsed into a fresh schema object. SessionChat re-normalizes messages whenever props.messages changes, and ensureToolBlock overwrites tool.input, so ordinary live refreshes can wipe a partially completed MCP form mid-edit.
Suggested fix:
useEffect(() => {
setLoading(null)
setError(null)
setFormState(formSchema ? createCodexMcpElicitationFormState(formSchema) : {})
}, [props.tool.id])|
|
||
| setLoading('accept') | ||
| try { | ||
| await run(() => props.api.respondToMcpElicitation(props.sessionId, parsed.requestId, { |
There was a problem hiding this comment.
[MAJOR] For url elicitations this handler only posts accept; it never opens parsed.url. Because the compact card body only renders this footer, the primary Open and continue action can approve a sign-in/OAuth flow without ever sending the user to the provided URL. The link is only visible in the expanded detail view.
Suggested fix:
if (parsed.mode === 'url') {
window.open(parsed.url, '_blank', 'noopener,noreferrer')
}
await run(() => props.api.respondToMcpElicitation(props.sessionId, parsed.requestId, {
action: 'accept',
content
}))There was a problem hiding this comment.
Findings
- [Minor] MCP tool metadata never reaches the web card, so the new
toolTitle/toolDescriptionpresentation is dead code and every elicitation falls back to the raw server name. Evidencecli/src/codex/codexRemoteLauncher.ts:280,web/src/components/ToolCard/codexMcpElicitation.ts:374,web/src/components/ToolCard/knownTools.tsx:179.
Suggested fix:const meta = asRecord(requestRecord._meta) return { requestId, threadId: params.threadId, turnId: params.turnId, serverName: params.serverName, mode, message, requestedSchema: mode === 'form' ? requestedSchema : undefined, url: mode === 'url' ? url : undefined, elicitationId: mode === 'url' ? elicitationId : undefined, _meta: meta ?? undefined }
Summary
Review mode: follow-up after new commits.
The earlier footer regressions look fixed. One remaining mismatch stays in the new MCP elicitation path: the CLI bridge drops _meta, so the new web titles and subtitles for elicitation cards never render.
Testing
- Not run (automation environment lacks
bunon PATH).
HAPI Bot
|
|
||
| const requestId = mode === 'url' ? (elicitationId ?? randomUUID()) : randomUUID(); | ||
|
|
||
| return { |
There was a problem hiding this comment.
[MINOR] The web side now reads MCP tool titles/descriptions from input._meta, but this bridge never forwards _meta into the tool-call payload. That makes the new presentation logic in web/src/components/ToolCard/codexMcpElicitation.ts:374 and web/src/components/ToolCard/knownTools.tsx:179 unreachable, so every elicitation still renders with the raw serverName fallback.
Suggested fix:
const meta = asRecord(requestRecord._meta)
return {
requestId,
threadId: params.threadId,
turnId: params.turnId,
serverName: params.serverName,
mode,
message,
requestedSchema: mode === 'form' ? requestedSchema : undefined,
url: mode === 'url' ? url : undefined,
elicitationId: mode === 'url' ? elicitationId : undefined,
_meta: meta ?? undefined
}There was a problem hiding this comment.
Findings
- [Major] URL-mode MCP elicitations are accepted even when the sign-in page never opens.
submitAccept()ignores a failedwindow.open()call, so popup-blocked browsers/WebViews still sendaction: "accept"and leave Codex waiting on a flow the user never saw. The added test also codifies thatnullreturn path as success. Evidenceweb/src/components/ToolCard/CodexMcpElicitationFooter.tsx:118,web/src/components/ToolCard/codexMcpElicitation.test.tsx:288.
Suggested fix:if (parsed.mode === 'url') { const opened = window.open(parsed.url, '_blank', 'noopener,noreferrer') if (!opened) { haptic.notification('error') setError('Failed to open the sign-in page. Allow popups and try again.') return } }
Summary
Review mode: follow-up after new commits.
One remaining issue in the MCP elicitation flow: URL-mode requests are marked accepted even when the browser blocks the popup, which can silently break sign-in/auth tool flows.
Testing
- Not run (automation environment;
bunnot on PATH).
HAPI Bot
| content = submission.content | ||
| } | ||
| if (parsed.mode === 'url') { | ||
| window.open(parsed.url, '_blank', 'noopener,noreferrer') |
There was a problem hiding this comment.
[MAJOR] URL-mode requests are accepted even if window.open() fails. In popup-blocked browsers or WebViews this still posts action: "accept", so Codex proceeds as if the sign-in page was shown even though the user never reached it. The companion test now locks that behavior in by mocking window.open() to return null and still expecting success.
Suggested fix:
const opened = window.open(parsed.url, '_blank', 'noopener,noreferrer')
if (!opened) {
haptic.notification('error')
setError('Failed to open the sign-in page. Allow popups and try again.')
return
}
problem
current codex service does not register the mcpServer/elicitation/request method, which causes MCP server-initiated elicitation requests to be rejected by default.
When an MCP server requires user confirmation (e.g., title modification MCPs, stdio MCPs, etc.), the requests are directly rejected or cancelled due to the absence of corresponding handling logic.
The related error log is as follows:
change
To address this issue, this change implements support for handling mcpServer/elicitation/request, based on the Codex documentation
Related Issue: tiann/hapi#287