fix(composio): collect Dynamics 365 org name via required-fields registry (#2127)#2181
Merged
senamakel merged 3 commits intoMay 20, 2026
Conversation
Declarative per-toolkit registry for provider-specific required fields the Composio connect flow must collect before calling openhuman.composio_authorize. Each entry binds a field key (also used as the extra_params name on the backend request) to label / hint / placeholder / optional suffix / optional validator. Adding a new provider-specific field is a single registry entry — no per-toolkit branches in ComposioConnectModal. Seeded with: - whatsapp.waba_id (existing) - jira.subdomain (existing) — uses the shared subdomain-label validator - dynamics365.org_name — new, with .crm.dynamics.com suffix and the same subdomain-label validator so users see the same DNS-label guidance Composio enforces server-side. Refs tinyhumansai#2127.
…lds registry (tinyhumansai#2127) Composio rejects Dynamics 365 authorization with code 612 (`ConnectedAccount_MissingRequiredFields`) because the org name (the `myorg` part of `myorg.crm.dynamics.com`) is required server-side but the connect modal previously rendered only a generic OAuth button. Replace the hand-rolled `needsWabaId` / `needsAtlassianSubdomain` booleans with the new [ToolkitRequiredField] registry. The modal now renders the registry's required-field list generically (via the new RequiredFieldsForm subcomponent), validates per-field with optional custom validators, and threads the values verbatim into the `extra_params` object of `openhuman.composio_authorize`. The recovery phase that fires when Composio still returns code 612 is generalised from `needs-subdomain` to `needs-fields` so it re-uses the same generic renderer. User-visible impact: - Dynamics 365 connect flow now collects the org name upfront, with `.crm.dynamics.com` suffix and a hint that the short org name is required. - Local validation blocks submission when the field is empty or looks like a full URL. - Existing Jira subdomain + WhatsApp WABA id flows still work — they are now sourced from the same registry. i18n: 5 new keys (`dynamicsOrgNameLabel`, `dynamicsOrgNameHint`, `wabaIdHint`, `needsFieldsPrefix`, `needsFieldsSuffix`, `requiredFieldEmpty`) shipped for 12 locales. Closes tinyhumansai#2127.
Contributor
📝 WalkthroughWalkthroughComposioConnectModal is refactored from per-toolkit required-field handling to a registry-driven workflow. A new ChangesProvider-Specific Required Fields for Composio Connect
Sequence Diagram(s)sequenceDiagram
participant User
participant Modal as ComposioConnectModal
participant Validator as validateRequiredFieldValues
participant Authorize as Composio Authorize
autonumber
User->>Modal: render, update fieldValue[key]
User->>Modal: click "Connect" (handleConnect)
Modal->>Validator: validateRequiredFieldValues(requiredFields, fieldValues)
alt validation errors
Validator-->>Modal: error map with field errors
Modal-->>User: display inline errors, stay in idle/needs-fields
else validation succeeds
Validator-->>Modal: empty error map
Modal->>Modal: build extraParams from fieldValues
Modal->>Authorize: authorize(oauth, extraParams)
alt success
Authorize-->>Modal: success response
Modal-->>User: close modal, update connection
else code 612 (missing required fields)
Authorize-->>Modal: ConnectedAccount_MissingRequiredFields
alt registry has entries for toolkit
Modal-->>User: transition to needs-fields phase<br/>(show recovery copy, retry button, form)
else no registry entries
Modal-->>User: error state (no recovery path)
end
else other error
Authorize-->>Modal: error response
Modal-->>User: error state
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/components/composio/ComposioConnectModal.test.tsx (1)
359-361:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis assertion no longer verifies that the active validation error is cleared.
Line 360 still checks for the retired Atlassian-specific copy, so the test can pass even if
"This field is required"remains visible.Proposed patch
await waitFor(() => { - expect(screen.queryByText(/Please enter your Atlassian subdomain/i)).not.toBeInTheDocument(); + expect(screen.queryByText(/This field is required/i)).not.toBeInTheDocument(); });🤖 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 359 - 361, The test currently only asserts that the retired Atlassian-specific copy is gone, which can mask that the active validation error ("This field is required") still shows; update the assertion inside the waitFor in ComposioConnectModal.test.tsx to assert that the actual validation message ("This field is required") is not in the document (use expect(screen.queryByText(/This field is required/i)).not.toBeInTheDocument()) instead of or in addition to the Atlassian copy, keeping the waitFor wrapper and existing use of screen.queryByText so the test fails if the required-field validation isn't cleared.
🧹 Nitpick comments (1)
app/src/components/composio/ComposioConnectModal.tsx (1)
343-348: ⚡ Quick winUse the namespaced frontend logger instead of raw
console.*in the new authorization logs.These added log points should follow the project’s debug-namespace pattern so verbosity can be controlled consistently in frontend builds.
Proposed patch
-import { type ChangeEvent, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import createDebug from 'debug'; +import { type ChangeEvent, useCallback, useEffect, useMemo, useRef, useState } from 'react'; @@ +const debug = createDebug('composio:connect-modal'); @@ - console.debug( + debug( '[composio][authorize] → toolkit=%s has_extra_params=%s field_count=%d', toolkit.slug, Object.keys(extraParams).length > 0, requiredFields.length ); @@ - console.error( + debug( '[composio][authorize] failed toolkit=%s slug_check=%s', toolkit.slug, isMissingRequiredFieldsError(err) ); @@ - console.debug( + debug( '[composio][authorize] missing-required-fields toolkit=%s registry_field_count=%d', toolkit.slug, requiredFields.length );As per coding guidelines "Follow existing patterns for debug logging (e.g. the
debugnpm package with a namespace per area) ...".Also applies to: 365-369, 379-383
🤖 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 343 - 348, Replace raw console calls in ComposioConnectModal.tsx with the project's namespaced frontend debug logger (the same debug/namespace pattern used elsewhere in the app). Locate the console.debug/console.* calls around the authorize flow (the log using toolkit.slug, Object.keys(extraParams).length, requiredFields.length and the two other console blocks referenced) and swap them to use the namespaced logger (e.g., logger('composio:authorize') or the existing debug instance used by this component), preserving the message and interpolated values (toolkit.slug, extraParams, requiredFields) so verbosity is controllable via the debug namespace.
🤖 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.
Outside diff comments:
In `@app/src/components/composio/ComposioConnectModal.test.tsx`:
- Around line 359-361: The test currently only asserts that the retired
Atlassian-specific copy is gone, which can mask that the active validation error
("This field is required") still shows; update the assertion inside the waitFor
in ComposioConnectModal.test.tsx to assert that the actual validation message
("This field is required") is not in the document (use
expect(screen.queryByText(/This field is required/i)).not.toBeInTheDocument())
instead of or in addition to the Atlassian copy, keeping the waitFor wrapper and
existing use of screen.queryByText so the test fails if the required-field
validation isn't cleared.
---
Nitpick comments:
In `@app/src/components/composio/ComposioConnectModal.tsx`:
- Around line 343-348: Replace raw console calls in ComposioConnectModal.tsx
with the project's namespaced frontend debug logger (the same debug/namespace
pattern used elsewhere in the app). Locate the console.debug/console.* calls
around the authorize flow (the log using toolkit.slug,
Object.keys(extraParams).length, requiredFields.length and the two other console
blocks referenced) and swap them to use the namespaced logger (e.g.,
logger('composio:authorize') or the existing debug instance used by this
component), preserving the message and interpolated values (toolkit.slug,
extraParams, requiredFields) so verbosity is controllable via the debug
namespace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0021c2cb-52c3-43df-ab8a-0ad7b5daea0c
📒 Files selected for processing (16)
app/src/components/composio/ComposioConnectModal.test.tsxapp/src/components/composio/ComposioConnectModal.tsxapp/src/components/composio/toolkitRequiredFields.test.tsapp/src/components/composio/toolkitRequiredFields.tsapp/src/lib/i18n/chunks/ar-4.tsapp/src/lib/i18n/chunks/bn-4.tsapp/src/lib/i18n/chunks/en-4.tsapp/src/lib/i18n/chunks/es-4.tsapp/src/lib/i18n/chunks/fr-4.tsapp/src/lib/i18n/chunks/hi-4.tsapp/src/lib/i18n/chunks/id-4.tsapp/src/lib/i18n/chunks/it-4.tsapp/src/lib/i18n/chunks/pt-4.tsapp/src/lib/i18n/chunks/ru-4.tsapp/src/lib/i18n/chunks/zh-CN-4.tsapp/src/lib/i18n/en.ts
graycyrus
reviewed
May 19, 2026
Contributor
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Nice refactor — the declarative registry approach is clean and will scale well for future Composio integrations. The three-way alignment with #2127 checks out: required field rendered, validated, forwarded, and recovery phase generalized. Tests are thorough.
Two minor cleanup items below (neither blocking). CodeRabbit already caught the stale test assertion at line ~360 and the console.debug → namespaced logger nit, so I'm skipping those.
| Area | Files | Verdict |
|---|---|---|
| Registry | toolkitRequiredFields.ts |
Clean — frozen, typed, shared DNS-label validator |
| Modal refactor | ComposioConnectModal.tsx |
Clean — per-toolkit booleans eliminated, generic form |
| Tests | both test files | Solid coverage, parity assertions |
| i18n | 12 locales + en.ts |
All 6 new keys present |
| "account. We'll open a browser window, you approve access there, and this app will detect the connection automatically.", | ||
| 'composio.connect.isConnected': 'is connected.', | ||
| 'composio.connect.manage': 'Manage', | ||
| 'composio.connect.needsFieldsPrefix': 'To connect', |
Contributor
There was a problem hiding this comment.
[minor] Dead i18n keys left behind: composio.connect.needsSubdomain, needsSubdomainSuffix, subdomainRequired, and wabaIdRequired are no longer referenced by the modal (replaced by needsFieldsPrefix/needsFieldsSuffix and requiredFieldEmpty). They're harmless but add translation maintenance burden across 12 locales — worth a quick cleanup pass.
| * `<subdomain>.atlassian.net` — alphanumerics and hyphens, 1-63 chars, | ||
| * no leading/trailing hyphens. Rejects full URLs so users are not confused | ||
| * about what to paste. | ||
| * |
Contributor
There was a problem hiding this comment.
[minor] The isValidAtlassianSubdomain export is retained for "backwards compatibility" but the registry now uses validateSubdomainLabel internally. Worth grepping for external callers — if none exist, this is dead code that can be removed to avoid maintaining two copies of the same regex.
mtkik
pushed a commit
to mtkik/openhuman-meet
that referenced
this pull request
May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ToolkitRequiredField] registry so any Composio toolkit with provider-specific required fields (Jira subdomain, WhatsApp WABA id, Dynamics 365 org name, …) renders its fields generically — no per-toolkit branches inside the connect modal.dynamics365.org_name(with the.crm.dynamics.comsuffix and the shared DNS-label validator) into the registry so the connect modal collects the org name before callingopenhuman.composio_authorize, instead of failing after submit.ComposioConnectModalto use the new registry + a genericRequiredFieldsFormsubcomponent. The 612-recovery phase is generalised from the oldneeds-subdomaintoneeds-fieldsso it re-uses the same renderer.Problem
When a user tries to connect Dynamics 365 today, Composio rejects the authorization request with code
612(ConnectedAccount_MissingRequiredFields):The connect modal renders a generic OAuth button and submits without first collecting the org name (the
myorgpart ofmyorg.crm.dynamics.com). Users only learn about the missing field after the failed submit — and on failure they see the raw backend payload, not an actionable form.The fix has to be generic, not a one-off Dynamics 365 patch: future Composio integrations with required fields should not require another UI patch.
Solution
app/src/components/composio/toolkitRequiredFields.tsdeclares one entry per toolkit-with-required-fields. Each entry binds a fieldkey(also used verbatim as theextra_paramsname on the backend authorize request) to label / hint / placeholder / optional fixed suffix / optional custom validator.waba_id), Jira (subdomain), and Dynamics 365 (org_name) — Jira and Dynamics 365 share a DNS-label validator that rejects full URLs and accepts the short subdomain form.ComposioConnectModaldrops the hand-rolledneedsWabaId/needsAtlassianSubdomainbooleans. Field state is now a singleRecord<string, string>keyed by registry field key; validation runs throughvalidateRequiredFieldValues; submission pulls values straight into theextra_paramsobject ofcomposio_authorize.RequiredFieldsFormwithautoFocusFirstso any registry-known toolkit gets a useful retry form, not just Jira. Toolkits without a registry entry fall back to the existing sanitized error path so the retry loop can never spin forever.Submission Checklist
## Related— N/A: no matrix feature ID changed.docs/RELEASE-MANUAL-SMOKE.md) — N/A: scoped Composio connect-flow change.Closes #NNNin the## RelatedsectionImpact
app/src/components/composio/*. No persistence, security, migration, or backend changes.openhuman.composio_authorizeAPI contract unchanged — the new fields ride the existingextra_paramschannel that already supportswaba_idandsubdomain.composio.connect.*keys (dynamicsOrgNameLabel,dynamicsOrgNameHint,wabaIdHint,needsFieldsPrefix,needsFieldsSuffix,requiredFieldEmpty).Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/2127-composio-required-fields-registry31dd2bc6a4378a460077428924421188680a26ceValidation Run
pnpm --filter openhuman-app format:check— pre-push hook ran prettier; format-only fixes amended in.pnpm typecheck— clean.pnpm debug unit toolkitRequiredFields10/10,pnpm debug unit ComposioConnectModal41/41.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
code: 612failure. The Composio missing-fields error path becomes a registry-driven recovery form rather than a Jira-only special case.Dynamics 365 Organization Nameinput with.crm.dynamics.comsuffix + hint; empty or full-URL values are blocked with inline validation. Existing Jira + WhatsApp UIs are visually identical (now driven by the same registry).Parity Contract
extra_paramspayload shape on the wire.isMissingRequiredFieldsErrorslug-only detection unchanged. The 612-recovery phase now activates for any registry-known toolkit (previously Jira-only); toolkits without a registry entry fall back to the existing sanitized error path so the retry loop cannot spin forever.Summary by CodeRabbit
New Features
Improvements
Internationalization