fix(acp): unblock question tool in ACP mode

[Haohao-end]

Issue for this PR

Closes #17920

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

ACP mode was not handling question.asked, so question tool requests could hang indefinitely waiting for a reply.

This PR adds a question.asked branch in packages/opencode/src/acp/agent.ts and bridges the question flow through the existing ACP requestPermission() interaction. The selected ACP response is then mapped back to the existing question APIs:

• sdk.question.reply(...) on success
• sdk.question.reject(...) on cancellation, invalid option ids, or request failures

This fixes the hang because Question.ask() now always receives either a reply or a rejection instead of waiting forever.

I kept the change minimal and limited it to the ACP bridge and targeted regression tests.

How did you verify your code works?

I verified it locally by:

• adding targeted tests in packages/opencode/test/acp/event-subscription.test.ts
• covering successful reply flow
• covering cancellation flow
• covering invalid ACP option id flow
• running:
  • bun test test/acp/event-subscription.test.ts
  • bun typecheck

Screenshots / recordings

N/A

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

[github-actions]

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate Found

PR #17921: fix(acp): handle question.asked event to prevent hanging

• fix(acp): handle question.asked event to prevent hanging #17921

This PR is directly related - it addresses the same issue (handling question.asked in ACP mode to prevent hanging). The current PR #18657 may be a newer attempt to fix the same problem, or PR #17921 may already be addressing this issue.

Recommend checking the status of PR #17921 and the relationship between these two PRs before merging.

[Haohao-end]

I noticed there is an earlier PR for the same issue (#17921).

This PR takes a slightly more defensive approach:

• handles cancellation by rejecting the question request
• handles invalid ACP option ids by rejecting instead of hanging
• supports multi-select question flow
• adds targeted regression coverage for reply / cancellation / invalid option id cases

All checks are passing on this PR, so I’m leaving it open in case the additional edge-case handling is useful.

[atharvau]

Question Tool Support in ACP - APPROVED ✅

Summary: Implements question tool support in ACP mode by handling question.asked events and routing them through the permission UI system.

Critical Feature Addition ✅

Problem solved: Question tools were causing hangs in ACP mode because question.asked events weren't being handled, leading to indefinite waits for user responses that never came.

Solution implemented:

• Routes question prompts through the same permission UI system
• Handles multiple questions in sequence
• Provides proper rejection mechanism for unsupported question types
• Queues questions behind existing permission prompts to maintain UX consistency

Technical Implementation ✅

Event Handling Architecture:

case "question.asked": {
  const question = event.properties
  const session = this.sessionManager.tryGet(question.sessionID)
  if (!session) return

  const prev = this.promptQueues.get(question.sessionID) ?? Promise.resolve()
  // ... queue handling logic
}

Quality analysis:

• Consistent queueing: Uses same prompt queue system as permissions for proper serialization
• Error handling: Comprehensive error catching with proper logging
• Session validation: Properly validates session exists before processing
• Resource cleanup: Queue cleanup in finally block prevents memory leaks

Question Mapping Logic ✅

Option mapping:

function toQuestionOptions(item: QuestionInfo) {
  if (item.multiple === true) return  // Reject multi-select
  if (item.options.length === 0) return // Reject empty options
  return [
    ...item.options.map((opt, i) => ({
      optionId: String(i),
      kind: "allow_once" as const,
      name: opt.label,
    })),
    {
      optionId: "reject",
      kind: "reject_once" as const, 
      name: "Dismiss",
    },
  ]
}

Strengths:

• Proper validation: Rejects unsupported question types (multiple selection, empty options)
• User-friendly: Always provides "Dismiss" option for user control
• Clear mapping: Numeric IDs map cleanly to array indices
• Type safety: Proper const assertions for option kinds

Answer Processing ✅

Answer conversion:

function toQuestionAnswer(item: QuestionInfo, id: string): QuestionAnswer | undefined {
  const index = Number(id)
  if (!Number.isInteger(index)) return
  const opt = item.options[index]
  if (!opt) return
  return [opt.label]
}

Quality:

• Safe parsing: Validates numeric ID before using as array index
• Bounds checking: Ensures option exists before accessing
• Expected format: Returns answer in expected array format

User Experience Design ✅

Question Presentation:

function toQuestionText(item: QuestionInfo, i: number, total: number) {
  return [
    total > 1 ? `Question ${i + 1} of ${total}` : undefined,
    item.question,
    ...item.options.map((opt, index) => `${index + 1}. ${opt.label}: ${opt.description}`),
    item.custom === false ? undefined : "Custom answers are not supported in this ACP prompt.",
  ]
    .filter((item): item is string => !!item)
    .join("\n")
}

UX strengths:

• Progress indication: Shows "Question X of Y" for multi-question sequences
• Rich formatting: Includes both question text and option descriptions
• Clear expectations: Explicitly states custom answer limitations
• Clean presentation: Filters out undefined elements for clean output

Test Coverage Excellence ✅

Comprehensive Test Scenarios:

1. Basic question handling: Single and multiple questions with proper answer mapping
2. Unsupported question rejection: Multi-select questions properly rejected
3. Queue integration: Questions wait behind existing permissions
4. Tool call ID generation: Proper ID handling for single vs multiple questions
5. Error paths: Various failure scenarios with proper cleanup

Test Quality Assessment:

• Realistic scenarios: Tests actual question flow with mock ACP connection
• Edge cases: Tests unsupported question types, empty options
• Concurrency: Tests queue behavior with mixed permission/question events
• State verification: Validates both API calls and internal state changes

Breaking Changes Analysis

API Compatibility: ✅ Perfect - only adds new functionality
Behavioral Changes: ✅ Expected - questions now work instead of hanging
Queue System: ✅ Enhanced - renamed permissionQueues to promptQueues (internal change)
Error Handling: ✅ Improved - better error isolation and logging

Security & Performance

Security: ✅ Good input validation, bounded option arrays
Performance: ✅ Minimal overhead, efficient queue management
Resource Management: ✅ Proper queue cleanup, no memory leaks
DoS Protection: ✅ Bounded processing, rejects malformed questions

Code Quality Observations

Architecture Excellence ✅

1. Consistent patterns: Follows same event handling pattern as permissions
2. Function decomposition: Well-separated concerns with single-purpose functions
3. Error boundaries: Comprehensive error handling at each level
4. Type safety: Proper TypeScript types throughout

Maintainability ✅

• Clear naming: Functions clearly describe their purpose
• Modular design: Each transformation function handles one concern
• Testable units: Functions are easily testable in isolation
• Documentation: Type imports clearly show dependencies

Real-World Impact

Before this fix:

• ❌ Question tools hang indefinitely in ACP mode
• ❌ Users forced to kill session to recover
• ❌ Tools that depend on user input completely broken

After this fix:

• ✅ Question tools work seamlessly in ACP mode
• ✅ Questions appear in familiar permission UI
• ✅ Users can dismiss questions they don't want to answer
• ✅ Multi-question sequences work properly

Testing Recommendations

Consider additional testing for:

1. Large question sets: Performance with many questions
2. Malformed questions: Questions with invalid option structures
3. Concurrent sessions: Question handling across multiple sessions
4. Session cleanup: Question queue cleanup when sessions are destroyed

Summary

This is an excellent feature implementation that:

Functional Excellence ✅

• ✅ Completely unblocks question tools in ACP mode
• ✅ Provides consistent user experience with permissions
• ✅ Handles complex multi-question scenarios
• ✅ Properly rejects unsupported question types

Technical Excellence ✅

• ✅ Clean integration with existing permission queue system
• ✅ Comprehensive error handling and logging
• ✅ Type-safe implementation throughout
• ✅ Excellent test coverage with realistic scenarios

UX Excellence ✅

• ✅ Questions presented clearly with context
• ✅ Progress indication for multi-question sequences
• ✅ User always has option to dismiss
• ✅ Maintains queue order for consistent experience

Strong recommendation: APPROVE and merge - this fixes a critical functionality gap that was making question tools unusable in ACP mode.

---

Risk Level: Low - well-tested addition with comprehensive error handling
User Impact: High - unblocks entire class of tools that were broken in ACP
Breaking Changes: None - purely additive functionality
Test Coverage: Excellent - comprehensive scenarios and edge cases covered

[atharvau]

Code Review Summary

Overall Assessment

✅ APPROVED - This is a well-implemented feature addition that properly handles question tools in ACP mode.

What This Adds

Implements support for the question tool in ACP (Agent Client Protocol) mode by:

1. Adding question.asked event handling: Processes question requests from the opencode core
2. UI mapping: Maps opencode question format to ACP permission request format
3. Response handling: Converts ACP user responses back to opencode question answers
4. Queue integration: Uses the same prompt queue as permissions to prevent conflicts

Code Quality Analysis

✅ Excellent Implementation:

• Proper queue management: Questions use the same prompt queue as permissions, preventing UI conflicts
• Comprehensive error handling: Graceful fallbacks when questions can't be mapped or answered
• Type safety: Proper TypeScript types imported and used throughout
• Clean separation: Question handling is well-separated into helper functions

✅ Robust Error Handling:

• Rejects questions when mapping fails
• Handles connection errors gracefully
• Validates option selections before processing
• Falls back to rejection when user dismisses

✅ Smart Design Decisions:

• Reuses permission UI: Questions are presented as permission requests, maintaining consistency
• Queue sharing: Uses existing promptQueues to serialize interactions per session
• Tool call ID generation: Handles both single and multi-question scenarios properly
• Limitations handling: Correctly rejects unsupported features (multiple selection)

Test Coverage

✅ Comprehensive testing with 3 detailed test cases:

• Basic question handling with multiple questions
• Rejection of unsupported question types (multiple selection)
• Queue ordering when questions follow permissions

Security & Performance

✅ No concerns:

• Input validation on all user selections
• Proper error handling prevents crashes
• No performance implications

Edge Cases Handled

• ✅ Unsupported question types (multiple selection) are properly rejected
• ✅ Invalid option selections are handled gracefully
• ✅ Questions properly queue behind existing prompts
• ✅ Tool call IDs handle single vs multi-question scenarios
• ✅ Network/SDK errors don't crash the system

Minor Observations

1. Good naming: Renamed permissionQueues to promptQueues to better reflect dual usage
2. Helper functions: Well-organized helper functions for mapping between formats
3. Consistent patterns: Follows same error handling patterns as permission requests

Limitations (By Design)

• Multiple selection questions are not supported (correctly rejected)
• Custom text input is not supported (documented in UI)

Verdict: This is a high-quality feature implementation with excellent error handling and test coverage. The design properly integrates with existing systems. Ready to merge! 🚀

[atharvau]

Code Review Summary

Overall Assessment: ✅ APPROVE

Solid implementation that adds question tool support to ACP with proper queuing and error handling.

✅ Strengths:

• Feature Completeness: Full question tool support with multi-question handling
• Proper Queuing: Questions wait behind permissions using shared promptQueues
• Error Handling: Comprehensive error handling with fallback to rejection
• Extensive Testing: 180+ lines of test coverage including edge cases and queuing behavior
• Type Safety: Proper imports and type definitions from SDK

Implementation Quality:

• ✅ Clean Architecture: Separates concerns between request handling, mapping, and response
• ✅ Async Safety: Proper Promise handling and queue management
• ✅ Mapping Logic: Well-structured conversion between question formats and ACP options
• ✅ Tool Call IDs: Smart handling of multi-question scenarios with proper ID generation

Security Analysis:

• ✅ Input Validation: Checks for valid question formats and options
• ✅ Safe Rejections: Proper rejection handling for unsupported question types
• ✅ No Injection Risks: All data properly escaped and validated

Test Coverage:

• ✅ Happy Path: Multi-question scenarios with different option selections
• ✅ Error Cases: Unsupported question types (multiple choice) properly rejected
• ✅ Queue Integration: Tests that questions wait behind existing permission prompts
• ✅ Edge Cases: Various question configurations and response mappings

Code Quality:

• ✅ Helper Functions: Well-structured utility functions for mapping and text generation
• ✅ Error Logging: Comprehensive logging for debugging and monitoring
• ✅ Consistent Patterns: Follows existing codebase patterns for event handling

Potential Limitations:

• 🟡 Multiple Choice: Currently unsupported (documented limitation)
• 🟡 Custom Answers: Not supported in ACP (properly communicated to users)

Minor Suggestion: Consider adding a configuration option for custom answer support in the future.

Recommendation: Ready to merge - well-implemented feature with excellent test coverage