Skip to content

fix: resolve plugin presets before conversation start - #339

Open
ShivangiRay wants to merge 1 commit into
OpenHands:mainfrom
ShivangiRay:codex/fix-plugin-preset-conversation
Open

fix: resolve plugin presets before conversation start#339
ShivangiRay wants to merge 1 commit into
OpenHands:mainfrom
ShivangiRay:codex/fix-plugin-preset-conversation

Conversation

@ShivangiRay

Copy link
Copy Markdown

Summary

Resolve and validate plugin sources in the plugin-preset runner before creating the remote conversation. The conversation receives verified local paths rather than remote coordinates, preventing lazy server-side fetch failures from surfacing as opaque errors during event polling.

Root cause

Plugin presets passed remote plugin coordinates directly to the in-sandbox agent server. Plugin fetching happens lazily when the conversation starts, so failures appear as 500 responses while reading conversation events.

Validation

  • uv run pytest tests/test_preset_router.py -q (97 passed, 41 skipped)
  • pre-commit run --files openhands/automation/presets/plugin/sdk_main.py tests/test_preset_router.py --show-diff-on-failure

Related: OpenHands/OpenHands#16611

AI assistance was used to prepare this change; the submitter reviewed the diff and test results.

Copy link
Copy Markdown

Thanks for this — the root-cause analysis matches what we isolated (prompt preset completes, any plugin preset 500s on /api/conversations/{id}/events), and resolving/validating plugin sources before the remote conversation is created is exactly the right fix.

This is blocking a real automation for us: an event-triggered "issue orchestrator" (single orchestrator that delegates to file-based specialist sub-agents bundled in a plugin, per the Shape A pattern in the automation docs). The automation is built and deployed but disabled until this lands; we have no code changes needed on our side once it merges — just re-enable and re-trigger.

The PR currently shows mergeable with PR-title checks green and a review requested from @hieptl. Is there anything else needed to finalize it — e.g., a live Cloud-sandbox validation run, additional CI, or info from the reporter? Happy to help validate: once it's merged/deployed I can re-enable our plugin-preset automation and run the no-op-plugin repro and a real end-to-end run to confirm the 500 is gone.

This comment was posted by an AI agent (OpenHands) on behalf of the repository maintainer (chkrause), who reported the upstream issue (#16611).

@all-hands-bot

Copy link
Copy Markdown
Contributor

👋 This PR needs a couple of things fixed before OpenHands can review it:

  • the PR description's HUMAN: section needs at least 20 characters describing what you tested, not just the template placeholder

Push an update once this is addressed and this check re-runs automatically.

This is an automated check - no AI was used to generate this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants