direct: Fix RemoteAlreadySet skipping input-only fields

.nextchanges/bundles/remote-already-set-input-only.md

@@ -0,0 +1 @@
+direct: A local change to an input-only field (one the API accepts on write but never returns on read, e.g. pipelines' `run_as` or external locations' `skip_validation`) is no longer silently skipped when the new value coincidentally matches the field's fabricated remote value. Previously such a change could hit the `remote_already_set` shortcut and be dropped from the plan.

acceptance/bundle/migrate/runas/out.pipelines_get.json

@@ -4,6 +4,9 @@
   "last_modified": [UNIX_TIME_MILLIS],
   "name": "DABs Revenue Pipeline",
   "pipeline_id": "[UUID]",
+  "run_as": {
+    "service_principal_name": "[UUID]"
+  },
   "run_as_user_name": "[USERNAME]",
   "spec": {
     "catalog": "main",

acceptance/bundle/migrate/runas/out.plan.json

@@ -27,22 +27,13 @@
         ],
         "name": "DABs Revenue Pipeline",
         "pipeline_id": "[UUID]",
+        "run_as": {
+          "service_principal_name": "[UUID]"
+        },
         "run_as_user_name": "[USERNAME]",
         "serverless": true,
         "state": "IDLE",
         "target": "team_eng_deco"
-      },
-      "changes": {
-        "run_as": {
-          "action": "skip",
-          "reason": "input_only",
-          "old": {
-            "service_principal_name": "[UUID]"
-          },
-          "new": {
-            "service_principal_name": "[UUID]"
-          }
-        }
       }
     },
     "resources.pipelines.foo.permissions": {

acceptance/bundle/refschema/out.fields.txt

@@ -279,7 +279,7 @@ resources.catalogs.*.grants[*]	catalog.PrivilegeAssignment	ALL
 resources.catalogs.*.grants[*].principal	string	ALL
 resources.catalogs.*.grants[*].privileges	[]catalog.Privilege	ALL
 resources.catalogs.*.grants[*].privileges[*]	catalog.Privilege	ALL
-resources.clusters.*.apply_policy_default_values	bool	INPUT	STATE
+resources.clusters.*.apply_policy_default_values	bool	ALL
 resources.clusters.*.autoscale	*compute.AutoScale	ALL
 resources.clusters.*.autoscale.max_workers	int	ALL
 resources.clusters.*.autoscale.min_workers	int	ALL

acceptance/bundle/resources/pipelines/remote_matches_config/databricks.yml

@@ -0,0 +1,13 @@
+bundle:
+  name: test-bundle
+
+resources:
+  pipelines:
+    my_pipeline:
+      name: "Test Pipeline run_as guard"
+      serverless: true
+      libraries:
+        - notebook:
+            path: /Users/{{workspace_user_name}}/test_notebook
+      run_as:
+        user_name: original@example.test

acceptance/bundle/resources/pipelines/remote_matches_config/out.plan.direct.json

@@ -0,0 +1,74 @@
+{
+  "plan_version": 2,
+  "cli_version": "[CLI_VERSION]",
+  "lineage": "[UUID]",
+  "serial": 1,
+  "plan": {
+    "resources.pipelines.my_pipeline": {
+      "action": "update",
+      "new_state": {
+        "value": {
+          "channel": "CURRENT",
+          "deployment": {
+            "kind": "BUNDLE",
+            "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
+          },
+          "edition": "ADVANCED",
+          "libraries": [
+            {
+              "notebook": {
+                "path": "/Users/{{workspace_user_name}}/test_notebook"
+              }
+            }
+          ],
+          "name": "Test Pipeline run_as guard",
+          "run_as": {
+            "user_name": "changed@example.test"
+          },
+          "serverless": true
+        }
+      },
+      "remote_state": {
+        "channel": "CURRENT",
+        "creator_user_name": "[USERNAME]",
+        "deployment": {
+          "kind": "BUNDLE",
+          "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
+        },
+        "edition": "ADVANCED",
+        "effective_publishing_mode": "DEFAULT_PUBLISHING_MODE",
+        "id": "[MY_PIPELINE_ID]",
+        "last_modified": [UNIX_TIME_MILLIS],
+        "libraries": [
+          {
+            "notebook": {
+              "path": "/Users/{{workspace_user_name}}/test_notebook"
+            }
+          }
+        ],
+        "name": "Test Pipeline run_as guard",
+        "pipeline_id": "[MY_PIPELINE_ID]",
+        "run_as": {
+          "user_name": "changed@example.test"
+        },
+        "run_as_user_name": "[USERNAME]",
+        "serverless": true,
+        "state": "IDLE",
+        "storage": "dbfs:/pipelines/[MY_PIPELINE_ID]"
+      },
+      "changes": {
+        "run_as.user_name": {
+          "action": "update",
+          "old": "original@example.test",
+          "new": "changed@example.test",
+          "remote": "changed@example.test"
+        },
+        "storage": {
+          "action": "skip",
+          "reason": "backend_default",
+          "remote": "dbfs:/pipelines/[MY_PIPELINE_ID]"
+        }
+      }
+    }
+  }
+}

acceptance/bundle/resources/pipelines/remote_matches_config/out.plan.terraform.json

@@ -0,0 +1,8 @@
+{
+  "cli_version": "[CLI_VERSION]",
+  "plan": {
+    "resources.pipelines.my_pipeline": {
+      "action": "update"
+    }
+  }
+}

acceptance/bundle/resources/pipelines/remote_matches_config/output.txt

@@ -0,0 +1,13 @@
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+=== Make local change: run_as user -> changed@example.test
+=== Force remote run_as to match the new config value
+>>> [CLI] bundle plan
+update pipelines.my_pipeline
+
+Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged

acceptance/bundle/resources/pipelines/remote_matches_config/script

@@ -0,0 +1,20 @@
+# run_as is declared ignore_remote_changes: input_only. This test forces the situation where
+# a genuine local change (state has run_as A, config changes it to B) coincides with remote
+# already reading back B. Before the RemoteAlreadySet input-only fix, remote == new wrongly
+# skipped the change (reason: remote_already_set); it must instead classify as an update.
+
+trace $CLI bundle deploy
+pipeline_id="$(read_id.py my_pipeline)"
+
+title "Make local change: run_as user -> changed@example.test"
+update_file.py databricks.yml "original@example.test" "changed@example.test"
+
+title "Force remote run_as to match the new config value"
+edit_resource.py pipelines "$pipeline_id" <<EOF
+r["run_as"] = {"user_name": "changed@example.test"}
+EOF
+
+trace $CLI bundle plan
+$CLI bundle plan -o json > out.plan.$DATABRICKS_BUNDLE_ENGINE.json
+
+rm out.requests.txt

acceptance/bundle/resources/pipelines/remote_matches_config/test.toml

@@ -0,0 +1,2 @@
+RecordRequests = true
+Ignore = [".databricks"]

bundle/direct/bundle_plan.go

@@ -385,7 +385,13 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change
 			return err
 		}
 
-		if structdiff.IsEqual(ch.Remote, ch.New) {
+		// RemoteAlreadySet only holds when ch.Remote is a real remote value we can compare
+		// against ch.New. Skip it for fields whose remote value is fabricated and thus
+		// meaningless: declared ignore_remote_changes (present in RemoteType but read back
+		// backend-managed/input-only), or absent from RemoteType (a guaranteed-nil
+		// placeholder, since RemapState is a dumb copy). Otherwise a coincidental
+		// new == remote (both nil, say) wrongly skips a real local change.
+		if structdiff.IsEqual(ch.Remote, ch.New) && !ignoreRemoteChanges(cfg, generatedCfg, path) && !isFieldMissingInRemote(adapter, path) {
 			ch.Action = deployplan.Skip
 			ch.Reason = deployplan.ReasonRemoteAlreadySet
 		} else if allEmpty(ch.Old, ch.New, ch.Remote) {
@@ -455,8 +461,28 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change
 	return nil
 }
 
+func ignoreRemoteChanges(cfg1, cfg2 *dresources.ResourceLifecycleConfig, path *structpath.PathNode) bool {
+	if _, ok := findMatchingRule(path, cfg1.IgnoreRemoteChanges); ok {
+		return true
+	}
+
+	if _, ok := findMatchingRule(path, cfg2.IgnoreRemoteChanges); ok {
+		return true
+	}
+
+	return false
+}
+
 // isFieldMissingInRemote reports whether path exists in StateType but is absent from RemoteType.
 // Such fields are accepted by the API on write but not returned by GET.
+//
+// Because RemapState is a dumb subset copy (see the "RemapState is a dumb copy" section in
+// dresources/README.md), a field absent from RemoteType is always nil/zero in the remapped
+// remote state the planner compares against (ch.Remote): there is nothing for RemapState to
+// copy it from, so it can never appear. That guarantee is what makes this a sound signal that
+// the field's remote value is meaningless. Fields the API returns under a different path are
+// added to RemoteType and populated in DoRead, so they are NOT missing here and keep their
+// real remote value.
 func isFieldMissingInRemote(adapter *dresources.Adapter, path *structpath.PathNode) bool {
 	if structaccess.ValidatePath(adapter.StateType(), path) != nil {
 		return false

bundle/direct/bundle_plan_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/dyn/yamlloader"
 	"github.com/databricks/cli/libs/structs/structpath"
+	"github.com/databricks/databricks-sdk-go/service/pipelines"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -168,6 +169,102 @@ func TestShouldSkipBackendDefault_ManagedPropertiesOnly(t *testing.T) {
 	}
 }
 
+// A field present in StateType but absent from RemoteType (input-only, e.g. external
+// locations' skip_validation) reads back nil, so a coincidental new == remote must NOT
+// trip RemoteAlreadySet — that would skip a real local change. A field present in RemoteType
+// (jobs' max_concurrent_runs) is not affected.
+func TestIsFieldMissingInRemote(t *testing.T) {
+	adapters, err := dresources.InitAll(nil)
+	require.NoError(t, err)
+
+	tests := []struct {
+		resource string
+		path     string
+		expected bool
+	}{
+		{"external_locations", "skip_validation", true},
+		{"jobs", "max_concurrent_runs", false},
+		{"jobs", "name", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.resource+"."+tt.path, func(t *testing.T) {
+			adapter, ok := adapters[tt.resource]
+			require.True(t, ok)
+			path, err := structpath.ParsePath(tt.path)
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, isFieldMissingInRemote(adapter, path))
+		})
+	}
+}
+
+// TestRemoteAlreadySetGuards drives addPerFieldActions directly to pin the two guards that
+// keep the RemoteAlreadySet shortcut from swallowing a real local change when the remote
+// value is fabricated (read back as a fixed nil/zero rather than the true remote value):
+//   - a field declared ignore_remote_changes (present in RemoteType, e.g. pipelines' run_as)
+//   - a field absent from RemoteType (missing_in_remote, e.g. external_locations' skip_validation)
+//
+// In both cases a genuine local change (old != new) that coincidentally makes new == remote
+// must still classify as Update, not Skip/remote_already_set.
+func TestRemoteAlreadySetGuards(t *testing.T) {
+	adapters, err := dresources.InitAll(nil)
+	require.NoError(t, err)
+
+	runAs := &pipelines.RunAs{UserName: "someone@example.test"}
+
+	tests := []struct {
+		name           string
+		resource       string
+		field          string
+		ch             *deployplan.ChangeDesc
+		expectedAction deployplan.ActionType
+		expectedReason string
+	}{
+		{
+			// ignore_remote_changes guard: run_as read back nil, config changed A -> B.
+			name:           "ignore_remote_changes real local change",
+			resource:       "pipelines",
+			field:          "run_as",
+			ch:             &deployplan.ChangeDesc{Old: runAs, New: nil, Remote: nil},
+			expectedAction: deployplan.Update,
+		},
+		{
+			// missing_in_remote guard: skip_validation absent from RemoteType (remote nil),
+			// config unset the field it previously had.
+			name:           "missing_in_remote real local change",
+			resource:       "external_locations",
+			field:          "skip_validation",
+			ch:             &deployplan.ChangeDesc{Old: true, New: nil, Remote: nil},
+			expectedAction: deployplan.Update,
+		},
+		{
+			// Control: a real remote value that matches config is correctly skipped.
+			name:           "genuine remote_already_set is skipped",
+			resource:       "jobs",
+			field:          "max_concurrent_runs",
+			ch:             &deployplan.ChangeDesc{Old: 1, New: 2, Remote: 2},
+			expectedAction: deployplan.Skip,
+			expectedReason: deployplan.ReasonRemoteAlreadySet,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			adapter, ok := adapters[tt.resource]
+			require.True(t, ok)
+			changes := deployplan.Changes{tt.field: tt.ch}
+			err := addPerFieldActions(t.Context(), adapter, changes, nil)
+			require.NoError(t, err)
+			assert.Equal(t, tt.expectedAction, tt.ch.Action)
+			if tt.expectedReason != "" {
+				assert.Equal(t, tt.expectedReason, tt.ch.Reason)
+			} else {
+				assert.NotEqual(t, deployplan.ReasonRemoteAlreadySet, tt.ch.Reason, "a real local change must not be skipped as remote_already_set")
+			}
+		})
+	}
+}
+
 // Map drift handling synthesizes child paths to match against rules. structdiff
 // always emits map keys in bracket notation, so synthetic child paths must too;
 // otherwise rules wouldn't match for identifier-like keys.

bundle/direct/dresources/README.md

@@ -47,17 +47,35 @@ If the API may return a slice's elements in a different order between calls (e.g
 The state struct is serialized to JSON and persisted between deploys. Backward incompatible changes will result in a drift, which depending
 on field behaviour might result in recreate. See dstate/migrate.go on how to handle state migration.
 
-## RemapState and missing remote fields
+## RemapState is a dumb copy; DoRead owns all remapping
 
-Do not populate a field in `RemapState` by mapping it from a differently-named field in `RemoteType`. When a field is absent from `RemoteType` the engine automatically suppresses remote drift for it (reason: `missing_in_remote`), which means any value `RemapState` sets there is invisible to drift detection — real remote changes go undetected.
+`RemapState` converts `RemoteType` to `StateType` only because `StateType` is typically a
+subset of `RemoteType`. It must be a field-by-field copy (or no-op), never a place for
+logic. In particular, do not remap a differently-named field there (e.g. `state.x = remote.status.x`).
+Any remapping the API requires belongs in `DoRead`: add `x` directly to `RemoteType` and
+populate it from `status.x` inside `DoRead`.
 
-The correct pattern is:
+This is not a style preference — the plan classifier depends on it. Because `RemapState` is a
+dumb copy, a field absent from `RemoteType` is always nil/zero in the remapped remote state the
+planner compares against: there is nothing to copy it from. So "path present in `StateType` but
+absent from `RemoteType`" reliably means "field the API accepts on write but never echoes on
+read," and its remote value is guaranteed meaningless (see `isFieldMissingInRemote` /
+`missing_in_remote` in `bundle_plan.go`).
+
+A `RemapState` that instead synthesizes `state.x = remote.status.x` breaks this: `x` is absent
+from `RemoteType`, so the classifier treats it as input-only and skips it (`missing_in_remote`),
+yet the field actually has a real remote value under `status.x`. Genuine remote drift is then
+silently suppressed. The fix is to make the field genuinely present in `RemoteType`, not to
+smuggle it in via `RemapState`.
+
+So when a field comes back under a different path than `StateType` uses:
 
 1. Add the field to `RemoteType` (the struct returned by `DoRead`).
 2. Populate it in `DoRead` by mapping from whatever the API returns under the other name.
-3. Keep `RemapState` trivial (a direct struct copy or no-op).
+3. Keep `RemapState` a trivial subset copy.
 
-This makes the field present in `InputType`, `StateType`, and `RemoteType`, so it participates in normal drift detection and is no longer subject to the `missing_in_remote` suppression.
+This makes the field present in `InputType`, `StateType`, and `RemoteType`, so it participates
+in normal drift detection and is no longer subject to the `missing_in_remote` suppression.
 
 ## OverrideChangeDesc