Make skills spec-compliant

[ScriptSmith]

No description provided.

[greptile-apps]

Greptile Summary

This PR replaces the agentskills.io-spec skill model with an OpenAI-compatible, immutable versioned design: a skills row holds identity and version pointers; each skill_versions row is immutable; content changes are published as new versions via POST /v1/skills/{id}/versions. The admin /admin/skills surface is removed in favor of the new /v1/skills API.

• Backend: New src/routes/api/skills.rs with full CRUD + versioning; skill_zip service for zip pack/unpack with zip-bomb protection; SkillRef + VersionSelector types enabling name-based and pinned-version resolution in the responses pipeline; Postgres/SQLite FOR UPDATE / BEGIN IMMEDIATE transactions to prevent concurrent default-pointer corruption.
• Frontend: Slash-command skill invocation now seeds SKILL.md directly into the request (deterministic load) instead of nudging the model via a text prefix; useUserSkills consolidated to a single /v1/skills call; SkillFormModal edit path publishes a new version and re-fetches the skill for onSaved.

Confidence Score: 4/5

The versioning logic and security controls are solid, but the in-place migration mutation will break sqlx migrate run on any database that already applied the old schema.

The core redesign is well-engineered with FOR UPDATE/BEGIN IMMEDIATE guards, zip-bomb protection, and correct org-scoping. The one deployment concern is the modified initial migration — sqlx checksums applied migrations, so existing databases will hit a hard failure.

Both migrations_sqlx/postgres/20250101000000_initial.sql and migrations_sqlx/sqlite/20250101000000_initial.sql need attention — a new additive migration file is the safe path for existing databases.

Important Files Changed

Filename	Overview
src/routes/api/skills.rs	New file: OpenAI-compatible /v1/skills endpoints. JSON upload path now correctly enforces max_files via check_file_count. Version limit check is non-atomic with create_version.
migrations_sqlx/postgres/20250101000000_initial.sql	Existing initial migration modified in-place to add skill_versions/skill_version_files and remove skill_files. Will cause sqlx checksum mismatch failures on existing databases.
migrations_sqlx/sqlite/20250101000000_initial.sql	Same in-place migration modification for SQLite. Same checksum mismatch risk as the Postgres migration.
src/db/postgres/skills.rs	Rewritten to support versioned skill_versions/skill_version_files schema. set_default_version and delete_version use FOR UPDATE transactions to prevent concurrent pointer corruption.
src/db/sqlite/skills.rs	Rewritten for versioned schema with BEGIN IMMEDIATE transactions for SQLite's lack of row-level locks.
src/services/skill_zip.rs	New zip pack/unpack service with zip-bomb protection (actual bytes counted), path traversal rejection, and common-dir stripping. Well-tested.
src/services/responses_pipeline.rs	Skill resolution now supports Default/Latest/Exact version selectors and SkillRef::Name, with proper org-scoping maintained.
ui/src/pages/chat/useChat.ts	Slash-invoked skills now seed SKILL.md directly into the request. Silent fallback when loadSkillSeed returns null leaves user without feedback.
ui/src/components/Admin/SkillFormModal/SkillFormModal.tsx	Update path now publishes a new version and re-fetches the skill for onSaved, addressing the stale-skill issue from the previous review.
src/models/skill.rs	Skill model refactored to a projection of its default version. Added SkillVersion, SkillRef, VersionSelector, and CreateSkillVersion. UpdateSkill removed.

Entity Relationship Diagram

%%{init: {'theme': 'neutral'}}%%
erDiagram
    skills {
        UUID id PK
        skill_owner_type owner_type
        UUID owner_id
        VARCHAR name
        BIGINT default_version_seq
        BIGINT latest_version_seq
        BIGINT next_version_seq
    }
    skill_versions {
        UUID id PK
        UUID skill_id FK
        BIGINT version_seq
        VARCHAR name
        VARCHAR description
        BIGINT total_bytes
    }
    skill_version_files {
        UUID skill_version_id FK
        VARCHAR path
        TEXT content
        BIGINT byte_size
    }
    skills ||--o{ skill_versions : "has versions"
    skill_versions ||--o{ skill_version_files : "contains files"
    skills }o--|| skill_versions : "default_version_seq"
    skills }o--|| skill_versions : "latest_version_seq"

Loading

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
migrations_sqlx/postgres/20250101000000_initial.sql:1202
**In-place mutation of an already-applied migration breaks existing databases**

`sqlx` records the SHA256 checksum of every applied migration in `_sqlx_migrations`. Because `20250101000000_initial.sql` has been modified rather than a new migration added, any database that already ran the old version will fail with `"migration was previously applied but its source has changed"` on the next `sqlx migrate run`. This affects every developer's local database, staging, and any CI pipeline that doesn't start from a clean schema. A new migration file (e.g. `20250530000000_skills_versioned.sql`) with `ALTER TABLE`, `CREATE TABLE`, and a data backfill is needed for existing deployments. The same applies to `migrations_sqlx/sqlite/20250101000000_initial.sql`.

### Issue 2 of 3
ui/src/pages/chat/useChat.ts:2360-2372
**Skill seed silently dropped on fetch failure**

`clearPendingSkill()` is called before `loadSkillSeed`, so if the fetch fails (network error, 404, or a cached skill missing its `files` array) and `seed` is `null`, the message is sent without the skill's `SKILL.md` injected and without any user-visible feedback. From the user's perspective they invoked a skill but the model responds as if it wasn't there. Consider toasting an error or, at minimum, restoring `pendingSkillId` on failure so the user can retry.

### Issue 3 of 3
src/routes/api/skills.rs:1324-1342
**Non-atomic version-count enforcement allows limit overshoot under concurrency**

`count_versions` and `create_version` are two separate DB round-trips with no transaction or lock between them. Two concurrent `POST /v1/skills/{id}/versions` requests can both observe `count < max` and both proceed to insert, leaving the skill one or more versions over `max_skill_versions_per_skill`. Since the default limit is 0 (unlimited), this only matters when an operator explicitly configures the cap — but once they do, the limit is not reliably enforced. Enforcing the cap inside the `create_version` DB transaction (e.g. a `SELECT COUNT … FOR UPDATE` before the insert) would close the gap.

_{Reviews (5): Last reviewed commit: "Seed slash-invoked skills directly into ..." | Re-trigger Greptile}

[ScriptSmith]

@greptile-apps

[ScriptSmith]

@greptile-apps

[greptile-apps]

In-place mutation of an already-applied migration breaks existing databases

sqlx records the SHA256 checksum of every applied migration in _sqlx_migrations. Because 20250101000000_initial.sql has been modified rather than a new migration added, any database that already ran the old version will fail with "migration was previously applied but its source has changed" on the next sqlx migrate run. This affects every developer's local database, staging, and any CI pipeline that doesn't start from a clean schema. A new migration file (e.g. 20250530000000_skills_versioned.sql) with ALTER TABLE, CREATE TABLE, and a data backfill is needed for existing deployments. The same applies to migrations_sqlx/sqlite/20250101000000_initial.sql.

Prompt To Fix With AI

This is a comment left during a code review.
Path: migrations_sqlx/postgres/20250101000000_initial.sql
Line: 1202

Comment:
**In-place mutation of an already-applied migration breaks existing databases**

`sqlx` records the SHA256 checksum of every applied migration in `_sqlx_migrations`. Because `20250101000000_initial.sql` has been modified rather than a new migration added, any database that already ran the old version will fail with `"migration was previously applied but its source has changed"` on the next `sqlx migrate run`. This affects every developer's local database, staging, and any CI pipeline that doesn't start from a clean schema. A new migration file (e.g. `20250530000000_skills_versioned.sql`) with `ALTER TABLE`, `CREATE TABLE`, and a data backfill is needed for existing deployments. The same applies to `migrations_sqlx/sqlite/20250101000000_initial.sql`.

How can I resolve this? If you propose a fix, please make it concise.