feat: add ordered --add field mutations

AGENTS.md

@@ -231,6 +231,12 @@ harness list pr_activity <repo_id>/<pr_number>
 The CLI reads auth from the active profile (typically `~/.harness/profiles.yaml`).
 For endpoint-backed create/update/execute commands, append the hidden `--preview-request` flag to inspect the assembled URL and body without sending the write (an update may still perform a preparatory GET).
 
+## Test selection
+
+Add tests for behavior with a plausible, non-obvious failure mode: interacting operations, ordering, branching rules, boundary cases, error handling, and exact request-body transformations. Prefer compact direct/table tests at the layer that owns that logic; run existing suites for the rest.
+
+Do not add tests for mechanical pass-through or framework guarantees (e.g. Cobra accepting a registered flag, a loop forwarding an element, or a handler receiving an operation explicitly supplied by the test). Do not build mock commands or HTTP servers just to reassert a body shape already covered by a direct mutation test. If you cannot name a realistic mistake the test would catch, omit it; redundant tests add maintenance and context cost.
+
 ## Current spec files
 
 | File | Commands |

pkg/cmdctx/cmdctx.go

@@ -122,6 +122,7 @@ type MutationKind string
 
 const (
 	MutationSet    MutationKind = "set"
+	MutationAdd    MutationKind = "add"
 	MutationDelete MutationKind = "del"
 )
 
@@ -225,29 +226,30 @@ type PageMeta struct {
 // Auth is nil for management commands (version, etc.) that do not require credentials.
 // When Auth is non-nil, OrgID and ProjectID already reflect any --org/--project overrides.
 type Ctx struct {
-	Context       context.Context
-	CancelFn      context.CancelCauseFunc
-	Auth          *auth.ResolvedAuth
-	Verb          string
-	VerbHandler   string // behavioral dispatch verb; defaults to Verb when verb_handler is unset in spec
-	Noun          string
-	FieldsNoun    string // overrides Noun for field lookup when set (from spec fields_noun)
-	Id            string
-	ParentId      string            // optional parent-id arg for list commands (e.g. pipeline ID on "list execution")
-	MigrateFrom   string            // --from flag value (pair verbs, e.g. migrate: identifies the source endpoint)
-	MigrateTo     string            // --to flag value (pair verbs, e.g. migrate: identifies the destination endpoint)
-	SetArgs       map[string]string // --set key=value pairs for update verb (when HasSetArg set on spec)
-	DelArgs       []string          // --del key targets for update verb (when HasSetArg set on spec)
-	MutationFlags []FieldMutation   // set flags, positional sets, then delete flags; not original argv interleaving
-	Args          []string          // extra positional args beyond [id] (when HasArgs set on spec)
-	IdParts       []string          // id split on "/" when id_parts > 1 on spec; length equals the number of actual parts
-	Level         string            // scope level: "account", "org", or "project" (empty when flag not present)
-	IsPty         bool              // true when stdout is an interactive terminal
-	IsCompletion  bool              // true when this ctx was built for a shell completion request
-	Resolver      Resolver
-	GlobalFlags   GlobalFlags
-	FormatFlags   FormatFlags
-	PagingFlags   PagingFlags
+	Context               context.Context
+	CancelFn              context.CancelCauseFunc
+	Auth                  *auth.ResolvedAuth
+	Verb                  string
+	VerbHandler           string // behavioral dispatch verb; defaults to Verb when verb_handler is unset in spec
+	Noun                  string
+	FieldsNoun            string // overrides Noun for field lookup when set (from spec fields_noun)
+	Id                    string
+	ParentId              string            // optional parent-id arg for list commands (e.g. pipeline ID on "list execution")
+	MigrateFrom           string            // --from flag value (pair verbs, e.g. migrate: identifies the source endpoint)
+	MigrateTo             string            // --to flag value (pair verbs, e.g. migrate: identifies the destination endpoint)
+	SetArgs               map[string]string // --set key=value pairs for update verb (when HasSetArg set on spec)
+	DelArgs               []string          // --del key targets for update verb (when HasSetArg set on spec)
+	MutationFlags         []FieldMutation   // explicit flags in parse order, followed by positional sets
+	MutationOrderCaptured bool              // use MutationFlags rather than the legacy SetArgs/DelArgs views
+	Args                  []string          // extra positional args beyond [id] (when HasArgs set on spec)
+	IdParts               []string          // id split on "/" when id_parts > 1 on spec; length equals the number of actual parts
+	Level                 string            // scope level: "account", "org", or "project" (empty when flag not present)
+	IsPty                 bool              // true when stdout is an interactive terminal
+	IsCompletion          bool              // true when this ctx was built for a shell completion request
+	Resolver              Resolver
+	GlobalFlags           GlobalFlags
+	FormatFlags           FormatFlags
+	PagingFlags           PagingFlags
 	// FlagValues holds typed flag values for this command, keyed by flag name. It contains:
 	//   - all flags declared in the spec (cs.Flags), typed as string/bool/[]string
 	//   - "page"         int    (0-indexed) when the spec declares builtin_flags.page

pkg/registry/buildctx.go

@@ -265,36 +265,40 @@ func buildCtx(cmd *cobra.Command, cs *spec.CommandSpec, args []string, r *Regist
 		}
 		ctx.Args = extra
 	}
-	if cs.BuiltinFlags.Set {
-		setVals, _ := cmd.Flags().GetStringArray("set")
-		// positional args after the id are also treated as key=value pairs
-		positional := args
-		if consumedIdArg {
-			positional = args[1:]
-		}
-		all := append(setVals, positional...)
-		if len(all) > 0 {
-			ctx.SetArgs = make(map[string]string, len(all))
-			for _, kv := range all {
-				k, v, ok := strings.Cut(kv, "=")
-				if !ok && !isBareSetField(k, cs, r) {
-					return nil, fmt.Errorf("invalid value %q: expected key=value format", kv)
+	if cs.BuiltinFlags.Set || cs.BuiltinFlags.Del {
+		ctx.MutationOrderCaptured = true
+		for _, op := range capturedMutationFlags(cmd) {
+			op.Key, op.Value, op.HasValue = strings.Cut(op.Raw, "=")
+			switch op.Kind {
+			case cmdctx.MutationSet:
+				if !op.HasValue && !isBareSetField(op.Key, cs, r) {
+					return nil, fmt.Errorf("invalid value %q: expected key=value format", op.Raw)
 				}
-				ctx.MutationFlags = append(ctx.MutationFlags, cmdctx.FieldMutation{
-					Kind: cmdctx.MutationSet, Raw: kv, Key: k, Value: v, HasValue: ok,
-				})
-				ctx.SetArgs[k] = v
+				if ctx.SetArgs == nil {
+					ctx.SetArgs = map[string]string{}
+				}
+				ctx.SetArgs[op.Key] = op.Value
+			case cmdctx.MutationDelete:
+				ctx.DelArgs = append(ctx.DelArgs, op.Raw)
 			}
+			ctx.MutationFlags = append(ctx.MutationFlags, op)
 		}
-	}
-	if cs.BuiltinFlags.Del {
-		delVals, _ := cmd.Flags().GetStringArray("del")
-		if len(delVals) > 0 {
-			ctx.DelArgs = delVals
-			for _, raw := range delVals {
+		if cs.BuiltinFlags.Set {
+			positional := args
+			if consumedIdArg {
+				positional = args[1:]
+			}
+			for _, raw := range positional {
 				key, value, hasValue := strings.Cut(raw, "=")
+				if !hasValue && !isBareSetField(key, cs, r) {
+					return nil, fmt.Errorf("invalid value %q: expected key=value format", raw)
+				}
+				if ctx.SetArgs == nil {
+					ctx.SetArgs = map[string]string{}
+				}
+				ctx.SetArgs[key] = value
 				ctx.MutationFlags = append(ctx.MutationFlags, cmdctx.FieldMutation{
-					Kind: cmdctx.MutationDelete, Raw: raw, Key: key, Value: value, HasValue: hasValue,
+					Kind: cmdctx.MutationSet, Raw: raw, Key: key, Value: value, HasValue: hasValue,
 				})
 			}
 		}

pkg/registry/buildctx_workflow_test.go

@@ -754,7 +754,7 @@ func TestBuildCtx_MutationFlagsPreserveRawOperands(t *testing.T) {
 				t.Fatal(err)
 			}
 			cs := &spec.CommandSpec{Command: "update widget", Verb: VerbUpdate, Noun: "widget",
-				NoAuth: true, BuiltinFlags: spec.BuiltinFlags{Set: true, Del: true}}
+				NoAuth: true, BuiltinFlags: spec.BuiltinFlags{Set: true}}
 			cmd := &cobra.Command{Use: "widget"}
 			if workflow {
 				cs.HandlerType = spec.HandlerWorkflow
@@ -765,7 +765,7 @@ func TestBuildCtx_MutationFlagsPreserveRawOperands(t *testing.T) {
 			}
 			cmd.Flags().Float64("timeout", 0, "Command timeout in seconds")
 			if err := cmd.ParseFlags([]string{"id", "--set", "modules.CD", "--set=modules.CD=", "--set", "owner=user:a@x.com",
-				"--set", "name=a=b", "--del", "owners=", "--del=modules.CD", "description=positional"}); err != nil {
+				"--del", "owners=", "--add=modules.CI", "--set", "name=a=b", "--del=modules.CD", "description=positional"}); err != nil {
 				t.Fatal(err)
 			}
 			ctx, err := buildCtx(cmd, cs, cmd.Flags().Args(), r)
@@ -776,10 +776,11 @@ func TestBuildCtx_MutationFlagsPreserveRawOperands(t *testing.T) {
 				{Kind: cmdctx.MutationSet, Raw: "modules.CD", Key: "modules.CD"},
 				{Kind: cmdctx.MutationSet, Raw: "modules.CD=", Key: "modules.CD", HasValue: true},
 				{Kind: cmdctx.MutationSet, Raw: "owner=user:a@x.com", Key: "owner", Value: "user:a@x.com", HasValue: true},
-				{Kind: cmdctx.MutationSet, Raw: "name=a=b", Key: "name", Value: "a=b", HasValue: true},
-				{Kind: cmdctx.MutationSet, Raw: "description=positional", Key: "description", Value: "positional", HasValue: true},
 				{Kind: cmdctx.MutationDelete, Raw: "owners=", Key: "owners", HasValue: true},
+				{Kind: cmdctx.MutationAdd, Raw: "modules.CI", Key: "modules.CI"},
+				{Kind: cmdctx.MutationSet, Raw: "name=a=b", Key: "name", Value: "a=b", HasValue: true},
 				{Kind: cmdctx.MutationDelete, Raw: "modules.CD", Key: "modules.CD"},
+				{Kind: cmdctx.MutationSet, Raw: "description=positional", Key: "description", Value: "positional", HasValue: true},
 			}
 			if !reflect.DeepEqual(ctx.MutationFlags, want) {
 				t.Fatalf("MutationFlags = %#v, want %#v", ctx.MutationFlags, want)

pkg/registry/endpoint.go

@@ -856,9 +856,30 @@ func runGetThenPutKV(ctx *cmdctx.Ctx, ep *spec.EndpointSpec, c *client.Client, p
 		}
 	}
 
-	maps.Copy(kvMap, ctx.SetArgs)
-	for _, k := range ctx.DelArgs {
-		delete(kvMap, k)
+	if ctx.MutationOrderCaptured {
+		for _, op := range ctx.MutationFlags {
+			switch op.Kind {
+			case cmdctx.MutationSet:
+				kvMap[op.Key] = op.Value
+			case cmdctx.MutationDelete:
+				delete(kvMap, op.Raw)
+			case cmdctx.MutationAdd:
+				if !op.HasValue {
+					return nil, fmt.Errorf("--add %s: expected key=value", op.Key)
+				}
+				if existing, found := kvMap[op.Key]; found && existing != op.Value {
+					return nil, fmt.Errorf("--add %s: key already exists with a different value; use --set to overwrite it", op.Key)
+				}
+				kvMap[op.Key] = op.Value
+			default:
+				return nil, fmt.Errorf("unknown mutation operation %q", op.Kind)
+			}
+		}
+	} else {
+		maps.Copy(kvMap, ctx.SetArgs)
+		for _, k := range ctx.DelArgs {
+			delete(kvMap, k)
+		}
 	}
 
 	// Rebuild as [{key, value}, ...].
@@ -909,6 +930,9 @@ func runSetFields(ctx *cmdctx.Ctx, ep *spec.EndpointSpec, c *client.Client, path
 
 func mutationBodyForCtx(ctx *cmdctx.Ctx, base map[string]any, wrap string, extra map[string]any) (map[string]any, error) {
 	ops := effectiveMutations(ctx.SetArgs, ctx.DelArgs, ctx.MutationFlags)
+	if ctx.MutationOrderCaptured {
+		ops = ctx.MutationFlags
+	}
 	if len(ops) > 0 && ctx.Resolver == nil {
 		return nil, fmt.Errorf("field type resolver is not available")
 	}

pkg/registry/endpoint_test.go

@@ -658,8 +658,10 @@ func TestCallEndpointFull_Priority2_GetThenPutKV(t *testing.T) {
 		name        string
 		setArgs     map[string]string
 		delArgs     []string
+		mutations   []cmdctx.FieldMutation
 		wantPresent map[string]string
 		wantAbsent  []string
+		wantError   string
 	}{
 		{
 			name:        "upsert_existing_key",
@@ -677,6 +679,25 @@ func TestCallEndpointFull_Priority2_GetThenPutKV(t *testing.T) {
 			wantPresent: map[string]string{"env": "prod"},
 			wantAbsent:  []string{"team"},
 		},
+		{
+			name: "ordered_add_delete_and_set",
+			mutations: []cmdctx.FieldMutation{
+				{Kind: cmdctx.MutationAdd, Key: "region", Value: "us", HasValue: true},
+				{Kind: cmdctx.MutationAdd, Key: "env", Value: "prod", HasValue: true},
+				{Kind: cmdctx.MutationDelete, Key: "team", Raw: "team"},
+				{Kind: cmdctx.MutationSet, Key: "env", Value: "stage", HasValue: true},
+			},
+			wantPresent: map[string]string{"env": "stage", "region": "us"},
+			wantAbsent:  []string{"team"},
+		},
+		{
+			name: "add_conflict_prevents_write",
+			mutations: []cmdctx.FieldMutation{
+				{Kind: cmdctx.MutationAdd, Key: "env", Value: "stage", HasValue: true},
+				{Kind: cmdctx.MutationDelete, Key: "env", Raw: "env"},
+			},
+			wantError: "key already exists",
+		},
 	}
 
 	for _, tc := range tests {
@@ -685,13 +706,21 @@ func TestCallEndpointFull_Priority2_GetThenPutKV(t *testing.T) {
 			ctx := testCtx(srv.URL, nil)
 			ctx.SetArgs = tc.setArgs
 			ctx.DelArgs = tc.delArgs
+			ctx.MutationFlags = tc.mutations
+			ctx.MutationOrderCaptured = len(tc.mutations) > 0
 			ctx.Resolver = testNounRegistry(t)
 
 			ep := &spec.EndpointSpec{
 				Path: "/kv", Method: "PUT",
 				UpdateStrategy: spec.UpdateStrategyGetThenPutKV, UpdateBodyWrap: "metadata",
 			}
 			_, _, err := callEndpointFull(ctx, ep, nil)
+			if tc.wantError != "" {
+				if err == nil || !strings.Contains(err.Error(), tc.wantError) || len(*caps) != 1 {
+					t.Fatalf("error = %v, server requests = %d; want GET only and %q", err, len(*caps), tc.wantError)
+				}
+				return
+			}
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}

pkg/registry/fieldtype.go

@@ -37,19 +37,38 @@ func (r *Registry) ResolveFieldType(id string) (cmdctx.FieldTypeHandler, bool) {
 }
 
 func mutateScalar(_ spec.FieldDef, _ any, op cmdctx.FieldMutation) (any, bool, error) {
-	if op.Kind == cmdctx.MutationDelete {
+	switch op.Kind {
+	case cmdctx.MutationSet:
+		return op.Value, true, nil
+	case cmdctx.MutationDelete:
+		if op.HasValue {
+			return nil, false, fmt.Errorf("--del %s: scalar fields do not take a value", op.Raw)
+		}
 		return nil, true, nil
+	case cmdctx.MutationAdd:
+		return nil, false, fmt.Errorf("--add %s: scalar fields do not support addition", op.Key)
+	default:
+		return nil, false, fmt.Errorf("unknown mutation operation %q", op.Kind)
 	}
-	return op.Value, true, nil
 }
 
 func mutateTags(_ spec.FieldDef, current any, op cmdctx.FieldMutation) (any, bool, error) {
-	_, tag, found := strings.Cut(op.Key, ".")
+	if op.Kind != cmdctx.MutationSet && op.Kind != cmdctx.MutationAdd && op.Kind != cmdctx.MutationDelete {
+		return nil, false, fmt.Errorf("unknown mutation operation %q", op.Kind)
+	}
+	selector := op.Key
+	if op.Kind == cmdctx.MutationDelete && op.HasValue {
+		selector = op.Raw
+	}
+	_, tag, found := strings.Cut(selector, ".")
 	if !found {
 		if op.Kind == cmdctx.MutationDelete {
 			return nil, false, fmt.Errorf("--del %s: tag fields require a key (e.g. --del tags.key)", op.Key)
 		}
-		return nil, false, fmt.Errorf("--set %s: tag fields require a key (e.g. --set tags.key=value)", op.Key)
+		return nil, false, fmt.Errorf("--%s %s: tag fields require a key (e.g. --%s tags.key=value)", op.Kind, op.Key, op.Kind)
+	}
+	if op.Kind == cmdctx.MutationAdd && !op.HasValue {
+		return nil, false, fmt.Errorf("--add %s: tag fields require key=value", op.Key)
 	}
 	tags, _ := current.(map[string]any)
 	if op.Kind == cmdctx.MutationDelete {
@@ -64,16 +83,34 @@ func mutateTags(_ spec.FieldDef, current any, op cmdctx.FieldMutation) (any, boo
 	if op.Kind == cmdctx.MutationDelete {
 		delete(next, tag)
 	} else {
+		if op.Kind == cmdctx.MutationAdd {
+			if existing, found := next[tag]; found {
+				if existing == op.Value {
+					return nil, false, nil
+				}
+				return nil, false, fmt.Errorf("--add %s: key already exists with a different value; use --set to overwrite it", op.Key)
+			}
+		}
 		next[tag] = op.Value
 	}
 	return next, true, nil
 }
 
 func mutateStringSet(_ spec.FieldDef, current any, op cmdctx.FieldMutation) (any, bool, error) {
-	_, member, found := strings.Cut(op.Key, ".")
-	if !found || (op.Kind == cmdctx.MutationSet && member == "") {
+	if op.Kind != cmdctx.MutationSet && op.Kind != cmdctx.MutationAdd && op.Kind != cmdctx.MutationDelete {
+		return nil, false, fmt.Errorf("unknown mutation operation %q", op.Kind)
+	}
+	selector := op.Key
+	if op.Kind == cmdctx.MutationDelete && op.HasValue {
+		selector = op.Raw
+	}
+	_, member, found := strings.Cut(selector, ".")
+	if !found || member == "" {
 		return nil, false, fmt.Errorf("--%s %s: set fields require a member (e.g. --%s modules.CD)", op.Kind, op.Key, op.Kind)
 	}
+	if op.Kind == cmdctx.MutationAdd && op.HasValue {
+		return nil, false, fmt.Errorf("--add %s: set members do not take a value", op.Key)
+	}
 	var arr []any
 	switch values := current.(type) {
 	case []any:

pkg/registry/fieldtype_test.go

@@ -134,7 +134,7 @@ func TestApplyMutations_ExistingNoWriteCases(t *testing.T) {
 	}
 }
 
-func TestApplyMutations_StillSetsBeforeDeleting(t *testing.T) {
+func TestLegacyMutationsStillSetBeforeDeleting(t *testing.T) {
 	r := New()
 	field := spec.FieldDef{ID: "name", Expr: "it.name", MutablePath: "name"}
 	m := map[string]any{"name": "old"}

pkg/registry/flag.go

@@ -129,6 +129,7 @@ var coreFlagTable = []CoreFlag{
 	{Name: "ui", Bool: true},
 	{Name: "force", Bool: true},
 	{Name: "set"},
+	{Name: "add"},
 	{Name: "del"},
 	{Name: "preview-request", Bool: true},
 	{Name: "from"},