Skip to content

chore: goose2 skill refactor#8897

Merged
lifeizhou-ap merged 10 commits intomainfrom
lifei/goose2-skill-refactor
Apr 30, 2026
Merged

chore: goose2 skill refactor#8897
lifeizhou-ap merged 10 commits intomainfrom
lifei/goose2-skill-refactor

Conversation

@lifeizhou-ap
Copy link
Copy Markdown
Collaborator

@lifeizhou-ap lifeizhou-ap commented Apr 29, 2026

Summary

Refactor the Skills feature for better separation of concerns. No user-facing behavior changes, except clearer error feedback via toasts.

Changes

Extractions

  • hooks/useSkillImportExport.ts — extracts import/export logic (file picker, drop zone, toast feedback) out of SkillsView.
    • Why: SkillsView had a duplicated import flow (button vs. drag-and-drop) and an inline export handler. The hook collapses these into one reusable surface.
  • lib/skillsPath.ts — moves path helpers (basename, normalizePath, getSkillFileLocation, deriveProjectRoot, getRenamedSkillFileLocation) out of api/skills.ts and CreateSkillDialog.
    • Why: path logic was duplicated across layers; co-locating it makes it testable and prevents drift.
  • lib/skillsHelpers.tsfilterSkills and groupSkills extracted from SkillsView's useMemo blocks; isValidSkillName and formatSkillName moved here from CreateSkillDialog.
    • Why: pulls pure functions out of the view so SkillsView becomes a thin orchestrator and the rules are unit-testable.
  • ui/SkillsToolbar.tsx and ui/SkillCategoryFilter.tsx — search bar, filter row, and category dropdown lifted out of SkillsView.
    • Why: self-contained UI blocks with their own props; inlining them bloated the parent without benefit.

Renames

  • CreateSkillDialogSkillEditor (and matching test file).
    • Why: the component already handled both create and edit modes; the old name was misleading.

Type cleanup on SkillInfo

  • Removed directoryPath — was always assigned source.directory, identical to path.
  • Removed editable — was hardcoded to true, so the skill.editable ? … : null gates around Edit and Delete were dead conditionals. Simplified to unconditional rendering.
  • Added EditingSkill = Pick<SkillInfo, …> so SkillEditor and SkillsDialogs share a single type instead of redeclaring the same five fields inline.
  • Why: removes dead fields and stops three files from drifting on the same shape.

User-facing feedback: custom notification → toast

  • Replaced the bottom-right notification div (export-only, 3-second timeout, manually managed) with sonner toasts.
  • Added toasts for previously silent failures: load error, import success and error, delete success and error.
  • Why: the old notification was export-only; other failures logged to the console and the user saw nothing. Toasts give consistent feedback and remove a piece of bespoke UI.

i18n

  • Added loadError, importSuccess, importError, exportError, deleteSuccess, deleteError in en and es.

Net effect

  • SkillsView.tsx: 325 → 55 lines (orchestration only).
  • No user-visible regressions; users now get toast feedback on operations that previously failed silently.

@lifeizhou-ap lifeizhou-ap requested a review from morgmart April 30, 2026 01:21
* main: (24 commits)
  fix: copy and content improvements in goose2 (#8886)
  feat: make ollama host configurable in goose2 (#8912)
  polish sidebar navigation and project icons (#8896)
  fix: model picker stays usable during provider loading (#8900)
  feat: update provider row after saving credentials (#8914)
  feat: support google model inventory refresh (#8913)
  chore: Added goose 2 UI refactor review skill (#8903)
  blog: goose with peekaboo (#8884)
  blog: Built-in Local Inference blogpost. (#8808)
  perf: parallelize provider resolution and eagerly init SQLite pool (#8899)
  refactor: update goose2 credential management behind provider-scoped ACP/core API (#8887)
  fix: handle acp requests concurrently (#8781)
  build: set LLAMA_STATIC_CRT for Windows CUDA (#8901)
  perf: deduplicate _goose/providers/list RPC call at startup (#8873)
  chore: add a bit more instructions in the release pr (#8890)
  chore: disable spellcheck in model search (#8889)
  add skills to the chat composer (#8881)
  mergeable configs + cleanup (#8378)
  refactor: agent provider to use explicit type states (#8879)
  [goose2] MCP Apps: hydrate and replay app payloads in Goose2 (#8632)
  ...
@lifeizhou-ap lifeizhou-ap requested a review from matt2e April 30, 2026 03:35
@lifeizhou-ap lifeizhou-ap enabled auto-merge April 30, 2026 04:17
@lifeizhou-ap lifeizhou-ap added this pull request to the merge queue Apr 30, 2026
Merged via the queue into main with commit 66692e9 Apr 30, 2026
25 checks passed
@lifeizhou-ap lifeizhou-ap deleted the lifei/goose2-skill-refactor branch April 30, 2026 04:34
DOsinga pushed a commit that referenced this pull request Apr 30, 2026
Pulls in 7 new commits from main:
- #8932 break up acp/server.rs into submodules
- #8924 custom providers UI/ACP
- #8935 keep renamed skills open in detail view
- #8897 skill refactor (CreateSkillDialog -> SkillEditor)
- #8886 copy/content polish
- #8905 Gemini OAuth fix
- #8546 command injection patterns

Conflicts resolved:
- crates/goose/src/acp/server.rs: took main's split structure;
  added project-related plumbing (properties, project_id,
  include_project_sources) into crates/goose/src/acp/server/sources.rs.
- update_thread_metadata: pub(super) so server/sessions.rs (which
  hosts on_update_session_project) can call it.
- ui/goose2/src/features/skills/api/skills.ts: dropped
  directoryPath/editable (not on main's SkillInfo); kept our
  projectName/projectDir lookup from backend properties.
- ui/goose2/src/features/skills/ui/SkillEditor.tsx (renamed from
  CreateSkillDialog.tsx in #8897): kept main's structure; re-added
  our save-location picker (Global vs each project) and onSaved
  callback.
- ui/goose2/src/shared/i18n/locales/en/skills.json: took main's
  casing for editTitle/newTitle; kept our new keys (global,
  globalHint, projectHint, saveLocation).

Bumped useChatSessionController.ts file-size limit by 10 lines
(840 -> 850); justification updated to mention ACP project sync.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants