Merge PR #568: plan-102: multi-output <?build?> directive

Author: merge-queue-bot

internal/index/build_coverage_test.go

@@ -173,3 +173,63 @@ func TestAbsToWorkspace_RelOutsideRoot(t *testing.T) {
 	got := absToWorkspace("/root", "/elsewhere/x.md")
 	assert.Equal(t, "/elsewhere/x.md", got)
 }
+
+// TestCollectDirectiveEdges_BuildInputs covers the DirectiveBuild
+// branches in collectDirectiveEdges:
+//   - a glob `inputs:` entry (d.IsUnresolved() == true) emits an
+//     EdgeBuild with Unresolved=true
+//   - a literal `inputs:` entry emits a resolved EdgeBuild with
+//     TargetFile set
+//   - an absolute path (ResolveRelTarget returns "") is skipped
+func TestCollectDirectiveEdges_BuildInputs(t *testing.T) {
+	t.Parallel()
+	// The build directive has three inputs (values must be quoted so
+	// the YAML parser surfaced them as strings via ValidateStringParams):
+	//   - "**/*.md"  → glob → unresolved edge
+	//   - "src.svg"  → literal path → resolved edge (target: dir/src.svg)
+	//   - "/abs.svg" → absolute → ResolveRelTarget returns "" → skipped
+	src := []byte(
+		"# T\n\n" +
+			"<?build\n" +
+			"recipe: render\n" +
+			"outputs:\n" +
+			"  - \"out.png\"\n" +
+			"inputs:\n" +
+			"  - \"**/*.md\"\n" +
+			"  - \"src.svg\"\n" +
+			"  - \"/abs.svg\"\n" +
+			"?>\n" +
+			"- [out.png](out.png)\n" +
+			"<?/build?>\n",
+	)
+	fe := buildFileEntry("dir/doc.md", src)
+	require.NotNil(t, fe)
+
+	// Collect only EdgeBuild edges.
+	var buildEdges []Edge
+	for _, e := range fe.Outgoing {
+		if e.Kind == EdgeBuild {
+			buildEdges = append(buildEdges, e)
+		}
+	}
+	// Expect exactly two build edges: the glob (unresolved) and the
+	// literal path (resolved). The absolute path is skipped.
+	require.Len(t, buildEdges, 2)
+
+	// Find the unresolved and resolved edges (order may vary).
+	var unresolved, resolved *Edge
+	for i := range buildEdges {
+		if buildEdges[i].Unresolved {
+			unresolved = &buildEdges[i]
+		} else {
+			resolved = &buildEdges[i]
+		}
+	}
+	require.NotNil(t, unresolved, "expected an unresolved build edge for the glob input")
+	assert.Equal(t, "dir/doc.md", unresolved.SourceFile)
+	assert.Empty(t, unresolved.TargetFile)
+
+	require.NotNil(t, resolved, "expected a resolved build edge for src.svg")
+	assert.Equal(t, "dir/doc.md", resolved.SourceFile)
+	assert.Equal(t, "dir/src.svg", resolved.TargetFile)
+}

internal/index/locate.go

@@ -439,7 +439,7 @@ func piToLocate(pi *piparser.ProcessingInstruction, source []byte, lines [][]byt
 		if key := enclosingListKey(lines, line); key != "" {
 			res.DirectiveArg = key
 			res.DirectiveValue = item
-			if pi.Name == "build" && key == "inputs" {
+			if pi.Name == "build" && key == "inputs" && !isGlobPattern(item) {
 				res.DirectiveTargetFile = item
 			}
 		}
@@ -484,6 +484,9 @@ var piArgRE = regexp.MustCompile(`^\s*([A-Za-z_][A-Za-z0-9_-]*)\s*:\s*(.*?)\s*$`
 // keeps its quote characters; the caller strips them.
 var piListItemRE = regexp.MustCompile(`^\s*-\s+(.*?)\s*$`)
 
+// isGlobPattern reports whether p contains doublestar glob metacharacters.
+func isGlobPattern(p string) bool { return strings.ContainsAny(p, "*?[{") }
+
 // headingOnLine returns the heading whose first source line equals
 // line, or nil.
 func headingOnLine(root ast.Node, source []byte, line int) *ast.Heading {

internal/index/locate_coverage_test.go

@@ -216,3 +216,68 @@ func TestEnclosingListKeyNoPrecedingKey(t *testing.T) {
 	}
 	assert.Empty(t, enclosingListKey(lines, 2))
 }
+
+func TestEnclosingListKey_MultipleListItemsThenKey(t *testing.T) {
+	t.Parallel()
+	lines := [][]byte{
+		[]byte("inputs:"),
+		[]byte("  - a.md"),
+		[]byte("  - b.md"),
+	}
+	got := enclosingListKey(lines, 3)
+	assert.Equal(t, "inputs", got)
+}
+
+func TestEnclosingListKey_EmptyLineSkipped(t *testing.T) {
+	t.Parallel()
+	lines := [][]byte{
+		[]byte("inputs:"),
+		[]byte(""),
+		[]byte("  - x"),
+	}
+	got := enclosingListKey(lines, 3)
+	assert.Equal(t, "inputs", got)
+}
+
+func TestEnclosingListKey_NonListNonKeyLine(t *testing.T) {
+	t.Parallel()
+	lines := [][]byte{
+		[]byte("some prose"),
+		[]byte("  - item"),
+	}
+	got := enclosingListKey(lines, 2)
+	assert.Empty(t, got)
+}
+
+// TestLocateBuildDirectiveInputsGlob: glob inputs must not set DirectiveTargetFile.
+func TestLocateBuildDirectiveInputsGlob(t *testing.T) {
+	t.Parallel()
+	src := "# T\n\n<?build\nrecipe: render\noutputs:\n  - \"out.png\"\ninputs:\n  - \"**/*.md\"\n?>\n<?/build?>\n"
+	res := Locator{Path: "a.md"}.Locate([]byte(src), 8, 5)
+	assert.Equal(t, TokenDirectiveArg, res.Tag)
+	assert.Equal(t, "build", res.DirectiveName)
+	assert.Equal(t, "inputs", res.DirectiveArg)
+	assert.Empty(t, res.DirectiveTargetFile, "glob input must not set DirectiveTargetFile")
+}
+
+// TestLocateBuildDirectiveInputsLiteral: literal inputs set DirectiveTargetFile.
+func TestLocateBuildDirectiveInputsLiteral(t *testing.T) {
+	t.Parallel()
+	src := "# T\n\n<?build\nrecipe: render\noutputs:\n  - \"out.png\"\ninputs:\n  - \"src.svg\"\n?>\n<?/build?>\n"
+	res := Locator{Path: "a.md"}.Locate([]byte(src), 8, 5)
+	assert.Equal(t, TokenDirectiveArg, res.Tag)
+	assert.Equal(t, "build", res.DirectiveName)
+	assert.Equal(t, "inputs", res.DirectiveArg)
+	assert.Equal(t, "src.svg", res.DirectiveTargetFile)
+}
+
+// TestLocateBuildDirectiveInputsBraceGlob: brace-expansion inputs must not set DirectiveTargetFile.
+func TestLocateBuildDirectiveInputsBraceGlob(t *testing.T) {
+	t.Parallel()
+	src := "# T\n\n<?build\nrecipe: render\noutputs:\n  - \"out.png\"\ninputs:\n  - \"{a,b}.md\"\n?>\n<?/build?>\n"
+	res := Locator{Path: "a.md"}.Locate([]byte(src), 8, 5)
+	assert.Equal(t, TokenDirectiveArg, res.Tag)
+	assert.Equal(t, "build", res.DirectiveName)
+	assert.Equal(t, "inputs", res.DirectiveArg)
+	assert.Empty(t, res.DirectiveTargetFile, "brace-expansion input must not set DirectiveTargetFile")
+}

internal/rules/build/rule.go

@@ -296,8 +296,8 @@ func (r *Rule) generateBody(
 	rendered := make([]string, 0, len(outputs))
 	for _, output := range outputs {
 		alt := fmt.Sprintf("%s output: %s", recipeName, output)
-		rendered = append(rendered,
-			strings.NewReplacer("{alt}", alt, "{output}", output).Replace(tmpl))
+		body := strings.NewReplacer("{output}", output, "{alt}", alt).Replace(tmpl)
+		rendered = append(rendered, body)
 	}
 	body := strings.Join(rendered, "\n")
 
@@ -369,11 +369,11 @@ func validatePathEntry(p string, allowGlob bool) string {
 	if strings.HasPrefix(p, "/") || strings.HasPrefix(p, "~") {
 		return "must be a relative path"
 	}
-	if !allowGlob && strings.ContainsAny(p, "*?[") {
+	if !allowGlob && strings.ContainsAny(p, "*?[{") {
 		return "must not contain glob characters"
 	}
 	cleaned := path.Clean(p)
-	if cleaned == ".." || strings.HasPrefix(cleaned, "../") {
+	if cleaned == ".." || cleaned == "." || strings.HasPrefix(cleaned, "../") {
 		return `must not contain ".." path components`
 	}
 	if underMdsmithDir(cleaned) {

internal/rules/build/rule_test.go

@@ -699,7 +699,6 @@ func TestValidatePathEntry_UNC(t *testing.T) {
 }
 
 func TestValidatePathEntry_NTFSADS(t *testing.T) {
-	// foo:bar — NTFS alternate data stream syntax.
 	assert.NotEmpty(t, validatePathEntry("foo:bar", false))
 	assert.NotEmpty(t, validatePathEntry("dir/foo:bar.txt", false))
 }
@@ -714,7 +713,6 @@ func TestValidatePathEntry_ReservedDeviceNames(t *testing.T) {
 }
 
 func TestValidatePathEntry_ReservedDeviceNames_NotMatchedAsSubstring(t *testing.T) {
-	// CONSOLE / NULLABLE are not reserved device names.
 	for _, p := range []string{"CONSOLE.md", "NULLABLE", "COMPANY", "LPT10"} {
 		assert.Empty(t, validatePathEntry(p, false), "path %q should be accepted", p)
 	}
@@ -733,34 +731,34 @@ func TestValidatePathEntry_Tilde(t *testing.T) {
 }
 
 func TestValidatePathEntry_DotDot(t *testing.T) {
-	// A path that escapes root after Clean (or is "..") is rejected.
 	for _, p := range []string{"../out.png", "..", "a/../../b.png"} {
 		assert.NotEmpty(t, validatePathEntry(p, false), "path %q should be rejected", p)
 	}
 }
 
 func TestValidatePathEntry_InteriorDotDotThatCleansInBounds(t *testing.T) {
-	// Per the plan's path-shape rule, the check is on the result of
-	// path.Clean: "a/../b.png" cleans to "b.png", which stays in-root,
-	// so it is accepted.
+	// "a/../b.png" cleans to "b.png" (stays in-root), so it is accepted.
 	assert.Empty(t, validatePathEntry("a/../b.png", false))
 }
 
+func TestValidatePathEntry_DotDotCollapsesToRoot(t *testing.T) {
+	// "a/.." cleans to "." (workspace root) — reject to prevent artifacts at ".".
+	assert.NotEmpty(t, validatePathEntry("a/..", false))
+}
+
 func TestValidatePathEntry_UnderMdsmithDir(t *testing.T) {
 	for _, p := range []string{".mdsmith/state", ".mdsmith/out.png"} {
 		assert.NotEmpty(t, validatePathEntry(p, false), "path %q should be rejected", p)
 	}
 }
 
 func TestValidatePathEntry_OutputsRejectGlobChars(t *testing.T) {
-	// allowGlob=false: glob meta-characters are rejected.
-	for _, p := range []string{"out*.png", "out?.png", "out[1].png"} {
+	for _, p := range []string{"out*.png", "out?.png", "out[1].png", "out{a,b}.png"} {
 		assert.NotEmpty(t, validatePathEntry(p, false), "path %q should be rejected for outputs", p)
 	}
 }
 
 func TestValidatePathEntry_InputsAcceptGlobChars(t *testing.T) {
-	// allowGlob=true: doublestar globs are accepted.
 	for _, p := range []string{
 		"src/*.md", "**/*.md", "chapters/[0-9]*.md", "a?b.md", "{a,b}.md",
 	} {

internal/rules/recipesafety/rule.go

@@ -268,8 +268,7 @@ func hasDotDotSegment(p string) bool {
 func (r *Rule) checkTokens(filePath, name string, tokens []string) []lint.Diagnostic {
 	var diags []lint.Diagnostic
 	for _, tok := range tokens {
-		isSinglePlaceholder := placeholderRe.MatchString(tok) &&
-			placeholderRe.FindString(tok) == tok
+		isSinglePlaceholder := placeholderRe.FindString(tok) == tok
 		if !isSinglePlaceholder {
 			for _, op := range shellOperators {
 				if strings.Contains(tok, op) {
@@ -280,16 +279,42 @@ func (r *Rule) checkTokens(filePath, name string, tokens []string) []lint.Diagno
 				}
 			}
 		}
-		if fusedRe.MatchString(tok) {
-			fused := fusedRe.FindString(tok)
+		if fused := fusedRe.FindString(tok); fused != "" {
 			diags = append(diags, r.diag(filePath, lint.Error,
 				fmt.Sprintf("recipe %q: command contains fused placeholders %q — separate with a delimiter",
 					name, fused)))
+		} else {
+			diags = append(diags, r.checkReservedTokenShape(filePath, name, tok)...)
 		}
 	}
 	return diags
 }
 
+// checkReservedTokenShape reports an error when a reserved collective
+// placeholder ({outputs} or {inputs}) is embedded in a larger argv
+// token rather than standing alone — list-expanding a fragment of a
+// token has no well-defined semantics.
+func (r *Rule) checkReservedTokenShape(filePath, name, tok string) []lint.Diagnostic {
+	var diags []lint.Diagnostic
+	for _, m := range placeholderRe.FindAllStringSubmatch(tok, -1) {
+		if isReservedParamName(m[1]) && tok != m[0] {
+			diags = append(diags, r.diag(filePath, lint.Error,
+				fmt.Sprintf("recipe %q: reserved placeholder %q must be a standalone argv token, not embedded in %q",
+					name, m[0], tok)))
+		}
+	}
+	return diags
+}
+
+func isReservedParamName(name string) bool {
+	for _, r := range reservedParamNames {
+		if name == r {
+			return true
+		}
+	}
+	return false
+}
+
 func (r *Rule) checkUnusedParams(filePath, name string, rec recipe) []lint.Diagnostic {
 	declared := make(map[string]bool)
 	for _, p := range rec.Required {

internal/rules/recipesafety/rule_test.go

@@ -644,6 +644,36 @@ func TestCheck_MultipleRecipes_SortedByName(t *testing.T) {
 	assert.Contains(t, diags[1].Message, `"z-recipe"`)
 }
 
+// --- checkReservedTokenShape ---
+
+func TestCheck_EmbeddedReservedPlaceholder_Outputs(t *testing.T) {
+	t.Parallel()
+	// {outputs} embedded inside a larger token — must be standalone.
+	r := newRule(map[string]recipe{"x": {Command: "tool --out={outputs}"}})
+	diags := r.Check(newFile(t, "f.md"))
+	require.Len(t, diags, 1)
+	assert.Equal(t, lint.Error, diags[0].Severity)
+	assert.Contains(t, diags[0].Message, `reserved placeholder "{outputs}" must be a standalone argv token`)
+}
+
+func TestCheck_EmbeddedReservedPlaceholder_Inputs(t *testing.T) {
+	t.Parallel()
+	// {inputs} embedded inside a larger token — must be standalone.
+	r := newRule(map[string]recipe{"x": {Command: "tool --in={inputs}"}})
+	diags := r.Check(newFile(t, "f.md"))
+	require.Len(t, diags, 1)
+	assert.Equal(t, lint.Error, diags[0].Severity)
+	assert.Contains(t, diags[0].Message, `reserved placeholder "{inputs}" must be a standalone argv token`)
+}
+
+func TestCheck_StandaloneReservedPlaceholder_NoDiagnostic(t *testing.T) {
+	t.Parallel()
+	// {outputs} as a standalone token is correct — no reserved-shape error.
+	r := newRule(map[string]recipe{"x": {Command: "tool {outputs} -o {inputs}"}})
+	diags := r.Check(newFile(t, "f.md"))
+	assert.Empty(t, diags)
+}
+
 // --- Diagnostic fields ---
 
 func TestCheck_DiagnosticFields(t *testing.T) {