From ff70a8082652ae0e4a65168729a8a48a2ee78e39 Mon Sep 17 00:00:00 2001
From: oritwoen <18102267+oritwoen@users.noreply.github.com>
Date: Mon, 30 Mar 2026 18:21:01 +0200
Subject: [PATCH] fix: return isolated copies from readLock cache

readLock returned the cached object directly on cache hits but a
shallow copy on first parse. Callers like removeLockEntry that
delete keys from the result were mutating the internal cache.
Align with readConfig which already returns copies consistently.
---
 src/core/lockfile.ts       |  2 +-
 test/unit/lockfile.test.ts | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/core/lockfile.ts b/src/core/lockfile.ts
index fe4a6ef5..b5afe06f 100644
--- a/src/core/lockfile.ts
+++ b/src/core/lockfile.ts
@@ -74,7 +74,7 @@ export function invalidateLockCache(skillsDir?: string): void {
 export function readLock(skillsDir: string): SkilldLock | null {
   const cached = lockCache.get(skillsDir)
   if (cached)
-    return cached
+    return { skills: { ...cached.skills } }
   const lockPath = join(skillsDir, 'skilld-lock.yaml')
   if (!existsSync(lockPath))
     return null
diff --git a/test/unit/lockfile.test.ts b/test/unit/lockfile.test.ts
index 2728111e..c6cd3ee6 100644
--- a/test/unit/lockfile.test.ts
+++ b/test/unit/lockfile.test.ts
@@ -236,6 +236,35 @@ describe('core/lockfile', () => {
       expect(written).toContain('generator: external')
     })
 
+    it('readLock returns isolated copies from cache', async () => {
+      const { existsSync, readFileSync } = await import('node:fs')
+      const { readLock } = await import('../../src/core/lockfile')
+
+      vi.mocked(existsSync).mockReturnValue(true)
+      vi.mocked(readFileSync).mockReturnValue(
+        'skills:\n'
+        + '  vue:\n'
+        + '    packageName: vue\n'
+        + '    version: "3.5.0"\n',
+      )
+
+      const first = readLock('/skills')
+      const second = readLock('/skills')
+
+      // Both should parse correctly
+      expect(first?.skills.vue?.packageName).toBe('vue')
+      expect(second?.skills.vue?.packageName).toBe('vue')
+
+      // Mutating first should not affect second (both are isolated copies)
+      delete first!.skills.vue
+      expect(first!.skills.vue).toBeUndefined()
+      expect(second?.skills.vue?.packageName).toBe('vue')
+
+      // A third read should still see the original data
+      const third = readLock('/skills')
+      expect(third?.skills.vue?.packageName).toBe('vue')
+    })
+
     it('writeLock omits git fields when not set', async () => {
       const { existsSync, writeFileSync } = await import('node:fs')
       const { writeLock } = await import('../../src/core/lockfile')