fix(skills): sync managed symlink skills as directories

steipete · steipete · commit 926bf66ee336 · 2026-05-14T22:11:01.000+01:00
diff --git a/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts b/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts
@@ -2,7 +2,7 @@ import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
-import { withEnv } from "../test-utils/env.js";
+import { withEnv, withEnvAsync } from "../test-utils/env.js";
 import { writeSkill } from "./skills.e2e-test-helpers.js";
 import { buildWorkspaceSkillsPrompt, syncSkillsToWorkspace } from "./skills/workspace.js";
 
@@ -357,4 +357,45 @@ describe("buildWorkspaceSkillsPrompt", () => {
       true,
     );
   });
+
+  it("syncs managed symlinked skills as real directories in the target workspace", async () => {
+    const sourceWorkspace = await createCaseDir("source");
+    const targetWorkspace = await createCaseDir("target");
+    const managedDir = path.join(sourceWorkspace, ".managed");
+    const skillName = "managed-linked";
+    const targetSkillDir = path.join(await createCaseDir("manager-cache"), ".hidden-target");
+    await writeSkill({
+      dir: targetSkillDir,
+      name: skillName,
+      description: "Managed symlink target",
+    });
+    await fs.mkdir(managedDir, { recursive: true });
+    await fs.symlink(
+      targetSkillDir,
+      path.join(managedDir, skillName),
+      process.platform === "win32" ? "junction" : "dir",
+    );
+
+    await withEnvAsync({ HOME: sourceWorkspace }, () =>
+      syncSkillsToWorkspace({
+        sourceWorkspaceDir: sourceWorkspace,
+        targetWorkspaceDir: targetWorkspace,
+        bundledSkillsDir: path.join(sourceWorkspace, ".bundled"),
+        managedSkillsDir: managedDir,
+        skillFilter: [skillName],
+      }),
+    );
+
+    const syncedSkillDir = path.join(targetWorkspace, "skills", skillName);
+    expect(await pathExists(path.join(syncedSkillDir, "SKILL.md"))).toBe(true);
+    expect((await fs.lstat(syncedSkillDir)).isSymbolicLink()).toBe(false);
+    expect(await pathExists(path.join(targetWorkspace, "skills", ".hidden-target"))).toBe(false);
+    expect(
+      buildWorkspaceSkillsPrompt(targetWorkspace, {
+        bundledSkillsDir: path.join(targetWorkspace, ".bundled"),
+        managedSkillsDir: path.join(targetWorkspace, ".managed"),
+        skillFilter: [skillName],
+      }),
+    ).toContain("Managed symlink target");
+  });
 });
diff --git a/src/agents/skills/types.ts b/src/agents/skills/types.ts
@@ -81,6 +81,8 @@ export type SkillEntry = {
   metadata?: OpenClawSkillMetadata;
   invocation?: SkillInvocationPolicy;
   exposure?: SkillExposure;
+  syncSourceDir?: string;
+  syncDirName?: string;
 };
 
 export type SkillEligibilityContext = {
diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts
@@ -154,10 +154,13 @@ type ResolvedSkillsLimits = {
 type LoadedSkillRecord = {
   skill: Skill;
   frontmatter?: ParsedSkillFrontmatter;
+  syncSourceDir?: string;
+  syncDirName?: string;
 };
 
 type CandidateSkillDir = {
   skillDir: string;
+  skillDirRealPath: string;
   name: string;
   skillMdRealPath: string;
 };
@@ -388,16 +391,53 @@ function loadContainedSkillRecords(params: {
   skillDir: string;
   source: string;
   maxSkillFileBytes: number;
+  canonicalSkillDir?: string;
 }): LoadedSkillRecord[] {
   const expectedBaseDir = path.resolve(params.skillDir);
   const loaded = loadSkillsFromDirSafe({
     dir: params.skillDir,
     source: params.source,
     maxBytes: params.maxSkillFileBytes,
   });
-  return unwrapLoadedSkillRecords(loaded).filter(
+  const records = unwrapLoadedSkillRecords(loaded).filter(
     (record) => path.resolve(record.skill.baseDir) === expectedBaseDir,
   );
+  const canonicalSkillDir = params.canonicalSkillDir;
+  return canonicalSkillDir
+    ? records.map((record) => canonicalizeLoadedSkillRecord(record, canonicalSkillDir))
+    : records;
+}
+
+function canonicalizeLoadedSkillRecord(
+  record: LoadedSkillRecord,
+  canonicalSkillDir: string,
+): LoadedSkillRecord {
+  const originalBaseDir = path.resolve(record.skill.baseDir);
+  const canonicalBaseDir = path.resolve(canonicalSkillDir);
+  if (originalBaseDir === canonicalBaseDir) {
+    return record;
+  }
+  const filePath = path.join(
+    canonicalBaseDir,
+    path.relative(originalBaseDir, record.skill.filePath),
+  );
+  return {
+    ...record,
+    syncSourceDir: canonicalBaseDir,
+    syncDirName: path.basename(originalBaseDir),
+    skill: {
+      ...record.skill,
+      filePath,
+      baseDir: canonicalBaseDir,
+      sourceInfo: record.skill.sourceInfo
+        ? {
+            ...record.skill.sourceInfo,
+            path: filePath,
+            baseDir: canonicalBaseDir,
+          }
+        : record.skill.sourceInfo,
+    },
+  };
 }
 
 function isPathInsideAnyRoot(rootRealPaths: readonly string[], candidateRealPath: string): boolean {
@@ -437,6 +477,10 @@ function resolveSkillRootCandidatePath(params: {
   });
 }
 
+function canonicalSkillDirForSource(source: string, skillDirRealPath: string): string | undefined {
+  return shouldEnforceConfiguredSkillRootContainment(source) ? undefined : skillDirRealPath;
+}
+
 function resolveSkillFilePath(params: {
   source: string;
   skillDir: string;
@@ -624,6 +668,7 @@ function loadSkillEntries(
         skillDir: baseDir,
         source: params.source,
         maxSkillFileBytes: limits.maxSkillFileBytes,
+        canonicalSkillDir: canonicalSkillDirForSource(params.source, baseDirRealPath),
       });
     }
 
@@ -658,7 +703,12 @@ function loadSkillEntries(
     }
 
     const loadedSkills: LoadedSkillRecord[] = [];
-    const loadCandidateSkill = ({ skillDir, name, skillMdRealPath }: CandidateSkillDir) => {
+    const loadCandidateSkill = ({
+      skillDir,
+      skillDirRealPath,
+      name,
+      skillMdRealPath,
+    }: CandidateSkillDir) => {
       try {
         const size = fs.statSync(skillMdRealPath).size;
         if (size > limits.maxSkillFileBytes) {
@@ -679,6 +729,7 @@ function loadSkillEntries(
           skillDir,
           source: params.source,
           maxSkillFileBytes: limits.maxSkillFileBytes,
+          canonicalSkillDir: canonicalSkillDirForSource(params.source, skillDirRealPath),
         }),
       );
     };
@@ -707,7 +758,7 @@ function loadSkillEntries(
           candidatePath: skillMd,
         });
         if (skillMdRealPath) {
-          loadCandidateSkill({ skillDir, name, skillMdRealPath });
+          loadCandidateSkill({ skillDir, skillDirRealPath, name, skillMdRealPath });
         }
       } else {
         // No SKILL.md here — check one level deeper for grouped skill directories.
@@ -764,6 +815,7 @@ function loadSkillEntries(
             if (nestedDirRealPath && nestedSkillMdRealPath) {
               loadCandidateSkill({
                 skillDir: nestedDir,
+                skillDirRealPath: nestedDirRealPath,
                 name: `${name}/${nestedName}`,
                 skillMdRealPath: nestedSkillMdRealPath,
               });
@@ -884,6 +936,8 @@ function loadSkillEntries(
         frontmatter,
         metadata: resolveOpenClawMetadata(frontmatter),
         invocation,
+        ...(record.syncSourceDir !== undefined ? { syncSourceDir: record.syncSourceDir } : {}),
+        ...(record.syncDirName !== undefined ? { syncDirName: record.syncDirName } : {}),
         exposure: {
           includeInRuntimeRegistry: true,
           // Freshly loaded entries preserve the documented disable-model-invocation
@@ -1169,7 +1223,9 @@ function resolveSyncedSkillDestinationPath(params: {
   entry: SkillEntry;
   usedDirNames: Set<string>;
 }): string | null {
-  const sourceDirName = path.basename(params.entry.skill.baseDir).trim();
+  const sourceDirName = (
+    params.entry.syncDirName ?? path.basename(params.entry.skill.baseDir)
+  ).trim();
   if (!sourceDirName || sourceDirName === "." || sourceDirName === "..") {
     return null;
   }
@@ -1233,7 +1289,7 @@ export async function syncSkillsToWorkspace(params: {
         continue;
       }
       try {
-        await fsp.cp(entry.skill.baseDir, dest, {
+        await fsp.cp(entry.syncSourceDir ?? entry.skill.baseDir, dest, {
           recursive: true,
           force: true,
           filter: (src) => {
