Fix -pm <value> parsing and add comma-separated multi-fact -F (#14)

Two related query-ergonomics fixes surfaced while using the tool interactively.

## 1. `-pm <value>` failed with `unknown command "k8s"`

pflag does not attach a space-separated value to a string flag (`-m`) grouped with a bool flag (`-p`), so `k8s` was left as a stray positional. Only `-pm=k8s` or the un-grouped `-p -m k8s` worked.

- Allow one positional argument (`cobra.MaximumNArgs(1)`) and fall back to it for the match value when `-m` is empty (`matchValue()`). `-pm/-im/-ipm <value>` and a bare `-p <value>` now all work; `-m` still wins when both are given.

## 2. `-F a,b` (multiple facts) returned `{}`

`-jF ipaddress,enc_role` queried a single fact literally named `ipaddress,enc_role`.

- Split `-F` on commas (`splitFactNames`) and match any of them via an `or` over `["=","name",<n>]` clauses (`nameFilter`); a single name keeps the plain `=` form.
- Key JSON output by each result's real fact name so all requested facts appear under the host.

Both paths have unit tests; verified live: `node-lookup -R -pm externaldns | node-lookup -jF ipaddress,enc_role` returns both facts per host. AGENTS.md updated.

Reviewed-on: #14
Co-authored-by: Ben Vincent <ben@unkin.net>
Co-committed-by: Ben Vincent <ben@unkin.net>

.woodpecker/build.yaml

@@ -0,0 +1,18 @@
+when:
+  - event: pull_request
+
+steps:
+  - name: build
+    image: golang:1.25
+    commands:
+      - make build
+    backend_options:
+      kubernetes:
+        serviceAccountName: default
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2

.woodpecker/lint.yaml

@@ -1,8 +0,0 @@
-when:
-  - event: pull_request
-
-steps:
-  - name: lint
-    image: golangci/golangci-lint:latest
-    commands:
-      - golangci-lint run ./...

.woodpecker/pre-commit.yaml

@@ -3,6 +3,16 @@ when:
 
 steps:
   - name: pre-commit
-    image: git.unkin.net/unkin/almalinux9-gobuilder:20260325
+    image: git.unkin.net/unkin/almalinux9-gobuilder:20260606
     commands:
       - uvx pre-commit run --all-files
+    backend_options:
+      kubernetes:
+        serviceAccountName: default
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2

.woodpecker/release.yaml

@@ -1,49 +1,127 @@
 when:
-  - event: release
+  - event: tag
 
 steps:
   - name: test
-    image: golang:latest
+    image: golang:1.25
     commands:
-      - go test ./...
+      - go test -race ./...
+    backend_options:
+      kubernetes:
+        serviceAccountName: default
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2
 
+  # Build the linux/amd64 binary into dist/ (consumed by the RPM step) plus the
+  # cross-platform binaries attached to the Gitea release.
   - name: build
-    image: golang:latest
+    image: git.unkin.net/unkin/almalinux9-gobuilder:20260606
     commands:
-      - VERSION=${CI_COMMIT_TAG}
-      - go build -ldflags="-s -w -X main.version=${VERSION}" -o node-lookup ./...
+      - make build VERSION=${CI_COMMIT_TAG}
+      - GOOS=linux  GOARCH=amd64 go build -ldflags="-s -w -X main.version=${CI_COMMIT_TAG}" -o node-lookup-linux-amd64 ./...
+      - GOOS=linux  GOARCH=arm64 go build -ldflags="-s -w -X main.version=${CI_COMMIT_TAG}" -o node-lookup-linux-arm64 ./...
+      - GOOS=darwin GOARCH=amd64 go build -ldflags="-s -w -X main.version=${CI_COMMIT_TAG}" -o node-lookup-darwin-amd64 ./...
+      - GOOS=darwin GOARCH=arm64 go build -ldflags="-s -w -X main.version=${CI_COMMIT_TAG}" -o node-lookup-darwin-arm64 ./...
     depends_on: [test]
+    backend_options:
+      kubernetes:
+        serviceAccountName: default
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2
 
+  # Package the built binary + generated shell completions into an RPM.
+  - name: package
+    image: git.unkin.net/unkin/almalinux9-rpmbuilder:latest
+    commands:
+      - ./scripts/build-rpm.sh ${CI_COMMIT_TAG}
+    depends_on: [build]
+    backend_options:
+      kubernetes:
+        serviceAccountName: default
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2
+
+  # Publish the RPM to the artifactapi local rpm repo (a real yum repo;
+  # repodata regenerates automatically).
+  - name: upload-rpm
+    image: git.unkin.net/unkin/almalinux9-base:20260606
+    commands:
+      - |
+        HOST="https://artifactapi.k8s.syd1.au.unkin.net"
+        REPO="rpm-internal"
+        for rpm in dist/*.rpm; do
+          FILE=$$(basename "$$rpm")
+          # artifactapi has no HEAD route (returns 405); probe with GET against
+          # the served path (RPMs are stored under Packages/) to avoid re-upload.
+          code=$$(curl -s -o /dev/null -w '%{http_code}' "$$HOST/api/v2/remotes/$$REPO/files/Packages/$$FILE" || true)
+          if [ "$$code" = "200" ]; then
+            echo "$$FILE already exists in $$REPO (HTTP $$code); skipping upload"
+            continue
+          fi
+          echo "Uploading $$FILE to $$REPO (existence probe returned $$code)"
+          curl -f -X PUT \
+            "$$HOST/api/v2/remotes/$$REPO/files/$$FILE" \
+            -H "Content-Type: application/x-rpm" \
+            --data-binary @"$$rpm"
+        done
+    depends_on: [package]
+    backend_options:
+      kubernetes:
+        serviceAccountName: default
+        resources:
+          requests:
+            memory: 128Mi
+            cpu: 100m
+          limits:
+            memory: 512Mi
+            cpu: 500m
+
+  # Cut a Gitea release with the cross-platform binaries attached.
   - name: release
-    image: git.unkin.net/unkin/almalinux9-base:20260325
+    image: git.unkin.net/unkin/almalinux9-base:20260606
     environment:
       RELEASER_TOKEN:
         from_secret: RELEASER_TOKEN
     commands:
       - |
+        curl --output /usr/local/bin/tea https://artifactapi.k8s.syd1.au.unkin.net/api/v1/remote/gitea-dl/tea/0.12.0/tea-0.12.0-linux-amd64 && chmod +x /usr/local/bin/tea
+        tea logins add --name gitea --url https://git.unkin.net --token "$${RELEASER_TOKEN}" --no-version-check
         PREV_TAG=$(git describe --tags --abbrev=0 HEAD^ 2>/dev/null || echo "")
         if [ -n "$PREV_TAG" ]; then
-          NOTES=$(git log "${PREV_TAG}..${CI_COMMIT_TAG}" --merges --pretty=format:"- %s")
+          NOTES=$(git log "${PREV_TAG}..${CI_COMMIT_TAG}" --pretty=format:"- %s")
         else
-          NOTES=$(git log --merges --pretty=format:"- %s")
+          NOTES=$(git log --pretty=format:"- %s")
         fi
-        BODY=$(printf '%s' "$NOTES" | sed 's/"/\\"/g; s/$/\\n/' | tr -d '\n')
-        GET_RESPONSE=$(curl -sk "https://git.unkin.net/api/v1/repos/${CI_REPO}/releases/tags/${CI_COMMIT_TAG}")
-        echo "GET response: ${GET_RESPONSE}"
-        RELEASE_ID=$(printf '%s' "${GET_RESPONSE}" | grep -o '"id":[0-9]*' | head -1 | cut -d: -f2)
-        if [ -z "$RELEASE_ID" ]; then
-          echo "ERROR: failed to find release for tag ${CI_COMMIT_TAG}" >&2
-          exit 1
-        fi
-        echo "Release ID: ${RELEASE_ID}"
-        curl -sk -X PATCH "https://git.unkin.net/api/v1/repos/${CI_REPO}/releases/${RELEASE_ID}" \
-          -H "Authorization: token ${RELEASER_TOKEN}" \
-          -H "Content-Type: application/json" \
-          -d "{\"body\":\"${BODY}\"}"
-        curl -sk -X POST "https://git.unkin.net/api/v1/repos/${CI_REPO}/releases/${RELEASE_ID}/assets" \
-          -H "Authorization: token ${RELEASER_TOKEN}" \
-          -F "attachment=@node-lookup"
+        tea releases create --tag "${CI_COMMIT_TAG}" --title "${CI_COMMIT_TAG}" --note "${NOTES}" --login gitea --repo "${CI_REPO}"
+        tea releases assets create "${CI_COMMIT_TAG}" \
+          node-lookup-linux-amd64 \
+          node-lookup-linux-arm64 \
+          node-lookup-darwin-amd64 \
+          node-lookup-darwin-arm64 \
+          --login gitea --repo "${CI_REPO}"
+    depends_on: [upload-rpm]
     backend_options:
       kubernetes:
         serviceAccountName: default
-    depends_on: [build]
+        resources:
+          requests:
+            memory: 128Mi
+            cpu: 100m
+          limits:
+            memory: 512Mi
+            cpu: 500m

.woodpecker/test.yaml

@@ -0,0 +1,33 @@
+when:
+  - event: pull_request
+
+steps:
+  - name: lint
+    image: golangci/golangci-lint:latest
+    commands:
+      - golangci-lint run ./...
+    backend_options:
+      kubernetes:
+        serviceAccountName: default
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2
+
+  - name: test
+    image: golang:1.25
+    commands:
+      - go test -v -race ./...
+    backend_options:
+      kubernetes:
+        serviceAccountName: default
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2

.woodpecker/unit-tests.yaml

@@ -1,8 +0,0 @@
-when:
-  - event: pull_request
-
-steps:
-  - name: unit-tests
-    image: golang:latest
-    commands:
-      - go test -v -race ./...

AGENTS.md

@@ -7,20 +7,51 @@
 ## Structure
 
 ```
-main.go        # entire application source
-go.mod         # Go module (module name: node-lookup)
-go.sum         # dependency checksums
-node-lookup    # compiled binary (not committed)
+main.go             # entire application source
+main_test.go        # unit tests (mock PuppetDB via httptest, no live deps)
+go.mod              # Go module (module name: node-lookup)
+go.sum              # dependency checksums
+Makefile            # build / test / lint / completions / rpm / version-bump targets
+packaging/nfpm.yaml # nfpm spec (envsubst-templated) for the RPM
+scripts/build-rpm.sh # generates completions + packages the RPM with nfpm
+.woodpecker/        # CI: build, test, pre-commit (PR) + release (tag)
+dist/               # build output: binary, completions, RPM (not committed)
 ```
 
 ## Build
 
 ```bash
+make build          # -> dist/node-lookup (CGO disabled, static)
+# or directly:
 go build -o node-lookup ./...
 ```
 
 Requires Go 1.21+. Dependencies: `github.com/spf13/cobra` (CLI), `gopkg.in/yaml.v3` (Ansible output).
 
+## Packaging (RPM)
+
+```bash
+make rpm            # build the binary + package it into dist/*.rpm via nfpm
+```
+
+`scripts/build-rpm.sh` generates bash/zsh/fish completions from the built binary
+and bundles them alongside `/usr/bin/node-lookup`. On a `v*` tag the release
+pipeline builds the RPM and `PUT`s it to the artifactapi `rpm-internal` repo.
+
+## Shell completions
+
+Cobra provides a `completion` subcommand:
+
+```bash
+node-lookup completion bash   # or zsh / fish / powershell
+```
+
+The RPM installs completions to the standard system paths
+(`/usr/share/bash-completion/completions/`, `/usr/share/zsh/site-functions/`,
+`/usr/share/fish/vendor_completions.d/`), so they work automatically once
+installed. To load ad-hoc in the current shell, e.g. zsh:
+`source <(node-lookup completion zsh)`.
+
 ## Running the Tool
 
 ```bash
@@ -28,12 +59,16 @@ Requires Go 1.21+. Dependencies: `github.com/spf13/cobra` (CLI), `gopkg.in/yaml.
 ./node-lookup -R                          # show all nodes with role fact
 ./node-lookup -n <hostname>               # lookup a specific node
 ./node-lookup -F <fact_name>              # filter by fact name
-./node-lookup -m <value>                  # exact value match
-./node-lookup --pm <pattern>              # partial/regex match on value
+./node-lookup -jF ipaddress,enc_role      # several facts at once (comma-separated)
+./node-lookup -R -m <value>               # exact value match (-m)
+./node-lookup -R -pm <value>              # partial/regex match (-p -m combined)
+./node-lookup -R -im <value>              # inverse exact match (-i -m combined)
+./node-lookup -R -ipm <value>             # inverse partial match (-i -p -m combined)
+./node-lookup -R -p <value>               # value may also be given positionally
 ./node-lookup -R -1                       # node names only
 ./node-lookup -R -2                       # values only
 ./node-lookup -R -C                       # count occurrences
-./node-lookup -R -A                       # output as Ansible YAML inventory
+./node-lookup -R -A                       # output as Ansible YAML inventory (queried facts become host vars)
 ./node-lookup -j                          # output as JSON { host → { fact → value } }
 ./node-lookup --url http://host:8080/...  # override PuppetDB URL for this invocation
 echo -e "node1\nnode2" | ./node-lookup -R # pipe node names via stdin
@@ -76,11 +111,13 @@ Show the active configuration (after all overrides applied):
 ## Code Patterns
 
 - **`loadConfig()`**: reads config file → applies env vars → returns `config` struct. Called once at startup in `main()`.
-- **`buildQuery()`**: returns a PuppetDB PQL-compatible JSON array string. Uses `roleFact` from config (not hardcoded).
+- **`buildQuery()`**: returns a PuppetDB PQL-compatible JSON array string. Uses `roleFact` from config (not hardcoded). Match modifiers: `-p` (partial/regex, uses `~` op), `-i` (inverse, wraps with `not`), composable.
+- **Multiple facts**: `-F` accepts a comma-separated list (`ipaddress,enc_role`). `splitFactNames()`/`nameFilter()` turn several names into an `or` over `["=","name",<n>]` clauses; JSON output keys each value by the fact's real name so all requested facts appear per host.
+- **Match value / `matchValue()`**: the value to match comes from `-m/--match` or, if that is empty, an optional positional argument. The positional fallback exists because pflag does not attach a space-separated value to a string flag grouped with a bool flag, so in `-pm k8s` the `k8s` arrives as a positional. `-m` still wins when both are given.
 - **`queryPuppetDB(url, query)`**: takes the URL as a parameter — never reads globals.
 - **`processResults()`**: iterates facts, returns sorted `"certname value"` strings. JSON string values are unquoted; other JSON types rendered as compact JSON.
-- **Output modes**: JSON (`-j`), count (`-C`), Ansible YAML (`-A`), node-only (`-1`), value-only (`-2`), default (node + value).
-- **Stdin support**: when stdin is not a TTY and no `-n` is given, node names are read line-by-line and queried individually (one HTTP request per node).
+- **Output modes**: JSON (`-j`), count (`-C`), Ansible YAML (`-A`), node-only (`-1`), value-only (`-2`), default (node + value). `-j` and `-A` share `factsByHost()`, so both attach the queried fact(s) per host — as an object under the host (`-j`) or as inventory host vars (`-A`).
+- **Stdin support**: `stdinReader()` reads node names from stdin only when it is a real pipe/redirect carrying data (and no `-n` given). Terminals, `/dev/null`, and empty/closed pipes fall through to a normal query — so running without a TTY (e.g. invoked by an agent or CI) behaves like an interactive run instead of consuming empty input.
 - **SIGPIPE handling**: `signal.Ignore(syscall.SIGPIPE)` so pipes to `head` etc. work cleanly.
 
 ## CLI Framework
@@ -89,11 +126,19 @@ Uses [Cobra](https://github.com/spf13/cobra). Root command is the query command.
 
 ## Testing
 
-No test suite exists. Manual testing requires access to the Consul/PuppetDB environment or a mock HTTP server.
+```bash
+make test           # go test -v -race ./...
+```
+
+`main_test.go` covers query construction (all `-m`/`-p`/`-i` combinations), value
+rendering, result processing/counting, config precedence (defaults < file < env),
+`writeDefaultConfig`, the `stdinReader` no-TTY behavior, and every `run()` output
+mode (default, `-1`, `-2`, `-C`, `-j`, `-A`, `-a`). PuppetDB is stubbed with
+`httptest` — no live Consul/PuppetDB access is required.
 
 ## Gotchas
 
 - `-1`, `-2`, `-C`, and `-A` all require `-R` or `-F`; the tool exits with an error otherwise.
 - `-C` (count) with stdin reads all lines as pre-fetched `"node value"` output for counting — it does **not** query PuppetDB per line.
-- JSON output (`-j`) builds `{ hostname: { factname: value } }` where the fact key is the `-F` value, the `role_fact` config value (if `-R`), or `"value"` as fallback.
+- JSON output (`-j`) builds `{ hostname: { factname: value } }` keyed by each result's actual fact name (so `-F ipaddress,enc_role` yields both per host); it falls back to the `-F` value, the `role_fact` config value (if `-R`), or `"value"` only when a result carries no name.
 - `config init` fails if the config file already exists (will not overwrite).

Makefile

@@ -1,13 +1,17 @@
-BINARY   := node-lookup
-VERSION  := $(shell git describe --tags --always --dirty 2>/dev/null || echo dev)
-GOFLAGS  := -ldflags="-s -w -X main.version=$(VERSION)"
+BINARY  := node-lookup
+DIST    := dist
+VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo dev)
+GOFLAGS := -ldflags="-s -w -X main.version=$(VERSION)"
+OS      ?= $(shell go env GOOS)
+ARCH    ?= $(shell go env GOARCH)
 
-.PHONY: all build test lint clean install patch minor major _release
+.PHONY: all build test lint fmt clean install completions rpm rpm-package patch minor major _tag
 
 all: build
 
+# Build into dist/ so the nfpm packaging step (scripts/build-rpm.sh) can find it.
 build:
-	go build $(GOFLAGS) -o $(BINARY) ./...
+	CGO_ENABLED=0 GOOS=$(OS) GOARCH=$(ARCH) go build $(GOFLAGS) -o $(DIST)/$(BINARY) ./...
 
 test:
 	go test -v -race ./...
@@ -15,12 +19,29 @@ test:
 lint:
 	golangci-lint run ./...
 
+fmt:
+	gofmt -w .
+
 clean:
-	rm -f $(BINARY)
+	rm -rf $(DIST) $(BINARY)
 
 install:
 	go install $(GOFLAGS) ./...
 
+# Generate bash/zsh/fish completions from the built binary into dist/completions.
+completions: build
+	@mkdir -p $(DIST)/completions
+	$(DIST)/$(BINARY) completion bash > $(DIST)/completions/$(BINARY).bash
+	$(DIST)/$(BINARY) completion zsh  > $(DIST)/completions/_$(BINARY)
+	$(DIST)/$(BINARY) completion fish > $(DIST)/completions/$(BINARY).fish
+
+# Build the binary then package it (with completions) into an RPM via nfpm.
+rpm: build rpm-package
+
+# Package an already-built binary into an RPM (used by CI after the build step).
+rpm-package:
+	./scripts/build-rpm.sh $(VERSION)
+
 # Bump helpers — reads the latest semver tag and creates the next one.
 # If no tag exists yet, starts from v0.0.0.
 _LATEST := $(shell git tag --sort=-v:refname | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$$' | head -1)
@@ -31,16 +52,15 @@ _PAT    := $(shell echo $(_BASE) | sed 's/^v//' | cut -d. -f3)
 
 patch:
 	@NEW=v$(_MAJ).$(_MIN).$(shell expr $(_PAT) + 1); \
-	git tag $$NEW && echo "Tagged $$NEW" && $(MAKE) _release TAG=$$NEW
+	git tag $$NEW && echo "Tagged $$NEW" && $(MAKE) _tag TAG=$$NEW
 
 minor:
 	@NEW=v$(_MAJ).$(shell expr $(_MIN) + 1).0; \
-	git tag $$NEW && echo "Tagged $$NEW" && $(MAKE) _release TAG=$$NEW
+	git tag $$NEW && echo "Tagged $$NEW" && $(MAKE) _tag TAG=$$NEW
 
 major:
 	@NEW=v$(shell expr $(_MAJ) + 1).0.0; \
-	git tag $$NEW && echo "Tagged $$NEW" && $(MAKE) _release TAG=$$NEW
+	git tag $$NEW && echo "Tagged $$NEW" && $(MAKE) _tag TAG=$$NEW
 
-_release:
+_tag:
 	git push origin $(TAG)
-	tea releases create --tag $(TAG) --title $(TAG)

main.go

@@ -110,23 +110,55 @@ type fact struct {
 	Value    json.RawMessage `json:"value"`
 }
 
-func buildQuery(node, factName, match, partialMatch, roleFact string, showRole bool) string {
+// splitFactNames splits a comma-separated -F value into trimmed, non-empty
+// names, so `-F ipaddress,enc_role` queries both facts.
+func splitFactNames(factName string) []string {
+	var names []string
+	for _, n := range strings.Split(factName, ",") {
+		if n = strings.TrimSpace(n); n != "" {
+			names = append(names, n)
+		}
+	}
+	return names
+}
+
+// nameFilter returns a PQL filter matching any of the given fact names: a plain
+// equality for one name, an "or" over per-name equalities for several.
+func nameFilter(names []string) []interface{} {
+	if len(names) == 1 {
+		return []interface{}{"=", "name", names[0]}
+	}
+	or := []interface{}{"or"}
+	for _, n := range names {
+		or = append(or, []interface{}{"=", "name", n})
+	}
+	return or
+}
+
+func buildQuery(node, factName, match, roleFact string, showRole, partial, inverse bool) string {
 	type filter = []interface{}
 	var filters []filter
 
 	if node != "" {
 		filters = append(filters, filter{"=", "certname", node})
 	}
-	if factName != "" {
-		filters = append(filters, filter{"=", "name", factName})
+	if names := splitFactNames(factName); len(names) > 0 {
+		filters = append(filters, nameFilter(names))
 	} else if showRole {
 		filters = append(filters, filter{"=", "name", roleFact})
 	}
 
 	if match != "" {
-		filters = append(filters, filter{"=", "value", match})
-	} else if partialMatch != "" {
-		filters = append(filters, filter{"~", "value", partialMatch})
+		op := "="
+		if partial {
+			op = "~"
+		}
+		inner := filter{op, "value", match}
+		if inverse {
+			filters = append(filters, filter{"not", inner})
+		} else {
+			filters = append(filters, inner)
+		}
 	}
 
 	if len(filters) == 0 {
@@ -207,12 +239,66 @@ func countResults(lines []string) []string {
 	return out
 }
 
-func isTerminal(f *os.File) bool {
+// stdinReader returns a buffered reader over f and true only when f actually
+// carries piped/redirected data to consume as node names. Terminals and
+// character devices such as /dev/null return false, and an empty pipe or empty
+// file (immediate EOF on peek) also returns false. This means running without a
+// TTY — e.g. invoked by an agent or CI where stdin is /dev/null or a closed
+// pipe — falls through to a normal query instead of silently consuming empty
+// input and printing nothing.
+func stdinReader(f *os.File) (*bufio.Reader, bool) {
 	fi, err := f.Stat()
 	if err != nil {
-		return false
+		return nil, false
 	}
-	return (fi.Mode() & os.ModeCharDevice) != 0
+	if (fi.Mode() & os.ModeCharDevice) != 0 {
+		return nil, false // terminal or /dev/null
+	}
+	r := bufio.NewReader(f)
+	if _, err := r.Peek(1); err != nil {
+		return nil, false // empty pipe / empty file (EOF)
+	}
+	return r, true
+}
+
+// matchValue resolves the value to match against. The -m/--match flag wins; if
+// it is empty, the (optional) positional argument is used instead. The
+// positional fallback exists so combined shorthands like `-pm k8s` work — pflag
+// leaves the space-separated `k8s` as a positional rather than attaching it to
+// the grouped -m flag.
+func matchValue(flagMatch string, args []string) string {
+	if flagMatch != "" {
+		return flagMatch
+	}
+	if len(args) > 0 {
+		return args[0]
+	}
+	return ""
+}
+
+// factsByHost groups collected facts into {certname: {factname: value}}. Each
+// value is keyed by the fact's real name, so multiple -F facts each appear
+// under the host; it falls back to the -F string, the role fact (with -R), or
+// "value" only when a result carries no name. Shared by the -j and -A outputs.
+func factsByHost(collected []fact, factName, roleFact string, showRole bool) map[string]map[string]interface{} {
+	out := map[string]map[string]interface{}{}
+	for _, f := range collected {
+		if _, ok := out[f.Certname]; !ok {
+			out[f.Certname] = map[string]interface{}{}
+		}
+		key := f.Name
+		if key == "" {
+			if key = factName; key == "" {
+				if showRole {
+					key = roleFact
+				} else {
+					key = "value"
+				}
+			}
+		}
+		out[f.Certname][key] = valueAny(f.Value)
+	}
+	return out
 }
 
 func allFactsForNode(puppetDBURL, node string) ([]fact, error) {
@@ -220,7 +306,7 @@ func allFactsForNode(puppetDBURL, node string) ([]fact, error) {
 	return queryPuppetDB(puppetDBURL, string(query))
 }
 
-func run(cfg config, nodeName, factName, match, partialMatch string, showRole, nodeOnly, valueOnly, count, ansible, jsonMode, allFacts bool) error {
+func run(cfg config, nodeName, factName, match string, showRole, partial, inverse, nodeOnly, valueOnly, count, ansible, jsonMode, allFacts bool) error {
 	signal.Ignore(syscall.SIGPIPE)
 
 	if allFacts {
@@ -241,12 +327,15 @@ func run(cfg config, nodeName, factName, match, partialMatch string, showRole, n
 	if (nodeOnly || valueOnly || count || ansible) && !showRole && factName == "" {
 		return fmt.Errorf("-R or -F must be used with -1, -2, -C, or -A")
 	}
+	if (match != "" || partial || inverse) && !showRole && factName == "" {
+		return fmt.Errorf("-R or -F must be used with -m, -p, or -i")
+	}
 
 	var collected []fact
 	var stdinLines []string
 
 	doQuery := func(node string) error {
-		query := buildQuery(node, factName, match, partialMatch, cfg.RoleFact, showRole)
+		query := buildQuery(node, factName, match, cfg.RoleFact, showRole, partial, inverse)
 		facts, err := queryPuppetDB(cfg.PuppetDBURL, query)
 		if err != nil {
 			return err
@@ -255,8 +344,8 @@ func run(cfg config, nodeName, factName, match, partialMatch string, showRole, n
 		return nil
 	}
 
-	if nodeName == "" && !isTerminal(os.Stdin) {
-		scanner := bufio.NewScanner(os.Stdin)
+	if reader, ok := stdinReader(os.Stdin); ok && nodeName == "" {
+		scanner := bufio.NewScanner(reader)
 		if count {
 			for scanner.Scan() {
 				stdinLines = append(stdinLines, scanner.Text())
@@ -282,25 +371,10 @@ func run(cfg config, nodeName, factName, match, partialMatch string, showRole, n
 
 	switch {
 	case jsonMode:
-		hostFactMap := map[string]map[string]interface{}{}
-		for _, f := range collected {
-			if _, ok := hostFactMap[f.Certname]; !ok {
-				hostFactMap[f.Certname] = map[string]interface{}{}
-			}
-			key := factName
-			if key == "" {
-				if showRole {
-					key = cfg.RoleFact
-				} else {
-					key = "value"
-				}
-			}
-			hostFactMap[f.Certname][key] = valueAny(f.Value)
-		}
 		enc := json.NewEncoder(os.Stdout)
 		enc.SetIndent("", "  ")
 		enc.SetEscapeHTML(false)
-		_ = enc.Encode(hostFactMap)
+		_ = enc.Encode(factsByHost(collected, factName, cfg.RoleFact, showRole))
 
 	case count:
 		values := stdinLines
@@ -310,10 +384,11 @@ func run(cfg config, nodeName, factName, match, partialMatch string, showRole, n
 		fmt.Println(strings.Join(countResults(values), "\n"))
 
 	case ansible:
+		// Attach each host's queried fact(s) as inventory host vars, e.g.
+		// `-F ipaddress,enc_role -A` yields hosts with ipaddress + enc_role set.
 		hosts := map[string]interface{}{}
-		for _, line := range returnData {
-			host := strings.Fields(line)[0]
-			hosts[host] = map[string]interface{}{}
+		for host, vars := range factsByHost(collected, factName, cfg.RoleFact, showRole) {
+			hosts[host] = vars
 		}
 		inventory := map[string]interface{}{
 			"all": map[string]interface{}{"hosts": hosts},
@@ -349,23 +424,30 @@ func main() {
 	}
 
 	var (
-		nodeName     string
-		factName     string
-		showRole     bool
-		match        string
-		partialMatch string
-		nodeOnly     bool
-		valueOnly    bool
-		count        bool
-		ansible      bool
-		jsonMode     bool
-		allFacts     bool
-		puppetDBURL  string
+		nodeName    string
+		factName    string
+		showRole    bool
+		match       string
+		partial     bool
+		inverse     bool
+		nodeOnly    bool
+		valueOnly   bool
+		count       bool
+		ansible     bool
+		jsonMode    bool
+		allFacts    bool
+		puppetDBURL string
 	)
 
 	rootCmd := &cobra.Command{
-		Use:   appName,
+		Use:   appName + " [value]",
 		Short: "Query PuppetDB for nodes.",
+		// Accept an optional positional match value in addition to -m. This makes
+		// combined shorthands like `-pm k8s` work: pflag does not attach a
+		// space-separated value to a string flag grouped with a bool flag (only
+		// `-pm=k8s` or `-p -m k8s` do), so `k8s` arrives here as a positional
+		// argument instead. Falling back to it keeps the ergonomic form working.
+		Args: cobra.MaximumNArgs(1),
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 			if cmd.Flags().Changed("url") {
 				cfg.PuppetDBURL = puppetDBURL
@@ -373,17 +455,18 @@ func main() {
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return run(cfg, nodeName, factName, match, partialMatch, showRole, nodeOnly, valueOnly, count, ansible, jsonMode, allFacts)
+			return run(cfg, nodeName, factName, matchValue(match, args), showRole, partial, inverse, nodeOnly, valueOnly, count, ansible, jsonMode, allFacts)
 		},
 		SilenceUsage: true,
 	}
 
 	f := rootCmd.Flags()
 	f.StringVarP(&nodeName, "node", "n", "", "Node name")
-	f.StringVarP(&factName, "fact", "F", "", "Fact name")
+	f.StringVarP(&factName, "fact", "F", "", "Fact name (comma-separated for several, e.g. -F ipaddress,enc_role)")
 	f.BoolVarP(&showRole, "role", "R", false, "Show role fact ("+defaultRoleFact+" by default)")
-	f.StringVarP(&match, "match", "m", "", "Exact value match")
-	f.StringVar(&partialMatch, "pm", "", "Partial match on value")
+	f.StringVarP(&match, "match", "m", "", "Value to match (use with -p and/or -i)")
+	f.BoolVarP(&partial, "partial", "p", false, "Partial/regex match modifier (combine with -m)")
+	f.BoolVarP(&inverse, "inverse", "i", false, "Inverse match modifier (combine with -m)")
 	f.BoolVarP(&nodeOnly, "nodeonly", "1", false, "Show only the node name")
 	f.BoolVarP(&valueOnly, "valueonly", "2", false, "Show only the value")
 	f.BoolVarP(&count, "count", "C", false, "Count fact occurrences")

main_test.go

@@ -9,6 +9,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"gopkg.in/yaml.v3"
 )
 
 // ---- helpers ----------------------------------------------------------------
@@ -26,74 +28,256 @@ func rawJSON(v interface{}) json.RawMessage {
 	return json.RawMessage(b)
 }
 
+// captureStdout redirects os.Stdout for the duration of fn and returns whatever
+// was written. run() writes directly to os.Stdout, so the output modes are
+// exercised end-to-end this way.
+func captureStdout(t *testing.T, fn func()) string {
+	t.Helper()
+	old := os.Stdout
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	os.Stdout = w
+	defer func() { os.Stdout = old }()
+
+	fn()
+
+	_ = w.Close()
+	var buf strings.Builder
+	_, _ = io.Copy(&buf, r)
+	return buf.String()
+}
+
 // ---- buildQuery -------------------------------------------------------------
 
 func TestBuildQuery_NoFilters(t *testing.T) {
-	q := buildQuery("", "", "", "", "enc_role", false)
-	if !strings.Contains(q, "enc_role") {
+	// With no node/fact/match, buildQuery falls back to a role-fact query.
+	q := buildQuery("", "", "", "enc_role", false, false, false)
+	if !strings.Contains(q, "enc_role") || !strings.Contains(q, "name") {
 		t.Fatalf("expected default role query, got %s", q)
 	}
 }
 
 func TestBuildQuery_Node(t *testing.T) {
-	q := buildQuery("host1", "", "", "", "enc_role", false)
+	q := buildQuery("host1", "", "", "enc_role", false, false, false)
 	if !strings.Contains(q, "certname") || !strings.Contains(q, "host1") {
 		t.Fatalf("unexpected query: %s", q)
 	}
 }
 
 func TestBuildQuery_FactAndMatch(t *testing.T) {
-	q := buildQuery("", "region", "syd1", "", "enc_role", false)
+	q := buildQuery("", "region", "syd1", "enc_role", false, false, false)
 	if !strings.Contains(q, "region") || !strings.Contains(q, "syd1") {
 		t.Fatalf("unexpected query: %s", q)
 	}
+	// exact match uses the "=" operator, never "~".
+	if strings.Contains(q, `"~"`) {
+		t.Fatalf("exact match should not use ~ operator: %s", q)
+	}
 }
 
 func TestBuildQuery_PartialMatch(t *testing.T) {
-	q := buildQuery("", "enc_role", "", "dns", "enc_role", false)
-	if !strings.Contains(q, "~") || !strings.Contains(q, "dns") {
-		t.Fatalf("expected partial match query, got %s", q)
+	// -p turns the value match into a regex ("~") comparison.
+	q := buildQuery("", "enc_role", "dns", "enc_role", false, true, false)
+	if !strings.Contains(q, `"~"`) || !strings.Contains(q, "dns") {
+		t.Fatalf("expected partial (~) match query, got %s", q)
+	}
+}
+
+func TestBuildQuery_InverseMatch(t *testing.T) {
+	// -i wraps the value comparison in a "not".
+	q := buildQuery("", "enc_role", "dns", "enc_role", false, false, true)
+	if !strings.Contains(q, `"not"`) {
+		t.Fatalf("expected inverse (not) match query, got %s", q)
+	}
+}
+
+func TestBuildQuery_InversePartialMatch(t *testing.T) {
+	// -i and -p compose: a negated regex match.
+	q := buildQuery("", "enc_role", "dns", "enc_role", false, true, true)
+	if !strings.Contains(q, `"not"`) || !strings.Contains(q, `"~"`) {
+		t.Fatalf("expected inverse partial match query, got %s", q)
+	}
+	// The regex operator must sit inside the "not" wrapper, not beside it.
+	notIdx := strings.Index(q, `"not"`)
+	tildeIdx := strings.Index(q, `"~"`)
+	if notIdx < 0 || tildeIdx < 0 || tildeIdx < notIdx {
+		t.Fatalf("expected ~ nested inside not, got %s", q)
 	}
 }
 
 func TestBuildQuery_ShowRole(t *testing.T) {
-	q := buildQuery("", "", "", "", "my_role_fact", true)
+	q := buildQuery("", "", "", "my_role_fact", true, false, false)
 	if !strings.Contains(q, "my_role_fact") {
 		t.Fatalf("expected role fact in query, got %s", q)
 	}
 }
 
 func TestBuildQuery_CustomRoleFact(t *testing.T) {
-	q := buildQuery("", "", "", "", "custom_role", true)
+	q := buildQuery("", "", "", "custom_role", true, false, false)
 	if !strings.Contains(q, "custom_role") {
 		t.Fatalf("expected custom role fact, got %s", q)
 	}
 }
 
-// ---- valueString ------------------------------------------------------------
+func TestBuildQuery_FactBeatsRole(t *testing.T) {
+	// An explicit -F fact name takes precedence over the role fact.
+	q := buildQuery("", "region", "", "enc_role", true, false, false)
+	if !strings.Contains(q, "region") {
+		t.Fatalf("expected explicit fact name, got %s", q)
+	}
+	if strings.Contains(q, "enc_role") {
+		t.Fatalf("role fact should not appear when -F is set: %s", q)
+	}
+}
+
+func TestBuildQuery_ValidJSON(t *testing.T) {
+	// Whatever the flag combination, the query must be a valid JSON array.
+	q := buildQuery("host1", "region", "syd1", "enc_role", false, true, true)
+	var v []interface{}
+	if err := json.Unmarshal([]byte(q), &v); err != nil {
+		t.Fatalf("query is not valid JSON: %v (%s)", err, q)
+	}
+	if v[0] != "and" {
+		t.Fatalf("expected combined query to start with 'and', got %v", v[0])
+	}
+}
+
+// ---- multi-fact -F (comma-separated) ----------------------------------------
+
+func TestSplitFactNames(t *testing.T) {
+	cases := map[string][]string{
+		"ipaddress":            {"ipaddress"},
+		"ipaddress,enc_role":   {"ipaddress", "enc_role"},
+		"ipaddress, enc_role ": {"ipaddress", "enc_role"}, // trims spaces
+		"a,,b,":                {"a", "b"},                // drops empties
+		"":                     nil,
+	}
+	for in, want := range cases {
+		got := splitFactNames(in)
+		if len(got) != len(want) {
+			t.Fatalf("splitFactNames(%q) = %v, want %v", in, got, want)
+		}
+		for i := range want {
+			if got[i] != want[i] {
+				t.Fatalf("splitFactNames(%q) = %v, want %v", in, got, want)
+			}
+		}
+	}
+}
+
+func TestBuildQuery_SingleFact_NoOr(t *testing.T) {
+	q := buildQuery("", "ipaddress", "", "enc_role", false, false, false)
+	if strings.Contains(q, `"or"`) {
+		t.Fatalf("single fact should not use 'or': %s", q)
+	}
+	if !strings.Contains(q, "ipaddress") {
+		t.Fatalf("expected fact name in query: %s", q)
+	}
+}
+
+func TestBuildQuery_MultiFact_UsesOr(t *testing.T) {
+	q := buildQuery("host1", "ipaddress,enc_role", "", "enc_role", false, false, false)
+	if !strings.Contains(q, `"or"`) {
+		t.Fatalf("expected 'or' over fact names: %s", q)
+	}
+	if !strings.Contains(q, "ipaddress") || !strings.Contains(q, "enc_role") {
+		t.Fatalf("expected both fact names: %s", q)
+	}
+	// Must remain valid PQL JSON, combined under "and" with the certname filter.
+	var v []interface{}
+	if err := json.Unmarshal([]byte(q), &v); err != nil {
+		t.Fatalf("query is not valid JSON: %v (%s)", err, q)
+	}
+	if v[0] != "and" {
+		t.Fatalf("expected top-level 'and', got %v", v[0])
+	}
+}
+
+func TestRun_JSON_MultipleFacts(t *testing.T) {
+	// Two facts returned for one host must both appear, keyed by their real name.
+	facts := []fact{
+		{Certname: "hosta", Name: "ipaddress", Value: rawJSON("198.18.0.1")},
+		{Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::dns")},
+	}
+	out := runToString(t, facts, func(a *runArgs) {
+		a.showRole = false
+		a.factName = "ipaddress,enc_role"
+		a.jsonMode = true
+	})
+	var parsed map[string]map[string]interface{}
+	if err := json.Unmarshal([]byte(out), &parsed); err != nil {
+		t.Fatalf("output is not valid JSON: %v (%s)", err, out)
+	}
+	host := parsed["hosta"]
+	if host["ipaddress"] != "198.18.0.1" || host["enc_role"] != "roles::dns" {
+		t.Fatalf("expected both facts under host, got: %v", host)
+	}
+}
+
+// ---- matchValue (positional fallback for `-pm value`) -----------------------
+
+func TestMatchValue_FlagWins(t *testing.T) {
+	// An explicit -m value takes precedence over any positional arg.
+	if got := matchValue("flagval", []string{"posval"}); got != "flagval" {
+		t.Fatalf("expected flag value to win, got %q", got)
+	}
+}
+
+func TestMatchValue_PositionalFallback(t *testing.T) {
+	// This is the `-pm k8s` case: pflag leaves k8s as a positional because the
+	// grouped -m flag does not attach the space-separated value.
+	if got := matchValue("", []string{"k8s"}); got != "k8s" {
+		t.Fatalf("expected positional fallback, got %q", got)
+	}
+}
+
+func TestMatchValue_NoneGiven(t *testing.T) {
+	if got := matchValue("", nil); got != "" {
+		t.Fatalf("expected empty, got %q", got)
+	}
+}
+
+// ---- valueString / valueAny -------------------------------------------------
 
 func TestValueString_String(t *testing.T) {
-	raw := rawJSON("hello")
-	if got := valueString(raw); got != "hello" {
+	if got := valueString(rawJSON("hello")); got != "hello" {
 		t.Fatalf("expected hello, got %s", got)
 	}
 }
 
 func TestValueString_Number(t *testing.T) {
-	raw := rawJSON(42)
-	if got := valueString(raw); got != "42" {
+	if got := valueString(rawJSON(42)); got != "42" {
 		t.Fatalf("expected 42, got %s", got)
 	}
 }
 
+func TestValueString_Bool(t *testing.T) {
+	if got := valueString(rawJSON(true)); got != "true" {
+		t.Fatalf("expected true, got %s", got)
+	}
+}
+
 func TestValueString_Object(t *testing.T) {
 	raw := json.RawMessage(`{"a":1}`)
-	got := valueString(raw)
-	if got != `{"a":1}` {
+	if got := valueString(raw); got != `{"a":1}` {
 		t.Fatalf("expected compact JSON, got %s", got)
 	}
 }
 
+func TestValueAny_TypesPreserved(t *testing.T) {
+	if v := valueAny(rawJSON("s")); v != "s" {
+		t.Fatalf("expected string, got %v", v)
+	}
+	if v := valueAny(rawJSON(3)); v != float64(3) {
+		t.Fatalf("expected float64(3), got %T %v", v, v)
+	}
+	if v := valueAny(rawJSON(true)); v != true {
+		t.Fatalf("expected bool true, got %v", v)
+	}
+}
+
 // ---- processResults ---------------------------------------------------------
 
 func TestProcessResults_Sorted(t *testing.T) {
@@ -108,8 +292,7 @@ func TestProcessResults_Sorted(t *testing.T) {
 }
 
 func TestProcessResults_Empty(t *testing.T) {
-	out := processResults(nil)
-	if len(out) != 0 {
+	if out := processResults(nil); len(out) != 0 {
 		t.Fatalf("expected empty, got %v", out)
 	}
 }
@@ -117,11 +300,7 @@ func TestProcessResults_Empty(t *testing.T) {
 // ---- countResults -----------------------------------------------------------
 
 func TestCountResults_Basic(t *testing.T) {
-	lines := []string{
-		"host1 syd1",
-		"host2 syd1",
-		"host3 mel1",
-	}
+	lines := []string{"host1 syd1", "host2 syd1", "host3 mel1"}
 	out := countResults(lines)
 	found := map[string]bool{}
 	for _, l := range out {
@@ -136,9 +315,8 @@ func TestCountResults_Basic(t *testing.T) {
 }
 
 func TestCountResults_Sorted(t *testing.T) {
-	lines := []string{"h1 z", "h2 a", "h3 a"}
-	out := countResults(lines)
-	// lexicographic sort: "1: z" < "2: a"
+	// Lexicographic sort of the "N: value" strings: "1: z" < "2: a".
+	out := countResults([]string{"h1 z", "h2 a", "h3 a"})
 	if out[0] != "1: z" || out[1] != "2: a" {
 		t.Fatalf("unexpected order: %v", out)
 	}
@@ -147,13 +325,11 @@ func TestCountResults_Sorted(t *testing.T) {
 // ---- queryPuppetDB ----------------------------------------------------------
 
 func TestQueryPuppetDB_Success(t *testing.T) {
-	want := []fact{
-		{Certname: "node1", Name: "enc_role", Value: rawJSON("roles::dns")},
-	}
+	want := []fact{{Certname: "node1", Name: "enc_role", Value: rawJSON("roles::dns")}}
 	srv := newTestServer(t, want)
 	defer srv.Close()
 
-	got, err := queryPuppetDB(srv.URL+"/pdb/query/v4/facts", `["=","name","enc_role"]`)
+	got, err := queryPuppetDB(srv.URL, `["=","name","enc_role"]`)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -162,14 +338,27 @@ func TestQueryPuppetDB_Success(t *testing.T) {
 	}
 }
 
+func TestQueryPuppetDB_SendsQueryParam(t *testing.T) {
+	var got string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		got = r.URL.Query().Get("query")
+		_ = json.NewEncoder(w).Encode([]fact{})
+	}))
+	defer srv.Close()
+
+	_, _ = queryPuppetDB(srv.URL, `["=","name","enc_role"]`)
+	if got != `["=","name","enc_role"]` {
+		t.Fatalf("query param not forwarded correctly, got %q", got)
+	}
+}
+
 func TestQueryPuppetDB_HTTPError(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "not found", http.StatusNotFound)
 	}))
 	defer srv.Close()
 
-	_, err := queryPuppetDB(srv.URL+"/pdb/query/v4/facts", `["=","name","enc_role"]`)
-	if err == nil {
+	if _, err := queryPuppetDB(srv.URL, `[]`); err == nil {
 		t.Fatal("expected error for 404")
 	}
 }
@@ -180,15 +369,13 @@ func TestQueryPuppetDB_BadJSON(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	_, err := queryPuppetDB(srv.URL+"/pdb/query/v4/facts", `["=","name","enc_role"]`)
-	if err == nil {
+	if _, err := queryPuppetDB(srv.URL, `[]`); err == nil {
 		t.Fatal("expected decode error")
 	}
 }
 
 func TestQueryPuppetDB_ConnectionRefused(t *testing.T) {
-	_, err := queryPuppetDB("http://127.0.0.1:1/facts", `["=","name","enc_role"]`)
-	if err == nil {
+	if _, err := queryPuppetDB("http://127.0.0.1:1/facts", `[]`); err == nil {
 		t.Fatal("expected connection error")
 	}
 }
@@ -292,8 +479,7 @@ func TestLoadConfig_InvalidYAML(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err := loadConfig()
-	if err == nil {
+	if _, err := loadConfig(); err == nil {
 		t.Fatal("expected error for invalid YAML")
 	}
 }
@@ -305,9 +491,7 @@ func TestWriteDefaultConfig(t *testing.T) {
 	if err := writeDefaultConfig(); err != nil {
 		t.Fatal(err)
 	}
-
-	path := filepath.Join(dir, appName, configFileName)
-	data, err := os.ReadFile(path)
+	data, err := os.ReadFile(filepath.Join(dir, appName, configFileName))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -323,15 +507,14 @@ func TestWriteDefaultConfig_AlreadyExists(t *testing.T) {
 	if err := writeDefaultConfig(); err != nil {
 		t.Fatal(err)
 	}
-	err := writeDefaultConfig()
-	if err == nil {
+	if err := writeDefaultConfig(); err == nil {
 		t.Fatal("expected error when config already exists")
 	}
 }
 
 // ---- allFactsForNode --------------------------------------------------------
 
-func TestAllFactsForNode_ReturnsSortedFacts(t *testing.T) {
+func TestAllFactsForNode_ReturnsFacts(t *testing.T) {
 	facts := []fact{
 		{Certname: "node1", Name: "zebra", Value: rawJSON("z-val")},
 		{Certname: "node1", Name: "alpha", Value: rawJSON("a-val")},
@@ -340,7 +523,7 @@ func TestAllFactsForNode_ReturnsSortedFacts(t *testing.T) {
 	srv := newTestServer(t, facts)
 	defer srv.Close()
 
-	got, err := allFactsForNode(srv.URL+"/pdb/query/v4/facts", "node1")
+	got, err := allFactsForNode(srv.URL, "node1")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -353,12 +536,11 @@ func TestAllFactsForNode_QueryContainsCertname(t *testing.T) {
 	var receivedQuery string
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		receivedQuery = r.URL.Query().Get("query")
-		w.Header().Set("Content-Type", "application/json")
 		_ = json.NewEncoder(w).Encode([]fact{})
 	}))
 	defer srv.Close()
 
-	_, _ = allFactsForNode(srv.URL+"/pdb/query/v4/facts", "mynode.example.com")
+	_, _ = allFactsForNode(srv.URL, "mynode.example.com")
 	if !strings.Contains(receivedQuery, "mynode.example.com") {
 		t.Fatalf("expected certname in query, got: %s", receivedQuery)
 	}
@@ -373,59 +555,275 @@ func TestAllFactsForNode_HTTPError(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	_, err := allFactsForNode(srv.URL+"/pdb/query/v4/facts", "node1")
-	if err == nil {
+	if _, err := allFactsForNode(srv.URL, "node1"); err == nil {
 		t.Fatal("expected error for HTTP 500")
 	}
 }
 
-// ---- run with -a flag -------------------------------------------------------
+// ---- stdinReader (no-TTY behavior) ------------------------------------------
+
+func TestStdinReader_CharDeviceFallsThrough(t *testing.T) {
+	// /dev/null is a character device — the same shape stdin has when the tool
+	// is invoked by an agent/CI without a TTY. It must NOT be treated as input.
+	f, err := os.Open(os.DevNull)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = f.Close() }()
+
+	if _, ok := stdinReader(f); ok {
+		t.Fatal("expected /dev/null to be treated as no stdin data")
+	}
+}
+
+func TestStdinReader_EmptyPipeFallsThrough(t *testing.T) {
+	// A closed, empty pipe (e.g. `true | node-lookup`) must fall through to a
+	// normal query rather than consuming empty input and printing nothing.
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	_ = w.Close() // no data written; reader sees immediate EOF
+	defer func() { _ = r.Close() }()
+
+	if _, ok := stdinReader(r); ok {
+		t.Fatal("expected empty pipe to be treated as no stdin data")
+	}
+}
+
+func TestStdinReader_PipeWithDataIsRead(t *testing.T) {
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	go func() {
+		_, _ = io.WriteString(w, "node1\nnode2\n")
+		_ = w.Close()
+	}()
+	defer func() { _ = r.Close() }()
+
+	reader, ok := stdinReader(r)
+	if !ok {
+		t.Fatal("expected pipe with data to be treated as stdin input")
+	}
+	got, _ := reader.ReadString('\n')
+	if strings.TrimSpace(got) != "node1" {
+		t.Fatalf("expected first line 'node1', got %q", got)
+	}
+}
+
+func TestStdinReader_RegularFileWithData(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "nodes.txt")
+	if err := os.WriteFile(path, []byte("host1\nhost2\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	f, err := os.Open(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = f.Close() }()
+
+	if _, ok := stdinReader(f); !ok {
+		t.Fatal("expected redirected file with data to be treated as stdin input")
+	}
+}
+
+func TestStdinReader_EmptyFileFallsThrough(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "empty.txt")
+	if err := os.WriteFile(path, nil, 0o644); err != nil {
+		t.Fatal(err)
+	}
+	f, err := os.Open(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = f.Close() }()
+
+	if _, ok := stdinReader(f); ok {
+		t.Fatal("expected empty file to be treated as no stdin data")
+	}
+}
+
+// ---- run: validation --------------------------------------------------------
 
 func TestRun_AllFacts_RequiresNode(t *testing.T) {
 	cfg := config{PuppetDBURL: "http://unused", RoleFact: "enc_role"}
-	err := run(cfg, "", "", "", "", false, false, false, false, false, false, true)
+	err := run(cfg, "", "", "", false, false, false, false, false, false, false, false, true)
 	if err == nil || !strings.Contains(err.Error(), "-a requires -n") {
 		t.Fatalf("expected -a requires -n error, got: %v", err)
 	}
 }
 
+func TestRun_OutputFlagsRequireRoleOrFact(t *testing.T) {
+	cfg := config{PuppetDBURL: "http://unused", RoleFact: "enc_role"}
+	// -1 (nodeOnly) with neither -R nor -F must error before any HTTP call.
+	err := run(cfg, "", "", "", false, false, false, true, false, false, false, false, false)
+	if err == nil || !strings.Contains(err.Error(), "-R or -F") {
+		t.Fatalf("expected -R/-F requirement error, got: %v", err)
+	}
+}
+
+func TestRun_MatchRequiresRoleOrFact(t *testing.T) {
+	cfg := config{PuppetDBURL: "http://unused", RoleFact: "enc_role"}
+	// -m with neither -R nor -F must error before any HTTP call.
+	err := run(cfg, "", "", "someval", false, false, false, false, false, false, false, false, false)
+	if err == nil || !strings.Contains(err.Error(), "-R or -F") {
+		t.Fatalf("expected -R/-F requirement error, got: %v", err)
+	}
+}
+
+// ---- run: output modes ------------------------------------------------------
+
+type runArgs struct {
+	cfg                                                    config
+	nodeName, factName, match                              string
+	showRole, partial, inverse, nodeOnly, valueOnly, count bool
+	ansible, jsonMode, allFacts                            bool
+}
+
+// runToString invokes run against a mock PuppetDB returning facts, capturing
+// stdout. nodeName defaults to "n1" so the stdin path is skipped.
+func runToString(t *testing.T, facts []fact, mutate func(*runArgs)) string {
+	t.Helper()
+	srv := newTestServer(t, facts)
+	t.Cleanup(srv.Close)
+
+	a := runArgs{
+		cfg:      config{PuppetDBURL: srv.URL, RoleFact: "enc_role"},
+		nodeName: "n1",
+		showRole: true,
+	}
+	if mutate != nil {
+		mutate(&a)
+	}
+	return captureStdout(t, func() {
+		if err := run(a.cfg, a.nodeName, a.factName, a.match, a.showRole, a.partial, a.inverse, a.nodeOnly, a.valueOnly, a.count, a.ansible, a.jsonMode, a.allFacts); err != nil {
+			t.Fatalf("run returned error: %v", err)
+		}
+	})
+}
+
+func TestRun_DefaultOutput(t *testing.T) {
+	facts := []fact{
+		{Certname: "hostb", Name: "enc_role", Value: rawJSON("roles::web")},
+		{Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::db")},
+	}
+	out := strings.TrimSpace(runToString(t, facts, nil))
+	lines := strings.Split(out, "\n")
+	if len(lines) != 2 || lines[0] != "hosta roles::db" || lines[1] != "hostb roles::web" {
+		t.Fatalf("unexpected default output: %q", out)
+	}
+}
+
+func TestRun_NodeOnly(t *testing.T) {
+	facts := []fact{
+		{Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::db")},
+		{Certname: "hostb", Name: "enc_role", Value: rawJSON("roles::web")},
+	}
+	out := strings.TrimSpace(runToString(t, facts, func(a *runArgs) { a.nodeOnly = true }))
+	if out != "hosta\nhostb" {
+		t.Fatalf("unexpected -1 output: %q", out)
+	}
+}
+
+func TestRun_ValueOnly(t *testing.T) {
+	facts := []fact{
+		{Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::db")},
+		{Certname: "hostb", Name: "enc_role", Value: rawJSON("roles::web")},
+	}
+	out := strings.TrimSpace(runToString(t, facts, func(a *runArgs) { a.valueOnly = true }))
+	if out != "roles::db\nroles::web" {
+		t.Fatalf("unexpected -2 output: %q", out)
+	}
+}
+
+func TestRun_Count(t *testing.T) {
+	facts := []fact{
+		{Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::web")},
+		{Certname: "hostb", Name: "enc_role", Value: rawJSON("roles::web")},
+		{Certname: "hostc", Name: "enc_role", Value: rawJSON("roles::db")},
+	}
+	out := strings.TrimSpace(runToString(t, facts, func(a *runArgs) { a.count = true }))
+	if !strings.Contains(out, "2: roles::web") || !strings.Contains(out, "1: roles::db") {
+		t.Fatalf("unexpected -C output: %q", out)
+	}
+}
+
+func TestRun_JSON(t *testing.T) {
+	facts := []fact{{Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::db")}}
+	out := runToString(t, facts, func(a *runArgs) { a.jsonMode = true })
+
+	var parsed map[string]map[string]interface{}
+	if err := json.Unmarshal([]byte(out), &parsed); err != nil {
+		t.Fatalf("output is not valid JSON: %v (%s)", err, out)
+	}
+	if parsed["hosta"]["enc_role"] != "roles::db" {
+		t.Fatalf("unexpected JSON structure: %v", parsed)
+	}
+}
+
+func TestRun_Ansible(t *testing.T) {
+	facts := []fact{
+		{Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::db")},
+		{Certname: "hostb", Name: "enc_role", Value: rawJSON("roles::web")},
+	}
+	out := runToString(t, facts, func(a *runArgs) { a.ansible = true })
+	if !strings.Contains(out, "all:") || !strings.Contains(out, "hosts:") {
+		t.Fatalf("expected Ansible inventory structure, got: %q", out)
+	}
+	if !strings.Contains(out, "hosta:") || !strings.Contains(out, "hostb:") {
+		t.Fatalf("expected both hosts in inventory, got: %q", out)
+	}
+	// The queried fact is attached as a host var.
+	if !strings.Contains(out, "enc_role: roles::db") || !strings.Contains(out, "enc_role: roles::web") {
+		t.Fatalf("expected fact host vars in inventory, got: %q", out)
+	}
+}
+
+func TestRun_Ansible_MultipleFacts(t *testing.T) {
+	// -F ipaddress,enc_role -A must include both facts as host vars.
+	facts := []fact{
+		{Certname: "hosta", Name: "ipaddress", Value: rawJSON("198.18.0.1")},
+		{Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::dns")},
+	}
+	out := runToString(t, facts, func(a *runArgs) {
+		a.showRole = false
+		a.factName = "ipaddress,enc_role"
+		a.ansible = true
+	})
+
+	// Parse it back as YAML and assert the structure precisely.
+	var inv struct {
+		All struct {
+			Hosts map[string]map[string]interface{} `yaml:"hosts"`
+		} `yaml:"all"`
+	}
+	if err := yaml.Unmarshal([]byte(out), &inv); err != nil {
+		t.Fatalf("inventory is not valid YAML: %v (%s)", err, out)
+	}
+	host := inv.All.Hosts["hosta"]
+	if host["ipaddress"] != "198.18.0.1" || host["enc_role"] != "roles::dns" {
+		t.Fatalf("expected both facts as host vars, got: %v", host)
+	}
+}
+
 func TestRun_AllFacts_PrintsSortedByName(t *testing.T) {
 	facts := []fact{
 		{Certname: "node1", Name: "zzz_fact", Value: rawJSON("last")},
 		{Certname: "node1", Name: "aaa_fact", Value: rawJSON("first")},
 		{Certname: "node1", Name: "mmm_fact", Value: rawJSON(true)},
 	}
-	srv := newTestServer(t, facts)
-	defer srv.Close()
-
-	old := os.Stdout
-	r, w, _ := os.Pipe()
-	os.Stdout = w
-
-	cfg := config{PuppetDBURL: srv.URL + "/pdb/query/v4/facts", RoleFact: "enc_role"}
-	err := run(cfg, "node1", "", "", "", false, false, false, false, false, false, true)
-
-	_ = w.Close()
-	os.Stdout = old
-	var buf strings.Builder
-	_, _ = io.Copy(&buf, r)
-	out := buf.String()
-
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	lines := strings.Split(strings.TrimSpace(out), "\n")
+	out := strings.TrimSpace(runToString(t, facts, func(a *runArgs) {
+		a.showRole = false
+		a.allFacts = true
+	}))
+	lines := strings.Split(out, "\n")
 	if len(lines) != 3 {
 		t.Fatalf("expected 3 lines, got %d: %q", len(lines), out)
 	}
-	if !strings.HasPrefix(lines[0], "aaa_fact") {
-		t.Errorf("first line should be aaa_fact, got: %s", lines[0])
-	}
-	if !strings.HasPrefix(lines[1], "mmm_fact") {
-		t.Errorf("second line should be mmm_fact, got: %s", lines[1])
-	}
-	if !strings.HasPrefix(lines[2], "zzz_fact") {
-		t.Errorf("third line should be zzz_fact, got: %s", lines[2])
+	if !strings.HasPrefix(lines[0], "aaa_fact") ||
+		!strings.HasPrefix(lines[1], "mmm_fact") ||
+		!strings.HasPrefix(lines[2], "zzz_fact") {
+		t.Fatalf("facts not sorted by name: %v", lines)
 	}
 }

packaging/nfpm.yaml

@@ -0,0 +1,46 @@
+---
+# nfpm config for building the node-lookup RPM.
+# Rendered through envsubst (see scripts/build-rpm.sh) then fed to `nfpm pkg`.
+
+name: ${PACKAGE_NAME}
+version: ${PACKAGE_VERSION}
+release: ${PACKAGE_RELEASE}
+arch: ${PACKAGE_ARCH}
+platform: ${PACKAGE_PLATFORM}
+section: default
+priority: extra
+description: "${PACKAGE_DESCRIPTION}"
+
+maintainer: ${PACKAGE_MAINTAINER}
+homepage: ${PACKAGE_HOMEPAGE}
+license: ${PACKAGE_LICENSE}
+
+disable_globbing: false
+
+replaces:
+  - node-lookup
+provides:
+  - node-lookup
+
+contents:
+  # The CLI binary.
+  - src: dist/node-lookup
+    dst: /usr/bin/node-lookup
+    file_info:
+      mode: 0755
+      owner: root
+      group: root
+
+  # Shell completions (generated by scripts/build-rpm.sh before packaging).
+  - src: dist/completions/node-lookup.bash
+    dst: /usr/share/bash-completion/completions/node-lookup
+    file_info:
+      mode: 0644
+  - src: dist/completions/_node-lookup
+    dst: /usr/share/zsh/site-functions/_node-lookup
+    file_info:
+      mode: 0644
+  - src: dist/completions/node-lookup.fish
+    dst: /usr/share/fish/vendor_completions.d/node-lookup.fish
+    file_info:
+      mode: 0644

scripts/build-rpm.sh

@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+#
+# Package the (already built) node-lookup binary into an RPM with nfpm,
+# bundling generated bash/zsh/fish shell completions.
+# Usage: scripts/build-rpm.sh [version]   (version defaults to $CI_COMMIT_TAG)
+#
+set -euo pipefail
+
+ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+cd "${ROOT_DIR}"
+
+VERSION="${1:-${CI_COMMIT_TAG:-0.0.0-dev}}"
+VERSION="${VERSION#v}" # strip a leading v
+BINARY="node-lookup"
+DIST="dist"
+
+if [ ! -f "${DIST}/${BINARY}" ]; then
+  echo "ERROR: ${DIST}/${BINARY} not found; run 'make build' first" >&2
+  exit 1
+fi
+
+# Generate shell completions from the freshly built binary so they always match
+# the shipped flags/subcommands.
+COMP_DIR="${DIST}/completions"
+mkdir -p "${COMP_DIR}"
+"./${DIST}/${BINARY}" completion bash >"${COMP_DIR}/${BINARY}.bash"
+"./${DIST}/${BINARY}" completion zsh >"${COMP_DIR}/_${BINARY}"
+"./${DIST}/${BINARY}" completion fish >"${COMP_DIR}/${BINARY}.fish"
+
+export PACKAGE_NAME="${BINARY}"
+export PACKAGE_VERSION="${VERSION}"
+export PACKAGE_RELEASE="1"
+export PACKAGE_ARCH="amd64"
+export PACKAGE_PLATFORM="linux"
+export PACKAGE_DESCRIPTION="CLI tool that queries the PuppetDB API to look up and filter node facts"
+export PACKAGE_MAINTAINER="Ben Vincent <ben@unkin.net>"
+export PACKAGE_HOMEPAGE="https://git.unkin.net/unkin/node-lookup"
+export PACKAGE_LICENSE="MIT"
+
+envsubst <packaging/nfpm.yaml >"${DIST}/nfpm.yaml"
+nfpm pkg --config "${DIST}/nfpm.yaml" --target "${DIST}" --packager rpm
+
+echo "Built:"
+ls -1 "${DIST}"/*.rpm