feat(skills): configure public extensions and organization skill precedence

.pr/prd-198/README.md

@@ -0,0 +1,26 @@
+# PRD-198 review evidence
+
+Temporary review artifacts. The `.pr/` directory is removed when the PR is approved.
+
+## Screenshots
+
+Both are 1280x860 captures of this branch's frontend in headless Chromium, driven by Playwright. All data is synthetic (`Example Org (synthetic)`, `github:example-org/approved-skills`). The repo's MSW mocks stood in for the backend, plus a small in-memory store for organization marketplaces and settings readback.
+
+- `01-org-marketplace-legacy-default.png`: an organization marketplace with no saved preference. The new selector shows **Use existing behavior**, which saves `null` and keeps today's merge order.
+- `03-org-marketplace-prefer-organization-saved-reopened.png`: the same marketplace after choosing **Use this marketplace's skill**, saving, refetching settings and reopening the editor. The saved `"marketplace"` value reappears.
+
+Assertions checked during the same run:
+- An unrelated edit on a legacy registration sends `public_skill_precedence: null`.
+- Choosing the marketplace option sends `"marketplace"`, and the settings readback and the reopened editor both return `"marketplace"`.
+
+## Matched-version probe
+
+`test_prd198_matched.py` runs Enterprise and the patched SDK/Agent Server in one process, with no network. It checks the effective `github` skill content under each preference with public skills on and off, and checks that the plugin catalog reports `disabled` when the catalog setting is off. 7 cases passed.
+
+To reproduce:
+1. Install the companion SDK change (`openhands-sdk` and `openhands-agent-server`, built from the SDK PR branch) into this repo's environment in place of the pinned 1.60.1.
+2. Copy `tests/public_extension_fixtures.py` from the SDK PR branch into this repo's `tests/` (the probe imports `tests.public_extension_fixtures`).
+3. Run `uv run --no-sync pytest .pr/prd-198/test_prd198_matched.py` so uv keeps the patched packages installed for the probe.
+4. Delete the copied fixture and restore the pinned versions with `uv sync --frozen`.
+
+Not covered here: live deployment, warm-claimed or resumed sandboxes, and automation-triggered conversations.

.pr/prd-198/test_prd198_matched.py

@@ -0,0 +1,163 @@
+"""Enterprise -> patched agent-server/SDK, same process, no network."""
+
+import httpx
+import pytest
+from fastapi.testclient import TestClient
+
+from openhands.agent_server.api import create_app
+from openhands.agent_server.config import Config
+from openhands.app_server.app_conversation import skill_loader
+from openhands.app_server.app_conversation.live_status_app_conversation_service import (
+    _to_sdk_marketplace_registrations,
+)
+from openhands.app_server.sandbox.sandbox_spec_service import get_agent_server_env
+from openhands.app_server.settings.settings_models import (
+    MarketplaceRegistration,
+    MarketplaceScope,
+)
+from openhands.sdk import LLM, Agent, AgentContext
+from openhands.sdk.conversation.impl.local_conversation import LocalConversation
+from openhands.sdk.skills.skill import _invalidate_public_skills_cache
+from tests.public_extension_fixtures import (
+    ORG_GITHUB,
+    PLUGIN_GITHUB,
+    PUBLIC_GITHUB,
+    make_plugin_marketplace,
+    make_public_repo,
+    make_standalone_marketplace,
+)
+
+
+@pytest.fixture
+def env(tmp_path, monkeypatch):
+    monkeypatch.setenv('EXTENSIONS_PATH', str(make_public_repo(tmp_path / 'pub')))
+    for k in ('OH_PUBLIC_SKILLS_ENABLED', 'OH_PUBLIC_PLUGIN_CATALOG_ENABLED'):
+        monkeypatch.delenv(k, raising=False)
+    _invalidate_public_skills_cache()
+    yield
+    _invalidate_public_skills_cache()
+
+
+def _org(tmp_path, factory, precedence):
+    # Production sources are remote; local fixtures bypass the source validator.
+    return [
+        MarketplaceRegistration.model_construct(
+            name='acme',
+            source=str(factory(tmp_path / 'org')),
+            ref=None,
+            repo_path=None,
+            auto_load=True,
+            public_skill_precedence=precedence,
+            scope=MarketplaceScope.ORG,
+        )
+    ]
+
+
+def _apply_sandbox_env(monkeypatch):
+    """What a cold, warm-claimed or resumed sandbox receives from Enterprise."""
+    for k, v in get_agent_server_env().items():
+        if k.startswith('OH_PUBLIC_'):
+            monkeypatch.setenv(k, v)
+
+
+async def _discover(monkeypatch, registrations, load_public):
+    client = TestClient(create_app(Config(session_api_keys=[])))
+
+    class _Async:
+        def __init__(self, *a, **k):
+            pass
+
+        async def __aenter__(self):
+            return self
+
+        async def __aexit__(self, *a):
+            return False
+
+        async def post(self, url, json, headers=None, timeout=None):
+            r = client.post('/api/skills', json=json)
+            return httpx.Response(
+                r.status_code, json=r.json(), request=httpx.Request('POST', url)
+            )
+
+    monkeypatch.setattr(skill_loader.httpx, 'AsyncClient', _Async)
+    skills = await skill_loader.load_skills_from_agent_server(
+        agent_server_url='http://sandbox',
+        session_api_key=None,
+        project_dir='/nonexistent',
+        load_public=load_public,
+        load_user=False,
+        load_project=False,
+        load_org=False,
+        registered_marketplaces=registrations,
+        load_registered_marketplaces=True,
+    )
+    return skills
+
+
+def _by_name(skills):
+    return {s.name: s.content.strip() for s in skills}
+
+
+def _runtime(tmp_path, registrations, skills):
+    agent = Agent(
+        llm=LLM(model='test/m', api_key='k'),
+        tools=[],
+        agent_context=AgentContext(
+            skills=skills,
+            registered_marketplaces=_to_sdk_marketplace_registrations(registrations),
+        ),
+    )
+    ws = tmp_path / 'ws'
+    ws.mkdir(exist_ok=True)
+    conv = LocalConversation(agent=agent, workspace=ws, visualizer=None)
+    conv._ensure_plugins_loaded()
+    ctx = conv.agent.agent_context
+    conv.close()
+    return _by_name(ctx.skills)
+
+
+@pytest.mark.usefixtures('env')
+@pytest.mark.parametrize(
+    ('public_on', 'factory', 'precedence', 'expected'),
+    [
+        (True, make_standalone_marketplace, None, PUBLIC_GITHUB),
+        (True, make_standalone_marketplace, 'marketplace', ORG_GITHUB),
+        (True, make_plugin_marketplace, 'public', PUBLIC_GITHUB),
+        (True, make_plugin_marketplace, 'marketplace', PLUGIN_GITHUB),
+        (False, make_standalone_marketplace, None, ORG_GITHUB),
+        (False, make_plugin_marketplace, 'public', PLUGIN_GITHUB),
+    ],
+)
+async def test_discovery_then_runtime(
+    tmp_path, monkeypatch, public_on, factory, precedence, expected
+):
+    monkeypatch.setenv('OH_PUBLIC_SKILLS_ENABLED', str(public_on).lower())
+    _apply_sandbox_env(monkeypatch)
+    regs = _org(tmp_path, factory, precedence)
+    from openhands.app_server.utils.public_extension_policy import public_skills_enabled
+
+    returned = await _discover(monkeypatch, regs, public_skills_enabled())
+    discovered = _by_name(returned)
+    assert discovered['github'] == expected
+    github = next(s for s in returned if s.name == 'github')
+    assert github.source  # provenance survives the launch handoff
+    assert ('public-only' in discovered) is public_on
+
+    # Enterprise hands the returned Skill objects (with source) to the
+    # conversation as explicit skills; the runtime then activates marketplaces.
+    effective = _runtime(tmp_path, regs, returned)
+    assert effective['github'] == expected
+    assert ('public-only' in effective) is public_on
+    if not public_on:
+        assert PUBLIC_GITHUB not in ''.join(effective.values())
+
+
+@pytest.mark.usefixtures('env')
+def test_catalog_off_on_sandbox(monkeypatch):
+    monkeypatch.setenv('OH_PUBLIC_PLUGIN_CATALOG_ENABLED', '0')
+    _apply_sandbox_env(monkeypatch)
+    client = TestClient(create_app(Config(session_api_keys=[])))
+    assert client.get('/api/plugins/marketplace').json() == {
+        'plugins': [],
+        'disabled': True,
+    }

frontend/__tests__/components/features/settings/marketplace-modal.test.tsx

@@ -546,13 +546,17 @@ describe("MarketplaceModal", () => {
   });
 
   it("renders scope selector when scope selection is allowed (add mode)", () => {
-    renderWithProviders(<MarketplaceModal {...baseProps} allowScopeSelection />);
+    renderWithProviders(
+      <MarketplaceModal {...baseProps} allowScopeSelection />,
+    );
 
     expect(screen.getByTestId("marketplace-scope-select")).toBeInTheDocument();
   });
 
   it("includes the chosen scope in onSave when scope selection is allowed", async () => {
-    renderWithProviders(<MarketplaceModal {...baseProps} allowScopeSelection />);
+    renderWithProviders(
+      <MarketplaceModal {...baseProps} allowScopeSelection />,
+    );
     const user = userEvent.setup();
 
     await user.type(screen.getByPlaceholderText("e.g., my-skills"), "team");
@@ -610,4 +614,73 @@ describe("MarketplaceModal", () => {
       screen.getByText("SETTINGS$MARKETPLACE_SOURCE_READONLY"),
     ).toBeInTheDocument();
   });
+
+  describe("public skill precedence (org marketplaces)", () => {
+    const orgMarketplace = {
+      name: "acme",
+      source: "github:acme/skills",
+      scope: "org" as const,
+      auto_load: true,
+    };
+
+    const renderEdit = (precedence?: "public" | "marketplace" | null) =>
+      renderWithProviders(
+        <MarketplaceModal
+          {...baseProps}
+          mode="edit"
+          marketplace={{
+            ...orgMarketplace,
+            public_skill_precedence: precedence,
+          }}
+        />,
+      );
+
+    it("keeps an unset preference as legacy (null) on an unrelated edit", async () => {
+      renderEdit(undefined);
+      const user = userEvent.setup();
+
+      const select = screen.getByLabelText(
+        "SETTINGS$MARKETPLACE_PUBLIC_PRECEDENCE_LABEL",
+      );
+      expect(select).toHaveValue("legacy");
+
+      await user.click(screen.getByTestId("marketplace-save-button"));
+      await waitFor(() => expect(onSave).toHaveBeenCalled());
+      expect(onSave.mock.calls[0][0].public_skill_precedence).toBeNull();
+    });
+
+    it("saves an explicit choice and can return to legacy", async () => {
+      renderEdit("public");
+      const user = userEvent.setup();
+      const select = screen.getByLabelText(
+        "SETTINGS$MARKETPLACE_PUBLIC_PRECEDENCE_LABEL",
+      );
+      expect(select).toHaveValue("public");
+
+      await user.selectOptions(select, "marketplace");
+      await user.click(screen.getByTestId("marketplace-save-button"));
+      await waitFor(() => expect(onSave).toHaveBeenCalledTimes(1));
+      expect(onSave.mock.calls[0][0].public_skill_precedence).toBe(
+        "marketplace",
+      );
+
+      await user.selectOptions(select, "legacy");
+      await user.click(screen.getByTestId("marketplace-save-button"));
+      await waitFor(() => expect(onSave).toHaveBeenCalledTimes(2));
+      expect(onSave.mock.calls[1][0].public_skill_precedence).toBeNull();
+    });
+
+    it("is not offered for personal marketplaces", () => {
+      renderWithProviders(
+        <MarketplaceModal
+          {...baseProps}
+          mode="edit"
+          marketplace={{ ...orgMarketplace, scope: "personal" }}
+        />,
+      );
+      expect(
+        screen.queryByTestId("marketplace-precedence-select"),
+      ).not.toBeInTheDocument();
+    });
+  });
 });

frontend/__tests__/routes/skills-settings.test.tsx

@@ -15,7 +15,7 @@ import SkillsService from "#/api/skills-service";
 import OptionService from "#/api/option-service/option-service.api";
 import * as orgStore from "#/stores/selected-organization-store";
 import { organizationService } from "#/api/organization-service/organization-service.api";
-import { MOCK_PERSONAL_ORG, MOCK_TEAM_ORG_ACME } from "#/mocks/org-handlers";
+import { MOCK_TEAM_ORG_ACME } from "#/mocks/org-handlers";
 import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers";
 import type { WebClientConfig } from "#/api/option-service/option.types";
 
@@ -469,3 +469,62 @@ describe("skill descriptions", () => {
     expect(screen.queryByText("Code review helpers")).not.toBeInTheDocument();
   });
 });
+
+describe("org marketplace public skill precedence", () => {
+  it("passes the saved preference into the edit modal", async () => {
+    vi.spyOn(OptionService, "getConfig").mockResolvedValue({
+      ...mockConfig,
+      app_mode: "saas",
+    });
+    orgStore.useSelectedOrganizationStore.setState({
+      organizationId: MOCK_TEAM_ORG_ACME.id,
+    });
+    vi.spyOn(organizationService, "getMe").mockResolvedValue({
+      org_id: MOCK_TEAM_ORG_ACME.id,
+      user_id: "user-1",
+      email: "test@example.com",
+      role: "admin",
+      llm_api_key: "",
+      max_iterations: 100,
+      llm_model: "gpt-4",
+      llm_base_url: "",
+      status: "active",
+      permissions: ["edit_org_settings"],
+    });
+    vi.spyOn(organizationService, "getOrganizations").mockResolvedValue({
+      items: [MOCK_TEAM_ORG_ACME],
+      currentOrgId: MOCK_TEAM_ORG_ACME.id,
+    });
+    vi.spyOn(SettingsService, "getSettings").mockResolvedValue({
+      ...MOCK_DEFAULT_USER_SETTINGS,
+      registered_marketplaces: [],
+      inherited_marketplaces: [
+        {
+          name: "example-org-skills",
+          source: "github:example-org/approved-skills",
+          scope: "org",
+          public_skill_precedence: "marketplace",
+        },
+      ],
+    });
+    vi.spyOn(SkillsService, "getSkills").mockResolvedValue([]);
+    vi.spyOn(SkillsService, "getMarketplaceSkills").mockResolvedValue({
+      skills: [],
+      plugins: [],
+      marketplace_skills: {},
+      errors: [],
+    });
+
+    renderSkillsSettings();
+    const user = userEvent.setup();
+    const editButton = await screen.findByRole("button", {
+      name: "BUTTON$EDIT example-org-skills",
+    });
+    await waitFor(() => expect(editButton).toBeEnabled());
+    await user.click(editButton);
+
+    expect(
+      await screen.findByTestId("marketplace-precedence-select"),
+    ).toHaveValue("marketplace");
+  });
+});