From 9b2b8596037e89000e9c924133a4fca0b5c597aa Mon Sep 17 00:00:00 2001 From: Ewan Harris Date: Thu, 17 Sep 2026 17:51:26 +0100 Subject: [PATCH 1/4] feat(mapping): add interactive explorer to mapping run Add an interactive -i/--interactive explorer to `fga mapping run`. It evaluates pasted or typed JSON documents against the mapping and prints the resulting tuple operations as an aligned table, with :reload, :trace on|off, and :quit commands. A document is evaluated as soon as it forms a complete JSON value: a single-line object on Enter, a multi-line one on its closing brace. The loop only waits for more input where a newline is a legal JSON separator, so an unterminated string or half-typed literal is evaluated and its error reported immediately rather than silently waiting. Parse errors are shown with the line, column, and a caret under the offending character, and a one-time hint explains the continuation prompt on interactive terminals. A real terminal gets raw-mode line editing (arrow keys, history); pipes and tests use a plain scanner. Replace the free-text mapping-file prompt in run/validate/test with a shared filesystem-aware .yaml/.yml file picker (promptMappingFile), rendered to stderr so a piped stdout is never corrupted. Promote golang.org/x/term to a direct dependency for raw-mode line editing and allow it in the depguard configuration. --- .golangci.yaml | 1 + README.md | 20 + cmd/mapping/mapping.go | 45 +- cmd/mapping/mapping_test.go | 21 + cmd/mapping/run.go | 40 +- cmd/mapping/run_interactive.go | 535 ++++++++++++++++++ cmd/mapping/run_interactive_test.go | 328 +++++++++++ cmd/mapping/test.go | 22 +- cmd/mapping/testdata/filter_with_desired.yaml | 10 + cmd/mapping/testdata/guarded.yaml | 8 + cmd/mapping/validate.go | 22 +- go.mod | 2 +- 12 files changed, 1007 insertions(+), 47 deletions(-) create mode 100644 cmd/mapping/mapping_test.go create mode 100644 cmd/mapping/run_interactive.go create mode 100644 cmd/mapping/run_interactive_test.go create mode 100644 cmd/mapping/testdata/filter_with_desired.yaml create mode 100644 cmd/mapping/testdata/guarded.yaml diff --git a/.golangci.yaml b/.golangci.yaml index 3e5cc369..12516cde 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -44,6 +44,7 @@ linters: - github.com/spf13/cobra - github.com/spf13/pflag - github.com/spf13/viper + - golang.org/x/term - golang.org/x/time/rate - google.golang.org/protobuf/encoding/protojson - google.golang.org/protobuf/proto diff --git a/README.md b/README.md index fede2d8c..21352ba4 100644 --- a/README.md +++ b/README.md @@ -1315,6 +1315,7 @@ Reads JSONL (one JSON object per line) from stdin (or `--input`) and emits tuple * `--writes-only`: Emit only write-action tuples in `ClientTupleKey` format, consumable directly by `fga tuple write --file` * `--aggregate`: Buffer all records and collapse them (dedup tuples and filters, detect write/delete conflicts) before emitting * `--continue-on-error`: Skip input records that fail to parse or evaluate (warn to stderr) and exit non-zero if any were skipped +* `--interactive` / `-i`: Explore the mapping in a terminal loop — type or paste a JSON document and see the tuple operations it produces. The document is evaluated as soon as it forms a complete JSON value, so a single-line object is evaluated on Enter and a multi-line one when its closing brace is typed. Supports line editing (arrow keys, history). Requires an interactive terminal and cannot be combined with `--input` or `--writes-only`. ###### Example `echo '{"id":"anne","org":"acme"}' | fga mapping run mapping.yaml` @@ -1326,6 +1327,25 @@ Reads JSONL (one JSON object per line) from stdin (or `--input`) and emits tuple {"op":"write","user":"user:anne","relation":"member","object":"org:acme"} ``` +###### Interactive mode +In a terminal, `-i` starts an explorer loop. Type or paste a JSON document and the resulting tuple operations are printed as an aligned table. The document is evaluated as soon as it parses as complete JSON — a single-line object on Enter, a multi-line one when its closing brace is typed. While more input is expected the prompt shows `...` and a one-time hint notes that the document is not yet valid JSON; pressing Enter on a blank line evaluates whatever is buffered. Invalid JSON is reported with the line, column, and a caret under the offending character. A rule with `tuple_filters` cannot be resolved offline, so its filter conditions are shown as `filter` rows, with the desired-state tuples it reconciles toward shown as indented `desired` rows (they drive a read-diff-write against a store rather than being written directly). Line editing (arrow keys, history) is available. Available commands: `:reload` re-reads and recompiles the mapping from disk, `:trace on|off` toggles the per-rule trace, and `:quit` exits. + +``` +$ fga mapping run mapping.yaml -i +mapping loaded: 2 rules. Type or paste a JSON document; it is evaluated once complete. commands: :reload :trace on|off :quit + +> {"id":"anne","org":"acme"} + write user:anne member org:acme +> {"id": bob} +Error: invalid JSON at line 1, column 8: invalid character 'b' looking for beginning of value + {"id": bob} + ^ +> {"id":"anne","org":"acme"} + filter user:anne * org:acme + desired user:anne viewer org:acme +> :quit +``` + ## Contributing See [CONTRIBUTING](https://github.com/openfga/.github/blob/main/CONTRIBUTING.md). diff --git a/cmd/mapping/mapping.go b/cmd/mapping/mapping.go index a4aede27..38707bbc 100644 --- a/cmd/mapping/mapping.go +++ b/cmd/mapping/mapping.go @@ -17,7 +17,15 @@ limitations under the License. // Package mapping implements the fga mapping command group. package mapping -import "github.com/spf13/cobra" +import ( + "fmt" + "io" + "os" + + "github.com/charmbracelet/huh" + "github.com/mattn/go-isatty" + "github.com/spf13/cobra" +) // MappingCmd is the root of the fga mapping command group. var MappingCmd = &cobra.Command{ @@ -26,6 +34,41 @@ var MappingCmd = &cobra.Command{ Long: "Validate, test, and run JSON-to-tuple mapping files.", } +// promptMappingFile resolves the mapping file path for a command that accepts an +// optional path argument. An explicit argument is returned unchanged. With no +// argument and an interactive stdin it prompts with a .yaml/.yml-scoped file +// picker; the picker renders to stderr (huh's default) so a piped stdout is never +// corrupted. A missing path with a non-interactive stdin, a cancelled picker, or +// an empty selection is a usage error: it is reported to errOut and exits with +// status 2. +func promptMappingFile(args []string, errOut io.Writer) string { + if len(args) > 0 { + return args[0] + } + + if !isatty.IsTerminal(os.Stdin.Fd()) { + fmt.Fprintln(errOut, "Error: mapping file path is required") + os.Exit(2) + } + + path := "" + + if err := huh.NewFilePicker(). + Title("Mapping file"). + CurrentDirectory("."). + AllowedTypes([]string{".yaml", ".yml"}). + ShowHidden(false). + Picking(true). + Height(15). + Value(&path). + Run(); err != nil || path == "" { + fmt.Fprintln(errOut, "Error: mapping file path is required") + os.Exit(2) + } + + return path +} + func init() { MappingCmd.AddCommand(validateCmd) MappingCmd.AddCommand(testCmd) diff --git a/cmd/mapping/mapping_test.go b/cmd/mapping/mapping_test.go new file mode 100644 index 00000000..53efda6e --- /dev/null +++ b/cmd/mapping/mapping_test.go @@ -0,0 +1,21 @@ +package mapping + +import ( + "io" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestPromptMappingFile(t *testing.T) { + t.Parallel() + + // An explicit path argument is returned verbatim without prompting. The + // no-argument branches (TTY picker, non-interactive usage error) call + // os.Exit and are exercised via the built binary, not here. + t.Run("returns an explicit path argument unchanged", func(t *testing.T) { + t.Parallel() + + assert.Equal(t, "mapping.yaml", promptMappingFile([]string{"mapping.yaml"}, io.Discard)) + }) +} diff --git a/cmd/mapping/run.go b/cmd/mapping/run.go index eb958d53..98ce5d69 100644 --- a/cmd/mapping/run.go +++ b/cmd/mapping/run.go @@ -26,6 +26,7 @@ import ( "os" "strings" + "github.com/mattn/go-isatty" "github.com/openfga/mapper" "github.com/openfga/mapper/language" "github.com/spf13/cobra" @@ -470,6 +471,7 @@ var ( runInputFile string runAggregate bool runContinueOnError bool + runInteractive bool ) var runCmd = &cobra.Command{ @@ -495,9 +497,37 @@ before emitting; the default streaming JSONL does not. continues; the command still exits non-zero if any record was skipped.`, Example: ` echo '{"id":"anne","org":"acme"}' | fga mapping run mapping.yaml fga mapping run mapping.yaml --input event.json --format json - fga mapping run --writes-only mapping.yaml > out.jsonl && fga tuple write --store-id $STORE_ID --file out.jsonl`, - Args: cobra.ExactArgs(1), + fga mapping run --writes-only mapping.yaml > out.jsonl && fga tuple write --store-id $STORE_ID --file out.jsonl + fga mapping run mapping.yaml -i`, + Args: cobra.RangeArgs(0, 1), RunE: func(cmd *cobra.Command, args []string) error { + errStream := cmd.ErrOrStderr() + + // Reject incompatible interactive flags before any prompt, so a bad flag + // combination never blocks on asking for a mapping path first. + if runInteractive { + if err := checkInteractiveFlags(runWritesOnly, runInputFile); err != nil { + fmt.Fprintln(errStream, "Error: "+err.Error()) + os.Exit(2) + } + } + + path := promptMappingFile(args, errStream) + + if runInteractive { + if !isatty.IsTerminal(os.Stdin.Fd()) { + fmt.Fprintln(errStream, "Error: --interactive requires an interactive terminal") + os.Exit(2) + } + + err := runMappingInteractive(cmd.Context(), path, cmd.InOrStdin(), cmd.OutOrStdout(), errStream) + if errors.Is(err, errMappingInvalid) { + os.Exit(2) + } + + return err + } + inputReader := cmd.InOrStdin() if runInputFile != "" { @@ -512,7 +542,7 @@ continues; the command still exits non-zero if any record was skipped.`, } err := runMapping( - cmd.Context(), args[0], + cmd.Context(), path, runMappingOptions{ format: runFormat, writesOnly: runWritesOnly, @@ -550,4 +580,8 @@ func init() { &runContinueOnError, "continue-on-error", false, "Skip input records that fail to parse or evaluate (warn to stderr) and exit non-zero if any were skipped", ) + runCmd.Flags().BoolVarP( + &runInteractive, "interactive", "i", false, + "Explore the mapping in a terminal loop: paste JSON documents and see the tuples they produce (requires a TTY)", + ) } diff --git a/cmd/mapping/run_interactive.go b/cmd/mapping/run_interactive.go new file mode 100644 index 00000000..d390c36f --- /dev/null +++ b/cmd/mapping/run_interactive.go @@ -0,0 +1,535 @@ +/* +Copyright © 2023 OpenFGA + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mapping + +import ( + "bufio" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "strings" + "text/tabwriter" + + "github.com/openfga/mapper" + "github.com/openfga/mapper/language" + "golang.org/x/term" +) + +var ( + errInteractiveWithWritesOnly = errors.New("--interactive cannot be combined with --writes-only") + errInteractiveWithInput = errors.New("--interactive cannot be combined with --input") +) + +const ( + promptPrimary = "> " + promptContinuation = "... " +) + +// checkInteractiveFlags reports the first flag that is incompatible with the +// interactive explorer. Interactive output is a human-readable table read from +// pasted documents, so the machine-output (--writes-only) and file-input +// (--input) flags have no meaning in the loop. +func checkInteractiveFlags(writesOnly bool, inputFile string) error { + switch { + case writesOnly: + return errInteractiveWithWritesOnly + case inputFile != "": + return errInteractiveWithInput + } + + return nil +} + +// interactiveSession holds the state of a single `run --interactive` loop: the +// currently loaded mapping, whether per-evaluation tracing is displayed, and the +// output streams. compiled is replaced in place by :reload. +type interactiveSession struct { + ctx context.Context //nolint:containedctx + path string + compiled *mapper.Mapping + traceOn bool + interactive bool + out io.Writer + errOut io.Writer +} + +// lineReader abstracts reading one line of input so the raw-terminal explorer +// (arrow-key editing, history) and the plain reader used by tests and pipes can +// share the same evaluation loop. SetPrompt switches between the primary and +// continuation prompts as a multi-line document is assembled. +type lineReader interface { + ReadLine() (string, error) + SetPrompt(prompt string) +} + +// termLineReader drives a golang.org/x/term terminal: full line editing, cursor +// movement, and history on a real TTY. +type termLineReader struct{ terminal *term.Terminal } + +func (r *termLineReader) ReadLine() (string, error) { + line, err := r.terminal.ReadLine() + if err != nil { + return "", fmt.Errorf("reading input: %w", err) + } + + return line, nil +} + +func (r *termLineReader) SetPrompt(prompt string) { r.terminal.SetPrompt(prompt) } + +// scannerLineReader reads whole lines from a plain io.Reader (tests, pipes). It +// has no line editing; it prints the current prompt and returns the next line, +// reporting io.EOF once the input is exhausted. +type scannerLineReader struct { + scanner *bufio.Scanner + out io.Writer + prompt string +} + +func (r *scannerLineReader) ReadLine() (string, error) { + fmt.Fprint(r.out, r.prompt) + + if r.scanner.Scan() { + return r.scanner.Text(), nil + } + + if err := r.scanner.Err(); err != nil { + return "", fmt.Errorf("reading input: %w", err) + } + + return "", io.EOF +} + +func (r *scannerLineReader) SetPrompt(prompt string) { r.prompt = prompt } + +// readWriter pairs an input reader with an output writer so a term.Terminal can +// read keystrokes from one stream and render to another. +type readWriter struct { + io.Reader + io.Writer +} + +// runMappingInteractive compiles the mapping (with tracing enabled so :trace can +// toggle display without recompiling) and runs the explore loop, reading pasted +// JSON documents from input and rendering the resulting tuple operations. A real +// terminal gets full line editing via raw mode; anything else (tests, pipes) +// uses a plain line scanner. A compile failure is reported as errMappingInvalid +// before the loop starts. +func runMappingInteractive(ctx context.Context, path string, input io.Reader, out, errOut io.Writer) error { + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("reading %s: %w", path, err) + } + + compiled, compileErr := mapper.Compile(data, mapper.WithTrace(true)) + if compileErr != nil { + fmt.Fprintln(errOut, mapper.DiagnosticsFrom(compileErr)) + + return errMappingInvalid + } + + session := &interactiveSession{ctx: ctx, path: path, compiled: compiled, out: out, errOut: errOut} + + if file, ok := input.(*os.File); ok && term.IsTerminal(int(file.Fd())) { + return session.runRaw(file) + } + + return session.runPlain(input) +} + +// runRaw puts the terminal into raw mode and drives the explorer with a +// term.Terminal, giving cursor movement, history, and line editing. Session +// output is routed through the terminal so it interleaves correctly with the +// editing line and gets CRLF translation. The terminal state is always restored +// on exit. +func (s *interactiveSession) runRaw(file *os.File) error { + fileDescriptor := int(file.Fd()) + + oldState, err := term.MakeRaw(fileDescriptor) + if err != nil { + return fmt.Errorf("entering raw mode: %w", err) + } + + defer func() { _ = term.Restore(fileDescriptor, oldState) }() + + terminal := term.NewTerminal(readWriter{Reader: file, Writer: s.out}, promptPrimary) + + s.out = terminal + s.errOut = terminal + s.interactive = true + + s.banner() + + return s.loop(&termLineReader{terminal: terminal}) +} + +// runPlain drives the explorer over a plain reader without raw mode, used by +// tests and non-terminal input. +func (s *interactiveSession) runPlain(input io.Reader) error { + s.banner() + + scanner := bufio.NewScanner(input) + // Allow pasted documents well beyond the 64KB default line cap. + scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) + + return s.loop(&scannerLineReader{scanner: scanner, out: s.out, prompt: promptPrimary}) +} + +func (s *interactiveSession) banner() { + fmt.Fprintf(s.out, + "mapping loaded: %d rules. Type or paste a JSON document; it is evaluated once complete. "+ + "commands: :reload :trace on|off :quit\n\n", + s.compiled.RuleCount()) +} + +// loop reads input line by line. A line starting with ":" while no document is +// buffered is a command. Otherwise lines accumulate into a document that is +// evaluated as soon as it forms a complete JSON value; an incomplete document +// switches to the continuation prompt and keeps reading. A blank line forces +// evaluation of whatever is buffered. +func (s *interactiveSession) loop(reader lineReader) error { + var doc []string + + nudged := false + + submit := func() { + s.evaluate(strings.Join(doc, "\n")) + doc = doc[:0] + nudged = false + + reader.SetPrompt(promptPrimary) + } + + for { + line, err := reader.ReadLine() + if err != nil { + if errors.Is(err, io.EOF) { + return nil + } + + return err //nolint:wrapcheck // already wrapped by the lineReader implementation + } + + trimmed := strings.TrimSpace(line) + + if len(doc) == 0 && strings.HasPrefix(trimmed, ":") { + if s.handleCommand(trimmed) { + return nil + } + + continue + } + + if trimmed == "" { + if len(doc) > 0 { + submit() + } + + continue + } + + doc = append(doc, line) + + if awaitingMoreInput(strings.Join(doc, "\n")) { + nudged = s.nudgeIncomplete(nudged) + + reader.SetPrompt(promptContinuation) + + continue + } + + submit() + } +} + +// nudgeIncomplete prints a one-time hint, on an interactive terminal only, that +// the buffered document is not yet valid JSON and how to force evaluation. It +// returns the updated nudged flag so the hint is shown at most once per document. +func (s *interactiveSession) nudgeIncomplete(nudged bool) bool { + if nudged || !s.interactive { + return nudged + } + + fmt.Fprintln(s.errOut, + " … incomplete JSON — keep typing, or press Enter on a blank line to evaluate as-is") + + return true +} + +type jsonCompleteness int + +const ( + jsonComplete jsonCompleteness = iota + jsonIncomplete + jsonInvalid +) + +// classifyJSON reports whether s is a complete JSON value, an incomplete one +// (more input needed), or invalid. A truncated document decodes with an +// unexpected-EOF error, which is the signal to keep reading rather than reject; +// any other decode error means the document will never parse, so it is submitted +// immediately and the evaluation step reports the error. +func classifyJSON(s string) jsonCompleteness { + var value any + + err := json.NewDecoder(strings.NewReader(s)).Decode(&value) + + switch { + case err == nil: + return jsonComplete + case errors.Is(err, io.ErrUnexpectedEOF) || errors.Is(err, io.EOF): + return jsonIncomplete + default: + return jsonInvalid + } +} + +// awaitingMoreInput reports whether the buffered document is incomplete in a way +// that more lines can legitimately complete. JSON is whitespace-insensitive +// between tokens, so a newline is a valid separator only where whitespace is +// legal. A document is therefore continuable only if it is incomplete and +// appending a newline keeps it incomplete rather than making it invalid: an +// unterminated string or a half-typed literal/number becomes invalid with a +// newline appended (a raw newline cannot appear mid-token), so it is evaluated +// and its error reported instead of silently waiting for input that can never +// make it valid. +func awaitingMoreInput(s string) bool { + if classifyJSON(s) != jsonIncomplete { + return false + } + + return classifyJSON(s+"\n") != jsonInvalid +} + +// handleCommand runs a `:` command and reports whether the loop should quit. +func (s *interactiveSession) handleCommand(cmd string) bool { + fields := strings.Fields(cmd) + if len(fields) == 0 { + return false + } + + switch fields[0] { + case ":quit", ":q", ":exit": + return true + case ":reload", ":r": + s.reload() + case ":trace": + s.setTrace(fields) + default: + fmt.Fprintf(s.errOut, "unknown command %q; use :reload, :trace on|off, :quit\n", fields[0]) + } + + return false +} + +func (s *interactiveSession) setTrace(fields []string) { + if len(fields) != 2 || (fields[1] != "on" && fields[1] != "off") { + fmt.Fprintln(s.errOut, "usage: :trace on|off") + + return + } + + s.traceOn = fields[1] == "on" + + fmt.Fprintf(s.out, "trace %s\n", fields[1]) +} + +// reload re-reads and recompiles the mapping from disk. On failure the loaded +// mapping is kept so the session stays usable. +func (s *interactiveSession) reload() { + data, err := os.ReadFile(s.path) + if err != nil { + fmt.Fprintf(s.errOut, "reload failed: %v\n", err) + + return + } + + compiled, compileErr := mapper.Compile(data, mapper.WithTrace(true)) + if compileErr != nil { + fmt.Fprintln(s.errOut, mapper.DiagnosticsFrom(compileErr)) + fmt.Fprintln(s.errOut, "reload failed; keeping the previously loaded mapping") + + return + } + + s.compiled = compiled + + fmt.Fprintf(s.out, "mapping reloaded: %d rules\n", compiled.RuleCount()) +} + +// evaluate parses and evaluates one pasted document, rendering its tuples (and, +// when tracing is on, the per-rule summary). Parse and evaluation errors are +// reported inline and do not end the session. +func (s *interactiveSession) evaluate(doc string) { + var event map[string]any + + if err := json.Unmarshal([]byte(doc), &event); err != nil { + s.reportJSONError(doc, err) + + return + } + + if event == nil { + fmt.Fprintf(s.errOut, "Error: %v\n", errNonObjectRecord) + + return + } + + result, err := s.compiled.Evaluate(s.ctx, event) + if err != nil { + fmt.Fprintf(s.errOut, "Error: evaluating mapping: %v\n", err) + + return + } + + s.renderTuples(result.Tuples, result.TupleFilterOperations) + + if s.traceOn { + s.renderTrace(result.Trace) + } +} + +// reportJSONError prints a parse failure. For a syntax error it locates the +// offending byte and points a caret at it (compiler style); other errors fall +// back to a plain message. +func (s *interactiveSession) reportJSONError(doc string, err error) { + var syntaxErr *json.SyntaxError + if !errors.As(err, &syntaxErr) { + fmt.Fprintf(s.errOut, "Error: reading input JSON: %v\n", err) + + return + } + + line, column, text := locateOffset(doc, int(syntaxErr.Offset)) + + fmt.Fprintf(s.errOut, "Error: invalid JSON at line %d, column %d: %v\n", line, column, err) + fmt.Fprintf(s.errOut, " %s\n %s^\n", text, strings.Repeat(" ", column-1)) +} + +// locateOffset maps a byte offset within doc to a 1-based line and column and +// returns the text of that line, so an error can be shown with a caret under the +// offending character. offset is a json.SyntaxError.Offset, which points just +// past the byte that triggered the error, so the caret targets offset-1. +func locateOffset(doc string, offset int) (int, int, string) { + if offset > len(doc) { + offset = len(doc) + } + + caret := max(offset-1, 0) + + prefix := doc[:caret] + line := strings.Count(prefix, "\n") + 1 + lineStart := strings.LastIndex(prefix, "\n") + 1 + column := caret - lineStart + 1 + + text := doc[lineStart:] + if end := strings.IndexByte(text, '\n'); end >= 0 { + text = text[:end] + } + + return line, column, text +} + +// renderTuples prints one aligned row per tuple as `op user relation object`. +// Tuple-filter operations render their filter conditions as `filter` rows, +// followed by the desired-state tuples that operation reconciles toward as +// indented `desired` rows; these cannot be applied offline (they drive a +// read-diff-write against a store) so they are shown for inspection, not as +// guaranteed writes. +func (s *interactiveSession) renderTuples(tuples []language.Tuple, ops []mapper.TupleFilterOperation) { + if len(tuples) == 0 && len(ops) == 0 { + fmt.Fprintln(s.out, " (no tuples)") + + return + } + + writer := tabwriter.NewWriter(s.out, 0, 0, 3, ' ', 0) + + for _, tuple := range tuples { + operation := string(tuple.Action) + if operation == "" { + operation = string(language.ActionWrite) + } + + fmt.Fprintf(writer, " %s\t%s\t%s\t%s\n", operation, tuple.User, tuple.Relation, tupleObject(tuple)) + } + + for _, filterOp := range ops { + for _, filter := range filterOp.Filters { + relation := filter.Relation + if relation == "" { + relation = "*" + } + + fmt.Fprintf(writer, " filter\t%s\t%s\t%s\n", filter.User, relation, filter.Object) + } + + for _, tuple := range filterOp.Tuples { + fmt.Fprintf(writer, " desired\t%s\t%s\t%s\n", tuple.User, tuple.Relation, tupleObject(tuple)) + } + } + + _ = writer.Flush() +} + +// tupleObject formats a tuple's object, appending its condition name in brackets +// when the tuple is conditioned. +func tupleObject(tuple language.Tuple) string { + if tuple.Condition != "" { + return fmt.Sprintf("%s [%s]", tuple.Object, tuple.Condition) + } + + return tuple.Object +} + +// renderTrace prints the same rule summary the test command emits: matched rules +// with their tuple counts, skipped rules, and any rule errors. +func (s *interactiveSession) renderTrace(trace *mapper.Trace) { + if trace == nil { + return + } + + var matched, skipped, errored []string + + for _, ruleTrace := range trace.Rules { + switch ruleTrace.Status { + case mapper.RuleMatched: + matched = append(matched, fmt.Sprintf("%s -> %d tuples", ruleTrace.Name, ruleTrace.EmittedN)) + case mapper.RuleSkipped: + skipped = append(skipped, ruleTrace.Name) + case mapper.RuleErrored: + errored = append(errored, fmt.Sprintf("%s: %v", ruleTrace.Name, ruleTrace.Error)) + } + } + + if len(matched) > 0 { + fmt.Fprintf(s.out, " rules matched: %s\n", strings.Join(matched, ", ")) + } + + if len(skipped) > 0 { + fmt.Fprintf(s.out, " rules skipped: %s (when guard false)\n", strings.Join(skipped, ", ")) + } + + if len(errored) > 0 { + fmt.Fprintf(s.out, " rules errored: %s\n", strings.Join(errored, ", ")) + } + + fmt.Fprintf(s.out, " (evaluated in %s)\n", trace.Duration) +} diff --git a/cmd/mapping/run_interactive_test.go b/cmd/mapping/run_interactive_test.go new file mode 100644 index 00000000..5ef56cf3 --- /dev/null +++ b/cmd/mapping/run_interactive_test.go @@ -0,0 +1,328 @@ +package mapping + +import ( + "bufio" + "bytes" + "context" + "os" + "strings" + "testing" + + "github.com/openfga/mapper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func runREPL(t *testing.T, path, input string) (string, string) { + t.Helper() + + var out, errOut bytes.Buffer + + err := runMappingInteractive(context.Background(), path, strings.NewReader(input), &out, &errOut) + require.NoError(t, err) + + return out.String(), errOut.String() +} + +func TestRunMappingInteractive(t *testing.T) { + t.Parallel() + + t.Run("banner reports the rule count and command help", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/valid.yaml", ":quit\n") + assert.Contains(t, out, "mapping loaded: 1 rules") + assert.Contains(t, out, ":reload") + assert.Contains(t, out, ":trace on|off") + assert.Contains(t, out, ":quit") + }) + + t.Run("evaluates a pasted document ended by a blank line", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/valid.yaml", `{"id":"anne"}`+"\n\n:quit\n") + assert.Contains(t, out, "write") + assert.Contains(t, out, "user:anne") + assert.Contains(t, out, "member") + assert.Contains(t, out, "org:acme") + }) + + t.Run("joins a multi-line document before evaluating", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/valid.yaml", "{\n\"id\":\"anne\"}\n\n:quit\n") + assert.Contains(t, out, "user:anne") + }) + + t.Run("evaluates a single-line document on its own Enter without a blank line or quit", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/valid.yaml", `{"id":"anne"}`+"\n") + assert.Contains(t, out, "user:anne") + }) + + t.Run("assembles a multi-line document and evaluates it once complete", func(t *testing.T) { + t.Parallel() + + // No blank-line terminator and no :quit: the document is evaluated as + // soon as the accumulated lines form a complete JSON value. + out, _ := runREPL(t, "testdata/valid.yaml", "{\n \"id\": \"anne\"\n}\n") + assert.Contains(t, out, "user:anne") + }) + + t.Run("renders a delete operation", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/mixed_actions.yaml", `{"id":"anne","org":"acme"}`+"\n\n:quit\n") + assert.Contains(t, out, "delete") + assert.Contains(t, out, "viewer") + }) + + t.Run("renders an unresolved tuple filter", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/with_filter.yaml", `{"id":"anne","org":"acme"}`+"\n\n:quit\n") + assert.Contains(t, out, "filter") + assert.Contains(t, out, "org:acme") + assert.Contains(t, out, "user:anne") + }) + + t.Run("renders a filter operation's desired-state tuples", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/filter_with_desired.yaml", `{"id":"anne","org":"acme"}`+"\n\n:quit\n") + assert.Contains(t, out, "filter") + assert.Contains(t, out, "desired") + assert.Contains(t, out, "viewer") + // The desired-state tuple the filter reconciles toward. + assert.Regexp(t, `desired\s+user:anne\s+viewer\s+org:acme`, out) + }) + + t.Run("reports no tuples when a guarded rule is skipped", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/guarded.yaml", `{"type":"nope","id":"anne"}`+"\n\n:quit\n") + assert.Contains(t, out, "(no tuples)") + }) + + t.Run(":trace on shows the per-rule summary", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/valid.yaml", ":trace on\n"+`{"id":"anne"}`+"\n\n:quit\n") + assert.Contains(t, out, "rules matched") + assert.Contains(t, out, "members") + }) + + t.Run(":trace on reports skipped rules", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/guarded.yaml", ":trace on\n"+`{"type":"nope"}`+"\n\n:quit\n") + assert.Contains(t, out, "rules skipped") + assert.Contains(t, out, "guarded") + }) + + t.Run("a malformed document warns and the loop continues", func(t *testing.T) { + t.Parallel() + + out, errOut := runREPL(t, "testdata/valid.yaml", "not json\n\n"+`{"id":"anne"}`+"\n\n:quit\n") + assert.Contains(t, errOut, "JSON") + assert.Contains(t, out, "user:anne") + }) + + t.Run("invalid JSON reports the line, column, and a caret", func(t *testing.T) { + t.Parallel() + + _, errOut := runREPL(t, "testdata/valid.yaml", `{"id": bob}`+"\n") + assert.Contains(t, errOut, "line 1, column 8") + assert.Contains(t, errOut, `{"id": bob}`) + assert.Contains(t, errOut, " ^") + }) + + t.Run("an unterminated string evaluates and errors rather than waiting", func(t *testing.T) { + t.Parallel() + + // `{"id}` is incomplete only because the string never closes; a newline + // can never make it valid, so it is evaluated on Enter, not continued. + _, errOut := runREPL(t, "testdata/valid.yaml", `{"id}`+"\n") + assert.Contains(t, errOut, "invalid JSON") + assert.NotContains(t, errOut, "incomplete JSON") + }) + + t.Run(":reload recompiles the mapping from disk", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/valid.yaml", ":reload\n:quit\n") + assert.Contains(t, out, "mapping reloaded: 1 rules") + }) + + t.Run("a blank command is a no-op and does not panic", func(t *testing.T) { + t.Parallel() + + session := &interactiveSession{out: &bytes.Buffer{}, errOut: &bytes.Buffer{}} + assert.False(t, session.handleCommand("")) + assert.False(t, session.handleCommand(" ")) + }) + + t.Run("an unknown command warns and the loop continues", func(t *testing.T) { + t.Parallel() + + out, errOut := runREPL(t, "testdata/valid.yaml", ":bogus\n"+`{"id":"anne"}`+"\n\n:quit\n") + assert.Contains(t, errOut, "unknown command") + assert.Contains(t, out, "user:anne") + }) + + t.Run("invalid mapping returns errMappingInvalid", func(t *testing.T) { + t.Parallel() + + var out, errOut bytes.Buffer + + err := runMappingInteractive(context.Background(), "testdata/invalid.yaml", strings.NewReader(":quit\n"), &out, &errOut) + require.ErrorIs(t, err, errMappingInvalid) + }) +} + +func TestClassifyJSON(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + in string + want jsonCompleteness + }{ + {"complete object", `{"id":"anne"}`, jsonComplete}, + {"complete across lines", "{\n\"id\":\"anne\"\n}", jsonComplete}, + {"open brace only", "{", jsonIncomplete}, + {"partial object", `{"id":`, jsonIncomplete}, + {"unterminated string", `{"id":"an`, jsonIncomplete}, + {"not json", "not json", jsonInvalid}, + {"leading garbage", "xyz{", jsonInvalid}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tc.want, classifyJSON(tc.in)) + }) + } +} + +func TestInteractiveIncompleteNudge(t *testing.T) { + t.Parallel() + + data, err := os.ReadFile("testdata/valid.yaml") + require.NoError(t, err) + + compiled, err := mapper.Compile(data, mapper.WithTrace(true)) + require.NoError(t, err) + + var out, errOut bytes.Buffer + + session := &interactiveSession{ + ctx: context.Background(), + path: "testdata/valid.yaml", + compiled: compiled, + out: &out, + errOut: &errOut, + interactive: true, + } + + // "{" is incomplete; the blank line then forces evaluation, surfacing the + // end-of-input error with a position. + reader := &scannerLineReader{ + scanner: bufio.NewScanner(strings.NewReader("{\n\n")), + out: &out, + prompt: promptPrimary, + } + require.NoError(t, session.loop(reader)) + + assert.Contains(t, errOut.String(), "incomplete JSON") + assert.Contains(t, errOut.String(), "unexpected end of JSON input") +} + +func TestInteractiveNudgeSuppressedWhenNotInteractive(t *testing.T) { + t.Parallel() + + // The plain (piped/test) path leaves interactive false, so no nudge noise. + _, errOut := runREPL(t, "testdata/valid.yaml", "{\n\n:quit\n") + assert.NotContains(t, errOut, "incomplete JSON") +} + +func TestLocateOffset(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + doc string + offset int + wantLine int + wantColumn int + wantText string + }{ + {"single line", `{"id": bob}`, 8, 1, 8, `{"id": bob}`}, + {"multi line", "{\n \"id\": bob\n}", 11, 2, 9, ` "id": bob`}, + {"end of input", `{"id":"anne"`, 12, 1, 12, `{"id":"anne"`}, + {"offset past end clamps", "{", 5, 1, 1, "{"}, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + line, column, text := locateOffset(testCase.doc, testCase.offset) + assert.Equal(t, testCase.wantLine, line) + assert.Equal(t, testCase.wantColumn, column) + assert.Equal(t, testCase.wantText, text) + }) + } +} + +func TestAwaitingMoreInput(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + in string + want bool + }{ + {"open brace", "{", true}, + {"after colon", `{"id":`, true}, + {"after comma", "[1,", true}, + {"unclosed object", `{"id":"anne"`, true}, + {"complete object", `{"id":"anne"}`, false}, + {"unterminated string", `{"id}`, false}, + {"unterminated value string", `{"a":"b`, false}, + {"partial literal", "tru", false}, + {"not json", "not json", false}, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, testCase.want, awaitingMoreInput(testCase.in)) + }) + } +} + +func TestCheckInteractiveFlags(t *testing.T) { + t.Parallel() + + t.Run("rejects --writes-only", func(t *testing.T) { + t.Parallel() + + assert.ErrorIs(t, checkInteractiveFlags(true, ""), errInteractiveWithWritesOnly) + }) + + t.Run("rejects --input", func(t *testing.T) { + t.Parallel() + + assert.ErrorIs(t, checkInteractiveFlags(false, "event.json"), errInteractiveWithInput) + }) + + t.Run("accepts a clean interactive invocation", func(t *testing.T) { + t.Parallel() + + assert.NoError(t, checkInteractiveFlags(false, "")) + }) +} diff --git a/cmd/mapping/test.go b/cmd/mapping/test.go index 22125fc7..ed1b4173 100644 --- a/cmd/mapping/test.go +++ b/cmd/mapping/test.go @@ -23,8 +23,6 @@ import ( "io" "os" - "github.com/charmbracelet/huh" - "github.com/mattn/go-isatty" "github.com/openfga/mapper" "github.com/spf13/cobra" ) @@ -187,25 +185,7 @@ Use --format to choose between human-readable text (default), JSON, or JUnit XML fga mapping test --fail-fast mapping.yaml`, Args: cobra.RangeArgs(0, 1), RunE: func(cmd *cobra.Command, args []string) error { - path := "" - - if len(args) == 0 { - if !isatty.IsTerminal(os.Stdin.Fd()) { - fmt.Fprintln(cmd.ErrOrStderr(), "Error: mapping file path is required") - os.Exit(2) - } - - if err := huh.NewInput(). - Title("Mapping file"). - Placeholder("mapping.yaml"). - Value(&path). - Run(); err != nil || path == "" { - fmt.Fprintln(cmd.ErrOrStderr(), "Error: mapping file path is required") - os.Exit(2) - } - } else { - path = args[0] - } + path := promptMappingFile(args, cmd.ErrOrStderr()) err := runMappingTests(cmd.Context(), path, runMappingTestsOptions{ filter: testRunFilter, diff --git a/cmd/mapping/testdata/filter_with_desired.yaml b/cmd/mapping/testdata/filter_with_desired.yaml new file mode 100644 index 00000000..c2200e13 --- /dev/null +++ b/cmd/mapping/testdata/filter_with_desired.yaml @@ -0,0 +1,10 @@ +version: "1" +rules: + - name: "reconcile-viewer" + tuple_filters: + - user: "user:{{ input.id }}" + object: "org:{{ input.org }}" + tuples: + - user: "user:{{ input.id }}" + relation: "viewer" + object: "org:{{ input.org }}" diff --git a/cmd/mapping/testdata/guarded.yaml b/cmd/mapping/testdata/guarded.yaml new file mode 100644 index 00000000..c94b2717 --- /dev/null +++ b/cmd/mapping/testdata/guarded.yaml @@ -0,0 +1,8 @@ +version: "1" +rules: + - name: "guarded" + when: 'input.type == "match"' + tuples: + - user: "user:{{ input.id }}" + relation: "member" + object: "org:acme" diff --git a/cmd/mapping/validate.go b/cmd/mapping/validate.go index b021c3b7..4215bc33 100644 --- a/cmd/mapping/validate.go +++ b/cmd/mapping/validate.go @@ -24,8 +24,6 @@ import ( "os" "strings" - "github.com/charmbracelet/huh" - "github.com/mattn/go-isatty" "github.com/openfga/mapper" "github.com/spf13/cobra" ) @@ -197,25 +195,7 @@ authorization model: object types, relations, and user types must exist and be v fga mapping validate --model-file model.fga mapping.yaml`, Args: cobra.RangeArgs(0, 1), RunE: func(cmd *cobra.Command, args []string) error { - path := "" - - if len(args) == 0 { - if !isatty.IsTerminal(os.Stdin.Fd()) { - fmt.Fprintln(cmd.ErrOrStderr(), "Error: mapping file path is required") - os.Exit(2) - } - - if err := huh.NewInput(). - Title("Mapping file"). - Placeholder("mapping.yaml"). - Value(&path). - Run(); err != nil || path == "" { - fmt.Fprintln(cmd.ErrOrStderr(), "Error: mapping file path is required") - os.Exit(2) - } - } else { - path = args[0] - } + path := promptMappingFile(args, cmd.ErrOrStderr()) err := validateMapping( path, validateFormat, validateModelFile, validateVerbose, diff --git a/go.mod b/go.mod index 77411409..212fabfd 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( github.com/spf13/viper v1.21.0 github.com/stretchr/testify v1.12.1 go.uber.org/mock v0.6.0 + golang.org/x/term v0.45.0 golang.org/x/time v0.16.0 google.golang.org/protobuf v1.36.12 gopkg.in/yaml.v3 v3.0.1 @@ -104,7 +105,6 @@ require ( golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect - golang.org/x/term v0.45.0 // indirect golang.org/x/text v0.41.0 // indirect gonum.org/v1/gonum v0.17.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260803160001-6ac0973c030d // indirect From 4b45131decd2761c81697784576c6daddfc2155d Mon Sep 17 00:00:00 2001 From: Ewan Harris Date: Fri, 18 Sep 2026 11:08:37 +0100 Subject: [PATCH 2/4] fix(mapping): address interactive explorer review feedback Render tuple-filter rows with their patch/delete action (filter:patch / filter:delete) and normalise unset user/relation/object fields to the "*" wildcard they represent, so a delete filter is no longer indistinguishable from a patch and blank cells no longer read as literal empty strings. Include a conditioned tuple's rendered context alongside its condition name so two tuples that differ only by context are distinguishable. Render the rule trace when evaluation fails under :trace on. Evaluate returns a populated trace alongside the error, which was previously discarded. Require both stdin and stdout to be a TTY for --interactive. The explorer drives a raw-mode terminal, so a redirected stdout previously sent the prompt, echo, and results to a file and left the user with a blank screen. Seed the terminal with the real TTY dimensions and track SIGWINCH resizes (no-op on Windows) instead of assuming term.NewTerminal's 80x24 default, which corrupted cursor and repaint maths on other widths. Reject --format, --aggregate, and --continue-on-error in interactive mode rather than silently ignoring them, mirroring the existing --input and --writes-only checks. Mark the mapping-file argument as optional in the run, test, and validate help strings and document the interactive file picker. --- README.md | 20 +++-- cmd/mapping/run.go | 24 +++--- cmd/mapping/run_interactive.go | 76 +++++++++++++++---- cmd/mapping/run_interactive_resize_unix.go | 56 ++++++++++++++ cmd/mapping/run_interactive_resize_windows.go | 27 +++++++ cmd/mapping/run_interactive_test.go | 64 ++++++++++++++-- cmd/mapping/test.go | 2 +- cmd/mapping/testdata/conditional_context.yaml | 10 +++ cmd/mapping/validate.go | 2 +- 9 files changed, 240 insertions(+), 41 deletions(-) create mode 100644 cmd/mapping/run_interactive_resize_unix.go create mode 100644 cmd/mapping/run_interactive_resize_windows.go create mode 100644 cmd/mapping/testdata/conditional_context.yaml diff --git a/README.md b/README.md index 21352ba4..2190cbfb 100644 --- a/README.md +++ b/README.md @@ -1228,7 +1228,9 @@ No store credentials or network access required. ##### Validate Mapping ###### Command -fga mapping **validate** \ +fga mapping **validate** [mapping-file] + +The mapping file is optional: omit it in an interactive terminal to choose one from a `.yaml`/`.yml` file picker. ###### Parameters * `--format`: Output format — `text` (default) or `json` @@ -1260,7 +1262,9 @@ JSON response: ##### Test Mapping ###### Command -fga mapping **test** \ +fga mapping **test** [mapping-file] + +The mapping file is optional: omit it in an interactive terminal to choose one from a `.yaml`/`.yml` file picker. ###### Parameters * `--format`: Output format — `text` (default), `json`, or `junit` @@ -1305,17 +1309,19 @@ Created mapping.yaml ##### Run Mapping ###### Command -fga mapping **run** \ +fga mapping **run** [mapping-file] Reads JSONL (one JSON object per line) from stdin (or `--input`) and emits tuple operations as JSONL (default) or a JSON batch. Runs entirely offline. Rules using `tuple_filters` cannot be expanded without a store; they are reported as warnings on stderr, or under `tuple_filter_operations` with `--format json`. +The mapping file is optional: omit it in an interactive terminal to choose one from a `.yaml`/`.yml` file picker. + ###### Parameters * `--input`: Path to a JSONL input file, one JSON object per line (default: stdin) * `--format`: Output format — `jsonl` (default) or `json` * `--writes-only`: Emit only write-action tuples in `ClientTupleKey` format, consumable directly by `fga tuple write --file` * `--aggregate`: Buffer all records and collapse them (dedup tuples and filters, detect write/delete conflicts) before emitting * `--continue-on-error`: Skip input records that fail to parse or evaluate (warn to stderr) and exit non-zero if any were skipped -* `--interactive` / `-i`: Explore the mapping in a terminal loop — type or paste a JSON document and see the tuple operations it produces. The document is evaluated as soon as it forms a complete JSON value, so a single-line object is evaluated on Enter and a multi-line one when its closing brace is typed. Supports line editing (arrow keys, history). Requires an interactive terminal and cannot be combined with `--input` or `--writes-only`. +* `--interactive` / `-i`: Explore the mapping in a terminal loop — type or paste a JSON document and see the tuple operations it produces. The document is evaluated as soon as it forms a complete JSON value, so a single-line object is evaluated on Enter and a multi-line one when its closing brace is typed. Supports line editing (arrow keys, history). Requires an interactive terminal (both stdin and stdout must be a TTY) and cannot be combined with `--input`, `--writes-only`, `--format`, `--aggregate`, or `--continue-on-error`. ###### Example `echo '{"id":"anne","org":"acme"}' | fga mapping run mapping.yaml` @@ -1328,7 +1334,7 @@ Reads JSONL (one JSON object per line) from stdin (or `--input`) and emits tuple ``` ###### Interactive mode -In a terminal, `-i` starts an explorer loop. Type or paste a JSON document and the resulting tuple operations are printed as an aligned table. The document is evaluated as soon as it parses as complete JSON — a single-line object on Enter, a multi-line one when its closing brace is typed. While more input is expected the prompt shows `...` and a one-time hint notes that the document is not yet valid JSON; pressing Enter on a blank line evaluates whatever is buffered. Invalid JSON is reported with the line, column, and a caret under the offending character. A rule with `tuple_filters` cannot be resolved offline, so its filter conditions are shown as `filter` rows, with the desired-state tuples it reconciles toward shown as indented `desired` rows (they drive a read-diff-write against a store rather than being written directly). Line editing (arrow keys, history) is available. Available commands: `:reload` re-reads and recompiles the mapping from disk, `:trace on|off` toggles the per-rule trace, and `:quit` exits. +In a terminal, `-i` starts an explorer loop. Type or paste a JSON document and the resulting tuple operations are printed as an aligned table. The document is evaluated as soon as it parses as complete JSON — a single-line object on Enter, a multi-line one when its closing brace is typed. While more input is expected the prompt shows `...` and a one-time hint notes that the document is not yet valid JSON; pressing Enter on a blank line evaluates whatever is buffered. Invalid JSON is reported with the line, column, and a caret under the offending character. A rule with `tuple_filters` cannot be resolved offline, so its filter conditions are shown as `filter:patch` or `filter:delete` rows (the action distinguishes how the store is reconciled), with the desired-state tuples it reconciles toward shown as indented `desired` rows (they drive a read-diff-write against a store rather than being written directly). Filter fields left unset match any value and render as `*`, and a conditioned tuple shows its condition name and rendered context in brackets. Line editing (arrow keys, history) is available. Available commands: `:reload` re-reads and recompiles the mapping from disk, `:trace on|off` toggles the per-rule trace, and `:quit` exits. ``` $ fga mapping run mapping.yaml -i @@ -1341,8 +1347,8 @@ Error: invalid JSON at line 1, column 8: invalid character 'b' looking for begin {"id": bob} ^ > {"id":"anne","org":"acme"} - filter user:anne * org:acme - desired user:anne viewer org:acme + filter:patch user:anne * org:acme + desired user:anne viewer org:acme > :quit ``` diff --git a/cmd/mapping/run.go b/cmd/mapping/run.go index 98ce5d69..32805932 100644 --- a/cmd/mapping/run.go +++ b/cmd/mapping/run.go @@ -475,7 +475,7 @@ var ( ) var runCmd = &cobra.Command{ - Use: "run ", + Use: "run [mapping-file]", Short: "Evaluate a mapping against JSON input and emit tuple operations", Long: `Reads JSONL from stdin (or --input) and evaluates it against the mapping file. Input is JSON Lines: one JSON object per line. Outputs tuple operations as JSONL (default) @@ -503,10 +503,17 @@ continues; the command still exits non-zero if any record was skipped.`, RunE: func(cmd *cobra.Command, args []string) error { errStream := cmd.ErrOrStderr() + opts := runMappingOptions{ + format: runFormat, + writesOnly: runWritesOnly, + aggregate: runAggregate, + continueOnError: runContinueOnError, + } + // Reject incompatible interactive flags before any prompt, so a bad flag // combination never blocks on asking for a mapping path first. if runInteractive { - if err := checkInteractiveFlags(runWritesOnly, runInputFile); err != nil { + if err := checkInteractiveFlags(opts, runInputFile); err != nil { fmt.Fprintln(errStream, "Error: "+err.Error()) os.Exit(2) } @@ -515,7 +522,10 @@ continues; the command still exits non-zero if any record was skipped.`, path := promptMappingFile(args, errStream) if runInteractive { - if !isatty.IsTerminal(os.Stdin.Fd()) { + // Both streams must be a TTY: the explorer drives a raw-mode terminal, + // so a redirected stdout would send the prompt, echo, and results to a + // file and leave the user staring at a blank screen. + if !isatty.IsTerminal(os.Stdin.Fd()) || !isatty.IsTerminal(os.Stdout.Fd()) { fmt.Fprintln(errStream, "Error: --interactive requires an interactive terminal") os.Exit(2) } @@ -542,13 +552,7 @@ continues; the command still exits non-zero if any record was skipped.`, } err := runMapping( - cmd.Context(), path, - runMappingOptions{ - format: runFormat, - writesOnly: runWritesOnly, - aggregate: runAggregate, - continueOnError: runContinueOnError, - }, + cmd.Context(), path, opts, inputReader, cmd.OutOrStdout(), cmd.ErrOrStderr(), ) if errors.Is(err, errUnknownRunFormat) { diff --git a/cmd/mapping/run_interactive.go b/cmd/mapping/run_interactive.go index d390c36f..b6511864 100644 --- a/cmd/mapping/run_interactive.go +++ b/cmd/mapping/run_interactive.go @@ -33,8 +33,11 @@ import ( ) var ( - errInteractiveWithWritesOnly = errors.New("--interactive cannot be combined with --writes-only") - errInteractiveWithInput = errors.New("--interactive cannot be combined with --input") + errInteractiveWithWritesOnly = errors.New("--interactive cannot be combined with --writes-only") + errInteractiveWithInput = errors.New("--interactive cannot be combined with --input") + errInteractiveWithFormat = errors.New("--interactive cannot be combined with --format") + errInteractiveWithAggregate = errors.New("--interactive cannot be combined with --aggregate") + errInteractiveWithContinueOnError = errors.New("--interactive cannot be combined with --continue-on-error") ) const ( @@ -43,15 +46,23 @@ const ( ) // checkInteractiveFlags reports the first flag that is incompatible with the -// interactive explorer. Interactive output is a human-readable table read from -// pasted documents, so the machine-output (--writes-only) and file-input -// (--input) flags have no meaning in the loop. -func checkInteractiveFlags(writesOnly bool, inputFile string) error { +// interactive explorer. Interactive output is a human-readable table evaluated +// one pasted document at a time, so the batch flags have no meaning in the loop: +// --writes-only and --format select machine output, --input reads from a file +// instead of the prompt, and --aggregate and --continue-on-error act on a whole +// input stream. Rejecting them is clearer than silently ignoring them. +func checkInteractiveFlags(opts runMappingOptions, inputFile string) error { switch { - case writesOnly: + case opts.writesOnly: return errInteractiveWithWritesOnly case inputFile != "": return errInteractiveWithInput + case opts.format != "" && opts.format != "jsonl": + return errInteractiveWithFormat + case opts.aggregate: + return errInteractiveWithAggregate + case opts.continueOnError: + return errInteractiveWithContinueOnError } return nil @@ -171,6 +182,15 @@ func (s *interactiveSession) runRaw(file *os.File) error { terminal := term.NewTerminal(readWriter{Reader: file, Writer: s.out}, promptPrimary) + // term.NewTerminal assumes 80x24; seed the real dimensions so cursor and + // repaint maths are correct on wider terminals when editing wrapped lines. + if width, height, sizeErr := term.GetSize(fileDescriptor); sizeErr == nil { + _ = terminal.SetSize(width, height) + } + + stopResize := watchResize(fileDescriptor, terminal) + defer stopResize() + s.out = terminal s.errOut = terminal s.interactive = true @@ -396,6 +416,12 @@ func (s *interactiveSession) evaluate(doc string) { if err != nil { fmt.Fprintf(s.errOut, "Error: evaluating mapping: %v\n", err) + // Evaluate returns a populated trace alongside the error, so under + // :trace on the failing rule is still surfaced instead of no trace. + if s.traceOn && result != nil { + s.renderTrace(result.Trace) + } + return } @@ -473,12 +499,14 @@ func (s *interactiveSession) renderTuples(tuples []language.Tuple, ops []mapper. for _, filterOp := range ops { for _, filter := range filterOp.Filters { - relation := filter.Relation - if relation == "" { - relation = "*" + action := string(filter.Action) + if action == "" { + // Match the compiler default so an unset action reads as patch. + action = string(language.FilterActionPatch) } - fmt.Fprintf(writer, " filter\t%s\t%s\t%s\n", filter.User, relation, filter.Object) + fmt.Fprintf(writer, " filter:%s\t%s\t%s\t%s\n", + action, orWildcard(filter.User), orWildcard(filter.Relation), orWildcard(filter.Object)) } for _, tuple := range filterOp.Tuples { @@ -489,14 +517,32 @@ func (s *interactiveSession) renderTuples(tuples []language.Tuple, ops []mapper. _ = writer.Flush() } +// orWildcard renders an empty tuple-filter field as "*", the wildcard it stands +// for: an unset user, relation, or object matches any value, so a blank cell +// would misleadingly read as a literal empty string. +func orWildcard(field string) string { + if field == "" { + return "*" + } + + return field +} + // tupleObject formats a tuple's object, appending its condition name in brackets -// when the tuple is conditioned. +// when the tuple is conditioned, together with the rendered context so two +// tuples that differ only by context are distinguishable. func tupleObject(tuple language.Tuple) string { - if tuple.Condition != "" { - return fmt.Sprintf("%s [%s]", tuple.Object, tuple.Condition) + if tuple.Condition == "" { + return tuple.Object + } + + if len(tuple.Context) > 0 { + if ctx, err := json.Marshal(tuple.Context); err == nil { + return fmt.Sprintf("%s [%s %s]", tuple.Object, tuple.Condition, ctx) + } } - return tuple.Object + return fmt.Sprintf("%s [%s]", tuple.Object, tuple.Condition) } // renderTrace prints the same rule summary the test command emits: matched rules diff --git a/cmd/mapping/run_interactive_resize_unix.go b/cmd/mapping/run_interactive_resize_unix.go new file mode 100644 index 00000000..22d6c485 --- /dev/null +++ b/cmd/mapping/run_interactive_resize_unix.go @@ -0,0 +1,56 @@ +//go:build !windows + +/* +Copyright © 2023 OpenFGA + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mapping + +import ( + "os" + "os/signal" + "syscall" + + "golang.org/x/term" +) + +// watchResize keeps the terminal's dimensions in sync with the TTY. On each +// SIGWINCH it re-queries the size and updates the terminal so cursor and +// repaint maths stay correct after the window is resized. The returned function +// stops the watcher and must be called when the explorer exits. +func watchResize(fileDescriptor int, terminal *term.Terminal) func() { + signals := make(chan os.Signal, 1) + signal.Notify(signals, syscall.SIGWINCH) + + done := make(chan struct{}) + + go func() { + for { + select { + case <-signals: + if width, height, err := term.GetSize(fileDescriptor); err == nil { + _ = terminal.SetSize(width, height) + } + case <-done: + return + } + } + }() + + return func() { + signal.Stop(signals) + close(done) + } +} diff --git a/cmd/mapping/run_interactive_resize_windows.go b/cmd/mapping/run_interactive_resize_windows.go new file mode 100644 index 00000000..a2a6df6c --- /dev/null +++ b/cmd/mapping/run_interactive_resize_windows.go @@ -0,0 +1,27 @@ +//go:build windows + +/* +Copyright © 2023 OpenFGA + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mapping + +import "golang.org/x/term" + +// watchResize is a no-op on Windows, which has no SIGWINCH signal. The terminal +// keeps the dimensions seeded at start-up. +func watchResize(_ int, _ *term.Terminal) func() { + return func() {} +} diff --git a/cmd/mapping/run_interactive_test.go b/cmd/mapping/run_interactive_test.go index 5ef56cf3..f8cb4e4b 100644 --- a/cmd/mapping/run_interactive_test.go +++ b/cmd/mapping/run_interactive_test.go @@ -78,13 +78,39 @@ func TestRunMappingInteractive(t *testing.T) { assert.Contains(t, out, "viewer") }) - t.Run("renders an unresolved tuple filter", func(t *testing.T) { + t.Run("renders an unresolved tuple filter with its action and wildcards", func(t *testing.T) { t.Parallel() + // with_filter.yaml deletes by user+object with no relation, so the + // missing relation renders as the "*" wildcard it represents. out, _ := runREPL(t, "testdata/with_filter.yaml", `{"id":"anne","org":"acme"}`+"\n\n:quit\n") - assert.Contains(t, out, "filter") - assert.Contains(t, out, "org:acme") - assert.Contains(t, out, "user:anne") + assert.Regexp(t, `filter:delete\s+user:anne\s+\*\s+org:acme`, out) + }) + + t.Run("renders a patch filter and wildcards an empty user", func(t *testing.T) { + t.Parallel() + + // with_patch_filter.yaml patches by object+relation with no user. + out, _ := runREPL(t, "testdata/with_patch_filter.yaml", `{"id":"anne","org":"acme"}`+"\n\n:quit\n") + assert.Regexp(t, `filter:patch\s+\*\s+member\s+org:acme`, out) + }) + + t.Run("renders a conditioned tuple's context so it is distinguishable", func(t *testing.T) { + t.Parallel() + + out, _ := runREPL(t, "testdata/conditional_context.yaml", `{"id":"anne","region":"us"}`+"\n\n:quit\n") + assert.Contains(t, out, `[in_region {"region":"us"}]`) + }) + + t.Run(":trace on shows the rule trace when evaluation errors", func(t *testing.T) { + t.Parallel() + + // valid.yaml interpolates input.id; a record without it fails evaluation. + // The error trace must still surface the failing rule under :trace on. + out, errOut := runREPL(t, "testdata/valid.yaml", ":trace on\n"+`{"org":"acme"}`+"\n\n:quit\n") + assert.Contains(t, errOut, "Error: evaluating mapping") + assert.Contains(t, out, "rules errored") + assert.Contains(t, out, "members") }) t.Run("renders a filter operation's desired-state tuples", func(t *testing.T) { @@ -311,18 +337,42 @@ func TestCheckInteractiveFlags(t *testing.T) { t.Run("rejects --writes-only", func(t *testing.T) { t.Parallel() - assert.ErrorIs(t, checkInteractiveFlags(true, ""), errInteractiveWithWritesOnly) + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{writesOnly: true}, ""), errInteractiveWithWritesOnly) }) t.Run("rejects --input", func(t *testing.T) { t.Parallel() - assert.ErrorIs(t, checkInteractiveFlags(false, "event.json"), errInteractiveWithInput) + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{}, "event.json"), errInteractiveWithInput) + }) + + t.Run("rejects a non-default --format", func(t *testing.T) { + t.Parallel() + + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{format: "json"}, ""), errInteractiveWithFormat) + }) + + t.Run("rejects --aggregate", func(t *testing.T) { + t.Parallel() + + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{aggregate: true}, ""), errInteractiveWithAggregate) + }) + + t.Run("rejects --continue-on-error", func(t *testing.T) { + t.Parallel() + + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{continueOnError: true}, ""), errInteractiveWithContinueOnError) }) t.Run("accepts a clean interactive invocation", func(t *testing.T) { t.Parallel() - assert.NoError(t, checkInteractiveFlags(false, "")) + assert.NoError(t, checkInteractiveFlags(runMappingOptions{}, "")) + }) + + t.Run("accepts the default jsonl format", func(t *testing.T) { + t.Parallel() + + assert.NoError(t, checkInteractiveFlags(runMappingOptions{format: "jsonl"}, "")) }) } diff --git a/cmd/mapping/test.go b/cmd/mapping/test.go index ed1b4173..61450db2 100644 --- a/cmd/mapping/test.go +++ b/cmd/mapping/test.go @@ -173,7 +173,7 @@ var ( ) var testCmd = &cobra.Command{ - Use: "test ", + Use: "test [mapping-file]", Short: "Run the embedded tests in a mapping file", Long: `Compiles the mapping and runs its embedded test cases, reporting pass/fail per case. Exits 1 when any test fails, 2 when the mapping file cannot be compiled. diff --git a/cmd/mapping/testdata/conditional_context.yaml b/cmd/mapping/testdata/conditional_context.yaml new file mode 100644 index 00000000..0ae2c277 --- /dev/null +++ b/cmd/mapping/testdata/conditional_context.yaml @@ -0,0 +1,10 @@ +version: "1" +rules: + - name: "regional-member" + tuples: + - user: "user:{{ input.id }}" + relation: "member" + object: "org:acme" + condition: "in_region" + context: + region: "{{ input.region }}" diff --git a/cmd/mapping/validate.go b/cmd/mapping/validate.go index 4215bc33..09bdef12 100644 --- a/cmd/mapping/validate.go +++ b/cmd/mapping/validate.go @@ -185,7 +185,7 @@ var ( ) var validateCmd = &cobra.Command{ - Use: "validate ", + Use: "validate [mapping-file]", Short: "Validate a mapping file", Long: `Validates that a mapping file is syntactically correct and all expressions compile. With --model-file, also checks that every tuple template is consistent with the From b734ae4a16714d6c500b853bb7ca2f316039ee93 Mon Sep 17 00:00:00 2001 From: Ewan Harris Date: Fri, 18 Sep 2026 12:01:12 +0100 Subject: [PATCH 3/4] fix(mapping): validate interactive streams early and poll Windows resize Move the interactive TTY and flag validation ahead of the mapping-file prompt so a redirected stdout or incompatible flag fails immediately instead of opening the file picker. Reject any explicitly supplied --format via the flag's Changed state, since its non-empty "jsonl" default cannot otherwise distinguish set from unset. Replace the Windows resize no-op with a poller that re-queries the console size and pushes changes to the terminal, bringing Windows to parity with the signal-driven Unix path. --- cmd/mapping/run.go | 15 +++--- cmd/mapping/run_interactive.go | 8 ++- cmd/mapping/run_interactive_resize_windows.go | 50 +++++++++++++++++-- cmd/mapping/run_interactive_test.go | 25 ++++++---- 4 files changed, 75 insertions(+), 23 deletions(-) diff --git a/cmd/mapping/run.go b/cmd/mapping/run.go index 32805932..6c5dd7c1 100644 --- a/cmd/mapping/run.go +++ b/cmd/mapping/run.go @@ -510,18 +510,15 @@ continues; the command still exits non-zero if any record was skipped.`, continueOnError: runContinueOnError, } - // Reject incompatible interactive flags before any prompt, so a bad flag - // combination never blocks on asking for a mapping path first. + // Validate the interactive invocation before any prompt, so a bad flag + // combination or a redirected stream never blocks on asking for a mapping + // path first. if runInteractive { - if err := checkInteractiveFlags(opts, runInputFile); err != nil { + if err := checkInteractiveFlags(opts, runInputFile, cmd.Flags().Changed("format")); err != nil { fmt.Fprintln(errStream, "Error: "+err.Error()) os.Exit(2) } - } - - path := promptMappingFile(args, errStream) - if runInteractive { // Both streams must be a TTY: the explorer drives a raw-mode terminal, // so a redirected stdout would send the prompt, echo, and results to a // file and leave the user staring at a blank screen. @@ -529,7 +526,11 @@ continues; the command still exits non-zero if any record was skipped.`, fmt.Fprintln(errStream, "Error: --interactive requires an interactive terminal") os.Exit(2) } + } + + path := promptMappingFile(args, errStream) + if runInteractive { err := runMappingInteractive(cmd.Context(), path, cmd.InOrStdin(), cmd.OutOrStdout(), errStream) if errors.Is(err, errMappingInvalid) { os.Exit(2) diff --git a/cmd/mapping/run_interactive.go b/cmd/mapping/run_interactive.go index b6511864..ff8ac723 100644 --- a/cmd/mapping/run_interactive.go +++ b/cmd/mapping/run_interactive.go @@ -51,13 +51,17 @@ const ( // --writes-only and --format select machine output, --input reads from a file // instead of the prompt, and --aggregate and --continue-on-error act on a whole // input stream. Rejecting them is clearer than silently ignoring them. -func checkInteractiveFlags(opts runMappingOptions, inputFile string) error { +// +// formatChanged reports whether --format was set on the command line: it has a +// non-empty default ("jsonl"), so its value alone cannot distinguish an unset +// flag from one the user explicitly passed. Any explicit --format is rejected. +func checkInteractiveFlags(opts runMappingOptions, inputFile string, formatChanged bool) error { switch { case opts.writesOnly: return errInteractiveWithWritesOnly case inputFile != "": return errInteractiveWithInput - case opts.format != "" && opts.format != "jsonl": + case formatChanged: return errInteractiveWithFormat case opts.aggregate: return errInteractiveWithAggregate diff --git a/cmd/mapping/run_interactive_resize_windows.go b/cmd/mapping/run_interactive_resize_windows.go index a2a6df6c..f3b4c6a1 100644 --- a/cmd/mapping/run_interactive_resize_windows.go +++ b/cmd/mapping/run_interactive_resize_windows.go @@ -18,10 +18,50 @@ limitations under the License. package mapping -import "golang.org/x/term" +import ( + "time" -// watchResize is a no-op on Windows, which has no SIGWINCH signal. The terminal -// keeps the dimensions seeded at start-up. -func watchResize(_ int, _ *term.Terminal) func() { - return func() {} + "golang.org/x/term" +) + +// resizePollInterval is how often the console size is re-queried. Windows has no +// SIGWINCH, so a resize is detected by polling rather than a signal. +const resizePollInterval = 250 * time.Millisecond + +// watchResize keeps the terminal's dimensions in sync with the console. Windows +// has no SIGWINCH, so the size is polled: on each tick the current console size +// is queried and, when it has changed, pushed to the terminal so cursor and +// repaint maths stay correct after the window is resized. term.Terminal +// serialises SetSize against an in-progress ReadLine with an internal lock, so +// the repaint applies on the next keystroke — the same behaviour as the +// signal-driven Unix path. The returned function stops the watcher and must be +// called when the explorer exits. +func watchResize(fileDescriptor int, terminal *term.Terminal) func() { + done := make(chan struct{}) + + go func() { + ticker := time.NewTicker(resizePollInterval) + defer ticker.Stop() + + // Seed from the size runRaw already applied so an unchanged console does + // not trigger a redundant SetSize. + lastWidth, lastHeight, _ := term.GetSize(fileDescriptor) + + for { + select { + case <-ticker.C: + width, height, err := term.GetSize(fileDescriptor) + if err != nil || (width == lastWidth && height == lastHeight) { + continue + } + + lastWidth, lastHeight = width, height + _ = terminal.SetSize(width, height) + case <-done: + return + } + } + }() + + return func() { close(done) } } diff --git a/cmd/mapping/run_interactive_test.go b/cmd/mapping/run_interactive_test.go index f8cb4e4b..5248df92 100644 --- a/cmd/mapping/run_interactive_test.go +++ b/cmd/mapping/run_interactive_test.go @@ -337,42 +337,49 @@ func TestCheckInteractiveFlags(t *testing.T) { t.Run("rejects --writes-only", func(t *testing.T) { t.Parallel() - assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{writesOnly: true}, ""), errInteractiveWithWritesOnly) + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{writesOnly: true}, "", false), errInteractiveWithWritesOnly) }) t.Run("rejects --input", func(t *testing.T) { t.Parallel() - assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{}, "event.json"), errInteractiveWithInput) + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{}, "event.json", false), errInteractiveWithInput) }) - t.Run("rejects a non-default --format", func(t *testing.T) { + t.Run("rejects an explicit --format", func(t *testing.T) { t.Parallel() - assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{format: "json"}, ""), errInteractiveWithFormat) + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{format: "json"}, "", true), errInteractiveWithFormat) + }) + + t.Run("rejects an explicit --format even when it matches the default", func(t *testing.T) { + t.Parallel() + + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{format: "jsonl"}, "", true), errInteractiveWithFormat) }) t.Run("rejects --aggregate", func(t *testing.T) { t.Parallel() - assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{aggregate: true}, ""), errInteractiveWithAggregate) + assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{aggregate: true}, "", false), errInteractiveWithAggregate) }) t.Run("rejects --continue-on-error", func(t *testing.T) { t.Parallel() - assert.ErrorIs(t, checkInteractiveFlags(runMappingOptions{continueOnError: true}, ""), errInteractiveWithContinueOnError) + assert.ErrorIs( + t, checkInteractiveFlags(runMappingOptions{continueOnError: true}, "", false), errInteractiveWithContinueOnError) }) t.Run("accepts a clean interactive invocation", func(t *testing.T) { t.Parallel() - assert.NoError(t, checkInteractiveFlags(runMappingOptions{}, "")) + assert.NoError(t, checkInteractiveFlags(runMappingOptions{}, "", false)) }) - t.Run("accepts the default jsonl format", func(t *testing.T) { + t.Run("accepts the default format when it was not set explicitly", func(t *testing.T) { t.Parallel() - assert.NoError(t, checkInteractiveFlags(runMappingOptions{format: "jsonl"}, "")) + assert.NoError(t, checkInteractiveFlags(runMappingOptions{format: "jsonl"}, "", false)) }) } From 265a57ba70981283adb6b90d05c939feb14d757c Mon Sep 17 00:00:00 2001 From: Ewan Harris Date: Fri, 18 Sep 2026 12:40:20 +0100 Subject: [PATCH 4/4] fix(mapping): require stderr TTY before showing the file picker The huh file picker renders to stderr, but promptMappingFile only checked stdin before launching it. With stderr redirected and no path argument, the picker ran invisibly and blocked. Require stderr to be a TTY as well so a missing path fails fast with a usage error instead of hanging. Fixes the same latent hang across validate, test, and run. --- cmd/mapping/mapping.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/mapping/mapping.go b/cmd/mapping/mapping.go index 38707bbc..a40b5170 100644 --- a/cmd/mapping/mapping.go +++ b/cmd/mapping/mapping.go @@ -36,17 +36,18 @@ var MappingCmd = &cobra.Command{ // promptMappingFile resolves the mapping file path for a command that accepts an // optional path argument. An explicit argument is returned unchanged. With no -// argument and an interactive stdin it prompts with a .yaml/.yml-scoped file -// picker; the picker renders to stderr (huh's default) so a piped stdout is never -// corrupted. A missing path with a non-interactive stdin, a cancelled picker, or -// an empty selection is a usage error: it is reported to errOut and exits with -// status 2. +// argument it prompts with a .yaml/.yml-scoped file picker, which needs a real +// terminal: stdin for keystrokes and stderr for rendering (huh's default output, +// chosen so a piped stdout is never corrupted). If either is not a TTY the picker +// cannot be shown or driven, so a missing path is a usage error rather than an +// invisible hang. A cancelled picker or an empty selection is likewise a usage +// error: all are reported to errOut and exit with status 2. func promptMappingFile(args []string, errOut io.Writer) string { if len(args) > 0 { return args[0] } - if !isatty.IsTerminal(os.Stdin.Fd()) { + if !isatty.IsTerminal(os.Stdin.Fd()) || !isatty.IsTerminal(os.Stderr.Fd()) { fmt.Fprintln(errOut, "Error: mapping file path is required") os.Exit(2) }