Skip to content

fix: keep renamed skills open in detail view#8935

Merged
kalvinnchau merged 1 commit intomainfrom
kalvin/skill-edit
Apr 30, 2026
Merged

fix: keep renamed skills open in detail view#8935
kalvinnchau merged 1 commit intomainfrom
kalvin/skill-edit

Conversation

@kalvinnchau
Copy link
Copy Markdown
Collaborator

Category: fix
User Impact: Users stay on the renamed skill's detail page after saving changes instead of being returned to the skills list.
Problem: Renaming a skill changed its ID, so the skills view could no longer match the previous active skill and fell back to the list page. This made a successful rename feel like it lost the user's place.
Solution: The editor now reports the saved skill back to the skills view, which refreshes the list and selects the saved skill ID when it is present.

File changes

ui/goose2/src/features/skills/ui/SkillEditor.tsx
Renamed the save callback to reflect both create and edit flows, and returns the saved skill from updates so the parent can keep the correct detail view selected.

ui/goose2/src/features/skills/ui/SkillsDialogs.tsx
Threaded the renamed save callback through the dialog wrapper so create and edit flows share the same post-save handling.

ui/goose2/src/features/skills/ui/SkillsView.tsx
Refreshes skills after a save and keeps the saved skill active when it appears in the refreshed list.

ui/goose2/src/features/skills/ui/tests/SkillEditor.test.tsx
Updated callback naming expectations to match the editor's broader save behavior.

ui/goose2/src/features/skills/ui/tests/SkillsView.test.tsx
Added coverage for renaming a skill and staying on the renamed skill's detail page.

Reproduction Steps

  1. Open the skills view and select an existing skill.
  2. Click Edit, change the skill name, and save the changes.
  3. Confirm the renamed skill remains open in the detail view and the editor closes.
  4. Run the skills view tests to confirm the rename flow remains covered.

Copy link
Copy Markdown
Collaborator

@morgmart morgmart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Reviewed locally and it looks good. Checks passed and I did not find any blocking issues.

@morgmart morgmart added this pull request to the merge queue Apr 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Apr 30, 2026
@kalvinnchau kalvinnchau added this pull request to the merge queue Apr 30, 2026
Merged via the queue into main with commit 43478a2 Apr 30, 2026
24 checks passed
@kalvinnchau kalvinnchau deleted the kalvin/skill-edit branch April 30, 2026 22:21
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