Fix Jira Composio subdomain authorization#1733
Conversation
📝 WalkthroughWalkthroughThis PR adds required Atlassian subdomain collection and validation to the Jira integration flow. The modal now collects the subdomain input, normalizes it, validates it before authorization, and recovers from missing-subdomain backend errors with targeted UI feedback instead of exposing raw provider errors. ChangesJira Atlassian Subdomain Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/components/composio/ComposioConnectModal.test.tsx (1)
169-189: ⚡ Quick winExtend this scenario to verify the retry path.
This covers the inline remap, but the linked Jira bug is specifically about retrying authorization without restarting the flow. Please continue this test by clicking Connect Jira again after the inline error and asserting a second successful
authorize('jira', { subdomain: 'acme' })call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/composio/ComposioConnectModal.test.tsx` around lines 169 - 189, Extend the test in ComposioConnectModal.test.tsx to exercise the retry path: after the first mocked rejection of authorize and after asserting the inline subdomain error is shown (using screen.findByText and the subdomain input), mock vi.mocked(authorize) to resolve successfully for the next call, trigger fireEvent.click on the "Connect Jira" button again and then assert that authorize was called a second time with the expected arguments (e.g., authorize('jira', { subdomain: 'acme' })), and that the success path behaviors (such as openUrl or modal close as appropriate) occurred; use the existing jiraToolkit, ComposioConnectModal render, screen and fireEvent helpers to locate and interact with the UI and ensure no duplicate error text remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/composio/ComposioConnectModal.tsx`:
- Around line 63-67: The Jira recovery matcher is only checking for
"ConnectedAccount_MissingRequiredFields" so it misses backend responses that
return numeric error code 612; update isMissingAtlassianSubdomainError to also
treat the numeric code by adding a check for "612" (e.g. include /\b612\b/i in
the first condition or combine with /ConnectedAccount_MissingRequiredFields/)
while keeping the existing subdomain check (/subdomain|Your Subdomain/i) so the
function returns true for either the textual code or the numeric 612 when
subdomain text is present.
- Around line 45-53: normalizeAtlassianSubdomain currently leaves query strings
or hash fragments on pasted URLs which breaks host validation; update
normalizeAtlassianSubdomain to also strip any query string or fragment after
trimming/protocol removal (e.g. remove everything from the first '?' or '#' like
using a regex replace for /[?#].*$/) before removing ports and the
ATLASSIAN_DOMAIN_SUFFIX, ensuring you still trim, lowercase, strip protocol,
strip path, strip port, then remove the ATLASSIAN_DOMAIN_SUFFIX if present.
---
Nitpick comments:
In `@app/src/components/composio/ComposioConnectModal.test.tsx`:
- Around line 169-189: Extend the test in ComposioConnectModal.test.tsx to
exercise the retry path: after the first mocked rejection of authorize and after
asserting the inline subdomain error is shown (using screen.findByText and the
subdomain input), mock vi.mocked(authorize) to resolve successfully for the next
call, trigger fireEvent.click on the "Connect Jira" button again and then assert
that authorize was called a second time with the expected arguments (e.g.,
authorize('jira', { subdomain: 'acme' })), and that the success path behaviors
(such as openUrl or modal close as appropriate) occurred; use the existing
jiraToolkit, ComposioConnectModal render, screen and fireEvent helpers to locate
and interact with the UI and ensure no duplicate error text remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10c90b9c-574e-43fe-9fce-3ee1a8b24b8d
📒 Files selected for processing (2)
app/src/components/composio/ComposioConnectModal.test.tsxapp/src/components/composio/ComposioConnectModal.tsx
| export function normalizeAtlassianSubdomain(value: string): string { | ||
| let normalized = value.trim().toLowerCase(); | ||
| normalized = normalized.replace(/^https?:\/\//, ''); | ||
| normalized = normalized.replace(/\/.*$/, ''); | ||
| normalized = normalized.replace(/:\d+$/, ''); | ||
| if (normalized.endsWith(ATLASSIAN_DOMAIN_SUFFIX)) { | ||
| normalized = normalized.slice(0, -ATLASSIAN_DOMAIN_SUFFIX.length); | ||
| } | ||
| return normalized; |
There was a problem hiding this comment.
Strip query strings and hash fragments from pasted Atlassian URLs.
A pasted Jira URL like https://acme.atlassian.net?atlOrigin=... or ...#... currently normalizes to acme.atlassian.net?..., which then fails validation even though the host is valid. That makes some “paste the full URL” flows fail unnecessarily.
Suggested fix
export function normalizeAtlassianSubdomain(value: string): string {
let normalized = value.trim().toLowerCase();
normalized = normalized.replace(/^https?:\/\//, '');
+ normalized = normalized.replace(/[?#].*$/, '');
normalized = normalized.replace(/\/.*$/, '');
normalized = normalized.replace(/:\d+$/, '');
if (normalized.endsWith(ATLASSIAN_DOMAIN_SUFFIX)) {
normalized = normalized.slice(0, -ATLASSIAN_DOMAIN_SUFFIX.length);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/components/composio/ComposioConnectModal.tsx` around lines 45 - 53,
normalizeAtlassianSubdomain currently leaves query strings or hash fragments on
pasted URLs which breaks host validation; update normalizeAtlassianSubdomain to
also strip any query string or fragment after trimming/protocol removal (e.g.
remove everything from the first '?' or '#' like using a regex replace for
/[?#].*$/) before removing ports and the ATLASSIAN_DOMAIN_SUFFIX, ensuring you
still trim, lowercase, strip protocol, strip path, strip port, then remove the
ATLASSIAN_DOMAIN_SUFFIX if present.
| export function isMissingAtlassianSubdomainError(message: string): boolean { | ||
| return ( | ||
| /ConnectedAccount_MissingRequiredFields/i.test(message) && | ||
| /subdomain|Your Subdomain/i.test(message) | ||
| ); |
There was a problem hiding this comment.
Handle Composio error code 612 in the Jira-field matcher.
The Jira recovery path only recognizes ConnectedAccount_MissingRequiredFields, but the failure is also called out as code 612. If the backend returns only the numeric code, this branch will miss and the raw provider payload will still be surfaced through Authorization failed: ....
Suggested fix
export function isMissingAtlassianSubdomainError(message: string): boolean {
return (
- /ConnectedAccount_MissingRequiredFields/i.test(message) &&
+ /(ConnectedAccount_MissingRequiredFields|(?:"code"|code)\D*612\b)/i.test(message) &&
/subdomain|Your Subdomain/i.test(message)
);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/components/composio/ComposioConnectModal.tsx` around lines 63 - 67,
The Jira recovery matcher is only checking for
"ConnectedAccount_MissingRequiredFields" so it misses backend responses that
return numeric error code 612; update isMissingAtlassianSubdomainError to also
treat the numeric code by adding a check for "612" (e.g. include /\b612\b/i in
the first condition or combine with /ConnectedAccount_MissingRequiredFields/)
while keeping the existing subdomain check (/subdomain|Your Subdomain/i) so the
function returns true for either the textual code or the numeric 612 when
subdomain text is present.
Summary
*.atlassian.netURLs and sendextra_params.subdomaintoopenhuman.composio_authorizeConnectedAccount_MissingRequiredFieldssubdomain errors back to the inline Jira form instead of showing raw provider payloadsCloses #1702
Validation
pnpm --filter openhuman-app exec vitest run --config test/vitest.config.ts src/components/composio/ComposioConnectModal.test.tsxpnpm typecheckpnpm --filter openhuman-app exec prettier --check src/components/composio/ComposioConnectModal.tsx src/components/composio/ComposioConnectModal.test.tsxpnpm --filter openhuman-app exec eslint src/components/composio/ComposioConnectModal.tsx src/components/composio/ComposioConnectModal.test.tsx --ext .ts,.tsxpython -m diff_cover.diff_cover_tool app/coverage/lcov.normalized.info --compare-branch=origin/main --fail-under=80 --markdown-report app/coverage/diff-coverage-jira.md(100% diff coverage)Notes
app/src-tauri/vendor/tauri-cef/crates/tauri/Cargo.toml, which blockspnpm rust:check. The change is limited to frontend TS/TSX files and the frontend checks above pass.Summary by CodeRabbit
Release Notes