Repository navigation
Conversation
…r-Agent
When a coding agent runs the CLI, the SDK already puts agent/<name> in the
User-Agent. Add aitools-agent/{plugin|skills|none} next to it: whether that
agent has the Databricks plugin or Databricks skills installed in its own
locations. The existing aitools/<tool>_<version> pairs list the plugins of
every tool on the machine, so they cannot answer this.
- agents.CallingAgent maps useragent.AgentProvider() to a registry agent.
- (*Agent).DatabricksTooling checks the agent's own plugin manifest (Claude
Code; project/local scope records count only inside their project), or
the global CLI install state for agents without a manifest reader, and
databricks* dirs in the agent's global and project skills dirs (looked up
from the working directory up to the repository root).
- The Claude Code manifest reader returns every databricks record from every
marketplace entry; DatabricksPluginVersion reports the highest version
instead of the first entry in random map order.
- Project paths match by lexical containment after resolving symlinks, then
by file identity (case-insensitive filesystems, 8.3 names, bind mounts).
Co-authored-by: Isaac <[email protected]>
✅ Integration test reportCommit: fd8c2c5
Top 4 slowest tests (at least 2 minutes):
|
simonfaltum
left a comment
simonfaltum
left a comment
There was a problem hiding this comment.
Approving. Nothing blocking; the inline comments are nits and nice-to-haves. Two larger points, neither blocking:
- The install hint (
hint.go,claudeToolingInstalled) and the newDatabricksToolingdefine "Claude has Databricks tooling" differently: the hint counts a plugin in any scope plus the shared~/.databricks/aitools/skillsstore and never looks at.claude/skills; the segment does the opposite. The description already calls this out. Either point the hint at the new check, or leave a cross-reference comment so the split reads as deliberate rather than accidental. - The marketplace version fix changes the existing
aitools/claude-code_<version>value on machines with the plugin from two marketplaces. Reasonable to keep here since the new code needs the full record list, but it does weaken the "revert removes only the segment" claim. A separate PR would give a cleaner revert story if you care about that.
Nit: the PR body uses Summary / Evidence / Merge Danger; the repo template (.github/PULL_REQUEST_TEMPLATE.md, enforced by the pr-checklist skill) is Changes / Why / Tests.
| // databricks plugin install for the agent. Only the global state is read: the | ||
| // CLI installs plugins in project scope only for agents with SupportsProjectScope | ||
| // (see mapAgentScope in cmd/aitools), and those have a manifest reader. | ||
| func pluginRecorded(ctx context.Context, agentName string) bool { |
There was a problem hiding this comment.
[nice to have] This re-implements install-state reading from installer: its own .state.json constant (installer.stateFileName), its own home + CanonicalSkillsDir join (installer.GlobalSkillsDir), and a partial struct instead of installer.LoadState. Harmless today (migrateState only bumps the version), but a future state change would silently miss this reader.
The only production caller is WithCallingAgentToolingInUserAgent in installer, which already has all the pieces. Consider computing the Tooling value there and exposing only a plugin-record check from agents, or moving the state types into a leaf package both can import.
| func projectDirs(cwd string) []string { | ||
| cwd = resolvePath(cwd) | ||
| dirs := []string{cwd} | ||
| root, err := folders.FindDirWithLeaf(cwd, ".git") |
There was a problem hiding this comment.
[nice to have] Stopping at the nearest .git undercounts in two cases: a Claude Code project that is not a git repo (only cwd is checked, so <project>/.claude/skills/databricks* is missed when the CLI runs from a subdirectory), and a nested repo or submodule inside the agent's project. The doc says this matches Claude Code's own lookup; a source link would settle that (the comments rule asks for one when depending on an external tool's behaviour). If Claude Code walks further, walking to the filesystem root is bounded and has no false positives since ~/.claude/skills is already the global dir.
| // and Windows, so it exercises the same identity check on Linux. Mounting needs | ||
| // a private mount namespace, so the test re-runs itself under `unshare -rm` | ||
| // (an unprivileged user namespace) and skips where those are not allowed. | ||
| func TestPluginInstallAppliesToBindMountedProjectPath(t *testing.T) { |
There was a problem hiding this comment.
[nice to have] In CI this skips on all three Linux jobs ("unprivileged mount namespaces are not available"), and TestPluginInstallAppliesToProjectPathInAnotherCase skips there too (ext4 is case-sensitive). So the positive os.SameFile path is only covered on the macOS and Windows runners, and this file adds 57 lines of unshare / syscall.Mount machinery that never runs in CI. Either drop the Linux test or add a deterministic positive case.
| // agent has them. Installs the CLI did not perform and that leave no trace in | ||
| // these locations (e.g. a Codex plugin installed with `codex` directly) read as | ||
| // ToolingNone. Read errors read as "not installed". | ||
| func (a *Agent) DatabricksTooling(ctx context.Context) Tooling { |
There was a problem hiding this comment.
[nice to have] Per-command cost under Claude Code: the plugin manifest is now parsed three times (InstalledTools, here, MaybeHint), and cwd is EvalSymlinks-resolved once in projectDirs and again inside isSameOrUnder for every project/local record. Also, pluginRecorded reads and decodes the state file for agents with Plugin == nil (Cursor, OpenCode, Pi, Gemini, Goose, Kiro, Antigravity), where the answer is false by construction. Resolving cwd once and guarding the state read with a.Plugin != nil would remove most of it.
| // a plugin removed with the agent's own CLI leaves a stale CLI state record | ||
| // behind, so the record is not consulted for these agents. A plugin that | ||
| // is installed but disabled in the agent's settings still counts; | ||
| // - for agents without a reader (Codex, Copilot), the plugin recorded for |
There was a problem hiding this comment.
[nice to have] The doc explains why the CLI state is ignored for Claude (stale after claude plugin uninstall), but for Codex and Copilot the state is the only source, so plugin here includes plugins since removed through the agent's own CLI. One sentence on that would tell analysts what plugin means for those agents.
| // installed in more than one scope has several records. | ||
| type pluginInstall struct { | ||
| Version string `json:"version"` | ||
| // Scope is Claude Code's install scope: "managed", "user", "project" or |
There was a problem hiding this comment.
[nit] Source link for the installed_plugins.json scope / projectPath semantics? The comments rule asks for one when relying on an external tool's format; the SDK link at the top of calling.go is the pattern.
| return useragent.InContext(ctx, callingAgentToolingUserAgentKey, toolingUserAgentValue(a.DatabricksTooling(ctx))) | ||
| } | ||
|
|
||
| func toolingUserAgentValue(t agents.Tooling) string { |
There was a problem hiding this comment.
[nit] Missing doc comment. This reads naturally as a String() method on agents.Tooling.
| } | ||
| } | ||
|
|
||
| func TestCallingAgentFromEnv(t *testing.T) { |
There was a problem hiding this comment.
[nit] These four TestCallingAgent* functions differ only in env input and expected result; a table fits the repo's testing rule. Also agents.CallingAgent().Name (and AgentForProvider(...).Name above) panics on nil instead of failing via require.NotNil. And TestAgentForProviderCoversRegistry hardcodes the SDK product names, so an SDK rename keeps it green while CallingAgent returns nil in production; driving it through the agents' env vars (CLAUDECODE, COPILOT_CLI, ...) would exercise the real mapping.
| // first only about one time in eight), so a reader that keeps only the | ||
| // first databricks entry fails some of these reads. | ||
| got := map[string]int{} | ||
| for range 200 { |
There was a problem hiding this comment.
[nit] The 200-iteration loop re-reads the manifest from disk to defeat map order probabilistically. Asserting len(installs) == 2 from claudePluginInstalls (this package has access) proves the same property deterministically, with one plain DatabricksPluginVersion assertion on top. Same for the loop in calling_test.go.
| ToolingPlugin | ||
| ) | ||
|
|
||
| // DatabricksTooling reports the Databricks AI tooling installed for this agent |
There was a problem hiding this comment.
[nit] This doc restates each branch of the 20-line body and has to move in lockstep with it. The non-obvious facts (the manifest is authoritative for Claude because claude plugin uninstall leaves a stale CLI record; the shared store cannot attribute skills to an agent) fit in about four lines. Same applies to the SetAgentEnv doc.
Summary
When a coding agent runs the CLI, the SDK already puts
agent/<name>in the User-Agent. This PR addsaitools-agent/{plugin|skills|none}next to it: whether that agent has the Databricks plugin or Databricks skills installed in its own locations. Usage analysis can then see whether the calling agent already has the Databricks tooling. The existingaitools/<tool>_<version>pairs cannot answer that: they list the plugins of every tool on the machine, so a Claude Code plugin also shows up on requests made by Codex.AGENT=...value that is not a registry agentaitools-agent/pluginaitools-agent/nonedatabricks aitools installaitools-agent/pluginclaude plugin)aitools-agent/none~/.claude/skills/databricks-appsaitools-agent/skillsrepo/jobsrepo/.claude/skills/databricks-apps,repo/.gitaitools-agent/skillsThe parent walk stops at the repository root, which is how Claude Code finds project skills. The shared
~/.databricks/aitools/skillsstore is not consulted: it holds the skills of every agent the CLI installed into, so it cannot say which agent has them. A Claude Code plugin that is installed but disabled in Claude's settings counts as installed.This check differs from the one the existing Claude Code install hint uses, in both directions. The hint counts a plugin record in any scope and the shared store, but never looks at
.claude/skillsdirs; the segment does the opposite. So in one command the hint can stay silent while the segment saysnone(plugin for another project only), or fire while the segment saysskills(project skills only).Supporting changes in
libs/aitools/agents/plugins.go:databricks@<marketplace>). Before, it kept the first matching entry in Go's random map order.DatabricksPluginVersionnow reports the highest version across those records, so the existingaitools/claude-code_<version>pair is stable too.os.SameFile) against the directory and each of its ancestors. This covers a different case on case-insensitive filesystems, Windows 8.3 short names, and bind mounts.No telemetry event or proto changes. The segment travels in the User-Agent of every request the command makes.
Evidence
TestDatabricksPluginVersionAcrossMarketplacescopied onto it, a manifest withdatabricks@a0.2.9 anddatabricks@b0.3.0, 200 reads):--- PASS: TestDatabricksPluginVersionAcrossMarketplacesaitools/claude-code_0.2.9and nothing about Codex.After:
TestWithCallingAgentToolingInUserAgentpasses its 8 cases, includingTestDatabricksTooling(per-agent locations, project-scope records inside/outside the project, stale CLI state, corrupt state, project skills at the repository root seen from a subdirectory, project skills above the repository root not counted),TestPluginInstallAppliesToBindMountedProjectPath(Linux, private mount namespace),TestPluginInstallAppliesToSymlinkedProjectPath,TestPluginInstallAppliesToProjectOutsideWorkingDir, andTestCallingAgent*: pass.TestPluginInstallAppliesToProjectPathInAnotherCaseruns only where the temp filesystem folds case (macOS, Windows) and skipped on the Linux run.go build ./...,./task test-exp-aitools(760 unit, 25 acceptance),go test ./libs/telemetry/... ./cmd/root/... ./libs/aitools/... ./cmd/aitools/...,./task lint-q(0 issues),./task deadcode: pass.auth/credentials,cmd/api,bundle/user_agent,cmd/workspace/apps) pass unchanged: the harness clears the agent env vars, so noagent/oraitools-agent/segment appears.Merge Danger
Door: two-way
Adds one User-Agent segment and changes no stored state or schema. Reverting removes the segment.
Blast Radius: User-Agent
aitools/claude-code_<version>pair can change value on machines where the plugin comes from more than one marketplace: it now always reports the highest version.filepath.Relandos.SameFilebehaviour. It was not run here; CI on those platforms runs the case-folding test.This pull request and its description were written by Isaac.