Merge pull request #43 from browser-use/fix/skills-dir-template-substitution

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

Author: Alezander9

packages/bcode-browser/src/skills.ts

@@ -1,78 +1,84 @@
 // Skills directory resolver.
 //
-// Two packaging modes:
+// Materializes the skills tree to `<dataDir>/skills/` and substitutes the
+// `{{SKILLS_DIR}}` placeholder in every file with that absolute path so
+// cross-references inside BROWSER.md (``read `{{SKILLS_DIR}}/cloud-browser.md` ``)
+// point at a real location.
 //
-// 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.
+// Compiled launches (the user-facing path) read a one-line sentinel at
+// `<target>/.bcode-build` recording `<buildHash>:<target>`. When it matches
+// — i.e. same build, same dataDir — the resolver returns immediately
+// without reading or writing any skill content. The build hash is computed
+// once by `script/embed-skills.ts` and lives in the binary, so the cost is
+// a single small file read.
 //
-// 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.
-//
-// 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.
+// Dev launches (`bun run dev`) always re-extract from the worktree so
+// editor saves to source skill files land on the next launch without a
+// separate invalidation step.
 
 import fs from "fs/promises"
 import path from "path"
 import { fileURLToPath } from "url"
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url))
-const isCompiled = (() => {
-  const d = __dirname.replaceAll("\\", "/")
-  return d.startsWith("/$bunfs/") || d.startsWith("B:/~BUN/")
-})()
+const isCompiled = __dirname.replaceAll("\\", "/").match(/^\/\$bunfs\/|^B:\/~BUN\//) !== null
 const DEV_SKILLS_DIR = path.resolve(__dirname, "..", "skills")
-const SENTINEL_NAME = ".bcode-build"
+const SENTINEL = ".bcode-build"
 
-// Static path so the agent permission glob can use a stable absolute path.
+// Static — the agent permission glob and the substituted placeholder both
+// resolve to this path.
 export const skillsDir = (dataDir: string) => path.join(dataDir, "skills")
 
-const readSentinel = async (dir: string) => {
-  try { return await fs.readFile(path.join(dir, SENTINEL_NAME), "utf8") }
-  catch { return null }
+const cache = new Map<string, Promise<string>>()
+
+export const resolveSkillsDir = (dataDir: string): Promise<string> => {
+  const cached = cache.get(dataDir)
+  if (cached) return cached
+  const fresh = materialize(skillsDir(dataDir))
+  cache.set(dataDir, fresh)
+  fresh.catch(() => { if (cache.get(dataDir) === fresh) cache.delete(dataDir) })
+  return fresh
 }
 
-const extractEmbeddedSkills = async (dataDir: string): Promise<string> => {
-  const target = skillsDir(dataDir)
+const materialize = async (target: string): Promise<string> => {
+  // Compiled-mode short-circuit: import the embed (cheap — just file
+  // handles, no content read), check the sentinel, return on hit.
   // @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 embed = isCompiled ? await import("bcode-skills.gen.ts").catch(() => null) : null
+  if (isCompiled && !embed) throw new Error("bcode-skills.gen.ts not found — was the build script updated?")
+  const want = `${embed?.buildHash ?? "dev"}:${target}`
+  if (embed && (await Bun.file(path.join(target, SENTINEL)).text().catch(() => null)) === want) return target
 
+  const files = embed
+    ? await readEmbed(embed.default as Record<string, string>)
+    : await readDevSkills()
   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]) => {
+    Object.entries(files).map(async ([rel, text]) => {
       const dest = path.join(target, rel)
       await fs.mkdir(path.dirname(dest), { recursive: true })
-      await Bun.write(dest, Bun.file(bunfsPath))
+      await fs.writeFile(dest, text.replaceAll("{{SKILLS_DIR}}", target), "utf8")
     }),
   )
-  await fs.writeFile(path.join(target, SENTINEL_NAME), buildHash, "utf8")
+  if (embed) await fs.writeFile(path.join(target, SENTINEL), want, "utf8")
   return target
 }
 
-const extractCache = new Map<string, Promise<string>>()
+const readEmbed = async (map: Record<string, string>): Promise<Record<string, string>> =>
+  Object.fromEntries(
+    await Promise.all(Object.entries(map).map(async ([rel, p]) => [rel, await Bun.file(p).text()])),
+  )
 
-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)
-  extractCache.set(dataDir, fresh)
-  fresh.catch(() => {
-    if (extractCache.get(dataDir) === fresh) extractCache.delete(dataDir)
-  })
-  return fresh
+const readDevSkills = async (): Promise<Record<string, string>> => {
+  const rels = await Array.fromAsync(new Bun.Glob("**/*").scan({ cwd: DEV_SKILLS_DIR }))
+  return Object.fromEntries(
+    await Promise.all(
+      rels.map(async (rel) => [
+        rel.replaceAll("\\", "/"),
+        await fs.readFile(path.join(DEV_SKILLS_DIR, rel), "utf8"),
+      ]),
+    ),
+  )
 }
 
 export * as Skills from "./skills"

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

@@ -0,0 +1,44 @@
+// Skills materialization with `{{SKILLS_DIR}}` template substitution.
+// Regression guard: the on-disk skill files must not contain literal
+// `{{SKILLS_DIR}}` strings — those are templates the agent reads as
+// resolved absolute paths.
+
+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 skills 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")
+    expect(browser).not.toContain("{{SKILLS_DIR}}")
+    expect(browser).toContain(path.join(dir, "cloud-browser.md"))
+    expect(browser).toContain(path.join(dir, "interaction-skills"))
+  } finally {
+    await fs.rm(dataDir, { recursive: true, force: true })
+  }
+})
+
+test("different dataDirs get their own substituted paths", async () => {
+  const a = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-skills-a-"))
+  const b = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-skills-b-"))
+  try {
+    const dirA = await Skills.resolveSkillsDir(a)
+    const dirB = await Skills.resolveSkillsDir(b)
+    const [browserA, browserB] = await Promise.all([
+      fs.readFile(path.join(dirA, "BROWSER.md"), "utf8"),
+      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(a, { recursive: true, force: true })
+    await fs.rm(b, { recursive: true, force: true })
+  }
+})

packages/opencode/src/agent/agent.ts

@@ -91,11 +91,10 @@ 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/ in both dev and compiled modes (so the
+        // `{{SKILLS_DIR}}` placeholder in BROWSER.md gets substituted with a
+        // stable absolute path). Read-only baseline.
         const browserSkillsGlob = path.join(Skills.skillsDir(Global.Path.data), "*")
         const whitelistedDirs = [
           Truncate.GLOB,