From 22f99d96f349ee55ee22a1ef5ea9513b9f65cac6 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 3 Jul 2026 15:17:20 +0200 Subject: [PATCH 1/8] Add local-env foundation: result types and env-key mapping First of a stacked series adding `databricks local-env python sync`, which provisions a local Python environment matched to a Databricks compute target. The feature lands across small, single-concern PRs; each layer is independently reviewable and adds no user-facing surface until the final PR wires the command in. This PR is the foundation the rest of the stack builds on: - result.go: the result types and the --json / E_* error contract that every phase reports through (Result, PipelineError, ErrorCode, PhaseName, PhaseStatus, Mode, TargetInfo, ResolvedInfo, Plan, Warning), plus the command-path constants (local-env / python / sync) defined once. - envkey.go: mapping a compute target to an environment key and parsing the Python minor from a requires-python specifier. Nothing imports this package yet, so the CLI is unchanged. The unexported filesystem/artifact constants and the canonical phase-order slice live with the pipeline that consumes them (a later PR in the stack). Co-authored-by: Isaac --- libs/localenv/envkey.go | 34 +++++++ libs/localenv/envkey_test.go | 34 +++++++ libs/localenv/result.go | 180 +++++++++++++++++++++++++++++++++++ libs/localenv/result_test.go | 27 ++++++ 4 files changed, 275 insertions(+) create mode 100644 libs/localenv/envkey.go create mode 100644 libs/localenv/envkey_test.go create mode 100644 libs/localenv/result.go create mode 100644 libs/localenv/result_test.go diff --git a/libs/localenv/envkey.go b/libs/localenv/envkey.go new file mode 100644 index 00000000000..1e9171b35b3 --- /dev/null +++ b/libs/localenv/envkey.go @@ -0,0 +1,34 @@ +package localenv + +import ( + "fmt" + "regexp" + "strings" +) + +var pythonVersionRe = regexp.MustCompile(`(\d+)\.(\d+)`) + +// NormalizeServerless returns the canonical "vN" spelling of a serverless +// version accepting "4", "v4", or "V4". +func NormalizeServerless(version string) string { + return "v" + strings.TrimPrefix(strings.ToLower(version), "v") +} + +// EnvKeyForServerless returns the environment key for a serverless version. +func EnvKeyForServerless(version string) string { + return "serverless/serverless-" + NormalizeServerless(version) +} + +// EnvKeyForSparkVersion returns the environment key for a Spark version. +func EnvKeyForSparkVersion(sparkVersion string) string { + return "dbr/" + sparkVersion +} + +// PythonMinorFromRequires parses a PEP 440 requires-python string and extracts MAJOR.MINOR. +func PythonMinorFromRequires(requiresPython string) (string, error) { + match := pythonVersionRe.FindStringSubmatch(requiresPython) + if match == nil { + return "", fmt.Errorf("cannot parse python version from %q", requiresPython) + } + return fmt.Sprintf("%s.%s", match[1], match[2]), nil +} diff --git a/libs/localenv/envkey_test.go b/libs/localenv/envkey_test.go new file mode 100644 index 00000000000..cb276af4ea2 --- /dev/null +++ b/libs/localenv/envkey_test.go @@ -0,0 +1,34 @@ +package localenv + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEnvKeyForServerless(t *testing.T) { + for _, in := range []string{"4", "v4", "V4"} { + assert.Equal(t, "serverless/serverless-v4", EnvKeyForServerless(in)) + } +} + +func TestEnvKeyForSparkVersion(t *testing.T) { + assert.Equal(t, "dbr/15.4.x-scala2.12", EnvKeyForSparkVersion("15.4.x-scala2.12")) +} + +func TestPythonMinorFromRequires(t *testing.T) { + cases := map[string]string{ + "==3.12.*": "3.12", + ">=3.12": "3.12", + "==3.12.3": "3.12", + "~=3.11": "3.11", + } + for in, want := range cases { + got, err := PythonMinorFromRequires(in) + require.NoError(t, err) + assert.Equal(t, want, got) + } + _, err := PythonMinorFromRequires("garbage") + assert.Error(t, err) +} diff --git a/libs/localenv/result.go b/libs/localenv/result.go new file mode 100644 index 00000000000..82faed1b1dd --- /dev/null +++ b/libs/localenv/result.go @@ -0,0 +1,180 @@ +package localenv + +import "fmt" + +// Command path components, defined once so a rename touches a single place +// (spec §0 / invariant 8 / scenario 21). The cmd layer builds the Cobra +// command tree from CommandGroup/CommandSubgroup/CommandVerb; the --json +// "command" field uses CommandName. No other string re-spells the command path. +const ( + CommandGroup = "local-env" + CommandSubgroup = "python" + CommandVerb = "sync" + CommandName = CommandGroup + " " + CommandSubgroup + " " + CommandVerb + + // SchemaVersion is the version of the --json output contract (spec §6). + // Bump it on any breaking change to the JSON shape. + SchemaVersion = 1 +) + +// Mode is the provisioning mode: a full environment (default) or the +// constraints-only variant that omits the databricks-connect dependency. +type Mode int + +const ( + ModeDefault Mode = iota + ModeConstraintsOnly +) + +// String returns the JSON/text spelling of the mode ("default" | "constraints-only"). +func (m Mode) String() string { + if m == ModeConstraintsOnly { + return "constraints-only" + } + return "default" +} + +// PhaseName is a canonical execution phase (spec §3 / §6). The set is fixed and +// ordered; the --json "phases" array reports every phase in this order. +type PhaseName string + +const ( + PhasePreflight PhaseName = "preflight" + PhaseResolve PhaseName = "resolve" + PhaseFetch PhaseName = "fetch" + PhaseMerge PhaseName = "merge" + PhaseProvision PhaseName = "provision" + PhaseValidate PhaseName = "validate" +) + +// Phase status values (spec §6.2). +const ( + StatusOK = "ok" + StatusError = "error" + StatusPending = "pending" +) + +// ErrorCode is a stable failure-class identifier surfaced in --json error.code +// (spec §7). Values are compared via the ErrorCode constants, never by +// string-matching messages, and are defined once here. +type ErrorCode string + +const ( + ErrNoTarget ErrorCode = "E_NO_TARGET" + ErrManagerUnsupported ErrorCode = "E_MANAGER_UNSUPPORTED" + ErrUvMissing ErrorCode = "E_UV_MISSING" + ErrNotWritable ErrorCode = "E_NOT_WRITABLE" + ErrResolve ErrorCode = "E_RESOLVE" + ErrEnvUnsupported ErrorCode = "E_ENV_UNSUPPORTED" + ErrFetch ErrorCode = "E_FETCH" + ErrWrite ErrorCode = "E_WRITE" + ErrMerge ErrorCode = "E_MERGE" + ErrPythonInstall ErrorCode = "E_PYTHON_INSTALL" + ErrProvision ErrorCode = "E_PROVISION" + ErrValidate ErrorCode = "E_VALIDATE" +) + +// PipelineError is a failure carrying a stable code, the phase at which it +// occurred, and whether disk was mutated before the failure. It marshals to the +// --json error object (spec §6.2). Code and FailurePhase are the stable +// contract; Err holds the wrapped cause for errors.Is/As and is not serialized. +type PipelineError struct { + Code ErrorCode `json:"code"` + FailurePhase PhaseName `json:"failurePhase"` + Msg string `json:"message"` + DiskMutated bool `json:"diskMutated"` + Err error `json:"-"` +} + +func (e *PipelineError) Error() string { + if e.Err != nil { + return e.Msg + ": " + e.Err.Error() + } + return e.Msg +} + +func (e *PipelineError) Unwrap() error { + return e.Err +} + +// NewError creates a PipelineError with a code and message. FailurePhase and +// DiskMutated are filled in by the pipeline when it records the failure. The +// message is formatted with fmt.Sprintf(format, args...); err may be nil. +func NewError(code ErrorCode, err error, format string, args ...any) *PipelineError { + return &PipelineError{ + Code: code, + Msg: fmt.Sprintf(format, args...), + Err: err, + } +} + +// TargetInfo is the resolved compute target (spec §6 "target"). Source records +// which of the four precedence sources was used. SparkVersion is the raw cluster +// runtime string the resolver read; it is folded into EnvKey (dbr/) +// and is not part of the JSON contract, kept only as intermediate resolver state. +type TargetInfo struct { + Source string `json:"source"` + ClusterID string `json:"clusterId,omitempty"` + ServerlessVersion string `json:"serverlessVersion,omitempty"` + EnvKey string `json:"envKey"` + + SparkVersion string `json:"-"` +} + +// ResolvedInfo is the resolved environment definition (spec §6 "resolved"). +// DBConnectVersion is omitted in constraints-only mode. +type ResolvedInfo struct { + PythonVersion string `json:"pythonVersion"` + DBConnectVersion string `json:"dbconnectVersion,omitempty"` + ArtifactSource string `json:"artifactSource"` +} + +// Plan describes the changes a --check run would apply (spec §6.3). +// ChangedRegions is retained for text output only and is not serialized. +type Plan struct { + WouldWrite string `json:"wouldWrite"` + WouldBackup string `json:"wouldBackup,omitempty"` + WouldInstallPython string `json:"wouldInstallPython,omitempty"` + Diff string `json:"diff"` + + ChangedRegions []string `json:"-"` +} + +// PhaseStatus is one entry in the --json "phases" array (spec §6). Detail is +// used for human-readable text output only and is not serialized. +type PhaseStatus struct { + Phase PhaseName `json:"phase"` + Status string `json:"status"` + + Detail string `json:"-"` +} + +// Warning is a non-fatal advisory surfaced in --json "warnings" (spec §6). +type Warning struct { + Code string `json:"code"` + Message string `json:"message"` +} + +// Result is the full outcome of a sync run and the root of the --json object +// (spec §6). Field order matches the spec's schema so JSON key order is stable. +type Result struct { + SchemaVersion int `json:"schemaVersion"` + Command string `json:"command"` + OK bool `json:"ok"` + Mode string `json:"mode"` + DryRun bool `json:"dryRun"` + Target *TargetInfo `json:"target,omitempty"` + Resolved *ResolvedInfo `json:"resolved,omitempty"` + Greenfield bool `json:"greenfield"` + Plan *Plan `json:"plan,omitempty"` + VenvPath string `json:"venvPath,omitempty"` + Phases []PhaseStatus `json:"phases"` + Warnings []Warning `json:"warnings"` + Error *PipelineError `json:"error"` + BackupPath string `json:"backupPath,omitempty"` + // DurationMs is part of the §6 contract but reserved for now: the pipeline + // does not measure wall time (a real clock would make acceptance goldens + // non-deterministic), so it is always emitted as 0 until timing is wired + // through a clock the tests can control. + DurationMs int64 `json:"durationMs"` +} diff --git a/libs/localenv/result_test.go b/libs/localenv/result_test.go new file mode 100644 index 00000000000..76d30ec22bf --- /dev/null +++ b/libs/localenv/result_test.go @@ -0,0 +1,27 @@ +package localenv + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestPipelineErrorWrapsAndExposesCode(t *testing.T) { + base := errors.New("boom") + err := NewError(ErrFetch, base, "fetch %s", "x") + assert.Equal(t, "fetch x: boom", err.Error()) + assert.Equal(t, ErrFetch, err.Code) + assert.ErrorIs(t, err, base) +} + +func TestModeString(t *testing.T) { + assert.Equal(t, "default", ModeDefault.String()) + assert.Equal(t, "constraints-only", ModeConstraintsOnly.String()) +} + +func TestCommandName(t *testing.T) { + // The --json "command" field and all help text derive from these; the + // three-part path must join to the full command a user types. + assert.Equal(t, "local-env python sync", CommandName) +} From 22c2902b0351285a8285fb5a31a1e660c2139ca0 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 3 Jul 2026 17:25:43 +0200 Subject: [PATCH 2/8] Fix PythonMinorFromRequires to pick the lower bound in multi-clause specifiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the foundation layer flagged that PythonMinorFromRequires took the first MAJOR.MINOR in the string via first-match regex. For a multi-clause requires-python where the exclusive upper bound comes first — e.g. "<3.13,>=3.10" — it returned 3.13, the version the "<3.13" clause forbids, because PEP 440 clause order is arbitrary. The result feeds PM.EnsurePython, so the tool could target a Python the constraint excludes. Prefer a lower-bound / pinning clause (>=, >, ==, ~=, ===) and only fall back to the first version when none is present. Adds multi-clause test coverage; the prior tests exercised only single-bound specifiers. Co-authored-by: Isaac --- libs/localenv/envkey.go | 20 +++++++++++++++++++- libs/localenv/envkey_test.go | 10 +++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/libs/localenv/envkey.go b/libs/localenv/envkey.go index 1e9171b35b3..a7c0612f69c 100644 --- a/libs/localenv/envkey.go +++ b/libs/localenv/envkey.go @@ -8,6 +8,13 @@ import ( var pythonVersionRe = regexp.MustCompile(`(\d+)\.(\d+)`) +// lowerBoundClauseRe matches a single requires-python clause that establishes +// the Python floor to install: a lower-bound or pinning operator (>=, >, ==, +// ~=, ===) followed by a MAJOR.MINOR version. Upper-bound (<, <=) and exclusion +// (!=) clauses are deliberately not matched — they must not be chosen as the +// version to install. +var lowerBoundClauseRe = regexp.MustCompile(`(?:>=|===|==|~=|>)\s*(\d+)\.(\d+)`) + // NormalizeServerless returns the canonical "vN" spelling of a serverless // version accepting "4", "v4", or "V4". func NormalizeServerless(version string) string { @@ -24,8 +31,19 @@ func EnvKeyForSparkVersion(sparkVersion string) string { return "dbr/" + sparkVersion } -// PythonMinorFromRequires parses a PEP 440 requires-python string and extracts MAJOR.MINOR. +// PythonMinorFromRequires parses a PEP 440 requires-python string and extracts +// the MAJOR.MINOR of the Python version to install. +// +// A requires-python may hold several comma-separated clauses in any order +// (e.g. "<3.13,>=3.10"). The version to install is the lower bound, so a +// lower-bound / pinning clause (>=, >, ==, ~=, ===) is preferred; taking the +// first number in the string would pick an upper bound like "<3.13" — a +// version the specifier forbids. Only when no lower-bound clause is present do +// we fall back to the first MAJOR.MINOR found. func PythonMinorFromRequires(requiresPython string) (string, error) { + if m := lowerBoundClauseRe.FindStringSubmatch(requiresPython); m != nil { + return fmt.Sprintf("%s.%s", m[1], m[2]), nil + } match := pythonVersionRe.FindStringSubmatch(requiresPython) if match == nil { return "", fmt.Errorf("cannot parse python version from %q", requiresPython) diff --git a/libs/localenv/envkey_test.go b/libs/localenv/envkey_test.go index cb276af4ea2..ea8b7c6a737 100644 --- a/libs/localenv/envkey_test.go +++ b/libs/localenv/envkey_test.go @@ -23,11 +23,19 @@ func TestPythonMinorFromRequires(t *testing.T) { ">=3.12": "3.12", "==3.12.3": "3.12", "~=3.11": "3.11", + // Multi-clause specifiers: the lower bound is the version to install, + // regardless of clause order. Taking the first number would pick the + // excluded upper bound (e.g. 3.13 from "<3.13"). + "<3.13,>=3.10": "3.10", + ">=3.10,<3.13": "3.10", + ">=3.10, <3.13": "3.10", + "<4.0,>=3.9": "3.9", + "===3.11": "3.11", } for in, want := range cases { got, err := PythonMinorFromRequires(in) require.NoError(t, err) - assert.Equal(t, want, got) + assert.Equal(t, want, got, "input %q", in) } _, err := PythonMinorFromRequires("garbage") assert.Error(t, err) From 5b0e0cce7073bbe9b4a039ffc5429a358a7c5387 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 3 Jul 2026 20:13:35 +0200 Subject: [PATCH 3/8] Reject requires-python with no lower bound instead of picking a forbidden version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review of the foundation layer noted that when a requires-python has no lower-bound/pin clause at all (only upper-bound or exclusion, e.g. "<3.13,!=3.12"), PythonMinorFromRequires fell back to the first number and returned 3.13 — a version the specifier forbids. Such a spec has no floor to install from, so it now errors rather than guessing. A bare "3.12" (no operator) is still accepted as a valid floor. Co-authored-by: Isaac --- libs/localenv/envkey.go | 20 ++++++++++++++++---- libs/localenv/envkey_test.go | 14 ++++++++++++-- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/libs/localenv/envkey.go b/libs/localenv/envkey.go index a7c0612f69c..eee1022d26f 100644 --- a/libs/localenv/envkey.go +++ b/libs/localenv/envkey.go @@ -15,6 +15,12 @@ var pythonVersionRe = regexp.MustCompile(`(\d+)\.(\d+)`) // version to install. var lowerBoundClauseRe = regexp.MustCompile(`(?:>=|===|==|~=|>)\s*(\d+)\.(\d+)`) +// boundedClauseRe matches a MAJOR.MINOR that is governed by an upper-bound or +// exclusion operator (<, <=, !=). Such a version is forbidden or capped, never a +// version to install, so a spec whose only version is bounded this way has no +// usable floor. +var boundedClauseRe = regexp.MustCompile(`(?:<=|<|!=)\s*(\d+)\.(\d+)`) + // NormalizeServerless returns the canonical "vN" spelling of a serverless // version accepting "4", "v4", or "V4". func NormalizeServerless(version string) string { @@ -36,10 +42,11 @@ func EnvKeyForSparkVersion(sparkVersion string) string { // // A requires-python may hold several comma-separated clauses in any order // (e.g. "<3.13,>=3.10"). The version to install is the lower bound, so a -// lower-bound / pinning clause (>=, >, ==, ~=, ===) is preferred; taking the -// first number in the string would pick an upper bound like "<3.13" — a -// version the specifier forbids. Only when no lower-bound clause is present do -// we fall back to the first MAJOR.MINOR found. +// lower-bound / pinning clause (>=, >, ==, ~=, ===) is preferred. A bare +// MAJOR.MINOR with no operator (e.g. "3.12") is also accepted as the floor. +// But a spec whose only version is governed by an upper-bound/exclusion +// operator (e.g. "<3.13" or "!=3.12") has no usable floor: picking that number +// would select a version the specifier forbids, so we error instead of guessing. func PythonMinorFromRequires(requiresPython string) (string, error) { if m := lowerBoundClauseRe.FindStringSubmatch(requiresPython); m != nil { return fmt.Sprintf("%s.%s", m[1], m[2]), nil @@ -48,5 +55,10 @@ func PythonMinorFromRequires(requiresPython string) (string, error) { if match == nil { return "", fmt.Errorf("cannot parse python version from %q", requiresPython) } + // A version exists but not behind a lower-bound/pin operator. If it is behind + // an upper-bound/exclusion operator, there is no floor to install. + if boundedClauseRe.MatchString(requiresPython) { + return "", fmt.Errorf("requires-python %q has no lower bound to install from", requiresPython) + } return fmt.Sprintf("%s.%s", match[1], match[2]), nil } diff --git a/libs/localenv/envkey_test.go b/libs/localenv/envkey_test.go index ea8b7c6a737..5230fe16305 100644 --- a/libs/localenv/envkey_test.go +++ b/libs/localenv/envkey_test.go @@ -37,6 +37,16 @@ func TestPythonMinorFromRequires(t *testing.T) { require.NoError(t, err) assert.Equal(t, want, got, "input %q", in) } - _, err := PythonMinorFromRequires("garbage") - assert.Error(t, err) + + // A bare version with no operator is a valid floor. + got, err := PythonMinorFromRequires("3.12") + require.NoError(t, err) + assert.Equal(t, "3.12", got) + + // No usable floor: only upper-bound / exclusion clauses. Must error rather + // than select a forbidden/capped version. + for _, in := range []string{"<3.13", "<=3.12", "!=3.12", "<3.13,!=3.12", "garbage"} { + _, err := PythonMinorFromRequires(in) + assert.Error(t, err, "input %q must error", in) + } } From 7af0d4840b5876214982910d3a5570849cf59d35 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 3 Jul 2026 20:25:56 +0200 Subject: [PATCH 4/8] Parse requires-python by clause and install the effective (highest) lower bound Round-3 review found two edge cases in the regex-based PythonMinorFromRequires: with multiple lower bounds (">=3.8,>=3.11") it returned the first (3.8) rather than the effective floor (3.11), and a bare floor alongside an exclusion ("!=3.11,3.12") was wrongly rejected as having no floor. Replaced the layered regexes with a small clause parser: split on commas, classify each clause by operator (>=,>,==,~=,=== and bare = floor; <,<=,!= never a floor), and return the highest floor. A spec with no floor clause ("<3.13", "!=3.12") still errors. Covers multi-lower-bound, bare-floor + exclusion, ordering, and whitespace. Co-authored-by: Isaac --- libs/localenv/envkey.go | 74 ++++++++++++++++++++---------------- libs/localenv/envkey_test.go | 17 ++++++--- 2 files changed, 53 insertions(+), 38 deletions(-) diff --git a/libs/localenv/envkey.go b/libs/localenv/envkey.go index eee1022d26f..47043324df5 100644 --- a/libs/localenv/envkey.go +++ b/libs/localenv/envkey.go @@ -3,23 +3,13 @@ package localenv import ( "fmt" "regexp" + "strconv" "strings" ) -var pythonVersionRe = regexp.MustCompile(`(\d+)\.(\d+)`) - -// lowerBoundClauseRe matches a single requires-python clause that establishes -// the Python floor to install: a lower-bound or pinning operator (>=, >, ==, -// ~=, ===) followed by a MAJOR.MINOR version. Upper-bound (<, <=) and exclusion -// (!=) clauses are deliberately not matched — they must not be chosen as the -// version to install. -var lowerBoundClauseRe = regexp.MustCompile(`(?:>=|===|==|~=|>)\s*(\d+)\.(\d+)`) - -// boundedClauseRe matches a MAJOR.MINOR that is governed by an upper-bound or -// exclusion operator (<, <=, !=). Such a version is forbidden or capped, never a -// version to install, so a spec whose only version is bounded this way has no -// usable floor. -var boundedClauseRe = regexp.MustCompile(`(?:<=|<|!=)\s*(\d+)\.(\d+)`) +// clauseRe splits a single requires-python clause into its operator (optional) +// and MAJOR.MINOR version. A clause with no operator is a bare floor. +var clauseRe = regexp.MustCompile(`^(>=|<=|===|==|~=|!=|<|>)?\s*(\d+)\.(\d+)`) // NormalizeServerless returns the canonical "vN" spelling of a serverless // version accepting "4", "v4", or "V4". @@ -37,28 +27,48 @@ func EnvKeyForSparkVersion(sparkVersion string) string { return "dbr/" + sparkVersion } -// PythonMinorFromRequires parses a PEP 440 requires-python string and extracts -// the MAJOR.MINOR of the Python version to install. +// PythonMinorFromRequires parses a PEP 440 requires-python string and returns +// the MAJOR.MINOR of the Python version to install: the effective lower bound. +// +// A requires-python is a comma-separated list of clauses in any order (e.g. +// "<3.13,>=3.10"). Each clause is classified by operator: +// - lower-bound / pinning (>=, >, ==, ~=, ===) or a bare MAJOR.MINOR with no +// operator establishes a floor; +// - upper-bound / exclusion (<, <=, !=) does not — those versions are capped +// or forbidden and must never be installed. // -// A requires-python may hold several comma-separated clauses in any order -// (e.g. "<3.13,>=3.10"). The version to install is the lower bound, so a -// lower-bound / pinning clause (>=, >, ==, ~=, ===) is preferred. A bare -// MAJOR.MINOR with no operator (e.g. "3.12") is also accepted as the floor. -// But a spec whose only version is governed by an upper-bound/exclusion -// operator (e.g. "<3.13" or "!=3.12") has no usable floor: picking that number -// would select a version the specifier forbids, so we error instead of guessing. +// The result is the highest floor across all floor clauses (so ">=3.8,>=3.11" +// yields 3.11, the version that satisfies every clause). A spec with no floor +// clause at all (e.g. "<3.13" or "!=3.12") is an error rather than a guess. func PythonMinorFromRequires(requiresPython string) (string, error) { - if m := lowerBoundClauseRe.FindStringSubmatch(requiresPython); m != nil { - return fmt.Sprintf("%s.%s", m[1], m[2]), nil + bestMajor, bestMinor := -1, -1 + sawClause := false + for clause := range strings.SplitSeq(requiresPython, ",") { + clause = strings.TrimSpace(clause) + if clause == "" { + continue + } + m := clauseRe.FindStringSubmatch(clause) + if m == nil { + continue + } + sawClause = true + op := m[1] + // Upper-bound and exclusion operators never establish a floor. + if op == "<" || op == "<=" || op == "!=" { + continue + } + major, _ := strconv.Atoi(m[2]) + minor, _ := strconv.Atoi(m[3]) + if major > bestMajor || (major == bestMajor && minor > bestMinor) { + bestMajor, bestMinor = major, minor + } } - match := pythonVersionRe.FindStringSubmatch(requiresPython) - if match == nil { - return "", fmt.Errorf("cannot parse python version from %q", requiresPython) + if bestMajor >= 0 { + return fmt.Sprintf("%d.%d", bestMajor, bestMinor), nil } - // A version exists but not behind a lower-bound/pin operator. If it is behind - // an upper-bound/exclusion operator, there is no floor to install. - if boundedClauseRe.MatchString(requiresPython) { + if sawClause { return "", fmt.Errorf("requires-python %q has no lower bound to install from", requiresPython) } - return fmt.Sprintf("%s.%s", match[1], match[2]), nil + return "", fmt.Errorf("cannot parse python version from %q", requiresPython) } diff --git a/libs/localenv/envkey_test.go b/libs/localenv/envkey_test.go index 5230fe16305..f9f334a2740 100644 --- a/libs/localenv/envkey_test.go +++ b/libs/localenv/envkey_test.go @@ -31,6 +31,16 @@ func TestPythonMinorFromRequires(t *testing.T) { ">=3.10, <3.13": "3.10", "<4.0,>=3.9": "3.9", "===3.11": "3.11", + // The effective floor is the HIGHEST lower bound, regardless of order. + ">=3.8,>=3.11": "3.11", + ">=3.11,>=3.8": "3.11", + // A bare floor alongside an exclusion is still a floor. + "!=3.11,3.12": "3.12", + "3.12,!=3.12.4": "3.12", + // Bare version with no operator. + "3.12": "3.12", + // Whitespace and patch components tolerated. + ">= 3.10 , < 3.13": "3.10", } for in, want := range cases { got, err := PythonMinorFromRequires(in) @@ -38,14 +48,9 @@ func TestPythonMinorFromRequires(t *testing.T) { assert.Equal(t, want, got, "input %q", in) } - // A bare version with no operator is a valid floor. - got, err := PythonMinorFromRequires("3.12") - require.NoError(t, err) - assert.Equal(t, "3.12", got) - // No usable floor: only upper-bound / exclusion clauses. Must error rather // than select a forbidden/capped version. - for _, in := range []string{"<3.13", "<=3.12", "!=3.12", "<3.13,!=3.12", "garbage"} { + for _, in := range []string{"<3.13", "<=3.12", "!=3.12", "<3.13,!=3.12", "garbage", ""} { _, err := PythonMinorFromRequires(in) assert.Error(t, err, "input %q must error", in) } From be61b7d31aded4d600d7acf4197e34627548df3c Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 3 Jul 2026 21:13:27 +0200 Subject: [PATCH 5/8] Treat a strict ">" python bound as excluding the whole minor series Round-4 review noted PythonMinorFromRequires returned 3.10 for ">3.10", but PEP 440's ">" excludes the entire given release series (neither 3.10 nor any 3.10.x satisfies ">3.10"), so the lowest installable minor is 3.11. The clause parser now bumps the minor by one for a strict ">" bound. Co-authored-by: Isaac --- libs/localenv/envkey.go | 6 ++++++ libs/localenv/envkey_test.go | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/libs/localenv/envkey.go b/libs/localenv/envkey.go index 47043324df5..39cff1a8331 100644 --- a/libs/localenv/envkey.go +++ b/libs/localenv/envkey.go @@ -60,6 +60,12 @@ func PythonMinorFromRequires(requiresPython string) (string, error) { } major, _ := strconv.Atoi(m[2]) minor, _ := strconv.Atoi(m[3]) + // A strict ">" excludes the whole given minor series (PEP 440: ">3.10" + // matches neither 3.10 nor any 3.10.x), so the lowest installable minor is + // the next one up. + if op == ">" { + minor++ + } if major > bestMajor || (major == bestMajor && minor > bestMinor) { bestMajor, bestMinor = major, minor } diff --git a/libs/localenv/envkey_test.go b/libs/localenv/envkey_test.go index f9f334a2740..8c5f3be671f 100644 --- a/libs/localenv/envkey_test.go +++ b/libs/localenv/envkey_test.go @@ -41,6 +41,11 @@ func TestPythonMinorFromRequires(t *testing.T) { "3.12": "3.12", // Whitespace and patch components tolerated. ">= 3.10 , < 3.13": "3.10", + // Strict ">" excludes the whole minor series (PEP 440), so the floor is + // the next minor up. + ">3.10": "3.11", + ">3.10,<3.13": "3.11", + ">=3.9,>3.10": "3.11", } for in, want := range cases { got, err := PythonMinorFromRequires(in) From 900518bcf616320ca0e3d9b3c4793cf00144fc1c Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Mon, 6 Jul 2026 13:39:37 +0200 Subject: [PATCH 6/8] Address review: fix patch-qualified strict python bound; guarantee --json arrays Two items from review of the foundation layer: - PythonMinorFromRequires bumped the minor for any strict ">" bound, but a patch-qualified bound like ">3.10.5" is still satisfied by 3.10.6, so the floor should stay 3.10 (only a bare ">3.10" excludes the whole 3.10.x series). clauseRe now captures the patch component and the minor is bumped only when it is absent. Adds >3.10.5 / >=3.10.2 test cases. - Result.Phases and Result.Warnings are non-omitempty slices, so a bare Result{} would marshal them as "null" rather than "[]", an ambiguity in the --json contract. Added NewResult() which seeds both to empty slices, a doc note on the invariant, and a test asserting the JSON emits [] not null. The pipeline (later in the stack) constructs its Result through this. Co-authored-by: Isaac --- libs/localenv/envkey.go | 19 ++++++++++++------- libs/localenv/envkey_test.go | 9 +++++++-- libs/localenv/result.go | 15 +++++++++++++++ libs/localenv/result_test.go | 18 ++++++++++++++++++ 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/libs/localenv/envkey.go b/libs/localenv/envkey.go index 39cff1a8331..070668ba826 100644 --- a/libs/localenv/envkey.go +++ b/libs/localenv/envkey.go @@ -7,9 +7,12 @@ import ( "strings" ) -// clauseRe splits a single requires-python clause into its operator (optional) -// and MAJOR.MINOR version. A clause with no operator is a bare floor. -var clauseRe = regexp.MustCompile(`^(>=|<=|===|==|~=|!=|<|>)?\s*(\d+)\.(\d+)`) +// clauseRe splits a single requires-python clause into its operator (optional), +// MAJOR.MINOR version, and an optional patch component. A clause with no operator +// is a bare floor. The patch capture (group 4) is needed to interpret a strict +// ">" correctly: ">3.10" excludes all of 3.10.x, but ">3.10.5" is still satisfied +// by 3.10.6, so only the former bumps the minor. +var clauseRe = regexp.MustCompile(`^(>=|<=|===|==|~=|!=|<|>)?\s*(\d+)\.(\d+)(\.\d+)?`) // NormalizeServerless returns the canonical "vN" spelling of a serverless // version accepting "4", "v4", or "V4". @@ -60,10 +63,12 @@ func PythonMinorFromRequires(requiresPython string) (string, error) { } major, _ := strconv.Atoi(m[2]) minor, _ := strconv.Atoi(m[3]) - // A strict ">" excludes the whole given minor series (PEP 440: ">3.10" - // matches neither 3.10 nor any 3.10.x), so the lowest installable minor is - // the next one up. - if op == ">" { + hasPatch := m[4] != "" + // A strict ">" with no patch excludes the whole given minor series (PEP 440: + // ">3.10" matches neither 3.10 nor any 3.10.x), so the lowest installable + // minor is the next one up. But ">3.10.5" is still satisfied by 3.10.6, so a + // patch-qualified strict bound leaves the minor unchanged. + if op == ">" && !hasPatch { minor++ } if major > bestMajor || (major == bestMajor && minor > bestMinor) { diff --git a/libs/localenv/envkey_test.go b/libs/localenv/envkey_test.go index 8c5f3be671f..898e93434b6 100644 --- a/libs/localenv/envkey_test.go +++ b/libs/localenv/envkey_test.go @@ -41,11 +41,16 @@ func TestPythonMinorFromRequires(t *testing.T) { "3.12": "3.12", // Whitespace and patch components tolerated. ">= 3.10 , < 3.13": "3.10", - // Strict ">" excludes the whole minor series (PEP 440), so the floor is - // the next minor up. + // Strict ">" with no patch excludes the whole minor series (PEP 440), so + // the floor is the next minor up. ">3.10": "3.11", ">3.10,<3.13": "3.11", ">=3.9,>3.10": "3.11", + // Strict ">" WITH a patch does not exclude the minor series: 3.10.6 + // satisfies ">3.10.5", so the floor stays 3.10. + ">3.10.5": "3.10", + ">3.10.5,<3.13": "3.10", + ">=3.10.2": "3.10", } for in, want := range cases { got, err := PythonMinorFromRequires(in) diff --git a/libs/localenv/result.go b/libs/localenv/result.go index 82faed1b1dd..1de8f66ba50 100644 --- a/libs/localenv/result.go +++ b/libs/localenv/result.go @@ -157,6 +157,11 @@ type Warning struct { // Result is the full outcome of a sync run and the root of the --json object // (spec §6). Field order matches the spec's schema so JSON key order is stable. +// +// Phases and Warnings are non-omitempty slices, so they must always be non-nil +// before marshalling or the --json contract would emit "null" instead of "[]" — +// a distinction that trips JSON consumers and golden diffs. Construct a Result +// with NewResult (or otherwise seed both) rather than a bare Result{} literal. type Result struct { SchemaVersion int `json:"schemaVersion"` Command string `json:"command"` @@ -178,3 +183,13 @@ type Result struct { // through a clock the tests can control. DurationMs int64 `json:"durationMs"` } + +// NewResult returns a Result with the non-omitempty slice fields initialized to +// empty (non-nil) slices, so the --json output always renders "phases": [] and +// "warnings": [] rather than "null". Callers fill in the remaining fields. +func NewResult() *Result { + return &Result{ + Phases: []PhaseStatus{}, + Warnings: []Warning{}, + } +} diff --git a/libs/localenv/result_test.go b/libs/localenv/result_test.go index 76d30ec22bf..75fe60ea66d 100644 --- a/libs/localenv/result_test.go +++ b/libs/localenv/result_test.go @@ -1,10 +1,12 @@ package localenv import ( + "encoding/json" "errors" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPipelineErrorWrapsAndExposesCode(t *testing.T) { @@ -25,3 +27,19 @@ func TestCommandName(t *testing.T) { // three-part path must join to the full command a user types. assert.Equal(t, "local-env python sync", CommandName) } + +func TestNewResultEmitsEmptyArraysNotNull(t *testing.T) { + // The --json contract requires phases/warnings to render as [] not null; + // NewResult seeds them so consumers and golden diffs see a stable shape. + b, err := json.Marshal(NewResult()) + require.NoError(t, err) + s := string(b) + assert.Contains(t, s, `"phases":[]`) + assert.Contains(t, s, `"warnings":[]`) + assert.NotContains(t, s, `"phases":null`) + assert.NotContains(t, s, `"warnings":null`) + // A bare Result{} literal is the shape NewResult exists to avoid. + bare, err := json.Marshal(&Result{}) + require.NoError(t, err) + assert.Contains(t, string(bare), `"phases":null`, "sanity: bare literal is the null case") +} From 28469e175c43fea3eb64ab195db72cbc3d0899da Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 3 Jul 2026 15:17:50 +0200 Subject: [PATCH 7/8] Add local-env compute-target resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second in the stacked local-env series (builds on the foundation types). target.go resolves a compute target to a TargetInfo (and its environment key) using ordered precedence: --cluster flag → --serverless flag → --job flag → bundle target. Compute lookups go through the narrow ComputeClient seam so the resolver is unit-tested against a stub with no SDK dependency. ValidateTargetFlags guards the library path against more than one target flag being set. The classic-compute job branch reads the Spark version from the first return of GetJobSparkVersion, per that method's documented contract, rather than the recorded-version third return. Depends on the foundation PR for NewError, the E_RESOLVE / E_NO_TARGET codes, TargetInfo, and the EnvKeyFor* helpers. Still dormant. Co-authored-by: Isaac --- libs/localenv/target.go | 140 +++++++++++++++++++++++++++++++++++ libs/localenv/target_test.go | 96 ++++++++++++++++++++++++ 2 files changed, 236 insertions(+) create mode 100644 libs/localenv/target.go create mode 100644 libs/localenv/target_test.go diff --git a/libs/localenv/target.go b/libs/localenv/target.go new file mode 100644 index 00000000000..336cf1e5667 --- /dev/null +++ b/libs/localenv/target.go @@ -0,0 +1,140 @@ +package localenv + +import ( + "context" + "fmt" + "strings" +) + +// ComputeClient is a narrow seam over the SDK so tests can stub it. +type ComputeClient interface { + // GetClusterSparkVersion returns the Spark version string for a cluster. + GetClusterSparkVersion(ctx context.Context, clusterID string) (string, error) + // GetJobSparkVersion returns either a Spark version (isServerless=false) or a + // serverless marker (isServerless=true) for a job, plus a recorded version string. + GetJobSparkVersion(ctx context.Context, jobID string) (sparkVersion string, isServerless bool, version string, err error) +} + +// TargetFlags holds the mutually-exclusive compute target flags from the CLI. +type TargetFlags struct { + Cluster string + Serverless string + Job string +} + +// BundleTarget is the three-state result of reading the bundle's configured +// target. Selected=false means nothing was configured. +type BundleTarget struct { + ClusterID string + Serverless bool + Selected bool +} + +// noTargetMessage is the actionable message shown when no target is selected, +// matching spec §2.3. +const noTargetMessage = "No compute target is selected. Select a cluster or serverless target, or pass --cluster / --serverless / --job" + +// ValidateTargetFlags returns an error if more than one of the three flags is set. +// Cobra marks them mutually exclusive too; this guards the library path. +func ValidateTargetFlags(f TargetFlags) error { + var set []string + if f.Cluster != "" { + set = append(set, "--cluster") + } + if f.Serverless != "" { + set = append(set, "--serverless") + } + if f.Job != "" { + set = append(set, "--job") + } + if len(set) > 1 { + return fmt.Errorf("flags %s are mutually exclusive; specify at most one", strings.Join(set, " and ")) + } + return nil +} + +// ResolveTarget resolves the compute target using ordered precedence: +// --cluster flag → --serverless flag → --job flag → bundle target. +// PythonVersion is left empty; it is filled later from constraint data. +func ResolveTarget(ctx context.Context, f TargetFlags, c ComputeClient, bt BundleTarget) (*TargetInfo, error) { + if f.Cluster != "" { + v, err := c.GetClusterSparkVersion(ctx, f.Cluster) + if err != nil { + return nil, NewError(ErrResolve, err, "resolving cluster %s", f.Cluster) + } + return &TargetInfo{ + Source: "cluster", + ClusterID: f.Cluster, + SparkVersion: v, + EnvKey: EnvKeyForSparkVersion(v), + }, nil + } + + if f.Serverless != "" { + return &TargetInfo{ + Source: "serverless", + ServerlessVersion: NormalizeServerless(f.Serverless), + EnvKey: EnvKeyForServerless(f.Serverless), + }, nil + } + + if f.Job != "" { + sparkVersion, isServerless, version, err := c.GetJobSparkVersion(ctx, f.Job) + if err != nil { + return nil, NewError(ErrResolve, err, "resolving job %s", f.Job) + } + if isServerless { + // Default to v4 when the job is serverless; the serverless env version + // is not recorded in the bundle/project (documented stand-in from the + // original script, spec §4.3). + v := version + if v == "" { + v = "v4" + } + return &TargetInfo{ + Source: "job", + ServerlessVersion: NormalizeServerless(v), + EnvKey: EnvKeyForServerless(v), + }, nil + } + // Classic compute: the Spark version is the first return per the + // GetJobSparkVersion contract, not the recorded-version third return. + return &TargetInfo{ + Source: "job", + SparkVersion: sparkVersion, + EnvKey: EnvKeyForSparkVersion(sparkVersion), + }, nil + } + + // Fall back to bundle target. + if !bt.Selected { + return nil, NewError(ErrNoTarget, nil, "%s", noTargetMessage) + } + + if bt.Serverless { + // Default to serverless-v4: the serverless env version is not recorded + // in the bundle/project (documented stand-in from the original script). + return &TargetInfo{ + Source: "bundle", + ServerlessVersion: "v4", + EnvKey: EnvKeyForServerless("v4"), + }, nil + } + + if bt.ClusterID != "" { + v, err := c.GetClusterSparkVersion(ctx, bt.ClusterID) + if err != nil { + return nil, NewError(ErrResolve, err, "resolving bundle cluster %s", bt.ClusterID) + } + return &TargetInfo{ + Source: "bundle", + ClusterID: bt.ClusterID, + SparkVersion: v, + EnvKey: EnvKeyForSparkVersion(v), + }, nil + } + + // Bundle target is selected but has neither serverless nor a cluster ID — + // treat this the same as nothing selected so the user gets a clear message. + return nil, NewError(ErrNoTarget, nil, "%s", noTargetMessage) +} diff --git a/libs/localenv/target_test.go b/libs/localenv/target_test.go new file mode 100644 index 00000000000..b2cc2a55df7 --- /dev/null +++ b/libs/localenv/target_test.go @@ -0,0 +1,96 @@ +package localenv + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type stubCompute struct { + clusterVersion string + clusterErr error +} + +func (s stubCompute) GetClusterSparkVersion(_ context.Context, _ string) (string, error) { + return s.clusterVersion, s.clusterErr +} + +func (s stubCompute) GetJobSparkVersion(_ context.Context, _ string) (string, bool, string, error) { + return "", false, "", nil +} + +func TestResolveServerlessFlag(t *testing.T) { + ti, err := ResolveTarget(t.Context(), TargetFlags{Serverless: "v4"}, stubCompute{}, BundleTarget{}) + require.NoError(t, err) + assert.Equal(t, "serverless", ti.Source) + assert.Equal(t, "v4", ti.ServerlessVersion) + assert.Equal(t, "serverless/serverless-v4", ti.EnvKey) +} + +func TestResolveClusterFlag(t *testing.T) { + c := stubCompute{clusterVersion: "15.4.x-scala2.12"} + ti, err := ResolveTarget(t.Context(), TargetFlags{Cluster: "abc"}, c, BundleTarget{}) + require.NoError(t, err) + assert.Equal(t, "cluster", ti.Source) + assert.Equal(t, "15.4.x-scala2.12", ti.SparkVersion) + assert.Equal(t, "dbr/15.4.x-scala2.12", ti.EnvKey) + assert.Equal(t, "abc", ti.ClusterID) +} + +func TestResolveClusterFlagError(t *testing.T) { + c := stubCompute{clusterErr: errors.New("cluster not found")} + _, err := ResolveTarget(t.Context(), TargetFlags{Cluster: "abc"}, c, BundleTarget{}) + var pe *PipelineError + require.ErrorAs(t, err, &pe) + assert.Equal(t, ErrResolve, pe.Code) +} + +func TestResolveBundleNothingSelected(t *testing.T) { + _, err := ResolveTarget(t.Context(), TargetFlags{}, stubCompute{}, BundleTarget{Selected: false}) + var pe *PipelineError + require.ErrorAs(t, err, &pe) + assert.Equal(t, ErrNoTarget, pe.Code) +} + +func TestResolveBundleServerless(t *testing.T) { + ti, err := ResolveTarget(t.Context(), TargetFlags{}, stubCompute{}, BundleTarget{Selected: true, Serverless: true}) + require.NoError(t, err) + assert.Equal(t, "bundle", ti.Source) + assert.Equal(t, "serverless/serverless-v4", ti.EnvKey) +} + +// jobStubCompute returns distinct values for the first (sparkVersion) and third +// (recorded version) results of GetJobSparkVersion so the classic-compute branch +// can be checked against the documented contract (it must use the first). +type jobStubCompute struct { + sparkVersion string + isServerless bool + version string +} + +func (jobStubCompute) GetClusterSparkVersion(_ context.Context, _ string) (string, error) { + return "", nil +} + +func (s jobStubCompute) GetJobSparkVersion(_ context.Context, _ string) (string, bool, string, error) { + return s.sparkVersion, s.isServerless, s.version, nil +} + +func TestResolveJobClassicUsesSparkVersionReturn(t *testing.T) { + // Contract: for a classic-compute job the Spark version is the FIRST return. + // The third "recorded version" return differs here to catch use of the wrong one. + c := jobStubCompute{sparkVersion: "15.4.x-scala2.12", isServerless: false, version: "wrong-recorded"} + ti, err := ResolveTarget(t.Context(), TargetFlags{Job: "42"}, c, BundleTarget{}) + require.NoError(t, err) + assert.Equal(t, "job", ti.Source) + assert.Equal(t, "15.4.x-scala2.12", ti.SparkVersion) + assert.Equal(t, "dbr/15.4.x-scala2.12", ti.EnvKey) +} + +func TestValidateTargetFlagsMutuallyExclusive(t *testing.T) { + assert.Error(t, ValidateTargetFlags(TargetFlags{Cluster: "a", Serverless: "v4"})) + assert.NoError(t, ValidateTargetFlags(TargetFlags{Cluster: "a"})) +} From 42a7988900a9467bd4994a23758eb47cd198a74e Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 3 Jul 2026 17:30:40 +0200 Subject: [PATCH 8/8] Validate mutually exclusive target flags inside ResolveTarget Review of the target layer noted that ResolveTarget accepted incompatible flags on the library path: called directly with e.g. TargetFlags{Cluster: "c", Serverless: "v4"} it silently took the first precedence branch and ignored the rest, resolving a different target than requested. Cobra and the cmd layer already reject this, but ResolveTarget is exported and ValidateTargetFlags exists specifically to guard callers that bypass Cobra, so the resolver now runs that check first and returns E_RESOLVE on conflicting flags. Co-authored-by: Isaac --- libs/localenv/target.go | 9 +++++++++ libs/localenv/target_test.go | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/libs/localenv/target.go b/libs/localenv/target.go index 336cf1e5667..b3ccb3171e1 100644 --- a/libs/localenv/target.go +++ b/libs/localenv/target.go @@ -56,7 +56,16 @@ func ValidateTargetFlags(f TargetFlags) error { // ResolveTarget resolves the compute target using ordered precedence: // --cluster flag → --serverless flag → --job flag → bundle target. // PythonVersion is left empty; it is filled later from constraint data. +// +// Incompatible flags are rejected up front: without this a library caller that +// bypasses Cobra (which also marks the flags mutually exclusive) and passes more +// than one target flag would have all but the first precedence branch silently +// ignored, resolving a different target than requested. func ResolveTarget(ctx context.Context, f TargetFlags, c ComputeClient, bt BundleTarget) (*TargetInfo, error) { + if err := ValidateTargetFlags(f); err != nil { + return nil, NewError(ErrResolve, err, "invalid compute target flags") + } + if f.Cluster != "" { v, err := c.GetClusterSparkVersion(ctx, f.Cluster) if err != nil { diff --git a/libs/localenv/target_test.go b/libs/localenv/target_test.go index b2cc2a55df7..22beaee3382 100644 --- a/libs/localenv/target_test.go +++ b/libs/localenv/target_test.go @@ -94,3 +94,13 @@ func TestValidateTargetFlagsMutuallyExclusive(t *testing.T) { assert.Error(t, ValidateTargetFlags(TargetFlags{Cluster: "a", Serverless: "v4"})) assert.NoError(t, ValidateTargetFlags(TargetFlags{Cluster: "a"})) } + +func TestResolveTargetRejectsConflictingFlags(t *testing.T) { + // ResolveTarget must reject incompatible flags rather than silently taking + // the first precedence branch, so a library caller bypassing Cobra can't + // resolve a different target than it asked for. + _, err := ResolveTarget(t.Context(), TargetFlags{Cluster: "c", Serverless: "v4"}, stubCompute{}, BundleTarget{}) + var pe *PipelineError + require.ErrorAs(t, err, &pe) + assert.Equal(t, ErrResolve, pe.Code) +}