fix: require Atlassian Rovo for Jira automation - #449
Conversation
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
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. |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable — Both fixes are directionally correct, but the jira-issue-to-pr change exposes an implementation mismatch worth resolving.
Summary of changes:
- Commit 1 (
9603ce8):manifest.json— swap required integrationjira→atlassian-rovo. Theatlassian-rovointegration exists in the catalog and is the right Atlassian surface (official hosted MCP with OAuth 2.1), so the ID reference is valid and the tests will pass. - Commit 2 (
59fe613): Fix a"integration"→"integrations"typo forupstream-fork-syncinopenhands-extensions.json, regenerateskills/index.js, and update the test's expected category count from 5 → 6. All three files are correct and consistent.
Key concern — manifest declares a connection the script never uses:
manifest.json now tells the UI that atlassian-rovo (the Atlassian Rovo OAuth MCP) is required before the automation can run. But scripts/main.py line 267-268 explicitly drops the entire mcp_config before spawning each agent conversation:
# Drop mcp_config to avoid MCP connection failures at conversation creation time.
agent_settings.pop("mcp_config", None)So the Atlassian Rovo MCP connection that users are gated on connecting will never actually be injected into the automation — the poller uses direct Jira REST API calls with Basic Auth (email + API token), and the spawned conversations lose all MCP config. This isn't a correctness bug in the current code (the automation still works via tokens), but it creates a misleading UX where users are asked to go through an OAuth flow for a connection that is immediately discarded.
If the intent is to eventually use the Atlassian Rovo MCP inside the spawned conversations (replacing the Basic Auth polling), the agent_settings.pop("mcp_config", None) workaround should be removed (or made selective) as the follow-up. If the intent is only to use token-based auth, the manifest should declare no Atlassian integration (or a lighter integration that maps to the API-token flow). The exampleImplementation text now says both are needed simultaneously, which is accurate for the current interim state but worth clarifying in a follow-up.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
Metadata/config-only changes. The automation script behavior is unchanged. The mismatch between the declared integration and the script's MCP-dropping workaround is a UX/documentation gap, not a runtime regression.
VERDICT:
✅ Worth merging — the integration ID switch is correct, the typo fix is correct, and CI is restored. The manifest/script mismatch predates this PR and is a follow-up concern.
KEY INSIGHT:
Requiring atlassian-rovo in the manifest is the right direction, but the automation's agent_settings.pop("mcp_config", None) workaround means the connection is never actually used — that workaround should be the next thing to address.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing. See the customization docs for the required frontmatter format.- Re-request a review — the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
- When your PR is merged, the guideline file goes through normal code review by repository maintainers.
Resolve with AI? Install the iterate skill in your agent and run
/iterateto automatically drive this PR through CI, review, and QA until it's merge-ready.Was this review helpful? React with 👍 or 👎 to give feedback.
| "jira": { | ||
| "message": "Reads the project for issues carrying the trigger label." | ||
| "atlassian-rovo": { | ||
| "message": "Provides the Atlassian Rovo MCP connection used to access Jira data." |
There was a problem hiding this comment.
🟡 Suggestion: The message says the Atlassian Rovo MCP connection is "used to access Jira data", but scripts/main.py lines 267-268 drop the entire mcp_config before spawning each agent conversation — so this connection is never injected. If the automation will eventually use the MCP for Jira access, remove that agent_settings.pop("mcp_config", None) workaround; if it will keep using direct API-token calls, update this message (or switch to a lighter integration type) to avoid asking users to OAuth into Atlassian Rovo for a connection that never gets used.
Summary
jiraautomation requirement with the existingatlassian-rovoMCP integration.The previous
jiracatalog entry is HTTP/OpenAPI-only. Canvas's recommended automation flow only renders MCP-compatible integrations, so the Jira dependency was silently omitted even though the automation declared it.Related issue
Closes #448
Verification
npm run build:automationsPATH="$HOME/.nvm/versions/node/v22.23.1/bin:$PATH" uv run pytest -q tests/test_catalogs.py tests/test_automation_setup.py tests/test_interface_manifest.py(67 passed)python3 scripts/sync_extensions.py --check(passes; existing non-blocking marketplace coverage warning remains)