From 550fc28a486d7bc8390cfdace80357fd11eb4944 Mon Sep 17 00:00:00 2001 From: Vasco Schiavo <115561717+VascoSch92@users.noreply.github.com> Date: Wed, 19 Aug 2026 17:31:47 +0200 Subject: [PATCH] chore: bump @openhands/extensions to 0.17.0 (#16717) --- .../recommended-automations.test.tsx | 59 +++++++++++--- .../manifest/manifest-setup-dialog.test.tsx | 14 ++++ __tests__/manifests/automation-setup.test.ts | 79 +++++++++++-------- package-lock.json | 8 +- package.json | 2 +- 5 files changed, 114 insertions(+), 48 deletions(-) diff --git a/__tests__/components/automations/recommended-automations.test.tsx b/__tests__/components/automations/recommended-automations.test.tsx index 7ca2db41aa..b179a37c7a 100644 --- a/__tests__/components/automations/recommended-automations.test.tsx +++ b/__tests__/components/automations/recommended-automations.test.tsx @@ -369,7 +369,41 @@ describe("recommended automations", () => { } }); + /** + * Puts a non-MCP-installable requirement back on `jira-issue-to-pr`. + * + * It declared the HTTP-only `jira` until @openhands/extensions 0.17.0 swapped it + * for the MCP `atlassian-rovo`, and no catalog automation declares a non-MCP + * integration any more. The cases below are about what a card does with one, so + * the requirement is restored for their duration rather than the assertions + * rewritten around a property the catalog stopped having. Mirrors the + * mutate-and-restore already used for the unknown-ID case. + * + * @returns the restore function, which the caller must run in a `finally`. + */ + function requireNonMcpIntegration(): () => void { + const automation = AUTOMATION_CATALOG.find( + (item) => item.id === "jira-issue-to-pr", + )!; + const mutable = automation as RecommendedAutomation & { + requires: { integrations: Record }; + }; + const original = mutable.requires.integrations; + const { "atlassian-rovo": rovo, ...rest } = original; + // Keyed first, so the pill order and the install queue start where they did. + mutable.requires.integrations = { + jira: { + message: rovo?.message ?? "Reads the project for issues.", + }, + ...rest, + }; + return () => { + mutable.requires.integrations = original; + }; + } + it("keeps a non-MCP-installable integration visible on its card instead of dropping it", () => { + const restoreRequirement = requireNonMcpIntegration(); // SkillCardPillRow folds pills behind "+N more" when it measures zero // widths in jsdom; give it room so every pill renders. const offsetWidthDescriptor = Object.getOwnPropertyDescriptor( @@ -428,6 +462,7 @@ describe("recommended automations", () => { "RECOMMENDED_AUTOMATIONS$MISSING_CONNECT:1", ); } finally { + restoreRequirement(); if (offsetWidthDescriptor) { Object.defineProperty( HTMLElement.prototype, @@ -497,17 +532,23 @@ describe("recommended automations", () => { }); it("queues installs only for MCP-installable required integrations", async () => { - renderLauncher(); + const restoreRequirement = requireNonMcpIntegration(); - fireEvent.click( - screen.getByTestId("recommended-automation-card-jira-issue-to-pr"), - ); + try { + renderLauncher(); - // jira cannot go through the local MCP install flow, so the queue starts - // directly at github rather than failing or skipping the automation. - const modal = await screen.findByTestId("mcp-install-modal"); - expect(modal).toHaveAttribute("data-marketplace-id", "github"); - expect(mockCreateConversationMutate).not.toHaveBeenCalled(); + fireEvent.click( + screen.getByTestId("recommended-automation-card-jira-issue-to-pr"), + ); + + // jira cannot go through the local MCP install flow, so the queue starts + // directly at github rather than failing or skipping the automation. + const modal = await screen.findByTestId("mcp-install-modal"); + expect(modal).toHaveAttribute("data-marketplace-id", "github"); + expect(mockCreateConversationMutate).not.toHaveBeenCalled(); + } finally { + restoreRequirement(); + } }); it("shows a decorative plus badge on each card without toggle behavior", () => { diff --git a/__tests__/components/manifest/manifest-setup-dialog.test.tsx b/__tests__/components/manifest/manifest-setup-dialog.test.tsx index 32ca0eaf20..fd1c0d7a04 100644 --- a/__tests__/components/manifest/manifest-setup-dialog.test.tsx +++ b/__tests__/components/manifest/manifest-setup-dialog.test.tsx @@ -22,6 +22,7 @@ const mocks = vi.hoisted(() => ({ runAction: vi.fn(), prerequisites: vi.fn(), capabilities: vi.fn(), + missingCreateEndpoints: vi.fn<(entry: SetupEntry) => string[]>(() => []), tracking: { trackAutomationSetupOpened: vi.fn(), trackAutomationSetupValidated: vi.fn(), @@ -52,6 +53,15 @@ vi.mock("#/hooks/query/use-manifest-prerequisites", () => ({ useSetupPrerequisites: () => mocks.prerequisites(), })); +// Which endpoints an entry cannot be created without is read off the published +// interface manifest, so a real one that declares them leaves the refusal path +// unreachable. Stubbed so the case states the manifest it is about, rather than +// depending on the packaged manifest continuing not to publish them. +vi.mock("#/manifests/automation-setup", async (importOriginal) => ({ + ...(await importOriginal()), + missingCreateEndpoints: mocks.missingCreateEndpoints, +})); + vi.mock("#/manifests/manifest-actions", () => ({ useSetupAction: () => mocks.runAction, })); @@ -98,6 +108,9 @@ async function fillForm(user: ReturnType) { beforeEach(() => { vi.clearAllMocks(); + // clearAllMocks resets calls, not implementations, so the one case that + // stubs a manifest without the bundle endpoints would leak into the rest. + mocks.missingCreateEndpoints.mockReturnValue([]); mocks.prerequisites.mockReturnValue(NOTHING_TO_CONNECT); mocks.capabilities.mockReturnValue({ capabilities: null, @@ -282,6 +295,7 @@ describe("SetupDialog", () => { it("refuses an entry the published interface declares no way to create", async () => { // Arrange — a bundle entry against an interface manifest published before // bundles: neither endpoint it needs exists, and no answer supplies them. + mocks.missingCreateEndpoints.mockReturnValue(["createBundle", "uploads"]); renderDialog(BUNDLE_ENTRY); // Assert — said before the form, rather than as a Continue button that diff --git a/__tests__/manifests/automation-setup.test.ts b/__tests__/manifests/automation-setup.test.ts index f2e79fc29c..0b8d5f9d0c 100644 --- a/__tests__/manifests/automation-setup.test.ts +++ b/__tests__/manifests/automation-setup.test.ts @@ -20,14 +20,16 @@ import { createSetup, createSetupEntry } from "./manifest-test-data"; // The one word of a derived name the host writes rather than reads off the // entry is translated, and the derivation runs where no translator can be -// passed in, so it reads the shared instance. Stubbed to pin the key and the -// count rather than a rendered sentence. -vi.mock("#/i18n", () => ({ - default: { - t: (key: string, options: Record) => - `${key}(${options.total})`, - }, +// passed in, so it reads the shared instance. Rendered as `en` does, because +// the fixtures pin the sentence the service was sent; the spy is what pins the +// key, so both halves stay covered. +const { translate } = vi.hoisted(() => ({ + translate: vi.fn( + (_key: string, options: Record) => + `${options.total} repositories`, + ), })); +vi.mock("#/i18n", () => ({ default: { t: translate } })); // The command a skill publishes in its own frontmatter, which the host looks // up rather than storing. Pinned so the assertion does not move when the @@ -42,9 +44,21 @@ vi.mock("@openhands/extensions/skills", () => ({ triggers: ["/incident-retro:setup"], content: "", }, + { + name: "github-repo-monitor", + description: "Watch a GitHub repository for mentions.", + triggers: ["/github-monitor:poll"], + content: "", + }, ], })); +/** The command each assisted entry's skill publishes, keyed by the entry it belongs to. */ +const SETUP_COMMANDS: Record = { + "incident-retrospective-drafter": "/incident-retro:setup", + "github-repo-monitor": "/github-monitor:poll", +}; + /** * The reference fixtures `OpenHands/extensions` publishes with its catalog. * Their request bodies were verified against the live service, and the create @@ -193,13 +207,16 @@ describe("the contract fixtures", () => { ); // Assert - // Every published fixture is a prompt entry created through the preset - // endpoint, so the deduped set collapses to that single path. + // The fixtures cover both creation paths: a prompt entry through the preset + // endpoint, and a bundle entry through the plain create it uploads to first. expect({ create: [...createPaths].sort(), preflight: [...preflightPaths], }).toEqual({ - create: [automationCreateEndpoint()], + create: [ + automationCreateEndpoint(requireEntry("github-pr-reviewer")), + automationCreateEndpoint(), + ].sort(), preflight: ["/v1/validate"], }); }); @@ -228,7 +245,7 @@ describe("buildCreatePayload", () => { // Act const payload = buildCreatePayload(entry, { - repository: "OpenHands/automation", + repositories: ["OpenHands/automation"], }); // Assert @@ -258,7 +275,10 @@ describe("buildCreatePayload", () => { }); // Assert - expect(payload?.name).toBe(`${entry.name} - SETUP$REPOSITORY_COUNT(2)`); + expect(payload?.name).toBe(`${entry.name} - 2 repositories`); + expect(translate).toHaveBeenCalledWith("SETUP$REPOSITORY_COUNT", { + total: 2, + }); }); it("sends no request body for an entry that hands setup to a conversation", () => { @@ -300,7 +320,7 @@ describe("buildAssistedMessage", () => { const seed = buildAssistedMessage(entry, formValues); // Assert - expect(seed).toBe(`/incident-retro:setup\n\n${message}`); + expect(seed).toBe(`${SETUP_COMMANDS[automationId]}\n\n${message}`); }, ); }); @@ -329,23 +349,12 @@ describe("service rejections mapped back to fields", () => { }); describe("local validation of fixture form values", () => { - it("blocks the unsafe trigger phrase before any request is made", () => { - // Arrange — the fixture names the failing field; the code is the host's - // own vocabulary, rendered through its translations. - const scenario = requireScenario( - BUNDLES[1], - "quote-in-trigger-phrase-blocked-locally", - ); - const entry = requireEntry("github-repo-monitor"); - - // Act - const errors = validateFormValues(entry.setup, scenario.formValues ?? {}); - - // Assert - expect(errors).toEqual({ - triggerPhrase: { code: "unsafeExpressionLiteral" }, - }); - }); + // The unsafe-trigger-phrase case that used to live here is gone: it belonged + // to github-repo-monitor's event trigger, whose JMESPath filter the phrase was + // interpolated into. The entry now runs on cron, so no catalog entry declares + // the `safeExpressionLiteral` constraint any more and there is no fixture to + // pin. The constraint itself is still exercised, on a synthetic setup, by + // `manifest-local-validation.test.ts`. it("passes an entirely blank assisted form, as its fixture records", () => { // Arrange @@ -365,13 +374,15 @@ describe("deriveErrorMap", () => { // Act const errorMap = deriveErrorMap(requireEntry("github-pr-reviewer")); - // Assert + // Assert — a bundle's answers reach the service through its rendered + // config rather than through a prompt, so the paths are the config's. expect(errorMap).toEqual({ - name: ["repository"], - prompt: ["triggerLabel", "repository", "reviewTone"], - "repos[0].url": ["repository"], + name: ["repositories"], "trigger.schedule": ["schedule"], "trigger.timezone": ["timezone"], + "template.config.repos": ["repositories"], + "template.config.trigger_label": ["triggerLabel"], + "template.config.review_tone": ["reviewTone"], }); }); }); diff --git a/package-lock.json b/package-lock.json index 784dbba39c..5847be392b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ "@heroui/react": "2.8.10", "@microlink/react-json-view": "1.31.25", "@monaco-editor/react": "4.7.0", - "@openhands/extensions": "0.16.0", + "@openhands/extensions": "0.17.0", "@openhands/typescript-client": "1.38.0", "@react-router/node": "7.18.2", "@react-router/serve": "7.18.2", @@ -4164,9 +4164,9 @@ "license": "MIT" }, "node_modules/@openhands/extensions": { - "version": "0.16.0", - "resolved": "https://registry.npmjs.org/@openhands/extensions/-/extensions-0.16.0.tgz", - "integrity": "sha512-t9rTxmR782UZ6nbbSBK23EMlzsgJzz4yi34q6mxgPY7ukWehiK7RkWY6rP/u4o7/K0zzvR6/nIJ4OijvttrfVA==", + "version": "0.17.0", + "resolved": "https://registry.npmjs.org/@openhands/extensions/-/extensions-0.17.0.tgz", + "integrity": "sha512-y3+OSHsMWN1sZVT1NERVdCnZVBgHg3F5E4tlYea6bb/nLzX750SuvJpfUJNitJqxN7ls+jMYVgBOgee6KoeLqA==", "license": "MIT", "engines": { "node": ">=18.20.0" diff --git a/package.json b/package.json index a2909c0430..e4d9dfd794 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "@heroui/react": "2.8.10", "@microlink/react-json-view": "1.31.25", "@monaco-editor/react": "4.7.0", - "@openhands/extensions": "0.16.0", + "@openhands/extensions": "0.17.0", "@openhands/typescript-client": "1.38.0", "@react-router/node": "7.18.2", "@react-router/serve": "7.18.2",