Skip to content

Don't mutate the caller's managed map in BuildPlans#89

Open
Soph wants to merge 1 commit into
mainfrom
fix/buildplans-managed-map-mutation
Open

Don't mutate the caller's managed map in BuildPlans#89
Soph wants to merge 1 commit into
mainfrom
fix/buildplans-managed-map-mutation

Conversation

@Soph

@Soph Soph commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Problem

BuildPlans passed the caller's managed map straight into addPruneCandidates, which inserts prune candidates. So a --prune plan mutated a map the caller still owns (and may reuse). BuildReplicationPlans already copied defensively, so the two diverged.

Fix

Copy managed inside the if cfg.Prune block in both functions — that's the only place the map is mutated, so neither touches the caller's map, both stay symmetric, and the copy is skipped entirely when not pruning.

Tests

TestBuildPlansDoesNotMutateManagedMap runs a --prune plan with a stale target ref, confirms it's planned for deletion (prune actually ran), and asserts the caller's managed map is unchanged (the prune candidate landed only in the copy).

🤖 Generated with Claude Code


Note

Low Risk
Localized defensive copy in plan builders with a regression test; behavior for prune/delete plans is unchanged aside from fixing unintended map mutation.

Overview
BuildPlans with --prune no longer mutates the caller’s managed map. Prune candidates from addPruneCandidates are applied only on a copyManagedTargets copy, matching BuildReplicationPlans.

BuildReplicationPlans now copies managed only when cfg.Prune is set (it previously copied on every call). Both builders share the same rule: no copy when not pruning, copy before the only mutation when pruning.

Adds TestBuildPlansDoesNotMutateManagedMap to assert a stale ref gets a delete plan while the caller’s managed map stays length 1 with no leaked prune entry.

Reviewed by Cursor Bugbot for commit cf4957c. Configure here.

BuildPlans passed the caller's managed map straight to addPruneCandidates,
which inserts prune candidates — so a --prune plan mutated a map the caller
still owns. BuildReplicationPlans already copied defensively. Copy inside the
prune branch in both (the only mutation happens there), so neither touches the
caller's map and the two functions stay symmetric.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: f3b681dacd36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant