aitools: make update-prune acceptance test hermetic#5784
Merged
Conversation
The update-prune acceptance test detected Claude Code (~/.claude), so `aitools update` tried to migrate raw skills to the native databricks plugin by shelling out to `claude plugin install`. The captured output depended on whether `claude` was on the host PATH: developer machines with claude installed recorded the marketplace "install-failed" error, while CI runners (no claude) produced a "cli-not-on-path" error, so the golden diverged and the build job failed on every platform. Detect Cursor instead, a skills-only agent with no databricks plugin, so update exercises the raw-skill prune path (the test's actual subject) without touching any plugin-capable agent's CLI. The plugin migration path is already covered hermetically by unit tests in cmd/aitools. The test now passes with or without claude on PATH. Co-authored-by: Isaac
pietern
approved these changes
Jul 1, 2026
| # A plugin agent (e.g. Claude Code) would run `claude plugin install` during | ||
| # update, making the output depend on whether `claude` is on the host PATH. | ||
| # Prefer USERPROFILE on Windows. | ||
| mkdir -p "${USERPROFILE:-$HOME}/.cursor" |
Contributor
There was a problem hiding this comment.
FYI, you can use $HOME, it is also set on Windows (see acceptance/script.prepare).
Member
Author
There was a problem hiding this comment.
Good call, thanks. Switched to $HOME and dropped the USERPROFILE fallback in 9668081.
Contributor
There was a problem hiding this comment.
Note, this same pattern shows up in other aitools acc tests.
Address review feedback: sethome exports HOME on Windows too (see
acceptance/script.prepare), so the ${USERPROFILE:-$HOME} fallback is
unnecessary here. Use plain $HOME.
Co-authored-by: Isaac
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
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.
Why
The
buildjob onmainis failing on every platform (linux, macOS, Windows), including the nightly scheduled run. The failing test isTestAccept/experimental/aitools/skills/update-prune, introduced in #5772.The test creates
~/.claude, so Claude Code is detected andaitools updatenow tries to migrate raw skills to the native databricks plugin by shelling out toclaude plugin install. The captured output depends on whether the realclaudebinary is on the host PATH:claudeinstalled record the marketplaceinstall-failederror (this is what got committed via-update).claude, so they produce acli-not-on-patherror.The golden output only matched on a machine with
claudepresent, so CI diverged and failed. This is a hermeticity bug, not flakiness: it reproduces deterministically by removingclaudefrom PATH.Changes
Before: the test detected Claude Code, a plugin-capable agent, so
updateran the plugin migration and captured a machine-dependent error message from the realclaudeCLI.Now: the test detects Cursor, a skills-only agent (no databricks plugin), so
updateexercises only the raw-skill prune path (the test's actual subject: "alpha updates, beta is pruned"). No plugin-capable agent CLI is invoked, so the output no longer depends on the host PATH.The plugin migration path itself is already covered hermetically by unit tests in
cmd/aitools/update_test.go(with mocked seams), so no coverage is lost.Test plan
claudefrom PATH.go test ./acceptance -run TestAccept/experimental/aitools/skills/update-prunepasses both withclaudepresent and absent from PATH.go test ./acceptance -run TestAccept/experimental/aitools.go test ./cmd/aitools/... ./libs/aitools/...../task checks,./task lint-q,./task fmt-qall clean.This pull request and its description were written by Isaac.