feat: add interactive menu for single select elicitations - #10327
Conversation
Adds support for single property schemas with oneOf/enum options, rendering them as an interactive menu using cliclack::select. Users can navigate options with arrow keys instead of manual input. Signed-off-by: Keerthana Panyam <panyam.k@northeastern.edu>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f722e9992a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f722e99 to
1f4f962
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f4f962c99
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| data.insert( | ||
| field_name.clone(), | ||
| Value::String(selected_value.to_string()), | ||
| ); |
There was a problem hiding this comment.
Allow optional single-select fields to be skipped
For a schema with exactly one enum/oneOf property that is omitted from required and has no default, this branch still only returns after inserting the selected option, and it returns before the later required handling can distinguish optional fields; that is the fresh evidence that the optional single-select case remains affected. The previous text prompt let an empty answer omit optional fields, but the new menu has no skip choice, so tools receive a value the user did not intend instead of an accepted {} response.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added Skip for optional fields with no default that returns Accept with empty data when Skip is selected
1f4f962 to
c176104
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1761049ac
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
c176104 to
f71d314
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f71d314908
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if selected_value.is_empty() { | ||
| return Ok(ElicitationInput { | ||
| action: ElicitationAction::Accept, | ||
| user_data: HashMap::new(), | ||
| }); |
There was a problem hiding this comment.
Do not overload empty selections as Skip
When a single-select schema legitimately includes "" as an enum value or oneOf const, this check treats selecting that value as the synthetic Skip action. For required fields no Skip option is added, but choosing the valid empty-string option still returns Accept with {} and bypasses the later required-field validation; for optional fields it makes the empty-string value impossible to submit. Use a distinct sentinel/typed selection for Skip instead of using the empty string itself.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed, now uses sentinel \x00__SKIP__ instead of empty string for Skip, so real empty values work correctly.
- Extracts default from schema and finds its position in options - Sets initial_value to highlight correct option - Pressing enter selects schema default instead of first item - oneOf const checking - Counts total oneOf branches vs const only branches - Only shows menu when all branches have const values - Falls back to text input for mixed schemas - Optional field handling - Prepends "Skip" option for optional fields with no default - Empty selection returns Accept with no data - Ctrl+C always returns Cancel Signed-off-by: Keerthana Panyam <panyam.k@northeastern.edu>
f71d314 to
a2489c0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2489c077c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Find option index that matches the default | ||
| let initial_index = if let Some(default) = default_value { | ||
| options.iter().position(|(value, _)| value == default) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
Default optional single-selects to Skip
For a schema with one optional enum/oneOf field and no default, the previous text prompt let users press Enter to omit the field, but this menu appends Skip without making it the initial selection; because initial_index is only set from default_value, a bare Enter now submits the first real option instead of {}. Even with the new Skip item, preserve the no-default optional behavior by initially highlighting Skip.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Starting at the first real option (not Skip) is correct because it prevents accidental skips, users must consciously arrow down to Skip, not accidentally press Enter too fast. Skip is clearly labeled at the bottom. The small extra effort I feel is worth the safety gain. Let me know what you think 😄
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3193c4c56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Sorry this took so long. I tried to clean up the implementation and pushed a follow-up commit, but Codex still isn't happy with part of it. Could you have a look at the latest review feedback? |
|
Sorry this took so long. I tried to clean up the implementation and pushed a follow-up, but Codex still isn't happy with it. Could you have another look at the remaining feedback when you get a chance? |
- Also check stderr is a terminal since cliclack renders via Term::stderr(); prevents failure when stderr is redirected - Include field description in selector prompt to match the text input path
|
Thanks for reviewing and the cleanup @DOsinga! Just pushed a fix addressing the two remaining comments. Let me know if anything else needs attention! 😁 |
* origin/main: (58 commits) Handle MCP tool list change notifications fix: restore final_output_tool when agent is recreated after LRU eviction (#10793) feat(ui): bring back make sidebar resizable with drag handle and persisted width (#10795) docs: document auto-injected GOOSE_SHELL flags (#10759) feat: surface output-token-limit info (#10831) fix(desktop): avoid O(n²) cloning during session load (#10665) feat: add interactive menu for single select elicitations (#10327) fix(acp): prefix child stderr log target so it passes the default goose=info filter (#10901) chore (codex-acp): migrate to @agentclientprotocol/codex-acp (#10923) style: add dark mode inline code styling for message bubbles (#10861) docs: clarify macOS sandbox feature was removed in post-v1.25.0 cleanup (#10900) fall back to static model list only for non-models payloads (#10189) Remove CLI project support (#10838) fix(anthropic): add claude-sonnet-5 and claude-fable-5 to known models list (#10865) fix: stdio extensions silently skipped when name missing or env: used in config (#10773) feat(desktop): show session metadata on sidebar chat hover (#10663) fix(serve): start scheduler at startup instead of first ACP connection (#10766) fix: respect disabled builtin extensions (developer) at session start (#10223) fix: parse PyPI requirements for OSV checks (#10510) fix(telegram): contain voice file extensions (#10456) ... # Conflicts: # Cargo.lock # crates/goose/src/agents/extension_manager.rs # crates/goose/tests/mcp_replays/npx-y@modelcontextprotocol_server-everything@2026.1.14
Adds support for single property schemas with oneOf/enum options, rendering them as an interactive menu using cliclack::select. Users can navigate options with arrow keys instead of manual input.
Supersedes: #9763
cliclack::selectfor single-property schemasoneOf(with const) andenumschemasAddresses all feedback from bot on #9763:
defaultfrom schema and sets it as initial valuerequiredarray to determine if field is optional and accepts elicitation w empty dataconstvalues