feat(integrations): declare HubSpot managed-connector migration + error hints#345
feat(integrations): declare HubSpot managed-connector migration + error hints#345neubig wants to merge 4 commits into
Conversation
Add a declarative `errorHints` field (HTTP-status-keyed messages) to OAuthProviderRegistrationDefaults and populate it for HubSpot (401/403 reconnection advice). This lets the integrations hub surface provider-specific failure guidance from catalog data instead of hardcoding provider names in its own source. Companion to OpenHands/integrations-hub#274. Co-authored-by: openhands <openhands@all-hands.dev>
neubig
force-pushed
the
codex/declarative-hubspot-config
branch
from
2d18021 to
1522794
Compare
|
✅ 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
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable - The error-hint data itself is simple, but the PR does not implement the migration contract it advertises.
[CRITICAL ISSUES]
- [integrations/oauth-provider-registration-defaults.js, Line 333] Correctness / Scope mismatch: The PR title and description claim this declares HubSpot managed-connector migration metadata (
managedConnectorMigration.canonicalServerUrlandlegacyScopeBundles), but the actual HubSpot defaults only adderrorHints;getOAuthProviderRegistrationDefaults("hubspot")has nomanagedConnectorMigration. The companion hub change would either have to keep the provider-specific migration branch this PR says it enables deleting, or it would silently lose migration behavior for stored HubSpot connectors. Add the missing migration schema/data and a catalog smoke test, or narrow this PR to an error-hints-only change before merging.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
This changes public integration catalog data and TypeScript declarations consumed by downstream hubs. The code change is small and CI is green, but the advertised migration contract is absent, so merging as-is risks breaking or stalling the companion provider-agnostic HubSpot migration work.
VERDICT:
❌ Needs rework: Either implement the claimed migration metadata or correct the PR scope and description before merging.
KEY INSIGHT:
The data-driven direction is right, but the catalog contract must carry every piece of provider-specific behavior the hub is supposed to delete.
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 (e.g., "Security concerns about X do not apply here because Y"). 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 is merge-ready.Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
Co-authored-by: openhands <openhands@all-hands.dev>
|
✅ 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
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste — focused, additive change that does what the title and description promise.
The HubSpot managedConnectorMigration descriptor is present (see line 363 of the new file) and exports the four legacy scope bundles plus canonicalServerUrl; the new Python test exercises real Node code via subprocess and verifies the migration, error hints, and the canonical OAuth config. The skills/SKILL.md link move is a small, orthogonal curation fix.
Note: the prior all-hands-bot critical comment on this PR is a false positive — the migration field is wired into the hubspot registration defaults at line 363, and the test asserts it. No blocking issues.
✅ Worth merging.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
Folds extensions#345 into this PR and generalizes it per the project's "no provider-specific code" direction. TS and Python now expose an exactly parallel, provider-agnostic API. JS (integrations): - Remove all hubspot* named exports: hubspotMcpServerUrl, hubspotMcpAuthorizationUrl, hubspotMcpTokenUrl, hubspotRequiredScopes, hubspotOptionalScopes (and the #345-added hubspotLegacyScopeBundle / hubspotLegacyScopeBundleWithoutOauth / hubspotManagedConnectorMigration consts, which are no longer needed). The hubspot MCP server URL is inlined into officialManagedMcpServerUrls and the registrationDefaults authorizationUrl/tokenUrl literals. - Declare the managed-connector migration descriptor + errorHints as INLINE DATA on the hubspot registrationDefaults entry (the generic managedConnectorMigration / errorHints fields), exactly the shape the integrations hub consumes (canonicalServerUrl + legacyScopeBundles. {required,optional,union,unionWithoutOauth}). No hubspot-named module symbols remain. - index.js / index.d.ts: drop the hubspot* re-exports/declarations; add the generic errorHints / managedConnectorMigration fields and the ManagedConnectorMigration interface (so any provider can declare one). Tests: - Replace #345's hubspot-specific test (which imported hubspot* symbols) with a provider-agnostic invariant test: every provider declaring a managedConnectorMigration has a well-formed descriptor (https canonical URL, all four bundles as string arrays, union == required+optional, unionWithoutOauth == union minus "oauth", errorHints declared). Asset: - Regenerated integrations/oauth-provider-catalog.json and the Python copy (byte-identical, single build pass). The hubspot entry now carries the migration descriptor + errorHints; scopes stays [] to preserve the hub's legacy-detection semantics. Supersedes extensions#345. The integrations-hub (#274) reads this via the generic accessors only — it never imported the removed hubspot* symbols. Co-authored-by: openhands <openhands@all-hands.dev>
|
Closing as superseded by #346, which folds this PR's content in and generalizes it per the project's "no provider-specific code" direction. #346 declares the HubSpot managed-connector migration descriptor + errorHints as inline data on the catalog entry (generic integrations-hub#274 reads this via the generic accessors only and never imported the removed (This comment was created by an AI agent, OpenHands, on behalf of the user.) |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Taste: 🟡 Acceptable
Nice move in the right direction: declaring the migration + error hints in the catalog (rather than branching on slug === 'hubspot' in the hub) is the right abstraction boundary, and the wiring reuses the existing hubspotRequiredScopes / hubspotOptionalScopes / hubspotMcpServerUrl constants instead of duplicating strings. The 5 catalog tests pass against real Node code (no mocks), and the new test specifically asserts the contract that the hub PR #273 will rely on (canonical server URL, the four scope-bundle keys, and 401/403 hints). The earlier CHANGES_REQUESTED review from all-hands-bot is now resolved — managedConnectorMigration is in fact declared.
A few non-blocking observations below; none prevent merging.
🟡 Suggestion: data structure — legacyScopeBundles carries redundant fields
ManagedConnectorMigration.legacyScopeBundles declares four arrays (required, optional, union, unionWithoutOauth), but union is unconditionally [...required, ...optional] and unionWithoutOauth is union.filter(s => s !== 'oauth'). The hub could derive both from required + optional itself, which removes an invariant the catalog has to keep honest: if a future HubSpot scope is added to hubspotRequiredScopes, the union array still has to be remembered (it's derived correctly today only because hubspotLegacyScopeBundle is literally [...hubspotRequiredScopes, ...hubspotOptionalScopes] at lines 97–100). Two fields are enough.
🟡 Suggestion: comments narrate the diff instead of explaining invariants
The 7-line JSDoc on hubspotLegacyScopeBundle (lines 90–95) and the 4-line JSDoc on hubspotManagedConnectorMigration (lines 105–108) describe what this commit is doing and what the hub does with the data — both belong in the PR description / commit message, not the source. They will silently go stale the first time the hub changes its scope-bundling strategy. The fields' meaning is already captured by the ManagedConnectorMigration interface JSDoc in index.d.ts. Trim these to a single line, or drop them.
🟡 Suggestion: PR hygiene — bundled unrelated changes
skills/index.js and skills/openhands-sdk/SKILL.md are touched in the same diff as the HubSpot catalog change, but the PR body only describes the HubSpot work. The skills changes look like an orthogonal SDK skill catalog rebuild. Fine to land together, but worth a one-line note in the PR description so reviewers (and git bisect) can tell intent apart.
Risk: 🟢 LOW
Catalog metadata change behind an existing typed surface. No runtime behavior for non-HubSpot providers changes; the hub's only behavioral delta is the new optional managedConnectorMigration and errorHints keys, both already gated by feature checks. No new dependencies, no public API removal, no security boundary change.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
| legacyScopeBundles: { | ||
| required: string[]; | ||
| optional: string[]; | ||
| union: string[]; | ||
| unionWithoutOauth: string[]; | ||
| }; |
There was a problem hiding this comment.
🟡 Suggestion: Four fields here where two would carry the same information. union is always [...required, ...optional] and unionWithoutOauth is union.filter(s => s !== 'oauth') — the hub can compute both from required + optional. Declaring all four as data creates a hidden invariant (changing required must imply recomputing union, otherwise the hub's match logic drifts). Either drop union / unionWithoutOauth and let the consumer derive them, or keep just union + an oauthScope: string field.
| legacyScopeBundles: { | |
| required: string[]; | |
| optional: string[]; | |
| union: string[]; | |
| unionWithoutOauth: string[]; | |
| }; | |
| legacyScopeBundles: { | |
| required: string[]; | |
| optional: string[]; | |
| }; |
| /** | ||
| * Historical HubSpot OAuth scope bundles. Before the managed connector pointed | ||
| * at mcp.hubspot.com, already-stored HubSpot connectors carried these scopes. | ||
| * The integrations hub compares a stored connector's scopes against these | ||
| * bundles to detect legacy connectors that need re-discovery, so the values | ||
| * here must match the bundles the hub previously hardcoded exactly. | ||
| */ |
There was a problem hiding this comment.
🟡 Suggestion: This 7-line block comment narrates the diff and explains what the hub does with the data — neither survives in the source. The data's meaning is already in the ManagedConnectorMigration interface JSDoc in index.d.ts. Trim to a single line:
| /** | |
| * Historical HubSpot OAuth scope bundles. Before the managed connector pointed | |
| * at mcp.hubspot.com, already-stored HubSpot connectors carried these scopes. | |
| * The integrations hub compares a stored connector's scopes against these | |
| * bundles to detect legacy connectors that need re-discovery, so the values | |
| * here must match the bundles the hub previously hardcoded exactly. | |
| */ | |
| // Scope bundles the hub matches against stored connectors to detect legacy ones. |
| /** | ||
| * Declarative HubSpot managed-connector migration descriptor. Lets the | ||
| * integrations hub migrate/normalize legacy HubSpot connectors from catalog | ||
| * data instead of branching on the "hubspot" slug in its own source. | ||
| */ |
There was a problem hiding this comment.
🟡 Suggestion: This 5-line block comment restates what index.d.ts already says about ManagedConnectorMigration. Drop it — the exported constant name + its type are self-describing.
| /** | |
| * Declarative HubSpot managed-connector migration descriptor. Lets the | |
| * integrations hub migrate/normalize legacy HubSpot connectors from catalog | |
| * data instead of branching on the "hubspot" slug in its own source. | |
| */ |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
|
Closing #345 in favor of #346, which supersedes this PR by adding the Python package and making the catalog fully provider-agnostic. The Critical review finding (missing managedConnectorMigration on the hubspot entry) was resolved on this branch in commit 3054242 and CI was green, but #346 takes the preferred direction forward. Note: #346 removes the managedConnectorMigration mechanism entirely, so the companion integrations-hub#274 (which repinned to this PR's SHA and reads managedConnectorMigration from the catalog) will need to be reworked against #346's merge commit (drop its managed-connector-migration module/tests) before it can land. (This comment was posted by an AI agent, OpenHands, on behalf of the user.) |
…S + Python) (#346) * feat: add openhands_extensions Python package sharing the integration catalog JSON asset Add a Python package (openhands_extensions) that reads the OAuth provider catalog from the same JSON asset as the JS package, so Python services (e.g. the integrations-hub backend) can import the catalog instead of vendoring a hand-maintained snapshot. Single source of truth: integrations/oauth-provider-catalog.json, generated from the JS authoring source by > @openhands/extensions@0.5.0 build:integration-catalog > node scripts/build-integration-catalog.mjs Generated /tmp/extensions/integrations/oauth-provider-catalog.json with 52 providers and 2 default managed connectors (scripts/build-integration-catalog.mjs). The asset is embedded into both the JS package (exported via package.json) and the Python package (bundled via hatchling, loaded via importlib.resources). Python API mirrors the JS API: - list_oauth_provider_catalog() -> listOAuthProviderCatalog() - get_oauth_provider_registration_defaults(slug) -> getOAuthProviderRegistrationDefaults() - default_managed_connectors() -> snapshot defaultManagedConnectors - hubspot_mcp_* / hubspot_*_scopes -> hubspot constants - INTEGRATION_CATALOG_SNAPSHOT -> { providers, defaultManagedConnectors } Tests: - tests/test_integration_catalog_in_sync.py regenerates the JSON from the JS source via Node and asserts the checked-in asset matches (catches drift), plus asserts the Python package reads the same asset. - tests/test_version_alignment.py asserts package.json, pyproject.toml, and _version.py versions stay in lock-step (release-please bumps all three). pyproject.toml is now an installable package (hatchling build backend) while preserving the existing uv test group. CI (tests.yml) gains a Node setup step so the parity test can run node. Co-authored-by: openhands <openhands@all-hands.dev> * chore: sync openhands-sdk SKILL.md (unrelated upstream drift) The sync-sdk-skill CI check regenerates skills/openhands-sdk/SKILL.md from the upstream software-agent-sdk repo. An upstream example was renamed (43_mixed_marketplace_skills -> 04_mixed_marketplace_skills) between the last main build and this PR, so the check failed on every PR opened today. Regenerating the file (script-generated, no manual edits) unblocks CI. Unrelated to the Python integrations package in this PR; included only to satisfy the repo-wide sync check. Co-authored-by: openhands <openhands@all-hands.dev> * chore: regenerate skills/index.js after SDK skill sync Re-running build:skills so skills/index.js reflects the regenerated openhands-sdk SKILL.md (required by test_skills_catalog). Unrelated to the Python integrations package; included to keep the repo-wide sync checks green. Co-authored-by: openhands <openhands@all-hands.dev> * chore: trigger CI re-run on latest SHA * chore: address PR review feedback (#346) 1. _version.py: derive __version__ from installed package metadata (importlib.metadata) so pyproject.toml is the single source of truth, and add python/openhands_extensions/_version.py to release-please extra-files so the fallback string stays in lock-step too. Avoids release-please leaving the runtime version stale. 2. HubSpot exports: expose module-level string/list constants (HUBSPOT_MCP_SERVER_URL, HUBSPOT_MCP_AUTHORIZATION_URL, HUBSPOT_MCP_TOKEN_URL, HUBSPOT_REQUIRED_SCOPES, HUBSPOT_OPTIONAL_SCOPES) instead of callables, to match the JS constant semantics and avoid the function-vs-value confusion. Add tests guarding the constants match the asset and are values (not callables). Co-authored-by: openhands <openhands@all-hands.dev> * refactor: generalize Python package, drop HubSpot special-casing Address all-hands-bot review on c1f77b0: - Remove all HubSpot-specific constants (HUBSPOT_MCP_*, HUBSPOT_*_SCOPES) from the Python public API and __init__ re-exports. The package now exposes only the generic, provider-agnostic catalog accessors that mirror the JS package (list_oauth_provider_catalog, get_oauth_provider_registration_defaults) plus the snapshot helpers. This eliminates the misleading empty-scope constants the bot flagged. - Fix __all__ in integrations.py: it still listed the old lowercase hubspot_* names, so `from openhands_extensions.integrations import *` raised AttributeError. __all__ now matches the exported symbols. - Cached accessors (list_oauth_provider_catalog, get_oauth_provider_registration_defaults, default_managed_connectors) now return independent deep copies so a caller mutating a returned value cannot corrupt the shared lru_cache for other callers. - Build script now writes BOTH asset locations (integrations/ and python/openhands_extensions/) from a single generation pass, so there is one source and one command. Previously it wrote only the root asset, leaving a second committed copy in the wheel that could drift. - Add test_python_asset_is_byte_identical_to_root_asset (drift guard) and test_accessors_return_independent_copies (mutability guard); drop the now-obsolete hubspot-constant tests. Co-authored-by: openhands <openhands@all-hands.dev> * refactor: remove all HubSpot special-casing, declare migration as data Folds extensions#345 into this PR and generalizes it per the project's "no provider-specific code" direction. TS and Python now expose an exactly parallel, provider-agnostic API. JS (integrations): - Remove all hubspot* named exports: hubspotMcpServerUrl, hubspotMcpAuthorizationUrl, hubspotMcpTokenUrl, hubspotRequiredScopes, hubspotOptionalScopes (and the #345-added hubspotLegacyScopeBundle / hubspotLegacyScopeBundleWithoutOauth / hubspotManagedConnectorMigration consts, which are no longer needed). The hubspot MCP server URL is inlined into officialManagedMcpServerUrls and the registrationDefaults authorizationUrl/tokenUrl literals. - Declare the managed-connector migration descriptor + errorHints as INLINE DATA on the hubspot registrationDefaults entry (the generic managedConnectorMigration / errorHints fields), exactly the shape the integrations hub consumes (canonicalServerUrl + legacyScopeBundles. {required,optional,union,unionWithoutOauth}). No hubspot-named module symbols remain. - index.js / index.d.ts: drop the hubspot* re-exports/declarations; add the generic errorHints / managedConnectorMigration fields and the ManagedConnectorMigration interface (so any provider can declare one). Tests: - Replace #345's hubspot-specific test (which imported hubspot* symbols) with a provider-agnostic invariant test: every provider declaring a managedConnectorMigration has a well-formed descriptor (https canonical URL, all four bundles as string arrays, union == required+optional, unionWithoutOauth == union minus "oauth", errorHints declared). Asset: - Regenerated integrations/oauth-provider-catalog.json and the Python copy (byte-identical, single build pass). The hubspot entry now carries the migration descriptor + errorHints; scopes stays [] to preserve the hub's legacy-detection semantics. Supersedes extensions#345. The integrations-hub (#274) reads this via the generic accessors only — it never imported the removed hubspot* symbols. Co-authored-by: openhands <openhands@all-hands.dev> * fix: isolate INTEGRATION_CATALOG_SNAPSHOT from cache; harden tests Address all-hands-bot review on 769f0c7: - INTEGRATION_CATALOG_SNAPSHOT is now a copy.deepcopy of the cached snapshot at module load, so mutating it cannot corrupt the shared cache backing list_oauth_provider_catalog / get_oauth_provider_registration_defaults / default_managed_connectors. - test_accessors_return_independent_copies extended to assert mutating INTEGRATION_CATALOG_SNAPSHOT does not leak into the accessors. - test_get_oauth_provider_registration_defaults_round_trip now uses provider.get('registrationDefaults') to match the implementation's .get() (returns None when absent), guarding future catalog additions. Co-authored-by: openhands <openhands@all-hands.dev> * docs: clarify build script comments (JS is source, not a reader of the asset) Address all-hands-bot review on beff741: reword the build-script header and OUTPUTS comments so future maintainers don't mistake the generated JSON for the JS runtime source. oauth-provider-catalog.js is the JS authoring/runtime source that this script imports to generate the asset; the Python package reads its embedded copy. CI keeps the checked-in copies in sync. Co-authored-by: openhands <openhands@all-hands.dev> * refactor: remove the managed-connector migration mechanism entirely Per the project's keep-it-clean direction (this is not yet production code, so backward compatibility is not a concern), drop the entire legacy managed-connector migration concept from the catalog: - integrations/oauth-provider-registration-defaults.js: remove the managedConnectorMigration descriptor and errorHints from the hubspot registrationDefaults entry. - integrations/index.d.ts: remove the errorHints / managedConnectorMigration fields from OAuthProviderRegistrationDefaults and the ManagedConnectorMigration interface. - tests/test_catalogs.py: remove test_managed_connector_migration_descriptors_are_well_formed. - Regenerate integrations/oauth-provider-catalog.json and the Python copy (byte-identical); the hubspot entry now carries only its standard OAuth config. BREAKING for integrations-hub#274: that PR's managed-connector-migration module (canonicalServerUrl + legacyScopeBundles detection, errorHints) read these fields and must be dropped/reworked — legacy HubSpot connectors will no longer be auto-migrated. Co-authored-by: openhands <openhands@all-hands.dev> * refactor(integrations): unify catalog into single JSON source of truth Replace the split oauth-provider-catalog (JS-authoring, JSON-generated) with a single unified integration-catalog.json that is the source of truth for both the JS and Python packages. Both now read the JSON asset at runtime instead of the JS package computing from authoring source and the Python package reading a generated copy. Each catalog entry merges oauth-provider registration defaults with the direct integration connection options, so a single entry can carry an oauth connector, an mcp/http connector, or both. Entries are tagged with supportsOauth / supportsMcp and exposed through listIntegrationCatalog({ mcp, oauth }) (JS) / list_integration_catalog(mcp=, oauth=) (Python) so callers can filter by connector type - e.g. only integrations that support an oauth connector. The authoring source is split into -source modules imported only by the build script; oauth-provider-catalog.js/.json and the duplicate Python asset are removed. The OAuth-provider-only view (providers) and defaultManagedConnectors are preserved in the unified asset so the integrations-hub backend contract is unchanged. Co-authored-by: openhands <openhands@all-hands.dev> * chore: regenerate openhands-sdk skill to satisfy sync-sdk-skill CI Co-authored-by: openhands <openhands@all-hands.dev> * chore: rebuild skills/index.js after SDK skill regen Co-authored-by: openhands <openhands@all-hands.dev> * chore: trigger CI on latest unified-catalog + skills-index fix * docs: note parity-test guarantee in build script header * ci: document pull_request trigger rationale in tests workflow * docs(agents): record unified integration-catalog single-source-of-truth model * refactor(integrations): make JSON the source of truth; merge providers into integrations The integration catalog is now a single hand-authored JSON asset (integrations/integration-catalog.json), not generated from any .mjs/.js source. Both the JS and Python packages read it at runtime. Changes: - Merge the top-level providers[] array into each integration as a minimal oauthProvider override (null when the integration has no OAuth provider). Only fields whose OAuth-context values differ from the integration's connector view (description, docsUrl, popularityRank) are stored; slug==id, availability==catalogStatus, and the rest were pure duplication. - Drop derived fields from the asset (supportsMcp/supportsOauth, providers[], defaultManagedConnectors) and derive them at runtime in index.js / integrations.py. Verified the reconstructed providers[] and defaultManagedConnectors are byte-identical to the previous output (0/52 provider mismatches; DMC content+order preserved via defaultManagedConnectorSlugs). - Remove the mjs/js authoring sources + build script (integration-catalog-source.mjs, oauth-provider-catalog-source.mjs, oauth-provider-registration-defaults-source.js, scripts/build-integration-catalog.mjs) and the build:integration-catalog npm script. - Create integrations/catalog/<id>.json for all 82 integrations (adds the 12 oauth-only entries that previously had no file); a CI parity test asserts every catalog file equals its master entry. - Update index.d.ts (oauthProvider type, derived flags optional), the Python module, README/integrations README, and tests. - JS<->Python parity confirmed across integrations, providers, defaultManagedConnectors, filters, and all 52 registration-defaults lookups. Co-authored-by: openhands <openhands@all-hands.dev> * chore: bump extensions packages to 0.6.0 Co-authored-by: openhands <openhands@all-hands.dev> * refactor(integrations): remove provider catalog compatibility Co-authored-by: openhands <openhands@all-hands.dev> * refactor: derive integration catalog fields from options Co-authored-by: openhands <openhands@all-hands.dev> * refactor: return raw integration catalog entries Co-authored-by: openhands <openhands@all-hands.dev> * refactor: simplify integration availability Co-authored-by: openhands <openhands@all-hands.dev> * refactor(integrations): package catalog without aggregate JSON Co-authored-by: openhands <openhands@all-hands.dev> * docs(integrations): mark catalog index as generated Co-authored-by: openhands <openhands@all-hands.dev> * refactor: remove integration catalog availability Co-authored-by: openhands <openhands@all-hands.dev> * refactor: remove integration runtime availability Co-authored-by: openhands <openhands@all-hands.dev> * refactor: move integration logos into catalog metadata Co-authored-by: openhands <openhands@all-hands.dev> * fix: add ordinal integration logo metadata Co-authored-by: openhands <openhands@all-hands.dev> * fix: correct remote MCP catalog auth metadata Co-authored-by: openhands <openhands@all-hands.dev> * ci: publish extensions package to PyPI Co-authored-by: openhands <openhands@all-hands.dev> --------- Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: Graham Neubig <gneubig@users.noreply.github.com>
3 participants
Summary
Companion to OpenHands/integrations-hub#273. Moves the last provider-specific knowledge that the integrations hub keeps hardcoded behind
slug === "hubspot"checks onto the HubSpot catalog entry in extensions, so the hub can become fully provider-agnostic.Adds two optional fields to
OAuthProviderRegistrationDefaults:managedConnectorMigration: { canonicalServerUrl, legacyScopeBundles }— declares the canonical MCP server URL a stored HubSpot connector must be pinned to, plus the historical scope bundles (required, optional, their union, and the union without"oauth") that identify a pre-migration connector whose cached tools should be cleared for re-discovery. These bundles match exactly what the hub currently hardcodes viahubspotRequiredScopes/hubspotOptionalScopes/hubspotLegacyScopeBundle/hubspotLegacyScopeBundleWithoutOauth.errorHints: Record<number, string>— HTTP-status-keyed user-facing hints (401/403) previously branched onslug === "hubspot"in the hub'smcp-client.ts.The canonical OAuth config (authorization/token URLs,
clientAuthentication: "body",pkce: true,scopes: [],provider: "mcp") was already declared inregistrationDefaults; the hub will now read it viagetOAuthProviderRegistrationDefaults("hubspot")instead of re-hardcoding thehubspotMcp*constants.Only HubSpot carries these new fields — no other provider is affected.
Why declarative
The hub currently branches on
slug === "hubspot"in three places (managed-connectors.tsOAuth-config override + legacy detection,mcp-client.tserror hints). By expressing the canonical config, the migration detector, and the error copy as data on the catalog entry, the hub applies whatever an entry declares — no provider names in hub source. This is the prerequisite for the hub PR that removes those branches and consumes the fullINTEGRATION_CATALOG.Verification
nodesmoke test confirmsgetOAuthProviderRegistrationDefaults("hubspot")returns the canonical config plus the new fields.legacyScopeBundlesagainst the hub's hardcodedhubspotLegacyScopeBundle/hubspotLegacyScopeBundleWithoutOauth/hubspotRequiredScopes/hubspotOptionalScopes— exact match (4 bundles).INTEGRATION_CATALOGstill has 82 entries.The hub PR (to follow) will pin
@openhands/extensionsto the merge commit of this PR and delete its HubSpot slug-branches.Closes OpenHands/integrations-hub#273 (companion side).
This PR was created by an AI agent (OpenHands) on behalf of the user.