Skip to content

feat(skills): add manage-skills skill (RES-793)#37

Open
KarinaKKarinaK wants to merge 5 commits into
mainfrom
karinakalicka/res-793-write-skill-to-manage-skills
Open

feat(skills): add manage-skills skill (RES-793)#37
KarinaKKarinaK wants to merge 5 commits into
mainfrom
karinakalicka/res-793-write-skill-to-manage-skills

Conversation

@KarinaKKarinaK
Copy link
Copy Markdown
Collaborator

Closes RES-793 (sub-issue of RES-790).

Summary

Adds the manage-skills skill — a full CRUD workflow for the orq.ai Skills entity, plus authoring guidance, governance, and documented workarounds for three known platform issues.

What's in this PR

New files

  • skills/manage-skills/SKILL.md — 5-phase workflow:
    • Phase 1 list/audit (list_skills)
    • Phase 2 get/inspect (get_skill) — defensively handles INN-2836 empty version and doc
    • Phase 3 create (create_skill) — gathers name/description/tags/project scope/body with validation, flags +NEVER+ prose constraints (ENG-1604)
    • Phase 4 update (update_skill) — always get_skill first, patches explicit fields, routes body rewrites through optimize-prompt
    • Phase 5 delete + orphan cleanup (delete_skill) — warn → confirm delete → offer prune separately with two explicit consents; never auto-prunes agent.skills[] (INN-2861 workaround)
  • skills/manage-skills/resources/authoring-guide.md — naming, description, tags, project scoping
  • skills/manage-skills/resources/governance-guide.md — wiring Skills to agents via agent.skills[], ownership conventions, lifecycle, quarterly audit checklist
  • skills/manage-skills/resources/known-caveats.md — INN-2861, INN-2836, ENG-1604 with workarounds
  • commands/manage-skills.md/manage-skills [list|get|create|update|delete] [name-or-id] slash command

Modified files

  • agents/AGENTS.md — registered skill in path list + <available_skills> block
  • README.md — added row to skills table
  • tests/skills.md — 4 smoke-test scenarios + Critical Files entries
  • tests/commands.md — slash-command smoke tests
  • .claude-plugin/plugin.json, .codex-plugin/plugin.json, .cursor-plugin/plugin.json — bumped 0.0.2 → 0.1.0 (MINOR per CLAUDE.md rules for a new skill; plugins/orq/.codex-plugin/plugin.json is a symlink and inherits)
  • CHANGELOG.md## [0.1.0] - 2026-05-14 entry

Ticket criteria coverage

RES-793 criterion Where it's addressed
CRUD: list_skills, get_skill, create_skill, update_skill, delete_skill SKILL.md MCP-tools table + Phases 1-5; REST fallback (/v2/skills endpoints) documented for workspaces where the MCP tools aren't exposed
Authoring guidance: naming, description, tags, project scoping resources/authoring-guide.md (all 4 sections) + Phase 3 step 1 enforces them
Governance: wire skills to agents (agent.skills[]) resources/governance-guide.md "Wiring Skills to Agents" + Phase 3 step 4 offers to wire after create
INN-2861 Orphaned skill ids not pruned from agent.skills[] after DELETE /v2/skills/{id} resources/known-caveats.md documents workaround; SKILL.md Phase 5 implements warn-then-offer flow (never auto-prunes)
INN-2836 Snippet→Skill migration leaves skill.version empty + skill.doc never stamped resources/known-caveats.md covers BOTH fields; Phase 2 displays (unset) for either
Anti-pattern: +NEVER+ prose constraints → ENG-1604 (treated as soft suggestions) resources/known-caveats.md explains why prose constraints fail and recommends MCP tool gates / upstream validators / post-output filters; SKILL.md constraints + Phase 3 validation + Phase 4 body-rewrite flow all flag this

Verification

  • tests/scripts/validate-plugin-manifests.sh passes
  • All 4 plugin manifests at 0.1.0
  • agents/AGENTS.md lists the skill in both the path list and <available_skills> block
  • README.md skills table includes the new row
  • tests/skills.md includes 4 smoke-test scenarios (list, create with authoring guidance, delete with orphan handling, update without blind overwrite) and Critical Files entries
  • tests/commands.md includes slash-command smoke tests
  • CHANGELOG.md updated with INN-2861 / INN-2836 / ENG-1604 references

Test plan

  • Trigger via prompt: "list my orq.ai skills" → verify list_skills is called (or /v2/skills REST fallback) and output is a scannable table, not raw JSON
  • Trigger via prompt: "create a skill called test-skill" → verify Phase 3 asks for description, tags, project scope; rejects descriptions without a trigger phrase; flags any +NEVER+ in the proposed body
  • Trigger via prompt: "delete the test-skill" → verify Phase 5 lists referencing agents BEFORE deleting, asks for delete consent, then asks SEPARATELY for orphan-cleanup consent; verify it never auto-prunes
  • Slash command /orq:manage-skills list runs the list flow end-to-end
  • Confirm with backend team whether the 5 *_skill MCP tools are exposed on https://my.orq.ai/v2/mcp — if not, the REST fallback path is documented and works today

CRUD workflow for the orq.ai Skills entity (list/get/create/update/delete
via list_skills, get_skill, create_skill, update_skill, delete_skill — backed
by /v2/skills REST endpoints), plus authoring guidance, governance, and
workarounds for known platform caveats.

Includes:
- SKILL.md with 5 phases (list / get / create / update / delete + orphan
  cleanup). Phase 5 uses a warn-then-offer flow with two explicit consents
  (one for delete, one for the orphan-cleanup pass) — never auto-prunes.
- resources/authoring-guide.md — naming, description, tags, project scoping
- resources/governance-guide.md — wiring Skills to agents via agent.skills[],
  ownership, lifecycle, audit checklist
- resources/known-caveats.md — INN-2861 (orphaned skill ids in agent.skills[]
  after DELETE /v2/skills/{id}), INN-2836 (empty skill.version AND unstamped
  skill.doc after snippet→skill migration), ENG-1604 (+NEVER+ prose
  constraints treated as soft suggestions; recommends MCP tool gates)
- /manage-skills slash command — routes to phases by argument
- AGENTS.md, README.md skills table, and tests/skills.md + tests/commands.md
  smoke-test scenarios

Version bumped 0.0.2 → 0.1.0 across all 4 plugin manifests (MINOR per
CLAUDE.md rules for a new skill). CHANGELOG.md entry added.
@Baukebrenninkmeijer
Copy link
Copy Markdown
Collaborator

PR Review — manage-skills skill

Verified against real OpenAPI (orquesta-web/.openapi/v2/public/openapi.json) and orq.ai docs. Do not merge until critical items below resolved.


Critical fixes

C1. Remove all internal Linear ticket references

Internal tickets must not appear in public skill content. Strip INN-2861, INN-2836, ENG-1604 and the linear.app/orqai/issue/ENG-1604 URL everywhere:

  • SKILL.md lines 31, 32, 53, 54, 55, 80, 181, 191
  • resources/known-caveats.md section headers + ENG-1604 link line 85
  • resources/governance-guide.md lines 29, 101
  • tests/skills.md lines 150, 157, 165

Replace with behavior-only descriptions: "orphaned-reference behavior on delete", "empty version on migrated skills", "prose-negation anti-pattern". Keep workarounds; drop ticket IDs.

C2. Fix all MCP/REST parameter names

Real schema:

CreateSkillRequest: display_name, description, tags, path, project_id, instructions
UpdateSkillRequest: skill_id, display_name, description, tags, path, instructions, project_id
Skill (read):      skill_id, display_name, description, tags, project_id, path,
                   workspace_id, created_at, updated_at, created_by_id,
                   updated_by_id, instructions, version

Rewrites needed in SKILL.md, authoring-guide.md, governance-guide.md, tests/skills.md:

  • namedisplay_name
  • body / docinstructions
  • project (key) → project_id
  • id_or_key lookups → skill_id only
  • Mention path field (e.g., Default/Skills)

C3. Drop doc field entirely

doc does not exist in any skill schema. All known-caveats.md text about "unstamped skill.doc" is fabricated. Body lives in instructions. Either delete the doc-related half of the migration caveat, or replace with confirmed empty-version behavior only.

C4. Fix false filter claims on list_skills

GET /v2/skills accepts only limit, starting_after, ending_before. No project, tags, q, name filter. Same for MCP list_skills. Rewrite SKILL.md:104-107 and Phase 1: filtering by project/tags/name must happen client-side after paginating, or via search_entities.

C5. Fix "verified" REST claim

SKILL.md:80 says "verified against /openapi/openapi.json". Endpoints real, but field names cited around it are wrong. Drop "verified" wording or re-verify and list correct request schemas inline. Add missing endpoint: POST /v2/skills:checkDisplayNameAvailability — replaces the "scan list_skills for name conflicts" client-side hack.

C6. Fix doc URLs

Replace fabricated/wrong paths:

  • docs.orq.ai/docs/skills/overview → ✅ docs.orq.ai/docs/prompt-snippets/overview
  • docs.orq.ai/reference/skills → drop, or link to confirmed API reference page
  • docs.orq.ai/docs/agents/skills → ✅ docs.orq.ai/docs/agents/build#skills or docs.orq.ai/docs/agents/agent-studio#skills
  • Add docs.orq.ai/docs/integrations/code-assistants/skills for disambiguation context

C7. Add disambiguation section

Top of SKILL.md: explicit "Skill on orq.ai (platform entity, this skill) vs. Orq Skills (code-assistant workflows, this repo)". Mention {{skill.key}} static-reference syntax — primary doc usage pattern, currently absent.

C8. Verify or remove agent-wiring pseudo-code

governance-guide.md:18-23 admits skills[] entry shape is unknown. Resolve before merge: run get_agent on a real agent with attached skills, lock the shape, update example. Otherwise the orphan-cleanup loop is speculative.


Suggestions

  • S1. Naming guidance — recommend, don't mandate. Authoring guide enforces "kebab-case lowercase ≤50 chars" but API allows mixed case + underscores, ≤255. Frame as repo convention or align to API regex ^[A-Za-z0-9]+(?:[_-][A-Za-z0-9]+)*$.
  • S2. Use checkDisplayNameAvailability in Phase 3. Replaces fragile list_skills scan.
  • S3. Mention {{skill.key}} injection. Primary platform feature; currently absent.
  • S4. Verify allowed-tools: orq* glob. Check Claude Code, Cursor, Codex parsers all support glob. If not, expand explicitly.
  • S5. Tighten optimize-prompt cross-refs. It operates on system prompts, not skill instructions. Confirm scope match or drop routing recommendation.
  • S6. Verify search_entities returns skills[] in list payload. If not, document get_agent fanout for orphan detection.
  • S7. Add path field guidance. Required for finder placement; default by project scope.
  • S8. Update tests after fixes. tests/skills.md:148-178 must assert real field names, no ticket IDs, correct filter expectations.
  • S9. Soften version semantics. update_skill does not accept version parameter — stamped server-side. Remove SKILL.md:151 patch/minor/major prompt.
  • S10. Document pagination. Cursor-based, default 10, max 200. Iteration pattern needed in Phase 1.
  • S11. CHANGELOG + 4-manifest version bump. Per CLAUDE.md, all 4 plugin.json files must bump for new skill. Confirm tests/scripts/validate-plugin-manifests.sh passes.
  • S12. Lock project-scope mechanics. project_id is optional → omit = workspace-wide. Pass real id for project-scoped.

Strengths

  • Two-step consent for delete + orphan cleanup is the right safety posture.
  • Phase split (list/get/create/update/delete) maps cleanly to user intent.
  • "Description is for retrieval, not marketing" guidance correct per docs.
  • Test file added with concrete scenarios.

🤖 Review generated with Claude Code

…ma, disambiguate

- Remove all internal ticket references (Linear IDs/URLs) from SKILL.md,
  resources/, tests/, AGENTS.md, and CHANGELOG.md. Replace with
  behavior-only descriptions.
- Switch to real /v2/skills field names: display_name, instructions,
  project_id, skill_id, path. Drop name / body / doc / id_or_key
  throughout.
- Drop the doc-field caveat entirely (the field does not exist in the
  Skill schema). Empty-version-on-migration caveat retained.
- Document GET /v2/skills cursor pagination (limit / starting_after /
  ending_before). Push project_id / tags / display_name filtering to
  the client. Add explicit pagination loop pseudocode.
- Add POST /v2/skills:checkDisplayNameAvailability for pre-create
  uniqueness checks; demote list_skills scan to fallback.
- Replace fabricated/wrong doc URLs:
  * docs/skills/overview -> docs/prompt-snippets/overview
  * drop /reference/skills
  * docs/agents/skills -> docs/agents/build#skills
  * add docs/integrations/code-assistants/skills for disambiguation
- Add top-of-SKILL.md disambiguation: platform Skill entity vs. this
  repo's code-assistant Orq Skills vs. the Agent Skills standard.
- Document the {{skill.<key>}} static-template inlining path alongside
  agent.skills[] runtime selection.
- Replace speculative agent-wiring snippet with a "mirror what
  get_agent returned" pattern that handles both string-id and object
  entries; flag the schema-version uncertainty explicitly.
- Soften display_name guidance — recommend the repo convention,
  reference the API regex, do not enforce.
- Add path field guidance to authoring guide.
- Reframe optimize-prompt as heuristics-to-reuse (Skill instructions
  are typically shorter / more capability-scoped than a system prompt).
- Replace allowed-tools glob with explicit MCP tool names.
- Note version is server-side only (do not pass in update_skill).
- Tests/skills.md updated to assert real field names and behaviors.
…ntax

Verified against orquesta-web apps/platform-api/skills (Go connect-rpc service),
apps/orq-mcp/src/tools/skills.tools.ts (MCP tool definitions), and
.openapi/fragments/platform-api/skills.json. Discovered several invented
endpoints/fields in the prior commit and a fictional consumption model.

Factual corrections:

- Drop {{skill.<key>}} template syntax. The real syntax is
  {{snippet.<display_name>}} -- the Snippets->Skills rename kept the legacy
  "snippet." prefix for backwards compatibility. There is no
  {{skill.<...>}} placeholder anywhere in libs/go/template-parser or the
  Studio UI.

- Drop the entire agent.skills[] orphan-cleanup workflow. Verified that
  agent.skills[] is the AI-generated A2A AgentCardSkill[] field
  (libs/models/agents/src/utils/index.ts -- generateAgentSkills), NOT a
  list of platform Skill references. Deleting a platform Skill does not
  orphan anything in agent.skills[]. The whole Phase 5 update_agent
  fan-out targeted a non-existent relationship.

- Replace Phase 5 with a reference scan: paginate search_entities, fetch
  each candidate's body with get_deployment / get_agent / get_skill, and
  substring-match {{snippet.<display_name>}} (case-sensitive) to find
  consumers before delete. Default to enabled: false (soft disable) when
  references are found.

- Drop POST /v2/skills:checkDisplayNameAvailability. No such endpoint
  exists on the SkillsService (verified all 5 methods in
  apps/platform-api/skills/connect_routes.go: CreateSkill, ListSkills,
  GetSkill, UpdateSkill, DeleteSkill). Replaced with: call create_skill
  and handle the AlreadyExists error.

- Drop the version field. The Skill openapi schema has no version
  property. Versioning is recorded as separate activity-log entries
  (recordVersionActivity in connect_routes.go), not as a field. Drop the
  empty-version migration caveat too -- no field to be empty.

- Add the enabled boolean field. It is on the real schema and surfaces
  the soft-disable lever. Now documented in the field reference, in
  Phase 4, and as a default-first-step alternative to delete in Phase 5.

- Add display_name rename warning to Phase 4. Renaming silently breaks
  every {{snippet.<old-name>}} reference -- same failure mode as delete.
  Same reference scan applies.

- Replace docs.orq.ai/docs/agents/build#skills (no such page) with
  docs.orq.ai/docs/agents/agent-studio (where the snippet/skill section
  actually lives).

Code hygiene:

- commands/manage-skills.md: replace allowed-tools: orq* glob with
  explicit MCP tool names; add disable as an action; drop update_agent
  references.
- Drop the "Why these constraints" generic justification paragraph.
- Add Scenario 5 to tests/skills.md covering AlreadyExists handling and
  the disable-vs-delete decision.
- Add error-handling guidance for create_skill failures (AlreadyExists,
  invalid project/path).
- /orq:quickstart reference updated to mention it is the Claude Code
  invocation; other assistants run the equivalent onboarding flow.
…g caveat

Verified end-to-end against orquesta-web that two soft claims in the
prior commit needed harder hedges:

1. The /v2/skills CRUD API and the *_skill MCP tools live only on the
   feature branch origin/thedevtoni/snippets-to-skills (a677849318,
   19c26518f4, eedf87c1ca, c13f227567). They are NOT on
   origin/main yet. A workspace pinned to a release without that branch
   merged will not have list_skills / create_skill / etc. Added a
   preflight check at the top of Prerequisites that probes list_skills
   once and falls back to managing the entity under its legacy name
   (Prompt Snippet via /v2/prompts/snippets) when the new tools are
   missing.

2. The {{snippet.<display_name>}} renderer reads from the
   PROMPT_SNIPPETS_KV Redis cache (libs/go/response-executor/snippets.go,
   libs/platform/prompts/prompts-manager/src/lib/prompts-manager.ts).
   That cache is populated only by the legacy
   apps/workspaces-api/src/handlers/prompts-snippets/* handlers. The new
   apps/platform-api/skills/connect_routes.go writes to MongoDB, records
   a semantic-version activity, and publishes a NATS entity event -- but
   nothing in the codebase consumes that event to update the snippet KV.
   The only NATS consumer (apps/responses-api/consumers/entity_event_consumer.go)
   handles agent.deleted only. So a Skill created via the new API may
   not be reachable via {{snippet.<display_name>}} until a bridge lands.
   Added a "Renderer wiring lag" caveat in known-caveats.md plus a
   test-render verification step, and softened the enabled-field row in
   the SKILL.md field reference to flag the same gap.

Both findings come from tracing the resolver code; if the wiring lands
later (NATS subscriber that mirrors skill.* events into the KV, or the
resolver moved to read MongoDB directly), the caveats can be removed.
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