Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion internal/requestflag/innerflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ type InnerFlag[
// map[string]any before SetInnerField runs. The hint is ignored for typed outer
// flags whose zero value already carries a dispatchable reflect.Kind.
OuterIsArrayOfObjects bool

hasBeenSet bool
}

// GetDataAliases returns the aliases recognized when parsing inner field keys from piped or flag YAML.
Expand Down Expand Up @@ -89,6 +91,7 @@ func (f *InnerFlag[T]) Set(name string, rawVal string) error {

if settableInnerField, ok := f.OuterFlag.(SettableInnerField); ok {
settableInnerField.SetInnerField(f.InnerField, parsedValue)
f.hasBeenSet = true
} else {
return fmt.Errorf("Cannot set inner field on %v", f.OuterFlag)
}
Expand All @@ -106,7 +109,7 @@ func (f *InnerFlag[T]) String() string {
}

func (f *InnerFlag[T]) IsSet() bool {
return false
return f.hasBeenSet
Comment on lines 111 to +112

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep set state scoped to the current array element

When a repeatable array-of-objects flag creates a second element, an inner flag used on the first element remains globally set. For example, after setting context-management.compact-threshold on the first entry and starting a second entry with another context-management.type, piped context_management.compact_threshold is now skipped because applyStdinDataToFlags checks IsSet() before innerFieldIsSet() can see that the trailing element lacks the field. This regresses the existing per-trailing-element merge behavior; track explicit state per element or retain the innerFieldIsSet decision for array-backed inner flags.

AGENTS.md reference: AGENTS.md:L15-L18

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 65a9973. Inner flags now check per-element outer state before the generic IsSet() gate, so stdin can fill an unset field on a later array element without overriding an explicitly set field on that element. Added a two-element regression; focused requestflag tests pass.

}

func (f *InnerFlag[T]) Names() []string {
Expand Down
36 changes: 36 additions & 0 deletions internal/requestflag/innerflag_precedence_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package requestflag

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/urfave/cli/v3"
)

func TestInnerFlagCLIValueBeatsPipedData(t *testing.T) {
t.Parallel()

outer := &Flag[map[string]any]{
Name: "address",
BodyPath: "address",
}
assert.NoError(t, outer.PreParse())

cityInner := &InnerFlag[string]{
Name: "address.city",
InnerField: "city",
OuterFlag: outer,
}
assert.NoError(t, cityInner.Set("address.city", "cli-value"))
assert.True(t, cityInner.IsSet())

data := map[string]any{
"address": map[string]any{"city": "piped-value"},
}
cmd := &cli.Command{Flags: []cli.Flag{outer, cityInner}}
assert.NoError(t, ApplyStdinDataToFlags(cmd, data))

outerVal, ok := outer.Get().(map[string]any)
assert.True(t, ok)
assert.Equal(t, "cli-value", outerVal["city"])
}
12 changes: 7 additions & 5 deletions internal/requestflag/requestflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,9 @@ type RequestContents struct {

func applyStdinDataToFlags(cmd *cli.Command, data map[string]any, onSet func(cli.Flag)) error {
for _, flag := range cmd.Flags {
if flag.IsSet() {
continue
}

// Handle inner flags: look for their value nested under the outer flag's body path.
// Handle inner flags before the generic IsSet check. InnerFlag tracks whether
// it has ever been set, while array-of-object precedence is scoped to the
// trailing element of the outer value.
if inner, ok := flag.(HasOuterFlag); ok {
outer, outerOk := inner.GetOuterFlag().(InRequest)
if !outerOk || outer.GetBodyPath() == "" {
Expand Down Expand Up @@ -179,6 +177,10 @@ func applyStdinDataToFlags(cmd *cli.Command, data map[string]any, onSet func(cli
continue
}

if flag.IsSet() {
continue
}

inReq, ok := flag.(InRequest)
if !ok {
continue
Expand Down
33 changes: 33 additions & 0 deletions internal/requestflag/stdinprovenance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,36 @@ func TestApplyStdinDataToFlagsWithProvenancePreservesExplicitEmptyCollections(t
})
}
}

func TestApplyStdinDataToFlagsFillsUnsetFieldOnTrailingArrayElement(t *testing.T) {
t.Parallel()

outer := &Flag[[]map[string]any]{Name: "entries", BodyPath: "entries"}
typeFlag := &InnerFlag[string]{
Name: "entries.type",
InnerField: "type",
OuterFlag: outer,
OuterIsArrayOfObjects: true,
}
thresholdFlag := &InnerFlag[int64]{
Name: "entries.compact-threshold",
InnerField: "compact_threshold",
OuterFlag: outer,
OuterIsArrayOfObjects: true,
}
require.NoError(t, outer.PreParse())
require.NoError(t, typeFlag.Set(typeFlag.Name, "first"))
require.NoError(t, thresholdFlag.Set(thresholdFlag.Name, "10"))
require.NoError(t, typeFlag.Set(typeFlag.Name, "second"))

command := &cli.Command{Flags: []cli.Flag{outer, typeFlag, thresholdFlag}}
err := ApplyStdinDataToFlags(command, map[string]any{
"entries": map[string]any{"compact_threshold": 20},
})

require.NoError(t, err)
require.Equal(t, []map[string]any{
{"type": "first", "compact_threshold": int64(10)},
{"type": "second", "compact_threshold": int64(20)},
}, outer.Get())
}