Skip to content

feat: handle OpenCode question requests#33

Closed
slegarraga wants to merge 5 commits into
ben-vargas:mainfrom
slegarraga:fix/question-asked-handler
Closed

feat: handle OpenCode question requests#33
slegarraga wants to merge 5 commits into
ben-vargas:mainfrom
slegarraga:fix/question-asked-handler

Conversation

@slegarraga

Copy link
Copy Markdown
Contributor

Summary

  • add an opt-in onQuestionAsked model setting for OpenCode question.asked stream events
  • answer questions through question.reply or reject them through question.reject
  • document the handler and cover answer/reject flows in stream tests

Closes #15.

Validation

  • npm test
  • npm test -- --run src/opencode-language-model.test.ts src/validation.test.ts
  • npm run typecheck
  • npm run lint (exits 0; existing no-explicit-any warnings remain in debug logging)
  • npm run build

slegarraga and others added 3 commits June 11, 2026 13:19
…n stream teardown

- Check question.reply/question.reject results via extractSdkResult:
  fields-style clients report API failures in the result instead of
  throwing, so failures were recorded as answered while OpenCode kept
  waiting on the question (issue ben-vargas#15).
- Pass the request abort signal to question.reply/question.reject so
  in-flight question requests are cancelled on stream teardown,
  matching session.prompt and session.abort.
- Export the question types from the package entry point so handlers
  can be typed outside inline settings.
- Cover reply API errors, handler throws, and the undefined fallthrough
  in stream tests.
@ben-vargas

ben-vargas commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Thanks for this contribution @slegarraga, here's a summary of my review with Fable and a few tweaks...

Fable 5 Generated:

While reviewing I found one real bug plus two smaller gaps. Since they're all mechanical, and one depends on changes that landed on main after you branched, I pushed the fixes directly to your branch (thanks for leaving "allow edits by maintainers" on) instead of asking for another round:

  1. question.reply/question.reject API errors were silently swallowed. The provider creates SDK clients with responseStyle: "fields" and no throwOnError, so API failures (unknown requestID, already-answered question, malformed answers) resolve as { data, error } rather than throwing. The try/catch never saw them: a failed reply was recorded as answered, and the stream kept waiting on a question OpenCode never received an answer to — the same hang as Add question.asked Support #15, just invisible. Fixed by checking results through the existing extractSdkResult helper.

  2. Merged main and threaded the request abort signal through the question calls. main moved under you (fix: abort SSE event subscription on stream close so process can exit #30fix: report stop finish reason when structured output completes #32). After the merge, question.reply/question.reject now receive requestAbortController.signal like session.prompt/session.abort do, so in-flight question requests are cancelled on stream teardown.

  3. Exported the new question types (OpencodeQuestionRequest, OpencodeQuestionResponse, etc.) from the package entry point so handlers can be typed outside inline settings, and added tests covering the reply-error, handler-throws, and undefined-fallthrough paths.

Everything passes locally (395 tests, typecheck, lint, build) and CI is running now.

One ask before merging: since you presumably have a real OpenCode flow that triggers question.asked, could you pull the updated branch and confirm everything still works as you expect on your side; both the answer and reject paths if you can? I'd like a real-world check on top of the unit tests before this ships. Thanks again!

ben-vargas added a commit that referenced this pull request Jun 11, 2026
…failures (#35)

* fix: surface permission reply API errors instead of recording silent failures

Managed clients use responseStyle "fields" without throwOnError, so a
failed permission.reply resolves as { error } rather than throwing.
replyToPendingApprovals never inspected the result, so API failures were
silently recorded as replied and the approval was never retried.

Check the result via extractSdkResult, matching the question-handling
fix in #33: on error, log a warning, surface it in the response
warnings, and skip adding the id to repliedApprovalIds so the next turn
retries the reply.

* docs: add changelog entry for permission reply error handling
@slegarraga slegarraga closed this Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add question.asked Support

2 participants