Conversation
Refresh a developer's UC-downloaded skills before `ug <agent>` launches, so a skill whose Unity Catalog source changed since download is re-downloaded in place without a manual `ug skills add`. The check reuses the download attribution manifest (`~/.ucode/skills.json`). Each install already records the skill's `uc_update_time` at download, and a new top-level `last_update_check` rate-limits the sweep to once every 24 hours, matching Isaac's plugin marketplace staleness window. Only the launching workspace's own non-managed downloads under the home or working directory are eligible; managed skills stay owned by `ug configure`. The sweep is best effort and fails open, so it never blocks a launch. Co-authored-by: Isaac <no-reply@databricks.com>
| working dir qualify. Managed skills are left to ``ug configure``, and other workspaces' | ||
| downloads are skipped because the launch token authenticates only this workspace. | ||
| """ | ||
| bases = {os.path.normpath(str(Path.home())), os.path.normpath(cwd)} |
There was a problem hiding this comment.
Is this true? I think for claude code at least, the entire every directory in the directory path can have valid skills dirs right? Do you think we can just drop the base check for now for simplicity?
There was a problem hiding this comment.
Good call. Dropped the base filter: _eligible_launch_records now refreshes every non-managed download for the current workspace regardless of base, since agents load .claude/skills from directories along the path, not only home and the working dir. Simpler, and it also covers project skills in ancestor dirs.
| ] | ||
|
|
||
|
|
||
| def _stale_launch_refs( |
There was a problem hiding this comment.
nit: can you use a more intuitive name? Maybe "get_updated_refs" sth like that?
There was a problem hiding this comment.
Renamed to _get_updated_refs.
| return pairs | ||
|
|
||
|
|
||
| def _apply_launch_updates(workspace: str, token: str, pairs: list[tuple[dict, SkillRef]]) -> int: |
There was a problem hiding this comment.
nit: maybe call this update_stale_skills or something more intuitive and human understandable?
There was a problem hiding this comment.
Renamed to _update_stale_skills.
| print_warning(f"Skipping `{ref.fqn}`: {reason}.") | ||
| continue | ||
| try: | ||
| write_skill(roots, ref, files) |
There was a problem hiding this comment.
So this write_skill overwrite the existing files, right?
There was a problem hiding this comment.
Yes. write_skill calls write_bytes per file, so it overwrites the bundle in place. One nuance: it writes the files present in the new bundle but does not remove files deleted upstream, so such a file would linger until the bundle dir name changes (that path is already reconciled). This is shared behavior across all download paths. Happy to add a clean-before-write in a follow-up if we want strict mirroring.
… records - Drop the home/cwd base filter; refresh all of the current workspace's non-managed downloads, since agents load skills from directories along the path, not only home and the working dir. - Forget a downloaded skill's record when the user deleted its directories, rather than re-downloading it. - Extract the shared fetch-and-write core (_fetch_and_write), now used by the download, managed-reconcile, and launch-refresh paths. - Rename _stale_launch_refs to _get_updated_refs and _apply_launch_updates to _update_stale_skills. Co-authored-by: Isaac <no-reply@databricks.com>
| # --- Launch-time refresh --------------------------------------------------- | ||
|
|
||
|
|
||
| def _eligible_launch_records(records: list[dict], workspace: str) -> list[dict]: |
There was a problem hiding this comment.
nit: better function name: maybe eligible_launch_refresh_records?
|
Superseded by a 2-PR stack (splitting the
Both carry every change from this PR plus the review follow-ups (the two renames, forgetting hand-deleted records, and the shared fetch/write helper). Closing here in favor of the stack. |
1 participant
What did you change, and why?
Change:
ug <agent>now refreshes a developer's UC-downloaded skills before the agent starts. At launch it reads the download attribution manifest, and for each of the current workspace's own (non-managed) downloads under the home or working directory it calls GetSkill and compares the skill's currentuc_update_timeagainst the value recorded at download. Any skill whose Unity Catalog source is newer is re-downloaded in place, with no overwrite prompt, since only manifest-attributed directories are touched.Why: A downloaded skill previously drifted from its Unity Catalog source until the developer re-ran
ug skills add. This keeps downloads current automatically, the way Isaac keeps installed plugins current.The sweep is rate-limited to once every 24 hours through a new top-level
last_update_checkkey in~/.ucode/skills.json, matching Isaac's plugin marketplace staleness window, so back-to-back launches make no network calls. It is best effort and fails open, so a network, auth, or fetch error is reported and the launch proceeds on whatever is already on disk. It is skipped under--skip-preflight.How do you know it works?
Testing: Added unit tests covering the new manifest accessors and that recording downloads preserves the
last_update_checkstamp (test_skills_state.py), plus the launch path (test_skills_download.py): eligibility filtering (home and working dir, current workspace, managed excluded), stale detection byuc_update_time(newer, equal, older, missing, deleted), the silent in-place overwrite that refreshes both skill roots and the manifest record, and the entry point's rate-limit gate, fail-open behavior, and end-to-end update.uv run ruff check,ruff format --check,ty check src, anduv run pytestall pass locally.This pull request and its description were written by Isaac.