From cb92156afb7274d1151e9cb4c58c96f6677b30c3 Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan Date: Sat, 1 Aug 2026 21:27:09 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(edge):=20filters=20for=20`edge=20list`?= =?UTF-8?q?=20=E2=80=94=20--direction,=20--name,=20--to,=20--from=20(#337)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `edge list` returned every edge in both directions with no way to narrow, so any question smaller than "all edges of this node" meant piping --json into a script. Building and running `coding review|preflight lint` I wrote the same shape of filter a dozen times, and filing hadron-server#845/#850 meant extracting edge ids and labels by hand. The three scripts named in #337 are now single commands: edge ls review -m …::mmdata --direction incoming → 55 edges edge ls preflight -m …::dev --name routes-to → 15 edges edge ls preflight -m …::dev --to findings:prisma-upsert-… → 019ed2e5327b7d1… That last id matches what I extracted by script for hadron-server#850. --to and --from are directional by construction: --to names a node this one points AT (outgoing), --from a node pointing at THIS one (incoming). Each matches the far endpoint by loc OR id, since --json prints both and #339 just made ids first-class refs. Combinations that can only ever match nothing — --to with --direction incoming, --to plus --from, an unknown --direction — are usage errors rather than an empty list that reads like "no such edges". A redacted cross-memory endpoint (#781) carries neither loc nor id, so it never satisfies --to/--from; matching on "" would have made every such edge look like the one asked for. It stays listable unfiltered and by --name. Purely client-side over what GetNode already returns: no new query, no server change, and the row shape is unchanged so existing scripts keep working — a test asserts a filtered row carries every field an unfiltered one does. Co-Authored-By: Claude Opus 5 --- internal/cmd/agentic/agentic-usage.md | 2 +- internal/cmd/edge/ls.go | 94 ++++++++++++++++- internal/cmd/edge/ls_test.go | 141 ++++++++++++++++++++++++++ internal/cmd/edge_test.go | 90 ++++++++++++++++ 4 files changed, 325 insertions(+), 2 deletions(-) create mode 100644 internal/cmd/edge/ls_test.go diff --git a/internal/cmd/agentic/agentic-usage.md b/internal/cmd/agentic/agentic-usage.md index 86f2d88..c862cda 100644 --- a/internal/cmd/agentic/agentic-usage.md +++ b/internal/cmd/agentic/agentic-usage.md @@ -73,7 +73,7 @@ hadron task run | -m [--arg k=v]... [--app [--as-s hadron chat read [--since ] [--node | -m --messages-loc ] | post (--body | --body-file ) [--node ] [--reply-to ] [--handle ] [--identity ] [--role ] hadron search [-m ]... [--mode hybrid|keyword|vector|regex] [--prefix ] [--type ] [--object-type ] [--tag ]... [--where ] [--sort-property ] [--limit N] [--offset N] [-l|--long] [--json] hadron replace text --field (--node | -m ) [--prefix ] [--regex] [-i] [--dry-run] [--yes] [--max-nodes N] -hadron edge list | add | update | rm +hadron edge list [--direction incoming|outgoing] [--name ] [--to ] [--from ] | add | update | rm hadron spec list [-m ] | get |--prefix | describe | use [] | register [--check] | find [--match-exactly] | grep [--regex] [-i] [--field content|abstract] [--prefix ] | replace [--regex] [--word-boundary=false] [--field content|abstract] [--dry-run] [--yes] [--max-specs N] | new ... | edit | extract --to-feature | link | lint [] | check-tools [--prefix ] | supersede | import spec-kit|code hadron coding review lint -m [--root ] [--toolchain |-] [--strict] [--suggest] [--fix [--yes]] [--json] | preflight lint -m [--root ] [--strict] [--json] hadron app list --org | install (--org | --owner-me) --agent --name [--type ] [--urn ] [--description ] | uninstall | use diff --git a/internal/cmd/edge/ls.go b/internal/cmd/edge/ls.go index b8ab999..b4574d5 100644 --- a/internal/cmd/edge/ls.go +++ b/internal/cmd/edge/ls.go @@ -2,6 +2,7 @@ package edge import ( "io" + "strings" "github.com/spf13/cobra" @@ -39,16 +40,101 @@ func edgeListRow(id, dir string, name *string, loc string, isRunnable *bool, pri } } +// edgeFilter narrows a node's edges. The zero value matches everything, so an +// unfiltered `edge list` is unchanged. +// +// To and From are directional by construction: an edge TO x is one this node +// points at (outgoing), an edge FROM x is one pointing at this node (incoming). +// Each matches the far endpoint by loc OR id, since both are printed in --json +// and either is what a caller has to hand. +type edgeFilter struct { + Direction string // "" | incoming | outgoing + Name string // case-insensitive substring of the edge label + To string // far endpoint loc or id, outgoing only + From string // far endpoint loc or id, incoming only +} + +func (fl edgeFilter) match(e edgeListDTO) bool { + if fl.Direction != "" && e.Direction != fl.Direction { + return false + } + if fl.Name != "" && !strings.Contains(strings.ToLower(e.Name), strings.ToLower(fl.Name)) { + return false + } + if fl.To != "" && (e.Direction != "outgoing" || !endpointIs(e, fl.To)) { + return false + } + if fl.From != "" && (e.Direction != "incoming" || !endpointIs(e, fl.From)) { + return false + } + return true +} + +// endpointIs reports whether the far endpoint is ref, by loc or by id. A +// redacted endpoint (#781) carries neither, so it never matches — correct: the +// caller asked for a specific node and this edge can't be shown to be it. +func endpointIs(e edgeListDTO, ref string) bool { + return (e.OtherLoc != "" && e.OtherLoc == ref) || (e.OtherID != "" && e.OtherID == ref) +} + +// filterEdges applies fl, always returning a non-nil slice so --json renders +// [] rather than null. +func filterEdges(edges []edgeListDTO, fl edgeFilter) []edgeListDTO { + out := []edgeListDTO{} + for _, e := range edges { + if fl.match(e) { + out = append(out, e) + } + } + return out +} + +// validate rejects combinations that can only ever match nothing, rather than +// letting them return an empty list that reads like "no such edges". +func (fl edgeFilter) validate() error { + switch fl.Direction { + case "", "incoming", "outgoing": + default: + return exitcode.Newf(exitcode.Usage, "--direction %q must be \"incoming\" or \"outgoing\"", fl.Direction) + } + if fl.To != "" && fl.From != "" { + return exitcode.Newf(exitcode.Usage, "--to and --from are mutually exclusive — an edge has one far endpoint, and the two name opposite directions") + } + if fl.To != "" && fl.Direction == "incoming" { + return exitcode.Newf(exitcode.Usage, "--to selects outgoing edges, so it cannot be combined with --direction incoming") + } + if fl.From != "" && fl.Direction == "outgoing" { + return exitcode.Newf(exitcode.Usage, "--from selects incoming edges, so it cannot be combined with --direction outgoing") + } + return nil +} + func newCmdLs(f *cmdutil.Factory) *cobra.Command { var memory string + var fl edgeFilter cmd := &cobra.Command{ Use: "list | -m ", Aliases: []string{"ls"}, Short: "List a node's edges (both directions)", + Long: `List a node's edges, outgoing and incoming. + +Without filters every edge is listed. --direction, --name, --to and --from +narrow that client-side; the row shape is unchanged, so a filtered --json +run is a subset of an unfiltered one. + +--to and --from are directional: --to names a node this one points AT +(outgoing), --from a node pointing at THIS one (incoming). Either takes the +far endpoint's loc or its id.`, Example: ` hadron edge list hadronmemory.com::dev::start-here - hadron edge list start-here -m hadronmemory.com::dev`, + hadron edge list start-here -m hadronmemory.com::dev + hadron edge list review -m micromentor.org::mmdata --direction incoming + hadron edge list preflight -m hadronmemory.com::dev --name routes-to + hadron edge list preflight -m hadronmemory.com::dev --to findings:prisma-upsert-not-race-safe`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if err := fl.validate(); err != nil { + return err + } client, err := f.GraphQLClient() if err != nil { return err @@ -84,6 +170,8 @@ func newCmdLs(f *cmdutil.Factory) *cobra.Command { edges = append(edges, edgeListRow(e.Id, "incoming", e.Name, e.Loc, e.IsRunnable, e.Priority, sid, sloc)) } + edges = filterEdges(edges, fl) + return output.Write(f.IOStreams, f.JSON, edges, func(w io.Writer) error { t := output.NewTable(w, "DIR", "REL", "NODE", "EDGE-ID") for _, e := range edges { @@ -102,5 +190,9 @@ func newCmdLs(f *cmdutil.Factory) *cobra.Command { }, } cmd.Flags().StringVarP(&memory, "memory", "m", "", "memory (org::memory) to resolve a bare against") + cmd.Flags().StringVar(&fl.Direction, "direction", "", `only "incoming" or only "outgoing" edges`) + cmd.Flags().StringVar(&fl.Name, "name", "", "only edges whose label contains this (case-insensitive)") + cmd.Flags().StringVar(&fl.To, "to", "", "only outgoing edges whose target is this loc or id") + cmd.Flags().StringVar(&fl.From, "from", "", "only incoming edges whose source is this loc or id") return cmd } diff --git a/internal/cmd/edge/ls_test.go b/internal/cmd/edge/ls_test.go new file mode 100644 index 0000000..cf17ba5 --- /dev/null +++ b/internal/cmd/edge/ls_test.go @@ -0,0 +1,141 @@ +package edge + +import ( + "strings" + "testing" + + "github.com/hadron-memory/hadron-cli/internal/exitcode" +) + +// A node's edges as `edge list` builds them: two out, two in, plus one whose +// far endpoint the server redacted (#781). +func sampleEdges() []edgeListDTO { + return []edgeListDTO{ + {ID: "e1", Direction: "outgoing", Name: "routes-to", OtherID: "n1", OtherLoc: "findings:race"}, + {ID: "e2", Direction: "outgoing", Name: "to diagnose a slow query", OtherID: "n2", OtherLoc: "findings:slow"}, + {ID: "e3", Direction: "incoming", Name: "Applies when a resolver changes", OtherID: "n3", OtherLoc: "review:thin-resolver"}, + {ID: "e4", Direction: "incoming", Name: "child-of", OtherID: "n4", OtherLoc: "review:posthog"}, + {ID: "e5", Direction: "outgoing", Name: "routes-to"}, // redacted endpoint: no loc, no id + } +} + +func ids(es []edgeListDTO) []string { + out := make([]string, 0, len(es)) + for _, e := range es { + out = append(out, e.ID) + } + return out +} + +func eq(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func TestFilterEdges(t *testing.T) { + cases := []struct { + name string + fl edgeFilter + want []string + }{ + {"zero value matches everything", edgeFilter{}, []string{"e1", "e2", "e3", "e4", "e5"}}, + {"direction outgoing", edgeFilter{Direction: "outgoing"}, []string{"e1", "e2", "e5"}}, + {"direction incoming", edgeFilter{Direction: "incoming"}, []string{"e3", "e4"}}, + {"name substring", edgeFilter{Name: "routes-to"}, []string{"e1", "e5"}}, + {"name is case-insensitive", edgeFilter{Name: "APPLIES WHEN"}, []string{"e3"}}, + {"name is a substring, not a prefix", edgeFilter{Name: "resolver"}, []string{"e3"}}, + {"to, by loc", edgeFilter{To: "findings:slow"}, []string{"e2"}}, + {"to, by id", edgeFilter{To: "n2"}, []string{"e2"}}, + {"from, by loc", edgeFilter{From: "review:posthog"}, []string{"e4"}}, + {"from, by id", edgeFilter{From: "n3"}, []string{"e3"}}, + // --to is outgoing-only: an incoming edge from that node must not match. + {"to does not match an incoming endpoint", edgeFilter{To: "review:posthog"}, []string{}}, + {"from does not match an outgoing endpoint", edgeFilter{From: "findings:slow"}, []string{}}, + {"combined", edgeFilter{Direction: "outgoing", Name: "routes-to"}, []string{"e1", "e5"}}, + {"no match", edgeFilter{Name: "nothing-like-this"}, []string{}}, + } + for _, tc := range cases { + got := ids(filterEdges(sampleEdges(), tc.fl)) + if !eq(got, tc.want) { + t.Errorf("%s:\n got %v\n want %v", tc.name, got, tc.want) + } + } +} + +// A redacted endpoint carries neither loc nor id, so it must never satisfy a +// request for a specific node — matching on "" would make every such edge look +// like the one asked for. +func TestRedactedEndpointNeverMatches(t *testing.T) { + redacted := edgeListDTO{ID: "e5", Direction: "outgoing", Name: "routes-to"} + if endpointIs(redacted, "") { + t.Error(`an empty ref must not match a redacted endpoint`) + } + if got := filterEdges([]edgeListDTO{redacted}, edgeFilter{To: "anything"}); len(got) != 0 { + t.Errorf("redacted endpoint matched --to: %v", ids(got)) + } + // It still shows up unfiltered, and under a name filter. + if got := filterEdges([]edgeListDTO{redacted}, edgeFilter{Name: "routes-to"}); len(got) != 1 { + t.Error("a redacted endpoint should still be listable by label") + } +} + +// filterEdges must return a non-nil slice so --json renders [] rather than null. +func TestFilterEdgesNeverReturnsNil(t *testing.T) { + if got := filterEdges(nil, edgeFilter{}); got == nil { + t.Error("nil input produced a nil slice") + } + if got := filterEdges(sampleEdges(), edgeFilter{Name: "no-match"}); got == nil { + t.Error("an empty result produced a nil slice") + } +} + +// Combinations that can only ever match nothing are rejected up front, rather +// than returning an empty list that reads like "no such edges". +func TestEdgeFilterValidate(t *testing.T) { + ok := []edgeFilter{ + {}, + {Direction: "incoming"}, + {Direction: "outgoing"}, + {To: "x"}, + {From: "x"}, + {Direction: "outgoing", To: "x"}, + {Direction: "incoming", From: "x"}, + {Name: "x", Direction: "incoming"}, + } + for _, fl := range ok { + if err := fl.validate(); err != nil { + t.Errorf("%+v should be valid, got %v", fl, err) + } + } + + bad := []struct { + fl edgeFilter + want string + }{ + {edgeFilter{Direction: "both"}, "--direction"}, + {edgeFilter{Direction: "Incoming"}, "--direction"}, // case-sensitive by design + {edgeFilter{To: "x", From: "y"}, "mutually exclusive"}, + {edgeFilter{To: "x", Direction: "incoming"}, "--to selects outgoing"}, + {edgeFilter{From: "x", Direction: "outgoing"}, "--from selects incoming"}, + } + for _, tc := range bad { + err := tc.fl.validate() + if err == nil { + t.Errorf("%+v should be rejected", tc.fl) + continue + } + if got := exitcode.FromError(err); got != exitcode.Usage { + t.Errorf("%+v should be a usage error, got %d", tc.fl, got) + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("%+v: error should mention %q, got %q", tc.fl, tc.want, err.Error()) + } + } +} diff --git a/internal/cmd/edge_test.go b/internal/cmd/edge_test.go index 1b88330..c4d3f41 100644 --- a/internal/cmd/edge_test.go +++ b/internal/cmd/edge_test.go @@ -6,6 +6,8 @@ import ( "net/http/httptest" "strings" "testing" + + "github.com/hadron-memory/hadron-cli/internal/exitcode" ) const edgeJSON = `{"id":"e1","name":"routes-to","loc":"findings:flaky-ci:routes-to:start-here","isRunnable":false,"priority":0, @@ -174,3 +176,91 @@ func TestEdgeRmWithYes(t *testing.T) { t.Fatalf("execute: %v", err) } } + +// #337 — filters are client-side over what GetNode already returns, so the row +// shape is unchanged and a filtered run is a subset of an unfiltered one. +func TestEdgeListFilters(t *testing.T) { + const detail = `{"id":"n1","memoryId":"mem1","loc":"preflight","name":"preflight", + "description":null,"abstract":null,"abstractOriginHash":null,"nodeType":"info","objectType":null, + "tags":[],"content":null,"data":null,"properties":null,"seq":null,"isRunnable":false, + "createdAt":"2026-08-01T00:00:00Z","updatedAt":"2026-08-01T00:00:00Z", + "outgoingEdges":[ + {"id":"e1","name":"routes-to","loc":"l1","isRunnable":false,"priority":0,"target":{"id":"t1","loc":"findings:race","memoryId":"mem1"}}, + {"id":"e2","name":"to diagnose a slow query","loc":"l2","isRunnable":false,"priority":0,"target":{"id":"t2","loc":"findings:slow","memoryId":"mem1"}}], + "incomingEdges":[ + {"id":"e3","name":"complements","loc":"l3","isRunnable":false,"priority":0,"source":{"id":"s1","loc":"instructions","memoryId":"mem1"}}]}` + + run := func(t *testing.T, extra ...string) []map[string]any { + t.Helper() + gql := fakeGraphQL(t, map[string]string{ + "ResolveUrn": resolveNodeJSON, + "GetNode": `{"data":{"node":` + detail + `}}`, + }) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs(append([]string{"edge", "list", nodeURN, "--json", "--server", gql.URL}, extra...)) + if err := root.Execute(); err != nil { + t.Fatalf("execute %v: %v", extra, err) + } + var rows []map[string]any + if err := json.Unmarshal([]byte(out.String()), &rows); err != nil { + t.Fatalf("--json must emit an array: %v (%q)", err, out.String()) + } + return rows + } + + edgeIDs := func(rows []map[string]any) []string { + out := []string{} + for _, r := range rows { + out = append(out, r["id"].(string)) + } + return out + } + + cases := []struct { + args []string + want string + }{ + {nil, "e1,e2,e3"}, + {[]string{"--direction", "outgoing"}, "e1,e2"}, + {[]string{"--direction", "incoming"}, "e3"}, + {[]string{"--name", "routes-to"}, "e1"}, + {[]string{"--to", "findings:slow"}, "e2"}, + {[]string{"--to", "t2"}, "e2"}, // by id + {[]string{"--from", "instructions"}, "e3"}, + {[]string{"--name", "no-such-label"}, ""}, + } + for _, tc := range cases { + got := strings.Join(edgeIDs(run(t, tc.args...)), ",") + if got != tc.want { + t.Errorf("%v: got %q, want %q", tc.args, got, tc.want) + } + } + + // The row shape is untouched by filtering. + full := run(t) + filtered := run(t, "--direction", "outgoing") + for k := range full[0] { + if _, ok := filtered[0][k]; !ok { + t.Errorf("filtering dropped the %q field from the row shape", k) + } + } +} + +// A contradictory combination fails up front rather than returning an empty +// list that reads like "no such edges". +func TestEdgeListRejectsContradictoryFilters(t *testing.T) { + for _, extra := range [][]string{ + {"--direction", "sideways"}, + {"--to", "x", "--from", "y"}, + {"--to", "x", "--direction", "incoming"}, + {"--from", "x", "--direction", "outgoing"}, + } { + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs(append([]string{"edge", "list", nodeURN, "--server", "http://127.0.0.1:1"}, extra...)) + if got := exitcode.FromError(root.Execute()); got != exitcode.Usage { + t.Errorf("%v should be a usage error, got %d", extra, got) + } + } +} From 4c2bef46e53aad22cbbbbd3a0c684a22e54dd7ee Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan Date: Sat, 1 Aug 2026 21:37:43 +0200 Subject: [PATCH 2/2] fix(edge): reject an explicitly-empty filter value; widen the surface line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on #340. An empty value alone can't be told from an absent flag, so `--to "$TARGET"` with TARGET unset reached the filter as "" and was treated as "no filter": the command returned every edge — 20 of 20 on the dev preflight router, identical to unfiltered — which is the opposite of what the caller asked for, and silent. validate now takes the set of flags cobra saw as Changed and rejects an explicitly-supplied empty (or whitespace-only) value on any of --direction/--name/--to/--from: hadron: --to was given an empty value — omit the flag to not filter on it (a shell variable that expanded to nothing?) Filter values are also trimmed on apply, so a ref padded by shell quoting still matches. agentic-usage.md's surface line named only ; `edge list` has taken ` -m ` all along and a bare since #339. That line is meant to be an accurate at-a-glance contract, so it now lists all three. validate takes a provided-set rather than the pflag FlagSet so the rule stays unit-testable without cobra. Co-Authored-By: Claude Opus 5 --- internal/cmd/agentic/agentic-usage.md | 2 +- internal/cmd/edge/ls.go | 26 +++++++++++--- internal/cmd/edge/ls_test.go | 49 +++++++++++++++++++++++++-- internal/cmd/edge_test.go | 22 ++++++++++++ 4 files changed, 92 insertions(+), 7 deletions(-) diff --git a/internal/cmd/agentic/agentic-usage.md b/internal/cmd/agentic/agentic-usage.md index c862cda..e66fb6e 100644 --- a/internal/cmd/agentic/agentic-usage.md +++ b/internal/cmd/agentic/agentic-usage.md @@ -73,7 +73,7 @@ hadron task run | -m [--arg k=v]... [--app [--as-s hadron chat read [--since ] [--node | -m --messages-loc ] | post (--body | --body-file ) [--node ] [--reply-to ] [--handle ] [--identity ] [--role ] hadron search [-m ]... [--mode hybrid|keyword|vector|regex] [--prefix ] [--type ] [--object-type ] [--tag ]... [--where ] [--sort-property ] [--limit N] [--offset N] [-l|--long] [--json] hadron replace text --field (--node | -m ) [--prefix ] [--regex] [-i] [--dry-run] [--yes] [--max-nodes N] -hadron edge list [--direction incoming|outgoing] [--name ] [--to ] [--from ] | add | update | rm +hadron edge list | -m | [--direction incoming|outgoing] [--name ] [--to ] [--from ] | add | update | rm hadron spec list [-m ] | get |--prefix | describe | use [] | register [--check] | find [--match-exactly] | grep [--regex] [-i] [--field content|abstract] [--prefix ] | replace [--regex] [--word-boundary=false] [--field content|abstract] [--dry-run] [--yes] [--max-specs N] | new ... | edit | extract --to-feature | link | lint [] | check-tools [--prefix ] | supersede | import spec-kit|code hadron coding review lint -m [--root ] [--toolchain |-] [--strict] [--suggest] [--fix [--yes]] [--json] | preflight lint -m [--root ] [--strict] [--json] hadron app list --org | install (--org | --owner-me) --agent --name [--type ] [--urn ] [--description ] | uninstall | use diff --git a/internal/cmd/edge/ls.go b/internal/cmd/edge/ls.go index b4574d5..b654b54 100644 --- a/internal/cmd/edge/ls.go +++ b/internal/cmd/edge/ls.go @@ -55,6 +55,7 @@ type edgeFilter struct { } func (fl edgeFilter) match(e edgeListDTO) bool { + fl.Name, fl.To, fl.From = strings.TrimSpace(fl.Name), strings.TrimSpace(fl.To), strings.TrimSpace(fl.From) if fl.Direction != "" && e.Direction != fl.Direction { return false } @@ -89,9 +90,22 @@ func filterEdges(edges []edgeListDTO, fl edgeFilter) []edgeListDTO { return out } -// validate rejects combinations that can only ever match nothing, rather than -// letting them return an empty list that reads like "no such edges". -func (fl edgeFilter) validate() error { +// validate rejects filters that can only ever match nothing, or that would +// silently widen the result instead of narrowing it. +// +// provided names the flags the user actually passed, which an empty value +// alone can't tell us: `--to "$TARGET"` with TARGET unset reaches us as an +// explicitly-supplied "". Treating that as "no filter" returned EVERY edge — +// the opposite of what the caller asked for, and silent. It is a usage error. +func (fl edgeFilter) validate(provided map[string]bool) error { + for flag, val := range map[string]string{ + "direction": fl.Direction, "name": fl.Name, "to": fl.To, "from": fl.From, + } { + if provided[flag] && strings.TrimSpace(val) == "" { + return exitcode.Newf(exitcode.Usage, + "--%s was given an empty value — omit the flag to not filter on it (a shell variable that expanded to nothing?)", flag) + } + } switch fl.Direction { case "", "incoming", "outgoing": default: @@ -132,7 +146,11 @@ far endpoint's loc or its id.`, hadron edge list preflight -m hadronmemory.com::dev --to findings:prisma-upsert-not-race-safe`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := fl.validate(); err != nil { + provided := map[string]bool{} + for _, n := range []string{"direction", "name", "to", "from"} { + provided[n] = cmd.Flags().Changed(n) + } + if err := fl.validate(provided); err != nil { return err } client, err := f.GraphQLClient() diff --git a/internal/cmd/edge/ls_test.go b/internal/cmd/edge/ls_test.go index cf17ba5..63c6eb4 100644 --- a/internal/cmd/edge/ls_test.go +++ b/internal/cmd/edge/ls_test.go @@ -110,7 +110,7 @@ func TestEdgeFilterValidate(t *testing.T) { {Name: "x", Direction: "incoming"}, } for _, fl := range ok { - if err := fl.validate(); err != nil { + if err := fl.validate(nil); err != nil { t.Errorf("%+v should be valid, got %v", fl, err) } } @@ -126,7 +126,7 @@ func TestEdgeFilterValidate(t *testing.T) { {edgeFilter{From: "x", Direction: "outgoing"}, "--from selects incoming"}, } for _, tc := range bad { - err := tc.fl.validate() + err := tc.fl.validate(nil) if err == nil { t.Errorf("%+v should be rejected", tc.fl) continue @@ -139,3 +139,48 @@ func TestEdgeFilterValidate(t *testing.T) { } } } + +// An explicitly-supplied empty value is a usage error, not "no filter". +// `--to "$TARGET"` with TARGET unset reaches us as "", and treating that as +// absent returned EVERY edge — the opposite of what the caller asked for. +func TestEmptyFilterValueIsRejected(t *testing.T) { + for _, flag := range []string{"direction", "name", "to", "from"} { + fl := edgeFilter{} + provided := map[string]bool{flag: true} + err := fl.validate(provided) + if err == nil { + t.Errorf("--%s \"\" should be rejected", flag) + continue + } + if got := exitcode.FromError(err); got != exitcode.Usage { + t.Errorf("--%s \"\": expected a usage error, got %d", flag, got) + } + if !strings.Contains(err.Error(), "--"+flag) { + t.Errorf("--%s: the error should name the flag, got %q", flag, err.Error()) + } + } + + // Whitespace-only counts as empty. + if err := (edgeFilter{To: " "}).validate(map[string]bool{"to": true}); err == nil { + t.Error(`--to " " should be rejected`) + } + // A flag NOT passed with an empty value is simply no filter. + if err := (edgeFilter{}).validate(map[string]bool{}); err != nil { + t.Errorf("an unset filter should be valid, got %v", err) + } + // A real value passed is fine. + if err := (edgeFilter{To: "findings:x"}).validate(map[string]bool{"to": true}); err != nil { + t.Errorf("a real --to should be valid, got %v", err) + } +} + +// Filter values are trimmed on apply, so a ref padded by shell quoting matches. +func TestFilterValuesAreTrimmed(t *testing.T) { + got := ids(filterEdges(sampleEdges(), edgeFilter{To: " findings:slow "})) + if len(got) != 1 || got[0] != "e2" { + t.Errorf("a padded --to should still match: %v", got) + } + if got := ids(filterEdges(sampleEdges(), edgeFilter{Name: " routes-to "})); len(got) != 2 { + t.Errorf("a padded --name should still match: %v", got) + } +} diff --git a/internal/cmd/edge_test.go b/internal/cmd/edge_test.go index c4d3f41..7c7801b 100644 --- a/internal/cmd/edge_test.go +++ b/internal/cmd/edge_test.go @@ -264,3 +264,25 @@ func TestEdgeListRejectsContradictoryFilters(t *testing.T) { } } } + +// End-to-end: an unset shell variable must fail loudly rather than returning +// every edge (#340 review). +func TestEdgeListRejectsEmptyFilterValue(t *testing.T) { + for _, extra := range [][]string{ + {"--to", ""}, + {"--from", ""}, + {"--name", ""}, + {"--direction", ""}, + } { + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs(append([]string{"edge", "list", nodeURN, "--server", "http://127.0.0.1:1"}, extra...)) + err := root.Execute() + if got := exitcode.FromError(err); got != exitcode.Usage { + t.Errorf("%v should be a usage error, got %d", extra, got) + } + if err != nil && !strings.Contains(err.Error(), "empty value") { + t.Errorf("%v: expected an empty-value message, got %q", extra, err.Error()) + } + } +}