Skip to content

AiSettingsNodeType.AddSkillSource is fire-and-forget — a fast uninstall can be undone by a late install write #683

Closed

Description

@rbuergi

The race

AiSettingsNodeType.AddSkillSource(hub, services, user, sourceNamespace) returns void and subscribes to its own write internally:

hub.GetWorkspace()
   .GetQuery(...)
   .Take(1)
   .SelectMany(nodes => meshService.CreateOrUpdateNode(target with { Content = merged }))
   .Subscribe(_ => { }, ex => logger.LogWarning(...));

The caller gets no completion signal, so the Store's install plan can write its manifest — and report the install finished — before the settings update has landed.

Sequence that loses the fix:

  1. install → AddSkillSource subscribes, write in flight
  2. install plan writes the manifest, reports success
  3. viewer uninstalls promptly → deregistration finds no row to remove, completes cleanly, clears the record
  4. the step-1 write lands → the viewer now has a skill source for a package whose nodes are gone, and no record that says they ever installed it

The result is exactly the stale composer source that MeshWeaver.Plugins#199 set out to eliminate, reachable whenever install and uninstall happen close together.

Why it isn't fixed in #199

That PR fixes the uninstall side (withdrawal now propagates failure instead of clearing the record on a silent miss). This one is the install side, and fixing it properly means giving AddSkillSource a completion signal — an IObservable<Unit> overload the install plan can sequence on, so registration is a phase like every other rather than a side effect racing the manifest.

That is a core API change with its own semantics and tests; smuggling it into an uninstall PR would have been the wrong shape.

Suggested fix

Add an observable-returning overload (keeping the void one for existing fire-and-forget callers) and have Localizer's Register skills phase sequence on it, so the manifest — which is deliberately the LAST phase — cannot be written before the skill source is actually registered.

Found by the Copilot review on MeshWeaver.Plugins#199.

Activity

  1. added 2 commits that reference this issue on Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions