From 1dd3abbd99e1859c0b654e2d0065123eab047b01 Mon Sep 17 00:00:00 2001 From: "@rugpanov" Date: Tue, 8 Sep 2026 15:22:51 +0200 Subject: [PATCH 1/8] feat(python-setup): recover from a constraint conflict with a Retry-as-DB-Connect fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* The uv-native "set up Python environment" Full preset pins the cluster's dependencies, so uv sync can fail when those pins conflict with the user's own dependencies. Today that surfaces as the generic E_PROVISION toast, leaving the user to work out on their own that dropping the pins (the DB Connect preset) would resolve it. *What* - Add the distinct E_PROVISION_CONFLICT code to the setup-local result model (the CLI reserves it for a genuine pin-vs-local conflict; a generic sync failure stays E_PROVISION) with dedicated, actionable copy. - On a Full-preset conflict, replace the generic toast with three buttons: "Retry as DB Connect setup" (re-runs with --no-constraints, keeping matched Python + databricks-connect), "Open pyproject.toml", and "Show Logs". Other provision failures are unchanged. - Add a run (in-process callback) arm to PythonSetupErrorAction so those runtime-stateful buttons reuse the existing showError plumbing; extract runGuarded/runResolved so the retry re-enters the flow with the dbconnect preset without re-prompting the compute or the preset picker. - A conflict on a run that already skipped constraints falls back to the generic doc-link handling (no nonsensical, looping retry). The conflict is never report-worthy (it is the user's own dependencies). *Verification* - test:unit — 1110 passing (the 2 failures are the stale bundled-CLI version check, a local worktree artifact; CI fetches the pinned binary and passes). - New unit coverage: conflict copy + fallback doc link, run-action dispatch, openProjectFile, and the controller conflict->retry flow (re-run drops the pins and adopts, dbconnect telemetry, open pyproject, reportOffered=false, and the non-Full fallback). - tsc + eslint clean on changed files. Co-authored-by: Isaac --- .../PythonSetupEnvironmentSetup.test.ts | 212 ++++++++++++++++++ .../PythonSetupEnvironmentSetup.ts | 115 ++++++++-- .../controllers/pythonSetupDeps.test.ts | 83 +++++++ .../controllers/pythonSetupDeps.ts | 18 +- .../python-setup/models/PythonSetupResult.ts | 8 + .../python-setup/utils/errorMessages.test.ts | 21 ++ .../src/python-setup/utils/errorMessages.ts | 36 ++- 7 files changed, 466 insertions(+), 27 deletions(-) diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts index b2c9088b8..6271a59c6 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts @@ -124,6 +124,7 @@ function makeDeps( compute: {kind: "serverless", version: "5"}, }), adoptInterpreter: async () => {}, + openProjectFile: async () => {}, saveState: () => {}, notify: async () => {}, showReauthPrompt: async () => {}, @@ -1695,3 +1696,214 @@ describe("PythonSetupEnvironmentSetup.setup preset selection", () => { expect(telemetry.attempts[0].mode).to.equal("constraints-only"); }); }); + +describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { + /** + * A Full-preset run whose pinned cluster dependencies conflict with the + * user's local ones: the CLI's distinct E_PROVISION_CONFLICT. Constraints are + * merged before the provision fails, so disk is mutated and a backup exists. + */ + function conflictResult(): PythonSetupResult { + return { + schemaVersion: 1, + command: "environments setup-local", + ok: false, + mode: "default", + dryRun: false, + greenfield: false, + backupPath: "/proj/pyproject.toml.bak", + phases: [ + {phase: "preflight", status: "ok"}, + {phase: "resolve", status: "ok"}, + {phase: "fetch", status: "ok"}, + {phase: "merge", status: "ok"}, + {phase: "provision", status: "error"}, + {phase: "validate", status: "pending"}, + ], + warnings: [], + durationMs: 0, + error: { + code: "E_PROVISION_CONFLICT", + failurePhase: "provision", + message: + "error: No solution found when resolving dependencies: the " + + "runtime requires pyarrow<19 but your project requires " + + "pyarrow==21.0.0", + diskMutated: true, + }, + }; + } + + /** A CLI that returns a scripted result per call, in order. */ + function makeScriptedCli(results: PythonSetupResult[]) { + const calls: SetupLocalInvocation[] = []; + let i = 0; + return { + calls, + run: async (invocation: SetupLocalInvocation) => { + calls.push(invocation); + return results[Math.min(i++, results.length - 1)]; + }, + }; + } + + it("shows the conflict copy with Retry and Open pyproject actions on a Full-preset conflict", async () => { + const shown: {message: string; actions?: PythonSetupErrorAction[]}[] = + []; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli: makeCli({resolve: conflictResult()}), + pickSetupPreset: async () => "full", + showError: async (message, _detail, actions) => { + shown.push({message, actions}); + }, + }) + ); + + await setup.setup(); + + expect(shown).to.have.length(1); + expect(shown[0].message).to.match(/cluster dependencies conflict/i); + // The two recovery buttons, in order; both are in-process run-actions. + expect(shown[0].actions?.map((a) => a.label)).to.deep.equal([ + "Retry as DB Connect setup", + "Open pyproject.toml", + ]); + for (const action of shown[0].actions ?? []) { + expect(action).to.have.property("run"); + } + }); + + it("re-runs with --no-constraints (and adopts) when Retry as DB Connect is picked", async () => { + const cli = makeScriptedCli([conflictResult(), SUCCESS_REAL_RUN]); + const adopted: string[] = []; + let retryAction: PythonSetupErrorAction | undefined; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli, + pickSetupPreset: async () => "full", + adoptInterpreter: async (venvPath) => { + adopted.push(venvPath); + }, + showError: async (_message, _detail, actions) => { + retryAction = actions?.find( + (a) => a.label === "Retry as DB Connect setup" + ); + }, + }) + ); + + await setup.setup(); + // The initial Full run failed; the user clicks Retry (a deferred click, + // after the original run settled). + expect(setup.ready).to.equal(false); + expect(retryAction).to.not.equal(undefined); + await (retryAction as {run: () => Promise}).run(); + + // Two runs: the Full attempt, then the DB Connect retry dropping the pins. + expect(cli.calls).to.have.length(2); + expect(cli.calls[0].skipConstraints).to.equal(undefined); + expect(cli.calls[1].skipConstraints).to.equal(true); + expect(cli.calls[1].skipDbconnect).to.equal(undefined); + // The retry succeeded, so the interpreter is adopted and the project is ready. + expect(adopted).to.deep.equal([SUCCESS_REAL_RUN.venvPath]); + expect(setup.ready).to.equal(true); + }); + + it("records the retry attempt as the dbconnect preset", async () => { + const cli = makeScriptedCli([conflictResult(), SUCCESS_REAL_RUN]); + const telemetry = makeTelemetryRecorder(); + let retryAction: PythonSetupErrorAction | undefined; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli, + ...telemetry, + pickSetupPreset: async () => "full", + showError: async (_message, _detail, actions) => { + retryAction = actions?.find( + (a) => a.label === "Retry as DB Connect setup" + ); + }, + }) + ); + + await setup.setup(); + await (retryAction as {run: () => Promise}).run(); + + expect(telemetry.attempts.map((a) => a.setupPreset)).to.deep.equal([ + "full", + "dbconnect", + ]); + }); + + it("opens pyproject.toml (at the run's cwd) when Open pyproject.toml is picked", async () => { + const openedRoots: string[] = []; + let openAction: PythonSetupErrorAction | undefined; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli: makeCli({resolve: conflictResult()}), + projectRoot: () => "/proj", + pickSetupPreset: async () => "full", + openProjectFile: async (root) => { + openedRoots.push(root); + }, + showError: async (_message, _detail, actions) => { + openAction = actions?.find( + (a) => a.label === "Open pyproject.toml" + ); + }, + }) + ); + + await setup.setup(); + await (openAction as {run: () => Promise}).run(); + + expect(openedRoots).to.deep.equal(["/proj"]); + }); + + it("records the conflict failure without offering a report", async () => { + const telemetry = makeTelemetryRecorder(); + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + ...telemetry, + cli: makeCli({resolve: conflictResult()}), + pickSetupPreset: async () => "full", + }) + ); + + await setup.setup(); + + expect(telemetry.results[0]).to.include({ + outcome: "failed", + errorCode: "E_PROVISION_CONFLICT", + failurePhase: "provision", + // A conflict is the user's own deps → never report-worthy. + reportOffered: false, + }); + }); + + it("falls back to the generic action when a conflict arrives on a run that already dropped the pins", async () => { + // Defensive: only Full pins cluster deps, so a conflict on a dbconnect + // run should never happen — but if it does, "Retry as DB Connect" would + // be nonsensical (and loop), so use the ordinary doc-link handling. + const shown: {actions?: PythonSetupErrorAction[]}[] = []; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli: makeCli({resolve: conflictResult()}), + pickSetupPreset: async () => "dbconnect", + showError: async (_message, _detail, actions) => { + shown.push({actions}); + }, + }) + ); + + await setup.setup(); + + expect(shown[0].actions).to.deep.equal([ + { + label: "Resolve dependency conflicts", + url: "https://docs.astral.sh/uv/concepts/resolution/", + }, + ]); + }); +}); diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts index 2ff4f21b9..bba9390fc 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts @@ -144,6 +144,14 @@ export interface PythonSetupSetupDeps { */ adoptInterpreter: (venvPath: string, projectRoot: string) => Promise; + /** + * Open the project's pyproject.toml in an editor — the "Open pyproject.toml" + * button on a constraint-conflict failure, so the user can inspect and + * adjust the dependencies that clashed. Takes the run's captured root so it + * targets the project the failing run mutated, even after a mid-run switch. + */ + openProjectFile: (projectRoot: string) => Promise; + saveState: (state: PythonSetupPersistedState) => void; /** @@ -305,19 +313,27 @@ export class PythonSetupEnvironmentSetup implements Disposable { } setup(): Promise { - // Re-entrancy guard: coalesce concurrent callers onto the running run - // rather than spawning a second project-mutating CLI process. The guard - // releases when the run's *work* settles; the terminal notification is - // presented via {@link present} (fire-and-forget), so a toast left open - // never wedges the entry -- see that method. + return this.runGuarded(() => this.runSetup()); + } + + /** + * Re-entrancy guard: coalesce concurrent callers onto the running run rather + * than spawning a second project-mutating CLI process. The guard releases + * when the run's *work* settles; the terminal notification is presented via + * {@link present} (fire-and-forget), so a toast left open never wedges the + * entry -- see that method. Used both for a fresh {@link setup} and for the + * constraint-conflict retry, so a retry click cannot race a run already in + * flight. + */ + private runGuarded(run: () => Promise): Promise { if (this.inFlight) { return this.inFlight; } - const run = this.runSetup().finally(() => { + const guarded = run().finally(() => { this.inFlight = undefined; }); - this.inFlight = run; - return run; + this.inFlight = guarded; + return guarded; } /** @@ -344,8 +360,7 @@ export class PythonSetupEnvironmentSetup implements Disposable { } private async runSetup(): Promise { - const {cli, projectRoot, isVisible, resolveCompute, withProgress} = - this.deps; + const {projectRoot, isVisible, resolveCompute} = this.deps; const cwd = projectRoot(); if (cwd === undefined) { @@ -389,6 +404,24 @@ export class PythonSetupEnvironmentSetup implements Disposable { return; } + await this.runResolved(compute, cwd, preset); + } + + /** + * Run a resolved invocation (compute + preset) to completion: record the + * attempt, spawn the CLI under a progress indicator, then adopt the + * interpreter and persist state on success — or surface a mapped error on + * failure. Split out from {@link runSetup} so the constraint-conflict + * "Retry as DB Connect" recovery can re-enter it with the `dbconnect` preset + * directly, without re-prompting the compute or the preset picker. + */ + private async runResolved( + compute: SetupCompute, + cwd: string, + preset: SetupPreset + ): Promise { + const {cli, withProgress} = this.deps; + const invocation: SetupLocalInvocation = { compute, ...presetToFlags(preset), @@ -469,14 +502,27 @@ export class PythonSetupEnvironmentSetup implements Disposable { result.pythonResolution === "installed_fallback" ? "manual_selection_requested" : result.pythonResolution; - const actions = remediationActions.some( - (candidate) => - candidate.command === SELECT_PYTHON_INTERPRETER_COMMAND_ID - ) - ? remediationActions - : reportAction - ? [reportAction] - : remediationActions; + // Only the Full preset pins cluster dependencies, so a constraint + // conflict is only recoverable when this run carried them: replace + // the generic actions with a one-click retry that drops the pins + // (the DB Connect preset) and a jump to the merged pyproject.toml. A + // conflict on a run that already skipped constraints (which shouldn't + // happen) falls through to the ordinary doc-link handling rather than + // offering a nonsensical, looping "retry as DB Connect". + const recoverableConflict = + result.error?.code === "E_PROVISION_CONFLICT" && + !invocation.skipConstraints; + const actions = recoverableConflict + ? this.buildConflictRecoveryActions(compute, cwd) + : remediationActions.some( + (candidate) => + candidate.command === + SELECT_PYTHON_INTERPRETER_COMMAND_ID + ) + ? remediationActions + : reportAction + ? [reportAction] + : remediationActions; reportResult({ outcome: "failed", ...(pythonSetupFlow !== undefined ? {pythonSetupFlow} : {}), @@ -601,6 +647,39 @@ export class PythonSetupEnvironmentSetup implements Disposable { this.present(this.deps.showSuccess(result)); } + /** + * The two recovery buttons for a Full-preset constraint conflict. Both are + * run-actions (their behavior needs the run's live compute/cwd, so they + * can't be a static url/command): + * + * - "Retry as DB Connect setup" re-enters {@link runResolved} with the + * `dbconnect` preset, which passes `--no-constraints` to drop the + * conflicting cluster-dependency pins while keeping matched Python + + * databricks-connect. It goes through {@link runGuarded} so a click cannot + * race a run already in flight, and because that preset skips constraints + * the retry can't itself surface a recoverable conflict (no loop). + * - "Open pyproject.toml" opens the file the failed run merged into, so the + * user can inspect and adjust the dependencies that clashed. + */ + private buildConflictRecoveryActions( + compute: SetupCompute, + cwd: string + ): PythonSetupErrorAction[] { + return [ + { + label: "Retry as DB Connect setup", + run: () => + this.runGuarded(() => + this.runResolved(compute, cwd, "dbconnect") + ), + }, + { + label: "Open pyproject.toml", + run: () => this.deps.openProjectFile(cwd), + }, + ]; + } + /** * Emit the attempt event for a run that is about to start and return its * outcome reporter. diff --git a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts index 2be23a135..0c7c21df4 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts @@ -919,6 +919,89 @@ describe("makePythonSetupDeps showError", () => { originalOpen; } }); + + it("runs a run-action's callback when its button is picked", async () => { + // A run-action carries an in-process closure (the constraint-conflict + // "Retry as DB Connect" / "Open pyproject.toml" buttons need runtime + // state, so they can't be a static url/command). + const deps = makePythonSetupDeps( + makeWiring({log: {append: () => {}, show: () => {}}}) + ); + let ran = 0; + reply = "Retry as DB Connect setup"; + + await deps.showError("conflict copy", "detail", [ + {label: "Retry as DB Connect setup", run: async () => void ran++}, + ]); + + expect(ran).to.equal(1); + }); + + it("does not run a run-action's callback when its button is not picked", async () => { + const deps = makePythonSetupDeps( + makeWiring({log: {append: () => {}, show: () => {}}}) + ); + let ran = 0; + reply = "Show Logs"; + + await deps.showError("conflict copy", "detail", [ + {label: "Retry as DB Connect setup", run: async () => void ran++}, + ]); + + expect(ran).to.equal(0); + }); + + it("does not reject when a run-action's callback throws", async () => { + // showError is the failure-reporting path and its one caller does not + // wrap it, so a throwing callback must be contained (logged), not escape. + const appended: string[] = []; + const deps = makePythonSetupDeps( + makeWiring({ + log: {append: (c) => appended.push(c), show: () => {}}, + }) + ); + reply = "Retry as DB Connect setup"; + + await deps.showError("conflict copy", "detail", [ + { + label: "Retry as DB Connect setup", + run: async () => { + throw new Error("retry blew up"); + }, + }, + ]); + + expect(appended.join("")).to.contain("retry blew up"); + }); +}); + +describe("makePythonSetupDeps openProjectFile", () => { + let originalShow: typeof window.showTextDocument; + let opened: string[]; + + beforeEach(() => { + originalShow = window.showTextDocument; + opened = []; + (window as unknown as {showTextDocument: unknown}).showTextDocument = + async (uri: Uri) => { + opened.push(uri.fsPath); + return {} as any; + }; + }); + + afterEach(() => { + (window as unknown as {showTextDocument: unknown}).showTextDocument = + originalShow; + }); + + it("opens the project's pyproject.toml in an editor", async () => { + const deps = makePythonSetupDeps(makeWiring()); + + await deps.openProjectFile("/proj"); + + expect(opened).to.have.length(1); + expect(opened[0]).to.match(/[/\\]proj[/\\]pyproject\.toml$/); + }); }); describe("makePythonSetupDeps showSuccess", () => { diff --git a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts index adbf317f8..2e46a892f 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts @@ -276,6 +276,15 @@ export function makePythonSetupDeps( Uri.file(projectRoot) ); }, + openProjectFile: async (projectRoot: string) => { + // Open the pyproject.toml the conflicting run merged into, so the + // user can inspect/adjust the dependencies that clashed. A + // constraint conflict always mutates disk (constraints are merged + // before provisioning fails), so the file exists. + await window.showTextDocument( + Uri.file(path.join(projectRoot, "pyproject.toml")) + ); + }, // Stamp the persisted state with the completion time here (the // orchestrator supplies the env identity; the timestamp is a wiring // concern) and hand it to the injected store for drift detection. @@ -369,11 +378,18 @@ export function makePythonSetupDeps( `\nCould not open ${chosen.url} in a browser.\n` ); } + } else if (chosen.run) { + // A run-action (the constraint-conflict "Retry as DB Connect" + // / "Open pyproject.toml" buttons) invokes an in-process + // callback the orchestrator built with the run's live state. + await chosen.run(); } } catch (e) { const what = chosen.command ? `run ${chosen.command}` - : `open ${chosen.url}`; + : chosen.url + ? `open ${chosen.url}` + : `run the "${chosen.label}" action`; wiring.log.append( `\nFailed to ${what}: ${ e instanceof Error ? e.message : String(e) diff --git a/packages/databricks-vscode/src/python-setup/models/PythonSetupResult.ts b/packages/databricks-vscode/src/python-setup/models/PythonSetupResult.ts index 9f9e50976..4ae3822b1 100644 --- a/packages/databricks-vscode/src/python-setup/models/PythonSetupResult.ts +++ b/packages/databricks-vscode/src/python-setup/models/PythonSetupResult.ts @@ -34,6 +34,13 @@ export type PythonSetupPhaseStatus = "ok" | "error" | "pending"; * ErrorCode set. `E_AUTH` / `E_PYTHON_POLICY` appear in the spec but are never * emitted by the CLI (auth is handled by the shared workspace-client preflight * before a result object is built), so they are intentionally absent here. + * + * `E_PROVISION_CONFLICT` is the distinct code the CLI emits when `uv sync` fails + * specifically because the runtime's pinned dependencies conflict with the + * user's own (only the Full preset pins them); a generic provision failure stays + * `E_PROVISION`. The constraints are already merged at the point of failure + * (`diskMutated: true`), which the extension's retry-as-DB-Connect recovery + * relies on. */ export type PythonSetupErrorCode = | "E_USAGE" @@ -48,6 +55,7 @@ export type PythonSetupErrorCode = | "E_MERGE" | "E_PYTHON_INSTALL" | "E_PROVISION" + | "E_PROVISION_CONFLICT" | "E_VALIDATE"; export interface PythonSetupComputeInfo { diff --git a/packages/databricks-vscode/src/python-setup/utils/errorMessages.test.ts b/packages/databricks-vscode/src/python-setup/utils/errorMessages.test.ts index e0ab5e5b6..84335348d 100644 --- a/packages/databricks-vscode/src/python-setup/utils/errorMessages.test.ts +++ b/packages/databricks-vscode/src/python-setup/utils/errorMessages.test.ts @@ -129,6 +129,15 @@ describe("getPythonSetupErrorMessage", () => { expect(msg).to.not.match(/UV_INDEX_URL|pip\.conf/i); }); + it("maps E_PROVISION_CONFLICT to a cluster-vs-local conflict message", () => { + // The distinct constraint-conflict code (only the Full preset pins + // cluster deps) gets its own actionable copy, not E_PROVISION's generic + // "adjust your dependencies" text. + const msg = getPythonSetupErrorMessage(failure("E_PROVISION_CONFLICT")); + expect(msg).to.match(/cluster dependencies conflict/i); + expect(msg).to.match(/local dependencies/i); + }); + it("maps E_FETCH to an offline/unreachable message", () => { expect(getPythonSetupErrorMessage(failure("E_FETCH"))).to.match( /reach|offline|network/i @@ -305,6 +314,18 @@ describe("getPythonSetupErrorAction", () => { ); }); + it("points an E_PROVISION_CONFLICT at the uv resolution docs (generic fallback)", () => { + // The Full-preset flow builds its own retry/open buttons in the + // orchestrator; this code-keyed link is the fallback for a conflict that + // somehow arrives on a run that already dropped the pins. + expect( + getPythonSetupErrorAction(failure("E_PROVISION_CONFLICT")) + ).to.deep.equal({ + label: "Resolve dependency conflicts", + url: "https://docs.astral.sh/uv/concepts/resolution/", + }); + }); + it("points E_MANAGER_UNSUPPORTED at the uv projects docs", () => { expect( getPythonSetupErrorAction( diff --git a/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts b/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts index df3d2663f..fb1b7ce87 100644 --- a/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts +++ b/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts @@ -152,16 +152,26 @@ export const INSTALL_UV_COMMAND_ID = "databricks.environment.installUv"; /** * An optional remediation button to attach to a failure popup. Exactly one of - * `url` / `command` is set — a discriminated union (`?: never` on the other arm) - * forbids both/neither at compile time, while still letting callers read - * `action.url` / `action.command` as `string | undefined` without narrowing. - * `url` opens an external page (docs, issue); `command` runs a VS Code command - * (e.g. the one-click switch to manual setup). Kept alongside - * {@link getPythonSetupErrorMessage} so the copy and its call-to-action live together. + * `url` / `command` / `run` is set — a discriminated union (`?: never` on the + * other arms) forbids more than one at compile time, while still letting callers + * read `action.url` / `action.command` as `string | undefined` without + * narrowing. `url` opens an external page (docs, issue); `command` runs a VS + * Code command (e.g. the one-click switch to manual setup); `run` invokes an + * in-process callback for a button whose behavior needs runtime state and so + * can't be expressed as a static url/command (the constraint-conflict + * "Retry as DB Connect" / "Open pyproject.toml" buttons, built by the + * orchestrator). Kept alongside {@link getPythonSetupErrorMessage} so the copy + * and its call-to-action live together. */ export type PythonSetupErrorAction = - | {label: string; url: string; command?: never} - | {label: string; command: string; url?: never}; + | {label: string; url: string; command?: never; run?: never} + | {label: string; command: string; url?: never; run?: never} + | { + label: string; + run: () => void | Promise; + url?: never; + command?: never; + }; /* eslint-disable @typescript-eslint/naming-convention */ const BASE_MESSAGE: Record< @@ -199,6 +209,9 @@ const BASE_MESSAGE: Record< E_PROVISION: () => "uv could not resolve the project's dependencies (a version conflict). " + "Review the conflict in the logs and adjust your dependencies.", + E_PROVISION_CONFLICT: () => + "The cluster dependencies conflict with your local dependencies, so uv " + + "sync couldn't resolve the environment.", E_VALIDATE: () => "The provisioned environment did not match the selected runtime.", }; @@ -255,6 +268,13 @@ const DOC_LINKS: Partial> = label: "Resolve dependency conflicts", url: UV_RESOLUTION_DOCS_URL, }, + // The Full-preset flow builds its own retry/open buttons in the + // orchestrator; this is the fallback link for a conflict that somehow + // reaches the generic path (a run that already dropped the pins). + E_PROVISION_CONFLICT: { + label: "Resolve dependency conflicts", + url: UV_RESOLUTION_DOCS_URL, + }, E_NO_TARGET: { label: "Configure compute", url: DATABRICKS_CONFIGURE_DOCS_URL, From abc623c770af8dc3e64a4af17a2f3c180635d04b Mon Sep 17 00:00:00 2001 From: "@rugpanov" Date: Tue, 8 Sep 2026 15:45:03 +0200 Subject: [PATCH 2/8] review: preserve the constraints-report log hint, document retry telemetry, and cover retry edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* Multi-source review (Claude + Codex) flagged: E_PROVISION_CONFLICT silently dropped the soft "report to databricks/environments" log pointer that the same conflict carried while it was E_PROVISION; the retry re-entrancy and no-loop guarantees were untested; and the retry's `initial` telemetry trigger deserved a recorded rationale. *What* - Extend the constraints-report log pointer in formatSetupFailureDetail to cover E_PROVISION_CONFLICT (a pins-vs-local conflict is exactly where the published constraints may be at fault, and it carried the pointer before the CLI split the code out). Button behaviour is unchanged (still no report button). - Add tests: the DB Connect retry failing again gets the generic action, never a second Retry (no loop); and a Retry click while another run is in flight coalesces onto it rather than spawning a concurrent CLI process. Coalescing (not queuing) is deliberate — a queued dbconnect retry could clobber a run about to succeed. - Document why a constraint-conflict retry reports trigger `initial` (the failed Full run never marked the project ready; the `dbconnect` setupPreset distinguishes it). *Verification* - test:unit — 1113 passing (the 2 failures are the stale bundled-CLI version check, a local worktree artifact). New/updated specs for the log pointer, the no-loop path, and the in-flight coalescing all green. - tsc + eslint + prettier clean on changed files. Co-authored-by: Isaac --- .../PythonSetupEnvironmentSetup.test.ts | 91 +++++++++++++++++++ .../PythonSetupEnvironmentSetup.ts | 5 +- .../python-setup/utils/errorMessages.test.ts | 17 ++++ .../src/python-setup/utils/errorMessages.ts | 17 +++- 4 files changed, 124 insertions(+), 6 deletions(-) diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts index 6271a59c6..1b1b44bc7 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts @@ -1906,4 +1906,95 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { }, ]); }); + + it("does not loop: a DB Connect retry that also fails gets the generic action, not another Retry", async () => { + // The retry runs with --no-constraints, so even if it somehow returns + // E_PROVISION_CONFLICT again the !skipConstraints gate makes it + // non-recoverable — the generic doc-link handling, never a second + // "Retry as DB Connect" (which would loop). + const cli = makeScriptedCli([conflictResult(), conflictResult()]); + const shown: {actions?: PythonSetupErrorAction[]}[] = []; + let retryAction: PythonSetupErrorAction | undefined; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli, + pickSetupPreset: async () => "full", + showError: async (_message, _detail, actions) => { + shown.push({actions}); + retryAction = actions?.find( + (a) => a.label === "Retry as DB Connect setup" + ); + }, + }) + ); + + await setup.setup(); + expect(retryAction).to.not.equal(undefined); + await (retryAction as {run: () => Promise}).run(); + + expect(cli.calls).to.have.length(2); + expect(cli.calls[1].skipConstraints).to.equal(true); + // The retry's failure toast carries only the generic doc link — no + // recovery button, so there is no way to loop. + expect(shown).to.have.length(2); + expect(shown[1].actions).to.deep.equal([ + { + label: "Resolve dependency conflicts", + url: "https://docs.astral.sh/uv/concepts/resolution/", + }, + ]); + }); + + it("coalesces a Retry click onto an in-flight run instead of starting a concurrent one", async () => { + // The retry goes through the same re-entrancy guard as a fresh setup: + // if another run is already in flight when the (still-open) conflict + // toast's Retry is clicked, it must coalesce onto that run rather than + // spawn a second, concurrent project-mutating CLI process — and, since a + // queued dbconnect retry could otherwise clobber a run that is about to + // succeed, coalescing (not queuing) is the intended, safe behavior. + let releaseSecond: (r: PythonSetupResult) => void = () => {}; + const secondGate = new Promise((res) => { + releaseSecond = res; + }); + const calls: SetupLocalInvocation[] = []; + let call = 0; + const cli = { + calls, + run: (invocation: SetupLocalInvocation) => { + calls.push(invocation); + return call++ === 0 + ? Promise.resolve(conflictResult()) + : secondGate; + }, + }; + let retryAction: PythonSetupErrorAction | undefined; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli, + pickSetupPreset: async () => "full", + showError: async (_message, _detail, actions) => { + retryAction = actions?.find( + (a) => a.label === "Retry as DB Connect setup" + ); + }, + }) + ); + + // First Full run fails with a conflict; the guard is now released. + await setup.setup(); + expect(calls).to.have.length(1); + expect(retryAction).to.not.equal(undefined); + + // A second setup starts and is left in flight (its guard is held). + const second = setup.setup(); + // Clicking the stale Retry now must coalesce onto the in-flight run. + const retryPromise = (retryAction as {run: () => Promise}).run(); + + releaseSecond(SUCCESS_REAL_RUN); + await Promise.all([second, retryPromise]); + + // Two runs total (the Full conflict and the second run) — the Retry did + // not spawn a third, concurrent CLI process. + expect(calls).to.have.length(2); + }); }); diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts index bba9390fc..319364497 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts @@ -747,7 +747,10 @@ export class PythonSetupEnvironmentSetup implements Disposable { // A run against a project already marked ready this session is a // re-run (the ready row's Re-run button / row click); anything // else is the first setup. Derived from state, not the command, - // so every entry point labels the same event correctly. + // so every entry point labels the same event correctly. A + // constraint-conflict retry therefore reports `initial` (the + // failed Full run never marked the project ready), distinguished + // from the first attempt only by its `dbconnect` setupPreset. trigger: this.readyRoots.has(projectRoot) ? "rerun" : "initial", }); return { diff --git a/packages/databricks-vscode/src/python-setup/utils/errorMessages.test.ts b/packages/databricks-vscode/src/python-setup/utils/errorMessages.test.ts index 84335348d..3823d6bc0 100644 --- a/packages/databricks-vscode/src/python-setup/utils/errorMessages.test.ts +++ b/packages/databricks-vscode/src/python-setup/utils/errorMessages.test.ts @@ -586,6 +586,23 @@ describe("formatSetupFailureDetail", () => { ); }); + it("keeps the constraints-report hint for E_PROVISION_CONFLICT", () => { + // A constraint conflict was E_PROVISION before the CLI split out the + // distinct code; the soft "if you think it's the published constraints, + // report it" log pointer must not silently vanish, since a pins-vs-local + // conflict is exactly the case where the published constraints may be at + // fault. + const detail = formatSetupFailureDetail( + failure("E_PROVISION_CONFLICT", { + message: "No solution found when resolving dependencies", + }) + ); + expect(detail).to.match(/constraint/i); + expect(detail).to.contain( + "https://github.com/databricks/environments/issues/new" + ); + }); + it("adds no constraints-report hint for a blocked-index E_PROVISION", () => { // A blocked index is a local network condition, not a constraint defect. const detail = formatSetupFailureDetail( diff --git a/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts b/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts index fb1b7ce87..b8da2b92f 100644 --- a/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts +++ b/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts @@ -423,11 +423,18 @@ export function formatSetupFailureDetail( 'setting to "manual". The extension then uses your existing interpreter/.venv (with its databricks-connect) as-is.' ); } - // A genuine E_PROVISION conflict gets no report button (it is usually the - // user's own dependencies). But if the *published constraints* are what - // conflict, that is a defect worth reporting — so offer a soft, conditional - // pointer here. Excludes the blocked-index variant, a local network issue. - if (err.code === "E_PROVISION" && !isIndexUnreachableFailure(result)) { + // A dependency conflict gets no report button (it is usually the user's own + // dependencies). But if the *published constraints* are what conflict, that + // is a defect worth reporting — so offer a soft, conditional pointer here. + // Covers both the generic E_PROVISION conflict and the distinct + // E_PROVISION_CONFLICT (a pins-vs-local conflict is exactly where the + // published constraints may be at fault, and it was E_PROVISION — carrying + // this pointer — before the CLI split the code out). Excludes the + // blocked-index variant, a local network issue. + if ( + (err.code === "E_PROVISION" || err.code === "E_PROVISION_CONFLICT") && + !isIndexUnreachableFailure(result) + ) { lines.push( "", "If you believe this conflict comes from the published runtime " + From 8e97a6ecf74397759dc7dce894e7604c8568b16f Mon Sep 17 00:00:00 2001 From: "@rugpanov" Date: Tue, 8 Sep 2026 15:56:14 +0200 Subject: [PATCH 3/8] review: guard the conflict Retry against re-provisioning an already-set-up project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* Re-review (Claude) flagged a footgun: the constraint-conflict toast lingers in the Notifications Center, so its "Retry as DB Connect" can be clicked long after a separate run has since provisioned the project. With no run in flight, that stale click would re-run with --no-constraints and silently downgrade the working Full environment. This is the first toast action that re-enters the mutating setup flow, so it introduces the hazard. *What* - The Retry callback now bails when the project is already set up (`readyRoots.has(cwd)`), so a stale click cannot re-provision. The immediate (intended) retry still runs — the failed Full run never marked the project ready. This also keeps the `trigger: "initial"` note accurate: the retry only ever runs while not ready. *Verification* - test:unit — 1114 passing (the 2 failures are the stale bundled-CLI version check, a local worktree artifact). New spec: a stale Retry after a later success does not re-run the CLI. - tsc + eslint + prettier clean on changed files. Co-authored-by: Isaac --- .../PythonSetupEnvironmentSetup.test.ts | 33 +++++++++++++++++++ .../PythonSetupEnvironmentSetup.ts | 18 +++++++--- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts index 1b1b44bc7..a3d2ab70e 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts @@ -1997,4 +1997,37 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { // not spawn a third, concurrent CLI process. expect(calls).to.have.length(2); }); + + it("does not re-provision when the project became ready before a stale Retry is clicked", async () => { + // The conflict toast lingers in the Notifications Center, so its Retry + // can be clicked long after a separate run has since set the project up. + // Re-provisioning as DB Connect then would silently downgrade the + // working environment, so a stale Retry must no-op once ready. + const cli = makeScriptedCli([conflictResult(), SUCCESS_REAL_RUN]); + let retryAction: PythonSetupErrorAction | undefined; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli, + pickSetupPreset: async () => "full", + showError: async (_message, _detail, actions) => { + retryAction = actions?.find( + (a) => a.label === "Retry as DB Connect setup" + ); + }, + }) + ); + + // Full run conflicts (retry captured, project not ready)… + await setup.setup(); + expect(setup.ready).to.equal(false); + // …then a later run succeeds and marks the project ready. + await setup.setup(); + expect(setup.ready).to.equal(true); + expect(cli.calls).to.have.length(2); + + // Clicking the now-stale Retry must not re-run the CLI. + await (retryAction as {run: () => Promise}).run(); + expect(cli.calls).to.have.length(2); + expect(setup.ready).to.equal(true); + }); }); diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts index 319364497..0e8a505d8 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts @@ -657,7 +657,11 @@ export class PythonSetupEnvironmentSetup implements Disposable { * conflicting cluster-dependency pins while keeping matched Python + * databricks-connect. It goes through {@link runGuarded} so a click cannot * race a run already in flight, and because that preset skips constraints - * the retry can't itself surface a recoverable conflict (no loop). + * the retry can't itself surface a recoverable conflict (no loop). It also + * bails when the project is already set up: the conflict toast lingers in + * the Notifications Center, so its Retry can be clicked long after a + * separate run has since provisioned the project — re-running as DB Connect + * then would silently drop the pins and downgrade a working environment. * - "Open pyproject.toml" opens the file the failed run merged into, so the * user can inspect and adjust the dependencies that clashed. */ @@ -668,10 +672,16 @@ export class PythonSetupEnvironmentSetup implements Disposable { return [ { label: "Retry as DB Connect setup", - run: () => - this.runGuarded(() => + run: () => { + // A stale Retry (project provisioned by a later run since the + // conflict) must not re-provision and downgrade it. + if (this.readyRoots.has(cwd)) { + return; + } + return this.runGuarded(() => this.runResolved(compute, cwd, "dbconnect") - ), + ); + }, }, { label: "Open pyproject.toml", From 9eec94b8bc2c5e893872d621b1385ea967780c02 Mon Sep 17 00:00:00 2001 From: "@rugpanov" Date: Tue, 8 Sep 2026 17:59:05 +0200 Subject: [PATCH 4/8] refine the constraint-conflict recovery toast buttons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* Local testing surfaced two UX rough edges on the new conflict toast: the retry label overflowed the VS Code notification's button row (truncating a button), and it read a little jargon-y on its own. Three buttons also crowded the row. *What* - Rename the retry action to "Retry DB Connect setup" — it mirrors the setup picker's "DB Connect setup" tier 1:1 (what the user would otherwise choose), and avoids CLI-internal wording ("constraints") that appears nowhere the user can see. - Add an `includeShowLogs` option to the `showError` seam and pass `false` for the recoverable conflict, so its toast shows just [Retry DB Connect setup] [Open pyproject.toml] — a self-service row whose own buttons are the remedy. The output channel is still written and revealed, so the log stays reachable. Every other error toast keeps its Show Logs button. *Verification* - test:unit — 1119 passing. New/updated specs: `showError` omits Show Logs when `includeShowLogs` is false; the conflict path passes it while an ordinary failure does not; label assertions updated. - tsc + eslint + prettier clean on changed files. Co-authored-by: Isaac --- .../PythonSetupEnvironmentSetup.test.ts | 53 +++++++++++++++---- .../PythonSetupEnvironmentSetup.ts | 21 ++++++-- .../controllers/pythonSetupDeps.test.ts | 38 ++++++++++--- .../controllers/pythonSetupDeps.ts | 16 ++++-- .../src/python-setup/utils/errorMessages.ts | 2 +- 5 files changed, 104 insertions(+), 26 deletions(-) diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts index a3d2ab70e..5d6bbd80e 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts @@ -1766,7 +1766,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { expect(shown[0].message).to.match(/cluster dependencies conflict/i); // The two recovery buttons, in order; both are in-process run-actions. expect(shown[0].actions?.map((a) => a.label)).to.deep.equal([ - "Retry as DB Connect setup", + "Retry DB Connect setup", "Open pyproject.toml", ]); for (const action of shown[0].actions ?? []) { @@ -1774,7 +1774,42 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { } }); - it("re-runs with --no-constraints (and adopts) when Retry as DB Connect is picked", async () => { + it("hides the Show Logs button on the conflict toast (self-service)", async () => { + const seen: Array<{includeShowLogs?: boolean} | undefined> = []; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli: makeCli({resolve: conflictResult()}), + pickSetupPreset: async () => "full", + showError: async (_message, _detail, _actions, options) => { + seen.push(options); + }, + }) + ); + + await setup.setup(); + + expect(seen).to.have.length(1); + expect(seen[0]).to.deep.equal({includeShowLogs: false}); + }); + + it("keeps the Show Logs button for an ordinary (non-conflict) failure", async () => { + const seen: Array<{includeShowLogs?: boolean} | undefined> = []; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli: makeCli({resolve: ERROR_NO_TARGET}), + showError: async (_message, _detail, _actions, options) => { + seen.push(options); + }, + }) + ); + + await setup.setup(); + + // No opt-out: showError keeps its default trailing Show Logs button. + expect(seen[0]).to.equal(undefined); + }); + + it("re-runs with --no-constraints (and adopts) when Retry DB Connect setup is picked", async () => { const cli = makeScriptedCli([conflictResult(), SUCCESS_REAL_RUN]); const adopted: string[] = []; let retryAction: PythonSetupErrorAction | undefined; @@ -1787,7 +1822,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { }, showError: async (_message, _detail, actions) => { retryAction = actions?.find( - (a) => a.label === "Retry as DB Connect setup" + (a) => a.label === "Retry DB Connect setup" ); }, }) @@ -1821,7 +1856,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { pickSetupPreset: async () => "full", showError: async (_message, _detail, actions) => { retryAction = actions?.find( - (a) => a.label === "Retry as DB Connect setup" + (a) => a.label === "Retry DB Connect setup" ); }, }) @@ -1884,7 +1919,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { it("falls back to the generic action when a conflict arrives on a run that already dropped the pins", async () => { // Defensive: only Full pins cluster deps, so a conflict on a dbconnect - // run should never happen — but if it does, "Retry as DB Connect" would + // run should never happen — but if it does, "Retry DB Connect setup" would // be nonsensical (and loop), so use the ordinary doc-link handling. const shown: {actions?: PythonSetupErrorAction[]}[] = []; const setup = new PythonSetupEnvironmentSetup( @@ -1911,7 +1946,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { // The retry runs with --no-constraints, so even if it somehow returns // E_PROVISION_CONFLICT again the !skipConstraints gate makes it // non-recoverable — the generic doc-link handling, never a second - // "Retry as DB Connect" (which would loop). + // "Retry DB Connect setup" (which would loop). const cli = makeScriptedCli([conflictResult(), conflictResult()]); const shown: {actions?: PythonSetupErrorAction[]}[] = []; let retryAction: PythonSetupErrorAction | undefined; @@ -1922,7 +1957,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { showError: async (_message, _detail, actions) => { shown.push({actions}); retryAction = actions?.find( - (a) => a.label === "Retry as DB Connect setup" + (a) => a.label === "Retry DB Connect setup" ); }, }) @@ -1974,7 +2009,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { pickSetupPreset: async () => "full", showError: async (_message, _detail, actions) => { retryAction = actions?.find( - (a) => a.label === "Retry as DB Connect setup" + (a) => a.label === "Retry DB Connect setup" ); }, }) @@ -2011,7 +2046,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { pickSetupPreset: async () => "full", showError: async (_message, _detail, actions) => { retryAction = actions?.find( - (a) => a.label === "Retry as DB Connect setup" + (a) => a.label === "Retry DB Connect setup" ); }, }) diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts index 0e8a505d8..dcfa267fd 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts @@ -179,11 +179,17 @@ export interface PythonSetupSetupDeps { * each opens an external URL or runs a VS Code command. Most failures carry * one; `E_UV_MISSING` carries two ("Install uv" + "Installation guide", see * `getPythonSetupErrorActions`). + * + * `options.includeShowLogs` defaults to true; pass `false` to omit the + * trailing "Show Logs" button — for a self-service toast whose own buttons + * are the remedy (the recoverable constraint conflict), so the row stays + * short. The channel is still written and revealed, so the log is reachable. */ showError: ( message: string, detail?: string, - actions?: PythonSetupErrorAction[] + actions?: PythonSetupErrorAction[], + options?: {includeShowLogs?: boolean} ) => Promise; showSuccess: (result: PythonSetupResult) => Promise; @@ -412,7 +418,7 @@ export class PythonSetupEnvironmentSetup implements Disposable { * attempt, spawn the CLI under a progress indicator, then adopt the * interpreter and persist state on success — or surface a mapped error on * failure. Split out from {@link runSetup} so the constraint-conflict - * "Retry as DB Connect" recovery can re-enter it with the `dbconnect` preset + * "Retry DB Connect setup" recovery can re-enter it with the `dbconnect` preset * directly, without re-prompting the compute or the preset picker. */ private async runResolved( @@ -541,7 +547,12 @@ export class PythonSetupEnvironmentSetup implements Disposable { result, reportRepo ? reportLogLink(reportRepo) : undefined ), - actions + actions, + // The recoverable conflict is self-service via its Retry / + // Open buttons, so drop the trailing "Show Logs" to keep the + // notification's button row short (the channel is revealed + // regardless). + recoverableConflict ? {includeShowLogs: false} : undefined ) ); return; @@ -652,7 +663,7 @@ export class PythonSetupEnvironmentSetup implements Disposable { * run-actions (their behavior needs the run's live compute/cwd, so they * can't be a static url/command): * - * - "Retry as DB Connect setup" re-enters {@link runResolved} with the + * - "Retry DB Connect setup" re-enters {@link runResolved} with the * `dbconnect` preset, which passes `--no-constraints` to drop the * conflicting cluster-dependency pins while keeping matched Python + * databricks-connect. It goes through {@link runGuarded} so a click cannot @@ -671,7 +682,7 @@ export class PythonSetupEnvironmentSetup implements Disposable { ): PythonSetupErrorAction[] { return [ { - label: "Retry as DB Connect setup", + label: "Retry DB Connect setup", run: () => { // A stale Retry (project provisioned by a later run since the // conflict) must not re-provision and downgrade it. diff --git a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts index 0c7c21df4..564569c01 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts @@ -922,21 +922,47 @@ describe("makePythonSetupDeps showError", () => { it("runs a run-action's callback when its button is picked", async () => { // A run-action carries an in-process closure (the constraint-conflict - // "Retry as DB Connect" / "Open pyproject.toml" buttons need runtime + // "Retry DB Connect setup" / "Open pyproject.toml" buttons need runtime // state, so they can't be a static url/command). const deps = makePythonSetupDeps( makeWiring({log: {append: () => {}, show: () => {}}}) ); let ran = 0; - reply = "Retry as DB Connect setup"; + reply = "Retry DB Connect setup"; await deps.showError("conflict copy", "detail", [ - {label: "Retry as DB Connect setup", run: async () => void ran++}, + {label: "Retry DB Connect setup", run: async () => void ran++}, ]); expect(ran).to.equal(1); }); + it("omits the Show Logs button when includeShowLogs is false", async () => { + // A self-service toast (the recoverable constraint conflict) drops the + // trailing Show Logs so its own action buttons fit; the channel is still + // revealed, so the log stays reachable. + const deps = makePythonSetupDeps( + makeWiring({log: {append: () => {}, show: () => {}}}) + ); + reply = undefined; + + await deps.showError( + "conflict copy", + "detail", + [ + {label: "Retry DB Connect setup", run: async () => {}}, + {label: "Open pyproject.toml", run: async () => {}}, + ], + {includeShowLogs: false} + ); + + expect(shownWith[0].actions).to.deep.equal([ + "Retry DB Connect setup", + "Open pyproject.toml", + ]); + expect(shownWith[0].actions).to.not.contain("Show Logs"); + }); + it("does not run a run-action's callback when its button is not picked", async () => { const deps = makePythonSetupDeps( makeWiring({log: {append: () => {}, show: () => {}}}) @@ -945,7 +971,7 @@ describe("makePythonSetupDeps showError", () => { reply = "Show Logs"; await deps.showError("conflict copy", "detail", [ - {label: "Retry as DB Connect setup", run: async () => void ran++}, + {label: "Retry DB Connect setup", run: async () => void ran++}, ]); expect(ran).to.equal(0); @@ -960,11 +986,11 @@ describe("makePythonSetupDeps showError", () => { log: {append: (c) => appended.push(c), show: () => {}}, }) ); - reply = "Retry as DB Connect setup"; + reply = "Retry DB Connect setup"; await deps.showError("conflict copy", "detail", [ { - label: "Retry as DB Connect setup", + label: "Retry DB Connect setup", run: async () => { throw new Error("retry blew up"); }, diff --git a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts index 2e46a892f..218c05462 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts @@ -316,7 +316,8 @@ export function makePythonSetupDeps( showError: async ( message: string, detail?: string, - actions: PythonSetupErrorAction[] = [] + actions: PythonSetupErrorAction[] = [], + options?: {includeShowLogs?: boolean} ) => { // The mapped one-liner is deliberately concise and drops the CLI's // own explanation; write that detail into the channel so the log the @@ -345,10 +346,15 @@ export function makePythonSetupDeps( }); // Lead with the remediation buttons (e.g. "Install uv", then // "Installation guide") in order, so the action the user most likely - // wants comes first; "Show Logs" always trails. - const buttons = [...remediations.map((a) => a.label), showLogs]; + // wants comes first; "Show Logs" trails, unless the caller opted it + // out (a self-service toast whose own buttons are the remedy) — the + // channel is revealed above regardless, so the log stays reachable. + const includeShowLogs = options?.includeShowLogs !== false; + const buttons = includeShowLogs + ? [...remediations.map((a) => a.label), showLogs] + : remediations.map((a) => a.label); const picked = await window.showErrorMessage(message, ...buttons); - if (picked === showLogs) { + if (includeShowLogs && picked === showLogs) { wiring.log.show(); return; } @@ -379,7 +385,7 @@ export function makePythonSetupDeps( ); } } else if (chosen.run) { - // A run-action (the constraint-conflict "Retry as DB Connect" + // A run-action (the constraint-conflict "Retry DB Connect setup" // / "Open pyproject.toml" buttons) invokes an in-process // callback the orchestrator built with the run's live state. await chosen.run(); diff --git a/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts b/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts index b8da2b92f..d5b8a63b8 100644 --- a/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts +++ b/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts @@ -159,7 +159,7 @@ export const INSTALL_UV_COMMAND_ID = "databricks.environment.installUv"; * Code command (e.g. the one-click switch to manual setup); `run` invokes an * in-process callback for a button whose behavior needs runtime state and so * can't be expressed as a static url/command (the constraint-conflict - * "Retry as DB Connect" / "Open pyproject.toml" buttons, built by the + * "Retry DB Connect setup" / "Open pyproject.toml" buttons, built by the * orchestrator). Kept alongside {@link getPythonSetupErrorMessage} so the copy * and its call-to-action live together. */ From abd1385b49a3c24c2cd3cf1491210e75391dd563 Mon Sep 17 00:00:00 2001 From: "@rugpanov" Date: Tue, 8 Sep 2026 18:52:52 +0200 Subject: [PATCH 5/8] telemetry: label the conflict retry with a conflict_retry trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* To measure how often the "Retry DB Connect setup" button is clicked and how often that retry then succeeds — reusing the existing 1:1 python_env.setup.attempt → python_env.setup.result pairing rather than adding a new event. The only gap was that a conflict retry is indistinguishable from a first-time DB Connect pick: both record trigger "initial" + setupPreset "dbconnect". A discriminator on the trigger dimension closes it. *What* - Add "conflict_retry" to PythonSetupRunTrigger; update the trigger comment on python_env.setup.attempt and the PythonSetupAttempt doc to describe it. - Thread an explicit trigger through runResolved → recordAttempt. The normal path still derives readyRoots.has(cwd) ? "rerun" : "initial"; the retry re-entry in buildConflictRecoveryActions passes "conflict_retry". No structural change downstream — the paired result event already carries the outcome. Resulting metrics: (a) clicks = attempts with trigger=conflict_retry; (b) success = the paired result with outcome=ok; rate = (b)/(a), with the failure breakdown free from the same result event. The two no-op click paths (stale / guard-coalesced) record no attempt by design, so (a) stays the true denominator. *Verification* - test:unit — 1119 passing. The conflict→retry test now asserts the retry attempt records trigger "conflict_retry" and the paired result its outcome. - tsc + eslint + prettier clean on changed files. Co-authored-by: Isaac --- .../PythonSetupEnvironmentSetup.test.ts | 21 ++++++++-- .../PythonSetupEnvironmentSetup.ts | 39 +++++++++++++------ .../src/telemetry/constants.ts | 10 +++-- .../src/telemetry/pythonSetupExtensions.ts | 8 ++-- 4 files changed, 56 insertions(+), 22 deletions(-) diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts index 5d6bbd80e..7aedbee23 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts @@ -1845,7 +1845,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { expect(setup.ready).to.equal(true); }); - it("records the retry attempt as the dbconnect preset", async () => { + it("records the retry as a dbconnect run with the conflict_retry trigger", async () => { const cli = makeScriptedCli([conflictResult(), SUCCESS_REAL_RUN]); const telemetry = makeTelemetryRecorder(); let retryAction: PythonSetupErrorAction | undefined; @@ -1865,10 +1865,23 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { await setup.setup(); await (retryAction as {run: () => Promise}).run(); - expect(telemetry.attempts.map((a) => a.setupPreset)).to.deep.equal([ - "full", - "dbconnect", + // The initial Full attempt is a first-time setup; the retry is labeled + // distinctly so a conflict recovery can be counted separately from a + // first-time DB Connect pick (both would otherwise be initial/dbconnect). + expect( + telemetry.attempts.map((a) => ({ + preset: a.setupPreset, + trigger: a.trigger, + })) + ).to.deep.equal([ + {preset: "full", trigger: "initial"}, + {preset: "dbconnect", trigger: "conflict_retry"}, ]); + // The paired result carries the retry's outcome — success here — so + // (b)/(a) is derivable from the same attempt→result pairing. + expect(telemetry.results).to.have.length(2); + expect(telemetry.results[0].outcome).to.equal("failed"); + expect(telemetry.results[1].outcome).to.equal("ok"); }); it("opens pyproject.toml (at the run's cwd) when Open pyproject.toml is picked", async () => { diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts index dcfa267fd..65840c021 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts @@ -34,6 +34,7 @@ import { PythonSetupAttempt, PythonSetupResultReporter, } from "../../telemetry/pythonSetupExtensions"; +import {PythonSetupRunTrigger} from "../../telemetry/constants"; import {PrimaryManager} from "../../language/packageManagerDetection"; import { isUvSetupSuitable, @@ -420,11 +421,16 @@ export class PythonSetupEnvironmentSetup implements Disposable { * failure. Split out from {@link runSetup} so the constraint-conflict * "Retry DB Connect setup" recovery can re-enter it with the `dbconnect` preset * directly, without re-prompting the compute or the preset picker. + * + * `trigger` overrides how the attempt is labeled: the retry passes + * `conflict_retry`; the normal path leaves it undefined so + * {@link recordAttempt} derives `initial` / `rerun` from readiness. */ private async runResolved( compute: SetupCompute, cwd: string, - preset: SetupPreset + preset: SetupPreset, + trigger?: PythonSetupRunTrigger ): Promise { const {cli, withProgress} = this.deps; @@ -440,7 +446,8 @@ export class PythonSetupEnvironmentSetup implements Disposable { const {reportResult, packageManager} = await this.recordAttempt( invocation, cwd, - preset + preset, + trigger ); // Per-run report context: the static build info plus this run's manager. const reportEnv: ReportEnvironment = { @@ -690,7 +697,12 @@ export class PythonSetupEnvironmentSetup implements Disposable { return; } return this.runGuarded(() => - this.runResolved(compute, cwd, "dbconnect") + this.runResolved( + compute, + cwd, + "dbconnect", + "conflict_retry" + ) ); }, }, @@ -717,7 +729,8 @@ export class PythonSetupEnvironmentSetup implements Disposable { private async recordAttempt( invocation: SetupLocalInvocation, projectRoot: string, - setupPreset: SetupPreset + setupPreset: SetupPreset, + trigger?: PythonSetupRunTrigger ): Promise<{ reportResult: PythonSetupResultReporter; packageManager: PrimaryManager; @@ -765,14 +778,16 @@ export class PythonSetupEnvironmentSetup implements Disposable { mode: invocation.skipDbconnect ? "constraints-only" : "default", setupPreset, isGreenfield, - // A run against a project already marked ready this session is a - // re-run (the ready row's Re-run button / row click); anything - // else is the first setup. Derived from state, not the command, - // so every entry point labels the same event correctly. A - // constraint-conflict retry therefore reports `initial` (the - // failed Full run never marked the project ready), distinguished - // from the first attempt only by its `dbconnect` setupPreset. - trigger: this.readyRoots.has(projectRoot) ? "rerun" : "initial", + // An explicit trigger wins (the constraint-conflict retry passes + // `conflict_retry`, so its recovery clicks are countable and not + // conflated with a first-time DB Connect pick). Otherwise it is + // derived from state: a run against a project already marked ready + // this session is a re-run (the ready row's Re-run button / row + // click), anything else the first setup. Derived from state, not + // the command, so every entry point labels the same event. + trigger: + trigger ?? + (this.readyRoots.has(projectRoot) ? "rerun" : "initial"), }); return { packageManager, diff --git a/packages/databricks-vscode/src/telemetry/constants.ts b/packages/databricks-vscode/src/telemetry/constants.ts index 8e0c91736..c14b9a929 100644 --- a/packages/databricks-vscode/src/telemetry/constants.ts +++ b/packages/databricks-vscode/src/telemetry/constants.ts @@ -134,7 +134,7 @@ export type SetupTrigger = "auto_open" | "explicit_command" | "run" | "debug"; * again. One event, one enum dimension — so re-runs stay analysable without * fingerprinting on the command id. */ -export type PythonSetupRunTrigger = "initial" | "rerun"; +export type PythonSetupRunTrigger = "initial" | "rerun" | "conflict_retry"; /** Categorical outcome of Python acquisition and its user recovery path. */ export type PythonSetupFlow = @@ -557,9 +557,13 @@ export class EventTypes { "IDs/names, paths, or package names.", trigger: { comment: - "initial (first setup for the project this session) or rerun (re-running over an " + + "initial (first setup for the project this session), rerun (re-running over an " + "environment already provisioned this session, e.g. via the ready row's Re-run " + - "button). Session-scoped: a run after a window reload reads as initial again", + "button), or conflict_retry (a Full-preset run hit a cluster-vs-local dependency " + + "conflict and the user clicked 'Retry DB Connect setup', which re-runs with " + + "--no-constraints). conflict_retry counts conflict-recovery clicks; pair it with " + + "the matching result's outcome for the recovery success rate. Session-scoped: a " + + "run after a window reload reads as initial again", }, packageManager: { comment: diff --git a/packages/databricks-vscode/src/telemetry/pythonSetupExtensions.ts b/packages/databricks-vscode/src/telemetry/pythonSetupExtensions.ts index a7e9a89dd..4af52c6cf 100644 --- a/packages/databricks-vscode/src/telemetry/pythonSetupExtensions.ts +++ b/packages/databricks-vscode/src/telemetry/pythonSetupExtensions.ts @@ -41,9 +41,11 @@ export interface PythonSetupAttempt { */ isGreenfield?: boolean; /** - * Whether this is the first setup for the project this session or a re-run - * over an environment already provisioned this session (session-scoped). - * Same event, one enum dimension. + * Whether this is the first setup for the project this session (`initial`), a + * re-run over an environment already provisioned this session (`rerun`), or a + * constraint-conflict recovery — the `Retry DB Connect setup` click on a + * failed Full-preset run (`conflict_retry`). Session-scoped; same event, one + * enum dimension. */ trigger: PythonSetupRunTrigger; } From cd2f6d056dea21b7e1394e9394ba0df187d69780 Mon Sep 17 00:00:00 2001 From: "@rugpanov" Date: Wed, 9 Sep 2026 12:38:21 +0200 Subject: [PATCH 6/8] Restore the pre-merge pyproject.toml backup before the DB Connect retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* The constraint-conflict "Retry DB Connect setup" recovery re-ran with --no-constraints, but the conflict fails in the provision phase *after* the runtime pins were already merged into pyproject.toml (diskMutated: true). --no-constraints only stops the retry from re-adding the pins; it does not remove the conflicting ones the first run already wrote, so the retry would resolve against the same conflict. *What* - Add an injected restoreProjectFile(projectRoot, backupPath) seam; the production wiring copies the CLI's backupPath over /pyproject.toml (no new temp backup — reuses PythonSetupResult.backupPath). - Thread result.backupPath into buildConflictRecoveryActions. On Retry, restore the file first, then re-run runResolved(..., "dbconnect", "conflict_retry"). - Gate the recoverable-conflict path on a present backupPath: with no backup there is nothing to roll back to, so Retry is not offered and the run falls through to the generic doc-link action. - Restore runs inside runGuarded, so a failed restore throws before the CLI spawns or a retry attempt is recorded; showError's action-error handling logs it. Trim the now-duplicated rationale comments to a single home. *Verification* - New/updated unit tests: restore-before-CLI ordering, skipConstraints still set, successful retry adopts + marks ready, restore failure prevents the second CLI run and retry telemetry, missing backupPath does not offer Retry, and the wiring copies the backup over pyproject.toml. - Full unit suite green (1123 passing); tsc, ESLint, Prettier clean on changed files. Co-authored-by: Isaac --- .../PythonSetupEnvironmentSetup.test.ts | 125 +++++++++++++++++- .../PythonSetupEnvironmentSetup.ts | 79 ++++++----- .../controllers/pythonSetupDeps.test.ts | 26 ++++ .../controllers/pythonSetupDeps.ts | 9 ++ .../python-setup/models/PythonSetupResult.ts | 7 +- .../src/python-setup/utils/errorMessages.ts | 5 +- 6 files changed, 215 insertions(+), 36 deletions(-) diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts index 7aedbee23..83cba4d1a 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts @@ -125,6 +125,7 @@ function makeDeps( }), adoptInterpreter: async () => {}, openProjectFile: async () => {}, + restoreProjectFile: async () => {}, saveState: () => {}, notify: async () => {}, showReauthPrompt: async () => {}, @@ -1835,7 +1836,7 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { expect(retryAction).to.not.equal(undefined); await (retryAction as {run: () => Promise}).run(); - // Two runs: the Full attempt, then the DB Connect retry dropping the pins. + // Two runs: the Full attempt, then the DB Connect retry (restore + --no-constraints). expect(cli.calls).to.have.length(2); expect(cli.calls[0].skipConstraints).to.equal(undefined); expect(cli.calls[1].skipConstraints).to.equal(true); @@ -2078,4 +2079,126 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { expect(cli.calls).to.have.length(2); expect(setup.ready).to.equal(true); }); + + it("restores the CLI's backup before the DB Connect retry re-runs", async () => { + // The conflict fails after the pins were merged into pyproject.toml, so + // the retry must roll the file back to the CLI's backup *before* + // re-running with --no-constraints (which alone would leave the + // already-written conflicting pins in place). + const scripted = makeScriptedCli([conflictResult(), SUCCESS_REAL_RUN]); + const events: string[] = []; + const restored: Array<{root: string; backupPath: string}> = []; + // A CliRunner wrapper that records the run order (assigned to a variable + // so the extra fields aren't excess-property-checked as a literal would). + const cli = { + run: async (invocation: SetupLocalInvocation) => { + events.push("cli"); + return scripted.run(invocation); + }, + }; + let retryAction: PythonSetupErrorAction | undefined; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli, + pickSetupPreset: async () => "full", + restoreProjectFile: async (root, backupPath) => { + events.push("restore"); + restored.push({root, backupPath}); + }, + showError: async (_message, _detail, actions) => { + retryAction = actions?.find( + (a) => a.label === "Retry DB Connect setup" + ); + }, + }) + ); + + await setup.setup(); + await (retryAction as {run: () => Promise}).run(); + + // The restore targets the run's cwd with the CLI-reported backup path… + expect(restored).to.deep.equal([ + {root: "/proj", backupPath: "/proj/pyproject.toml.bak"}, + ]); + // …and it happens between the failed Full run and the DB Connect retry. + expect(events).to.deep.equal(["cli", "restore", "cli"]); + }); + + it("does not run the CLI or record a retry attempt when the restore fails", async () => { + // A failed restore leaves the conflicting pins on disk, so re-running + // would just conflict again: abort before spawning the CLI or recording + // a second attempt, and let showError's action-error handling log it. + const cli = makeScriptedCli([conflictResult(), SUCCESS_REAL_RUN]); + const telemetry = makeTelemetryRecorder(); + let retryAction: PythonSetupErrorAction | undefined; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli, + ...telemetry, + pickSetupPreset: async () => "full", + restoreProjectFile: async () => { + throw new Error("copy failed"); + }, + showError: async (_message, _detail, actions) => { + retryAction = actions?.find( + (a) => a.label === "Retry DB Connect setup" + ); + }, + }) + ); + + await setup.setup(); + expect(retryAction).to.not.equal(undefined); + + // The run-action rejects so showError (its only production caller) can + // log the failure; here we assert only that nothing was launched. + let threw = false; + try { + await (retryAction as {run: () => Promise}).run(); + } catch { + threw = true; + } + expect(threw).to.equal(true); + + // Only the initial Full attempt happened: no DB Connect retry ran, and + // no second attempt/result was recorded. + expect(cli.calls).to.have.length(1); + expect(telemetry.attempts).to.have.length(1); + expect(telemetry.results).to.have.length(1); + expect(setup.ready).to.equal(false); + }); + + it("does not offer Retry when the conflict result has no backup to restore", async () => { + // Without a backupPath there is nothing to roll the merged pins back to, + // so the restore-then-retry recovery cannot run — fall through to the + // ordinary doc-link handling instead of offering a Retry that would + // re-run against the still-conflicting file. + const noBackup: PythonSetupResult = { + ...conflictResult(), + backupPath: undefined, + }; + const shown: {actions?: PythonSetupErrorAction[]}[] = []; + const seenOptions: Array<{includeShowLogs?: boolean} | undefined> = []; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli: makeCli({resolve: noBackup}), + pickSetupPreset: async () => "full", + showError: async (_message, _detail, actions, options) => { + shown.push({actions}); + seenOptions.push(options); + }, + }) + ); + + await setup.setup(); + + expect(shown[0].actions).to.deep.equal([ + { + label: "Resolve dependency conflicts", + url: "https://docs.astral.sh/uv/concepts/resolution/", + }, + ]); + // Not a self-service toast without the Retry button, so Show Logs stays. + expect(seenOptions[0]).to.equal(undefined); + }); }); diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts index 65840c021..4455c2fb0 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts @@ -153,6 +153,18 @@ export interface PythonSetupSetupDeps { */ openProjectFile: (projectRoot: string) => Promise; + /** + * Restore `/pyproject.toml` from the CLI's pre-merge + * `backupPath` (see {@link PythonSetupResult.backupPath}) by copying it over + * the file. First step of the "Retry DB Connect setup" recovery, whose + * rationale lives on {@link buildConflictRecoveryActions}; a rejection there + * aborts the retry before any CLI run. + */ + restoreProjectFile: ( + projectRoot: string, + backupPath: string + ) => Promise; + saveState: (state: PythonSetupPersistedState) => void; /** @@ -515,18 +527,23 @@ export class PythonSetupEnvironmentSetup implements Disposable { result.pythonResolution === "installed_fallback" ? "manual_selection_requested" : result.pythonResolution; - // Only the Full preset pins cluster dependencies, so a constraint - // conflict is only recoverable when this run carried them: replace - // the generic actions with a one-click retry that drops the pins - // (the DB Connect preset) and a jump to the merged pyproject.toml. A - // conflict on a run that already skipped constraints (which shouldn't - // happen) falls through to the ordinary doc-link handling rather than - // offering a nonsensical, looping "retry as DB Connect". - const recoverableConflict = + // A constraint conflict is recoverable only when this run carried the + // pins (Full preset) AND the CLI saved a pre-merge backup to roll them + // back to. Without either, fall through to the ordinary doc-link + // handling rather than offer a "retry as DB Connect" that can't work + // (nothing to restore) or would loop (a run that already skipped pins). + const conflictBackupPath = result.error?.code === "E_PROVISION_CONFLICT" && - !invocation.skipConstraints; + !invocation.skipConstraints + ? result.backupPath + : undefined; + const recoverableConflict = conflictBackupPath !== undefined; const actions = recoverableConflict - ? this.buildConflictRecoveryActions(compute, cwd) + ? this.buildConflictRecoveryActions( + compute, + cwd, + conflictBackupPath + ) : remediationActions.some( (candidate) => candidate.command === @@ -666,26 +683,25 @@ export class PythonSetupEnvironmentSetup implements Disposable { } /** - * The two recovery buttons for a Full-preset constraint conflict. Both are - * run-actions (their behavior needs the run's live compute/cwd, so they - * can't be a static url/command): + * The two recovery buttons for a Full-preset constraint conflict, both + * run-actions (their behavior needs the run's live compute/cwd): * - * - "Retry DB Connect setup" re-enters {@link runResolved} with the - * `dbconnect` preset, which passes `--no-constraints` to drop the - * conflicting cluster-dependency pins while keeping matched Python + - * databricks-connect. It goes through {@link runGuarded} so a click cannot - * race a run already in flight, and because that preset skips constraints - * the retry can't itself surface a recoverable conflict (no loop). It also - * bails when the project is already set up: the conflict toast lingers in - * the Notifications Center, so its Retry can be clicked long after a - * separate run has since provisioned the project — re-running as DB Connect - * then would silently drop the pins and downgrade a working environment. - * - "Open pyproject.toml" opens the file the failed run merged into, so the - * user can inspect and adjust the dependencies that clashed. + * - "Retry DB Connect setup" restores pyproject.toml from the CLI's pre-merge + * `backupPath`, then re-runs as DB Connect (`--no-constraints`). The restore + * is load-bearing: the conflict fails *after* the pins were merged to disk, + * so `--no-constraints` alone would leave the conflicting pins in place and + * only skip re-adding them. It runs through {@link runGuarded} — so a click + * can't race an in-flight run, and the restore only fires when the retry + * actually runs — and no-ops once the project is ready, so a stale toast's + * Retry can't downgrade an environment a later run provisioned. A failed + * restore throws before the CLI spawns, leaving showError to log it. + * - "Open pyproject.toml" opens the merged file so the user can adjust the + * dependencies that clashed. */ private buildConflictRecoveryActions( compute: SetupCompute, - cwd: string + cwd: string, + backupPath: string ): PythonSetupErrorAction[] { return [ { @@ -696,14 +712,17 @@ export class PythonSetupEnvironmentSetup implements Disposable { if (this.readyRoots.has(cwd)) { return; } - return this.runGuarded(() => - this.runResolved( + return this.runGuarded(async () => { + // Restore before re-running; a failure throws here, before + // any attempt is recorded or the CLI spawns. + await this.deps.restoreProjectFile(cwd, backupPath); + await this.runResolved( compute, cwd, "dbconnect", "conflict_retry" - ) - ); + ); + }); }, }, { diff --git a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts index 564569c01..1844696af 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts @@ -1,4 +1,7 @@ import {expect} from "chai"; +import {mkdtemp, readFile, rm, writeFile} from "fs/promises"; +import {tmpdir} from "os"; +import path from "path"; import {commands, env, QuickPick, QuickPickItem, Uri, window} from "vscode"; import { makePythonSetupDeps, @@ -1030,6 +1033,29 @@ describe("makePythonSetupDeps openProjectFile", () => { }); }); +describe("makePythonSetupDeps restoreProjectFile", () => { + it("copies the CLI's backup over the project's pyproject.toml", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "vpex-restore-")); + try { + const pyproject = path.join(dir, "pyproject.toml"); + const backup = path.join(dir, "pyproject.toml.bak"); + // The failed run's conflicting file, and the pre-merge backup. + await writeFile(pyproject, "conflicting = true\n"); + await writeFile(backup, "original = true\n"); + + const deps = makePythonSetupDeps(makeWiring()); + await deps.restoreProjectFile(dir, backup); + + // pyproject.toml now holds the backup's (original) contents again. + expect(await readFile(pyproject, "utf8")).to.equal( + "original = true\n" + ); + } finally { + await rm(dir, {recursive: true, force: true}); + } + }); +}); + describe("makePythonSetupDeps showSuccess", () => { let originalInfo: typeof window.showInformationMessage; let originalWarn: typeof window.showWarningMessage; diff --git a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts index 218c05462..6989a654d 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts @@ -1,4 +1,5 @@ import {existsSync} from "fs"; +import {copyFile} from "fs/promises"; import path from "path"; import {commands, ProgressLocation, Uri, window} from "vscode"; import {PackageManagerDetection} from "../../language/packageManagerDetection"; @@ -285,6 +286,14 @@ export function makePythonSetupDeps( Uri.file(path.join(projectRoot, "pyproject.toml")) ); }, + restoreProjectFile: async (projectRoot: string, backupPath: string) => { + // Copy the CLI's pre-merge backup over pyproject.toml (the seam's doc + // covers why the DB Connect retry needs this). + await copyFile( + backupPath, + path.join(projectRoot, "pyproject.toml") + ); + }, // Stamp the persisted state with the completion time here (the // orchestrator supplies the env identity; the timestamp is a wiring // concern) and hand it to the injected store for drift detection. diff --git a/packages/databricks-vscode/src/python-setup/models/PythonSetupResult.ts b/packages/databricks-vscode/src/python-setup/models/PythonSetupResult.ts index 4ae3822b1..3f7766f5a 100644 --- a/packages/databricks-vscode/src/python-setup/models/PythonSetupResult.ts +++ b/packages/databricks-vscode/src/python-setup/models/PythonSetupResult.ts @@ -38,9 +38,10 @@ export type PythonSetupPhaseStatus = "ok" | "error" | "pending"; * `E_PROVISION_CONFLICT` is the distinct code the CLI emits when `uv sync` fails * specifically because the runtime's pinned dependencies conflict with the * user's own (only the Full preset pins them); a generic provision failure stays - * `E_PROVISION`. The constraints are already merged at the point of failure - * (`diskMutated: true`), which the extension's retry-as-DB-Connect recovery - * relies on. + * `E_PROVISION`. The pins are already merged into pyproject.toml at the point of + * failure (`diskMutated: true`, with the pre-merge file saved to `backupPath`), + * which the extension's retry-as-DB-Connect recovery relies on: it restores that + * backup before re-running. */ export type PythonSetupErrorCode = | "E_USAGE" diff --git a/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts b/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts index d5b8a63b8..933fc1497 100644 --- a/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts +++ b/packages/databricks-vscode/src/python-setup/utils/errorMessages.ts @@ -269,8 +269,9 @@ const DOC_LINKS: Partial> = url: UV_RESOLUTION_DOCS_URL, }, // The Full-preset flow builds its own retry/open buttons in the - // orchestrator; this is the fallback link for a conflict that somehow - // reaches the generic path (a run that already dropped the pins). + // orchestrator; this is the fallback link for a conflict that reaches the + // generic path instead — a run that already skipped constraints, or one + // with no backup to restore. E_PROVISION_CONFLICT: { label: "Resolve dependency conflicts", url: UV_RESOLUTION_DOCS_URL, From d51cfc397ec0de06619f45868cda2d28ec17d8fc Mon Sep 17 00:00:00 2001 From: "@rugpanov" Date: Wed, 9 Sep 2026 13:00:36 +0200 Subject: [PATCH 7/8] Harden the pyproject.toml restore and correct recovery docs *Why* Review of the constraint-conflict recovery surfaced two hardening gaps in the backup restore and two doc inaccuracies. The restore behavior itself (a full revert to the pre-merge backup) is kept as designed. *What* - restoreProjectFile now writes atomically: copy the backup to a temp sibling, then rename over pyproject.toml, so an interrupted/failed copy can't leave the project file truncated (temp cleaned up on error). - Reject a backupPath that resolves outside the project before copying, so a malformed CLI result can't land an arbitrary file as pyproject.toml. - Telemetry: clarify that conflict_retry counts recovery runs that started, not raw clicks (a failed-restore/coalesced/stale click records none). - README: note E_PROVISION_CONFLICT follows the same no-report policy as E_PROVISION and points at the restore-and-retry recovery. *Verification* - New wiring unit tests: atomic restore leaves no temp artifact; refuses an out-of-project backup and leaves pyproject.toml untouched; a failed copy leaves the file untouched with no temp left. Existing copy test still green. - Full unit suite green (1126 passing); tsc, ESLint, Prettier clean on changed files. Co-authored-by: Isaac --- .../src/python-setup/README.md | 3 + .../controllers/pythonSetupDeps.test.ts | 79 ++++++++++++++++++- .../controllers/pythonSetupDeps.ts | 36 +++++++-- .../src/telemetry/constants.ts | 6 +- 4 files changed, 114 insertions(+), 10 deletions(-) diff --git a/packages/databricks-vscode/src/python-setup/README.md b/packages/databricks-vscode/src/python-setup/README.md index c24b48e72..3f349c10b 100644 --- a/packages/databricks-vscode/src/python-setup/README.md +++ b/packages/databricks-vscode/src/python-setup/README.md @@ -19,6 +19,9 @@ the mapped message alone (see `reportSetupIssue.ts` for the closed routing list) such a conflict is usually the user's own declared dependencies. When the published constraints are what conflict, that genuine case is served by a soft, conditional pointer in the output log instead (see `formatSetupFailureDetail`). +`E_PROVISION_CONFLICT` (the CLI's distinct pins-vs-local code) follows the same +policy; the Full-preset flow additionally offers a restore-and-retry recovery +(see `buildConflictRecoveryActions`). **Privacy posture.** The issue body carries build metadata (error code, phase, env key, package manager, extension/CLI versions, OS) plus the CLI's stderr. The diff --git a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts index 1844696af..79e316e5c 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.test.ts @@ -1,5 +1,5 @@ import {expect} from "chai"; -import {mkdtemp, readFile, rm, writeFile} from "fs/promises"; +import {mkdtemp, readdir, readFile, rm, writeFile} from "fs/promises"; import {tmpdir} from "os"; import path from "path"; import {commands, env, QuickPick, QuickPickItem, Uri, window} from "vscode"; @@ -1054,6 +1054,83 @@ describe("makePythonSetupDeps restoreProjectFile", () => { await rm(dir, {recursive: true, force: true}); } }); + + it("leaves no temp artifact behind after a successful restore", async () => { + // The restore writes atomically (copy to a temp sibling, then rename), + // so no stray *.tmp file may survive a successful run. + const dir = await mkdtemp(path.join(tmpdir(), "vpex-restore-")); + try { + const backup = path.join(dir, "pyproject.toml.bak"); + await writeFile(path.join(dir, "pyproject.toml"), "conflicting\n"); + await writeFile(backup, "original\n"); + + const deps = makePythonSetupDeps(makeWiring()); + await deps.restoreProjectFile(dir, backup); + + const entries = await readdir(dir); + expect(entries.sort()).to.deep.equal([ + "pyproject.toml", + "pyproject.toml.bak", + ]); + } finally { + await rm(dir, {recursive: true, force: true}); + } + }); + + it("refuses to restore from a backup outside the project (and leaves the file untouched)", async () => { + // backupPath comes from the CLI result; a path outside the project must + // never be copied over pyproject.toml. + const project = await mkdtemp(path.join(tmpdir(), "vpex-proj-")); + const outside = await mkdtemp(path.join(tmpdir(), "vpex-out-")); + try { + const pyproject = path.join(project, "pyproject.toml"); + await writeFile(pyproject, "conflicting\n"); + const foreign = path.join(outside, "secrets.bak"); + await writeFile(foreign, "SHOULD NOT LAND\n"); + + const deps = makePythonSetupDeps(makeWiring()); + let threw = false; + try { + await deps.restoreProjectFile(project, foreign); + } catch { + threw = true; + } + + expect(threw).to.equal(true); + // pyproject.toml is untouched; the foreign file never lands. + expect(await readFile(pyproject, "utf8")).to.equal("conflicting\n"); + } finally { + await rm(project, {recursive: true, force: true}); + await rm(outside, {recursive: true, force: true}); + } + }); + + it("leaves pyproject.toml untouched and cleans up when the copy fails", async () => { + // A missing backup (in-project path, so it passes the guard) makes the + // copy fail: the destination must be untouched and no temp left behind. + const dir = await mkdtemp(path.join(tmpdir(), "vpex-restore-")); + try { + const pyproject = path.join(dir, "pyproject.toml"); + await writeFile(pyproject, "conflicting\n"); + const missing = path.join(dir, "pyproject.toml.bak"); // never created + + const deps = makePythonSetupDeps(makeWiring()); + let threw = false; + try { + await deps.restoreProjectFile(dir, missing); + } catch { + threw = true; + } + + expect(threw).to.equal(true); + expect(await readFile(pyproject, "utf8")).to.equal("conflicting\n"); + // No *.tmp artifact survived the failed copy. + const entries = await readdir(dir); + expect(entries).to.deep.equal(["pyproject.toml"]); + } finally { + await rm(dir, {recursive: true, force: true}); + } + }); }); describe("makePythonSetupDeps showSuccess", () => { diff --git a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts index 6989a654d..5e41e996a 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/pythonSetupDeps.ts @@ -1,5 +1,6 @@ +import {randomBytes} from "crypto"; import {existsSync} from "fs"; -import {copyFile} from "fs/promises"; +import {copyFile, rename, rm} from "fs/promises"; import path from "path"; import {commands, ProgressLocation, Uri, window} from "vscode"; import {PackageManagerDetection} from "../../language/packageManagerDetection"; @@ -287,12 +288,33 @@ export function makePythonSetupDeps( ); }, restoreProjectFile: async (projectRoot: string, backupPath: string) => { - // Copy the CLI's pre-merge backup over pyproject.toml (the seam's doc - // covers why the DB Connect retry needs this). - await copyFile( - backupPath, - path.join(projectRoot, "pyproject.toml") - ); + // Restore the CLI's pre-merge backup over pyproject.toml (the seam's + // doc covers why the DB Connect retry needs this). + const root = path.resolve(projectRoot); + // backupPath comes from the CLI result: refuse anything resolving + // outside the project so a malformed/unexpected path can't copy an + // arbitrary file over pyproject.toml. Lexical containment (path is + // from the trusted local CLI, so symlink canonicalization is not + // warranted). + const rel = path.relative(root, path.resolve(backupPath)); + if (rel === "" || rel.startsWith("..") || path.isAbsolute(rel)) { + throw new Error( + `Refusing to restore pyproject.toml from a backup outside the project: ${backupPath}` + ); + } + // Write atomically: copy to a temp sibling on the same filesystem, + // then rename over pyproject.toml, so an interrupted or failed copy + // can never leave the project file truncated. Clean the temp up if + // either step throws (rename consumes it on success). + const dest = path.join(root, "pyproject.toml"); + const tmp = `${dest}.${randomBytes(6).toString("hex")}.tmp`; + try { + await copyFile(backupPath, tmp); + await rename(tmp, dest); + } catch (e) { + await rm(tmp, {force: true}); + throw e; + } }, // Stamp the persisted state with the completion time here (the // orchestrator supplies the env identity; the timestamp is a wiring diff --git a/packages/databricks-vscode/src/telemetry/constants.ts b/packages/databricks-vscode/src/telemetry/constants.ts index c14b9a929..e36223804 100644 --- a/packages/databricks-vscode/src/telemetry/constants.ts +++ b/packages/databricks-vscode/src/telemetry/constants.ts @@ -561,8 +561,10 @@ export class EventTypes { "environment already provisioned this session, e.g. via the ready row's Re-run " + "button), or conflict_retry (a Full-preset run hit a cluster-vs-local dependency " + "conflict and the user clicked 'Retry DB Connect setup', which re-runs with " + - "--no-constraints). conflict_retry counts conflict-recovery clicks; pair it with " + - "the matching result's outcome for the recovery success rate. Session-scoped: a " + + "--no-constraints). conflict_retry counts conflict-recovery runs that started " + + "(a click whose pre-retry pyproject.toml restore fails, coalesces, or no-ops " + + "records none); pair it with the matching result's outcome for the recovery " + + "success rate. Session-scoped: a " + "run after a window reload reads as initial again", }, packageManager: { From b9a33038bba5491b18d897cbdaec2376cc4a6794 Mon Sep 17 00:00:00 2001 From: "@rugpanov" Date: Wed, 9 Sep 2026 14:07:22 +0200 Subject: [PATCH 8/8] Guard the conflict recovery against an empty backupPath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* A second review round flagged that the recoverable-conflict gate keyed on `!== undefined`, so a (contract-forbidden) empty backupPath would still offer a "Retry DB Connect setup" button that can only throw — the restore rejects an empty/at-root path — while hiding "Show Logs". Truthiness is strictly safer. *What* - Gate the recovery on a truthy backupPath, so an empty string falls through to the ordinary doc-link handling (like a missing backup) instead of a broken Retry toast. - Make PythonSetupRunTrigger a type-only import (CODE_CONVENTIONS §7, new code). *Verification* - New unit test: an E_PROVISION_CONFLICT result with backupPath "" gets the generic "Resolve dependency conflicts" doc link and keeps Show Logs. - Full unit suite green (1127 passing); tsc, ESLint, Prettier clean on changed files. Co-authored-by: Isaac --- .../PythonSetupEnvironmentSetup.test.ts | 33 +++++++++++++++++++ .../PythonSetupEnvironmentSetup.ts | 8 +++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts index 83cba4d1a..f7913796a 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.test.ts @@ -2201,4 +2201,37 @@ describe("PythonSetupEnvironmentSetup constraint-conflict recovery", () => { // Not a self-service toast without the Retry button, so Show Logs stays. expect(seenOptions[0]).to.equal(undefined); }); + + it("does not offer Retry when backupPath is an empty string", async () => { + // The CLI marks backupPath omitempty, so "" shouldn't occur — but if it + // did, offering Retry would hand the user a button that can only throw + // (restore rejects an empty path) while hiding Show Logs. Treat empty as + // "no backup": fall through to the ordinary doc-link handling. + const emptyBackup: PythonSetupResult = { + ...conflictResult(), + backupPath: "", + }; + const shown: {actions?: PythonSetupErrorAction[]}[] = []; + const seenOptions: Array<{includeShowLogs?: boolean} | undefined> = []; + const setup = new PythonSetupEnvironmentSetup( + makeDeps({ + cli: makeCli({resolve: emptyBackup}), + pickSetupPreset: async () => "full", + showError: async (_message, _detail, actions, options) => { + shown.push({actions}); + seenOptions.push(options); + }, + }) + ); + + await setup.setup(); + + expect(shown[0].actions).to.deep.equal([ + { + label: "Resolve dependency conflicts", + url: "https://docs.astral.sh/uv/concepts/resolution/", + }, + ]); + expect(seenOptions[0]).to.equal(undefined); + }); }); diff --git a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts index 4455c2fb0..cfe2f6063 100644 --- a/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts +++ b/packages/databricks-vscode/src/python-setup/controllers/PythonSetupEnvironmentSetup.ts @@ -34,7 +34,7 @@ import { PythonSetupAttempt, PythonSetupResultReporter, } from "../../telemetry/pythonSetupExtensions"; -import {PythonSetupRunTrigger} from "../../telemetry/constants"; +import type {PythonSetupRunTrigger} from "../../telemetry/constants"; import {PrimaryManager} from "../../language/packageManagerDetection"; import { isUvSetupSuitable, @@ -534,7 +534,11 @@ export class PythonSetupEnvironmentSetup implements Disposable { // (nothing to restore) or would loop (a run that already skipped pins). const conflictBackupPath = result.error?.code === "E_PROVISION_CONFLICT" && - !invocation.skipConstraints + !invocation.skipConstraints && + // Truthiness, not just `!== undefined`: a (contract-forbidden) + // empty backupPath has nothing to restore, so it must fall + // through rather than offer a Retry that could only throw. + result.backupPath ? result.backupPath : undefined; const recoverableConflict = conflictBackupPath !== undefined;