fix(elixir): handle MCP elicitation requests - #1
Open
shaunr0b wants to merge 4 commits into
Open
Conversation
📝 WalkthroughWalkthroughRoutes "mcpServer/elicitation/request" into the existing Codex user-input flow; adds tracker.required_labels config and threads required-label filtering through Linear client fetch/pagination/decoding with test helpers; introduces slot-wait retry type and surfaces capacity-wait entries in snapshot, presenter, dashboard, and tests. ChangesMCP Server Elicitation Request Handling
Tracker required_labels and Linear client filtering
Capacity-wait retry scheduling and UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@elixir/lib/symphony_elixir/status_dashboard.ex`:
- Around line 1367-1377: The
humanize_codex_method("mcpServer/elicitation/request", payload) currently only
checks params.question and params.prompt and thus drops params.message; update
the function to also read params.message (e.g., via map_path(payload,
["params","message"])) and prefer it when present (or include it in the
fallback) so the returned string uses inline_text(message) just like
question/prompt; reference the existing humanize_codex_method and map_path
helpers and inline_text to implement this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6da31965-9cfa-4158-b654-8a03892e18ea
📒 Files selected for processing (2)
elixir/lib/symphony_elixir/codex/app_server.exelixir/lib/symphony_elixir/status_dashboard.ex
Comment on lines
+1367
to
+1377
| defp humanize_codex_method("mcpServer/elicitation/request", payload) do | ||
| question = | ||
| map_path(payload, ["params", "question"]) || | ||
| map_path(payload, ["params", "prompt"]) | ||
|
|
||
| if is_binary(question) and String.trim(question) != "" do | ||
| "MCP server requires user input: #{inline_text(question)}" | ||
| else | ||
| "MCP server requires user input" | ||
| end | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for MCP elicitation payload examples and field names
# Search for MCP elicitation request payloads to see which field names are used
rg -A5 'mcpServer/elicitation/request' --type=elixir
# Also check test files for payload examples
rg -A10 'mcpServer.*elicitation' --glob '*test*.exs'Repository: LegendsOfLearning/symphony
Length of output: 8246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all occurrences of the MCP elicitation method and capture surrounding payload shapes
rg -n --hidden --no-ignore -S 'mcpServer/elicitation/request' elixir -g'*.ex' -g'*.exs'
echo "----"
# Look for params payload keys used with elicitation/request
rg -n --hidden --no-ignore -S 'mcpServer/elicitation/request.*params|params.*mcpServer/elicitation/request' elixir -g'*.ex' -g'*.exs' || true
echo "----"
rg -n --hidden --no-ignore -S '"(message|question|prompt)"' elixir -g'*.ex' -g'*.exs' | head -n 200
echo "----"
# Narrow: any mention of "question"/"prompt" in the context of elicitation
rg -n --hidden --no-ignore -S '(elicitation/request|turn_input_required|input required).{0,80}(question|prompt)' elixir -g'*.ex' -g'*.exs' || trueRepository: LegendsOfLearning/symphony
Length of output: 7377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect map_path implementation and nearby helpers in status_dashboard.ex
rg -n "defp map_path\\(" -S elixir/lib/symphony_elixir/status_dashboard.ex
sed -n '1,220p' elixir/lib/symphony_elixir/status_dashboard.ex | head -n 220
# Show the humanize_codex_method clauses around the mcp elicitation and requestUserInput handling
sed -n '1280,1425p' elixir/lib/symphony_elixir/status_dashboard.ex
# Inspect codex/app_server paths for mcpServer/elicitation/request where params are constructed/forwarded
sed -n '620,760p' elixir/lib/symphony_elixir/codex/app_server.ex
sed -n '1020,1160p' elixir/lib/symphony_elixir/codex/app_server.ex
# Search for creation of "mcpServer/elicitation/request" with params.question/prompt in the codebase
rg -n -S 'mcpServer/elicitation/request.*params|params.*(question|prompt)' elixir -g'*.ex' -g'*.exs'Repository: LegendsOfLearning/symphony
Length of output: 20907
Show params.message for mcpServer/elicitation/request in the status dashboard.
humanize_codex_method("mcpServer/elicitation/request", ...) only reads params.question / params.prompt, but elixir/test/symphony_elixir/app_server_test.exs sends {"method":"mcpServer/elicitation/request","params":{"message":"Need operator input"}}. This will cause the dashboard to fall back to the generic text and drop the specific prompt.
Suggested change
defp humanize_codex_method("mcpServer/elicitation/request", payload) do
question =
+ map_path(payload, ["params", "message"]) ||
map_path(payload, ["params", "question"]) ||
map_path(payload, ["params", "prompt"])📝 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
| defp humanize_codex_method("mcpServer/elicitation/request", payload) do | |
| question = | |
| map_path(payload, ["params", "question"]) || | |
| map_path(payload, ["params", "prompt"]) | |
| if is_binary(question) and String.trim(question) != "" do | |
| "MCP server requires user input: #{inline_text(question)}" | |
| else | |
| "MCP server requires user input" | |
| end | |
| end | |
| defp humanize_codex_method("mcpServer/elicitation/request", payload) do | |
| question = | |
| map_path(payload, ["params", "message"]) || | |
| map_path(payload, ["params", "question"]) || | |
| map_path(payload, ["params", "prompt"]) | |
| if is_binary(question) and String.trim(question) != "" do | |
| "MCP server requires user input: #{inline_text(question)}" | |
| else | |
| "MCP server requires user input" | |
| end | |
| end |
🤖 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 `@elixir/lib/symphony_elixir/status_dashboard.ex` around lines 1367 - 1377, The
humanize_codex_method("mcpServer/elicitation/request", payload) currently only
checks params.question and params.prompt and thus drops params.message; update
the function to also read params.message (e.g., via map_path(payload,
["params","message"])) and prefer it when present (or include it in the
fallback) so the returned string uses inline_text(message) just like
question/prompt; reference the existing humanize_codex_method and map_path
helpers and inline_text to implement this change.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
elixir/lib/symphony_elixir/status_dashboard.ex (1)
1398-1408:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
params.messagecheck for MCP elicitation prompts.The
humanize_codex_method("mcpServer/elicitation/request", payload)clause only readsparams.questionandparams.prompt, but the test suite usesparams.message(as noted in previous reviews). Without this check, the dashboard will fall back to generic text and drop the specific prompt.Suggested fix
defp humanize_codex_method("mcpServer/elicitation/request", payload) do question = + map_path(payload, ["params", "message"]) || map_path(payload, ["params", "question"]) || map_path(payload, ["params", "prompt"])🤖 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 `@elixir/lib/symphony_elixir/status_dashboard.ex` around lines 1398 - 1408, The clause humanize_codex_method("mcpServer/elicitation/request", payload) only checks map_path(payload, ["params", "question"]) and ["params", "prompt"] but tests supply the prompt under ["params", "message"], so update the payload extraction to also check map_path(payload, ["params", "message"]) (e.g., try question = map_path(..., ["params","question"]) || map_path(..., ["params","prompt"]) || map_path(..., ["params","message"])); keep the existing is_binary and String.trim check and the inline_text call so the specific MCP prompt is shown instead of the generic fallback.
🧹 Nitpick comments (1)
elixir/lib/symphony_elixir/status_dashboard.ex (1)
658-668: ⚡ Quick winConsider removing redundant
format_retry_summary/1clause.The second clause at line 666 appears to be defensive programming for non-map entries, but
format_backoff_retry_summary/1expects map fields (.issue_id,.identifier,.attempt, etc.) and will crash on non-maps anyway. Since the first clause at line 658 already handles all maps (both capacity-wait and backoff cases), the second clause is unreachable for valid retry entries.Simplification
defp format_retry_summary(%{} = retry_entry) do if capacity_wait_entry?(retry_entry) do format_capacity_wait_summary(retry_entry) else format_backoff_retry_summary(retry_entry) end end - - defp format_retry_summary(retry_entry) do - format_backoff_retry_summary(retry_entry) - end🤖 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 `@elixir/lib/symphony_elixir/status_dashboard.ex` around lines 658 - 668, Remove the redundant fallback clause for format_retry_summary/1: delete the second clause "defp format_retry_summary(retry_entry) do format_backoff_retry_summary(retry_entry) end" and keep only the map-matching clause that dispatches to format_capacity_wait_summary/1 or format_backoff_retry_summary/1; ensure no other code relies on a non-map arity by running tests for format_retry_summary/1 and callers of format_retry_summary to confirm behavior remains correct.
🤖 Prompt for all review comments with 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.
Duplicate comments:
In `@elixir/lib/symphony_elixir/status_dashboard.ex`:
- Around line 1398-1408: The clause
humanize_codex_method("mcpServer/elicitation/request", payload) only checks
map_path(payload, ["params", "question"]) and ["params", "prompt"] but tests
supply the prompt under ["params", "message"], so update the payload extraction
to also check map_path(payload, ["params", "message"]) (e.g., try question =
map_path(..., ["params","question"]) || map_path(..., ["params","prompt"]) ||
map_path(..., ["params","message"])); keep the existing is_binary and
String.trim check and the inline_text call so the specific MCP prompt is shown
instead of the generic fallback.
---
Nitpick comments:
In `@elixir/lib/symphony_elixir/status_dashboard.ex`:
- Around line 658-668: Remove the redundant fallback clause for
format_retry_summary/1: delete the second clause "defp
format_retry_summary(retry_entry) do format_backoff_retry_summary(retry_entry)
end" and keep only the map-matching clause that dispatches to
format_capacity_wait_summary/1 or format_backoff_retry_summary/1; ensure no
other code relies on a non-map arity by running tests for format_retry_summary/1
and callers of format_retry_summary to confirm behavior remains correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7222a1fb-b136-4b92-b018-6ed3503423af
📒 Files selected for processing (6)
elixir/lib/symphony_elixir/orchestrator.exelixir/lib/symphony_elixir/status_dashboard.exelixir/lib/symphony_elixir_web/live/dashboard_live.exelixir/lib/symphony_elixir_web/presenter.exelixir/test/symphony_elixir/core_test.exselixir/test/symphony_elixir/extensions_test.exs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mcpServer/elicitation/requestthrough the existing user-input answer pathVerification
elixir/bin/symphonyfrom the patched checkout and restartedmeta-symphony.serviceSummary by CodeRabbit
New Features
UI/Behavior
Tests