fix: return isolated copies from readLock cache

[oritwoen]

readLock returned the cached SkilldLock directly on cache hits but a shallow copy on first parse. removeLockEntry does delete lock.skills[name] on the result - on cached path, that mutated the internal lockCache map.

readConfig in the same module already copies consistently. One-line fix to align readLock with the same pattern, plus a test proving cache isolation across multiple reads.

Summary by CodeRabbit

• Bug Fixes
  • Fixed lock file read operations to return independent copies, ensuring that modifications to returned data do not affect subsequent reads.

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified readLock() to return shallow cloned lock objects from cache instead of the cached object directly, preventing callers from mutating the internal cache. Added a unit test verifying that cached reads return isolated copies that are independently mutable without affecting subsequent reads.

Changes

Cohort / File(s)	Summary
Cache isolation fix src/core/lockfile.ts	Modified cached entry return to use shallow clone ({ skills: { ...cached.skills } }) instead of returning the cached object directly, ensuring external mutations don't affect the internal cache.
Cache isolation test test/unit/lockfile.test.ts	Added test readLock returns isolated copies from cache that verifies cached results are independently mutable across multiple calls and mutations don't propagate between returned objects.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• perf: cache repeated fs reads for package.json, config, and lockfile #50: Directly related—both PRs modify readLock() cache behavior to ensure defensive copying of cached lock data.

Suggested reviewers

• harlan-zw

Poem

🐰 A cache that clones, protects the crown,
No mutations sneak the secrets down,
Each caller reads their own fresh copy,
The original stays safe and poppy! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: modifying readLock to return isolated copies from cache instead of returning cached objects directly.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/readlock-cache-mutation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/core/lockfile.ts`:
- Line 77: The current return in readLock (return { skills: { ...cached.skills }
}) only shallow-copies the skills map so SkillInfo objects remain shared; update
readLock to deep-clone each SkillInfo entry from cached.skills before returning
(e.g., iterate keys of cached.skills and clone each SkillInfo object and any
nested arrays/objects) so mutations to the returned skills do not affect
lockCache; reference the symbols readLock, SkillInfo, cached.skills, and
lockCache when implementing the deep clone.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7ac130d9-d5c8-4a12-b2b5-f7d4b6fe9bb5

📥 Commits

Reviewing files that changed from the base of the PR and between 1c18fde and ff70a80.

📒 Files selected for processing (2)

• src/core/lockfile.ts
• test/unit/lockfile.test.ts

[oritwoen]

About coderabbit:

Checked all readLock callers across the codebase - none mutate SkillInfo properties directly. They either read, replace whole entries (lock.skills[name] = newInfo), or delete keys from the dict. Shallow copy of the skills map is sufficient here, same pattern readConfig uses for its cache in the same module.