tools+ui: add user response handling for Claude questionsg#3
tools+ui: add user response handling for Claude questionsg#3
Conversation
Implements mcp__obsidian__ask_user tool to allow users to answer Claude's questions directly in the Obsidian UI with an interactive modal. Changes: - Add AskUserQuestionModal component for displaying questions with multiple-choice options - Add ask_user MCP tool to ObsidianMcpServer - Add CSS styling for the modal interface Users can now interact with Claude's questions through a native Obsidian modal instead of via command line, improving the UX when Claude needs user input or clarification. Closes #1
Add thorough test coverage for the AskUserQuestion modal feature including unit tests, integration tests, and edge cases. Changes: - Add 24 integration tests for AskUserQuestionModal component - Test modal initialization and rendering - Test single-select and multi-select behavior - Test "Other" option with custom input - Test submit behavior and answer formatting - Test modal lifecycle and cleanup - Add 15 tests for ObsidianMcpServer ask_user tool - Test tool invocation with various question configurations - Test multi-select questions - Test edge cases (empty answers, special characters, long text) - Update Modal mock to support Obsidian HTMLElement extensions - Add addClass, removeClass, toggleClass, hasClass methods - Add empty, createEl, createDiv, createSpan methods - Add setText method for text content updates All new tests passing (39 tests total). Test coverage now includes full user interaction flows for answering Claude's questions in the UI.
|
@claude review this |
|
Claude finished @Roasbeef's task in 1m 18s —— View job Code Review Complete ✅
SummaryThis is a well-implemented feature with comprehensive test coverage and clean code. The AskUserQuestion modal integration is solid and follows Obsidian conventions. I found a few areas for improvement, but nothing blocking. Strengths✅ Excellent test coverage - 645+ lines of thorough tests covering edge cases Issues & Recommendations🟡 Minor Issues1. Missing error handling in MCP tool (src/agent/ObsidianMcpServer.ts:437-448)The Recommendation: tool(
"ask_user",
// ... existing config
async (args) => {
try {
const answers = await showAskUserQuestionModal(app, args.questions);
return {
content: [
{
type: "text" as const,
text: JSON.stringify({ answers }, null, 2),
},
],
};
} catch (error) {
return {
content: [
{
type: "text" as const,
text: JSON.stringify({ error: "User cancelled the question dialog" }, null, 2),
},
],
isError: true,
};
}
}
),2. Modal doesn't handle cancellation (src/views/AskUserQuestionModal.ts:148-158)The modal has no "Cancel" button, and closing it without submitting doesn't reject the promise. This can leave the agent waiting indefinitely. Recommendations:
Example: onOpen() {
// ... existing code
// Add cancel button
const cancelBtn = buttonContainer.createEl("button", {
text: "Cancel",
});
cancelBtn.addEventListener("click", () => {
this.resolve(null); // or reject with an error
this.close();
});
// Handle ESC key
this.scope.register([], "Escape", () => {
this.resolve(null);
this.close();
});
}3. Type mismatch in resolve signature (src/views/AskUserQuestionModal.ts:14)The private resolve: (answers: Record<string, string> | null) => void;
// OR use a reject callback
private reject: (error: Error) => void;4. Missing validation for empty/required questions (src/views/AskUserQuestionModal.ts:148-158)The modal allows submitting without answering any questions (returns empty strings). Consider whether this is desired behavior or if validation is needed. Recommendation: private submit() {
// Validate all questions have answers
const hasUnanswered = this.questions.some((_, i) => {
const answers = this.answers.get(i) || [];
return answers.length === 0 || answers[0] === "";
});
if (hasUnanswered) {
new Notice("Please answer all questions");
return;
}
// ... existing submit logic
}5. CSS class naming inconsistency (styles.css:950-1028)The CSS uses both 🟢 Suggestions for Enhancement6. Consider accessibility improvements
Example: optBtn.setAttribute("role", "button");
optBtn.setAttribute("aria-pressed", "false");
optBtn.setAttribute("tabindex", "0");7. Header character limit documentationThe schema says
8. Multi-select UX improvementFor multi-select questions, consider adding:
9. Test coverage for modal lifecycleThe tests cover most scenarios, but could add:
Architecture ReviewModal Implementation ✅
MCP Integration ✅
State Management ✅
Test QualityThe test suite is exceptional:
One gap: No tests for modal cancellation/rejection scenarios (because that feature doesn't exist yet - see issue #2). Security Review✅ No security issues identified:
Performance✅ No performance concerns:
Style & Conventions✅ Code style is clean:
✅ CSS follows Obsidian patterns:
Final RecommendationAPPROVE with minor improvements suggested. This is production-ready code with solid implementation. The main improvement needed is handling modal cancellation to prevent agents from hanging. The other suggestions are nice-to-haves that can be addressed in follow-up PRs. Priority Improvements:
Great work on this feature! The test coverage is particularly impressive. 🎉 |
Summary
This PR implements an interactive
AskUserQuestionmodal that enables Claude to ask users questions during conversations. When Claude needs clarification on preferences, implementation choices, or decisions, it can now display a native Obsidian modal with up to 4 questions, each supporting 2-4 predefined options plus an "Other" field for custom responses. This mirrors theAskUserQuestiontool from the Claude Agent SDK, providing a seamless interactive experience within Obsidian.Implementation
The core implementation consists of
AskUserQuestionModal.ts, a new modal class extending Obsidian'sModalAPI. The modal renders questions with header badges for quick context (e.g., "Auth method", "Library"), descriptive option buttons, and supports both single-select and multi-select modes. Users can click predefined options or toggle an "Other" input for custom responses. The modal resolves a promise with the collected answers, making it easy to integrate into async agent workflows.MCP Integration
The modal is exposed as an
ask_usertool in the Obsidian MCP server (ObsidianMcpServer.ts). This allows Claude to invoke the tool duringquery()execution, presenting questions to the user and receiving structured answers. The tool follows the SDK's question schema withquestion,header,options, andmultiSelectfields, ensuring compatibility with existing Claude Code conventions.Testing
Comprehensive test coverage is added in
tests/integration/views/AskUserQuestionModal.test.ts(645+ lines). Tests verify modal construction, question rendering, single/multi-select behavior, "Other" input toggle, answer collection, and edge cases like empty submissions. The Modal mock intests/mocks/obsidian/Modal.mock.tswas enhanced to support DOM operations needed for testing. Additional integration tests inObsidianMcpServer.test.tsverify the tool integration.Styling
New CSS classes prefixed with
claude-code-ask-provide styling for the modal: question headers appear as colored badges, option buttons have hover/selected states, and the layout is clean and consistent with Obsidian's design language. The "Other" input field toggles visibility when selected and receives focus automatically for a smooth user experience.