Conversation
xsh310
added this pull request to stack #765
September 20, 2026 01:24
This was referenced Sep 20, 2026
xsh310
marked this pull request as ready for review
September 20, 2026 04:17
xsh310
force-pushed
the
skills-refresh-on-launch
branch
from
September 20, 2026 19:20
f7792f9 to
ba3a75d
Compare
xsh310
removed this pull request from stack #765
September 20, 2026 19:21
xsh310
changed the base branch from
skills-fetch-bundles-and-write
to
skills-mirror-download
September 20, 2026 19:21
xsh310
added this pull request to stack #768
September 20, 2026 19:23
xsh310
force-pushed
the
skills-refresh-on-launch
branch
from
September 21, 2026 20:34
ba3a75d to
49f3e33
Compare
xsh310
force-pushed
the
skills-refresh-on-launch
branch
from
September 21, 2026 20:45
49f3e33 to
fb4fc82
Compare
xsh310
force-pushed
the
skills-refresh-on-launch
branch
2 times, most recently
from
September 24, 2026 14:51
864c5bd to
0cfd268
Compare
xsh310
added a commit
that referenced
this pull request
Sep 24, 2026
## What did you change, and why? **Change:** Extracted the fetch-bundles-then-write-each-skill core (with the per-skill fetch-failure and disk-error skip) out of `_download_refs` and `reconcile_managed_skills` into a single `_fetch_bundles_and_write` helper in `skills_download.py`. **Why:** The two callers had duplicated this loop, and a third caller, a launch-time skill refresh stacked on top in #764, needs the same core. Sharing one helper keeps them consistent. This is a pure refactor with no behavior change, except that the download path now gains the same per-skill `OSError` guard the managed reconcile already had. ## How do you know it works? **Testing:** The existing `_download_refs` and `reconcile_managed_skills` tests still pass, plus a new `TestFetchBundlesAndWrite` covering the write-survivors, skip-fetch-failure, and empty-refs cases. `uv run ruff check`, `ruff format --check`, `ty check src`, and `uv run pytest` all pass locally. This pull request and its description were written by Isaac. --- <sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub> Co-authored-by: Isaac <no-reply@databricks.com>
xsh310
added a commit
that referenced
this pull request
Sep 24, 2026
…em (#767) ## What did you change, and why? **Change:** `write_skill` now clears a skill's bundle directory before rewriting it, so the on-disk copy is a fresh mirror of the current bundle rather than the new files merged on top of the old ones. **Why:** The writer merged: it wrote every file from the new bundle but never removed files no longer in it, so a file deleted from a skill in Unity Catalog lingered on disk after a re-download. Clearing the directory first makes dropped files vanish. The clear-then-rewrite is also self-healing: a write interrupted partway leaves only that one directory incomplete (never a stray copy elsewhere), and the next write clears and rebuilds it, so a retry always converges on the current bundle. On the launch-refresh path the manifest's `uc_update_time` is advanced only after a successful write, so an interrupted write stays flagged as stale and is retried on the next launch. This is the shared writer for every download path, so `ug skills add` re-downloads get the same mirror semantics; the launch-time auto-update stacked on top (#764) relies on it. ## How do you know it works? **Testing:** Added `TestWriteSkill` cases for the deleted-upstream-file case, an empty bundle leaving the existing copy untouched, replacing a symlinked bundle without following it to its target, converging from an interrupted previous write (a pre-seeded partial directory), and leaving only the bundle directory behind. `uv run ruff check`, `ruff format --check`, `ty check src`, and `uv run pytest` all pass locally. This pull request and its description were written by Isaac. Co-authored-by: Isaac <no-reply@databricks.com>
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 way Isaac keeps plugins current. At launch, for each of the current workspace's own (non-managed) downloads, the recorded uc_update_time is compared against the skill's current update_time via GetSkill; any that changed are re-downloaded through _fetch_bundles_and_write. A record whose directories the user deleted by hand is forgotten instead of re-downloaded. The sweep is rate-limited to once every 24 hours via a new last_update_check stamp in ~/.ucode/skills.json (matching Isaac's plugin marketplace staleness window), is skipped under --skip-preflight, and fails open so it never blocks a launch. Co-authored-by: Isaac <no-reply@databricks.com>
Print a note when the launch-time refresh passes its rate-limit gate and starts, so the sweep is visible even when it finds nothing to update. Move the call up to run right after configure_tool, alongside the other on-disk launch preparation, instead of between the launch summary and "Starting", so its output no longer splits the summary block. Co-authored-by: Isaac <no-reply@databricks.com>
Fix a same-second staleness miss: UC serializes update_time with fractional seconds only when non-zero, so a raw string compare orders "...:25.400Z" before "...:25Z" and a genuinely newer skill was judged unchanged. Parse both values before comparing, reusing one shared parse_update_time helper lifted into string_utils (managed_config's private copy now delegates to it), so the two "is the UC copy newer" paths stay consistent. Also gate the "Checking Unity Catalog..." note on there being downloaded skills to check, so users who never use UC skills see nothing, while skill users keep the "checked, nothing changed" visibility. Co-authored-by: Isaac <no-reply@databricks.com>
set_last_update_check ran only after a successful sweep, so a persistent auth, network, or UC error left the stamp unset and let every launch past the 24h window re-run the check: refetch a token, fire the GetSkill calls, and reprint the progress notes on the launch hot path. Stamp before the sweep instead, so a failed check waits out the interval like a successful one while the launch still proceeds on whatever is already on disk. Co-authored-by: Isaac <no-reply@databricks.com>
refresh_downloaded_skills_on_launch forgot a download the moment any one of its two skill directories was gone, orphaning the surviving copy: untracked, never updated, and invisible to `ug skills remove`. Forget a record only when every directory is gone (the user removed the skill); when only some are gone, re-download it so both roots are rewritten and the on-disk copy mirrors UC again. A record whose UC skill no longer resolves is still left alone. Co-authored-by: Isaac <no-reply@databricks.com>
last_update_check parsed its stamp with a strict strptime while the rest of the skill-update code uses string_utils.parse_update_time; reuse that one parser so there is a single RFC-3339 reader (it also accepts the sub-second form). And normalize to UTC before formatting in set_last_update_check, so a non-UTC aware datetime is converted rather than stamped with a literal Z. Co-authored-by: Isaac <no-reply@databricks.com>
xsh310
force-pushed
the
skills-refresh-on-launch
branch
from
September 24, 2026 15:01
0cfd268 to
f786224
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What did you change, and why?
Change:
ug <agent>now refreshes a developer's UC-downloaded skills before the agent starts. For each of the current workspace's own (non-managed) downloads, theuc_update_timerecorded at download is compared against the skill's currentupdate_timevia GetSkill, and any that changed are re-downloaded in place through_fetch_bundles_and_write(from #763). A record whose directories the user deleted by hand is forgotten instead of re-downloaded.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 via a new top-level
last_update_checkstamp in~/.ucode/skills.json, matching Isaac's plugin marketplace staleness window, so back-to-back launches make no network calls. It is skipped under--skip-preflightand fails open, so any auth, network, or fetch error is reported and the launch proceeds on whatever is already on disk.Stacked on #763 (the
_fetch_bundles_and_writerefactor).How do you know it works?
Testing: New unit tests cover the 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, stale detection byuc_update_time(newer, equal, older, missing, deleted upstream), a hand-deleted record being forgotten rather than re-downloaded, the silent in-place overwrite that refreshes both 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.
Screen.Recording.2026-09-21.at.2.09.16.PM.mov
Stack created with GitHub Stacks CLI • Give Feedback 💬