fix(skill-memory): address third review round (cubic run 17d21f5f)

- provenance.ts: anchor the Base-directory regex to line-start (^…/gm) and take
  the LAST match — opencode appends the provenance line at the END of tool
  output, so a skill whose CONTENT echoes 'Base directory for this skill:' (e.g.
  a skill documenting skill-memory) would otherwise shadow the real line and
  misdirect recall to a bogus identity.
- read-session-formatting.ts: narrow the marker-safe exclusion to CR/LF/tab only
  — a ')' does not break the single-line TC: skill(<name>) marker and the
  historian reads it as natural language, so a ')'-containing name is preserved
  verbatim (identity key) instead of dropped.
- ARCHITECTURE.md: update the 'Skill-memory (motor memory)' Key Abstraction to
  the shipped reality (v50/51/52, multi-rung embedding+FTS recall, global-'*'
  union) — was stale (v37, 'P2 TODO'). Remove the PR-added duplicate
  'Tag Identity (v3.3.1+)' section (upstream owns the lean '## Tag identity';
  Tag Identity is unrelated to skill-memory — rebase scope-creep).

Regression tests: provenance last-match + mid-line rejection; ')' name preserved
+ CR/LF/tab still dropped.

Author: Tehan

ARCHITECTURE.md

@@ -219,8 +219,8 @@ Three effective modes; the heavier features (historian, nudges, adjunct injectio
 
 **Skill-memory (motor memory for skills):**
 - Purpose: Per-skill cross-session recall — when a skill declares `skill-memory: { enabled: true }` in its frontmatter, accumulated gotchas/discoveries/fixes/workflow steps surface in a `<skill-memory>` block appended to the skill tool's RESULT on every load. Agents write back via `ctx_skill_note`; explicit recall (without re-loading) is `ctx_skill_recall`. The transparent after-hook is the primary path; the two tools are companions.
-- Location: `src/features/magic-context/skill-memory/{frontmatter,provenance,storage,recall}.ts`; `src/hooks/magic-context/skill-tool-definition.ts` + the `skill-memory` branches in `src/hooks/magic-context/hook-handlers.ts`; `src/tools/ctx-skill-note/`, `src/tools/ctx-skill-recall/`. Table created in migration v37 (`skill_memory`).
-- Pattern: Three-hook transparent augmentation (definition → before → after). The before-hook stashes a per-callID `intent` (bounded 60s TTL + 256-cap + session-delete clear). The after-hook parses the `Base directory for this skill: file:///...` line (cross-platform via `fileURLToPath`), reads the skill's `SKILL.md` from disk to recover its `skill-memory:` frontmatter (opencode strips it from the model-facing output), populates a session-scoped `SkillLoadRegistry` (NOT persisted), and calls `recallSkillMemoryBlock` (feature layer — shared core used by the tool too) to format the injected block. Append lands in the tool RESULT (conversation tail) — cache-safe by construction. P1 retrieval is flat: recency × hit_count, no embeddings (P2 rungs are designed and marked TODO in `recall.ts`). Per-skill opt-in via SKILL.md frontmatter (`enabled: true` required; `max_tokens` 1500 / `max_pinned_tokens` 4000 / `dedup_threshold` 0.92 are tunable). Optional dreamer `distill-skill-memory` task (opt-in, NOT a default) handles merge/prune/promote maintenance.
+- Location: `src/features/magic-context/skill-memory/{frontmatter,provenance,storage,recall}.ts`; `src/hooks/magic-context/skill-tool-definition.ts` + the `skill-memory` branches in `src/hooks/magic-context/hook-handlers.ts`; `src/tools/ctx-skill-note/`, `src/tools/ctx-skill-recall/`. Tables: `skill_memory` (migration v50), embedding columns + content-linked `skill_memory_fts` vtable (v51), `origin_project`/`source_type` + global `'*'` collision-merge (v52). See the fuller "Skill-memory flow" subsection above for the per-phase wiring.
+- Pattern: Three-hook transparent augmentation (definition → before → after). The before-hook stashes an `intent` keyed by `${sessionId}:${callID}` (bounded 60s TTL + 256-cap + per-session prefix-prune on delete). The after-hook parses the trailing `Base directory for this skill: file:///...` line (line-anchored, last-match, cross-platform via `fileURLToPath`), reads the skill's `SKILL.md` from disk to recover its `skill-memory:` frontmatter (opencode strips it from the model-facing output), populates a session-scoped `SkillLoadRegistry` (NOT persisted), and calls `recallSkillMemoryBlock` (feature layer — shared core used by the tool too) to format the injected block. Append lands in the tool RESULT (conversation tail) — cache-safe by construction. Retrieval is a multi-rung recall cascade (intent embeddings → FTS5 → flat recency×hit), unioning the skill's own partition with the global `'*'` partition; the dreamer `distill-skill-memory` task (opt-in, NOT a default) re-embeds stale vectors and runs merge/prune/promote maintenance. Per-skill opt-in via SKILL.md frontmatter (`enabled: true` required; `max_tokens` 1500 / `max_pinned_tokens` 4000 / `dedup_threshold` 0.92 + `ranking_*` weights are tunable).
 
 **TUI ↔ server RPC:**
 - Purpose: Localhost RPC for sidebar data, status/recomp dialogs, and TUI-action consumption.
@@ -361,40 +361,3 @@ Each `tags` row is one taggable source-content unit (`message`, `file`, or `tool
 **Schema migrations:** `src/features/magic-context/migrations.ts` declares versioned migrations v1–v52 (`LATEST_SUPPORTED_VERSION = 52` in `storage-db.ts` is the schema-fence ceiling and MUST be bumped with every new migration; a unit test — `schema-version-fence.test.ts` — asserts `LATEST_SUPPORTED_VERSION === LATEST_MIGRATION_VERSION` so the two can't drift). Notable: v10 `tool_owner_message_id` (composite tool-tag identity); v11 `todo_synthetic_*` (synthetic-todowrite); v12 orphan `memory_embeddings` cleanup; v13 `pending_compaction_marker_state` (deferred-marker drain); v14 project-scoped key files + version counter; v15 `deferred_execute_state` (boundary execution); v16 context-limit cache sentinels; v17 multi-anchor note-nudge/auto-search JSON storage; v18 `pending_pi_compaction_marker_state`; v19 compartment-state lease table; v20 subagent invocation token accounting; v21 session lifetime work metrics; **v22 the v2.0 cache-architecture foundation (m[0]/m[1] split tables, `project_state` epoch counter, plus per-compartment `p1`–`p4` tier columns, `importance`, `episode_type`, `p1_embedding`, and `legacy` flag); v23 `compartment_events` (historian-extracted causal_incident / trajectory_correction, stored-not-rendered in v2.0); v24 `historian_runs` telemetry (per-run chunk range, compartment/fact/event counts, importance min/max/avg, status + failure reason, FK to `subagent_invocations`); v25 `pi_stable_id_scheme` (Pi stable-id cutover watermark); v26 `memory_mutation_log` + `cached_m1_bytes` (memory supersede-delta — non-additive in-session memory mutations render as an m[1] `<memory-updates>` delta instead of bumping the project epoch, plus the frozen-m[1]-bytes cache column); v27 `tags.entry_fingerprint` (Pi fallback-tag adoption); v28 `git_sweep_coordinator` (lease/cooldown for cross-process git-commit sweeps); v29 `notes.anchor_ordinal` (note→conversation-tail traceback); v30 `cached_m0_system_hash` / `cached_m0_tool_set_hash` / `cached_m0_model_key` (HARD-bust m[0] markers — provider-side cache-eviction detection for the materialization taxonomy; the migration clears the m[0]/m[1] cache once so pre-v30 rows re-materialize cleanly); v31 ctx_reduce-nudge state (`last_nudge_undropped`, `channel2_nudge_state`, `last_emergency_input_sample` + startup heal zeroing legacy sticky/anchor nudge state); v32 protected-tail v3 boundary state + per-tag cached token counts (`tags.token_count` / `input_token_count` / `reasoning_token_count` — computed once on tag insert, summed for sidebar/boundary/nudge math); v33 `compartment_chunk_embeddings` table for cross-session semantic search across compartment windows; v34 `workspaces` / `workspace_members` tables plus `cached_m0_workspace_fingerprint` m[0] marker (with a one-shot m[0]/m[1] cache reset so pre-v34 rows re-materialize cleanly); v35 `workspaces.share_categories` default + epoch refresh for existing members; v36 `session_projects` ownership map + seed for pre-v36 embedded sessions; v37 emergency drain catch-up latch + historian drain failure backoff; v38 `transform_decisions` table for durable cache-event cause attribution; v40 index Pi fallback tool owners for stable-id cutover; v41 key detected context limits by model; v42 per-task dreamer scheduling state (Dreamer v2 A+B); v43 memory verification side table and verify watermarks; v44 memory classification scope and shareability columns; v45 retrospective content watermark and processed-window idempotence; v46 Primers v1 candidate and promoted primer storage; v47 compiled smart-note checks and runtime policy state; v48 DreamerV2 rework: memory→file mapping vs verification split, classify marker; v49 per-model embedding coexistence and active identity tracking; **v50 `skill_memory` table for per-skill cross-session recall (P1 — see "Skill-memory" in Key Abstractions) with `(skill_id, tier, project_identity, normalized_hash)` UNIQUE, plus `idx_skill_memory_lookup` and `idx_skill_memory_fts_prep` indexes for the flat-recall path; v51 skill-memory P2 — `delta_embedding` + `recall_count` columns + content-linked `skill_memory_fts` FTS5 vtable (intent+delta, porter+unicode61 tokenizer, INSERT/UPDATE/DELETE triggers, post-migration rebuild) for the multi-rung recall cascade; v52 skill-memory historian extraction — `origin_project` + `source_type` columns and global-tier `'*'` collision-merge (one row per global lesson, recallable from any repo, with `origin_project` preserved).** Migration runner uses `schema_migrations` table with version-ordered execution and sibling-startup race protection (duplicate-insert is tolerated).
 
 **Harness-aware behavior:** `src/shared/harness.ts` exposes `setHarness()`/`getHarness()` for the runtime to identify itself; production INSERTs into session-scoped tables tag rows with the current harness. Pi-specific session-resolution paths are skipped on OpenCode and vice versa.
-
-## Tag Identity (v3.3.1+)
-
-**Tag types:** `message`, `file`, `tool`. Each row in the `tags` table represents one source-content unit that can be tagged with `§N§` and dropped/truncated/replayed by the runtime.
-
-**Identity composition by type:**
-
-- **`message` and `file` tags:** identified by `(session_id, message_id)`. The `message_id` for these is a synthetic content id (`<msgId>:p<partIndex>` for text, `<msgId>:fileN` for files). These ids are globally unique within a session.
-
-- **`tool` tags:** identified by `(session_id, message_id, tool_owner_message_id)` — a *composite* identity. For tool tags, `message_id` is the OpenCode-generated callID (e.g. `read:32`). Pre-v3.3.1 the runtime keyed tool tags by callID alone, but OpenCode reuses a callID counter per assistant turn — so two assistant turns that each invoke `read:32` produced the SAME callID for different invocations. The fix: include the *owning assistant message id* in the key so each invocation gets its own row.
-
-**Schema enforcement:** schema migration v10 (`src/features/magic-context/migrations.ts`) adds `tool_owner_message_id` (`TEXT NULL`), a partial UNIQUE index `idx_tags_tool_composite` on `(session_id, message_id, tool_owner_message_id) WHERE type='tool' AND tool_owner_message_id IS NOT NULL`, and a partial lookup index `idx_tags_tool_null_owner` on `(session_id, message_id) WHERE type='tool' AND tool_owner_message_id IS NULL` to back lazy adoption.
-
-**Helper API surface (`src/features/magic-context/storage-tags.ts`):**
-
-- `getToolTagNumberByOwner(db, sessionId, callId, ownerMsgId)`: composite-identity lookup.
-- `getNullOwnerToolTag(db, sessionId, callId)`: find a legacy NULL-owner orphan to lazily adopt.
-- `adoptNullOwnerToolTag(db, tagId, ownerMsgId)`: attempt to claim a NULL-owner row (NULL guard ensures first claim wins).
-- `getPersistedToolOwnerNearestPrior(db, sessionId, callId, beforeMessageId)`: derive the most recent prior owner for a tool result whose invocation isn't in the visible window.
-- `deleteToolTagsByOwner(db, sessionId, ownerMsgId)`: cascade delete on `message.removed`.
-
-**Owner derivation (`src/hooks/magic-context/tag-messages.ts`):**
-
-For each tool observation in a transform pass:
-
-1. **Invocation parts** (`tool-invocation` / `tool_use`): owner = the message hosting the part.
-2. **Result parts** (`tool` with output / `tool_result`): pop the FIFO queue of unpaired invocations for that callId; owner = the popped invocation's message id.
-3. **Result-only window** (invocation compacted away): fall back to `getPersistedToolOwnerNearestPrior` for the most recent prior persisted owner; if none found, last-resort owner = the result's own message id.
-
-The same logic mirrors in `src/hooks/magic-context/read-session-chunk.ts: getRawSessionTagKeysThrough` so the drop queue produces composite keys that match what the tagger persisted.
-
-**Cleanup paths:**
-
-- `deleteTagsByMessageId(db, sessionId, messageId)` (called from `event-handler.ts` on `message.removed`) deletes BOTH content-id-scoped tags (text/file on the removed message) AND owner-scoped tool tags (`tool_owner_message_id == messageId`).
-- `applyHeuristicCleanup` keys both the tag-side index and fingerprint-side map by composite `<ownerMsgId>\x00<callId>`. The fingerprint VALUE includes ownerMsgId too, so cross-owner pairs with same `(toolName, args)` produce DISTINCT fingerprints and are NOT merged.
-
-**Legacy NULL-owner handling:** rows written by pre-v3.3.1 plugin versions have `tool_owner_message_id = NULL`. The Layer B backfill (`src/features/magic-context/tool-owner-backfill.ts`) populates those rows from OpenCode's session DB on plugin upgrade (lease-based concurrency, batched commits). When backfill is skipped (no OpenCode DB attached) lazy adoption converts orphans to non-NULL on the next observation. Drop queue and heuristic cleanup gracefully fall back to bare-callId match for unbackfilled NULL-owner rows.

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

@@ -69,4 +69,26 @@ describe("parseSkillProvenance", () => {
         expect(result!.tier).toBe("global");
         expect(result!.skillSource).toBe("opencode-global");
     });
+
+    test("takes the LAST line-anchored match when skill CONTENT echoes the marker phrase", () => {
+        // A skill that documents skill-memory itself could contain the marker
+        // phrase in its body. opencode appends the REAL provenance line last, so
+        // last-match must win — a first-match parse would resolve the bogus URL.
+        const output =
+            `# Skill: skill-memory internals\n` +
+            `Example: Base directory for this skill: file:///decoy/path/evil-skill\n` +
+            `more prose\n` +
+            `Base directory for this skill: file://${HOME}/.config/opencode/skills/real-skill`;
+        const result = parseSkillProvenance(output, "real-skill");
+        expect(result!.resolvedPath).toBe(`${HOME}/.config/opencode/skills/real-skill/SKILL.md`);
+        expect(result!.tier).toBe("global");
+    });
+
+    test("ignores a mid-line (non-line-anchored) marker mention", () => {
+        // "see the Base directory for this skill: file:///x" embedded mid-sentence
+        // (not at column 0) must NOT be captured.
+        const output = `Note: see the Base directory for this skill: file:///wrong/x for details.\nBase directory for this skill: file://${HOME}/.config/opencode/skills/right`;
+        const result = parseSkillProvenance(output, "right");
+        expect(result!.resolvedPath).toBe(`${HOME}/.config/opencode/skills/right/SKILL.md`);
+    });
 });

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

@@ -10,13 +10,20 @@ export interface SkillProvenance {
 
 // Matches: "Base directory for this skill: file:///abs/path/to/skill/dir"
 // Uses fileURLToPath (not naive regex capture) for cross-platform correctness.
-const BASE_DIR_REGEX = /Base directory for this skill: (file:\/\/\/[^\n\r]+)/m;
+// Anchored to line-start (`^…/gm`) AND we take the LAST match: opencode appends
+// this provenance line at the END of the tool output, so if the skill's own
+// CONTENT contains the same phrase (e.g. a skill documenting skill-memory
+// provenance), a first-match/unanchored parse would capture the wrong URL and
+// misdirect recall to a bogus skill identity. Line-anchoring rejects mid-prose
+// mentions; last-match ensures the real trailing provenance line wins even if an
+// example block reproduces it at column 0.
+const BASE_DIR_REGEX = /^Base directory for this skill: (file:\/\/\/[^\n\r]+)/gm;
 
 export function parseSkillProvenance(output: string, skillId: string): SkillProvenance | null {
-    const match = output.match(BASE_DIR_REGEX);
-    if (!match) return null;
+    const matches = [...output.matchAll(BASE_DIR_REGEX)];
+    if (matches.length === 0) return null;
 
-    const fileUrl = match[1].trim();
+    const fileUrl = matches[matches.length - 1][1].trim();
     let absDir: string;
     try {
         // Normalize OS-native separators to forward slashes: on Windows

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

@@ -32,4 +32,24 @@ describe("extractToolCallSummaries — skill tool", () => {
 
         expect(extractToolCallSummaries(parts)).toEqual(["TC: skill"]);
     });
+
+    test("preserves a skill name containing ')' verbatim (does not drop the marker)", () => {
+        // A ")" doesn't break the single-line marker and the historian reads it as
+        // natural language — preserve the name (identity key) rather than drop it.
+        const parts = [
+            {
+                type: "tool",
+                tool: "skill",
+                state: { input: { name: "weird(name)" }, metadata: {} },
+            },
+        ];
+        expect(extractToolCallSummaries(parts)).toEqual(["TC: skill(weird(name))"]);
+    });
+
+    test("drops the name only when it contains a real line-breaker (CR/LF/tab)", () => {
+        const parts = [
+            { type: "tool", tool: "skill", state: { input: { name: "bad\nname" }, metadata: {} } },
+        ];
+        expect(extractToolCallSummaries(parts)).toEqual(["TC: skill"]);
+    });
 });

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

@@ -82,7 +82,14 @@ export function extractToolCallSummaries(parts: unknown[]): string[] {
         // mutated identity.
         if (p.tool === "skill") {
             const rawName = input && typeof input.name === "string" ? input.name : "";
-            const markerSafe = rawName !== "" && !/[\r\n\t)]/.test(rawName);
+            // Only CR/LF/tab genuinely corrupt the single-line `TC: skill(<name>)`
+            // marker; a ")" does NOT break the line and the historian reads the
+            // marker as natural language (not a strict paren-matched parse), so a
+            // ")"-containing name is preserved VERBATIM rather than dropped —
+            // dropping the name loses historian attribution + recall keying, which
+            // is worse than a cosmetically-ambiguous paren. (Real skill directory
+            // names are slugs; this is defensive.)
+            const markerSafe = rawName !== "" && !/[\r\n\t]/.test(rawName);
             summaries.push(markerSafe ? `TC: skill(${rawName})` : "TC: skill");
             continue;
         }