Repository navigation
Restore latest skills for CLI 1.22+ - #7069
simonfaltum wants to merge 5 commits into
Conversation
Signed-off-by: simonfaltum <simon.faltum@databricks.com>
Signed-off-by: simonfaltum <simon.faltum@databricks.com>
Integration test reportCommit: 2c34f94
Top 6 slowest tests (at least 2 minutes):
|
CLIs before 1.6.0 validate every manifest entry as semver, so a single "skills": "latest" entry makes them reject the whole remote manifest and fall back to stale caches or old embedded versions. Keep skills concrete in every entry and mark the 1.22.0 range with skills_track_latest instead. The concrete skills value doubles as the embedded fallback, which removes skills_fallback. Tests now derive pins from the embedded manifest so routine bumps don't break them. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: simonfaltum <simon.faltum@databricks.com>
| @@ -1,4 +1,5 @@ | |||
| { | |||
| "1.22.0": { "appkit": "0.84.0", "skills": "0.2.30", "skills_track_latest": true }, | |||
There was a problem hiding this comment.
why not just have a latest value under the skills? That entry will be read by the CLI which already supports it, right?
There was a problem hiding this comment.
Yeah, it should work, and it shouldn't break the previous CLIs - https://github.com/databricks/cli/blame/c782e67e37116c2887002b456525d20da43a4929/libs/clicompat/clicompat.go#L432
There was a problem hiding this comment.
Yeah, the current CLI supports it. My concern is the pre-1.6 clients. That latest exception was added in #5745 and first shipped in 1.6.0.
In v1.5.0, we still:
- Fetch the manifest from
main, so older clients also see the new1.22.0entry. - Validate every entry, with skills required to be semver. There's no
latestexception.
Version selection happens afterward. So even if the 1.5.0 range stays pinned, a latest value under 1.22.0 makes that client reject the whole remote manifest and fall back to stale or embedded data, including for AppKit.
That's why I'm keeping skills concrete and adding skills_track_latest: old clients ignore the new field and can still parse the manifest; 1.22.0+ uses it to opt into latest.
Signed-off-by: simonfaltum <simon.faltum@databricks.com>
| // overrides everything for eval/reproducibility. (AppKit version resolution is | ||
| // unaffected; cli-compat still manages it via its own field.) | ||
| func GetSkillsRef(ctx context.Context) (ref string, explicit bool, err error) { | ||
| return getSkillsRef(ctx, true) |
There was a problem hiding this comment.
aitools version also uses this (cmd/aitools/version.go:51). If the GitHub API call fails, it gets the fallback v0.2.30. A user on v0.2.31 then sees "Update available: 0.2.30", and aitools update refuses that downgrade. The plugin paths (cmd/aitools/update.go:82, install.go:529) also save the fallback as the plugin version. Maybe these callers should skip the fallback and treat the error as "latest unknown"?
| @@ -110,7 +110,7 @@ If validation fails, show the errors and fix them before proceeding. | |||
| git checkout -b bump-cli-compat-appkit-{appkit_version}-skills-{skills_version} | |||
There was a problem hiding this comment.
Nit: the branch, commit and PR titles still say skills-{skills_version}. That value is now the fallback, not a manifest pin, so the titles may confuse readers later.
This branch has not been deployed
3 participants
Changes
"skills": "latest". Keep earlier ranges pinned.0.2.30as a compiled fallback. Reject automatic downgrades before overwriting or pruning skills, including with--force. ExplicitDATABRICKS_SKILLS_REFand compatibility pins still allow intentional downgrades.Why
Raw skills were stuck on exact pins while plugin installs already follow the marketplace's current version. The manifest's existing
"latest"value expresses the intended behavior without another property.Pre-1.6 CLIs reject the updated remote manifest and use a valid cached or embedded manifest without returning a parse error. Losing remote AppKit and skills compatibility updates for these CLIs is accepted.
Tests
./task fmt,./task checks,./task lint,./task test, and./task test-exp-aitools.This PR was written by Isaac and updated with Codex.