fix(snapping): avoid mutating the previous snap log when tagging from scope

[davidfirst]

When tagging from scope with copyLogFromPreviousSnap (the _tag flow), getLog mutated the previous snap's log object in place with the new tag's message and date. Since the consumer component's log is a reference to the cached Version object's log, and that same cached Version is later persisted when marked as hidden (and exported via includeParents), the previous snap ended up stored with the tag's message and timestamp — losing its original ones. Visible in bit log --show-hidden: the hidden merged snap and the tag share the exact same timestamp and message, which also breaks the log ordering between them.

The fix returns a new log object instead of mutating, keeping the intended behavior for the new tag (author attribution from the previous snap, new date) while leaving the previous snap's log intact.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix tagging flow to avoid mutating previous snap log when copying

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Prevent tagging-from-scope from overwriting the previous snap’s stored log metadata.
• Return a new Log object when copying from the previous snap to preserve cache integrity.
• Keep tag attribution behavior while restoring correct log ordering for hidden snaps.

Diagram

graph TD
  A["VersionMaker.addLogToComponents"] --> B["getLog(component)"] --> C["copyLogFromPreviousSnap?"] --> D["Previous snap log (cached)"] --> E["New Log object (copy+override)"] --> F[("Repository persists Version")]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Clone at cache boundary (Version/ConsumerComponent log accessor)

• ➕ Eliminates this class of bugs by preventing shared mutable references broadly
• ➕ Keeps mutation-safe semantics consistent across all callers
• ➖ Wider behavioral surface area change; may have performance implications
• ➖ Harder to audit all consumers relying on reference identity

2. Enforce immutability (freeze) for cached Version models

• ➕ Fails fast on accidental mutation of cached objects
• ➕ Makes ownership boundaries explicit
• ➖ Potential runtime overhead and new exceptions in existing flows
• ➖ Requires broader auditing and possible refactors where mutation is currently relied upon

Recommendation: The PR’s approach (returning a new Log object in the copy-from-previous-snap path) is the best near-term fix: it’s minimal, targets the precise ownership violation (mutating a cached previous Version log), and preserves intended behavior (reuse author attribution while updating date/message for the new tag). Broader cache-boundary cloning or immutability would be more comprehensive but significantly larger in scope.

Files changed (1) +9 / -3

Bug fix (1) +9 / -3

version-maker.tsAvoid in-place mutation when copying log from previous snap +9/-3

Avoid in-place mutation when copying log from previous snap

• In the copyLogFromPreviousSnap path, stop mutating the previous snap’s log object and instead return a new object with overridden message and date. This prevents cached previous Version instances from being accidentally persisted with the tag’s log metadata (e.g., when later marked hidden).

scopes/component/snapping/version-maker.ts

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review