Dedupe and organize skills/sources by jamadeo · Pull Request #8731 · aaif-goose/goose

jamadeo · 2026-04-21T17:02:30Z

No description provided.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47b8d88047

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-21T17:08:52Z

+        dirs.push((wd.join(".claude").join("skills"), false));
+        dirs.push((wd.join(".agents").join("skills"), false));


Restrict listed skills to CRUD-backed directories

This adds non-canonical skill roots (for example .claude/skills and .agents/skills) to shared discovery, and list_sources now uses that shared discovery path. However, create/update/delete/export still resolve only <project>/.goose/skills for project scope and ~/.agents/skills for global scope, so some listed entries cannot be updated or deleted and will fail with "Source not found" despite being returned by the list API.

Useful? React with 👍 / 👎.

alexhancock

Looks good for organizing skills in project vs global

Will there be a followup UI one to allow for creation in either a project or global?

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87eec39364

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-21T19:22:12Z

+    let mut sources: Vec<SourceEntry> = discover_skills(working_dir.as_deref())
+        .into_iter()
+        .filter(|s| s.source_type == SourceType::Skill)
+        .collect();


Keep listed source names directory-based for CRUD

list_sources now passes through entries from discover_skills, but that discovery path sets SourceEntry.name from SKILL.md frontmatter rather than the folder name. update_source/delete_source/export_source still resolve on-disk paths as <base>/<name>, so any skill whose frontmatter name differs from its directory (or omits name) becomes non-round-trippable: it is listed under one name but CRUD operations against that listed name return "Source not found". Use directory-derived names (or a stable directory identifier) in the list API for editable sources.

Useful? React with 👍 / 👎.

baxen

i like it

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aee5c41acf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-23T13:48:16Z

+    if name.contains('/') {
+        warn!("Skill name '{}' contains '/', skipping", name);
+        return None;
+    }


Reject non-kebab skill names during discovery

parse_skill_content now accepts any non-empty frontmatter name except those containing /, but downstream CRUD still enforces validate_skill_name (lowercase/digits/hyphen) and the Goose2 edit/create flows assume that same constraint. In practice, a discovered skill named like Code Review or my_skill will appear in the list but cannot be duplicated (create fails validation) and may be impossible to edit in the dialog when the read-only name is considered invalid. Discovery should either enforce the same validator or map invalid frontmatter names to a stable valid identifier.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4f88211a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-23T15:48:37Z

+    require_skill_type(source_type)?;
+    validate_skill_name(name)?;
+
+    let dir = resolve_discoverable_skill_dir(path)?;


Enforce editable scope before updating a source

update_source resolves the target with resolve_discoverable_skill_dir, so any discovered skill path (including .claude/skills and other read-only locations) can still be renamed and rewritten even when it is advertised as editable: false. This breaks the new SourceEntry.editable contract and allows API clients to mutate files outside Goose-managed writable roots; use resolve_skill_dir (or an equivalent editable check) before performing rename/write.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-23T15:48:37Z

+pub(crate) fn is_global_skill_dir(path: &Path) -> bool {
+    global_skills_dir().as_deref().is_some_and(|root| {
+        canonicalize_or_original(path).starts_with(canonicalize_or_original(root))
+    })


Return correct global scope for non-~/.agents skills

is_global_skill_dir only treats ~/.agents/skills as global, but discovery marks additional roots as global (Paths::config_dir()/skills, ~/.claude/skills, ~/.config/agents/skills). After editing one of those skills, update_source will return global: false, causing scope drift between list and update responses and breaking clients that rely on global for source management logic.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8d71e82b7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-23T16:18:53Z

    if base.join(&final_name).exists() {
        final_name = format!("{}-imported", name);


Revalidate imported name after collision suffixing

The code validates name once, but the collision path appends -imported / -imported-N without validating the new value. With the new 64-character limit, importing a colliding 64-char skill produces an overlong final_name that bypasses validation and creates a source name inconsistent with normal create/update rules. Re-run validate_skill_name (or truncate safely) after generating final_name so imported names always satisfy the same constraints.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-23T16:18:53Z

+        fs::rename(&dir, &target_dir).map_err(|e| {
+            Error::internal_error().data(format!("Failed to rename source directory: {e}"))
+        })?;


Avoid renaming before write succeeds

This renames the source directory before writing the updated SKILL.md. If the subsequent write fails (e.g., permissions or disk-full), the call returns an error but the directory has already moved, so the caller’s original path is now broken and the update is partially applied. Make the operation atomic by writing first or rolling back the rename when the write fails.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da7a542a61

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-23T16:45:38Z

+    if let Some(wd) = working_dir {
+        dirs.push((wd.join(".goose").join("skills"), false));
+        dirs.push((wd.join(".claude").join("skills"), false));
+        dirs.push((wd.join(".agents").join("skills"), false));


Deduplicate overlapping roots before assigning scope

When working_dir is the user's home directory, all_skill_dirs pushes ~/.agents/skills and ~/.claude/skills twice (first as project-scoped false, then as global true). Discovery dedupes by skill name later, so the first pass wins and those globally installed skills are returned with global: false. Any client that relies on the global flag for scope-aware management will misclassify global skills in this common "run from home" context.

Useful? React with 👍 / 👎.

jamadeo · 2026-04-23T19:07:41Z

Looks good for organizing skills in project vs global

Will there be a followup UI one to allow for creation in either a project or global?

@alexhancock yeah, there should be more UI work around project-discovered skills. I'm not even sure how you access them in the current app.

This PR has grown quite big with skills editing and I'm still not even sure it goes where we want it to, but I'll merge and we can keep iterating.

* main: (29 commits) chore(deps): bump winreg from 0.55.0 to 0.56.0 (#8829) Fix grammar issue (#8669) colorize context window indicator (#8851) Refresh canonical model metadata from models.dev (#8838) fix(ci): prevent flaky smoke test timeouts from failing the build (#8837) updates: release 0.19.0 of the tui/sdk/etc (#8806) add a goose2 signed release flow (#8728) Port provider tests to typescript (#8237) refactor: make ACP server smaller (#8787) Add NVIDIA provider, and improve declarative provider UX (#8798) fix: removed failed provider test for deprecated providers (#8801) fix: only call cleanup when the pr is from same repo (#8799) chore: check stale for draft pr (#8803) fix: use _meta instead of meta in newSession request (#8796) fix: add missing underscore prefix in updateWorkingDir method name (#8743) feat: migrate session metadata storage from frontend overlay to backend (#8769) Add more info to BUILDING_LINUX (#8789) feat(acp): Align to new request patterns of ACP Streamable HTTP/WS transport (#8605) Dedupe and organize skills/sources (#8731) docs: add skills slash command (#8783) ...

…esign Merges main's path-based source addressing (#8731), redesigned skills library (#8868), and 70+ other commits while preserving the projects-as-sources work. Backend: - SourceType gains Project alongside main's Skill/BuiltinSkill/Recipe/Subrecipe/Agent - SourceEntry.directory renamed to .path so the field name matches its role (a stable path identifier passed back to update/delete/export, regardless of whether the underlying entity is a directory or file) - SourceEntry.properties (free-form metadata bag) coexists with main's supporting_files - ListSourcesRequest.include_project_sources scans every known project's workingDirs and tags resulting skills with projectName/projectDir - CreateSourceRequest.project_id resolves to the project's first workingDir - Projects stored as <dataDir>/projects/<slug>.md flat-file frontmatter - read_project / project_working_dirs helpers used by agent.rs to inject project instructions into the system prompt - Drops our SetSessionProjectRequest in favor of main's identical UpdateSessionProjectRequest (_goose/session/update_project) Frontend: - features/projects/api/projects.ts rewritten on the typed SDK; ProjectInfo loses createdAt/updatedAt/artifactsDir, gains path - Skills API uses main's typed GooseSourcesCreate/List/Update/Delete/Export/ Import calls. toSkillInfo prefers backend-provided properties.projectName/ projectDir tags over path-derived guesses - CreateSkillDialog keeps our save-location picker (Global vs each project) on top of main's redesigned UI; edits use path-based addressing - SkillsView takes main's redesigned filter row + sectioned list rendering - AppShell drops the old inline-type adapter - Removes ui/goose2/src-tauri/src/commands/projects.rs (508 lines) and its 9 Tauri command registrations; main's project_icons.rs (filesystem icon scanning, a real shell concern) is kept Verification: cargo test -p goose passes (incl. 17 sources tests, 2 new project ones); cargo clippy clean; pnpm tsc clean; pnpm test 538/538; pnpm check clean.

jamadeo added 3 commits April 21, 2026 12:56

Consolidate and dedupe skills

18a340f

other way around

47b8d88

back to the original

17b3c01

chatgpt-codex-connector Bot reviewed Apr 21, 2026

View reviewed changes

alexhancock approved these changes Apr 21, 2026

View reviewed changes

Consolidate existing Source type

87eec39

chatgpt-codex-connector Bot reviewed Apr 21, 2026

View reviewed changes

move parse_frontmatter

c757c43

baxen approved these changes Apr 22, 2026

View reviewed changes

jamadeo added 11 commits April 22, 2026 10:12

paths are identifiers

e18f113

Merge remote-tracking branch 'origin/main' into jamadeo/dedupe-sources

155dc89

merge and schema

bc68085

update callers to use path

76f9ed4

global is inferred

aba3b59

docs

0159df2

validation match

336d8f2

allow export

e6e08ea

clean up

f8e6464

type gen

aee5c41

show loading

6c43c51

chatgpt-codex-connector Bot reviewed Apr 23, 2026

View reviewed changes

jamadeo added 4 commits April 23, 2026 10:48

edit name

ad06590

Merge remote-tracking branch 'origin/main' into jamadeo/dedupe-sources

306adb1

format

af52741

fix type

d4f8821

chatgpt-codex-connector Bot reviewed Apr 23, 2026

View reviewed changes

update e2e

c8d71e8

chatgpt-codex-connector Bot reviewed Apr 23, 2026

View reviewed changes

forget about non-editable for now

da7a542

chatgpt-codex-connector Bot reviewed Apr 23, 2026

View reviewed changes

jamadeo added this pull request to the merge queue Apr 23, 2026

Merged via the queue into main with commit 5dd9f08 Apr 23, 2026
24 of 26 checks passed

jamadeo deleted the jamadeo/dedupe-sources branch April 23, 2026 19:22

github-actions Bot mentioned this pull request Apr 28, 2026

chore(release): release version 1.33.0 #8875

Closed

		dirs.push((wd.join(".claude").join("skills"), false));
		dirs.push((wd.join(".agents").join("skills"), false));

		if base.join(&final_name).exists() {
		final_name = format!("{}-imported", name);

Conversation

jamadeo commented Apr 21, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

alexhancock left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

baxen left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

jamadeo commented Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants