fix(skills): substitute {{SKILLS_DIR}} in materialized skill files

packages/bcode-browser/src/skills.ts

@@ -1,23 +1,34 @@
 // Skills directory resolver.
 //
-// Two packaging modes:
+// Skills are always materialized to `<dataDir>/skills/` so the agent reads
+// from a stable absolute path on every platform. During materialization,
+// `{{SKILLS_DIR}}` placeholders inside `*.md` files are replaced with the
+// target absolute path so cross-references in BROWSER.md (e.g. "read
+// `{{SKILLS_DIR}}/cloud-browser.md`") resolve to a path the agent can use.
 //
-// 1. Dev mode — `import.meta.url` resolves to `packages/bcode-browser/src/`
-//    on disk, skills live at the sibling `../skills/`. Used by `bun run
-//    --cwd packages/opencode dev` and tests.
+// Two packaging modes feed the materializer:
+//
+// 1. Dev mode — `import.meta.url` resolves to `packages/bcode-browser/src/`;
+//    source skills live at the sibling `../skills/`. The hash is computed
+//    over the on-disk source files so edits during dev iteration trigger
+//    re-extraction on the next launch.
 //
 // 2. Compiled mode — running from a `bun build --compile` binary.
-//    `import.meta.dir` lives under `/$bunfs/` (or `B:/~BUN/` on Windows),
-//    a read-only virtual filesystem the agent's `read` tool can't see in a
-//    useful path shape. We extract the embedded skills (built into the
-//    binary by `script/embed-skills.ts`) to `<dataDir>/skills/`. A content-
-//    hash sentinel at `<dataDir>/skills/.bcode-build` records the embed
-//    bundle that produced the on-disk tree; warm launches stat-and-skip.
+//    `import.meta.dir` lives under `/$bunfs/` (or `B:/~BUN/` on Windows), a
+//    read-only virtual filesystem the agent's `read` tool can't see in a
+//    useful path shape. We use the embed map generated by
+//    `script/embed-skills.ts` and its precomputed `buildHash`.
+//
+// In both modes a content-hash sentinel at `<dataDir>/skills/.bcode-build`
+// records the bundle + target path that produced the tree; warm launches
+// stat-and-skip when both match. Including the target path in the sentinel
+// guards against a stale tree if `dataDir` ever changes between launches.
 //
 // Skills are read-only baseline: every launch overwrites the on-disk tree
-// from the binary's embed (no agent-editable surface). The agent's editable
-// surface is `<projectDir>/.bcode/agent-workspace/`, per-project, never here.
+// (no agent-editable surface). The agent's editable surface is
+// `<projectDir>/.bcode/agent-workspace/`, per-project, never here.
 
+import crypto from "crypto"
 import fs from "fs/promises"
 import path from "path"
 import { fileURLToPath } from "url"
@@ -29,6 +40,7 @@ const isCompiled = (() => {
 })()
 const DEV_SKILLS_DIR = path.resolve(__dirname, "..", "skills")
 const SENTINEL_NAME = ".bcode-build"
+const PLACEHOLDER = "{{SKILLS_DIR}}"
 
 // Static path so the agent permission glob can use a stable absolute path.
 export const skillsDir = (dataDir: string) => path.join(dataDir, "skills")
@@ -38,36 +50,94 @@ const readSentinel = async (dir: string) => {
   catch { return null }
 }
 
-const extractEmbeddedSkills = async (dataDir: string): Promise<string> => {
-  const target = skillsDir(dataDir)
+// `.md` files get template substitution; everything else is copied byte-for-
+// byte. Substitution operates on the raw bytes rather than a UTF-8 round-trip
+// to keep non-Markdown assets (images, binary fixtures) untouched if the
+// skills tree ever grows beyond Markdown.
+const writeSkillFile = async (dest: string, content: Uint8Array, target: string) => {
+  await fs.mkdir(path.dirname(dest), { recursive: true })
+  if (!dest.endsWith(".md") || !indexOfPlaceholder(content)) {
+    await Bun.write(dest, content)
+    return
+  }
+  const text = new TextDecoder("utf-8").decode(content).replaceAll(PLACEHOLDER, target)
+  await fs.writeFile(dest, text, "utf8")
+}
+
+// Cheap byte-level pre-check so we skip the UTF-8 decode on files that don't
+// need substitution. The placeholder is pure ASCII so a byte search is safe.
+const PLACEHOLDER_BYTES = new TextEncoder().encode(PLACEHOLDER)
+const indexOfPlaceholder = (buf: Uint8Array) => {
+  outer: for (let i = 0; i + PLACEHOLDER_BYTES.length <= buf.length; i++) {
+    for (let j = 0; j < PLACEHOLDER_BYTES.length; j++) {
+      if (buf[i + j] !== PLACEHOLDER_BYTES[j]) continue outer
+    }
+    return true
+  }
+  return false
+}
+
+// Stable hash over (rel + NUL + content) for every source file in sorted
+// order — same shape as the build-time hash in `script/embed-skills.ts`.
+const computeDevHash = async (files: string[]) => {
+  const hash = crypto.createHash("sha256")
+  for (const rel of files) {
+    hash.update(rel)
+    hash.update("\0")
+    hash.update(await fs.readFile(path.join(DEV_SKILLS_DIR, rel)))
+  }
+  return hash.digest("hex")
+}
+
+// Sentinel = "<bundleHash>:<target>". Including `target` invalidates the tree
+// if `dataDir` (and therefore the substituted absolute path) ever changes.
+const sentinelFor = (bundleHash: string, target: string) => `${bundleHash}:${target}`
+
+const materializeFromSource = async (target: string): Promise<string> => {
+  const files = (await Array.fromAsync(new Bun.Glob("**/*").scan({ cwd: DEV_SKILLS_DIR })))
+    .map((f) => f.replaceAll("\\", "/"))
+    .sort()
+  const bundleHash = await computeDevHash(files)
+  const sentinel = sentinelFor(bundleHash, target)
+  if ((await readSentinel(target)) === sentinel) return target
+
+  await fs.mkdir(target, { recursive: true })
+  await Promise.all(
+    files.map(async (rel) => {
+      const buf = await fs.readFile(path.join(DEV_SKILLS_DIR, rel))
+      await writeSkillFile(path.join(target, rel), buf, target)
+    }),
+  )
+  await fs.writeFile(path.join(target, SENTINEL_NAME), sentinel, "utf8")
+  return target
+}
+
+const materializeFromEmbed = async (target: string): Promise<string> => {
   // @ts-expect-error generated at build time
   const mod = await import("bcode-skills.gen.ts").catch(() => null)
   if (!mod) throw new Error("bcode-skills.gen.ts not found in compiled binary — was the build script updated?")
   const fileMap = mod.default as Record<string, string>
-  const buildHash = mod.buildHash as string
-
-  if ((await readSentinel(target)) === buildHash) return target
+  const sentinel = sentinelFor(mod.buildHash as string, target)
+  if ((await readSentinel(target)) === sentinel) return target
 
   await fs.mkdir(target, { recursive: true })
-  // Skills are baseline-overwrite — every file from the embed lands on disk.
   await Promise.all(
     Object.entries(fileMap).map(async ([rel, bunfsPath]) => {
-      const dest = path.join(target, rel)
-      await fs.mkdir(path.dirname(dest), { recursive: true })
-      await Bun.write(dest, Bun.file(bunfsPath))
+      const buf = new Uint8Array(await Bun.file(bunfsPath).arrayBuffer())
+      await writeSkillFile(path.join(target, rel), buf, target)
     }),
   )
-  await fs.writeFile(path.join(target, SENTINEL_NAME), buildHash, "utf8")
+  await fs.writeFile(path.join(target, SENTINEL_NAME), sentinel, "utf8")
   return target
 }
 
 const extractCache = new Map<string, Promise<string>>()
 
 export const resolveSkillsDir = (dataDir: string): Promise<string> => {
-  if (!isCompiled) return Promise.resolve(DEV_SKILLS_DIR)
   const cached = extractCache.get(dataDir)
   if (cached) return cached
-  const fresh = extractEmbeddedSkills(dataDir)
+  const target = skillsDir(dataDir)
+  const fresh = isCompiled ? materializeFromEmbed(target) : materializeFromSource(target)
   extractCache.set(dataDir, fresh)
   fresh.catch(() => {
     if (extractCache.get(dataDir) === fresh) extractCache.delete(dataDir)

packages/bcode-browser/test/skills.test.ts

@@ -0,0 +1,68 @@
+// Skills materialization with `{{SKILLS_DIR}}` template substitution.
+//
+// Regression guard: the on-disk `BROWSER.md` (and any other markdown skill)
+// must not contain literal `{{SKILLS_DIR}}` strings — those are templates the
+// agent is supposed to see resolved to the absolute extraction path.
+
+import { expect, test } from "bun:test"
+import fs from "fs/promises"
+import os from "os"
+import path from "path"
+import { Skills } from "../src/skills"
+
+test("resolveSkillsDir materializes BROWSER.md with {{SKILLS_DIR}} substituted", async () => {
+  const dataDir = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-skills-"))
+  try {
+    const dir = await Skills.resolveSkillsDir(dataDir)
+    expect(dir).toBe(path.join(dataDir, "skills"))
+
+    const browser = await fs.readFile(path.join(dir, "BROWSER.md"), "utf8")
+    // No literal placeholder leaks through to the agent.
+    expect(browser).not.toContain("{{SKILLS_DIR}}")
+    // Cross-references resolve to absolute paths under the materialized dir.
+    expect(browser).toContain(path.join(dir, "cloud-browser.md"))
+    expect(browser).toContain(path.join(dir, "interaction-skills"))
+
+    // Non-Markdown sentinel itself must not be substituted.
+    const sentinel = await fs.readFile(path.join(dir, ".bcode-build"), "utf8")
+    expect(sentinel).not.toContain("{{SKILLS_DIR}}")
+  } finally {
+    await fs.rm(dataDir, { recursive: true, force: true })
+  }
+})
+
+test("resolveSkillsDir is idempotent — second call hits the sentinel and skips rewrite", async () => {
+  const dataDir = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-skills-"))
+  try {
+    const dir = await Skills.resolveSkillsDir(dataDir)
+    const browser = path.join(dir, "BROWSER.md")
+    const before = (await fs.stat(browser)).mtimeMs
+    // Yield to push mtime forward if a rewrite happens.
+    await new Promise((r) => setTimeout(r, 20))
+    // Bypass in-process cache by reaching through to a fresh data dir handle.
+    const dir2 = await Skills.resolveSkillsDir(dataDir)
+    expect(dir2).toBe(dir)
+    const after = (await fs.stat(browser)).mtimeMs
+    expect(after).toBe(before)
+  } finally {
+    await fs.rm(dataDir, { recursive: true, force: true })
+  }
+})
+
+test("resolveSkillsDir re-materializes when the target path changes (sentinel mismatch)", async () => {
+  const dataDirA = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-skills-a-"))
+  const dataDirB = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-skills-b-"))
+  try {
+    const dirA = await Skills.resolveSkillsDir(dataDirA)
+    const dirB = await Skills.resolveSkillsDir(dataDirB)
+    const browserA = await fs.readFile(path.join(dirA, "BROWSER.md"), "utf8")
+    const browserB = await fs.readFile(path.join(dirB, "BROWSER.md"), "utf8")
+    expect(browserA).toContain(dirA)
+    expect(browserB).toContain(dirB)
+    expect(browserA).not.toContain(dirB)
+    expect(browserB).not.toContain(dirA)
+  } finally {
+    await fs.rm(dataDirA, { recursive: true, force: true })
+    await fs.rm(dataDirB, { recursive: true, force: true })
+  }
+})

packages/opencode/src/agent/agent.ts

@@ -91,11 +91,14 @@ export const layer = Layer.effect(
         // to whichever project is open (Phase H hard rule #3 — workspace as
         // plain code, per-project).
         const agentWorkspaceGlob = "**/.bcode/agent-workspace/**/*"
-        // Browser-skills tree shipped inside the binary, extracted at runtime
-        // to <Global.Path.data>/skills/. Read-only baseline; the agent reads
-        // BROWSER.md + interaction-skills/ when driving the browser. In dev
-        // mode the skills live inside the worktree, so this glob is a no-op
-        // there.
+        // Browser-skills tree, materialized at runtime to
+        // <Global.Path.data>/skills/. Read-only baseline; the agent reads
+        // BROWSER.md + interaction-skills/ when driving the browser.
+        // Materialization happens in both dev and compiled modes so the
+        // `{{SKILLS_DIR}}` placeholder in BROWSER.md is substituted with a
+        // stable absolute path the agent can use in cross-references. The
+        // wildcard impl (Wildcard.match) treats `*` as `.*` (greedy across
+        // `/`), so this single-segment glob matches the entire subtree.
         const browserSkillsGlob = path.join(Skills.skillsDir(Global.Path.data), "*")
         const whitelistedDirs = [
           Truncate.GLOB,