feat(skills): accept comma-separated list of managed-skills manifests

[pjdoland]

Summary

NBI_SKILLS_MANIFEST (and the matching skills_manifest traitlet) now accept a comma-separated list of YAML/JSON manifests. The reconciler loads each independently and unions the entries, with first-wins URL dedupe and per-entry-error name dedupe. Single-manifest deployments behave identically; the wire format is strictly additive.

The use case: a JupyterHub deployment can ship an org-wide baseline manifest plus a per-team manifest in one env var, without baking a single combined file at deploy time.

Solution

Three layers of change, one commit each, plus a follow-up commit applying the convergent review findings.

• load_manifests(sources, *, token) in notebook_intelligence/skill_manifest.py. Iterates sources, catches ManifestError per source so a transient outage on one manifest doesn't block the others, and merges entries with two passes of dedupe:
  • By URL: same url: in two manifests means the earlier one wins; a warning names both manifests.
  • By installed name: predicted via the same _slug() step the real installer applies (subpath casing, whitespace, capitals all normalize), so two URLs that would install to the same name collide before staging. The second entry drops with a per-entry error.
• SkillReconciler now takes manifest_sources: List[str] instead of a single string. When any source fails to load, the cycle skips stale-removal so the missing source's skills don't get falsely orphaned. Per-entry installs still run against whatever did load.
• Wire format resolved through a new util.split_csv helper that both _resolve_csv_appended and the new _resolve_skills_manifest_sources share. The new helper means \" ,,, \" and other all-separator inputs degrade cleanly to "no manifests configured" rather than constructing phantom sources.

Testing

• tests/test_skill_manifest.py: 13 new cases. split_sources (5), load_manifests (8 covering union, URL dedupe with warning, name collision with explicit names, name collision via slugging, non-GitHub URL warning, partial fetch failure both orderings, URLError isolation, all-fail, order preservation, token-per-source).
• tests/test_skill_reconciler.py: 7 new cases in a TestMultipleManifests class. Multi-source union, URL dedupe through the reconciler, name collision through the reconciler, partial failure preserving managed skills (both [good, missing] and [missing, good] orderings), all-sources-fail, empty-sources no-op. Plus 15 existing tests adapted from the single-string signature to the new list signature.
• tests/test_config_integration.py: 7 new cases in TestSkillsManifestResolver pinning the env-vs-traitlet contract end to end, including the all-separator-input edge case.

Full suite green: pytest tests/ --ignore=tests/test_claude_client.py → 908 passed. jlpm tsc --noEmit, jlpm lint:check, jlpm jest all clean.

Risks and follow-ups

• Permanent partial-failure means stale skills accumulate. A manifest source that fails forever pins stale-removal off indefinitely. Documented as accepted behavior in docs/skills.md; a follow-up could add a "force stale removal after K consecutive partial-failure cycles" escape hatch.
• Stop-signal latency. SkillReconciler.stop() can block up to N * FETCH_TIMEOUT_SECONDS if the first manifest fetch hangs. Pre-existing single-manifest behavior; scales linearly with N. Follow-up could thread the stop event through load_manifests so the loop bails between sources.
• Per-source ownership of installed skills. Today Skill.managed_source records the entry URL, not the owning manifest URL. With per-source ownership the reconciler could remove only the skills owned by the loaded manifests on partial failure, instead of skipping removal entirely. That's a schema change in serialize_skill_md and warrants its own PR.
• Token fanout. NBI_MANAGED_SKILLS_TOKEN is shipped as a Bearer header to every manifest URL in the list. Pre-existing single-manifest behavior; the multi-manifest path multiplies the blast radius of a misconfigured host. Worth a one-line caveat in the docs; deferred to keep this PR scoped to the wire-format change.

The full review pass (six agents: code-reuse, code-quality, efficiency, backend/contract, distributed-systems, test-architecture) produced fix-before-merge items that all landed in the final commit. Remaining items above are follow-ups.