Skip to content

feat(skills): gaia skill migrate — OpenClaw/Hermes skills to GAIA format - #2693

Open
kovtcharov wants to merge 6 commits into
mainfrom
claudia/task-2eec97f9
Open

feat(skills): gaia skill migrate — OpenClaw/Hermes skills to GAIA format#2693
kovtcharov wants to merge 6 commits into
mainfrom
claudia/task-2eec97f9

Conversation

@kovtcharov

Copy link
Copy Markdown
Contributor

Skill authors on OpenClaw/ClawHub and Hermes had no path onto GAIA short of rewriting each SKILL.md by hand, which meant GAIA's catalog started empty while a large body of compatible skills sat one format away. gaia skill migrate --from openclaw|hermes|auto reads the foreign metadata.<vendor> namespace and writes a metadata.gaia block, so an existing library moves over largely untouched — point it at one skill or a whole ClawHub checkout. Every migrated skill lands at the experimental tier via the same trust reset gaia skill import uses, and a skill that needs a local capability GAIA cannot yet enforce is refused with the reason rather than quietly stripped of the permission to make the migration look like it worked.

Validated against 26 real published ClawHub skills (commit-pinned, licenses and digests recorded in tests/fixtures/openclaw_skills/PROVENANCE.md): 13 migrate cleanly, 13 are correctly refused — 10 because they shell out, 3 because they ship no frontmatter at all.

The dashboard half of #692 is deferred to #701, deliberately. There is no skills surface in the Agent UI today — no /api/skills router, no panel, not even the word "skill" anywhere in src/gaia/ui/ — and no in-flight branch adding one. Building visual import here would mean building Phase 3's surface to hang it off, and a user would import a skill into a UI that cannot list it. The migrator is UI-ready instead: MigrationOutcome.to_dict() is JSON-serializable and --json emits the full report, so the panel wires to gaia.skills.migrate rather than reimplementing the mapping. The plan doc's Phase 3 section now records that.

Two bugs surfaced only by the real corpus, both silent-loss paths that a docs-driven implementation would have shipped:

  • A skill putting its OpenClaw fields directly under metadata (or at the top level, with no namespace key) migrated with zero permissions — so 1password shelling out to op was refused when namespaced and silently accepted when not. Same upstream skill, opposite verdicts, the unsafe one passing.
  • metadata.openclaw: "a string" had its value dropped while the report claimed the namespace was "absent or empty".

Stacked on #2669 (Phase 1) — base retargets to main when that merges.

Closes #692

Test plan

  • python -m pytest tests/unit/test_skills_migrate.py -q — 64 tests: round-trip through parse_skill_file, vendor fields surviving under metadata.<vendor>, experimental stamped over a claimed verified/community, local-capability sources reported unmigratable rather than downgraded, and the Hermes path through the same command.
  • python -m pytest tests/unit/test_skills_format.py tests/unit/test_skills_cli.py -q — Phase 1 unaffected by the shared reset_security_tier() / split_frontmatter() extraction.
  • Every one of the 26 real fixtures is asserted to reach a defined verdict (parametrized), and two guard tests keep the corpus honest: ≥10 real skills present with provenance, and both verdicts still exercised.
  • Cold-state CLI run — nothing touches a real ~/.gaia/skills:
    export GAIA_CONFIG_DIR=$(mktemp -d)
    gaia skill migrate tests/fixtures/openclaw_skills --from openclaw --dry-run
    gaia skill migrate tests/fixtures/openclaw_skills --from openclaw   # exit 4: 13 refused
    gaia skill info linear                                              # security tier: experimental
  • python util/lint.py --all

@github-actions github-actions Bot added documentation Documentation changes tests Test changes labels Jul 30, 2026
itomek
itomek previously approved these changes Jul 30, 2026
@kovtcharov kovtcharov self-assigned this Jul 30, 2026
kovtcharov-amd
kovtcharov-amd previously approved these changes Jul 30, 2026
Base automatically changed from claudia/task-7afdc4b3 to main July 31, 2026 00:46
@kovtcharov-amd
kovtcharov-amd dismissed stale reviews from itomek and themself July 31, 2026 00:46

The base branch was changed.

Skill authors on OpenClaw/ClawHub and Hermes had no path onto GAIA short of
rewriting each SKILL.md by hand. `gaia skill migrate --from openclaw|hermes|auto`
reads the foreign metadata.<vendor> namespace and writes a metadata.gaia block,
so an existing library moves over largely untouched.

Fields GAIA models fully are consumed into metadata.gaia; anything partially or
un-modeled stays under metadata.<vendor> and is named in the migration report,
so nothing is dropped silently. Every migrated skill lands at the experimental
tier via the same trust-reset `gaia skill import` uses — now a single shared
reset_security_tier() rather than two security paths.

v1 bridges instruction-only and connector-backed skills. A source that shells
out (requires.bins) or reads config files (requires.config) maps onto shell /
filesystem, which have no enforcement until the Phase 2 sandbox, so it is
refused with that phase's own error rather than quietly stripped of the
permission to make the migration look successful.

Output is round-tripped through parse_skill() before install, so a migration
can never emit a SKILL.md the parser rejects.

Fixtures are 26 real published OpenClaw skills, each fetched from a
commit-pinned URL and recorded in PROVENANCE.md with its license and digest.
Validating against 26 published ClawHub skills found two bugs that only appear
off the documented happy path.

An OpenClaw skill that puts its fields directly under `metadata`, or at the top
level with no namespace key, migrated "successfully" with zero permissions — so
`1password` shelling out to `op` was refused when namespaced and silently
accepted when not. Same upstream skill, opposite verdicts, with the unsafe one
passing. Vendor fields are now located wherever the source actually put them
(metadata.<ns>, top-level <ns>, or inlined), and leftovers are written back to
that same place so a round-trip never relocates them.

About 5% of published skills ship no frontmatter at all; that raised out of the
batch loop and killed the whole run. It is now a per-skill blocker.

Also: aliases are first-wins with a note rather than merged (several skills
duplicate one payload under two aliases, and merging double-counts every
requirement), and envVars entries keyed `key` rather than `name` are read.
61 tests over the migrator and the shipped CLI verb. Beyond the unit cases
(round-trip, vendor preservation, trust reset, local-capability refusal, the
Hermes path), every one of the 26 real published skills is asserted to reach a
defined verdict: migrated-and-validating, or refused-with-a-reason, never a
crash and never a 'success' that quietly dropped a declared capability.

Two tests guard the corpus itself — that at least 10 real skills are present
with provenance, and that it still exercises both verdicts, so the refusal path
cannot rot untested if fixtures are re-pinned.
… migrate

Self-review caught a second silent-loss path: `metadata.openclaw: "a string"`
has no fields to map, and the value was dropped while the report claimed the
namespace was "absent or empty". It is now carried through verbatim with an
accurate note. Pointing migrate at ./OTHER.md also resolved to a sibling
SKILL.md instead of the named file.

Docs: `gaia skill migrate` is documented in the CLI reference, and the plan
doc's migration section moves from PROPOSED to shipped — with the field-by-field
OpenClaw/Hermes mapping tables, the divergences real published skills show
against OpenClaw's own documented schema, and the honest 13-migrate/13-refused
split of the 26-skill corpus. Phase 3 now scopes to the Agent UI panel alone
(#701) and records that its visual import should reuse this migrator.
An install collision on the fifth skill of a batch aborted the run and printed
nothing about the four that had already installed. Install failures are now
tracked per skill and reported separately from `blockers` — a skill that
migrated cleanly but could not be written is a different thing to tell a user
than one that was unmigratable, and the report prints either way.
…time

#888 landing rewrote the shipped-vs-proposed claims in skill-format.mdx and
agent-skills.mdx. Rebasing on top merged textually clean but left both files
contradicting themselves about what exists.

skill-format.mdx: the audit table's migrate row was column-misaligned (path in
the Symbol column, "SHIPPED" in Location), so it rendered as garbage and sat
below the NOT FOUND block; it now sits with the other shipped rows in the right
columns, alongside a row for the shared trust reset. The Retractions warning
read as if `gaia skill migrate` were itself retracted — it is the runtime
adapter that is deferred, not the migration path — and the "no longer a
retraction" list now names migrate. The acceptance-traceability CLI row
enumerated the shipped verbs without it.

agent-skills.mdx: its own intro promises the spec treats Hermes and OpenClaw as
compatible formats, but the compatibility section documented only two of the
three directions; third-party -> GAIA is now covered, and the header's shipped
list names the verb.

Also unified the #cross-format-compatibility--migration anchor across all three
references. The repo's link checker strips fragments before validating, so a
wrong anchor fails silently rather than in CI.
@kovtcharov
kovtcharov force-pushed the claudia/task-2eec97f9 branch from a7bac64 to 5b64e99 Compare July 31, 2026 01:02
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Verdict: Request changes

This PR adds gaia skill migrate to convert OpenClaw/Hermes skills into GAIA format, with a solid real-corpus fixture set (26 published skills) and thorough tests. Nice work overall.

The one blocking issue: the documented flagship command aborts the whole batch on real input. Running gaia skill migrate ~/clawhub-skills --dry-run (default --from auto, exactly the "whole collection" example in the CLI docs) crashes out at the first skill that has no vendor namespace — none of the other skills in the directory are migrated or even reported. A real ClawHub collection routinely contains plain Agent-Skills documents, so this fires on the primary use case. The fix: one un-detectable skill in a collection should be reported as a per-skill blocker and the batch should continue — which is exactly what this PR already does for the no-frontmatter case (and even tests). The no-vendor-namespace case just wasn't given the same treatment at the batch layer.

Real-world evidence

The evidence bundle (evidence-bundle.md, CLI-only surface — API/MCP correctly N/A) exercised the real gaia skill migrate CLI and reproduced the bug above against this PR's own fixture corpus:

$ gaia skill migrate tests/fixtures/openclaw_skills --dry-run
❌ tests/fixtures/openclaw_skills/anki-connect/SKILL.md: no vendor namespace found, so there is nothing to migrate. ...
$ echo $?   # 4 — that is the ENTIRE output; the other 25 skills are never processed

Forcing --from openclaw (bypassing auto-detect) instead produces the full 26-skill per-skill report (13/26 migrated), confirming the abort is specific to vendor="auto" hitting a no-vendor skill. Real fixtures that trigger it: anki-connect, clawhip, gui-agent-harness. The bundle also verified the happy paths (single migrate + install, --force collision, --json, Hermes auto-detect, error paths), 264 unit tests passing, lint clean, and a spot-regression pass over sibling gaia skill subcommands. My verdict rests on that evidence plus static review of the two code paths below.

🔍 Technical details

🔴 migrate aborts the whole batch when auto-detect fails on any skill (src/gaia/skills/cli.py:432)

_handle_migrate builds outcomes in a bare list comprehension with no per-item guard:

outcomes = [
    migrate_skill_dir(source, vendor=args.vendor, name=args.name)
    for source in sources
]

For vendor="auto" (the default), migrate_text (src/gaia/skills/migrate.py:617-627) raises SkillValidationError when detect_vendor() returns None. That raise propagates out of the comprehension, through handle()'s single outer except SkillValidationError (cli.py:213), and aborts the entire run — so a collection where one member is a plain Agent-Skills doc reports nothing for the other members.

This contradicts the PR's own batch-resilience design: the no-frontmatter case is caught inside migrate_text and returned as a per-skill blocker (migrate.py:605-615), locked in by test_missing_frontmatter_is_a_blocker_not_a_crash ("one must not abort a batch"). The no-vendor-namespace case deserves the same treatment.

Note the single-skill raise semantics are intentional and tested at the library level (test_auto_detect_refuses_a_plain_agent_skills_document calls migrate_skill_dir directly), so the cleanest fix is at the batch/CLI layer — guard each item and convert the raise to a blocker outcome:

    outcomes = []
    for source in sources:
        try:
            outcomes.append(
                migrate_skill_dir(source, vendor=args.vendor, name=args.name)
            )
        except SkillValidationError as exc:
            # One un-detectable skill in a collection must not abort the batch —
            # report it as a per-skill blocker, like the no-frontmatter case.
            outcomes.append(
                MigrationOutcome(source=Path(source), vendor="unknown", blockers=[str(exc)])
            )

This needs MigrationOutcome added to the from gaia.skills.migrate import (...) block at cli.py:34. Please also add a CLI-level regression test: a directory where one skill has a vendor namespace and one is a plain doc, run with default vendor="auto", and assert the batch still reports both (the existing test_cli_migrate_reports_a_collision_without_hiding_the_batch doesn't cover this because both its fixtures carry a vendor block).

Strengths

  • Batch resilience is a first-class concern elsewhere — the no-frontmatter and install-collision paths are both handled as per-skill outcomes with dedicated tests and clear user messaging.
  • Migration is validated against 26 real published skills as fixtures, not just synthetic inputs, and refused skills fail loudly with actionable reasons rather than silently stripping permissions.
  • Clean separation: migrate_text (pure) vs migrate_skill_dir/install_migrated (I/O), --json output for scripting, and consistent exit codes.

kovtcharov added a commit that referenced this pull request Aug 4, 2026
Review of #2806 caught three artifacts the plan cited as existing that are
not on main — they live in open PRs. An implementer grepping for them would
come up empty.

  src/gaia/skills/sets.py                  -> PR #2695
  workers/agent-hub/src/skill-manifest.ts  -> PR #2668
  tests/fixtures/openclaw_skills/          -> PR #2693

Adds a Forward dependencies table naming each PR, what needs it, and what to
do if it has not merged. The material consequence: P3.4 skill sets is gated
on #2695 landing, not just on P3.1-P3.3, because porting a module still in
review yields two implementations that diverge on the first round of review
feedback. P3.1 and P5.2 are not gated — they work from the 6-skill corpus on
main and widen to 32 when #2693 lands, so the conformance gate discovers its
corpus instead of hardcoding a path that does not yet resolve.

Also corrects "FAISS in exactly three places" to name all five construction
sites across four subsystems. The load-bearing claim is unchanged and in fact
stronger: every one is IndexFlatL2/IndexFlatIP, and a grep for
IndexIVF/IndexHNSW/IndexPQ/index_factory returns nothing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Documentation changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenClaw skill compatibility layer with dashboard integration

3 participants