#340: Prune orphan prompt values on pack removal#342
Merged
Conversation
- Drop resolvedValues entries whose keys no surviving pack declares, so later packs declaring the same key are asked fresh instead of inheriting a stale prior from a removed pack. - Apply pruning inside unconfigurePack so both mcs sync deselection and mcs pack remove federated cleanup benefit from the same logic. - Cover orphan removal and shared-key retention with lifecycle tests.
There was a problem hiding this comment.
Pull request overview
Prunes orphaned prompt resolvedValues from persisted project/global state when a tech pack is deselected/removed, preventing stale cross-pack prompt reuse (and improving credential hygiene for prompt-stored secrets).
Changes:
- Prune
ProjectState.resolvedValuesafter pack unconfiguration to keep only keys still declared by remaining configured packs. - Add
ProjectState.pruneResolvedValues(keepingKeys:)helper to encapsulate the pruning behavior. - Add lifecycle integration tests covering orphan pruning and shared-key retention when removing one of multiple declaring packs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Tests/MCSTests/LifecycleIntegrationTests.swift | Adds lifecycle tests asserting orphan prompt values are removed on pack deselection and retained when still declared by another pack. |
| Sources/mcs/Sync/Configurator.swift | Invokes pruning after unconfigurePack completes (including the “no artifact record” path) and computes surviving declared prompt keys. |
| Sources/mcs/Core/ProjectState.swift | Introduces pruneResolvedValues(keepingKeys:) to drop resolved values not in a provided key set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Skip pruning entirely when any configured pack is unresolvable from the registry, matching ResourceRefCounter's conservative fallback. - Cover the fail-safe with a lifecycle test that drives unconfigurePack with a narrowed registry. - Drop incidental WHAT comments in the new tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Closes #340. After #339 introduced prompt-value reuse on re-sync, removing a pack leaves its prompt keys behind in project state. A later-installed pack that happens to declare the same key (realistic collisions:
BRANCH_PREFIX,LABEL_PREFIX,DEFAULT_ASSIGNEE) sees the removed pack's answer as a "prior" in the reuse gate — surfacing a value the current pack selection never authored. Prompt values can also hold MCP env vars (API keys, tokens), so orphan retention is a minor credential-hygiene issue in addition to the surprise factor.Changes
mcs syncdeselecting a pack, andmcs pack remove(which unconfigures per affected scope).Test plan
.claude/.mcs-projectand confirm the key is gone.mcs pack remove— confirm the shared value is retained because the remaining pack still declares it.