Skip to content

Commit f64da5d

Browse files
author
Tehan
committed
fix(skill-memory): renumber migrations v51/52/53 after upstream v0.31 v50 collision
Upstream v0.31.0 added its own migration v50 (ctx-wrapup durable marker), colliding with skill-memory's v50/51/52. Renumbered skill migrations to v51 (skill_memory table) / v52 (embeddings+FTS) / v53 (historian extraction), bumped LATEST_SUPPORTED_VERSION to 53, and rotated the migration test files (v42/v51/v52 -> v51/v52/v53) with corrected internal version refs + fence assertions.
1 parent 471135c commit f64da5d

5 files changed

Lines changed: 345 additions & 345 deletions

File tree

‎packages/plugin/src/features/magic-context/migrations-v42.test.ts‎

Lines changed: 0 additions & 126 deletions
This file was deleted.

‎packages/plugin/src/features/magic-context/migrations-v51.test.ts‎

Lines changed: 86 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,113 +1,126 @@
11
import { describe, expect, test } from "bun:test";
22
import { Database } from "../../shared/sqlite";
33
import { closeQuietly } from "../../shared/sqlite-helpers";
4-
import { LATEST_MIGRATION_VERSION, MIGRATIONS, runMigrations } from "./migrations";
5-
import { initializeDatabase, LATEST_SUPPORTED_VERSION } from "./storage-db";
4+
import { LATEST_MIGRATION_VERSION, runMigrations } from "./migrations";
5+
import { initializeDatabase, LATEST_SUPPORTED_VERSION } from "./storage-db"; // ESM import (not require) — matches codebase pattern
66

77
function columnNames(db: Database, table: string): string[] {
88
return (db.prepare(`PRAGMA table_info(${table})`).all() as Array<{ name: string }>).map(
99
(c) => c.name,
1010
);
1111
}
12+
1213
function tableExists(db: Database, name: string): boolean {
1314
return Boolean(
1415
db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = ?").get(name),
1516
);
1617
}
1718

18-
describe("migration v51 — skill_memory embeddings + FTS", () => {
19-
test("fresh DB: delta_embedding column and skill_memory_fts exist, no throw", () => {
19+
describe("migration v51 — skill_memory table", () => {
20+
test("creates skill_memory table with correct columns on fresh DB, idempotently", () => {
2021
const db = new Database(":memory:");
2122
try {
2223
initializeDatabase(db);
2324
runMigrations(db);
24-
runMigrations(db); // idempotency
25+
runMigrations(db); // idempotency check
26+
27+
expect(tableExists(db, "skill_memory")).toBe(true);
28+
29+
const cols = columnNames(db, "skill_memory");
30+
expect(cols).toContain("id");
31+
expect(cols).toContain("skill_id");
32+
expect(cols).toContain("resolved_path");
33+
expect(cols).toContain("tier");
34+
expect(cols).toContain("skill_source");
35+
expect(cols).toContain("project_identity");
36+
expect(cols).toContain("intent");
37+
expect(cols).toContain("intent_embedding");
38+
expect(cols).toContain("embedding_model_version");
39+
expect(cols).toContain("kind");
40+
expect(cols).toContain("delta");
41+
expect(cols).toContain("tags");
42+
expect(cols).toContain("hit_count");
43+
expect(cols).toContain("pinned");
44+
expect(cols).toContain("normalized_hash");
45+
expect(cols).toContain("created_at");
46+
expect(cols).toContain("last_used_at");
2547

26-
expect(columnNames(db, "skill_memory")).toContain("delta_embedding");
27-
expect(tableExists(db, "skill_memory_fts")).toBe(true);
48+
expect(
49+
db
50+
.prepare("SELECT version FROM schema_migrations ORDER BY version DESC LIMIT 1")
51+
.get(),
52+
).toEqual({ version: LATEST_MIGRATION_VERSION });
2853
} finally {
2954
closeQuietly(db);
3055
}
3156
});
3257

33-
test("FTS triggers keep skill_memory_fts in sync with skill_memory", () => {
58+
test("skill_memory CHECK constraints reject invalid tier and kind values", () => {
3459
const db = new Database(":memory:");
3560
try {
3661
initializeDatabase(db);
3762
runMigrations(db);
38-
db.prepare(
39-
`INSERT INTO skill_memory
40-
(skill_id, resolved_path, tier, project_identity, intent, kind, delta, normalized_hash, hit_count, pinned, created_at)
41-
VALUES (?,?,?,?,?,?,?,?,0,0,?)`,
42-
).run(
43-
"s1",
44-
"/p/SKILL.md",
45-
"global",
46-
"git:abc",
47-
"fix a flaky auth test",
48-
"fix",
49-
"mock Date.now in auth tests",
50-
"h1",
51-
Date.now(),
52-
);
5363

54-
const hit = db
55-
.prepare(
56-
`SELECT m.id FROM skill_memory_fts f JOIN skill_memory m ON m.id = f.rowid
57-
WHERE skill_memory_fts MATCH ?`,
58-
)
59-
.get('"auth"');
60-
expect(hit).toBeTruthy();
61-
} finally {
62-
closeQuietly(db);
63-
}
64-
});
64+
const insert = db.prepare(`
65+
INSERT INTO skill_memory
66+
(skill_id, resolved_path, tier, project_identity, intent, kind, delta, normalized_hash, hit_count, pinned, created_at)
67+
VALUES (?, ?, ?, ?, ?, ?, ?, ?, 0, 0, ?)
68+
`);
6569

66-
// requires `import { MIGRATIONS } from "./migrations";` (added above)
67-
test("v51 migration backfills FTS for rows that pre-existed v40", () => {
68-
const db = new Database(":memory:");
69-
try {
70-
initializeDatabase(db);
71-
// Build a PRE-v40 schema: apply every migration BELOW v40 directly via up() (runMigrations has no
72-
// target-version param). skill_memory is created at v39; the FTS table + delta_embedding do NOT exist yet.
73-
for (const m of MIGRATIONS.filter((x) => x.version < 51).sort(
74-
(a, b) => a.version - b.version,
75-
)) {
76-
m.up(db);
77-
}
78-
// Insert a row under the pre-v51 schema — no FTS table yet, so no AFTER-INSERT trigger indexes it.
79-
db.prepare(
80-
`INSERT INTO skill_memory
81-
(skill_id, resolved_path, tier, project_identity, intent, kind, delta, normalized_hash, hit_count, pinned, created_at)
82-
VALUES (?,?,?,?,?,?,?,?,0,0,?)`,
83-
).run(
84-
"s2",
85-
"/p/SKILL.md",
86-
"global",
87-
"git:abc",
88-
"handle oauth refresh",
89-
"fix",
90-
"rotate the token early",
91-
"h2",
92-
Date.now(),
93-
);
94-
// Apply ONLY v40's up() — its body must ALTER + create the FTS table + BACKFILL the pre-existing row.
95-
const v51 = MIGRATIONS.find((m) => m.version === 51);
96-
if (!v51) throw new Error("v40 migration not found");
97-
v51.up(db);
98-
const hit = db
99-
.prepare(
100-
`SELECT m.id FROM skill_memory_fts f JOIN skill_memory m ON m.id = f.rowid WHERE skill_memory_fts MATCH ?`,
101-
)
102-
.get('"oauth"');
103-
expect(hit).toBeTruthy();
70+
// Valid row
71+
expect(() =>
72+
insert.run(
73+
"test-skill",
74+
"/path/SKILL.md",
75+
"project",
76+
"git:abc123",
77+
"test intent",
78+
"gotcha",
79+
"test delta",
80+
"hash1",
81+
Date.now(),
82+
),
83+
).not.toThrow();
84+
85+
// Invalid tier
86+
expect(() =>
87+
insert.run(
88+
"test-skill",
89+
"/path/SKILL.md",
90+
"invalid-tier",
91+
"git:abc123",
92+
"test intent",
93+
"gotcha",
94+
"test delta",
95+
"hash2",
96+
Date.now(),
97+
),
98+
).toThrow();
99+
100+
// Invalid kind
101+
expect(() =>
102+
insert.run(
103+
"test-skill",
104+
"/path/SKILL.md",
105+
"project",
106+
"git:abc123",
107+
"test intent",
108+
"general",
109+
"test delta",
110+
"hash3",
111+
Date.now(),
112+
),
113+
).toThrow();
104114
} finally {
105115
closeQuietly(db);
106116
}
107117
});
108118

109119
test("LATEST_SUPPORTED_VERSION equals LATEST_MIGRATION_VERSION after v51", () => {
120+
// This test will fail until storage-db.ts is bumped to 39.
121+
// Belt-and-braces: mirrors schema-version-fence.test.ts but is co-located with the migration.
122+
// If this feels redundant, keep it with this comment — co-location aids discoverability.
123+
// NOTE: use ESM import at the top of the file (not require()) to match codebase pattern.
110124
expect(LATEST_SUPPORTED_VERSION).toBe(LATEST_MIGRATION_VERSION);
111-
expect(LATEST_SUPPORTED_VERSION).toBe(52);
112125
});
113126
});

0 commit comments

Comments
 (0)