feat: allow to register skills by extensions

[benoitf]

fixes #1031

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds skill registration capability to the Provider API by introducing a registerSkill method and supporting CreateSkillParams interface. The implementation delegates to SkillManager, with dependency injection wiring updated in ProviderRegistry to pass the skill manager to ProviderImpl.

Changes

Cohort / File(s)	Summary
API Declaration packages/extension-api/src/extension-api.d.ts	Added new CreateSkillParams interface with label and path fields; extended Provider interface with registerSkill(skill: CreateSkillParams): Disposable method.
Implementation & Tests packages/main/src/plugin/provider-impl.ts, packages/main/src/plugin/provider-impl.spec.ts	Implemented registerSkill method in ProviderImpl that delegates to SkillManager.registerSkillFolder(), mapping path to baseDirectory and setting badge to extension display name; added comprehensive test suite verifying delegation and disposable chaining behavior.
Dependency Injection & Tests packages/main/src/plugin/provider-registry.ts, packages/main/src/plugin/provider-registry.spec.ts	Updated ProviderRegistry constructor to inject SkillManager and pass it to ProviderImpl instances; updated test setup to provide mocked skillManager dependency.

Sequence Diagram

sequenceDiagram
    actor Extension
    participant Provider as ProviderImpl
    participant SkillManager
    participant Backend as Skill Registration<br/>Backend

    Extension->>Provider: registerSkill({label, path})
    activate Provider
    Provider->>SkillManager: registerSkillFolder({<br/>label,<br/>baseDirectory: path,<br/>badge: extensionDisplayName<br/>})
    activate SkillManager
    SkillManager->>Backend: register skill folder
    activate Backend
    Backend-->>SkillManager: Disposable
    deactivate Backend
    SkillManager-->>Provider: Disposable
    deactivate SkillManager
    Provider-->>Extension: Disposable
    deactivate Provider
    Extension->>Provider: disposable.dispose()
    activate Provider
    Provider->>SkillManager: inner disposable.dispose()
    SkillManager->>Backend: cleanup
    deactivate Provider

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding the ability for extensions to register skills.
Description check	✅ Passed	The description references issue #1031 which is directly related to the changeset's objective of enabling extensions to register skills.
Linked Issues check	✅ Passed	The code changes implement the requirement from issue #1031 by adding a registerSkill method to the Provider interface and ProviderImpl class, enabling extensions to register skills.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing skill registration for extensions; no out-of-scope modifications were introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/extension-api/src/extension-api.d.ts (1)

943-946: Consider documenting path expectations on CreateSkillParams.path.

Since this is public API, a short TSDoc note (absolute vs relative, expected directory shape) would reduce extension-author ambiguity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/extension-api/src/extension-api.d.ts` around lines 943 - 946, Add a
short TSDoc comment to the CreateSkillParams.path declaration describing
expected format and semantics: state whether path is absolute or relative to the
extension root, whether it should point to a directory or a manifest file, and
any required directory structure or naming conventions; update the declaration
for the interface CreateSkillParams and specifically document the path property
so extension authors know the expected input for path.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/extension-api/src/extension-api.d.ts`:
- Around line 943-946: Add a short TSDoc comment to the CreateSkillParams.path
declaration describing expected format and semantics: state whether path is
absolute or relative to the extension root, whether it should point to a
directory or a manifest file, and any required directory structure or naming
conventions; update the declaration for the interface CreateSkillParams and
specifically document the path property so extension authors know the expected
input for path.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 52cdf950-a063-4a97-b7b2-e93b15044397

📥 Commits

Reviewing files that changed from the base of the PR and between 757516e and 58f49e4.

📒 Files selected for processing (5)

• packages/extension-api/src/extension-api.d.ts
• packages/main/src/plugin/provider-impl.spec.ts
• packages/main/src/plugin/provider-impl.ts
• packages/main/src/plugin/provider-registry.spec.ts
• packages/main/src/plugin/provider-registry.ts

[jeffmaury]

Seems to me the implementation of the extension API is missing ?

[benoitf]

Seems to me the implementation of the extension API is missing ?

AFAIK it's there. It's adding a new entry inside a provider so it's modifying ProviderImpl and reusing existing methods where we could add a folder and gets a Disposable

[gastoner]

LGTM codewise

[jeffmaury]

LGTM