Stability review: nine findings closed, one rejected, one observation overturned

CHANGELOG.md

@@ -349,6 +349,83 @@ because it turns other people's test suites red.
   `height` can be set, and naming one lets the other be worked out from the
   size. The smallest TIFF this produces is 183 B.
 
+### Fixed
+
+- **A run of a few files is now weighed against the memory ceiling too.** The
+  ceiling that stops a run from planning more than it can hold only started
+  counting once a run asked for sixty four files, so a smaller run had no
+  ceiling at all - and it counted files across the whole run, so a recipe of
+  sixty four one file targets began counting after sixty three of them were
+  already planned.
+
+  That was reachable with ordinary settings rather than with a contrived one. A
+  zip of ten thousand entries costs about 75 MB to plan, so twenty nine of them
+  come to 2.17 GB, past the ceiling and without a single check. Such a run is
+  now refused before anything is written, and the refusal says how far it had
+  got and what the ceiling is.
+
+  Runs this tool was designed around are orders of magnitude under the ceiling
+  and are unaffected. Nothing about the files that are produced changes.
+
+- **A crash inside a generator now costs one file instead of the whole run.**
+  Until this release a defect in one of them ended the process, and it left the
+  file it was writing on the disk under its temporary name - a name `cleanup`
+  will not remove, because `cleanup` only removes what the manifest lists, and a
+  file that never finished never reached one. `verify` then reported it for
+  good.
+
+  Such a crash is now an ordinary failure of one file: the rest of the run
+  carries on, the manifest says which file it was and what happened, the
+  temporary file is removed, and the run ends with the partial exit code. A
+  crash while planning ends the run instead, with the exit code that means this
+  tool has a defect rather than the recipe does.
+
+  This is a safety net, not a licence. A crash is still a defect worth
+  reporting, and the message says so.
+
+- **Ctrl+C now stops `generate`, `validate` and `preset show` while they are
+  still planning.** Until this release they finished planning first and noticed
+  the key only afterwards, so a large batch could look frozen: ten thousand
+  pictures is about a minute and a half of planning before the first byte is
+  written, and all of it ignored the key. Under a CI timeout the grace period
+  ran out and the job was killed rather than shutting down.
+
+  Writing was never affected. A run interrupted while producing files already
+  stopped promptly, saved its manifest and left no partial files behind.
+
+- **A recipe can no longer use up all the memory on the machine.** A document
+  of 40 kB that nested brackets twenty thousand deep took most of a gigabyte
+  before it was refused, and the size limit did not help - forty kilobytes is a
+  small fraction of what a recipe is allowed to be.
+
+  Recipes now refuse to nest brackets and braces more than thirty two deep.
+  Nothing anybody writes comes close: a list such as `[1B, 1kb, 1mb]` is one
+  deep, and so is a target written out with braces. Brackets inside a quoted
+  value are text and are not counted.
+
+- **A compressed `zip` is now either produced at the size you asked for or
+  refused, never quietly written at the wrong length.** Two sizes did the
+  second thing, and both passed `--dry-run` first.
+
+  Asking for a compressed zip at exactly the smallest size the tool reports
+  produced no file at all. That size is now refused, and the smallest
+  compressed zip is a little larger than the smallest stored one - the padding
+  entry a compressed archive needs costs a few bytes of its own. `tfg formats`
+  is unchanged, because it reports the stored floor and compression is off by
+  default.
+
+  Asking for a zip whose contents are already compressed - other zips, Office
+  documents, pictures - could also land in a narrow band of sizes that could
+  not be produced. Deflate makes such data slightly larger rather than smaller,
+  and the padding could not shrink far enough to make up for it. Those sizes
+  are now refused with the smallest size that does work.
+
+  Sizes that worked before are unaffected, byte for byte.
+
+- **A `validate --json` that is stopped no longer says the recipe is invalid.**
+  It never finished reading the recipe, so it has no verdict to report. The
+  exit code says what happened instead.
+
 ## [0.2.0] - 2026-08-28
 
 ### Breaking

internal/cli/cli.go

@@ -72,15 +72,15 @@ func Run(ctx context.Context, args []string, out, errOut io.Writer) int {
 	case "generate":
 		return generate(ctx, args[1:], out, errOut)
 	case "validate":
-		return validate(args[1:], out, errOut)
+		return validate(ctx, args[1:], out, errOut)
 	case "verify":
 		return verify(ctx, args[1:], out, errOut)
 	case "cleanup":
 		return cleanup(ctx, args[1:], out, errOut)
 	case "recipe":
 		return recipeCmd(args[1:], out, errOut)
 	case "preset":
-		return presetCmd(args[1:], out, errOut)
+		return presetCmd(ctx, args[1:], out, errOut)
 	case "formats":
 		return formats(args[1:], out, errOut)
 	case "--version", "version":

internal/cli/errors.go

@@ -146,6 +146,13 @@ func classifyRequest(err error) (int, bool) {
 	if errors.As(err, &recipeTooLarge) {
 		return ExitRecipe, true
 	}
+	// Same class, one more shape: a recipe whose brackets nest far enough to
+	// exhaust this machine. A fact about the document rather than a fault of
+	// ours, so it ends the way a recipe that will not parse does.
+	var recipeTooDeep *recipe.TooDeepError
+	if errors.As(err, &recipeTooDeep) {
+		return ExitRecipe, true
+	}
 	// Two parts of one recipe saying different things about the same archive
 	// is a recipe problem, like a boundary stated beside a size.
 	var conflict *format.ContentsConflictError

internal/cli/generate.go

@@ -372,7 +372,13 @@ func sizesFromFlags(g *generateOpts, errOut io.Writer) (sizes []int64, low, high
 
 // produce plans the run, writes it and reports what happened.
 func produce(ctx context.Context, targets []engine.Target, opt engine.Options, g *generateOpts, out, errOut io.Writer) int {
-	planned, err := engine.Plan(targets, opt)
+	// PlanContext rather than Plan, because planning is where the time goes for
+	// anything that encodes a picture and it is work somebody may want to stop.
+	// Measured 2026-09-02: about 8.5 ms a png, so ten thousand of them is about
+	// a minute and a half before a byte is written. preflight would notice the
+	// signal on the way to writing, which is late enough that Ctrl+C looks
+	// ignored and SIGTERM in CI runs out its grace period.
+	planned, err := engine.PlanContext(ctx, targets, opt)
 	if err != nil {
 		fmt.Fprintf(errOut, "tfg: %s\n", describeError(err))
 		return classify(err)

internal/cli/preset.go

@@ -2,6 +2,7 @@
 package cli
 
 import (
+	"context"
 	"flag"
 	"fmt"
 	"io"
@@ -49,7 +50,7 @@ type budget struct {
 	Formats []string `json:"formats"`
 }
 
-func budgetOf(e *preset.Expansion) (budget, error) {
+func budgetOf(ctx context.Context, e *preset.Expansion) (budget, error) {
 	rec, err := recipe.Parse(e.Source, e.Preset.ID)
 	if err != nil {
 		return budget{}, err
@@ -60,7 +61,7 @@ func budgetOf(e *preset.Expansion) (budget, error) {
 		targets = append(targets, engineTarget(t, t.Label))
 		seen[t.Format] = true
 	}
-	planned, err := engine.Plan(targets, engine.Options{OutDir: rec.Output.Dir, Seed: rec.Seed})
+	planned, err := engine.PlanContext(ctx, targets, engine.Options{OutDir: rec.Output.Dir, Seed: rec.Seed})
 	if err != nil {
 		return budget{}, err
 	}
@@ -338,13 +339,13 @@ func targetsFromPreset(fs *flag.FlagSet, g *generateOpts, given map[string]bool,
 	return targetsFromParsedRecipe(rec, hash, g, given, opt), ExitOK
 }
 
-func presetCmd(args []string, out, errOut io.Writer) int {
+func presetCmd(ctx context.Context, args []string, out, errOut io.Writer) int {
 	if len(args) > 0 {
 		switch args[0] {
 		case "list":
 			return presetList(args[1:], out, errOut)
 		case "show":
-			return presetShow(args[1:], out, errOut)
+			return presetShow(ctx, args[1:], out, errOut)
 		case "eject":
 			return presetEject(args[1:], out, errOut)
 		}
@@ -483,7 +484,7 @@ Flags:
 	return ExitOK
 }
 
-func presetShow(args []string, out, errOut io.Writer) int {
+func presetShow(ctx context.Context, args []string, out, errOut io.Writer) int {
 	usage := func(w io.Writer) {
 		fmt.Fprint(w, `tfg preset show - what a preset takes and what it would produce.
 
@@ -502,7 +503,7 @@ Usage:
 		return code
 	}
 
-	b, err := budgetOf(expanded)
+	b, err := budgetOf(ctx, expanded)
 	if err != nil {
 		fmt.Fprintf(errOut, "tfg: %s\n", describeError(err))
 		return classify(err)

internal/cli/recipecmd.go

@@ -3,6 +3,7 @@ package cli
 
 import (
 	"bytes"
+	"context"
 	"errors"
 	"flag"
 	"fmt"
@@ -36,7 +37,7 @@ func loadRecipe(path string, errOut io.Writer) (*recipe.Recipe, string, int) {
 
 // validate runs the checks a run would run and writes nothing at all, so it
 // suits a pre commit hook.
-func validate(args []string, out, errOut io.Writer) int {
+func validate(ctx context.Context, args []string, out, errOut io.Writer) int {
 	fs := flag.NewFlagSet("validate", flag.ContinueOnError)
 	fs.SetOutput(errOut)
 	asJSON := fs.Bool("json", false, "write the result as JSON, with every problem separately")
@@ -79,14 +80,9 @@ func validate(args []string, out, errOut io.Writer) int {
 	for _, t := range rec.Targets {
 		targets = append(targets, engineTarget(t, t.Label))
 	}
-	planned, err := engine.Plan(targets, planningOptions(rec))
+	planned, err := engine.PlanContext(ctx, targets, planningOptions(rec))
 	if err != nil {
-		if *asJSON {
-			return writeJSON(errOut, errOut, validateReport{Recipe: path, Valid: false,
-				Problems: []validateProblem{problemOf(err)}}, classify(err))
-		}
-		fmt.Fprintf(errOut, "tfg: %s\n", describeError(err))
-		return classify(err)
+		return planningRefusal(err, path, *asJSON, errOut)
 	}
 
 	if *asJSON {
@@ -103,6 +99,25 @@ func validate(args []string, out, errOut io.Writer) int {
 	return ExitOK
 }
 
+// planningRefusal reports a plan that did not finish, and gives the exit code
+// for it.
+//
+// The stopped case is told apart because a run that was stopped has no verdict
+// about the recipe and must not print one. "valid: false" is a claim about the
+// file, and all that happened is that somebody pressed Ctrl+C - a consumer
+// reading that one field would act on a recipe this never finished reading. The
+// exit code says what really happened. Untouchable rule 5 in the other surface:
+// do not claim a certainty there is none of.
+func planningRefusal(err error, path string, asJSON bool, errOut io.Writer) int {
+	stopped := errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)
+	if asJSON && !stopped {
+		return writeJSON(errOut, errOut, validateReport{Recipe: path, Valid: false,
+			Problems: []validateProblem{problemOf(err)}}, classify(err))
+	}
+	fmt.Fprintf(errOut, "tfg: %s\n", describeError(err))
+	return classify(err)
+}
+
 // planningOptions is what this command hands the planner.
 //
 // The manifest name is in it, and leaving it out was a hole this command exists

internal/core/diskspace.go

@@ -0,0 +1,38 @@
+package core
+
+import "math"
+
+// AvailableFrom turns what the system said about free space into a number of
+// bytes this program can work with.
+//
+// The two platform files below both had the same shape at the end of them: a
+// number the system reports as unsigned, multiplied or converted into an int64
+// and handed on. On a filesystem of eight exbibytes or more that conversion
+// wraps and free space comes back NEGATIVE, which the free space check reads as
+// a disk smaller than any run - so the tool would refuse to write to the
+// largest disk it will ever meet.
+//
+// Nobody has such a disk. It is here as a pure function rather than as a
+// condition inside the syscall because a condition in there is one nothing
+// could ever turn red, and this project has taken seven of those back out. As a
+// function it can be asked directly, which is what a guard does.
+//
+// Saturating rather than erroring, and that is the truthful answer rather than
+// the convenient one: a disk too large to count in int64 is a disk with more
+// room than any run this tool can plan, so the largest number is what the
+// caller should hear. An error would be read as "this disk cannot be measured",
+// which is a different thing and is already what a failing syscall means.
+func AvailableFrom(blocks, blockSize uint64) int64 {
+	if blockSize == 0 {
+		// A block size of nought would make the multiplication below divide by
+		// nought in its own overflow check. No filesystem reports it, and the
+		// answer if one did is that we know nothing about the space rather
+		// than that there is none - but nought is what a failing syscall
+		// already returns beside its error, so it is what this returns too.
+		return 0
+	}
+	if blocks > uint64(math.MaxInt64)/blockSize {
+		return math.MaxInt64
+	}
+	return int64(blocks * blockSize)
+}

internal/core/diskspace_unix.go

@@ -20,8 +20,10 @@ func AvailableBytes(path string) (int64, error) {
 		return 0, err
 	}
 	// Blocks available to an unprivileged user, which is the number that
-	// matters to us rather than the total free on the device.
-	return int64(st.Bavail) * int64(st.Bsize), nil
+	// matters to us rather than the total free on the device. The arithmetic
+	// is next door because it is the half that can be wrong on its own, and a
+	// check written in here would be one nothing could ever reach.
+	return AvailableFrom(uint64(st.Bavail), uint64(st.Bsize)), nil
 }
 
 // existingAncestor walks up until it finds a directory that exists, because

internal/core/diskspace_windows.go

@@ -34,7 +34,10 @@ func AvailableBytes(path string) (int64, error) {
 	if r == 0 {
 		return 0, e
 	}
-	return int64(freeForCaller), nil
+	// Already in bytes, so the block size is one - but through the same
+	// function as the other platform, because the conversion from unsigned is
+	// where both of them could hand back a negative.
+	return AvailableFrom(freeForCaller, 1), nil
 }
 
 // The handle is looked up once rather than on every call. It was built inside

internal/core/records.go

@@ -153,6 +153,22 @@ func AppendFiller(dst []byte, words []string, n int64, separator func(i int) str
 		// which mistake it is beats a runtime panic with no name on it.
 		panic("core: AppendFiller was given no words to pad with")
 	}
+	if !wordsHaveBytes(words) {
+		// The same mistake spelled differently, and it ends worse. The loop
+		// below stops only by adding bytes, so a vocabulary that adds none
+		// spins without end and without growing - the one failure shape a size
+		// guard cannot see, because no file is ever produced to measure.
+		// FillRecords above names it in those words and refuses. This one did
+		// not, which made it a difference between two halves of one primitive
+		// rather than a hypothetical.
+		//
+		// The question is about the VOCABULARY rather than about each word,
+		// and that distinction is the whole of the check. One word with bytes
+		// in it ends the loop whatever the separator returns, because every
+		// cycle through the vocabulary appends it - so asking per iteration
+		// would refuse ["", "ab"], which pads perfectly well.
+		panic("core: AppendFiller was given words that are all empty, so it could never reach the length asked for")
+	}
 
 	start := len(dst)
 	for i := 0; int64(len(dst)-start) < n; i++ {
@@ -201,3 +217,17 @@ func WriteAll(w io.Writer, b []byte) error {
 	}
 	return nil
 }
+
+// wordsHaveBytes says whether a vocabulary can add anything at all.
+//
+// One word is enough, whatever the separator does with the rest, because the
+// loop in AppendFiller walks the vocabulary in a cycle and reaches that word
+// every time round.
+func wordsHaveBytes(words []string) bool {
+	for _, w := range words {
+		if w != "" {
+			return true
+		}
+	}
+	return false
+}