Found while adding the regression test for #373.
Problem
UpdateCommand's re-apply phase computes each scope's pack list as
configuredPackIDs.subtracting(skippedPackIDs) and hands it to
Configurator.configure(confirmRemovals: false). configure treats that list as the complete
desired state: every previously-configured pack missing from it is unconfigured, and on this
path there is no prompt. A pack that was skipped by the update phase therefore gets removed
from the scope, together with its artifacts, as a side effect of mcs update.
A pack lands in skipped when its fetch failed (network down, remote gone), when the user
declined to trust its new scripts, when its registry path is invalid, or when it is tracked in
state but missing from the registry (unresolved).
Repro
# two packs configured in the same scope (project or global)
mcs sync --pack a --pack b
# make one of them fail to update: unplug the network, delete its remote, or decline trust
mcs update
# pack b is unconfigured from the scope; its hooks/skills/settings entries are gone
With a single configured pack the empty-list guard (No packs to refresh in this scope.) masks
it. It only bites with two or more packs in the scope.
Why it exists
The exclusion is intentional: after a declined trust prompt the checkout on disk is already at
the new commit, so re-applying would install the untrusted scripts. The mechanism chosen for
"do not re-install this pack" is the wrong one, though, because filtering the desired set is
indistinguishable from the user deselecting the pack.
This is the third occurrence of the same class of mistake on the convergence engine: an earlier
mcs update --pack iteration and the original design for the #369 global-pack block both
filtered the list before configure and silently uninstalled the rest.
Suggested fix
Do not pass a subset. If any of a scope's packs is in skipped, skip re-apply for the whole
scope and warn naming the pack(s) and the reason, so the next successful mcs update refreshes
everything at once. Same treatment for unresolved. This needs no engine change.
UpdateCommand.reapplyScope (introduced by the #373 PR) is directly testable from
LifecycleIntegrationTests, so the fix can carry a two-pack regression test.
Related: #373, #346, #369.
Found while adding the regression test for #373.
Problem
UpdateCommand's re-apply phase computes each scope's pack list asconfiguredPackIDs.subtracting(skippedPackIDs)and hands it toConfigurator.configure(confirmRemovals: false).configuretreats that list as the completedesired state: every previously-configured pack missing from it is unconfigured, and on this
path there is no prompt. A pack that was skipped by the update phase therefore gets removed
from the scope, together with its artifacts, as a side effect of
mcs update.A pack lands in
skippedwhen its fetch failed (network down, remote gone), when the userdeclined to trust its new scripts, when its registry path is invalid, or when it is tracked in
state but missing from the registry (
unresolved).Repro
With a single configured pack the empty-list guard (
No packs to refresh in this scope.) masksit. It only bites with two or more packs in the scope.
Why it exists
The exclusion is intentional: after a declined trust prompt the checkout on disk is already at
the new commit, so re-applying would install the untrusted scripts. The mechanism chosen for
"do not re-install this pack" is the wrong one, though, because filtering the desired set is
indistinguishable from the user deselecting the pack.
This is the third occurrence of the same class of mistake on the convergence engine: an earlier
mcs update --packiteration and the original design for the #369 global-pack block bothfiltered the list before
configureand silently uninstalled the rest.Suggested fix
Do not pass a subset. If any of a scope's packs is in
skipped, skip re-apply for the wholescope and warn naming the pack(s) and the reason, so the next successful
mcs updaterefresheseverything at once. Same treatment for
unresolved. This needs no engine change.UpdateCommand.reapplyScope(introduced by the #373 PR) is directly testable fromLifecycleIntegrationTests, so the fix can carry a two-pack regression test.Related: #373, #346, #369.