feat: enable interactive form - #2272
Merged
qdaxb merged 10 commits intoJul 28, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds MCP interactive-form documentation, Codex elicitation and approval configuration, MCP approval-mode propagation, task turn-resolution handling, form-card styling, and desktop automation and E2E validation changes. ChangesMCP interactive forms and runtime configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MCPServer
participant CodexAppServer
participant Executor
participant Wework
participant Plugin
MCPServer->>CodexAppServer: Send elicitation/create with mode form
CodexAppServer->>Executor: Emit MCP elicitation request
Executor->>Wework: Forward mcpServer/elicitation/request
Wework->>Plugin: Return accept, cancel, or decline
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/components/chat/RequestUserInputCard.tsx (1)
143-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnsure action buttons meet the 44px mobile touch target.
Both buttons use
h-8(32px), so their height is below the required 44px mobile minimum. Use a responsive height such ash-11 md:h-8so mobile controls remain touch-friendly without changing desktop density.🤖 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 `@wework/src/components/chat/RequestUserInputCard.tsx` around lines 143 - 158, Update both action buttons in RequestUserInputCard, including the ignore button and the button identified by request-user-input-submit-button, to use a mobile height of at least 44px with the existing 32px height retained at the md breakpoint. Preserve all other styling and behavior.Source: Coding guidelines
🤖 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 `@docs/en/wework/developer-guide/wework-plugin-interactive-forms.md`:
- Around line 85-91: Remove the unapproved compatibility fallback directive from
the English interactive-forms guidance at
docs/en/wework/developer-guide/wework-plugin-interactive-forms.md:85-91, unless
an approved product decision is documented there; retain only the supported
elicitation and capability-check guidance. Apply the same policy-aligned change
to docs/zh/wework/developer-guide/wework-plugin-interactive-forms.md:85-91 so
the translation matches the English guidance.
In `@executor/src/runtime_work/connectors.rs`:
- Around line 93-104: Update the rollback handling in the write_mcp_config error
path to inspect failures from persist_connector_gateway_config and
clear_connector_gateway_config instead of discarding them. Only restore cloud to
previous_config or None after the corresponding disk rollback succeeds; when
rollback fails, preserve the in-memory state matching the persisted
configuration and return an error that surfaces the rollback failure.
---
Outside diff comments:
In `@wework/src/components/chat/RequestUserInputCard.tsx`:
- Around line 143-158: Update both action buttons in RequestUserInputCard,
including the ignore button and the button identified by
request-user-input-submit-button, to use a mobile height of at least 44px with
the existing 32px height retained at the md breakpoint. Preserve all other
styling and behavior.
🪄 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 Plus
Run ID: e3afead0-fdf1-4097-8972-befdb39cb00b
📒 Files selected for processing (12)
docs/en/wework/developer-guide/wework-plugin-interactive-forms.mddocs/zh/wework/developer-guide/wework-plugin-interactive-forms.mdexecutor/src/agents/codex.rsexecutor/src/agents/codex/tests.rsexecutor/src/agents/mod.rsexecutor/src/agents/runtime_capabilities.rsexecutor/src/runtime_work/connectors.rsexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/handler/tasks.rsexecutor/tests/codex_app_server_contract.rswework/src/components/chat/RequestUserInputCard.test.tsxwework/src/components/chat/RequestUserInputCard.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wework/e2e/desktop/task-flow.e2e.mjs (1)
1828-1834: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBegin the scenario with a snapshot.
This helper clicks immediately after setting the scenario. Capture and assert the initial stable UI state first, then perform the click.
Suggested adjustment
async function verifyProviderSwitchRetry(control, composerSelector) { control.setScenario('provider_switch_retry') + const snapshot = JSON.parse(await control.command('snapshot', 'body')) + assert.ok(snapshot.testIds.includes('new-chat-button')) await control.command('click', '[data-testid="new-chat-button"]')As per coding guidelines, desktop verification must use existing
data-testidselectors, begin withsnapshot, and assert visible text or stable elements after critical actions.🤖 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 `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 1828 - 1834, Update verifyProviderSwitchRetry to capture and assert the initial stable UI state with control.snapshot immediately after setting the provider_switch_retry scenario, before clicking [data-testid="new-chat-button"]; use existing data-testid selectors and verify visible text or stable elements before continuing with waitFor and model selection.Source: Coding guidelines
🤖 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 `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 2189-2196: Update the assertions after switching back in the
conversation flow around middlePositionAfterSwitch to retain the tolerance-based
comparison with middlePositionBeforeSwitch or middleDistanceBeforeSwitch, while
keeping the existing top and bottom guards. Verify the restored scroll position
remains equivalent to the saved position within the established tolerance,
rather than only confirming it is somewhere in the middle.
---
Nitpick comments:
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 1828-1834: Update verifyProviderSwitchRetry to capture and assert
the initial stable UI state with control.snapshot immediately after setting the
provider_switch_retry scenario, before clicking [data-testid="new-chat-button"];
use existing data-testid selectors and verify visible text or stable elements
before continuing with waitFor and model selection.
🪄 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 Plus
Run ID: d7af47fa-b0a7-4904-985d-1a073ffe7eff
📒 Files selected for processing (1)
wework/e2e/desktop/task-flow.e2e.mjs
Comment on lines
+2189
to
+2196
| const middleDistanceAfterSwitch = distanceFromBottom(middlePositionAfterSwitch) | ||
| assert.ok( | ||
| Math.abs(middlePositionAfterSwitch.scrollTop - middlePositionBeforeSwitch.scrollTop) <= 32, | ||
| `The middle scroll position moved from ${middlePositionBeforeSwitch.scrollTop}px to ${middlePositionAfterSwitch.scrollTop}px` | ||
| middlePositionAfterSwitch.scrollTop > 100, | ||
| 'The restored long conversation unexpectedly returned to the top' | ||
| ) | ||
| assert.ok( | ||
| middleDistanceAfterSwitch > 100, | ||
| 'The restored long conversation unexpectedly returned to the bottom' |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Retain the saved-position comparison.
These assertions only prove that the conversation is somewhere in the middle after switching back. A regression that restores a different middle position will pass. Keep the previous tolerance-based comparison against middlePositionBeforeSwitch or middleDistanceBeforeSwitch, alongside these top/bottom guards.
🤖 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 `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 2189 - 2196, Update the
assertions after switching back in the conversation flow around
middlePositionAfterSwitch to retain the tolerance-based comparison with
middlePositionBeforeSwitch or middleDistanceBeforeSwitch, while keeping the
existing top and bottom guards. Verify the restored scroll position remains
equivalent to the saved position within the established tolerance, rather than
only confirming it is somewhere in the middle.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/e2e/desktop/task-flow.e2e.mjs (1)
2136-2146: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the official label constant for the model option assertion.
officialModelSelectorhas already been waited on withPROVIDER_SWITCH_OFFICIAL_LABEL, and its text isGPT 5.5(PROVIDER_SWITCH_OFFICIAL_LABEL), notGPT 5.6 Sol(PROVIDER_SWITCH_SOL_LABEL). Switching this assertion to the official label constant avoids a deterministically failing check.🐛 Proposed fix
const disabledModelText = await control.command('getText', officialModelSelector) assert.ok( - disabledModelText.includes(PROVIDER_SWITCH_SOL_LABEL), + disabledModelText.includes(PROVIDER_SWITCH_OFFICIAL_LABEL), 'The target model option did not display the expected model label' )🤖 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 `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 2136 - 2146, Update the assertion using disabledModelText in the official model option check to compare against PROVIDER_SWITCH_OFFICIAL_LABEL instead of PROVIDER_SWITCH_SOL_LABEL, matching the label already validated by waitFor and preserving the existing assertion message.
🤖 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.
Outside diff comments:
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 2136-2146: Update the assertion using disabledModelText in the
official model option check to compare against PROVIDER_SWITCH_OFFICIAL_LABEL
instead of PROVIDER_SWITCH_SOL_LABEL, matching the label already validated by
waitFor and preserving the existing assertion message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ed10a02-439f-45af-8b31-4b22056f331f
📒 Files selected for processing (6)
executor/src/agents/codex.rsexecutor/src/agents/codex/tests.rsexecutor/tests/codex_app_server_contract.rswework/e2e/desktop/task-flow.e2e.mjswework/src-tauri/tauri.conf.jsonwework/src/e2e/automation.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- wework/src/e2e/automation.ts
- executor/src/agents/codex/tests.rs
- executor/tests/codex_app_server_contract.rs
- executor/src/agents/codex.rs
…ctive-form-pr2272 # Conflicts: # wework/e2e/desktop/scenarios/streaming-text.scenario.mjs # wework/e2e/desktop/task-flow.e2e.mjs
…ctive-form-pr2272
…ctive-form-pr2272 # Conflicts: # wework/src-tauri/tauri.conf.json
This was referenced Aug 11, 2026
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
2 participants
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.
Enable interactive MCP form elicitations in Wework conversations while preserving the existing no-approval behavior for normal MCP tool calls.
This change allows plugins to show user-facing form cards through MCP
elicitation/createwithmode: "form", maps supported JSON schema fields into Weworkrequest_user_inputUI, and returns the submitted values back to the plugin as MCP elicitation results.Changes
mcp_elicitationsturned on.features.tool_call_mcp_elicitation=false.default_tools_approval_mode="approve".default_tools_approval_mode="prompt"from request-level and bot-level MCP server config.wegent_appsMCP server config during connector configure/app sync so existing local configs receive the approval-mode update.RequestUserInputCard.Summary by CodeRabbit