← back to Cli Printing Press
fix(cli): validate &&-chained narrative recipes segment-by-segment (#1447)
bb5e2defcb03a91235aa3a39415ea509a5b8dc3b · 2026-05-15 12:24:49 -0700 · Trevin Chow
* fix(cli): validate &&-chained narrative recipes segment-by-segment
validate-narrative previously fed the whole recipe string to
shellargs.Split, so `cli sync && cli expiring --within 60d` ran with
`&&` as a positional arg and failed with `unknown flag: --within`.
splitShellChain walks the command with shellargs-compatible quote
handling and returns top-level segments separated by `&&`, `||`, or
`;`. classify then validates each segment independently. Top-level `|`
recipes classify as Unsupported with `pipe-skipped` rather than running.
Closes #1271
* fix(cli): pass trimmed segment to classify in single-segment fast path
Greptile flagged that splitShellChain trims trailing operators
(e.g. "stub sync && " → ["stub sync"]) but classify was still handing
the original command to classifySegment. In FullExamples mode that
leaked the operator into shellargs.Split's token stream, producing a
spurious StatusExampleFailed.
Pass segments[0] when present, fall back to the original command for
the zero-segment case (whitespace-only or operator-only inputs).
Tighten the test comparison to reflect.DeepEqual now that the suite
exercises pipe-bearing inputs.
Refs #1271
Files touched
M internal/narrativecheck/narrativecheck.goM internal/narrativecheck/narrativecheck_test.go
Diff
commit bb5e2defcb03a91235aa3a39415ea509a5b8dc3b
Author: Trevin Chow <trevin@trevinchow.com>
Date: Fri May 15 12:24:49 2026 -0700
fix(cli): validate &&-chained narrative recipes segment-by-segment (#1447)
* fix(cli): validate &&-chained narrative recipes segment-by-segment
validate-narrative previously fed the whole recipe string to
shellargs.Split, so `cli sync && cli expiring --within 60d` ran with
`&&` as a positional arg and failed with `unknown flag: --within`.
splitShellChain walks the command with shellargs-compatible quote
handling and returns top-level segments separated by `&&`, `||`, or
`;`. classify then validates each segment independently. Top-level `|`
recipes classify as Unsupported with `pipe-skipped` rather than running.
Closes #1271
* fix(cli): pass trimmed segment to classify in single-segment fast path
Greptile flagged that splitShellChain trims trailing operators
(e.g. "stub sync && " → ["stub sync"]) but classify was still handing
the original command to classifySegment. In FullExamples mode that
leaked the operator into shellargs.Split's token stream, producing a
spurious StatusExampleFailed.
Pass segments[0] when present, fall back to the original command for
the zero-segment case (whitespace-only or operator-only inputs).
Tighten the test comparison to reflect.DeepEqual now that the suite
exercises pipe-bearing inputs.
Refs #1271
---
internal/narrativecheck/narrativecheck.go | 122 ++++++++++++++++++
internal/narrativecheck/narrativecheck_test.go | 164 +++++++++++++++++++++++++
2 files changed, 286 insertions(+)
diff --git a/internal/narrativecheck/narrativecheck.go b/internal/narrativecheck/narrativecheck.go
index 4c1b9159..d211a2c0 100644
--- a/internal/narrativecheck/narrativecheck.go
+++ b/internal/narrativecheck/narrativecheck.go
@@ -181,6 +181,48 @@ func loadCommands(researchPath string) ([]sectionCommand, error) {
// non-identifier character. Hyphens stay because Cobra subcommands use
// them (`list-projects`).
func classify(ctx context.Context, binaryPath string, section Section, command string, opts Options) Result {
+ segments, hasPipe, err := splitShellChain(command)
+ if err != nil {
+ return Result{
+ Section: section,
+ Command: command,
+ Status: StatusExampleFailed,
+ Error: err.Error(),
+ }
+ }
+ if hasPipe {
+ return Result{
+ Section: section,
+ Command: command,
+ Status: StatusUnsupported,
+ Error: "pipe-skipped: command contains a top-level `|` which cannot run safely under PRINTING_PRESS_VERIFY=1",
+ }
+ }
+ if len(segments) <= 1 {
+ seg := command
+ if len(segments) == 1 {
+ seg = segments[0]
+ }
+ r := classifySegment(ctx, binaryPath, section, seg, opts)
+ r.Command = command
+ return r
+ }
+
+ var last Result
+ for i, seg := range segments {
+ sub := classifySegment(ctx, binaryPath, section, seg, opts)
+ if sub.Status != StatusOK {
+ sub.Command = command
+ sub.Error = fmt.Sprintf("segment %d (%q): %s", i+1, seg, sub.Error)
+ return sub
+ }
+ last = sub
+ }
+ last.Command = command
+ return last
+}
+
+func classifySegment(ctx context.Context, binaryPath string, section Section, command string, opts Options) Result {
words := extractSubcommandWords(command)
r := Result{Section: section, Command: command, Words: strings.Join(words, " ")}
@@ -302,6 +344,86 @@ func extractSubcommandWords(command string) []string {
return words
}
+// splitShellChain walks command and returns the segments separated by
+// top-level `&&`, `||`, or `;` operators. Quoted text is preserved.
+// hasPipe is true when a bare `|` appears at the top level; callers
+// should treat such commands as unrunnable in verify mode rather than
+// attempting to validate the segments around the pipe.
+//
+// Backslash-escapes and quoted-string handling mirror shellargs.Split
+// so the segments are safe to feed back into shellargs.Split.
+func splitShellChain(command string) ([]string, bool, error) {
+ var (
+ segments []string
+ quote rune
+ escaped bool
+ hasPipe bool
+ start int
+ )
+ flush := func(end int) {
+ if seg := strings.TrimSpace(command[start:end]); seg != "" {
+ segments = append(segments, seg)
+ }
+ }
+ for i := 0; i < len(command); i++ {
+ c := command[i]
+ if escaped {
+ escaped = false
+ continue
+ }
+ if quote == '\'' {
+ if c == '\'' {
+ quote = 0
+ }
+ continue
+ }
+ if quote == '"' {
+ switch c {
+ case '\\':
+ escaped = true
+ case '"':
+ quote = 0
+ }
+ continue
+ }
+ switch c {
+ case '\\':
+ escaped = true
+ case '\'', '"':
+ quote = rune(c)
+ case '&':
+ if i+1 < len(command) && command[i+1] == '&' {
+ flush(i)
+ i++
+ start = i + 1
+ }
+ case '|':
+ if i+1 < len(command) && command[i+1] == '|' {
+ flush(i)
+ i++
+ start = i + 1
+ continue
+ }
+ hasPipe = true
+ case ';':
+ flush(i)
+ start = i + 1
+ }
+ }
+ if quote != 0 {
+ return nil, false, fmt.Errorf("unclosed %s quote in %q", quoteName(quote), command)
+ }
+ flush(len(command))
+ return segments, hasPipe, nil
+}
+
+func quoteName(r rune) string {
+ if r == '\'' {
+ return "single"
+ }
+ return "double"
+}
+
// isIdentifierToken reports whether s contains only ASCII alphanumerics,
// underscores, and hyphens. Anything else (=, :, /, quotes, JSON-string
// arguments, etc.) signals the start of a non-subcommand token and ends
diff --git a/internal/narrativecheck/narrativecheck_test.go b/internal/narrativecheck/narrativecheck_test.go
index 11d1fc60..f6c1bd6e 100644
--- a/internal/narrativecheck/narrativecheck_test.go
+++ b/internal/narrativecheck/narrativecheck_test.go
@@ -6,6 +6,7 @@ import (
"os"
"os/exec"
"path/filepath"
+ "reflect"
"strings"
"sync"
"testing"
@@ -252,6 +253,169 @@ func TestValidate_EmptyResearchFlagsResearchEmpty(t *testing.T) {
}
}
+func TestSplitShellChain(t *testing.T) {
+ t.Parallel()
+
+ cases := []struct {
+ name string
+ in string
+ segments []string
+ hasPipe bool
+ wantErr bool
+ }{
+ {"plain command", "stub widgets list", []string{"stub widgets list"}, false, false},
+ {"and-chain", "stub sync && stub list --within 60d", []string{"stub sync", "stub list --within 60d"}, false, false},
+ {"semicolon-chain", "stub sync ; stub list", []string{"stub sync", "stub list"}, false, false},
+ {"or-chain", "stub sync || stub list", []string{"stub sync", "stub list"}, false, false},
+ {"top-level pipe sets flag, leaves segments unsplit", "stub list | grep foo", []string{"stub list | grep foo"}, true, false},
+ {"and inside double quotes", `stub run --msg "a && b"`, []string{`stub run --msg "a && b"`}, false, false},
+ {"semicolon inside single quotes", "stub run --msg 'a ; b'", []string{"stub run --msg 'a ; b'"}, false, false},
+ {"pipe inside quotes is not top-level", `stub run --msg "a | b"`, []string{`stub run --msg "a | b"`}, false, false},
+ {"empty trailing segment dropped", "stub sync &&", []string{"stub sync"}, false, false},
+ {"unclosed quote errors", `stub run --msg "open`, nil, false, true},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+ segs, hasPipe, err := splitShellChain(tc.in)
+ if tc.wantErr {
+ if err == nil {
+ t.Fatalf("splitShellChain(%q) = nil error, want error", tc.in)
+ }
+ return
+ }
+ if err != nil {
+ t.Fatalf("splitShellChain(%q) errored: %v", tc.in, err)
+ }
+ if hasPipe != tc.hasPipe {
+ t.Errorf("hasPipe = %v, want %v", hasPipe, tc.hasPipe)
+ }
+ want := tc.segments
+ if want == nil {
+ want = []string{}
+ }
+ got := segs
+ if got == nil {
+ got = []string{}
+ }
+ if !reflect.DeepEqual(got, want) {
+ t.Errorf("segments = %q, want %q", segs, tc.segments)
+ }
+ })
+ }
+}
+
+func TestValidate_ChainedRecipePassesWhenBothHalvesResolve(t *testing.T) {
+ t.Parallel()
+
+ binary := buildStubBinary(t)
+ research := writeFile(t, `{"narrative":{
+ "recipes":[
+ {"command":"stub widgets list && stub widgets show 42"}
+ ]
+ }}`)
+
+ report, err := Validate(context.Background(), research, binary)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if report.Walked != 1 || report.HasFailures() {
+ t.Fatalf("chained recipe should walk OK, got walked=%d failures=%v results=%+v", report.Walked, report.HasFailures(), report.Results)
+ }
+}
+
+func TestValidate_ChainedRecipeFlagsBrokenRHS(t *testing.T) {
+ t.Parallel()
+
+ binary := buildStubBinary(t)
+ research := writeFile(t, `{"narrative":{
+ "recipes":[
+ {"command":"stub widgets list && stub typo-here"}
+ ]
+ }}`)
+
+ report, err := Validate(context.Background(), research, binary)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if report.Missing != 1 {
+ t.Fatalf("chained recipe with broken RHS should report missing, got %+v", report)
+ }
+ got := report.Results[0]
+ if !strings.Contains(got.Error, "segment 2") {
+ t.Errorf("error should attribute failure to segment 2: %s", got.Error)
+ }
+ if got.Command != "stub widgets list && stub typo-here" {
+ t.Errorf("Result.Command should preserve the original recipe, got %q", got.Command)
+ }
+}
+
+func TestValidateWithOptions_ChainedRecipeRunsBothFullExamples(t *testing.T) {
+ t.Parallel()
+
+ binary := buildStubBinary(t)
+ research := writeFile(t, `{"narrative":{
+ "recipes":[
+ {"command":"stub widgets list && stub widgets show 42"}
+ ]
+ }}`)
+
+ report, err := ValidateWithOptions(context.Background(), research, binary, Options{FullExamples: true})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if report.Walked != 1 || report.HasFailures() {
+ t.Fatalf("chained full-example recipe should pass, got %+v", report)
+ }
+}
+
+// TestValidate_TrailingOperatorDoesNotLeakIntoArgs covers the
+// single-segment fast path: when splitShellChain trims a trailing `&&`,
+// classify must hand the trimmed segment (not the original) to
+// classifySegment so FullExamples mode doesn't pass `&&` as a positional
+// arg to the binary.
+func TestValidate_TrailingOperatorDoesNotLeakIntoArgs(t *testing.T) {
+ t.Parallel()
+
+ binary := buildStubBinary(t)
+ research := writeFile(t, `{"narrative":{
+ "recipes":[
+ {"command":"stub widgets list &&"}
+ ]
+ }}`)
+
+ report, err := ValidateWithOptions(context.Background(), research, binary, Options{FullExamples: true})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if report.Walked != 1 || report.HasFailures() {
+ t.Fatalf("trailing && should be trimmed and the lone segment should pass, got %+v", report)
+ }
+}
+
+func TestValidate_PipeRecipeSkipsWithReason(t *testing.T) {
+ t.Parallel()
+
+ binary := buildStubBinary(t)
+ research := writeFile(t, `{"narrative":{
+ "recipes":[
+ {"command":"stub widgets list | jq ."}
+ ]
+ }}`)
+
+ report, err := Validate(context.Background(), research, binary)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if report.Unsupported != 1 {
+ t.Fatalf("piped recipe should classify as Unsupported, got %+v", report)
+ }
+ got := report.Results[0]
+ if !strings.Contains(got.Error, "pipe-skipped") {
+ t.Errorf("error should explain the pipe skip: %s", got.Error)
+ }
+}
+
// writeFile writes content to a temp file and returns the path.
func writeFile(t *testing.T, content string) string {
t.Helper()
← a1098f15 fix(cli): populate printer + run_id in generated .printing-p
·
back to Cli Printing Press
·
fix(cli): query generic resources_fts from search empty-type 35e515c1 →