fix(slos): address PR #207 review feedback

- delete: add IsAlreadyDeleted idempotency branch so `slos delete --force`
  on an already-deleted SLO exits 0 (matches the 8 other delete commands,
  CHANGELOG 1.16.2 / #217); cover with an integration test.
- update: StripSLOServerFields before the dry-run diff and PUT so
  `get -o yaml | update -f` no longer sends server-managed labels/timestamps
  or shows a spurious diff (mirrors views/update.go).
- asset/ImportSLO: mirror ImportTeam upsert-key selection — prefer
  dash0.com/id, fall back to dash0.com/origin (read before strip), POST only
  when neither is present, so an origin-only document upserts via PUT instead
  of duplicating on every apply (#227 shape).
- list: replace interface{} with any on the column closures (matches
  views/list.go).
- integration tests: assert the update wire body strips dash0.com/version,
  dash0.com/origin, and created-at; add create/update --dry-run tests; add a
  second element to list_success.json and exercise --limit truncation.
- roundtrip: add test_apply_slo_idempotency.sh (double-apply, no duplicate /
  stable id) and register it in run_all.sh.
- docs: add teams to the command-group enumerations in commands.md and
  cli-naming-conventions.md and adjust the count wording.

Author: nicolastakashi

docs/cli-naming-conventions.md

@@ -5,12 +5,12 @@ The reason for this is that the word "resource" is overloaded in OpenTelemetry,
 Use the word "asset" consistently where appropriate.
 
 ## Top-level Asset Commands
-- Use **plural form**: `dashboards`, `views`, `check-rules`, `synthetic-checks`, `slos`, `recording-rules`, `notification-channels`, `spam-filters`
+- Use **plural form**: `dashboards`, `views`, `check-rules`, `synthetic-checks`, `slos`, `recording-rules`, `notification-channels`, `spam-filters`, `teams`
 - Use **kebab-case** for multi-word names: `check-rules`, `synthetic-checks`, `recording-rules`, `notification-channels`, `spam-filters`
 - Group related functionality: `config profiles` for profile management
 
 ## Standard CRUD Subcommands for Assets
-All asset commands (`dashboards`, `check-rules`, `views`, `synthetic-checks`, `slos`, `recording-rules`, `notification-channels`, `spam-filters`) use these subcommands:
+All asset commands (`dashboards`, `check-rules`, `views`, `synthetic-checks`, `slos`, `recording-rules`, `notification-channels`, `spam-filters`), along with the organization-level `teams` command, use these subcommands:
 
 | Subcommand | Alias    | Description                          |
 |------------|----------|--------------------------------------|

docs/commands.md

@@ -494,8 +494,9 @@ Profile:    prod    (from DASH0_PROFILE environment variable)
 Asset CRUD commands create, list, get, update, and delete Dash0 assets.
 Dash0 calls dashboards, views, synthetic checks, and check rules "assets" (not "resources", which is an overloaded term in OpenTelemetry).
 
-All eight asset types (`dashboards`, `check-rules`, `synthetic-checks`, `slos`, `views`, `recording-rules`, `notification-channels`, `spam-filters`) share the same CRUD subcommands.
-The examples below use `dashboards`, but the same patterns apply to every asset type.
+All nine of these command groups (`dashboards`, `check-rules`, `synthetic-checks`, `slos`, `views`, `recording-rules`, `notification-channels`, `spam-filters`, and the organization-level `teams`) share the same CRUD subcommands.
+The first eight are dataset-scoped assets; `teams` is organization-level (no `--dataset`, no `apply`) but shares the same subcommand shape.
+The examples below use `dashboards`, but the same patterns apply to every command group.
 
 ### `list`
 

internal/asset/slo.go

@@ -6,19 +6,46 @@ import (
 	dash0api "github.com/dash0hq/dash0-api-client-go"
 )
 
-// ImportSLO creates or updates an SLO via the standard CRUD APIs.
-// When the input has a user-defined ID, UPDATE is always used — PUT has
-// create-or-replace semantics, so this is idempotent regardless of whether the
-// SLO already exists.
-// When the input has no ID, CREATE is used and the server assigns an ID.
+// ImportSLO creates or upserts an SLO via the standard CRUD APIs.
+//
+// Upsert key selection mirrors ImportTeam. The SLO API routes GET, PUT, and
+// DELETE by origin-or-id (unlike dashboards, views, check rules, and synthetic
+// checks, where the server treats dash0.com/origin as provenance metadata and
+// it must not be used as an upsert key). So:
+//
+//   - Prefer the stable dash0.com/id when present.
+//   - Fall back to dash0.com/origin when there is no id. An origin-only
+//     document (e.g. a UI CR download) must upsert via PUT on that origin
+//     rather than POST a fresh duplicate on every apply — the #227 team bug.
+//   - Only POST (server assigns id and origin) when neither is present.
+//
+// PUT is create-or-replace, so upserting on either key is idempotent across
+// repeated applies. The origin label is captured before StripSLOServerFields
+// runs because that helper clears dash0.com/origin along with the other
+// server-managed labels.
 func ImportSLO(ctx context.Context, apiClient dash0api.Client, slo *dash0api.SloDefinition, dataset *string) (ImportResult, error) {
+	// Capture identifiers before stripping — StripSLOServerFields clears the
+	// dash0.com/origin label, so origin-based routing must observe the input
+	// first.
+	origin := ""
+	if slo.Metadata.Labels != nil && slo.Metadata.Labels.Dash0Comorigin != nil {
+		origin = *slo.Metadata.Labels.Dash0Comorigin
+	}
+	id := dash0api.GetSLOID(slo)
 	dash0api.StripSLOServerFields(slo)
 
+	var upsertKey string
+	switch {
+	case id != "":
+		upsertKey = id
+	case origin != "":
+		upsertKey = origin
+	}
+
 	action := ActionCreated
 	var before any
-	id := dash0api.GetSLOID(slo)
-	if id != "" {
-		existing, err := apiClient.GetSLO(ctx, id, dataset)
+	if upsertKey != "" {
+		existing, err := apiClient.GetSLO(ctx, upsertKey, dataset)
 		if err == nil {
 			action = ActionUpdated
 			before = existing
@@ -27,8 +54,8 @@ func ImportSLO(ctx context.Context, apiClient dash0api.Client, slo *dash0api.Slo
 
 	var result *dash0api.SloDefinition
 	var err error
-	if id != "" {
-		result, err = apiClient.UpdateSLO(ctx, id, slo, dataset)
+	if upsertKey != "" {
+		result, err = apiClient.UpdateSLO(ctx, upsertKey, slo, dataset)
 	} else {
 		result, err = apiClient.CreateSLO(ctx, slo, dataset)
 	}

internal/slos/delete.go

@@ -55,10 +55,11 @@ func runDelete(ctx context.Context, id string, flags *asset.DeleteFlags) error {
 
 	err = apiClient.DeleteSLO(ctx, id, client.ResolveDataset(ctx, flags.Dataset))
 	if err != nil {
-		return client.HandleAPIError(err, client.ErrorContext{
-			AssetType: "SLO",
-			AssetID:   id,
-		})
+		ectx := client.ErrorContext{AssetType: "SLO", AssetID: id}
+		if client.IsAlreadyDeleted(err, flags.Force, ectx) {
+			return nil
+		}
+		return client.HandleAPIError(err, ectx)
 	}
 
 	fmt.Printf("SLO %q deleted\n", id)

internal/slos/integration_test.go

@@ -21,6 +21,7 @@ const (
 	apiPathSLOs          = "/api/slos"
 	testAuthToken        = "auth_test_token"
 	testSLOID            = "00000000-0000-0000-0000-000000000001"
+	testSLOID2           = "00000000-0000-0000-0000-000000000002"
 	fixtureListSuccess   = "slos/list_success.json"
 	fixtureListEmpty     = "slos/list_empty.json"
 	fixtureGetSuccess    = "slos/get_success.json"
@@ -43,7 +44,8 @@ func TestListSLOs_JSONFormat(t *testing.T) {
 	})
 
 	cmd := NewSlosCmd()
-	cmd.SetArgs([]string{"list", "--api-url", server.URL, "--auth-token", testAuthToken, "-o", "json", "--limit", "2"})
+	// The fixture carries two SLOs; --limit 1 must truncate to the first.
+	cmd.SetArgs([]string{"list", "--api-url", server.URL, "--auth-token", testAuthToken, "-o", "json", "--limit", "1"})
 
 	var err error
 	output := testutil.CaptureStdout(t, func() {
@@ -55,6 +57,9 @@ func TestListSLOs_JSONFormat(t *testing.T) {
 	assert.Contains(t, output, `"apiVersion": "openslo.com/v1"`)
 	assert.Contains(t, output, `"metadata"`)
 	assert.Contains(t, output, `"spec"`)
+	// --limit 1 truncates: only the first SLO is emitted, not the second.
+	assert.Contains(t, output, testSLOID)
+	assert.NotContains(t, output, testSLOID2)
 }
 
 func TestListSLOs_YAMLFormat(t *testing.T) {
@@ -357,6 +362,141 @@ func TestDeleteSLO_Success(t *testing.T) {
 	assert.Contains(t, output, "deleted")
 }
 
+// TestDeleteSLO_ForceIdempotentOn404 asserts that `delete --force` treats an
+// already-deleted SLO as success (exit 0) and prints an "already deleted" line
+// to stderr. Regression coverage matching CHANGELOG 1.16.2 / issue #217.
+func TestDeleteSLO_ForceIdempotentOn404(t *testing.T) {
+	testutil.SetupTestEnv(t)
+
+	server := testutil.NewMockServer(t, testutil.FixturesDir())
+	server.OnPattern(http.MethodDelete, sloIDPattern, testutil.MockResponse{
+		StatusCode: http.StatusNotFound,
+		BodyFile:   fixtureNotFound,
+		Validator:  testutil.RequireHeaders,
+	})
+
+	cmd := NewSlosCmd()
+	cmd.SetArgs([]string{"delete", testSLOID, "--force", "--api-url", server.URL, "--auth-token", testAuthToken})
+
+	var err error
+	stderr := testutil.CaptureStderr(t, func() {
+		err = cmd.Execute()
+	})
+
+	require.NoError(t, err, "expected exit 0 with --force on 404")
+	assert.Contains(t, stderr, "was already deleted")
+	assert.Contains(t, stderr, testSLOID)
+}
+
+// TestUpdateSLO_StripsServerFields pins the strip contract: an input that
+// still carries server-managed fields (as an exported `slos get -o yaml`
+// would) must not send dash0.com/version, dash0.com/origin, or the
+// created-at/updated-at timestamps back on the wire.
+func TestUpdateSLO_StripsServerFields(t *testing.T) {
+	testutil.SetupTestEnv(t)
+
+	server := testutil.NewMockServer(t, testutil.FixturesDir())
+	server.OnPattern(http.MethodGet, sloIDPattern, testutil.MockResponse{
+		StatusCode: http.StatusOK,
+		BodyFile:   fixtureGetSuccess,
+		Validator:  testutil.RequireHeaders,
+	})
+	server.OnPattern(http.MethodPut, sloIDPattern, testutil.MockResponse{
+		StatusCode: http.StatusOK,
+		BodyFile:   fixtureUpdateSuccess,
+		Validator:  testutil.RequireHeaders,
+	})
+
+	tmpDir := t.TempDir()
+	yamlFile := filepath.Join(tmpDir, "slo.yaml")
+	require.NoError(t, os.WriteFile(yamlFile, []byte(sloUpdateWithServerFieldsYAML), 0644))
+
+	cmd := NewSlosCmd()
+	cmd.SetArgs([]string{"update", "-f", yamlFile, "--api-url", server.URL, "--auth-token", testAuthToken})
+
+	var err error
+	testutil.CaptureStdout(t, func() {
+		err = cmd.Execute()
+	})
+	require.NoError(t, err)
+
+	req := server.LastRequest()
+	require.NotNil(t, req)
+	assert.Equal(t, http.MethodPut, req.Method)
+
+	// Decode the wire body and assert the server-managed fields are absent.
+	var sent dash0api.SloDefinition
+	require.NoError(t, json.Unmarshal(req.Body, &sent))
+	require.NotNil(t, sent.Metadata.Labels)
+	assert.Nil(t, sent.Metadata.Labels.Dash0Comversion, "dash0.com/version must be stripped")
+	assert.Nil(t, sent.Metadata.Labels.Dash0Comorigin, "dash0.com/origin must be stripped")
+	if sent.Metadata.Annotations != nil {
+		_, hasCreatedAt := sent.Metadata.Annotations.Get("dash0.com/created-at")
+		assert.False(t, hasCreatedAt, "dash0.com/created-at must be stripped")
+	}
+	// Belt-and-suspenders: the raw body must not carry the stripped keys.
+	body := string(req.Body)
+	assert.NotContains(t, body, "dash0.com/version")
+	assert.NotContains(t, body, "dash0.com/created-at")
+	assert.NotContains(t, body, "dash0.com/origin")
+}
+
+// TestCreateSLO_DryRun asserts that `create --dry-run` validates without
+// touching the API.
+func TestCreateSLO_DryRun(t *testing.T) {
+	testutil.SetupTestEnv(t)
+
+	server := testutil.NewMockServer(t, testutil.FixturesDir())
+
+	tmpDir := t.TempDir()
+	yamlFile := filepath.Join(tmpDir, "slo.yaml")
+	require.NoError(t, os.WriteFile(yamlFile, []byte(sloCreateYAML), 0644))
+
+	cmd := NewSlosCmd()
+	cmd.SetArgs([]string{"create", "-f", yamlFile, "--api-url", server.URL, "--auth-token", testAuthToken, "--dry-run"})
+
+	var err error
+	output := testutil.CaptureStdout(t, func() {
+		err = cmd.Execute()
+	})
+
+	require.NoError(t, err)
+	assert.Contains(t, output, "Dry run")
+	// No API call must have been made.
+	assert.Nil(t, server.LastRequest())
+}
+
+// TestUpdateSLO_DryRun asserts that `update --dry-run` fetches the current
+// state for the diff but never issues the PUT.
+func TestUpdateSLO_DryRun(t *testing.T) {
+	testutil.SetupTestEnv(t)
+
+	server := testutil.NewMockServer(t, testutil.FixturesDir())
+	server.OnPattern(http.MethodGet, sloIDPattern, testutil.MockResponse{
+		StatusCode: http.StatusOK,
+		BodyFile:   fixtureGetSuccess,
+		Validator:  testutil.RequireHeaders,
+	})
+
+	tmpDir := t.TempDir()
+	yamlFile := filepath.Join(tmpDir, "slo.yaml")
+	require.NoError(t, os.WriteFile(yamlFile, []byte(sloUpdateYAML), 0644))
+
+	cmd := NewSlosCmd()
+	cmd.SetArgs([]string{"update", "-f", yamlFile, "--api-url", server.URL, "--auth-token", testAuthToken, "--dry-run"})
+
+	var err error
+	testutil.CaptureStdout(t, func() {
+		err = cmd.Execute()
+	})
+
+	require.NoError(t, err)
+	// The only request must be the GET used to build the diff — never a PUT.
+	req := server.LastRequest()
+	require.NotNil(t, req)
+	assert.Equal(t, http.MethodGet, req.Method)
+}
+
 const sloCreateYAML = `apiVersion: openslo.com/v1
 kind: SLO
 metadata:
@@ -428,3 +568,49 @@ spec:
     - displayName: 99.5% availability
       target: 0.995
 `
+
+// sloUpdateWithServerFieldsYAML mirrors the shape of an exported
+// `slos get -o yaml`: it carries the server-managed dash0.com/version,
+// dash0.com/dataset, and dash0.com/origin labels plus the created-at/updated-at
+// annotations. The update path must strip all of these before the PUT.
+const sloUpdateWithServerFieldsYAML = `apiVersion: openslo.com/v1
+kind: SLO
+metadata:
+  name: checkout-availability
+  labels:
+    dash0.com/id: 00000000-0000-0000-0000-000000000001
+    dash0.com/version: "1"
+    dash0.com/dataset: default
+    dash0.com/origin: terraform
+  annotations:
+    dash0.com/display-name: Checkout availability
+    dash0.com/enabled: "true"
+    dash0.com/created-at: "2026-01-15T10:00:00Z"
+    dash0.com/updated-at: "2026-01-15T10:00:00Z"
+spec:
+  description: 99 percent of checkout HTTP requests succeed over a rolling 28-day window.
+  service: checkout
+  budgetingMethod: Occurrences
+  timeWindow:
+    - duration: 28d
+      isRolling: true
+  indicator:
+    metadata:
+      name: checkout-success-ratio
+    spec:
+      ratioMetric:
+        counter: true
+        good:
+          metricSource:
+            type: Prometheus
+            spec:
+              query: 'http_server_request_duration_seconds_count{service_name="checkout",http_response_status_code!~"5.."}'
+        total:
+          metricSource:
+            type: Prometheus
+            spec:
+              query: 'http_server_request_duration_seconds_count{service_name="checkout"}'
+  objectives:
+    - displayName: 99% availability
+      target: 0.99
+`

internal/slos/list.go

@@ -83,7 +83,7 @@ func runList(ctx context.Context, flags *asset.ListFlags) error {
 
 	switch format {
 	case output.FormatJSON, output.FormatYAML:
-		definitions := make([]interface{}, 0, len(items))
+		definitions := make([]any, 0, len(items))
 		for _, slo := range items {
 			definitions = append(definitions, slo)
 		}
@@ -98,20 +98,20 @@ func runList(ctx context.Context, flags *asset.ListFlags) error {
 
 func printSLOTable(f *output.Formatter, items []*dash0api.SloDefinition, format output.Format, apiUrl string, dataset *string) error {
 	columns := []output.Column{
-		{Header: internal.HEADER_NAME, Width: 40, Value: func(item interface{}) string {
+		{Header: internal.HEADER_NAME, Width: 40, Value: func(item any) string {
 			return dash0api.GetSLOName(item.(*dash0api.SloDefinition))
 		}},
-		{Header: internal.HEADER_ID, Width: 36, Value: func(item interface{}) string {
+		{Header: internal.HEADER_ID, Width: 36, Value: func(item any) string {
 			return dash0api.GetSLOID(item.(*dash0api.SloDefinition))
 		}},
 	}
 
 	if format == output.FormatWide || format == output.FormatCSV {
 		columns = append(columns,
-			output.Column{Header: internal.HEADER_DATASET, Width: 15, Value: func(item interface{}) string {
+			output.Column{Header: internal.HEADER_DATASET, Width: 15, Value: func(item any) string {
 				return dash0api.GetSLODataset(item.(*dash0api.SloDefinition))
 			}},
-			output.Column{Header: internal.HEADER_URL, Width: 70, Value: func(item interface{}) string {
+			output.Column{Header: internal.HEADER_URL, Width: 70, Value: func(item any) string {
 				return dash0api.DeeplinkURL(apiUrl, dash0api.DeeplinkAssetTypeSLO, dash0api.GetSLOID(item.(*dash0api.SloDefinition)), dataset)
 			}},
 		)
@@ -122,7 +122,7 @@ func printSLOTable(f *output.Formatter, items []*dash0api.SloDefinition, format
 		return nil
 	}
 
-	data := make([]interface{}, len(items))
+	data := make([]any, len(items))
 	for i, s := range items {
 		data[i] = s
 	}

internal/slos/update.go

@@ -76,6 +76,15 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags)
 		})
 	}
 
+	// Strip server-managed fields before diffing or sending the body back. An
+	// exported SLO (e.g. via `dash0 slos get -o yaml`) carries dash0.com/origin
+	// plus version, dataset, source, and created/updated timestamps, which the
+	// server rejects or echoes back as spurious diff noise (the same class of
+	// "origin does not match" 400 that bit views). The apply code path
+	// (asset.ImportSLO) does the same strip; without it, `apply -f` and
+	// `update -f` diverge on the same exported file.
+	dash0api.StripSLOServerFields(&slo)
+
 	if flags.DryRun {
 		return asset.PrintDiff(os.Stdout, "SLO", slo.Metadata.Name, before, &slo)
 	}