refactor(skills): drop company_skills.markdown column, read from disk via IPC
`company_skills.markdown` stored the full SKILL.md body. The on-disk
file under `profile/skills/{companyId}/{slug}/SKILL.md` is what pi-
coding-agent actually reads via `loadSkills` at runtime. The DB column
was a duplicate — import wrote to both, renderer read from DB. Any
discrepancy between disk and DB was invisible until the agent behaved
differently from what the user saw in SkillDetail.
Disk becomes the single source of truth. DB keeps name / description /
metadata / source info / file inventory — the cheap lookup row. UI reads
SKILL.md content live via a new IPC channel.
## Migration v37 (rebuild company_skills without markdown)
Table-rebuild pattern matching v35/v36: `fn:` form, PRAGMA walk for
drift columns, row-count assertion before DROP, `defer_foreign_keys`
for the `agent_skill_assignments(skill_id)` reference. Indexes
recreated. Idempotent via `schema_migrations` ledger.
Existing DB rows' markdown content is discarded — but the on-disk
SKILL.md files are the authoritative copy, so no user-visible data
loss. Any agent using those skills keeps working; the UI just starts
reading from disk instead of the DB cache.
## Code changes
- `src/main/database/skills.ts`: drop `markdown` from read mapping,
upsert input type, INSERT column/bind list, ON CONFLICT DO UPDATE.
- `src/shared/types/records-core.ts`: `SkillRecord.markdown` removed.
- `src/main/skills/import.ts` + `import-github.ts`: drop `markdown:`
from 4 `saveCompanySkill` call sites. Disk-write via
`materializeSkillFiles` unchanged (it's what the file agent reads).
- `src/main/skills/index.ts`: expose `skillFilesDir` for the new IPC
handler to resolve `${profile}/skills/${companyId}/${slug}/SKILL.md`.
- `src/shared/contracts/channels.ts` + `schemas-execution.ts` +
`desktop-api/skills.ts`: new `getSkillMarkdown` channel
`{ companyId, slug } → DesktopResult<string>`.
- `src/preload/index.ts`: expose `getSkillMarkdown(companyId, slug)`.
- `src/main/ipc-skill-handlers.ts`: handler validates schema, ensures
company, resolves path, `readFile`, returns content. Fails closed
with `SKILL_NOT_FOUND` on ENOENT, `SKILL_READ_FAILED` on other
read errors.
- `src/renderer/components/skills/SkillDetail.tsx`: `useEffect` fetches
markdown via IPC when the skill detail loads; shows "Loading…" until
resolved, surfaces read errors. Preserves `stripFrontmatter`.
- Tests: drop `markdown:` from 2 fixture sites
(`skill-types.test.ts`, `skill-aware-assignment.spec.ts`).
## Verification
- typecheck / lint / 578 tests green
- `grep 'skill\.markdown' src/` → 0 DB-record reads
- `grep '\.markdown\b' src/main/database/skills.ts` → 0
- Remaining `.markdown` references are all `ghSkill.markdown` (GitHub
fetcher's intermediate struct feeding `materializeSkillFiles` to
disk) and parameter names in `stripFrontmatter(markdown: string)` /
fs-writer helpers — none read from a DB row.
This commit is contained in:
1 parent
7e1d4199ee
commit
f012f8e880
15 files changed
+155
-16
No files matched your search
@@ -739,6 +739,96 @@ export function runMigrations(
|
||||
);
|
||||
},
|
||||
},
|
||||
{
|
||||
// Drop the `markdown` column from company_skills. SKILL.md on disk
|
||||
// (profile/skills/{companyId}/{slug}/SKILL.md) is the authoritative
|
||||
// source of truth read by pi-coding-agent's loadSkills. The DB column
|
||||
// duplicated that content. UI now loads the body live via IPC.
|
||||
//
|
||||
// SQLite can't DROP COLUMN cleanly pre-3.35 and even the supported path
|
||||
// is implemented as an invisible rebuild, so we do an explicit
|
||||
// drift-safe rebuild mirroring v35/v36:
|
||||
// 1. Snapshot row count
|
||||
// 2. PRAGMA table_info to discover the live column set
|
||||
// 3. CREATE company_skills_new without `markdown`
|
||||
// 4. Copy every kept column
|
||||
// 5. Assert copied count matches
|
||||
// 6. DROP old, RENAME new, recreate indexes
|
||||
//
|
||||
// FK from agent_skill_assignments(skill_id) -> company_skills(id) must
|
||||
// survive. PRAGMA defer_foreign_keys = ON keeps checks deferred until
|
||||
// transaction commit (by which time RENAME has restored the table).
|
||||
version: 37, name: "drop_company_skills_markdown",
|
||||
fn: () => {
|
||||
if (!tableExists(db, "company_skills")) {
|
||||
log.info("[migration v37] company_skills table absent; skipping");
|
||||
return;
|
||||
}
|
||||
if (!columnExists(db, "company_skills", "markdown")) {
|
||||
log.info("[migration v37] company_skills.markdown already absent; skipping");
|
||||
return;
|
||||
}
|
||||
|
||||
const beforeCount = (db
|
||||
.prepare("SELECT COUNT(*) as cnt FROM company_skills")
|
||||
.get() as { cnt: number }).cnt;
|
||||
|
||||
const existingCols = (db
|
||||
.prepare("PRAGMA table_info(company_skills)")
|
||||
.all() as Array<{ name: string; type: string; notnull: number; dflt_value: unknown; pk: number }>);
|
||||
const keepCols = existingCols.filter((c) => c.name !== "markdown");
|
||||
|
||||
db.exec("PRAGMA defer_foreign_keys = ON");
|
||||
|
||||
// Declared v14 column set (minus markdown). Drift columns preserved
|
||||
// verbatim from PRAGMA metadata.
|
||||
const declaredCols = new Map<string, string>([
|
||||
["id", "TEXT PRIMARY KEY"],
|
||||
["company_id", "TEXT NOT NULL"],
|
||||
["key", "TEXT NOT NULL"],
|
||||
["slug", "TEXT NOT NULL"],
|
||||
["name", "TEXT NOT NULL"],
|
||||
["description", "TEXT"],
|
||||
["source_type", "TEXT NOT NULL DEFAULT 'local_path'"],
|
||||
["source_locator", "TEXT"],
|
||||
["source_ref", "TEXT"],
|
||||
["trust_level", "TEXT NOT NULL DEFAULT 'markdown_only'"],
|
||||
["compatibility", "TEXT NOT NULL DEFAULT 'compatible'"],
|
||||
["file_inventory", "TEXT NOT NULL DEFAULT '[]'"],
|
||||
["metadata", "TEXT"],
|
||||
["created_at", "TEXT NOT NULL"],
|
||||
["updated_at", "TEXT NOT NULL"],
|
||||
]);
|
||||
|
||||
const ddl = keepCols.map((c) => {
|
||||
const declared = declaredCols.get(c.name);
|
||||
if (declared) return `${c.name} ${declared}`;
|
||||
return `${c.name} ${c.type || "TEXT"}`;
|
||||
});
|
||||
|
||||
db.exec(`CREATE TABLE company_skills_new (${ddl.join(", ")})`);
|
||||
|
||||
const copyCols = keepCols.map((c) => c.name).join(", ");
|
||||
db.exec(`INSERT INTO company_skills_new (${copyCols}) SELECT ${copyCols} FROM company_skills`);
|
||||
|
||||
const afterCount = (db
|
||||
.prepare("SELECT COUNT(*) as cnt FROM company_skills_new")
|
||||
.get() as { cnt: number }).cnt;
|
||||
if (afterCount !== beforeCount) {
|
||||
throw new Error(
|
||||
`company_skills copy row count mismatch: before=${beforeCount} after=${afterCount}; aborting before DROP`,
|
||||
);
|
||||
}
|
||||
|
||||
db.exec("DROP TABLE company_skills");
|
||||
db.exec("ALTER TABLE company_skills_new RENAME TO company_skills");
|
||||
|
||||
db.exec("CREATE UNIQUE INDEX IF NOT EXISTS idx_company_skills_company_key ON company_skills(company_id, key)");
|
||||
db.exec("CREATE INDEX IF NOT EXISTS idx_company_skills_company_name ON company_skills(company_id, name)");
|
||||
|
||||
log.info(`[migration v37] dropped company_skills.markdown; preserved ${afterCount} row(s)`);
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
for (const migration of migrations) {
|
||||
|
||||
@@ -15,7 +15,6 @@ export function buildSkillMethods(
|
||||
slug: row.slug,
|
||||
name: row.name,
|
||||
description: row.description ?? null,
|
||||
markdown: row.markdown ?? "",
|
||||
sourceType: (row.source_type ?? "local_path") as SkillSourceType,
|
||||
sourceLocator: row.source_locator ?? null,
|
||||
sourceRef: row.source_ref ?? null,
|
||||
@@ -31,7 +30,7 @@ export function buildSkillMethods(
|
||||
return {
|
||||
upsertCompanySkill(skill: {
|
||||
id: string; companyId: string; key: string; slug: string; name: string;
|
||||
description: string | null; markdown: string; sourceType: string;
|
||||
description: string | null; sourceType: string;
|
||||
sourceLocator: string | null; sourceRef: string | null; trustLevel: string;
|
||||
compatibility: string; fileInventory: string; metadata: string | null;
|
||||
createdAt: string; updatedAt: string;
|
||||
@@ -43,15 +42,14 @@ export function buildSkillMethods(
|
||||
|
||||
getDb().prepare(`
|
||||
INSERT INTO company_skills (
|
||||
id, company_id, key, slug, name, description, markdown,
|
||||
id, company_id, key, slug, name, description,
|
||||
source_type, source_locator, source_ref, trust_level,
|
||||
compatibility, file_inventory, metadata, created_at, updated_at
|
||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
ON CONFLICT(company_id, key) DO UPDATE SET
|
||||
slug = excluded.slug,
|
||||
name = excluded.name,
|
||||
description = excluded.description,
|
||||
markdown = excluded.markdown,
|
||||
source_ref = excluded.source_ref,
|
||||
trust_level = excluded.trust_level,
|
||||
file_inventory = excluded.file_inventory,
|
||||
@@ -59,7 +57,7 @@ export function buildSkillMethods(
|
||||
updated_at = excluded.updated_at
|
||||
`).run(
|
||||
skill.id, skill.companyId, skill.key, skill.slug,
|
||||
skill.name, skill.description, skill.markdown,
|
||||
skill.name, skill.description,
|
||||
skill.sourceType, skill.sourceLocator, skill.sourceRef,
|
||||
skill.trustLevel, skill.compatibility, skill.fileInventory,
|
||||
skill.metadata, skill.createdAt, skill.updatedAt,
|
||||
|
||||
Reference in new issue
Block a user