Skill ID/folder basename is indeterminate

Skills installed via the Neon appstore are appended with .neon, regardless of the owner repository name; skills installed via github URL are appended with .neongeckocom, but could be the same reference as the .neon skill. I think this raises a couple questions:

Should a skill be able to set it's ID independent of GitHub Repo + Owner? (Like an override in skill.json)
Is there ever a reason to search a Github URL instead of using it directly?

My inclination is to say that we could allow for some skill ID override which could prevent installing 2 forks of the same skill for example, but could also provide another way to maliciously impersonate a different skill (though that's always possible).

For URL parsing, I think the rationale is that a cached entry from an appstore will be faster to load, though I'm thinking parsing the URL directly is more in line with what a user would expect if they pass a URL (rather than a skill ID, search term, etc).

i think the id should be deterministic and always come from the github url, repo.author

but i agree we need a mechanism for duplicate skills, im thinking about adding 2 fields to the json:

conflicts_with -> known intent collisions, but is not a replacement skill
wants_to_replace -> these skills should be uninstalled before installing this one, require user confirmation

this means we have 2 problems to solve, adding this new metadata and ensure skill_id is always determined from url

Skills installed via the Neon appstore are appended with .neon, regardless of the owner repository name; skills installed via github URL are appended with .neongeckocom, but could be the same reference as the .neon skill. I think this raises a couple questions:

bug - neon appstore should use the url for final skill_id

* Should a skill be able to set it's ID independent of GitHub Repo + Owner? (Like an override in `skill.json`)

no, this allows evil skills and makes us lose determinism, once we get to skill plugins and system skills this determinism becomes super important to determine priorities and which skill replaces which

* Is there ever a reason to search a Github URL instead of using it directly?

only to find out in what appstores it was submitted, maybe to find a reviewed version

bug - neon appstore should use the url for final skill_id

I will investigate this as a bug then later today

no, this allows evil skills and makes us lose determinism, once we get to skill plugins and system skills this determinism becomes super important to determine priorities and which skill replaces which

I think plugins will have to use some different methodology since they are not installed from a single source; module names will be guaranteed unique within a given environment though and author may be usable to find conflicts with locally cloned versions of the same skill.

only to find out in what appstores it was submitted, maybe to find a reviewed version

Maybe this should be bypassed then when a branch is specified in the URL? This would override any version specified in an appstore

this means we have 2 problems to solve, adding this new metadata and ensure skill_id is always determined from url

For the case of forked skills, does it make sense to assume a skill conflicts with any skill with the same repo name? This could be a good starting point; then the metadata spec might be:

replaces:
    - skill-weather
    - weather-skill

rather than:

replaces:
  - skill-weather.mycroftai
  - skill-weather.neongeckocom
  - weather-skill.some_other_owner

the idea is that we want to follow XDG specs and allow system skills, in this case a user skill would replace a system skill only if skill_id is the same

our current plugin implementation allows plugins to specify their skill_id, if a user skill has the same skill_id i think it should also replace it

the priority order becomes

system folder
plugins
user folder

the skill_id is what identifies unique skills, regardless of the appstore or version they are running.

this would require a PR in core to handle the loading/unloading of skills and monitoring of these locations

a forked skill could probably be detected and used to auto populate the new fields if they are not specified by the user

in our store we should require users to manually define these fields probably

the skill_id is what identifies unique skills, regardless of the appstore or version they are running.

This does mean that a properly forked and modified skill will have a different ID than the base.

our current plugin implementation allows plugins to specify their skill_id, if a user skill has the same skill_id i think it should also replace it

Mostly a matter of documentation I suppose, but a developer here needs to remember to use their github repo name here, which could be lost or misrepresented when copy/pasting boilerplate; would it make sense to build the skill_id from package name?

in our store we should require users to manually define these fields probably

Makes sense, though this wouldn't account for forks made after the fact; I think both auto-detection and manual specification may be needed here..

repo.author should ensure its unique and always built from a url, a fork would immediately get a unique id since author changes. since we only support github right now this is a safe assumption

core skills (fundamental functionality) -> plugins version pinned in requirements.txt
pre installed skills -> system folder
osm installs -> user folder

the only situation where a skill_id does not match repo.author is if user git clones manually or somehow adds the skill to the folder, its a matter of documentation as you said

bug - neon appstore should use the url for final skill_id

Looks like authorname in skill.json is preferred over URL parsing. Similar for skill_folder which prefers JSON value over git parsing. Couple things I see here:

install should probably just use uuid instead of building it again from repo and author
uuid should probably prefer repo parsing over json

ovos_skill_manager/ovos_skills_manager/skill_entry.py

Line 127 in 9f3f4f7

    
           return self.json.get("authorname") or self.url.split("/")[-2] if self.url and "/" in self.url else ""

ovos-core now supports multiple skill folders, the discussion in this issue is relevant to this commit OpenVoiceOS/ovos-core@aaa159a