feat(skills): track upstream for user-imported GitHub skills

docs/skills.md

@@ -34,6 +34,29 @@ Pick the target scope (user or project) and NBI fetches, validates, and installs
 
 GitHub auth for imports uses, in order: `GITHUB_TOKEN` → `GH_TOKEN` → `gh` CLI auth. Public-repo imports work without auth.
 
+## Tracking upstream for user-imported skills
+
+The Import-from-GitHub dialog has a **Track upstream** checkbox. When you tick it, the installed skill is stamped with a `tracks_upstream: true` frontmatter flag and the Skills panel shows a per-skill **Sync** button (↻) and a panel-level **Sync tracking skills** button.
+
+Tracking is opt-in per skill and entirely manual. Clicking Sync probes GitHub for the latest commit at the recorded `source` URL, and:
+
+- If the commit SHA matches the last recorded `tracking_ref`, the bundle is left alone (no tarball fetch, no rewrite).
+- Otherwise NBI fetches the new bundle, replaces the bundle directory on disk, and stamps the new `tracking_ref`.
+
+The bundle on disk is **never deleted** by a sync action, even when:
+
+- The GitHub commits-API probe fails (network outage, rate limit). Sync raises a visible error and the existing bundle stays in place.
+- The tarball fetch fails after a successful probe. The rmtree only happens after staging succeeds, so a mid-fetch failure leaves the previous version intact.
+- The upstream repo or subpath disappears entirely. Sync errors; the bundle stays.
+
+A tracking skill is the user's. You can still edit its `SKILL.md`, rename it, delete it. The next Sync overwrites local edits the same way `git pull` would.
+
+Tracking is mutually exclusive with the org-managed flag: a skill that the reconciler installed via the org manifest cannot also be set to track upstream from the user-skill side. To migrate, remove it from the manifest first.
+
+To toggle tracking on a skill you already imported (without the checkbox), open the skill in the editor. A **Track upstream** toggle appears for any non-managed skill with a recorded source URL. Toggling off removes the `tracks_upstream` and `tracking_ref` frontmatter and the Sync button disappears; the bundle becomes an ordinary user-authored skill.
+
+The admin policy `allow_github_skill_import = False` blocks sync too: the same network egress is gated by the same flag, so an admin who disables imports also disables sync.
+
 ## Managed skills via an org manifest
 
 For organization-wide deployments (e.g., Kubeflow notebooks), NBI can install and keep a curated set of Claude skills in sync from a YAML or JSON manifest. Skills installed this way are marked **Managed** in the UI — they are read-only (edit, rename, and delete are disabled) and refreshed on a schedule.

notebook_intelligence/extension.py

@@ -1134,6 +1134,11 @@ class SkillsBaseHandler(PolicyGatedHandler):
     exception_status_map = {
         FileExistsError: 409,
         FileNotFoundError: 404,
+        # RuntimeError is reserved for "upstream unreachable" in the sync
+        # path. 502 reflects the wire reality (we proxied to GitHub and
+        # got nothing usable) rather than the default 400 fallback, which
+        # would mislead clients into thinking the request was malformed.
+        RuntimeError: 502,
     }
 
     @property
@@ -1214,13 +1219,24 @@ def put(self, scope, name):
         data = self._parse_json_body()
         if data is None:
             return
+        # The `tracks_upstream` toggle opts a skill into a future sync
+        # action, which would phone home to GitHub. `allow_github_skill_import
+        # = False` is the network-egress kill switch: blocking sync without
+        # also blocking the toggle would let the bit get set on disk while
+        # the admin's kill switch is supposed to be in effect.
+        if "tracks_upstream" in data and self._reject_if_github_import_disabled():
+            return
         try:
             skill = self.skill_manager.update_skill(
                 scope=scope,
                 name=name,
                 description=data.get("description"),
                 allowed_tools=data.get("allowed_tools"),
                 body=data.get("body"),
+                # None preserves the current value; explicit True/False applies.
+                # The manager rejects invalid combinations (managed + tracking,
+                # no source + tracking).
+                tracks_upstream=data.get("tracks_upstream"),
             )
             self.finish(json.dumps({"skill": skill.to_dict(include_body=True)}))
         except (FileNotFoundError, ValueError) as e:
@@ -1277,12 +1293,96 @@ def post(self):
                 scope=scope,
                 name_override=data.get("name"),
                 overwrite=bool(data.get("overwrite", False)),
+                tracks_upstream=bool(data.get("tracks_upstream", False)),
             )
             self.finish(json.dumps({"skill": skill.to_dict(include_body=True)}))
         except (FileExistsError, FileNotFoundError, ValueError) as e:
             self._error(e)
 
 
+class SkillSyncHandler(SkillsBaseHandler):
+    """Re-fetch a single user-imported skill that has opted into tracking.
+
+    Gated by the same `allow_github_skill_import` policy as the initial
+    import: sync is the same network egress with the same trust
+    boundary, and an admin who's disabled imports does not want sync
+    silently keeping skills fresh on the side.
+    """
+
+    @tornado.web.authenticated
+    async def post(self, scope, name):
+        if self._reject_if_github_import_disabled():
+            return
+        # The sync action does blocking HTTP (commits-API probe plus
+        # optional tarball download). Run off the event loop so the
+        # Tornado IO thread keeps serving other requests.
+        loop = asyncio.get_event_loop()
+        try:
+            result = await loop.run_in_executor(
+                None,
+                lambda: self.skill_manager.sync_tracking_skill(scope, name),
+            )
+            self.finish(json.dumps(result))
+        except (FileNotFoundError, ValueError, RuntimeError) as e:
+            self._error(e)
+
+
+class SkillsSyncAllTrackingHandler(SkillsBaseHandler):
+    """Batch sync every skill that has `tracks_upstream` enabled.
+
+    Returns per-skill results so the UI can show which ones updated,
+    which were unchanged, and which failed. Failures are isolated per
+    skill: a single broken upstream doesn't stop the rest of the sync.
+    """
+
+    # Bound concurrency on the GitHub commits-API probe to keep a
+    # 10-tracking-skill click from exhausting the unauthenticated 60/hour
+    # rate limit in a single burst. Three keeps the probes civil while
+    # collapsing serial worst-case (N * 15s) to roughly N/3 * 15s.
+    SYNC_CONCURRENCY = 3
+
+    @tornado.web.authenticated
+    async def post(self):
+        if self._reject_if_github_import_disabled():
+            return
+        loop = asyncio.get_event_loop()
+        skills = await loop.run_in_executor(
+            None, self.skill_manager.list_tracking_skills
+        )
+        if not skills:
+            self.finish(json.dumps({"results": []}))
+            return
+
+        sem = asyncio.Semaphore(self.SYNC_CONCURRENCY)
+
+        async def sync_one(skill):
+            async with sem:
+                try:
+                    # `lambda s=skill:` captures the skill at iteration
+                    # time so all coroutines don't share the same loop
+                    # variable.
+                    outcome = await loop.run_in_executor(
+                        None,
+                        lambda s=skill: self.skill_manager.sync_tracking_skill(
+                            s.scope, s.name
+                        ),
+                    )
+                    return {
+                        "scope": skill.scope,
+                        "name": skill.name,
+                        **outcome,
+                    }
+                except Exception as e:  # noqa: BLE001 — per-skill isolation
+                    return {
+                        "scope": skill.scope,
+                        "name": skill.name,
+                        "error": str(e),
+                    }
+
+        results = await asyncio.gather(*(sync_one(s) for s in skills))
+        self.finish(json.dumps({"results": results}))
+
+
 class SkillsReconcileHandler(SkillsBaseHandler):
     """Manual trigger for the managed-skills reconciler."""
 
@@ -2701,6 +2801,8 @@ def _setup_handlers(self, web_app, feature_policies: dict, string_overrides: dic
         route_pattern_skills_import_preview = url_path_join(base_url, "notebook-intelligence", "skills", "import", "preview")
         route_pattern_skills_import = url_path_join(base_url, "notebook-intelligence", "skills", "import")
         route_pattern_skills_reconcile = url_path_join(base_url, "notebook-intelligence", "skills", "reconcile")
+        route_pattern_skills_sync_all = url_path_join(base_url, "notebook-intelligence", "skills", "sync-all-tracking")
+        route_pattern_skill_sync = url_path_join(base_url, "notebook-intelligence", "skills", r"(user|project)", skill_name, "sync")
         route_pattern_skills_reconciler_stop = url_path_join(
             base_url, "notebook-intelligence", "skills", "reconciler", "stop"
         )
@@ -2819,6 +2921,8 @@ def _setup_handlers(self, web_app, feature_policies: dict, string_overrides: dic
             (route_pattern_skills_import_preview, SkillsImportPreviewHandler),
             (route_pattern_skills_import, SkillsImportHandler),
             (route_pattern_skills_reconcile, SkillsReconcileHandler),
+            (route_pattern_skills_sync_all, SkillsSyncAllTrackingHandler),
+            (route_pattern_skill_sync, SkillSyncHandler),
             # Deliberately not gated by SkillsBaseHandler — the kill switch
             # must remain reachable while skills_management_policy=force-off
             # is the active state. See the handler docstring.

notebook_intelligence/skill_github_import.py

@@ -234,6 +234,29 @@ def _fetch_tarball(
     return data
 
 
+_SHA_RE = re.compile(r"^[0-9a-f]{40}$")
+
+
+def resolve_desired_sha(
+    ref_info: "GitHubRef", *, token: Optional[str] = None
+) -> Optional[str]:
+    """Resolve the SHA the caller should treat as "latest" for this ref.
+
+    Short-circuits the commits-API probe when the URL already pins a
+    full 40-char SHA (no point round-tripping GitHub to confirm a SHA
+    we already have). Otherwise probes `get_latest_commit_sha`; returns
+    None on any probe failure so the caller can decide whether to fall
+    back to a tarball fetch or surface the outage. Shared between the
+    managed-skills reconciler and the user-skill track-upstream sync
+    path so both paths agree on what "up to date" means.
+    """
+    if ref_info.ref and _SHA_RE.match(ref_info.ref):
+        return ref_info.ref
+    return get_latest_commit_sha(
+        ref_info.owner, ref_info.repo, ref_info.ref, ref_info.subpath, token=token
+    )
+
+
 def get_latest_commit_sha(
     owner: str,
     repo: str,

notebook_intelligence/skill_manager.py

@@ -12,7 +12,11 @@
     SkillScope,
     serialize_skill_md,
 )
-from notebook_intelligence.skill_github_import import stage_skill_from_github
+from notebook_intelligence.skill_github_import import (
+    parse_github_url,
+    resolve_desired_sha,
+    stage_skill_from_github,
+)
 
 log = logging.getLogger(__name__)
 
@@ -202,6 +206,7 @@ def update_skill(
         description: Optional[str] = None,
         allowed_tools: Optional[List[str]] = None,
         body: Optional[str] = None,
+        tracks_upstream: Optional[bool] = None,
     ) -> Skill:
         skill = self.get_skill(scope, name)
         if skill is None:
@@ -211,6 +216,28 @@ def update_skill(
         new_allowed_tools = allowed_tools if allowed_tools is not None else skill.allowed_tools
         new_body = body if body is not None else skill.body
 
+        # `tracks_upstream=None` means "leave unchanged" — caller didn't touch
+        # the toggle. Explicit True/False both apply. Managed skills cannot
+        # opt into tracking; the two metadata pairs are mutually exclusive.
+        if tracks_upstream is None:
+            new_tracks_upstream = skill.tracks_upstream
+        else:
+            if tracks_upstream and skill.managed:
+                raise ValueError(
+                    "Managed skills cannot track upstream; remove from the "
+                    "manifest first."
+                )
+            if tracks_upstream and not skill.source:
+                raise ValueError(
+                    f"Skill '{name}' has no recorded source URL; can only "
+                    "track upstream for GitHub-imported skills."
+                )
+            new_tracks_upstream = bool(tracks_upstream)
+        # Toggling off clears tracking_ref so a future re-opt-in starts
+        # from a clean slate; the existing bundle on disk is otherwise
+        # untouched and becomes a plain user-authored skill again.
+        new_tracking_ref = skill.tracking_ref if new_tracks_upstream else ""
+
         md_content = serialize_skill_md(
             name,
             new_description,
@@ -219,6 +246,8 @@ def update_skill(
             source=skill.source,
             managed_source=skill.managed_source,
             managed_ref=skill.managed_ref,
+            tracks_upstream=new_tracks_upstream,
+            tracking_ref=new_tracking_ref,
         )
         skill.skill_md_path().write_text(md_content, encoding="utf-8")
 
@@ -234,6 +263,8 @@ def update_skill(
             source=skill.source,
             managed_source=skill.managed_source,
             managed_ref=skill.managed_ref,
+            tracks_upstream=new_tracks_upstream,
+            tracking_ref=new_tracking_ref,
         )
         self._notify_skills_changed()
         return updated
@@ -263,6 +294,8 @@ def rename_skill(self, scope: SkillScope, old_name: str, new_name: str) -> Skill
             source=skill.source,
             managed_source=skill.managed_source,
             managed_ref=skill.managed_ref,
+            tracks_upstream=skill.tracks_upstream,
+            tracking_ref=skill.tracking_ref,
         )
         (new_bundle_dir / SKILL_ENTRY_FILE).write_text(md_content, encoding="utf-8")
 
@@ -359,8 +392,16 @@ def import_from_github(
         scope: SkillScope,
         name_override: Optional[str] = None,
         overwrite: bool = False,
+        tracks_upstream: bool = False,
     ) -> Skill:
-        """Fetch, validate, and install a skill from GitHub into the given scope."""
+        """Fetch, validate, and install a skill from GitHub into the given scope.
+
+        When ``tracks_upstream=True``, stamps the skill with a `tracks_upstream`
+        frontmatter flag so a later sync action can re-fetch from the same
+        URL. The initial install doesn't record a `tracking_ref` because
+        the staging API doesn't surface the resolved commit SHA; the next
+        sync probes the commits API and stamps it.
+        """
         staged = stage_skill_from_github(url)
         try:
             name = name_override.strip() if name_override else staged.name
@@ -386,6 +427,7 @@ def import_from_github(
                 staged.allowed_tools,
                 staged.body,
                 source=staged.canonical_url,
+                tracks_upstream=tracks_upstream,
             )
             (target_dir / SKILL_ENTRY_FILE).write_text(md_content, encoding="utf-8")
 
@@ -395,6 +437,77 @@ def import_from_github(
         finally:
             shutil.rmtree(staged.tmp_root, ignore_errors=True)
 
+    def sync_tracking_skill(self, scope: SkillScope, name: str) -> dict:
+        """Re-fetch a tracking skill from its recorded GitHub source.
+
+        Returns a status dict ``{updated: bool, ref: str|None}``. Raises
+        ``FileNotFoundError`` if the skill doesn't exist, ``ValueError`` if
+        the skill isn't tracking-eligible (not opted in, no source URL, or
+        managed). Network/parse failures bubble up so the HTTP layer can
+        surface them as errors; the existing bundle on disk is left intact
+        on any error path (the rmtree happens only after staging succeeds).
+        """
+        skill = self.get_skill(scope, name)
+        if skill is None:
+            raise FileNotFoundError(f"Skill '{name}' not found in {scope} scope")
+        if skill.managed:
+            raise ValueError(
+                f"Skill '{name}' is managed by the org manifest; sync is "
+                "handled by the reconciler, not the track-upstream button."
+            )
+        if not skill.tracks_upstream:
+            raise ValueError(
+                f"Skill '{name}' is not tracking upstream; enable tracking "
+                "before syncing."
+            )
+        if not skill.source:
+            raise ValueError(
+                f"Skill '{name}' has no recorded source URL; cannot sync."
+            )
+
+        # SHA probe first. If the commits API returns a SHA equal to the
+        # last-synced ref, skip the tarball fetch entirely — saves bandwidth
+        # and avoids an unnecessary rmtree/copytree of an unchanged bundle.
+        # If the probe fails (None), surface that as an error rather than
+        # silently re-downloading: the user clicked the button expecting a
+        # fresh check, and a probe-then-fetch dance would mask real outages.
+        ref_info = parse_github_url(skill.source)
+        desired_sha = resolve_desired_sha(ref_info)
+        if desired_sha is None:
+            raise RuntimeError(
+                "Could not probe GitHub for the latest commit; the existing "
+                "skill is unchanged. Try again later."
+            )
+        if skill.tracking_ref == desired_sha:
+            return {"updated": False, "ref": desired_sha}
+
+        staged = stage_skill_from_github(skill.source)
+        try:
+            # Stage succeeded; replacing the bundle is the destructive step.
+            # We preserve the on-disk name (the user may have renamed) and
+            # write the tracking pair back so frontmatter stays consistent.
+            shutil.rmtree(skill.root_path)
+            shutil.copytree(staged.skill_root, skill.root_path)
+            md_content = serialize_skill_md(
+                skill.name,
+                staged.description,
+                staged.allowed_tools,
+                staged.body,
+                source=staged.canonical_url,
+                tracks_upstream=True,
+                tracking_ref=desired_sha,
+            )
+            skill.skill_md_path().write_text(md_content, encoding="utf-8")
+        finally:
+            shutil.rmtree(staged.tmp_root, ignore_errors=True)
+
+        self._notify_skills_changed()
+        return {"updated": True, "ref": desired_sha}
+
+    def list_tracking_skills(self) -> List[Skill]:
+        """Return only installed skills with `tracks_upstream` set."""
+        return [s for s in self.list_skills() if s.tracks_upstream]
+
     def install_managed_from_github(
         self,
         url: str,