Uh oh!
There was an error while loading. Please reload this page.
[VPEX][5b] Add local-env six-phase pipeline orchestrator - #5851
Conversation
Integration test reportCommit: 2ef43e0
10 interesting tests: 4 SKIP, 3 RECOVERED, 2 flaky, 1 KNOWN
Top 10 slowest tests (at least 2 minutes):
|
Split from the pipeline layer so the PackageManager seam (implemented by the uv backend in a later PR), manager detection, and the writability preflight land as a small, independently reviewable unit. - pkgmanager.go: the PackageManager interface the pipeline provisions through. - detect.go: uv-vs-not-uv detection (biased toward uv, whose native project file is the pyproject.toml this command drives), the non-blaming guidance message for unsupported managers, and the ensureWritable preflight. Co-authored-by: Isaac
The orchestrator runs the layers in order (preflight → resolve → fetch → merge → provision → validate), records per-phase status into Result, and returns typed PipelineErrors carrying FailurePhase and DiskMutated. Under --check it computes and reports the plan (with a diff) and performs no writes — skipping the writability probe and package-manager availability, neither of which is needed to compute the plan. Backups are created only when no .bak exists and the original is a genuine os.ErrNotExist-free read; an unreadable or unstattable existing file fails loudly rather than being misread as greenfield and overwritten. Stacked on the package-manager/detection layer; provisions through the PackageManager interface against a fake in tests. Co-authored-by: Isaac
cb166cb to
f24c320Compareaf9d41b to
c2e8b69Compare| } | ||
| if err := os.WriteFile(dst, data, 0o644); err != nil { | ||
| return fmt.Errorf("write %s: %w", dst, err) | ||
| } |
There was a problem hiding this comment.
copyFile creates the .bak at a hardcoded 0o644. If the user's pyproject.toml is locked down (e.g. 0o600 because it carries a private index URL or token), the backup silently becomes world-readable.
The main-file write at applyMerge is fine only incidentally — os.WriteFile ignores the mode arg when the file already exists, so the existing pyproject keeps its perms. But the backup is freshly created here, so 0o644 actually applies.
Suggest either preserving the source mode (os.Stat(src) → pass info.Mode()) or, if 0o644 is deliberate, a one-line comment saying so (the repo's comment-the-why rule).
This comment was generated with GitHub MCP.
There was a problem hiding this comment.
Good catch — fixed in 6f31579. copyFile now stats the source and creates the backup with info.Mode().Perm(), so a 0o600 pyproject (private index URL/token) is no longer widened to world-readable. Added TestCopyFilePreservesMode to lock it in.
| // Phase: preflight — manager detection, writability, package-manager availability. | ||
| // P0 supports only uv; any other detected manager is a clean, non-blaming exit. | ||
| if m := detectManager(p.ProjectDir); m != managerUv { | ||
| return p.fail(PhasePreflight, false, NewError(ErrManagerUnsupported, nil, "%s", managerGuidance(m))) |
There was a problem hiding this comment.
Two preflight exits look untested at the pipeline level:
- this unsupported-manager path →
E_MANAGER_UNSUPPORTED, and - the
EnsureAvailablefailure below →E_UV_MISSING(line ~113).
detect_test.go covers detectManager in isolation, but not the pipeline's wiring of it into a clean preflight failure (correct phase attribution, DiskMutated=false, later phases still pending). The noProvisionPM fixture already exists — a test that drops an environment.yml and asserts the E_MANAGER_UNSUPPORTED / PhasePreflight result would close the gap cheaply.
This comment was generated with GitHub MCP.
There was a problem hiding this comment.
Done in 6f31579. Added TestPipelineManagerUnsupportedFailsAtPreflight (drops an environment.yml → E_MANAGER_UNSUPPORTED) and TestPipelineUvMissingFailsAtPreflight (new uvMissingPM fixture → E_UV_MISSING). Both assert phase attribution (PhasePreflight), DiskMutated=false, and that all later phases stay pending, via a shared assertPreflightFailure helper.
Address review on the pipeline PR: - copyFile created the .bak at a hardcoded 0o644, silently widening a locked-down pyproject.toml (e.g. 0o600 carrying a private index URL) to world-readable. Stat the source and create the backup with its permission bits; os.WriteFile applies the mode only when creating the file, which is always the case for the fresh .bak. - Add pipeline-level tests for the two preflight exits that were only covered in isolation before: E_MANAGER_UNSUPPORTED (a non-uv project) and E_UV_MISSING (package manager unavailable), asserting phase attribution, DiskMutated=false, and that later phases stay pending. Add a direct copyFile mode-preservation test. Co-authored-by: Isaac
anton-107
left a comment
There was a problem hiding this comment.
Nice work — phase/error attribution is airtight, the backup-safety branches are exactly right, and --check correctly avoids touching the machine or disk. My concerns below are about behavior, not code hygiene. The first is the headline one.
1. Re-basing the merge on .bak silently discards user edits made after the first sync
mergePlan uses the backup as the canonical merge base whenever a .bak exists (pipeline.go:227-238), and TestPipelineRestoresBackupBeforeMerge asserts this is intended. The consequence is that on re-runs the merge no longer starts from the livepyproject.toml — it starts from the pristine original captured on the first sync.
I confirmed the fallout with a scratch test:
- Sync once (creates
.bak). - Developer hand-edits the live
pyproject.toml— e.g. addsdependencies = ["rich", "requests"]. - Sync again (e.g. after switching compute target).
- The
requestsedit is gone — the second merge re-based on.bak, which never saw it.
This defeats the purpose of the byte-preserving MergeManaged (which exists specifically to touch only the three managed regions and preserve everything else). And re-basing on .bak buys nothing for the managed regions: MergeManaged is idempotent on its own output and preserves non-managed deps, so merging onto the current live file produces identical managed-region results — without throwing away edits. The only marginal difference is self-healing a databricks-connect pin the user manually deleted, which seems like an odd thing to optimize for at the cost of silent edit loss.
The .bak original is recoverable, but edits made between syncs are not — and that's the normal workflow (sync, add a dep, re-sync). Could you confirm whether this is truly intended? I'd argue the live file should be the merge base and .bak should remain purely a one-time safety copy. There's also no test covering "second run preserves a non-managed user edit" — today that test would fail.
2. --check reports a plan a real re-run wouldn't perform (downstream of #1)
On a re-run where .bak already exists, two things in the plan are inaccurate:
plan.WouldBackupis always set for a non-greenfield project (pipeline.go:297-299), but a real re-run keeps the existing.bakand does not write a backup (applyMerge,pipeline.go:317-320). So--checkclaims a backup that won't happen.- The diff base is
.bak, not the live file (pipeline.go:279-289). On an idempotent re-run the live file already equals the merged output — a real re-run changes nothing — yet--checkprints a full diff against the original. I confirmed both: the plan shows the whole managed block as an addition even though the on-disk file is already identical to what the run would write.
Lower severity since it's advisory output, but a dry-run whose plan doesn't match the real run is misleading. Fixing #1 fixes this too.
3. constraints-only doesn't "omit the databricks-connect dependency entirely" on existing projects
The PR description says constraints-only "omits the databricks-connect dependency entirely," and the pipeline clears dbcPin (pipeline.go:144-147). That's accurate for greenfield (dev = []). But on an existing project that already pins databricks-connect, the empty value makes mergeDatabricksConnect a no-op, so the existing pin is retained (confirmed by scratch test). This is intended per the merge.go comment ("left untouched rather than ... blanked out") and is defensible — but the description over-claims. Please align the wording, or document that constraints-only only stops managing the pin rather than removing it.
Address review on the pipeline PR (concerns #1 and #2). Re-basing the merge on .bak silently discarded edits the user made to pyproject.toml between syncs: mergePlan used .bak as the merge base whenever one existed, so a re-run started from the pristine pre-sync original and any dependency the user added afterward was lost. MergeManaged rewrites only the three managed regions and is idempotent on its own output, so merging onto the live file yields identical managed regions without throwing away edits. - mergePlan now reads the live pyproject.toml as the merge base; .bak stays a one-time safety copy of the pre-sync original. - --check no longer reports a backup on a re-run (applyMerge keeps the existing .bak, so WouldBackup is set only when no .bak exists yet), and the diff is now against the live file, so an idempotent re-run reports no change — matching what a real run would do. - Replace TestPipelineRestoresBackupBeforeMerge (which encoded the old discard-edits behavior) with TestPipelineMergesOnLiveFileNotBackup, and add TestPipelineCheckReRunPlanMatchesRealRun. Also clarify the constraints-only comment: it stops managing the databricks-connect pin (greenfield dev = []; existing pin retained) rather than removing it, matching mergeDatabricksConnect's no-op-on-empty behavior. Co-authored-by: Isaac
rugpanov
commented
Jul 10, 2026
Verified all three against the code (reproduced #1 and #3 with scratch tests first) and pushed fixes in 2ef43e0. #1 — re-basing on
#2 —
#3 — All gates green (build, test, lint, deadcode, gofmt). #5854 was cascade-rebased on top. |
anton-107
left a comment
There was a problem hiding this comment.
Approving — the orchestration design is well-defended (correct phase ordering, first-error-aborts, fail-closed greenfield detection, backup safety). Two correctness issues worth a look before this hits the core existing-project flow; left them inline. Neither blocks the design.
| if greenfield { | ||
| // No existing pyproject.toml — render a fresh one. The project name comes | ||
| // from the directory name as a reasonable default. | ||
| projectName := filepath.Base(p.ProjectDir) |
There was a problem hiding this comment.
Greenfield renders an invalid .name when ProjectDir is a relative or root path.
projectName := filepath.Base(p.ProjectDir) — when ProjectDir is . (the common case of running from the project's own directory) filepath.Base returns .; "" also returns ., and / returns /. RenderFreshPyproject then writes name = "." verbatim, which isn't a valid PEP 621/PEP 508 name (must start and end with an alphanumeric), so uv sync rejects the generated file and provisioning fails. ProjectDir isn't normalized anywhere in this PR.
Suggest normalizing to an absolute path before taking the base (e.g. filepath.Abs then filepath.Base) so ./"" resolve to the real directory name, and falling back to a safe default (or sanitizing to a valid PEP 508 identifier) when the base still isn't usable.
There was a problem hiding this comment.
Confirmed and fixed in #5854 (commit 4bd0477) — this PR was already queued to merge, so the fix lands in the stacked follow-up rather than reopening this one. projectName() now resolves ./""/root via filepath.Abs before taking the base and sanitizes to a valid PEP 508 name (non-alnum runs → -, trimmed), falling back to app when nothing usable remains. Covered by TestProjectName and TestPipelineGreenfieldFromDotDirRendersValidName.
| "cannot determine databricks-connect major version from pin %q", dbcPin)) | ||
| } | ||
| installedMajor := majorVersion(dbcVer) | ||
| if installedMajor == "" { |
There was a problem hiding this comment.
Default-mode sync fails for an existing project that doesn't already pin databricks-connect.
For a non-greenfield project the merge goes through mergeDatabricksConnect, which only replaces an existing databricks-connect element — it never inserts one. It returns a no-op (lines, false) in every no-entry branch: no [dependency-groups] table, no dev key, single-line array without the token, or multi-line with no match (merge.go:241-292). So an existing pyproject.toml with a dev group but no databricks-connect is written back with no pin added; uv sync installs nothing; and the default-mode assertion here fails with cannot determine installed databricks-connect major version (E_VALIDATE).
Greenfield only works because RenderFreshPyproject inserts the pin — a path the existing-project merge lacks. Suggest having the merge insert a databricks-connect entry into [dependency-groups].dev (creating the group/key when absent) when none exists, mirroring RenderFreshPyproject, rather than treating a missing entry as a no-op.
There was a problem hiding this comment.
Confirmed (reproduced: existing project with a dev group but no databricks-connect → no-op → E_VALIDATE) and fixed in #5854 (commit 4bd0477). mergeDatabricksConnect now inserts the pin when absent, mirroring RenderFreshPyproject: into the dev array (single- and multi-line), and creating the dev key / [dependency-groups] table when those are missing. Constraints-only (empty value) stays a no-op, and the insert path is idempotent. Covered by TestMergeInserts{DatabricksConnectMultiLine,SingleLine,EmptyDev,DevKeyWhenAbsent,DependencyGroupsWhenAbsent} and TestMergeConstraintsOnlyDoesNotInsertDatabricksConnect.
Address two correctness nits from review of the pipeline PR (#5851). Since that PR was already queued to merge, the fixes land here in the follow-up. - mergeDatabricksConnect only ever *replaced* an existing databricks-connect element, so a default-mode sync of an existing project that didn't already pin databricks-connect wrote the file back with no pin; uv installed nothing and validate failed with E_VALIDATE. It now *inserts* the pin — into the dev array (single- or multi-line), creating the dev key and the [dependency-groups] table when absent — mirroring RenderFreshPyproject. Constraints-only (empty value) stays a no-op; the insert path is idempotent. - Greenfield rendered name = filepath.Base(ProjectDir), which is "." when run from the project's own directory (and "/" at root) — not a valid PEP 621 name, so uv sync rejected the file. projectName() now resolves via filepath.Abs and sanitizes to a valid PEP 508 identifier, falling back to a default when nothing usable remains. Co-authored-by: Isaac
Uh oh!
There was an error while loading. Please reload this page.
Address two correctness nits from review of the pipeline PR (#5851). Since that PR was already queued to merge, the fixes land here in the follow-up. - mergeDatabricksConnect only ever *replaced* an existing databricks-connect element, so a default-mode sync of an existing project that didn't already pin databricks-connect wrote the file back with no pin; uv installed nothing and validate failed with E_VALIDATE. It now *inserts* the pin — into the dev array (single- or multi-line), creating the dev key and the [dependency-groups] table when absent — mirroring RenderFreshPyproject. Constraints-only (empty value) stays a no-op; the insert path is idempotent. - Greenfield rendered name = filepath.Base(ProjectDir), which is "." when run from the project's own directory (and "/" at root) — not a valid PEP 621 name, so uv sync rejected the file. projectName() now resolves via filepath.Abs and sanitizes to a valid PEP 508 identifier, falling back to a default when nothing usable remains. Co-authored-by: Isaac
Address two correctness nits from review of the pipeline PR (#5851). Since that PR was already queued to merge, the fixes land here in the follow-up. - mergeDatabricksConnect only ever *replaced* an existing databricks-connect element, so a default-mode sync of an existing project that didn't already pin databricks-connect wrote the file back with no pin; uv installed nothing and validate failed with E_VALIDATE. It now *inserts* the pin — into the dev array (single- or multi-line), creating the dev key and the [dependency-groups] table when absent — mirroring RenderFreshPyproject. Constraints-only (empty value) stays a no-op; the insert path is idempotent. - Greenfield rendered name = filepath.Base(ProjectDir), which is "." when run from the project's own directory (and "/" at root) — not a valid PEP 621 name, so uv sync rejected the file. projectName() now resolves via filepath.Abs and sanitizes to a valid PEP 508 identifier, falling back to a default when nothing usable remains. Co-authored-by: Isaac
eng-dev-ecosystem-bot
commented
Jul 13, 2026
Integration test reportCommit: a28e610
18 interesting tests: 12 flaky, 2 FAIL, 2 SKIP, 1 KNOWN, 1 RECOVERED
Top 50 slowest tests (at least 2 minutes):
|
Address two correctness nits from review of the pipeline PR (#5851). Since that PR was already queued to merge, the fixes land here in the follow-up. - mergeDatabricksConnect only ever *replaced* an existing databricks-connect element, so a default-mode sync of an existing project that didn't already pin databricks-connect wrote the file back with no pin; uv installed nothing and validate failed with E_VALIDATE. It now *inserts* the pin — into the dev array (single- or multi-line), creating the dev key and the [dependency-groups] table when absent — mirroring RenderFreshPyproject. Constraints-only (empty value) stays a no-op; the insert path is idempotent. - Greenfield rendered name = filepath.Base(ProjectDir), which is "." when run from the project's own directory (and "/" at root) — not a valid PEP 621 name, so uv sync rejected the file. projectName() now resolves via filepath.Abs and sanitizes to a valid PEP 508 identifier, falling back to a default when nothing usable remains. Co-authored-by: Isaac
Address two correctness nits from review of the pipeline PR (#5851). Since that PR was already queued to merge, the fixes land here in the follow-up. - mergeDatabricksConnect only ever *replaced* an existing databricks-connect element, so a default-mode sync of an existing project that didn't already pin databricks-connect wrote the file back with no pin; uv installed nothing and validate failed with E_VALIDATE. It now *inserts* the pin — into the dev array (single- or multi-line), creating the dev key and the [dependency-groups] table when absent — mirroring RenderFreshPyproject. Constraints-only (empty value) stays a no-op; the insert path is idempotent. - Greenfield rendered name = filepath.Base(ProjectDir), which is "." when run from the project's own directory (and "/" at root) — not a valid PEP 621 name, so uv sync rejected the file. projectName() now resolves via filepath.Abs and sanitizes to a valid PEP 508 identifier, falling back to a default when nothing usable remains. Co-authored-by: Isaac
…bricks-connect insertion (databricks#5854) ## Why Follow-up fixes to the `local-env python sync` pipeline (databricks#5851, now merged), from codex and reviewer feedback. Three independent correctness issues in the merge / dry-run paths, none of which the pipeline PR's own tests caught: 1. **`--check` still wrote to disk.** `FetchConstraints` populated the on-disk constraint cache on every successful live fetch, so a dry run mutated the cache directory even though the pipeline skips every other write-side step under `--check`. 2. **Greenfield rendered an invalid project name.** `filepath.Base(ProjectDir)` is `.` when run from the project's own directory (and `/` at root) — not a valid PEP 621/508 name, so `uv sync` rejected the generated `pyproject.toml`. 3. **Default-mode sync failed for an existing project without a databricks-connect pin.** The merge only *replaced* an existing pin; it never *inserted* one, so an existing `pyproject.toml` with a dev group but no databricks-connect was written back unchanged, `uv` installed nothing, and validation failed with `E_VALIDATE`. Greenfield worked only because `RenderFreshPyproject` inserts the pin — a path the existing-project merge lacked. ## What - **`--check` no cache write** — thread a `writeCache bool` through `FetchConstraints`; the pipeline passes `!p.Check`. An existing cache is still *read* for offline fallback (reading is not a mutation); only the write is suppressed under a dry run. - **Valid greenfield name** — `projectName()` resolves `.`/`""`/root via `filepath.Abs` before taking the base and sanitizes to a valid PEP 508 identifier (non-alphanumeric runs collapse to `-`, trimmed), falling back to `app` when nothing usable remains. - **Insert databricks-connect when absent** — `mergeDatabricksConnect` now inserts the pin into `[dependency-groups].dev` (single- or multi-line array), creating the `dev` key and the `[dependency-groups]` table when absent, mirroring `RenderFreshPyproject`. Constraints-only (empty value) stays a no-op. Detection is comment-aware and spans the whole array, so a pin on the `dev = [` line or one carrying a trailing comment is rewritten in place rather than duplicated (a duplicate-pin bug codex caught in the first cut of this change). The insert path is idempotent. ## Testing strategy - `--check` purity: `TestFetchConstraintsSkipsCacheWriteWhenDisabled`, and `TestPipelineCheckMutatesNothing` strengthened to assert the cache dir stays empty; `TestPipelineCheckReRunPlanMatchesRealRun` for accurate re-run plans. - Greenfield name: `TestProjectName`, `TestPipelineGreenfieldFromDotDirRendersValidName`. - databricks-connect insertion: `TestMergeInserts{DatabricksConnectMultiLine,SingleLine,EmptyDev,DevKeyWhenAbsent,DependencyGroupsWhenAbsent}`, no-duplicate regressions `TestMergeReplacesDatabricksConnect{OnDevLine,WithTrailingComment}`, and `TestMergeConstraintsOnlyDoesNotInsertDatabricksConnect`. - Gates: `go build`, `go test`, `golangci-lint` (0 issues), `deadcode`, `gofmt` — all green. --- ## About this stack Follow-up to the `databricks local-env python sync` stack; targets `main` now that the pipeline PR (databricks#5851) has merged. This pull request and its description were written by Isaac.
…ks#5832) ## Why - The engine needs a real `PackageManager` implementation (uv) and a CLI entry point so a user can actually run the feature. - Wiring it in while keeping it `Hidden` lets the command be dogfooded and exercised by acceptance tests without becoming user-visible until the stack is complete. - This is the first layer reachable from `main`, so it also makes the whole `libs/localenv` package live for the `deadcode` checker. ## What - **`libs/localenv/uv.go`** — the uv implementation of `PackageManager`: discover/install uv, install the Python minor, `uv sync`, seed pip into the venv, validate; plus the `pip.conf` → `UV_INDEX_URL` bridge for Databricks-managed machines. - **`cmd/localenv/`** — the command tree matching `local-env python sync`: a `local-env` group (`Hidden: true`), a `python` subgroup, and the `sync` verb. Parent nodes use `root.ReportUnknownSubcommand`; `sync` uses `cobra.NoArgs`, resolves flags/bundle target, builds the `Pipeline` with the uv manager, and renders text or `--json`. All Cobra `Use` values + the `--json` command field come from the `libs/localenv` constants. - **`cmd/cmd.go`** — registers the group. ## Testing strategy - Unit tests for uv helper logic (discovery, `pip.conf` index-url, arg builders, stderr surfacing) (`uv_test.go`). - Runtime smoke test of the hidden 3-level command: absent from top-level help; help works at each level; unknown subcommand exits non-zero; bare group shows help; flags + mutual-exclusion + `NoArgs` behave. - Gates: `go build ./...`, `go test`, `golangci-lint`, `deadcode` (whole tree, no pragmas), `gofmt` — all green. - `cmd/localenv/` unit tests are intentionally deferred to the acceptance PR (databricks#5833), per the repo convention that user-visible CLI output is covered by acceptance tests. --- ## About this stack This is one of a series of small, stacked PRs that together add the `databricks local-env python sync` command — it provisions a local Python environment (Python version, `databricks-connect` pin, and dependency constraints) matched to a selected Databricks compute target. The work was split from one large branch into single-concern layers so each is independently reviewable; the command is kept hidden until the final PR so nothing is user-visible mid-stack. **Review bottom-up.** Each PR targets the previous one as its base branch (this PR targets `main`), so its diff shows only that layer. Layers 1–5 have merged; the original layer-5 PR (databricks#5828) was split into 5a/5b/5c during review. | # | PR | What | Status | |---|----|------|--------| | 1 | databricks#5823 | foundation: result types + env-key mapping | merged | | 2 | databricks#5824 | compute-target resolution | merged | | 3 | databricks#5826 | constraint fetch + offline cache | merged | | 4 | databricks#5827 | formatting-preserving pyproject.toml merge | merged | | 5a | databricks#5850 | package-manager interface + detection | merged | | 5b | databricks#5851 | six-phase pipeline orchestrator | merged | | 5c | databricks#5854 | --check cache purity, greenfield name, dbc insertion | merged | | 6 | **databricks#5832 ← you are here** | uv backend + CLI command (registered hidden) | | | 7 | databricks#5833 | acceptance tests | | | 8 | databricks#5835 | unveil (unhide + help + changelog) | | This pull request and its description were written by Isaac.
## Why - The command's user-visible behavior — text and `--json` output, and every error path — needs end-to-end coverage against the real CLI. - `cmd/localenv/` carries no unit tests by design, so acceptance tests are where that surface is verified (repo convention: user-visible CLI output is covered by acceptance tests). ## What - **`acceptance/localenv/`** — 9 scenarios driven through the (hidden) command against the in-process fake server: `help` (three-level tree), `no-target` (`E_NO_TARGET`), `flag-conflict` (Cobra mutual-exclusion), `manager-unsupported` (conda project → clean P1 exit), `env-unsupported` (404 → `E_ENV_UNSUPPORTED` at fetch), `json-error` (`--output json` error object), `serverless-check` (dry-run plan), `serverless-json` (`--json` plan), and `constraints-only`. - Scripts use `local-env python sync` and the `DATABRICKS_LOCALENV_CONSTRAINT_SOURCE` override; goldens show the `local-env python sync` command field and managed marker. No source changes. ## Testing strategy - Goldens generated with `-update` and verified **stable on a clean re-run** (no `-update`); all 9 subtests pass. - `musterr` guards the five expected-failure scenarios; `trace` shows the three output-producing ones. - Full acceptance suite run to confirm no regressions elsewhere (only pre-existing, environment-specific failures unrelated to this change). - Diff confined to `acceptance/localenv/`. - Independently verified by a review subagent (PASS — goldens, scripts, stubs, stale-refs, hygiene) and by codex (no issues). --- ## About this stack This is one of a series of small, stacked PRs that together add the `databricks local-env python sync` command — it provisions a local Python environment (Python version, `databricks-connect` pin, and dependency constraints) matched to a selected Databricks compute target. The work was split from one large branch into single-concern layers so each is independently reviewable; the command is kept hidden until the final PR so nothing is user-visible mid-stack. **Review bottom-up.** Layers 1–5 have merged (the original layer-5 PR databricks#5828 was split into 5a/5b/5c during review). | # | PR | What | Status | |---|----|------|--------| | 1 | databricks#5823 | foundation: result types + env-key mapping | merged | | 2 | databricks#5824 | compute-target resolution | merged | | 3 | databricks#5826 | constraint fetch + offline cache | merged | | 4 | databricks#5827 | formatting-preserving pyproject.toml merge | merged | | 5a | databricks#5850 | package-manager interface + detection | merged | | 5b | databricks#5851 | six-phase pipeline orchestrator | merged | | 5c | databricks#5854 | --check cache purity, greenfield name, dbc insertion | merged | | 6 | databricks#5832 | uv backend + CLI command (registered hidden) | | | 7 | **databricks#5833 ← you are here** | acceptance tests | | | 8 | databricks#5835 | unveil (unhide + help + changelog) | | This pull request and its description were written by Isaac.
Why
--checkdry-run mode.What
pipeline.go— the six-phase orchestrator (preflight → resolve → fetch → merge → provision → validate). It records per-phase status intoResultand returns typedPipelineErrors carryingFailurePhaseandDiskMutated. Under--checkit computes and reports the plan (with a diff) and performs no writes, skipping the writability probe and package-manager availability (neither is needed to compute the plan).pyproject.toml.MergeManagedrewrites only the three managed regions and is idempotent on its own output, so a re-run preserves edits the user made between syncs. The.bakis a one-time safety copy of the pre-sync original (created only when none exists, on the first sync of an existing project); an unreadable or unstattable existing file fails loudly rather than being misread as greenfield and overwritten.constraints-onlystops managing the databricks-connect pin rather than removing it: a greenfield project rendersdev = [](no databricks-connect), and an existing project that already pins databricks-connect keeps its pin untouched.Testing strategy
PackageManager+ stub compute +httptestserver:--checkmutates nothing (even on a read-only dir / without a package manager) and reports a plan that matches a real re-run (no spurious backup, empty diff when idempotent), greenfield vs. existing, the merge basing on the live file so between-sync edits survive, backup safety on unreadable/unstattable files, constraints-only behavior, preflight exits (E_MANAGER_UNSUPPORTED/E_UV_MISSING), and phase/error attribution (pipeline_test.go).go build,go test,golangci-lint(0 issues),deadcode,gofmt— all green.About this stack
Review bottom-up. This PR targets #5850 as its base, so its diff shows only the pipeline layer.
This PR and #5850 together supersede #5828.
This pull request and its description were written by Isaac.