Skip to content

aitools: make update-prune acceptance test hermetic#5784

Merged
simonfaltum merged 2 commits into
mainfrom
simonfaltum/fix-aitools-update-prune-ci
Jul 1, 2026
Merged

aitools: make update-prune acceptance test hermetic#5784
simonfaltum merged 2 commits into
mainfrom
simonfaltum/fix-aitools-update-prune-ci

Conversation

@simonfaltum

Copy link
Copy Markdown
Member

Why

The build job on main is failing on every platform (linux, macOS, Windows), including the nightly scheduled run. The failing test is TestAccept/experimental/aitools/skills/update-prune, introduced in #5772.

The test creates ~/.claude, so Claude Code is detected and aitools update now tries to migrate raw skills to the native databricks plugin by shelling out to claude plugin install. The captured output depends on whether the real claude binary is on the host PATH:

  • Developer machines with claude installed record the marketplace install-failed error (this is what got committed via -update).
  • CI runners have no claude, so they produce a cli-not-on-path error.

The golden output only matched on a machine with claude present, so CI diverged and failed. This is a hermeticity bug, not flakiness: it reproduces deterministically by removing claude from PATH.

Changes

Before: the test detected Claude Code, a plugin-capable agent, so update ran the plugin migration and captured a machine-dependent error message from the real claude CLI.

Now: the test detects Cursor, a skills-only agent (no databricks plugin), so update exercises 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

  • Reproduced the CI failure locally by removing claude from PATH.
  • go test ./acceptance -run TestAccept/experimental/aitools/skills/update-prune passes both with claude present and absent from PATH.
  • Full aitools acceptance suite: go test ./acceptance -run TestAccept/experimental/aitools.
  • Unit tests: go test ./cmd/aitools/... ./libs/aitools/....
  • ./task checks, ./task lint-q, ./task fmt-q all clean.

This pull request and its description were written by Isaac.

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
@simonfaltum simonfaltum temporarily deployed to test-trigger-is July 1, 2026 08:31 — with GitHub Actions Inactive
# 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"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, you can use $HOME, it is also set on Windows (see acceptance/script.prepare).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, thanks. Switched to $HOME and dropped the USERPROFILE fallback in 9668081.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@simonfaltum simonfaltum temporarily deployed to test-trigger-is July 1, 2026 09:28 — with GitHub Actions Inactive
@simonfaltum simonfaltum added this pull request to the merge queue Jul 1, 2026
Merged via the queue into main with commit 7c8fa11 Jul 1, 2026
23 checks passed
@simonfaltum simonfaltum deleted the simonfaltum/fix-aitools-update-prune-ci branch July 1, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants