Repository navigation
aitools: refresh only the plugin's own marketplace, and do not fail the install on it - #7473
Conversation
cc3a109 to
3e0bf18
Compare
37c16a6 to
feb1410
Compare
✅ Integration test reportCommit: 0134b4b
Top 18 slowest tests (at least 2 minutes):
|
simonfaltum
left a comment
simonfaltum
left a comment
There was a problem hiding this comment.
🤖 Automated review from simonfaltum's review agent (pr-review skill) at feb1410. Each finding was verified against the code; simonfaltum may follow up.
Not approving yet: 2 open design questions.
This PR makes the Claude Code marketplace refresh name the plugin's own marketplace, and stops a failed refresh from failing aitools install. The named refresh is a clean fix, and all CI checks pass. Nothing is blocking, but the continue-on-failure path hides a likely stale install: it logs only at debug and records the latest version. The changelog also names the wrong marketplace, and Codex's update step still refreshes every marketplace. Two design questions remain: whether the non-fatal fallback is needed, and why update and install now treat a refresh failure differently.
Questions
aitools installnow goes on whenclaude plugin marketplace update <name>fails, butaitools updatestill stops on the same refresh failure (UpdatePluginForAgentreturns on the first failing step, plugin.go:312-316). Is that difference intended? If it is, could the reason go in a code comment? If not, should update also tryplugin updateafter a failed refresh, the same way install does?- Naming the marketplace in the refresh already stops an unrelated, unreachable marketplace from failing the install. After that change, a refresh fails only when the plugin's own marketplace can't be refreshed, which is when the local copy is most likely out of date. AGENTS.md asks to avoid speculative fallbacks. Is the continue-on-refresh-failure path still needed? If so, would you surface it (warn, and don't record the latest version)? Or would you keep only the named refresh and restore the fatal refresh with rollback?
Comments outside the changed lines
- [nice to have]
libs/aitools/installer/plugin.go:296: When the refresh fails, the install now proceeds from the local marketplace copy, but the record still storesVersion: DisplaySkillsVersion(ref). Ifclaude-plugins-officialis already registered with an old copy and the refresh fails, the install succeeds with an older plugin. The CLI printsdatabricks plugin v<latest>, andaitools update --check(cmd/aitools/update.go:280-285) andaitools listreport "up to date". Before this PR a failed refresh stopped the install, so this drift could not happen. Consider leavingVersionempty whenrefreshErr != nil, so the record shows "version unknown" until the next successful update. - [nit]
libs/aitools/installer/plugin.go:108: The PR addsrecordedMarketplace, butrecordedInstallTargetstill has the samerec.Marketplace→agent.Plugin.Marketplacefallback written inline. Writereturn plugin + "@" + recordedMarketplace(agent, rec)so the refreshed marketplace and the update target cannot drift.
| } | ||
| } | ||
| return PluginRecord{}, &BlockedError{Agent: agent.Name, Reason: ReasonInstallFailed, Detail: stderrOf(err)} | ||
| log.Debugf(ctx, "%s plugin marketplace refresh failed, installing from the local copy: %v", agent.DisplayName, err) |
There was a problem hiding this comment.
[nice to have] When the refresh fails and the install succeeds, the user gets a clean success with no hint that the plugin may be out of date. .agents/rules/style-guide-go.md:237 says to "Warn for things the user should know about and might act on". The line also logs %v, which for a *process.ProcessError is only "<command>: exit status N" (libs/process/background.go:25-27), so even --debug loses the reason. Log at Warn with stderrOf(err) and an actionable hint, e.g. "could not refresh the marketplace, installed from the local copy; run databricks aitools update later: ".
There was a problem hiding this comment.
Done in 02922d2: the refresh failure is now logged with log.Warnf and stderrOf(err), with the hint: "Could not refresh the marketplace, installing the plugin from the local copy; run databricks aitools update later: ".
| @@ -0,0 +1 @@ | |||
| * `databricks aitools install` refreshes only the Databricks Claude Code marketplace, and installs from the local copy when the refresh fails, so another unreachable marketplace no longer blocks the install. ([#7473](https://github.com/databricks/cli/pull/7473)) | |||
There was a problem hiding this comment.
[nice to have] The entry says install "refreshes only the Databricks Claude Code marketplace", but the Claude Code registry entry uses claudePlugin(), i.e. Anthropic's shared claude-plugins-official marketplace (libs/aitools/agents/agents.go:163-168, 182). The refresh therefore runs claude plugin marketplace update claude-plugins-official. The tests' claudeAgent() helper uses databricks-agent-skills, which is probably where the wording came from. Reword it to something like "refreshes only the marketplace the Databricks plugin is listed in", and update the PR title the same way.
There was a problem hiding this comment.
Done in 02922d2: the fragment now says install and update "refresh only the marketplace the Databricks plugin is listed in", and mentions the warning and the "version unknown" record. PR title updated to "refresh only the plugin's own marketplace". The commit subject stays as is for now.
| case agents.NameClaudeCode: | ||
| return [][]string{ | ||
| marketplaceUpdateArgs(agent), | ||
| marketplaceUpdateArgs(agent, recordedMarketplace(agent, rec)), |
There was a problem hiding this comment.
[nice to have] The Codex branch a few lines up (line 187) still runs a bare codex plugin marketplace upgrade. codex plugin marketplace upgrade --help says "Omit MARKETPLACE_NAME to upgrade all configured Git marketplaces", so one unreachable third-party marketplace still fails databricks aitools update for Codex, which is the failure this PR fixes for Claude. Pass the name there too: {"plugin", "marketplace", "upgrade", recordedMarketplace(agent, rec)}.
There was a problem hiding this comment.
Done in 02922d2: Codex's update step now runs codex plugin marketplace upgrade <recordedMarketplace(agent, rec)>; TestUpdatePluginForAgentCodexTwoStep asserts codex plugin marketplace upgrade databricks-agent-skills. Changelog and PR body mention it.
| // A refresh failure (e.g. another marketplace the user added is unreachable) | ||
| // used to fail the install although the local marketplace copy had the plugin. |
There was a problem hiding this comment.
[nit] "A refresh failure (e.g. another marketplace the user added is unreachable)" no longer applies: the refresh is now named, so another marketplace cannot make it fail. Describe the case the fallback is actually for, such as the plugin's own marketplace being unreachable.
There was a problem hiding this comment.
Done in 02922d2: the comment now describes the plugin's own marketplace being unreachable while the local copy still lists the plugin.
| } | ||
|
|
||
| func TestInstallPluginRollsBackMarketplaceOnRefreshFailure(t *testing.T) { | ||
| func TestInstallPluginContinuesWhenRefreshFails(t *testing.T) { |
There was a problem hiding this comment.
[nit] TestInstallPluginContinuesWhenRefreshFails covers only a just-added marketplace, where the local copy is fresh by construction. The case the fallback exists for, a marketplace already registered whose refresh fails, is untested, and so is recordedMarketplace's fallback for a record with an empty Marketplace in pluginUpdateSteps. Add a case where plugin marketplace list returns the marketplace and the refresh fails: assert no marketplace remove and InstalledMarketplace == false. Add another where an update with PluginRecord{Marketplace: ""} refreshes the agent's marketplace.
There was a problem hiding this comment.
Done in 02922d2: added TestInstallPluginContinuesWhenRefreshOfRegisteredMarketplaceFails (marketplace list returns the marketplace, refresh fails: install runs, no marketplace remove, InstalledMarketplace == false, empty Version) and TestUpdatePluginForAgentClaudeRecordWithoutMarketplace (PluginRecord{Marketplace: ""} refreshes and updates against the agent's marketplace).
3e0bf18 to
410f869
Compare
feb1410 to
4eff713
Compare
410f869 to
88362b5
Compare
056b3e7 to
02922d2
Compare
anton-107
changed the title
|
Comments outside the changed lines, both done in 02922d2:
|
88362b5 to
679dd3e
Compare
02922d2 to
d467e3f
Compare
679dd3e to
ca64d39
Compare
4cc5453 to
605b293
Compare
|
Answers to the two Questions in the review, with the change in 605b293: 1. Install and update treat a refresh failure differently. That difference is gone. 2. Is the continue-on-refresh-failure path still needed? Yes, I kept it. A stale local copy that still lists the plugin installs a working plugin, so failing the whole run there blocks a usable install over a refresh that can be retried later. The run no longer hides that it fell back: it prints a warning that tells the user to run |
ca64d39 to
9d4b058
Compare
605b293 to
eb09c76
Compare
simonfaltum
left a comment
simonfaltum
left a comment
There was a problem hiding this comment.
🤖 Automated review from simonfaltum's review agent (pr-review skill) at eb09c76. Each finding was verified against the code; simonfaltum may follow up.
Not approving yet: 1 open design question.
This PR names the marketplace in the Claude Code refresh and the Codex upgrade, so an unreachable third-party marketplace no longer breaks aitools install or aitools update. A failed refresh now warns, continues from the local copy, and records the version as unknown. Everything from the earlier review is addressed, CI is green, and nothing is blocking. The main remaining point is that the refresh warning fires before the install or update result, so a failed install shows wrong advice and repeats the refresh error. The rest are small nits, plus one question about whether to split the fail-soft change into its own PR.
Questions
- This PR makes two changes: naming the marketplace in the refresh, which fixes the reported failure on its own, and making a failed refresh non-fatal for install and update, with the version-unknown record. AGENTS.md asks to keep each PR focused on one change. You explained earlier why you kept the fallback. Would you move the fallback into its own PR, so the naming fix can land and be reverted on its own? Or do you prefer to keep them together, and if so, why?
Comments outside the changed lines
- [nit]
cmd/aitools/list.go:394: This PR now writesVersion: ""after every failed refresh.agentStatusLabelusesversion == ""to mean "nothing picked yet". Example: Claude Code is installed in both scopes and the project refresh fails, so project has""and global has0.2.7, with release0.2.7. Project is visited first and is stale. Global then overwritesversion, so the label reads "databricks plugin · v0.2.7 · update available": the up-to-date version shows next to "update available", and the unknown scope is hidden. Track the pick with a separate bool instead ofversion == "". This code is outside the diff, but the PR is what makes the case common. - [nit]
.github/PULL_REQUEST_TEMPLATE.md:.agents/skills/pr-checklist/SKILL.md:48asks agent-written PRs to follow.github/PULL_REQUEST_TEMPLATE.mdexactly:## Changes,## Why,## Tests, in that order, with no other sections. This body uses## Summary,## Evidence, and## Merge Danger. It also opens with a duplicated "Stacked on" line. Rename the sections to the template headings and trim the Evidence block.
| } | ||
| _, err := runAgentCmd(ctx, pluginCmdTimeout, prepend(bin, args)) | ||
| if err != nil { | ||
| log.Warnf(ctx, "Could not refresh the %s marketplace, using its local copy for the %s plugin; run `databricks aitools update` later: %s", marketplace, agent.DisplayName, stderrOf(err)) |
There was a problem hiding this comment.
[nice to have] refreshMarketplace prints "Could not refresh the X marketplace, using its local copy …; run databricks aitools update later" as soon as the refresh fails, before the install or update has run. If that command then fails, the warning is wrong, and the refresh stderr is printed a second time through withRefreshError in the "Skipped …" line. Example: a first aitools install while GitHub is unreachable. marketplace add fails (its error is ignored at line 291), the refresh fails, and the install fails. No record is saved, so aitools update never retries Claude Code: UpdateInstalledPlugins only walks recorded plugins, and migration needs raw skills. The warning still tells the user to run it. Have refreshMarketplace only run the command and return the error, and warn only after InstallPluginForAgent/UpdatePluginForAgent succeeds. The failure path already carries the refresh error in the BlockedError detail.
There was a problem hiding this comment.
Done in 7123aae. refreshMarketplace now only runs the command and returns the error. The warning moved to warnLocalMarketplaceCopy, which InstallPluginForAgent and UpdatePluginForAgent call only after the install or update succeeds. On failure the refresh error appears once, in the BlockedError detail via withRefreshError, and there is no "run databricks aitools update later" advice.
| } | ||
|
|
||
| func TestInstallPluginRollsBackMarketplaceOnRefreshFailure(t *testing.T) { | ||
| func TestInstallPluginContinuesWhenRefreshFails(t *testing.T) { |
There was a problem hiding this comment.
[nit] On a successful install or update after a failed refresh, the warning is the only thing that tells the user they got the local copy. No test checks it, so it could be dropped or demoted to debug without any test failing. The package already captures logs (update_test.go:651-653 puts a slog.TextHandler on a buffer). Assert the message here and in the update equivalent. If the warning moves after the result, also assert that a failed install does not print it.
There was a problem hiding this comment.
Done in 7123aae. A captureWarnings helper puts a slog.TextHandler on a buffer. TestInstallPluginContinuesWhenRefreshFails (already registered case) and TestUpdatePluginForAgentContinuesWhenRefreshFails (Claude Code and Codex) assert the warning and the agent's stderr. TestInstallPluginRollsBackMarketplaceWhenRefreshAndInstallFail and TestUpdatePluginForAgentReportsRefreshWhenUpdateFails assert that a failed install or update does not print it. Demoting the warning to debug makes the two success tests fail.
| } | ||
| rec.Version = version | ||
| if !refreshed { | ||
| rec.Version = "" |
There was a problem hiding this comment.
[nit] Codex has no manifest reader (pluginInstalls is nil), so InstalledTools uses only the recorded version, and recordedPluginVersions skips empty ones (detect.go:58). After codex plugin marketplace upgrade databricks-agent-skills fails and codex plugin add succeeds, the record goes from 0.2.6 to "". Every later CLI call then reports no codex_* pair, although the plugin is installed. Before this PR, the update was skipped and the old version stayed. If that's intended, a note in InstalledTools would help. Otherwise, keep the previous version for telemetry, or emit an explicit unknown token.
There was a problem hiding this comment.
Kept the empty version and documented it in 7123aae: the InstalledTools doc comment now names this case (an agent without a plugin cache reader, such as Codex, whose last install or update ran from a marketplace copy that could not be refreshed). Keeping 0.2.6 would report a version the CLI can't vouch for: codex plugin add ran from the local copy, which may hold a newer or the same version. The empty record is also what makes aitools list and aitools version say "version unknown" for Codex. The next successful update records the version again. An explicit unknown token would change the <agent>_<version> token format, so I left that out of this PR.
| // After a failed refresh we can't tell which version the local copy held, | ||
| // so the record says "version unknown" until the next successful update. | ||
| version := DisplaySkillsVersion(ref) | ||
| if refreshErr != nil { |
There was a problem hiding this comment.
[nit] When installedMarketplace is true, marketplace add has just cloned the marketplace, so the local copy is current. A refresh failure right after that still records Version: "", and aitools update --check and aitools version then show "version unknown" for an up-to-date install (TestInstallPluginContinuesWhenRefreshFails asserts this). Either skip the refresh when we just added the marketplace, or clear the version only when refreshErr != nil && !installedMarketplace.
There was a problem hiding this comment.
Done in 7123aae. When installedMarketplace is true, marketplace add just cloned the marketplace, so a failed refresh right after it neither clears the version nor prints the warning. The version is cleared and the warning printed only when refreshErr != nil && !installedMarketplace. TestInstallPluginContinuesWhenRefreshFails is now a table: "just added" records 0.2.6 with no warning, and "already registered" records an empty version with the warning. A failed install still carries the refresh error in its detail in both cases.
| // failed refresh is not fatal: the local copy of the marketplace usually still | ||
| // lists the plugin, so the caller goes on with it. The local copy may be out of | ||
| // date, so the caller records the plugin version as unknown. | ||
| func refreshMarketplace(ctx context.Context, agent *agents.Agent, bin, marketplace string, args []string) error { |
There was a problem hiding this comment.
[nit] The marketplace parameter is only used in the log line, and the same name is already inside args. Install passes agent.Plugin.Marketplace twice, and update calls recordedMarketplace both in pluginRefreshArgs and at the call site. The argv is also built in two places: marketplaceUpdateArgs for Claude and pluginRefreshArgs for Codex. A single marketplaceRefreshArgs(agent, marketplace) switch, called inside refreshMarketplace(ctx, agent, bin, marketplace), would serve install and update and remove pluginRefreshArgs.
There was a problem hiding this comment.
Partly done in 7123aae. refreshMarketplace(ctx, bin, args) no longer takes agent or marketplace. The marketplace name is used only in the success-path warning, so recordedMarketplace is called once per path. I kept two argv builders: install refreshes only for Claude Code (marketplaceUpdateArgs), while update refreshes for Claude Code and Codex (pluginRefreshArgs). A single switch used by both would add codex plugin marketplace upgrade to aitools install for Codex, which install has never run. That is a behavior change outside this PR.
| assert.NotContains(t, stub.Commands(), "claude plugin marketplace remove databricks-agent-skills") | ||
| } | ||
|
|
||
| func TestInstallPluginContinuesWhenRefreshOfRegisteredMarketplaceFails(t *testing.T) { |
There was a problem hiding this comment.
[nit] .agents/rules/testing.md:51 says "Use table-driven tests for multiple similar cases". This test differs from TestInstallPluginContinuesWhenRefreshFails (line 166) only in the plugin marketplace list stdout and the InstalledMarketplace assertion. TestUpdatePluginForAgentClaudeRecordWithoutMarketplace (line 243) differs from TestUpdatePluginForAgentClaudeOneStep (line 230) only in rec.Marketplace. Fold each pair into a table, as TestUpdatePluginForAgentContinuesWhenRefreshFails already does.
There was a problem hiding this comment.
Done in 7123aae. TestInstallPluginContinuesWhenRefreshFails is a table over "just added" and "already registered" (marketplace list output, InstalledMarketplace, version, warning). TestUpdatePluginForAgentClaudeOneStep is a table over a record with a marketplace and one without, and it asserts the exact command sequence.
eb09c76 to
7123aae
Compare
|
Third review at Question 1. Yes. When an already-registered marketplace can't be refreshed, the warning plus "version unknown" is the signal I want. A stale local copy that still lists the plugin installs a working plugin. The warning names the marketplace, shows the agent's error and says to run
|
3e979d0 to
99db1f7
Compare
40e1da8 to
c2eaa89
Compare
99db1f7 to
07cf845
Compare
c2eaa89 to
20e3ef4
Compare
07cf845 to
d82eb6b
Compare
20e3ef4 to
06fd311
Compare
simonfaltum
left a comment
simonfaltum
left a comment
There was a problem hiding this comment.
🤖 Approval from simonfaltum's review agent at 06fd311, on Simon's decision.
The design questions from the earlier automated reviews no longer block this PR: your answers are accepted, and any still-open ones are non-blocking. The changes since the last automated review (0d74442 → 06fd311) address its comments.
rugpanov
left a comment
rugpanov
left a comment
There was a problem hiding this comment.
[nit] The Blast Radius section says "Only the Claude Code plugin install and update path". This PR also changes the Codex update path: aitools update now runs codex plugin marketplace upgrade <name>, and a failed refresh no longer stops it. Add Codex to that sentence.
| // used a local marketplace copy that could not be refreshed, so the plugin may | ||
| // be out of date. | ||
| func warnLocalMarketplaceCopy(ctx context.Context, agent *agents.Agent, marketplace string, refreshErr error) { | ||
| log.Warnf(ctx, "Could not refresh the %s marketplace, so the %s plugin came from its local copy and may be out of date; run `databricks aitools update` later: %s", marketplace, agent.DisplayName, stderrOf(refreshErr)) |
There was a problem hiding this comment.
[nit] UpdatePluginForAgent (line 348) prints this warning too. So a user who runs databricks aitools update is told to run databricks aitools update later. Use words that work for both commands, for example "retry later", or let the caller give the command.
There was a problem hiding this comment.
Done in c9feaa5. The caller now passes the hint: aitools install says "run databricks aitools update later", and aitools update says "rerun databricks aitools update later". TestUpdateInstalledPluginsRecordsUnknownVersionWhenRefreshFails asserts the update wording.
| // After a failed refresh the update ran from the local copy, so its | ||
| // version is unknown, unless the record already holds this release: the | ||
| // local copy can only have moved forward since the refresh that wrote it. | ||
| if refreshed || rec.Version == version { |
There was a problem hiding this comment.
[nit] When the refresh fails but the record already holds this release, update keeps the version here. But UpdatePluginForAgent has already printed that the plugin "may be out of date". So the user sees that warning, then databricks plugin v<current>, and update --check later reports it as up to date. install does the opposite in the same case: it clears the version (plugin.go:317). Use one rule for both paths, and print the warning only when the version is cleared.
There was a problem hiding this comment.
Done in c9feaa5. Both paths now follow the same rule: warn only when the recorded version is cleared. UpdatePluginForAgent no longer warns. It returns the refresh error when the update still succeeded, and UpdateInstalledPlugins warns only in the branch that clears the version. If the record already holds this release, it keeps the version and prints nothing, so databricks plugin v<current> and update --check are no longer contradicted by a warning. Install has no earlier record to compare against, so after a failed refresh of an already-registered marketplace it still clears the version and warns. When we just added the marketplace, it keeps the version and doesn't warn. TestUpdateInstalledPluginsRecordsUnknownVersionWhenRefreshFails covers both update cases: a warning when the version is cleared, none when it is kept.
d82eb6b to
e9ddafa
Compare
06fd311 to
b2ee61c
Compare
e9ddafa to
b221ddf
Compare
dab2b7c to
c9feaa5
Compare
|
Blast Radius fixed in the description: it now names the Codex plugin update path ( |
b221ddf to
78b9500
Compare
c9feaa5 to
af8a5dd
Compare
78b9500 to
55c0421
Compare
af8a5dd to
dfb22a8
Compare
55c0421 to
5cb043a
Compare
dfb22a8 to
4075f1c
Compare
5cb043a to
d27386e
Compare
… install on it
Claude Code's `plugin marketplace update` with no name refreshes every marketplace the user has added, so one unreachable third-party marketplace failed the Databricks plugin install. The refresh now names the marketplace the Databricks plugin is listed in, and a failed refresh no longer stops the install or the update: both go on from the local copy of the marketplace and, once that succeeds, warn and record the plugin version as unknown. Codex's `plugin marketplace upgrade` in `aitools update` names the marketplace too.
```diff
InstallPluginForAgent (libs/aitools/installer/plugin.go)
marketplace add (if absent)
- claude plugin marketplace update
- on error: remove the marketplace we added, return install-failed
+ claude plugin marketplace update <marketplace>
+ on error: keep the error, continue
claude plugin install databricks@<marketplace>
on error: remove the marketplace we added
- return install-failed with the install error
+ return install-failed with the install error
+ + "(refreshing the marketplace also failed: <refresh error>)" if the refresh failed
- record Version = <ref>
+ if the refresh failed and the marketplace was not just added by us:
+ warn that the plugin came from the local copy (with the agent's stderr)
+ record Version = "" (version unknown)
+ else record Version = <ref>
UpdatePluginForAgent (aitools update)
- claude plugin marketplace update
+ claude plugin marketplace update <recorded marketplace, or the agent's current one>
- on error: return install-failed
+ on error: keep the error, continue
claude plugin update ...
- codex plugin marketplace upgrade
+ codex plugin marketplace upgrade <recorded marketplace, or the agent's current one>
+ on error: same as above
codex plugin add ...
on update error: return install-failed
+ + "(refreshing the marketplace also failed: <refresh error>)" if the refresh failed
- record Version = <ref>
+ if the refresh failed and the record does not already hold <ref>:
+ warn that the plugin came from the local copy, record Version = "" (version unknown)
+ else record Version = <ref>
```
A record without a version still counts as an installed plugin for the `databricks version --check` tip (`InstalledPluginAgents`), and `aitools update --check` shows it as "version unknown (update may be available)".
Changelog fragment: `.nextchanges/cli/aitools-marketplace-refresh.md`.
Co-authored-by: Isaac <no-reply@databricks.com>
4075f1c to
0134b4b
Compare
Stacked on #7472.
Stacked on: aitools: raise the plugin probe timeout from 5s to 20s
Summary
Claude Code's
plugin marketplace updatewith no name refreshes every marketplace the user has added, so one unreachable third-party marketplace failed the Databricks plugin install. The refresh now names the marketplace the Databricks plugin is listed in (claude-plugins-officialfor Claude Code), and a failed refresh no longer stops the install or the update: both go on from the local copy of the marketplace and, once that succeeds, warn and record the plugin version as unknown. Codex'splugin marketplace upgradeinaitools updatenames the marketplace too.Changelog fragment:
.nextchanges/cli/aitools-marketplace-refresh.md.Evidence
libs/aitools/installer/plugin_test.go, run at this head withplugin.gotemporarily reverted to the parent's version and then restored:The existing tests now also check that the refresh command names the marketplace (
claude plugin marketplace update databricks-agent-skills,codex plugin marketplace upgrade databricks-agent-skills) and that a failed refresh leaves the recorded version empty unless we just added the marketplace.TestInstallPluginContinuesWhenRefreshFailsis a table: a just-added marketplace (version recorded, no warning) and an already registered one (install goes on, nomarketplace remove,InstalledMarketplace == false, empty version, warning with the agent's stderr).TestUpdatePluginForAgentClaudeOneStepalso covers a record without a marketplace (falls back to the agent's marketplace). The failed install and failed update tests assert that the local-copy warning is not printed. Foraitools update:TestUpdatePluginForAgentContinuesWhenRefreshFails(Claude Code and Codex: the refresh fails, the update still runs),TestUpdatePluginForAgentReportsRefreshWhenUpdateFails(both errors in the detail), andTestUpdateInstalledPluginsRecordsUnknownVersionWhenRefreshFails(the update succeeds, the state records an empty version).TestUpdateInstalledPluginsRecordsUnknownVersionWhenRefreshFailsalso covers a record that already holds the current release (kept).TestInstalledPluginAgentscovers plugin records without a version, andTestUpdateCheckPrintsPluginStatethe three--checkstatuses.TestAgentStatusLabelKeepsUnknownStaleScopecovers theaitools listlabel when one scope has an unknown version and the other is current. At this head:go build ./..., unit tests forlibs/aitools/...andcmd/aitools/...,task test-exp-aitools, thecmd/aitoolsacceptance tests, lint and format on changed files,task checks, andgo vetfor windows and darwin on the aitools packages pass. Not run on Windows or macOS, and not run against a realclaudebinary.Merge Danger
Door: two-way
Behavior change: a failing refresh used to fail
aitools installwith an install-failed error and remove the marketplace we had just added. Now the install goes on using the local copy of the marketplace; if that copy is stale and lacks the plugin, the install fails and the error includes both messages. A refresh failure that is followed by a successful install prints a warning with the agent's stderr and a hint to rundatabricks aitools update, and the plugin is recorded as "version unknown" soaitools update --checkandaitools listdon't report it as up to date. The warning is printed only after the install succeeds, and not when we just added the marketplace (that copy is fresh, so the version is recorded as usual).aitools listnow shows "version unknown · update available" when one scope has an unknown version and another is current, instead of the current scope's version. No exit code changes. A refresh that hangs until the 60s command timeout used to end the run; now the install or update runs next and can wait up to another 60s, so on a machine where GitHub hangs instead of failing fast,aitools installoraitools updatecan take about a minute longer than before.aitools updateused to skip the plugin with a warning when the refresh failed; now, for both Claude Code and Codex, it still runs the plugin update and, unless the record already holds the current release, warns (with a hint to rerundatabricks aitools updatelater) and records the version as unknown. When the record already holds the current release, it keeps that version and prints no warning.aitools update --checkshows an unknown version as "version unknown (update may be available)", anddatabricks version --checkstill counts a plugin recorded without a version as installed, so it does not suggest installing Databricks tooling. If the update itself fails, the plugin is skipped as before and the error includes both messages. Both commands refresh only the one marketplace now.Blast Radius: small
Only the Claude Code plugin install and update path and the Codex plugin update path in
databricks aitools. This relies onclaude plugin marketplace update <name>andcodex plugin marketplace upgrade <name>accepting a marketplace name; that is checked only through the stubbed commands in the tests here.This pull request and its description were written by Isaac.