fix(skill-memory): address second review round (greptile P1 + cubic P2/P3)

- P1: recall now unions the skill's own partition with the global '*' partition
  (recallPartitionPredicate helper) so a PROJECT-LOCAL skill surfaces
  historian-written global notes — previously orphaned (tier='project' query
  never matched tier='global'/'*'). Write/dedup paths stay exact-partition.
- Escape apostrophes in projectPath before SQL string interpolation in the
  distill prompt template.
- Frontmatter regex tolerates a leading UTF-8 BOM / whitespace (still start-anchored).
- TC: skill(<name>) marker emits the name VERBATIM when marker-safe, else drops
  it — never mutates the identity key (recall keys on raw input.name).
- Fix misleading frontmatter test: now actually exercises a '#' inside a quoted
  scalar (preserved) vs unquoted (comment-stripped).

Regression tests: project-local skill recalls a global historian note; agent
project note + historian global note both surface for the same skill.

Author: Tehan

packages/plugin/src/features/magic-context/dreamer/task-prompts.ts

@@ -393,7 +393,7 @@ function buildDistillSkillMemoryPrompt(projectPath: string): string {
           SUM(CASE WHEN pinned = 1 THEN 1 ELSE 0 END) as pinned_count,
           SUM(CASE WHEN intent_embedding IS NULL OR delta_embedding IS NULL THEN 1 ELSE 0 END) as missing_embedding_count
    FROM skill_memory
-   WHERE project_identity IN ('${projectPath}', '*')
+   WHERE project_identity IN ('${projectPath.replace(/'/g, "''")}', '*')
    GROUP BY skill_id, tier
    ORDER BY note_count DESC
    LIMIT 20;

packages/plugin/src/features/magic-context/skill-memory/frontmatter.test.ts

@@ -91,10 +91,20 @@ body`;
         expect(parseFrontmatterConfig(md)?.enabled).toBe(true);
     });
 
-    test("does not strip a '#' inside a quoted scalar", () => {
+    test("a plain quoted scalar still enables", () => {
         const md = `---\nskill-memory:\n  enabled: "true"\n---\nbody`;
-        // "true" (quoted) still enables; the quote-strip path runs after the
-        // unquoted-only comment strip, so quoted values are untouched.
         expect(parseFrontmatterConfig(md)?.enabled).toBe(true);
     });
+
+    test("does NOT strip a '#' inside a quoted scalar (comment-strip is unquoted-only)", () => {
+        // Quoted value containing a '#': the inline-comment strip must NOT fire,
+        // so the value stays the literal "true # x" (≠ "true") and the config is
+        // inert. If the '#' WERE wrongly stripped, it would collapse to "true"
+        // and enable — so a passing assertion proves the '#' was preserved.
+        const quoted = `---\nskill-memory:\n  enabled: "true # x"\n---\nbody`;
+        expect(parseFrontmatterConfig(quoted)).toBeNull();
+        // Contrast: the SAME text UNquoted is a real inline comment → stripped → enables.
+        const unquoted = `---\nskill-memory:\n  enabled: true # x\n---\nbody`;
+        expect(parseFrontmatterConfig(unquoted)?.enabled).toBe(true);
+    });
 });

packages/plugin/src/features/magic-context/skill-memory/frontmatter.ts

@@ -19,8 +19,11 @@ export interface SkillMemoryConfig {
 // Anchored to the very start of the file (NO `m` flag): frontmatter is only
 // valid as the first bytes of the document. With `m`, `^` matches any line
 // start, so a later `--- ... ---` block (e.g. a markdown horizontal rule) could
-// be misparsed as config.
-const FRONTMATTER_REGEX = /^---\r?\n([\s\S]*?)\r?\n---/;
+// be misparsed as config. A leading UTF-8 BOM and leading whitespace/blank
+// lines are tolerated (`\uFEFF?\s*`) so an editor-saved SKILL.md with a BOM or a
+// stray blank first line still parses; this stays start-anchored because `\s*`
+// only spans leading whitespace before the first `---`, never a mid-document rule.
+const FRONTMATTER_REGEX = /^\uFEFF?\s*---\r?\n([\s\S]*?)\r?\n---/;
 
 export function parseFrontmatterConfig(content: string): SkillMemoryConfig | null {
     try {

packages/plugin/src/features/magic-context/skill-memory/recall.test.ts

@@ -427,6 +427,66 @@ describe("cross-project global recall", () => {
             closeQuietly(db);
         }
     });
+
+    test("a PROJECT-LOCAL skill surfaces a historian-written global note (P1 regression)", async () => {
+        // Regression: promoteSkillObservations always writes tier='global'/'*',
+        // but recall for a project-local skill uses scope='project'. Before the
+        // union-on-recall fix, the project-tier query never matched the global
+        // '*' rows, so historian notes were orphaned for project-local skills.
+        const db = makeDb();
+        try {
+            promoteSkillObservations(db, "git:repoA", [
+                {
+                    skillId: "tdd",
+                    kind: "discovery",
+                    lesson: "spike the parser before writing the plan",
+                },
+            ]);
+            // Recall as a PROJECT-tier skill (the failing case pre-fix).
+            const block = await recallSkillMemoryBlock(db, {
+                skill: "tdd",
+                scope: "project",
+                projectIdentity: "git:repoA",
+                frontmatterConfig: cfg,
+            });
+            expect(block).toContain("spike the parser before writing the plan");
+        } finally {
+            closeQuietly(db);
+        }
+    });
+
+    test("a project-local AGENT note and a global HISTORIAN note both surface for the same skill", async () => {
+        const db = makeDb();
+        try {
+            // Agent-written, project-tier note.
+            insertSkillMemoryNote(db, {
+                skillId: "tdd",
+                resolvedPath: "/repo/.opencode/skills/tdd/SKILL.md",
+                tier: "project",
+                skillSource: "opencode-project",
+                projectIdentity: "git:repoLocal",
+                intent: "local lesson",
+                kind: "gotcha",
+                delta: "project-local agent note",
+                normalizedHash: "pl1",
+                createdAt: Date.now(),
+            });
+            // Historian-written, global note for the same skill.
+            promoteSkillObservations(db, "git:repoLocal", [
+                { skillId: "tdd", kind: "discovery", lesson: "global historian note" },
+            ]);
+            const block = await recallSkillMemoryBlock(db, {
+                skill: "tdd",
+                scope: "project",
+                projectIdentity: "git:repoLocal",
+                frontmatterConfig: cfg,
+            });
+            expect(block).toContain("project-local agent note");
+            expect(block).toContain("global historian note");
+        } finally {
+            closeQuietly(db);
+        }
+    });
 });
 
 describe("recallSkillMemoryBlock bumps recall_count for surfaced notes", () => {

packages/plugin/src/features/magic-context/skill-memory/storage.ts

@@ -10,6 +10,37 @@ export function partitionKey(tier: "project" | "global", projectIdentity: string
     return tier === "global" ? "*" : projectIdentity;
 }
 
+/**
+ * RECALL-path partition predicate: a note is recallable for (tier, projectIdentity)
+ * when it is either in the skill's OWN partition OR in the cross-project global
+ * '*' partition (where historian-extracted lessons live). This is why a
+ * project-local skill still surfaces global historian notes — without the global
+ * branch, recall for a project-tier skill (`tier='project'`) would never match the
+ * `tier='global'/project_identity='*'` rows that promoteSkillObservations writes.
+ *
+ * For a global-tier skill the two branches are identical (both `tier='global' AND
+ * project_identity='*'`), so the OR is a harmless no-op — a row still matches once
+ * (it's a row filter, not a join, so no duplication).
+ *
+ * WRITE/dedup paths (insert, findExistingNote, bumpHitCount, getDedupCandidates)
+ * deliberately do NOT use this — they must target one exact partition.
+ *
+ * Returns `{ clause, binds }`; callers splice `clause` into the WHERE and spread
+ * `binds` (tier, ownPartition) at the matching `?` positions.
+ */
+function recallPartitionPredicate(
+    tier: "project" | "global",
+    projectIdentity: string,
+    columnPrefix = "",
+): { clause: string; binds: [string, string] } {
+    const t = `${columnPrefix}tier`;
+    const p = `${columnPrefix}project_identity`;
+    return {
+        clause: `((${t} = ? AND ${p} = ?) OR (${t} = 'global' AND ${p} = '*'))`,
+        binds: [tier, partitionKey(tier, projectIdentity)],
+    };
+}
+
 export interface SkillMemoryNote {
     id: number;
     skill_id: string;
@@ -123,11 +154,12 @@ export function getSkillMemoryNotes(
     projectIdentity: string,
     limit: number,
 ): SkillMemoryNote[] {
+    const { clause, binds } = recallPartitionPredicate(tier, projectIdentity);
     return db
         .prepare(
             `SELECT *
              FROM skill_memory
-             WHERE skill_id = ? AND tier = ? AND project_identity = ?
+             WHERE skill_id = ? AND ${clause}
              ORDER BY
                 pinned DESC,
                (
@@ -145,7 +177,7 @@ export function getSkillMemoryNotes(
                created_at DESC
              LIMIT ?`,
         )
-        .all(skillId, tier, partitionKey(tier, projectIdentity), limit) as SkillMemoryNote[];
+        .all(skillId, ...binds, limit) as SkillMemoryNote[];
 }
 
 /**
@@ -249,13 +281,14 @@ export function getRankingCandidates(
     projectIdentity: string,
     limit: number,
 ): SkillMemoryNote[] {
+    const { clause, binds } = recallPartitionPredicate(tier, projectIdentity);
     return db
         .prepare(
             `SELECT * FROM skill_memory
-         WHERE skill_id=? AND tier=? AND project_identity=?
+         WHERE skill_id=? AND ${clause}
          ORDER BY COALESCE(last_used_at, created_at) DESC LIMIT ?`,
         )
-        .all(skillId, tier, partitionKey(tier, projectIdentity), limit) as SkillMemoryNote[];
+        .all(skillId, ...binds, limit) as SkillMemoryNote[];
 }
 
 export function searchSkillMemoryFts(
@@ -271,15 +304,14 @@ export function searchSkillMemoryFts(
             `SELECT m.* FROM skill_memory_fts f
          JOIN skill_memory m ON m.id = f.rowid
          WHERE skill_memory_fts MATCH ?
-           AND m.skill_id=? AND m.tier=? AND m.project_identity=?
+           AND m.skill_id=? AND ${recallPartitionPredicate(tier, projectIdentity, "m.").clause}
          ORDER BY bm25(skill_memory_fts) ASC, COALESCE(m.last_used_at, m.created_at) DESC
          LIMIT ?`,
         )
         .all(
             matchQuery,
             skillId,
-            tier,
-            partitionKey(tier, projectIdentity),
+            ...recallPartitionPredicate(tier, projectIdentity, "m.").binds,
             limit,
         ) as SkillMemoryNote[];
 }
@@ -290,13 +322,14 @@ export function getPinnedNotes(
     tier: "project" | "global",
     projectIdentity: string,
 ): SkillMemoryNote[] {
+    const { clause, binds } = recallPartitionPredicate(tier, projectIdentity);
     return db
         .prepare(
             `SELECT * FROM skill_memory
-         WHERE skill_id=? AND tier=? AND project_identity=? AND pinned=1
+         WHERE skill_id=? AND ${clause} AND pinned=1
          ORDER BY COALESCE(last_used_at, created_at) DESC`,
         )
-        .all(skillId, tier, partitionKey(tier, projectIdentity)) as SkillMemoryNote[];
+        .all(skillId, ...binds) as SkillMemoryNote[];
 }
 
 export function getSkillMemoryStats(

packages/plugin/src/hooks/magic-context/read-session-formatting.ts

@@ -72,14 +72,17 @@ export function extractToolCallSummaries(parts: unknown[]): string[] {
 
         // Skill tool: surface the skill name (input.name) before the description
         // fallback, which would otherwise mask it if metadata.description exists.
-        // The name is an IDENTITY key (the historian extracts skill-id from this
-        // marker), so do NOT truncate it. Sanitize newlines/control chars and a
-        // stray ")" so the single-line `TC: skill(<name>)` marker can't be
-        // corrupted — skill names are normally slugs, this is defensive only.
+        // The name is an IDENTITY key — the historian extracts the skill-id from
+        // this marker and recall keys on the raw input.name, so the marker name
+        // MUST equal the raw name (no truncation, no mutation) or the stored id
+        // won't match recall. Emit it VERBATIM when marker-safe; if it contains a
+        // marker-breaking char (CR/LF/tab/")") — which a real skill directory name
+        // never does — drop the name (`TC: skill`) rather than emit a corrupted or
+        // mutated identity.
         if (p.tool === "skill") {
             const rawName = input && typeof input.name === "string" ? input.name : "";
-            const name = rawName.replace(/[\r\n\t)]/g, " ").trim();
-            summaries.push(name ? `TC: skill(${name})` : "TC: skill");
+            const markerSafe = rawName !== "" && !/[\r\n\t)]/.test(rawName);
+            summaries.push(markerSafe ? `TC: skill(${rawName})` : "TC: skill");
             continue;
         }