Repository navigation
Make saved skills name-addressable and upsert by name #119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
29c49a6
83eed1c
13fd468
987d70d
a25714f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| ALTER TABLE mcp_skills ADD COLUMN name TEXT; | ||
|
|
||
| WITH base_names AS ( | ||
| SELECT | ||
| id, | ||
| user_id, | ||
| lower(replace(trim(title), ' ', '-')) AS base_name | ||
| FROM mcp_skills | ||
| WHERE name IS NULL | ||
| ), | ||
| suffixes AS ( | ||
| SELECT | ||
| user_id, | ||
| base_name, | ||
| rtrim(base_name, '0123456789') AS trimmed_base, | ||
| CASE | ||
| WHEN rtrim(base_name, '0123456789') <> base_name | ||
| AND length(rtrim(base_name, '0123456789')) > 1 | ||
| AND substr(rtrim(base_name, '0123456789'), -1) = '-' | ||
| THEN substr( | ||
| rtrim(base_name, '0123456789'), | ||
| 1, | ||
| length(rtrim(base_name, '0123456789')) - 1 | ||
| ) | ||
| ELSE NULL | ||
| END AS prefix, | ||
| CASE | ||
| WHEN rtrim(base_name, '0123456789') <> base_name | ||
| AND length(rtrim(base_name, '0123456789')) > 1 | ||
| AND substr(rtrim(base_name, '0123456789'), -1) = '-' | ||
| THEN CAST( | ||
| substr(base_name, length(rtrim(base_name, '0123456789')) + 1) AS INTEGER | ||
| ) | ||
| ELSE NULL | ||
| END AS suffix_number | ||
| FROM base_names | ||
| ), | ||
| max_suffixes AS ( | ||
| SELECT | ||
| user_id, | ||
| prefix AS base_name, | ||
| MAX(suffix_number) AS max_suffix | ||
| FROM suffixes | ||
| WHERE prefix IS NOT NULL AND suffix_number IS NOT NULL | ||
| GROUP BY user_id, prefix | ||
| ), | ||
| ranked AS ( | ||
| SELECT | ||
| id, | ||
| user_id, | ||
| base_name, | ||
| COUNT(*) OVER (PARTITION BY user_id, base_name) AS name_count, | ||
| ROW_NUMBER() OVER (PARTITION BY user_id, base_name ORDER BY id) AS name_index | ||
| FROM base_names | ||
| ) | ||
| UPDATE mcp_skills | ||
| SET name = ( | ||
| SELECT | ||
| CASE | ||
| WHEN ranked.name_count = 1 OR ranked.name_index = 1 THEN ranked.base_name | ||
| ELSE ranked.base_name || '-' || (COALESCE(max_suffixes.max_suffix, 1) + ranked.name_index - 1) | ||
| END | ||
| FROM ranked | ||
| LEFT JOIN max_suffixes | ||
| ON max_suffixes.user_id = ranked.user_id | ||
| AND max_suffixes.base_name = ranked.base_name | ||
| WHERE ranked.id = mcp_skills.id | ||
| ) | ||
| WHERE name IS NULL; | ||
|
cursor[bot] marked this conversation as resolved.
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| CREATE TABLE IF NOT EXISTS mcp_skills_next ( | ||
| id TEXT PRIMARY KEY NOT NULL, | ||
| user_id TEXT NOT NULL, | ||
| title TEXT NOT NULL, | ||
| description TEXT NOT NULL, | ||
| keywords TEXT NOT NULL, | ||
| code TEXT NOT NULL, | ||
| search_text TEXT, | ||
| uses_capabilities TEXT, | ||
| inferred_capabilities TEXT NOT NULL, | ||
| inference_partial INTEGER NOT NULL DEFAULT 0, | ||
| read_only INTEGER NOT NULL, | ||
| idempotent INTEGER NOT NULL, | ||
| destructive INTEGER NOT NULL, | ||
| created_at TEXT NOT NULL DEFAULT (CURRENT_TIMESTAMP), | ||
| updated_at TEXT NOT NULL DEFAULT (CURRENT_TIMESTAMP), | ||
| parameters TEXT, | ||
| collection_name TEXT, | ||
| collection_slug TEXT, | ||
| name TEXT NOT NULL | ||
| ); | ||
|
|
||
| INSERT INTO mcp_skills_next ( | ||
| id, user_id, title, description, keywords, code, search_text, | ||
| uses_capabilities, inferred_capabilities, inference_partial, read_only, | ||
| idempotent, destructive, created_at, updated_at, parameters, | ||
| collection_name, collection_slug, name | ||
| ) | ||
| SELECT | ||
| id, user_id, title, description, keywords, code, search_text, | ||
| uses_capabilities, inferred_capabilities, inference_partial, read_only, | ||
| idempotent, destructive, created_at, updated_at, parameters, | ||
| collection_name, collection_slug, name | ||
| FROM mcp_skills; | ||
|
|
||
| DROP TABLE mcp_skills; | ||
| ALTER TABLE mcp_skills_next RENAME TO mcp_skills; | ||
|
|
||
| CREATE INDEX IF NOT EXISTS idx_mcp_skills_user_id ON mcp_skills(user_id); | ||
| CREATE INDEX IF NOT EXISTS idx_mcp_skills_user_collection_slug | ||
| ON mcp_skills(user_id, collection_slug); | ||
| CREATE UNIQUE INDEX IF NOT EXISTS idx_mcp_skills_user_name | ||
| ON mcp_skills(user_id, name); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -213,4 +213,3 @@ export async function fetchNegotiatedThenMaybeBrowserRender( | |
| }, | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,10 @@ import { z } from 'zod' | |
| import { defineDomainCapability } from '#mcp/capabilities/define-domain-capability.ts' | ||
| import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' | ||
| import { type CapabilityContext } from '#mcp/capabilities/types.ts' | ||
| import { deleteMcpSkill } from '#mcp/skills/mcp-skills-repo.ts' | ||
| import { | ||
| deleteMcpSkill, | ||
| getMcpSkillByNameInput, | ||
| } from '#mcp/skills/mcp-skills-repo.ts' | ||
|
Comment on lines
+5
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Delete should use exact-name resolution only.
🔒 Safer lookup for delete import {
deleteMcpSkill,
- getMcpSkillByNameInput,
+ getMcpSkillByName,
} from '#mcp/skills/mcp-skills-repo.ts'
@@
- const existing = await getMcpSkillByNameInput(
+ const existing = await getMcpSkillByName(
ctx.env.APP_DB,
user.userId,
args.name,
)Also applies to: 31-35 🤖 Prompt for AI Agents |
||
| import { deleteSkillVector } from '#mcp/skills/skill-vectorize.ts' | ||
| import { requireMcpUser } from './require-user.ts' | ||
|
|
||
|
|
@@ -20,21 +23,26 @@ export const metaDeleteSkillCapability = defineDomainCapability( | |
| idempotent: true, | ||
| destructive: true, | ||
| inputSchema: z.object({ | ||
| skill_id: z | ||
| .string() | ||
| .min(1) | ||
| .describe('Skill id returned by meta_save_skill.'), | ||
| name: z.string().min(1).describe('Unique lower-kebab-case skill name.'), | ||
| }), | ||
| outputSchema, | ||
| async handler(args, ctx: CapabilityContext) { | ||
| const user = requireMcpUser(ctx.callerContext) | ||
| const existing = await getMcpSkillByNameInput( | ||
| ctx.env.APP_DB, | ||
| user.userId, | ||
| args.name, | ||
| ) | ||
| if (!existing) { | ||
| return { deleted: false } | ||
| } | ||
| const removed = await deleteMcpSkill( | ||
| ctx.env.APP_DB, | ||
| user.userId, | ||
| args.skill_id, | ||
| existing.name, | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| ) | ||
| if (removed) { | ||
| await deleteSkillVector(ctx.env, args.skill_id) | ||
| await deleteSkillVector(ctx.env, existing.id) | ||
| } | ||
| return { deleted: removed } | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Backfill formula doesn't match
normalizeSkillName, causing lookup failures for skills with special characters.The SQL backfill uses
lower(replace(trim(title), ' ', '-')), but the application'snormalizeSkillName(inskill-names.ts) applies significantly more transformations:For any skill with accents, punctuation, or special characters, the backfilled
namewill not match whatnormalizeSkillNameproduces, causing lookup failures inmeta_run_skill,meta_get_skill, andmeta_delete_skill.Example:
"Résumé Builder!""résumé-builder!"normalizeSkillName:"resume-builder"Proposed fix: Use a backfill that mimics normalizeSkillName more closely
SQLite lacks full Unicode normalization, so consider a two-phase approach:
Option 1: Fix in migration with closer approximation (won't handle all edge cases but handles common ones):
Option 2 (recommended): Run a TypeScript migration script post-deploy that re-normalizes all skill names using the actual
normalizeSkillNamefunction to ensure consistency.🤖 Prompt for AI Agents