Repository navigation
Add databricks apps init --package-manager (pnpm default, npm supported) - #6902
Conversation
Integration test reportCommit: 99e7ebd
Top 13 slowest tests (at least 2 minutes):
|
48512a3 to
0ccf915
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Lockless installs, version-like branches, and Yarn/Bun argument rewrites currently produce incorrect behavior.
Review effort: Balanced
Findings: 3
Open (4)
What changed in this PR
Adds pnpm/npm selection to AppKit project initialization, including package metadata rewriting, artifact pruning, installs, compatibility fallback, and tests.
Changes:
- Add
--package-manager, defaulting to pnpm. - Rewrite scripts/pins and select manager-specific install behavior.
- Add unit and acceptance coverage for selection, compatibility, and pruning.
| File | Description |
|---|---|
libs/apps/pkgmanager/scripts.go |
Implements script rewriting. |
libs/apps/pkgmanager/scripts_test.go |
Tests script conversion. |
libs/apps/pkgmanager/manager.go |
Defines manager behavior and compatibility. |
libs/apps/pkgmanager/manager_test.go |
Tests manager selection, pins, and pruning. |
libs/apps/initializer/nodejs.go |
Uses the selected manager for Node.js. |
libs/apps/initializer/nodejs_test.go |
Tests missing manager handling. |
libs/apps/initializer/initializer.go |
Passes manager configuration to Node.js. |
libs/apps/initializer/initializer_test.go |
Tests manager-specific commands. |
cmd/apps/init.go |
Adds the flag and integrates manager handling. |
cmd/apps/init_test.go |
Tests init integration and background installs. |
acceptance/cmd/apps/init/package_manager/test.toml |
Configures package-manager acceptance tests. |
acceptance/cmd/apps/init/package_manager/template/package.json |
Provides a dual-manager fixture. |
acceptance/cmd/apps/init/package_manager/template/databricks.yml |
Provides bundle fixture configuration. |
acceptance/cmd/apps/init/package_manager/template/appkit.plugins.json |
Provides plugin metadata. |
acceptance/cmd/apps/init/package_manager/template/app.yaml.tmpl |
Tests manager template rendering. |
acceptance/cmd/apps/init/package_manager/standard-template/package.json |
Provides a standard package fixture. |
acceptance/cmd/apps/init/package_manager/standard-template/databricks.yml.tmpl |
Provides rendered bundle configuration. |
acceptance/cmd/apps/init/package_manager/standard-template/appkit.plugins.json |
Provides standard plugin metadata. |
acceptance/cmd/apps/init/package_manager/standard-template/app.yaml.tmpl |
Tests standard manager rendering. |
acceptance/cmd/apps/init/package_manager/script |
Exercises package-manager scenarios. |
acceptance/cmd/apps/init/package_manager/output.txt |
Records expected acceptance output. |
acceptance/cmd/apps/init/package_manager/out.test.toml |
Records generated test configuration. |
acceptance/cmd/apps/init/package_manager/npm/main.go |
Implements a fake npm executable. |
acceptance/cmd/apps/init/package_manager/npm-only-template/package.json |
Provides an npm-only fixture. |
acceptance/cmd/apps/init/package_manager/npm-only-template/package-lock.json |
Provides its npm lockfile. |
acceptance/cmd/apps/init/package_manager/install-template/package.json.tmpl |
Tests rendered npm installation. |
acceptance/cmd/apps/init/package_manager/install-template/package-lock.json |
Provides its install lockfile. |
acceptance/cmd/apps/init/package_manager/cache/compat-manifest.json |
Tests embedded-version fallback. |
acceptance/cmd/apps/init/package_manager_prune/test.toml |
Configures pruning tests. |
acceptance/cmd/apps/init/package_manager_prune/template/pnpm-workspace.yaml |
Provides pnpm workspace metadata. |
acceptance/cmd/apps/init/package_manager_prune/template/pnpm-lock.yaml |
Provides a pnpm lockfile. |
acceptance/cmd/apps/init/package_manager_prune/template/package.json |
Provides the pruning package fixture. |
acceptance/cmd/apps/init/package_manager_prune/template/package-lock.json |
Provides an npm lockfile to prune. |
acceptance/cmd/apps/init/package_manager_prune/template/npm-shrinkwrap.json |
Provides an npm shrinkwrap fixture. |
acceptance/cmd/apps/init/package_manager_prune/template/app.yaml.tmpl |
Tests manager rendering during pruning. |
acceptance/cmd/apps/init/package_manager_prune/shrinkwrap-template/pnpm-lock.yaml |
Provides a competing pnpm lockfile. |
acceptance/cmd/apps/init/package_manager_prune/shrinkwrap-template/package.json |
Provides a shrinkwrap package fixture. |
acceptance/cmd/apps/init/package_manager_prune/shrinkwrap-template/npm-shrinkwrap.json |
Provides the selected shrinkwrap. |
acceptance/cmd/apps/init/package_manager_prune/script |
Exercises artifact pruning. |
acceptance/cmd/apps/init/package_manager_prune/output.txt |
Records expected pruning output. |
acceptance/cmd/apps/init/package_manager_prune/out.test.toml |
Records generated pruning configuration. |
.nextchanges/cli/apps-init-package-manager.md |
Adds the user-facing changelog entry. |
Files not reviewed (7)
- acceptance/cmd/apps/init/package_manager/install-template/package-lock.json: Generated file
- acceptance/cmd/apps/init/package_manager/npm-only-template/package-lock.json: Generated file
- acceptance/cmd/apps/init/package_manager_prune/shrinkwrap-template/npm-shrinkwrap.json: Generated file
- acceptance/cmd/apps/init/package_manager_prune/shrinkwrap-template/pnpm-lock.yaml: Generated file
- acceptance/cmd/apps/init/package_manager_prune/template/npm-shrinkwrap.json: Generated file
- acceptance/cmd/apps/init/package_manager_prune/template/package-lock.json: Generated file
- acceptance/cmd/apps/init/package_manager_prune/template/pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ger flag + template var Co-authored-by: Isaac <no-reply@databricks.com>
…ability map Co-authored-by: Isaac <no-reply@databricks.com>
…packageManager pin + scripts normalization) Co-authored-by: Isaac <no-reply@databricks.com>
…ll + InitializerNodeJs route through capability map) Co-authored-by: Isaac <no-reply@databricks.com>
…gelog fragment Co-authored-by: Isaac <no-reply@databricks.com>
…confirmed) Co-authored-by: Isaac <no-reply@databricks.com>
5ef7127 to
1df3054
Compare
simonfaltum
left a comment
There was a problem hiding this comment.
Nice work on this, the acceptance coverage is really thorough 😄 A few things came up in review that I think we should sort out before it ships, roughly ordered by impact. Isaac reproduced 1, 4 and 5 against the branch, so there are repro details below.
Before shipping
1. Missing pnpm fails after the project is created
Run apps init with Node and npm on the PATH but no pnpm, which I think is the most common setup. It gets through "Creating project" and then fails with:
Error: Failed to install dependencies: pnpm is unavailable; install it and retry, or use --skip-install to scaffold without running setup: exec: "pnpm": executable file not found in $PATH
The app directory stays on disk, so following the advice and retrying fails with directory myapp already exists. On main, a missing npm only gave a warning. I think we should check for the binary up front, next to the ValidateTemplate call in runCreate, before anything is written. The error could point to --package-manager npm, corepack enable pnpm, or --skip-install.
2. Default pnpm breaks existing npm custom templates
Custom templates (--template or DATABRICKS_APPKIT_TEMPLATE_PATH) that only ship a package-lock.json now fail unless the user adds --package-manager npm. The "Npm-only custom template rejects pnpm before copying" acceptance case shows it. My concern is that people with an existing npm template get broken by a CLI upgrade without changing anything on their side.
3. Downgrade warning on every default init
cli-compat.json currently resolves to AppKit 0.57.0. That means every plain databricks apps init prints Package manager "pnpm" is not supported for AppKit version 0.57.0, using npm instead, even though the user never asked for pnpm. The fallback-app case in the acceptance output shows it.
I think 2 and 3 share one fix: default the flag to "" and check cmd.Flags().Changed("package-manager"). When the flag isn't set, pick the manager quietly from the template's lockfile and the version threshold. When it is set, keep the strict validation and the warning as they are now. What do you think?
4. package.json gets reformatted for the regular AppKit template
The new else if rewritePackageJSON(...) branch now sends the full AppKit template's package.json through a map[string]any round trip and json.MarshalIndent. Before this PR, only pre-rendered templates went through that path. Running rewritePackageJSON on a small package.json gives:
{
"dependencies": { // keys are now sorted, so "name" is no longer first
"react": "^18.0.0"
},
"name": "my-app",
"packageManager": "pnpm@11.0.8",
"scripts": {
"build": "tsc -b && vite build" // && gets HTML-escaped
},
"version": "0.1.0"
}It's valid JSON, but it's one of the first files people open in a new project. The acceptance tests read it through jq, which hides this. A json.Encoder with SetEscapeHTML(false) fixes the escaping. Keeping the key order would need an order-preserving decode.
Would be nice
5. The template's pnpm pin gets overwritten
Rewrite always sets packageManager to the hardcoded pnpm@11.0.8, so a template pinned to pnpm@11.2.0 comes out as 11.0.8. Once AppKit bumps pnpm, older CLIs would downgrade the pin, and it might no longer match the lockfile. I'm wondering if we should keep an existing pnpm@ pin and only fill in our default when it's missing?
6. Smoke tests could be their own PR
appkit_smoke_test.go, appkit_local_smoke_test.go and the README section add about 350 lines that CI doesn't run, since they sit behind a build tag. They're also pinned to a GitHub Actions run whose artifacts will expire, and the README notes they currently fail. I think the feature would be easier to land without them, and we can work out how to run them regularly in a follow-up.
7. Release order
There's no template-v0.82.0 tag in databricks/appkit yet. So the pnpm default won't take effect on the default path until that tag exists and cli-compat.json is bumped. Totally fine to land the CLI side first, but I think we want 1 fixed before that bump happens.
Smaller questions
- Most of
scripts.goexists because the 0.82.0 template hardcodespnpm run ...in its scripts. Sincepackage.jsonis already rendered as a Go template (it uses{{.projectName}}), could AppKit use{{.packageManager}}in the scripts likeapp.yamldoes? That could let us drop most of the rewriter, which I think would be a lot easier to maintain. - npm installs got
--include=devto handleNODE_ENV=production. I believe pnpm also skips devDependencies in that case. Should the pnpm install get an equivalent? --package-manageris silently ignored for Python templates. It's minor, but the repo convention is to error on flags that don't apply.- Nit:
npmInstallChand the "skipping background npm install" log message still say npm.
Happy to pair on any of this if it helps 😄
Integration test reportCommit: 31aaf37
65 interesting tests: 38 FAIL, 27 flaky
Top 50 slowest tests (at least 2 minutes):
|
## Release v1.20.0 ### Notable Changes * Remove the Terraform deployment engine. `bundle.engine: terraform` and `DATABRICKS_BUNDLE_ENGINE=terraform` now error, and a failed migration of existing Terraform state is reported as an error instead of falling back to Terraform. To keep deploying with Terraform, use Databricks CLI v1.19.x. ([#6888](#6888), [#6889](#6889)) ### CLI * `databricks aitools install` now supports Kiro, installing Databricks agent skills into its skills directory. ([#6908](#6908)) * Fixed `databricks api` corrupting integers larger than 2^53 (such as job and pipeline ids) — request bodies and responses now preserve them exactly. ([#6884](#6884)) * Added `--auth-mode` and `--set <plugin>.<resourceKey>.authMode=obo|sp|both` to `databricks apps init` so AppKit resources can be accessed on behalf of the user, by the service principal, or both. The default stays service principal. ([#6886](#6886)) * `databricks apps init` now requires a value for every field a service principal resource binding references, prompting for missing values in an interactive terminal and otherwise failing with the `--set` key to use, instead of creating a project with unset variables. ([#6903](#6903)) * Add `databricks apps init --package-manager <npm|pnpm>` to select the package manager for Node.js templates. Infer the default quietly from template lockfiles and AppKit version, check prerequisites before creating files, and preserve template formatting and pnpm version pins. ([#6902](#6902)) * Select npm or pnpm from `packageManager` declarations and lockfiles for `apps validate` and project validation during `apps deploy`. ([#6892](#6892)) * Fix `auth docker host` reporting the credential helper as configured when its executable is missing from `PATH`. ([#6880](#6880)) * Warn when the CLI binary was built more than 6 months ago and recommend updating. ([#6898](#6898)) ### AI Runtime * Add an experimental rank-partitioned container images to AI Runtime jobs. ([#6841](#6841)) * Support snapshot fields directly under `code_source` without requiring `type` or a nested `snapshot` block. ([#6927](#6927)) * Map AIR priority and Unity Catalog image fields when converting run configurations to bundles. ([#6905](#6905)) * Add workspace backend validation to `air run --dry-run`. ([#6934](#6934)) ### Bundles * Warn that `bundle.terraform` is deprecated and has no effect since the Terraform deployment engine was removed. ([#6940](#6940)) * Direct engine now detects and applies an explicitly configured zero-value boolean or float (e.g. `gcp_attributes.use_preemptible_executors: false`, `azure_attributes.spot_bid_max_price: 0`) added to a resource first deployed without the field, matching the existing handling of an explicit integer zero. ([#6882](#6882)) * Fix `bundle deployment migrate` failing with "no such file or directory" when the Terraform state has no resources or the configuration no longer declares any of them. ([#6958](#6958)) * `bundle run` and `pipelines run` now send the per-update `development` parameter for pipelines in development mode targets. Setting `development` on a pipeline is deprecated and now emits a warning; use `mode: development` instead. ([#6863](#6863)) * Remove the hidden `bundle debug terraform` command. ([#6933](#6933)) * Add support for `run_as.group_name` at the bundle and target levels for jobs and pipelines. ([#6676](#6676)) * Fix recreating a secret scope that was deleted outside of the bundle with the direct deployment engine. ([#6970](#6970)) * Accept title-case booleans (`True`/`False`, as rendered by Azure Pipelines) for boolean variables, and accept the same boolean strings (`yes`/`no`, `on`/`off`, ...) in Python bundles as in YAML. ([#6942](#6942)) ### Dependency Updates * Bump `github.com/databricks/databricks-sdk-go` from v0.182.0 to v0.185.0. ([#6928](#6928))
…0.0-1.19.x to appkit 0.76.1 (databricks#6988) ## Summary Update `internal/build/cli-compat.json`: | CLI version | AppKit | Agent Skills | |---|---|---| | 1.20.0 and newer (including dev builds) | `0.84.0` | `0.2.27` | | 1.0.0 through 1.19.x | `0.76.1` | `0.2.10` (unchanged) | | below 1.0.0 (`0.299.2`) | `0.24.0` (unchanged) | `0.1.5` (unchanged) | The new `1.20.0` entry is needed because AppKit 0.84.0 templates rely on `apps init` behavior that ships in CLI 1.20.0 (for example the pnpm package manager support from databricks#6902), so older CLIs stay on 0.76.1. Test updates that follow from the new versions: - `cmd/apps/init/package_manager` acceptance test: its local repository tags the embedded template version, so it now tags `template-v0.84.0`. Because 0.84.0 is at or above the pnpm dual-manager threshold (0.82.0), the "falls back to the embedded template" case now produces a pnpm app instead of npm. The explicit npm downgrade cases pin `--version 0.81.0` and are unchanged. - `TestGetSkillsRefLatestReleaseFallsBackToEmbeddedPin`: the embedded pin comes from the highest entry, so it now expects `v0.2.27`. ## Tests - `go test ./libs/clicompat/... -run TestEmbeddedManifest` passes. - Resolution checked for CLI 0.299.2, 1.0.0, 1.17.0, 1.19.0, 1.19.9 (AppKit 0.76.1), 1.20.0, 1.20.0-dev, 1.21.3 (AppKit 0.84.0). - `go test ./libs/clicompat/... ./libs/aitools/... ./cmd/aitools/... ./cmd/apps/... ./libs/apps/...` and the `apps`, `cmd/apps`, and aitools acceptance tests pass. - Tags `v0.76.1`, `template-v0.76.1`, `v0.84.0`, `template-v0.84.0` (databricks/appkit) and `v0.2.27` (databricks/databricks-agent-skills) exist. ## Checklist - [ ] Evals passed with no regressions - [x] `go test ./libs/clicompat/... -run TestEmbeddedManifest` passes This pull request and its description were written by Isaac. --------- Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com> Co-authored-by: MarioCadenas <MarioCadenas@users.noreply.github.com> Co-authored-by: Isaac <no-reply@databricks.com>


Changes
Add
databricks apps init --package-manager <npm|pnpm>, defaulting to pnpm. The selected manager controls template rendering, script rewrites, lockfile/config pruning, and foreground/background installs. Templates older than AppKit 0.82.0 use npm with a warning.Keep pnpm pinned to
pnpm@11.0.8. For npm installs, record the detectednpm --versioninpackage.json. With--skip-install, preserve an existing npm pin or remove a different manager’s pin without invoking npm.Why
AppKit templates support both managers. The CLI needs to keep the generated project and its install commands consistent. AppKit does not publish a canonical npm pin, so recording the version used for installation avoids writing an unrelated fixed version.
Tests
./task fmt,./task checks,./task lint, andgo test ./libs/apps/... ./cmd/appspassed../task testwas interrupted becauselibs/auth/u2m/TestChallengestalled with another local app occupying its hard-coded callback port 8020../task test-accpassed: 5,368 tests, 10 skipped.This PR and its description were written with Isaac and Codex.