From 703726b48d01244e8efd1221834a9c0aea36fa78 Mon Sep 17 00:00:00 2001
From: Nicolas CHAUVIN <nicolas.chauvin@gmail.com>
Date: Wed, 9 Sep 2026 11:28:16 +0200
Subject: [PATCH] fix(lang): refuse a def call missing a required argument

---
 CHANGELOG.md                          |  4 +++
 internal/lang/eval.go                 | 14 +++++++--
 internal/lang/text_primitives_test.go | 44 +++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5c076f8..f70ed28 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Fixed
+
+- A def calling another instruction with too few arguments is refused instead of binding the missing one to the empty string. `file.write(path)` inside a def overwrote the file with nothing and reported `ok.done` — a file destroyed under a success verdict. A plan-level call was always checked on both bounds; only the def side was not (#633).
+
 ## [0.13.0] - 2026-09-09
 
 ### Changed
diff --git a/internal/lang/eval.go b/internal/lang/eval.go
index 7ef857f..e673a67 100644
--- a/internal/lang/eval.go
+++ b/internal/lang/eval.go
@@ -702,8 +702,18 @@ func (ev *evaluator) evalCall(c Call) value {
 	if !ok {
 		ev.fail("unknown instruction %q", c.Name)
 	}
-	if len(c.Args) > len(def.Params) {
-		ev.fail("%s takes %d argument(s), got %d", c.Name, len(def.Params), len(c.Args))
+	// Both bounds, as a plan-level call has always been checked (parser.go). Only the
+	// upper one was tested here, so a call missing an argument bound the parameter to the
+	// empty string: `file.write(path)` overwrote a file with nothing and reported
+	// `ok.done` — a file destroyed under a success verdict (#633).
+	//
+	// The lower bound is the count of parameters with no default, since omitting a
+	// defaulted one is what defaults are for.
+	if req := requiredCount(def); len(c.Args) < req || len(c.Args) > len(def.Params) {
+		if req == len(def.Params) {
+			ev.fail("%s takes %d argument(s), got %d", c.Name, req, len(c.Args))
+		}
+		ev.fail("%s takes %d–%d argument(s), got %d", c.Name, req, len(def.Params), len(c.Args))
 	}
 	// Positional arguments, evaluated in THIS def's scope, then handed over as the
 	// callee's own params. Nothing else of this def crosses over.
diff --git a/internal/lang/text_primitives_test.go b/internal/lang/text_primitives_test.go
index 6fcd111..6f5789f 100644
--- a/internal/lang/text_primitives_test.go
+++ b/internal/lang/text_primitives_test.go
@@ -112,3 +112,47 @@ func TestText_Arity(t *testing.T) {
 		})
 	}
 }
+
+// #633. A def calling another instruction with too few arguments was accepted: the missing
+// parameter bound to the empty string, so `file.write(p)` overwrote a file with nothing and
+// reported `ok.done`. Reproduced on a real target before this test existed.
+//
+// The plan path checks both bounds (parser.go). This one checked only the upper one, which
+// is why the same mistake is refused in a plan and silent in a def — where defs compose and
+// nobody reads the call site.
+func TestCallArity_TooFewIsRefused(t *testing.T) {
+	src := `def callee(a: str, b: str) { check { return ok.done } }
+def caller(p: str) { check { callee(p) return ok.done } }`
+	_, err := evalWithFetch(t, src, "caller", map[string]string{"p": "x"}, nil)
+	if err == nil {
+		t.Fatal("a call missing a required argument must be refused, not bound to empty")
+	}
+	for _, want := range []string{"callee", "2", "1"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Fatalf("the refusal must name the callee and both counts, missing %q: %v", want, err)
+		}
+	}
+}
+
+// The other bound still holds, and its message is unchanged.
+func TestCallArity_TooManyIsStillRefused(t *testing.T) {
+	src := `def callee(a: str) { check { return ok.done } }
+def caller(p: str) { check { callee(p, p) return ok.done } }`
+	if _, err := evalWithFetch(t, src, "caller", map[string]string{"p": "x"}, nil); err == nil {
+		t.Fatal("too many arguments must stay refused")
+	}
+}
+
+// A parameter with a default may be omitted — that is what defaults are for, and four
+// stdlib defs rely on it (`docker.prune`, `dir.copy`, `dir.sync`, `docker.compose-restart`).
+func TestCallArity_ADefaultedParameterMayBeOmitted(t *testing.T) {
+	src := `def callee(a: str, b: str = "fallback") { check { if b == "fallback" { return ok.defaulted } return err.bound } }
+def caller(p: str) { check { c = callee(p) if c { return ok.defaulted } return err.bound } }`
+	res, err := evalWithFetch(t, src, "caller", map[string]string{"p": "x"}, nil)
+	if err != nil {
+		t.Fatalf("omitting a defaulted parameter must stay legal: %v", err)
+	}
+	if got := res.String(); got != "ok.defaulted" {
+		t.Fatalf("got %s, want ok.defaulted — the default must reach the callee", got)
+	}
+}