From 36e3c446ee60ca7507343685ff54b58e48a0841d Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 27 Oct 2026 18:26:09 +0100 Subject: [PATCH 1/5] engine: drawing the sizes of a range stops when the run is stopped Every probe of the range draw planned its file with a context of its own, so Ctrl+C waited for the whole draw: 13.7 to 15.8 s for three thousand jpg files with --size-range, against 0.01 s for a fixed size. The run's context now reaches the draw. Co-Authored-By: Claude Opus 5.5 --- internal/engine/drawsizes.go | 25 +++++++---- internal/engine/engine.go | 6 +-- internal/guard/mutation/entries.py | 23 +++++++++- internal/guard/rangestops_test.go | 68 ++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 13 deletions(-) create mode 100644 internal/guard/rangestops_test.go diff --git a/internal/engine/drawsizes.go b/internal/engine/drawsizes.go index 1274ad07..dee68a2d 100644 --- a/internal/engine/drawsizes.go +++ b/internal/engine/drawsizes.go @@ -41,11 +41,11 @@ import ( // SmallestAccepted is called from guards and from the window, where that wrapper // is not in hand. Merging them would mean handing a planner in, which buys less // than it costs. -func firstWritable(desc format.Descriptor, r format.Request, from, limit int64) (int64, error) { +func firstWritable(ctx context.Context, desc format.Descriptor, r format.Request, from, limit int64) (int64, error) { var first error for at := from; at <= limit; { r.Bytes = at - _, err := planWithoutCrashing(context.Background(), desc, r) + _, err := planWithoutCrashing(ctx, desc, r) if err == nil { return at, nil } @@ -100,7 +100,15 @@ func firstWritable(desc format.Descriptor, r format.Request, from, limit int64) // Bytes do not move for any run that worked before. A run that succeeds today // has every drawn size writable, or it would have failed, and snapping a // writable size returns it unchanged. -func drawSizes(t *Target, desc format.Descriptor, targetSeed uint64) error { +// +// It takes the run's context because drawing IS planning: every probe plans +// the file, and a jpg plans by encoding its picture. Until 2026-10-27 each +// probe got a context of its own, so a stop waited for the draw to finish - +// 13.7 to 15.8 s after Ctrl+Break for three thousand jpg files, where a fixed +// size stopped in 0.01 s (docs/STABILITY-REVIEW-2026-10-26.md, R05). +// format.PlanWith asks the context before anything else, and a stop is not a +// refusal about the size, so firstWritable hands it straight back. +func drawSizes(ctx context.Context, t *Target, desc format.Descriptor, targetSeed uint64) error { first := format.Request{ Contains: t.Contains, Seed: core.FileSeed(targetSeed, 0), @@ -120,7 +128,7 @@ func drawSizes(t *Target, desc format.Descriptor, targetSeed uint64) error { // UTF-16, the band above a PNG's encoded picture - is a different thing: // nobody can be expected to enumerate those, and snapping inside the range // is the answer. - floor, err := firstWritable(desc, first, 0, t.Range.Max) + floor, err := firstWritable(ctx, desc, first, 0, t.Range.Max) if err != nil { // The floor is above the whole range, so nothing in it is writable. return err @@ -129,7 +137,7 @@ func drawSizes(t *Target, desc format.Descriptor, targetSeed uint64) error { // Asked again at the low end so the format words its own refusal, with // the number the person actually wrote. first.Bytes = t.Range.Min - if _, err := planWithoutCrashing(context.Background(), desc, first); err != nil { + if _, err := planWithoutCrashing(ctx, desc, first); err != nil { return err } } @@ -154,13 +162,14 @@ func drawSizes(t *Target, desc format.Descriptor, targetSeed uint64) error { Properties: t.Properties, } - got, err := firstWritable(desc, req, want, t.Range.Max) + got, err := firstWritable(ctx, desc, req, want, t.Range.Max) if err != nil && want > t.Range.Min { // Nothing writable from the draw upwards. The bottom of the range // can still hold something - a draw landing on the last odd number // of a range has nowhere above it and plenty below - so the range is - // only empty once THAT fails too. - got, err = firstWritable(desc, req, t.Range.Min, t.Range.Max) + // only empty once THAT fails too. After a stop this second try + // hands the stop back at its first probe. + got, err = firstWritable(ctx, desc, req, t.Range.Min, t.Range.Max) } if err != nil { return err diff --git a/internal/engine/engine.go b/internal/engine/engine.go index ad84d819..67769d30 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -251,7 +251,7 @@ func (f PlannedFile) Damaged() bool { // askable at all", and the loop it was taken out of answers "what files does it // come to". seen carries across targets because a duplicate id is a fact about // the run rather than about one entry. -func settleTarget(t *Target, opt Options, seen map[string]bool) (format.Descriptor, error) { +func settleTarget(ctx context.Context, t *Target, opt Options, seen map[string]bool) (format.Descriptor, error) { if t.ID == "" { return format.Descriptor{}, &RecipeError{Setting: SettingID, Detail: core.Says("engine.NoID", "a target has no id"), @@ -297,7 +297,7 @@ func settleTarget(t *Target, opt Options, seen map[string]bool) (format.Descript // for every registered format. It was removed rather than kept as defence // nobody can verify. if t.Range.Used { - if err := drawSizes(t, desc, core.TargetSeed(opt.Seed, t.ID)); err != nil { + if err := drawSizes(ctx, t, desc, core.TargetSeed(opt.Seed, t.ID)); err != nil { return format.Descriptor{}, err } } @@ -427,7 +427,7 @@ func PlanContext(ctx context.Context, targets []Target, opt Options) ([]PlannedF // Everything refused from here down is refused about one target, so it // carries which one. See atTarget for what that is worth and what it // deliberately leaves alone. - desc, err := settleTarget(t, opt, seen) + desc, err := settleTarget(ctx, t, opt, seen) if err != nil { return nil, atTarget(i+1, t, err) } diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index 5c8988dc..9039723a 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -8678,8 +8678,8 @@ ( "snapping starts from somewhere that depends on how many files were asked for", "internal/engine/drawsizes.go", - "\t\tgot, err := firstWritable(desc, req, want, t.Range.Max)", - "\t\tgot, err := firstWritable(desc, req, want+int64(len(t.Sizes)), t.Range.Max)", + "\t\tgot, err := firstWritable(ctx, desc, req, want, t.Range.Max)", + "\t\tgot, err := firstWritable(ctx, desc, req, want+int64(len(t.Sizes)), t.Range.Max)", "TestSnappingStillLeavesTheEarlierFilesAlone", ), # --- HTML is a whole page or only the blocks that sit inside one @@ -15833,4 +15833,23 @@ "return Fits(width, chars) && height > textHeight(width, chars)", "TestALabelIsReadableFromTheFirstHeightThatHoldsItsText", ), + ( + # R05: a probe of the range draw plans the file with a context of its + # own again, so a stop waits for the whole draw. The range under the + # pdf floor is refused by the format in both halves of the guard. + "a range draws its sizes without looking at the stop", + "internal/engine/drawsizes.go", + "_, err := planWithoutCrashing(ctx, desc, r)", + "_, err := planWithoutCrashing(context.Background(), desc, r)", + "TestARangeStopsWhileItsSizesAreDrawn", + ), + ( + # The same, one layer up: the engine stops handing its context to the + # draw, which is the shape the draw had from 2026-09-08 until R05. + "the engine draws the sizes of a range with a context of its own", + "internal/engine/engine.go", + "if err := drawSizes(ctx, t, desc, core.TargetSeed(opt.Seed, t.ID)); err != nil {", + "if err := drawSizes(context.Background(), t, desc, core.TargetSeed(opt.Seed, t.ID)); err != nil {", + "TestARangeStopsWhileItsSizesAreDrawn", + ), ] diff --git a/internal/guard/rangestops_test.go b/internal/guard/rangestops_test.go new file mode 100644 index 00000000..963b3eaa --- /dev/null +++ b/internal/guard/rangestops_test.go @@ -0,0 +1,68 @@ +package guard + +import ( + "bytes" + "context" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/cli" +) + +// Drawing the sizes of a range is planning, and it has to stop when it is told +// to - docs/STABILITY-REVIEW-2026-10-26.md, R05. +// +// The size range with snapping arrived on 2026-09-08, six days after planning +// was made stoppable, and it planned every file of the range with a context of +// its own. TestPlanningStopsBeforeItWalksToTheFarTarget stayed green, because +// it never asks for a range - the guard did not reach the new path. Measured on +// 2026-10-27 with a real Ctrl+Break: three thousand jpg files, --size-range, +// --dry-run, and the run went on for 13.7 to 15.8 s after the signal, where a +// fixed size ended in 0.01 s. +// +// The same two halves as the far target guard, and for the same reason they +// need no clock. The range lies wholly under the smallest pdf, so drawing its +// sizes is where the run is refused - with a live context by the format, with +// a finished one by the stop. A draw that ignores the context answers the +// format's refusal both times. +func TestARangeStopsWhileItsSizesAreDrawn(t *testing.T) { + cases := []struct { + name string + args func(dir string) []string + }{ + {"generate", func(dir string) []string { + return []string{"generate", "--format", "pdf", "--size-range", "1-10", "--out", dir, "--dry-run"} + }}, + {"validate", func(dir string) []string { + return []string{"validate", writeRecipe(t, dir, rangeUnderTheFloorRecipeBody)} + }}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + args := c.args(t.TempDir()) + + // The live half asserts the state the stopped half depends on: the + // refusal really comes from drawing the sizes. If the range ever + // became writable, the stopped half would end with the interrupt + // code for a reason that has nothing to do with the draw. + var out, errOut bytes.Buffer + if code := cli.Run(context.Background(), args, &out, &errOut); code != cli.ExitFormat { + t.Fatalf("with a live context this ended with %d, expected %d from the range under the floor - the two halves would no longer differ\nstderr: %s", + code, cli.ExitFormat, errOut.String()) + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + out.Reset() + errOut.Reset() + + if code := cli.Run(ctx, args, &out, &errOut); code != cli.ExitInterrupted { + t.Errorf("with a finished context this ended with %d, expected %d - the sizes of the range were drawn without looking at the context\nstderr: %s", + code, cli.ExitInterrupted, errOut.String()) + } + }) + } +} + +// rangeUnderTheFloorRecipeBody is a pdf range no pdf can be inside. +const rangeUnderTheFloorRecipeBody = "version: 1\nseed: 7\noutput:\n dir: out\ntargets:\n - id: a\n format: pdf\n size-range: 1-10\n count: 3\n" From 60a00d1cc8483ae4aed8515d919a30b0e055ccb7 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 27 Oct 2026 18:34:46 +0100 Subject: [PATCH 2/5] opc: the padding checksum stops when the write is stopped An Office package hashes its padding before writing it, and that pass ran under a context of its own, so a stop waited for up to four gigabytes of generated bytes: about half a second for a 2 GB docx. The pass now takes the write's context and hands the stop back. A new guard walks cmd/ and internal/ and refuses a context made from nothing outside a list of the places a context begins, each with its reason - the range draw and this pass were the same mistake twice. Co-Authored-By: Claude Opus 5.5 --- internal/format/opc/opc.go | 26 +++- internal/guard/contextroots_test.go | 196 ++++++++++++++++++++++++++++ internal/guard/mutation/entries.py | 28 ++++ internal/guard/opcstop_test.go | 105 +++++++++++++++ 4 files changed, 348 insertions(+), 7 deletions(-) create mode 100644 internal/guard/contextroots_test.go create mode 100644 internal/guard/opcstop_test.go diff --git a/internal/format/opc/opc.go b/internal/format/opc/opc.go index 026df7c6..8a9b7ae3 100644 --- a/internal/format/opc/opc.go +++ b/internal/format/opc/opc.go @@ -269,7 +269,11 @@ func write(ctx context.Context, w io.Writer, p Package, withPadding bool) error // check it and once to write it. Both passes are the same arithmetic // from the same seed, which is what makes them agree. head := &zip.FileHeader{Name: FillerName, Method: zip.Store, Modified: fixedTime} - head.CRC32 = fillerChecksum(p.Seed, p.Filler) + sum, err := fillerChecksum(ctx, p.Seed, p.Filler) + if err != nil { + return err + } + head.CRC32 = sum head.UncompressedSize64 = uint64(p.Filler) head.CompressedSize64 = uint64(p.Filler) entry, err := zw.CreateRaw(head) @@ -353,14 +357,22 @@ func (c *compressor) press(body []byte) ([]byte, error) { } // fillerChecksum is the first of the two passes over the padding. -func fillerChecksum(seed uint64, n int64) uint32 { +// +// It takes the write's context, because at the zip32 ceiling this pass is four +// gigabytes of generated bytes. Until 2026-10-27 it ran under a context of its +// own and a stop waited for all of them - about half a second for a 2 GB docx, +// where a txt of the same size stopped in 0.02 s +// (docs/STABILITY-REVIEW-2026-10-26.md, R09). The stop comes back as the +// error, so no header is ever written with a checksum of a pass that did not +// finish - the old code handed back 0 for that case. +func fillerChecksum(ctx context.Context, seed uint64, n int64) (uint32, error) { sum := crc32.NewIEEE() - if err := writeFiller(context.Background(), sum, seed, n); err != nil { - // writeFiller only fails on a write error or a cancelled context, and - // a hash never refuses a write. - return 0 + if err := writeFiller(ctx, sum, seed, n); err != nil { + // writeFiller only fails on a write error or a stopped context, and a + // hash never refuses a write, so this is the stop. + return 0, err } - return sum.Sum32() + return sum.Sum32(), nil } // comment is filler a person can read, because a reader that shows the archive diff --git a/internal/guard/contextroots_test.go b/internal/guard/contextroots_test.go new file mode 100644 index 00000000..e35cddfa --- /dev/null +++ b/internal/guard/contextroots_test.go @@ -0,0 +1,196 @@ +package guard + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "sort" + "strconv" + "strings" + "testing" +) + +// contextRoots is every place a context may be made from nothing, and why. +// +// Everywhere else the context arrives from the caller, because a context made +// from nothing is one nobody can stop. That mistake was made twice in the same +// way and found by a report rather than by a guard (docs/STABILITY-REVIEW- +// 2026-10-26.md, R05 and R09): the size range drew its sizes under a context +// of its own and an Office package checksummed four gigabytes of padding under +// another, so Ctrl+C waited for both. Each had a guard of its own after that, +// and neither guard reaches the next place the shape is written. This one does, +// which is pitfall 7 - a mistake that comes back is a missing rule rather than +// a missing fix. +// +// Keyed by file and by the function the call sits in, never by line, so moving +// code inside a function does not make the list lie (pitfall 11). +var contextRoots = map[string]string{ + "cmd/tfg/main.go main": "the process's own context, the one the signal cancels", + "internal/gui/window/run.go runner.onPreview": "a preview starts here, and Cancel on the screen is what stops it", + "internal/gui/window/run.go runner.startRun": "a run started in the window begins here, and Cancel is what stops it", + "internal/gui/window/tools.go startTool": "a tool started in the window begins here, and its Cancel is what stops it", + "internal/oracle/oracle.go Checker.Check": "the oracle runs somebody else's reader under a time limit of its own - a measuring instrument for the guards, never on the path of a run", + "internal/oracle/oracle.go PictureChanges": "the same instrument, a reference reader under a time limit", + "internal/oracle/oracle.go Strict": "the same instrument, a reference reader under a time limit", + "internal/engine/engine.go Plan": "the version without a context, for callers that have none - it is PlanContext, so it cannot disagree with it", + "internal/format/eml/eml.go generator.Plan": "the Generator interface's Plan for callers without a context - the engine asks PlanContext", + "internal/format/htmlfile/html.go generator.Plan": "the Generator interface's Plan for callers without a context - the engine asks PlanContext", + "internal/format/rtf/rtf.go generator.Plan": "the Generator interface's Plan for callers without a context - the engine asks PlanContext", + "internal/format/targz/targz.go generator.Plan": "the Generator interface's Plan for callers without a context - the engine asks PlanContext", + "internal/format/zip/zip.go generator.Plan": "the Generator interface's Plan for callers without a context - the engine asks PlanContext", + "internal/format/opc/opc.go size": "measures a package with the padding left out, so there is nothing long to stop", + "internal/format/zip/zip.go archiveSize": "measures an archive with the contents left out, so there is nothing long to stop", +} + +func TestAContextIsMadeFromNothingOnlyWhereItIsMeantToStart(t *testing.T) { + root := repoRoot(t) + found := map[string]bool{} + var stray []string + files := 0 + + for _, p := range packages(t) { + if p.rel != "cmd" && !strings.HasPrefix(p.rel, "cmd/") && !strings.HasPrefix(p.rel, "internal/") { + continue + } + for _, path := range p.files { + rel, err := filepath.Rel(root, path) + if err != nil { + t.Fatal(err) + } + rel = filepath.ToSlash(rel) + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatalf("parsing %s: %v", path, err) + } + files++ + for _, at := range contextsFromNothing(fset, file) { + key := rel + " " + at.function + if _, allowed := contextRoots[key]; allowed { + found[key] = true + continue + } + stray = append(stray, rel+":"+strconv.Itoa(at.line)+" in "+at.function) + } + } + } + + // The state this depends on: the walk read the tree it is about. A walk + // that found no files would find no stray context and say so in green. + if files < 100 { + t.Fatalf("read %d files under cmd/ and internal/ - the walk is not reaching the source", files) + } + sort.Strings(stray) + for _, s := range stray { + t.Errorf("%s makes a context from nothing, so whatever runs under it cannot be stopped. Take the caller's context, or add the place to contextRoots with the reason it is where a context begins", s) + } + // And the list is closed the other way, or it fills with places that + // stopped existing and keeps excusing the next one written there. + var idle []string + for key := range contextRoots { + if !found[key] { + idle = append(idle, key) + } + } + sort.Strings(idle) + for _, key := range idle { + t.Errorf("contextRoots names %q and no context is made there any more - take it off the list", key) + } +} + +// contextFromNothing is one call to context.Background or context.TODO. +type contextFromNothing struct { + function string + line int +} + +// contextsFromNothing finds every call to Background or TODO of the context +// package, under whatever name the file imports it, with the function around +// it - "Type.Method" for a method. +func contextsFromNothing(fset *token.FileSet, file *ast.File) []contextFromNothing { + name := "" + for _, imp := range file.Imports { + if path, _ := strconv.Unquote(imp.Path.Value); path == "context" { + name = "context" + if imp.Name != nil { + name = imp.Name.Name + } + } + } + if name == "" || name == "_" { + return nil + } + var out []contextFromNothing + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Body == nil { + continue + } + where := fn.Name.Name + if fn.Recv != nil && len(fn.Recv.List) == 1 { + where = receiverName(fn.Recv.List[0].Type) + "." + where + } + ast.Inspect(fn.Body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || (sel.Sel.Name != "Background" && sel.Sel.Name != "TODO") { + return true + } + if pkg, ok := sel.X.(*ast.Ident); ok && pkg.Name == name { + out = append(out, contextFromNothing{function: where, line: fset.Position(call.Pos()).Line}) + } + return true + }) + } + return out +} + +// receiverName is the type a method hangs on, without the star. +func receiverName(e ast.Expr) string { + switch v := e.(type) { + case *ast.StarExpr: + return receiverName(v.X) + case *ast.Ident: + return v.Name + case *ast.IndexExpr: + return receiverName(v.X) + } + return "?" +} + +// The finder itself, on a file written to have one of each shape: the plain +// import, a call inside a method, a call inside a function literal, and a +// selector that only looks like one. +func TestTheContextFinderSeesEveryShapeOfTheCall(t *testing.T) { + const src = `package a + +import ctxpkg "context" + +type r struct{} + +func (*r) m() { _ = ctxpkg.Background() } + +func f() { + go func() { _ = ctxpkg.TODO() }() + var other struct{ Background func() int } + _ = other.Background +} +` + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "a.go", src, 0) + if err != nil { + t.Fatal(err) + } + var got []string + for _, at := range contextsFromNothing(fset, file) { + got = append(got, at.function) + } + want := "r.m f" + if strings.Join(got, " ") != want { + t.Errorf("found the calls in %q, expected %q", strings.Join(got, " "), want) + } +} diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index 9039723a..c746f6bd 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -15852,4 +15852,32 @@ "if err := drawSizes(context.Background(), t, desc, core.TargetSeed(opt.Seed, t.ID)); err != nil {", "TestARangeStopsWhileItsSizesAreDrawn", ), + ( + # R09: the write stops handing its context to the checksum pass, so a + # stop during it waits for four gigabytes of padding to be hashed. + "an Office package checksums its padding without looking at the stop", + "internal/format/opc/opc.go", + "sum, err := fillerChecksum(ctx, p.Seed, p.Filler)", + "sum, err := fillerChecksum(context.Background(), p.Seed, p.Filler)", + "TestAnOfficePackageStopsWhileItChecksumsItsPadding", + ), + ( + # The same, one layer down: the pass takes the context and does not + # use it - the shape the pass had until R09. + "the checksum pass of the padding runs under a context of its own", + "internal/format/opc/opc.go", + "if err := writeFiller(ctx, sum, seed, n); err != nil {", + "if err := writeFiller(context.Background(), sum, seed, n); err != nil {", + "TestAnOfficePackageStopsWhileItChecksumsItsPadding", + ), + ( + # The class guard on its own: a context from nothing in the engine, in + # a place no behaviour guard of R05 or R09 looks at. Only the walk over + # the source sees it, which is why that walk exists. + "the engine makes a context from nothing in a place nobody listed", + "internal/engine/plantarget.go", + "if err := ctx.Err(); err != nil {", + "if err := context.Background().Err(); err != nil {", + "TestAContextIsMadeFromNothingOnlyWhereItIsMeantToStart", + ), ] diff --git a/internal/guard/opcstop_test.go b/internal/guard/opcstop_test.go new file mode 100644 index 00000000..61808429 --- /dev/null +++ b/internal/guard/opcstop_test.go @@ -0,0 +1,105 @@ +package guard + +import ( + "context" + "errors" + "io" + "sync/atomic" + "testing" + "time" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" +) + +// An Office package stops while it checksums its padding, not after it - +// docs/STABILITY-REVIEW-2026-10-26.md, R09. +// +// The padding part is stored, and its checksum has to be in the header before +// the part is written, so the padding is generated twice: once into a hash and +// once into the file. Until 2026-10-27 the first pass ran under a context of +// its own, so a stop that arrived during it waited for the whole padding to be +// hashed. Measured that day with a real Ctrl+Break on a 2 GB docx: 0.44 to 0.50 +// s after the signal, where a txt of the same size ended in 0.02 s. At the +// zip32 ceiling of 4 GiB it is about twice that. +// +// The stop is placed by count rather than by clock. stopAfter answers "not +// stopped" to its first n questions and "stopped" to every one after, and the +// guard walks n upwards, so one of them lands inside the checksum pass whatever +// the number of parts in front of it - a number this guard does not have to +// know. A pass that does not ask cannot be stopped there, and hashes four +// gigabytes before the next question. +// +// Only the time is read, and the gap it has to see is three orders of +// magnitude: the first probe of a stopped pass is a 32 KiB chunk, the pass that +// ignores the stop is 4 GiB of generated bytes. docx stands for the three, as +// they reach the padding through one function in internal/format/opc. +func TestAnOfficePackageStopsWhileItChecksumsItsPadding(t *testing.T) { + desc, err := format.Get("docx") + if err != nil { + t.Fatal(err) + } + // Just under the zip32 ceiling, so the padding is as large as a package + // can carry and the pass that ignores the stop is as long as it can be. + const size = 4<<30 - 1<<20 + plan, err := format.PlanWith(context.Background(), desc.Generator, format.Request{Bytes: size, Seed: 1, Label: true}) + if err != nil { + t.Fatalf("planning a %d B docx: %v", size, err) + } + + stoppedSomewhere := false + for n := int64(0); n <= 64; n++ { + ctx := &stopAfter{Context: context.Background(), after: n} + started := time.Now() + err := desc.Generator.Write(ctx, io.Discard, plan) + took := time.Since(started) + + // The state the measurement depends on: the stop really happened. + // Every n here is far below the questions a whole package asks, so a + // write that ends without the stop has written four gigabytes and + // says nothing about where the stop would have landed. + if !errors.Is(err, context.Canceled) { + t.Fatalf("with the stop after %d questions the write ended with %v rather than the stop", n, err) + } + stoppedSomewhere = true + if took > stopGrace { + t.Fatalf("with the stop after %d questions the write took %v to end - the padding was checksummed without looking at the stop", n, took) + } + } + if !stoppedSomewhere { + t.Fatal("no write was stopped, so nothing was measured") + } +} + +// stopGrace is how long a stopped write may take to end. Measured on +// 2026-10-27 on this machine: the 65 stopped writes of this guard took 0.06 s +// together, and the first write whose pass ignored the stop took 3.46 s. The +// line sits more than two hundred times away from each. +const stopGrace = 250 * time.Millisecond + +// stopAfter is a context that is live for its first after questions and +// stopped for every one after. +type stopAfter struct { + context.Context + after int64 + asked atomic.Int64 +} + +func (c *stopAfter) Done() <-chan struct{} { + if c.asked.Add(1) > c.after { + return closedDone + } + return nil +} + +func (c *stopAfter) Err() error { + if c.asked.Load() > c.after { + return context.Canceled + } + return nil +} + +var closedDone = func() chan struct{} { + ch := make(chan struct{}) + close(ch) + return ch +}() From 8eb99326cab2fe75bb5185b27495c3f7eb32578f Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 27 Oct 2026 18:55:33 +0100 Subject: [PATCH 3/5] engine: a name that differs only in case is refused before the first byte Preflight compared names letter for letter, so with FILES_0001.TXT in the directory --dry-run said nothing was in the way and the run wrote the other files and the manifest before the rename met the name, ending with 8. The listing is now also folded, a folded match is confirmed by asking the system about the path, and the refusal names the spelling on the disk. A system that keeps case apart still writes both. Co-Authored-By: Claude Opus 5.5 --- internal/engine/errors.go | 11 +++ internal/engine/preflight.go | 90 ++++++++++++++++------ internal/guard/casecollision_test.go | 103 ++++++++++++++++++++++++++ internal/guard/mutation/entries.py | 40 ++++++++-- internal/gui/text/locale/said/en.json | 5 ++ internal/gui/text/locale/said/pl.json | 1 + 6 files changed, 222 insertions(+), 28 deletions(-) create mode 100644 internal/guard/casecollision_test.go diff --git a/internal/engine/errors.go b/internal/engine/errors.go index 8a227662..1251d85d 100644 --- a/internal/engine/errors.go +++ b/internal/engine/errors.go @@ -259,6 +259,12 @@ type CollisionError struct { // Instructions is set when what is in the way is the instructions of an // earlier run, which describe that run's files. Instructions bool + // OnDisk is the name in the directory when it is spelt differently from + // the one this run would write - FILES_0001.TXT in the way of + // files_0001.txt on a system that ignores case. Empty when the two are the + // same spelling. Without it the refusal named a file nobody could find in + // the listing. + OnDisk string } func (e *CollisionError) Error() string { return e.Said().String() } @@ -275,6 +281,11 @@ func (e *CollisionError) Said() core.Said { "%s already exists and this run will not write over it. It is the only record of what an earlier run wrote, so replacing it would leave those files with nothing to remove them by. Generate into an empty directory, or move the old manifest aside", core.A("Path", core.Shown(e.Path))) } + if e.OnDisk != "" { + return core.Says("engine.CollisionOtherCase", + "%s already exists as %s, and this system does not tell the two names apart, so this run would write over it. Generate into an empty directory, or remove the file first", + core.A("Path", core.Shown(e.Path)), core.A("OnDisk", core.Shown(e.OnDisk))) + } return core.Says("engine.Collision", "%s already exists and this run will not write over it. Generate into an empty directory, or remove the file first", core.A("Path", core.Shown(e.Path))) diff --git a/internal/engine/preflight.go b/internal/engine/preflight.go index 2621bf8f..deaf1062 100644 --- a/internal/engine/preflight.go +++ b/internal/engine/preflight.go @@ -150,16 +150,26 @@ func preflight(ctx context.Context, files []PlannedFile, opt Options, record Rec if err := ctx.Err(); err != nil { return err } - if path := filepath.Join(opt.OutDir, f.Name); isTaken(taken, path, f.Name) { - return &CollisionError{Path: path} + path := filepath.Join(opt.OutDir, f.Name) + if hit, onDisk := taken.holds(path, f.Name); hit { + return &CollisionError{Path: path, OnDisk: onDisk} } - if path := tempPathFor(opt.OutDir, f.Name); isTaken(taken, path, tempNameFor(f.Name)) { - return &CollisionError{Path: path} + temp := tempPathFor(opt.OutDir, f.Name) + if hit, onDisk := taken.holds(temp, tempNameFor(f.Name)); hit { + return &CollisionError{Path: temp, OnDisk: onDisk} } } return nil } +// listing is what the output directory holds, kept two ways: every name as it +// is spelt, and every name folded the way a filesystem that ignores case +// compares them, pointing back at the spelling on the disk. +type listing struct { + exact map[string]struct{} + folded map[string]string +} + // namesIn is every name the output directory holds, or nil when it could not be // listed at all. // @@ -172,36 +182,72 @@ func preflight(ctx context.Context, files []PlannedFile, opt Options, record Rec // A directory that does not exist yet is a different answer and a correct one: // it holds no names, nothing can collide, and that is the ordinary case for a // fresh run. -func namesIn(dir string) map[string]struct{} { +// +// The folded half costs one core.FoldName a name, measured at 254 ns on +// 2026-09-03 (docs/STABILITY-REVIEW-2026-09-02.md section 3.2) - about 2.5 ms +// for a directory of ten thousand, against the 1.9 s that asking the system +// about every planned name cost before the listing. +func namesIn(dir string) *listing { entries, err := os.ReadDir(dir) if errors.Is(err, fs.ErrNotExist) { - return map[string]struct{}{} + return &listing{exact: map[string]struct{}{}, folded: map[string]string{}} } if err != nil { return nil } - taken := make(map[string]struct{}, len(entries)) + l := &listing{exact: make(map[string]struct{}, len(entries)), folded: make(map[string]string, len(entries))} for _, e := range entries { - taken[e.Name()] = struct{}{} + l.exact[e.Name()] = struct{}{} + if key := collisionKey(e.Name()); l.folded[key] == "" { + l.folded[key] = e.Name() + } } - return taken + return l } -// isTaken answers whether a name is already in use - from the listing when -// there is one, and from the filesystem when there is not. +// holds answers whether a name is already in use - from the listing when there +// is one, and from the filesystem when there is not - and, when it is in use +// under another spelling, which one. +// +// The listing asks the better question of the two. os.Stat follows a link, so +// a link pointing at nothing answered "no name here" and the run went on to +// replace it. A directory ENTRY is what a name being taken means, whatever it +// points at, and the rule this serves is that nothing already there is written +// over. So the listing refuses a little more than the stat did, in the +// direction that cannot lose somebody's work. // -// The two are not quite the same question and the listing asks the better one. -// os.Stat follows a link, so a link pointing at nothing answered "no name -// here" and the run went on to replace it. A directory ENTRY is what a name -// being taken means, whatever it points at, and the rule this serves is that -// nothing already there is written over. So the listing refuses a little more -// than the stat did, in the direction that cannot lose somebody's work. -func isTaken(taken map[string]struct{}, path, name string) bool { - if taken == nil { - return exists(path) +// The listing compared spellings alone until 2026-10-27, and on a system that +// ignores case that answered "free" for files_0001.txt beside FILES_0001.TXT: +// --dry-run said nothing was in the way, and the run wrote the other files and +// its manifest before the rename met the name (docs/STABILITY-REVIEW-2026-10- +// 26.md, R07). A folded match is only a candidate, though. FoldName answers +// whether two names could EVER be one file, on some system, and this has to +// answer whether they are one on THIS one - a Linux directory holds both +// spellings side by side and a run there is right to write its own. So the +// system is asked about the path itself: Lstat finds it where the two are one +// name, and not where they are two. Any answer other than "not there" counts +// as taken, the same direction as above. +// +// What this cannot see, named rather than implied: a pair a filesystem joins +// and FoldName does not. None is known - core/foldname.go records that +// cases.Fold agreed with the filesystems on every pair tools/probes/casefold +// asked about - and such a pair would still meet the protection at the +// rename, as every case did before. +func (l *listing) holds(path, name string) (bool, string) { + if l == nil { + return exists(path), "" + } + if _, ok := l.exact[name]; ok { + return true, "" + } + onDisk, ok := l.folded[collisionKey(name)] + if !ok { + return false, "" + } + if _, err := os.Lstat(path); errors.Is(err, fs.ErrNotExist) { + return false, "" } - _, ok := taken[name] - return ok + return true, onDisk } // tempPathFor is the name a file is written under before it is renamed into diff --git a/internal/guard/casecollision_test.go b/internal/guard/casecollision_test.go new file mode 100644 index 00000000..7de47b37 --- /dev/null +++ b/internal/guard/casecollision_test.go @@ -0,0 +1,103 @@ +package guard + +import ( + "bytes" + "context" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/core" +) + +// A file whose name differs from a planned one only in case is refused before +// the first byte on a system that treats the two as one name, and left alone +// beside it on a system that does not - docs/STABILITY-REVIEW-2026-10-26.md, +// R07. +// +// Until 2026-10-27 preflight compared names letter for letter. With +// FILES_0001.TXT in the directory, --dry-run said nothing was in the way and +// ended with 0, and the run itself wrote files_0002.txt and its manifest and +// only then met the protection at the rename, ending with 8 and a directory +// half written. A name that is truly taken is refused with 5 before anything +// exists, in --dry-run as well, and this is the same case. +// +// The guard ASKS the system which kind it is rather than assuming from the +// platform, because a Windows directory can be case sensitive and a Linux one +// can fold. Both answers have a branch, and CI runs this on Linux, Windows and +// macOS, so both are reached. +func TestAFileWhoseNameDiffersOnlyInCaseIsNotWrittenOver(t *testing.T) { + for _, c := range []struct { + name string + // inTheWay is the name already in the directory, from the name this + // run gives its first file. + inTheWay func(first string) string + }{ + {"a planned file", strings.ToUpper}, + // The temporary name a file is written under before the rename is + // guarded by the same check, and the run here is in this process, so + // it carries this process's id. + {"the temporary name of one", func(first string) string { + return strings.ToUpper(core.SiblingName(first, fmt.Sprintf("%s%d", core.PartialMarker, os.Getpid()))) + }}, + } { + t.Run(c.name, func(t *testing.T) { + const first = "files_0001.txt" + dir := t.TempDir() + theirs := c.inTheWay(first) + const content = "somebody else's work\n" + if err := os.WriteFile(filepath.Join(dir, theirs), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + _, err := os.Lstat(filepath.Join(dir, strings.ToLower(theirs))) + folds := err == nil + t.Logf("this directory treats %s and %s as one name: %v", theirs, strings.ToLower(theirs), folds) + + for _, dry := range []bool{true, false} { + args := []string{"generate", "--format", "txt", "--size", "10B", "--count", "2", "--out", dir} + if dry { + args = append(args, "--dry-run") + } + var out, errOut bytes.Buffer + code := cli.Run(context.Background(), args, &out, &errOut) + if folds { + if code != cli.ExitIO { + t.Errorf("dry run %v: ended with %d, expected %d - %s is %s on this system and the run went ahead\nstderr: %s", + dry, code, cli.ExitIO, theirs, strings.ToLower(theirs), errOut.String()) + } + if !strings.Contains(errOut.String(), theirs) { + t.Errorf("dry run %v: the refusal does not name %s, the file that is in the directory\nstderr: %s", dry, theirs, errOut.String()) + } + if got := namesInDir(t, dir); strings.Join(got, " ") != theirs { + t.Errorf("dry run %v: the directory holds %q after the refusal, expected %s alone - something was written before it", dry, got, theirs) + } + } else if code != cli.ExitOK { + t.Errorf("dry run %v: ended with %d on a system where %s and %s are two names\nstderr: %s", + dry, code, theirs, strings.ToLower(theirs), errOut.String()) + } + } + if got, err := os.ReadFile(filepath.Join(dir, theirs)); err != nil || string(got) != content { + t.Errorf("%s was changed: %q, %v", theirs, got, err) + } + }) + } +} + +// namesInDir is everything the directory holds, the manifest included. +func namesInDir(t *testing.T, dir string) []string { + t.Helper() + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + var out []string + for _, e := range entries { + out = append(out, e.Name()) + } + sort.Strings(out) + return out +} diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index c746f6bd..44a70300 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -1673,8 +1673,8 @@ # the collision check only ever looked at the name a file ends up with. "the name a file is written through is not checked for a collision", "internal/engine/preflight.go", - "if path := tempPathFor(opt.OutDir, f.Name); isTaken(taken, path, tempNameFor(f.Name)) {", - "if path := tempPathFor(opt.OutDir, f.Name); isTaken(taken, path, tempNameFor(f.Name)) && false {", + "if hit, onDisk := taken.holds(temp, tempNameFor(f.Name)); hit {", + "if hit, onDisk := taken.holds(temp, tempNameFor(f.Name)); hit && false {", "TestAFileUnderTheTemporaryNameIsNotWrittenOver", ), ( @@ -8283,8 +8283,8 @@ # the old behaviour really did replace it. "a name held by a link pointing nowhere stops being a name that is taken", "internal/engine/preflight.go", - "\tif taken == nil {\n\t\treturn exists(path)\n\t}\n\t_, ok := taken[name]\n\treturn ok", - "\treturn exists(path)", + "\tif l == nil {\n\t\treturn exists(path), \"\"\n\t}", + "\tif true {\n\t\treturn exists(path), \"\"\n\t}", "TestANameTakenByALinkPointingNowhereIsStillTaken", ), ( @@ -8293,8 +8293,8 @@ # The mutation has to be tried on one of those. "a directory that cannot be listed is read as an empty one, so a run writes over what is inside", "internal/engine/preflight.go", - "\tentries, err := os.ReadDir(dir)\n\tif errors.Is(err, fs.ErrNotExist) {\n\t\treturn map[string]struct{}{}\n\t}\n\tif err != nil {\n\t\treturn nil\n\t}", - "\tentries, err := os.ReadDir(dir)\n\tif errors.Is(err, fs.ErrNotExist) {\n\t\treturn map[string]struct{}{}\n\t}\n\tif err != nil {\n\t\treturn map[string]struct{}{}\n\t}", + "\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string]string{}}\n\t}\n\tif err != nil {\n\t\treturn nil\n\t}", + "\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string]string{}}\n\t}\n\tif err != nil {\n\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string]string{}}\n\t}", "TestADirectoryThatCannotBeListedIsStillAskedAboutEveryName", ), ( @@ -15880,4 +15880,32 @@ "if err := context.Background().Err(); err != nil {", "TestAContextIsMadeFromNothingOnlyWhereItIsMeantToStart", ), + ( + # R07: a name that only folds onto one in the directory is taken without + # asking the system. On a system that keeps case apart - the Linux this + # runs on in CI - that refuses a run which is right to write files_0001.txt + # beside FILES_0001.TXT. + # + # The other half of the check - the folded match itself, and the refusal + # naming the spelling on the disk - is reached only where the system + # folds, so a mutation of it is caught on Windows and macOS and would be + # a HOLE here. Both were proved by hand on Windows on 2026-10-27 (the + # guard red, the file restored by md5sum -c) rather than written down as + # entries this run cannot prove. + "a name that only folds onto one in the directory is taken without asking the system", + "internal/engine/preflight.go", + "if _, err := os.Lstat(path); errors.Is(err, fs.ErrNotExist) {", + "if _, err := os.Lstat(path); false && errors.Is(err, fs.ErrNotExist) {", + "TestAFileWhoseNameDiffersOnlyInCaseIsNotWrittenOver", + ), + ( + # The finder stops following the name a file imports context under, so + # `ctxpkg.Background()` walks past it - and a file that renames the + # import is a file the class guard no longer reads. + "the context finder looks for the package under its own name only", + "internal/guard/contextroots_test.go", + "name = imp.Name.Name", + "name = \"context\"", + "TestTheContextFinderSeesEveryShapeOfTheCall", + ), ] diff --git a/internal/gui/text/locale/said/en.json b/internal/gui/text/locale/said/en.json index 8fdb4a8b..4cef848c 100644 --- a/internal/gui/text/locale/said/en.json +++ b/internal/gui/text/locale/said/en.json @@ -891,6 +891,11 @@ "hash": "sha256-cc110e79b974", "other": "{{.Path}} already exists and this run will not write over it. It is the only record of what an earlier run wrote, so replacing it would leave those files with nothing to remove them by. Generate into an empty directory, or move the old manifest aside" }, + "engine.CollisionOtherCase": { + "description": "Said by internal/engine - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Path}}, {{.OnDisk}}, written the way the program writes them - keep each spelled exactly that way.", + "hash": "sha256-b0a9d4ed13aa", + "other": "{{.Path}} already exists as {{.OnDisk}}, and this system does not tell the two names apart, so this run would write over it. Generate into an empty directory, or remove the file first" + }, "engine.ControlCharacter": { "description": "Said by internal/engine - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Code}}, written the way the program writes them - keep each spelled exactly that way.", "hash": "sha256-519de8af1277", diff --git a/internal/gui/text/locale/said/pl.json b/internal/gui/text/locale/said/pl.json index 0c85e098..298da2d3 100644 --- a/internal/gui/text/locale/said/pl.json +++ b/internal/gui/text/locale/said/pl.json @@ -177,6 +177,7 @@ "engine.Collision": { "hash": "sha256-9b5b48b73ad2", "other": "{{.Path}} już istnieje i ten przebieg go nie nadpisze. Generuj do pustego katalogu albo najpierw usuń ten plik" }, "engine.CollisionInstructions": { "hash": "sha256-f380a98d3a7f", "other": "{{.Path}} już istnieje i ten przebieg go nie nadpisze. Mówi, do czego służą pliki wcześniejszego przebiegu. Generuj do pustego katalogu albo nazwij manifest tego przebiegu inaczej" }, "engine.CollisionManifest": { "hash": "sha256-cc110e79b974", "other": "{{.Path}} już istnieje i ten przebieg go nie nadpisze. To jedyny zapis tego, co napisał wcześniejszy przebieg, więc zastąpienie go zostawiłoby tamte pliki bez niczego, czym można je usunąć. Generuj do pustego katalogu albo odsuń stary manifest na bok" }, + "engine.CollisionOtherCase": { "hash": "sha256-b0a9d4ed13aa", "other": "{{.Path}} już istnieje jako {{.OnDisk}}, a ten system nie rozróżnia tych dwóch nazw, więc ten przebieg nadpisałby ten plik. Generuj do pustego katalogu albo najpierw usuń ten plik" }, "engine.ControlCharacter": { "hash": "sha256-519de8af1277", "other": "znak sterujący U+{{.Code}}" }, "engine.CrashPlanning": { "hash": "sha256-90f7d5da615f", "other": "planowania pliku" }, "engine.CrashWhile": { "hash": "sha256-071bf3d7823e", "other": "{{.While}}" }, From 9c679bbe67153087869059a6e13ca2c760f7c238 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 27 Oct 2026 18:58:46 +0100 Subject: [PATCH 4/5] changelog: Ctrl+C in a range draw and an Office padding, and names that differ only in case Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e13bd1e..d818d758 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,21 @@ because it turns other people's test suites red. ### Fixed +- **Ctrl+C stops a run that draws its sizes from a range.** With + `--size-range`, every file was tried at its drawn size before anything else, + and that part did not look at Ctrl+C - three thousand JPG files went on for + about fifteen seconds after it. It now stops at once, `--dry-run` and + `validate` included. +- **Ctrl+C stops a large DOCX, PPTX or XLSX at once.** The padding of a large + Office file is generated twice, once to sum it and once to write it, and the + first pass did not look at Ctrl+C - about half a second for a 2 GB file. It + now stops in the middle of it. +- **A file whose name differs from a planned one only in case is never + written over.** On Windows and macOS `FILES_0001.TXT` and `files_0001.txt` + are one file, and `--dry-run` said nothing was in the way while the run + wrote the other files before it stopped with code 8. Such a run is now + refused with code 5 before anything is written, naming the file that is + there. Where names keep case apart, both are written as before. - **A size with a decimal point is read to the byte.** A fraction finer than the arithmetic behind it was rounded without a word, so `--size 255.99999999999999B` made a file of 256 B and `9007199254740993.0B` From cd3e7d9b9841fac40bac3b9f00d4752f3989be16 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 27 Oct 2026 19:20:45 +0100 Subject: [PATCH 5/5] review: the refusal names the file really in the way, and the Office stop is counted FoldName joins pairs a filesystem keeps apart: on NTFS STRASSE.txt and the sharp s spelling sit side by side and fold alike, and the refusal named the first one. Every spelling of a fold is kept and the one that is the same file as the path is named. The Office padding guard counts the context's questions through Write instead of timing a 4 GiB pass. The duplicate directory listing helper is gone. Co-Authored-By: Claude Opus 5.5 --- internal/engine/preflight.go | 42 +++++++---- internal/guard/casecollision_test.go | 50 ++++++++++--- internal/guard/mutation/entries.py | 18 +++-- internal/guard/mutationcoverage_test.go | 6 ++ internal/guard/opcstop_test.go | 94 +++++++++++++------------ 5 files changed, 138 insertions(+), 72 deletions(-) diff --git a/internal/engine/preflight.go b/internal/engine/preflight.go index deaf1062..78b2a4ab 100644 --- a/internal/engine/preflight.go +++ b/internal/engine/preflight.go @@ -164,10 +164,17 @@ func preflight(ctx context.Context, files []PlannedFile, opt Options, record Rec // listing is what the output directory holds, kept two ways: every name as it // is spelt, and every name folded the way a filesystem that ignores case -// compares them, pointing back at the spelling on the disk. +// compares them, pointing back at each spelling on the disk with that fold. +// +// Each, rather than the first. FoldName joins pairs a filesystem keeps apart: +// measured on 2026-10-27 on NTFS, STRASSE.txt and Stra\u00dfe.txt are two files +// side by side and both fold to strasse.txt, and a lookup of stra\u00dfe.txt finds +// the second. Keeping one spelling named STRASSE.txt - the file a refusal +// would then tell somebody to remove, and the wrong one. Found by CodeRabbit +// on #207 (docs/REVIEW-207-2026-10-27.md). type listing struct { exact map[string]struct{} - folded map[string]string + folded map[string][]string } // namesIn is every name the output directory holds, or nil when it could not be @@ -190,17 +197,16 @@ type listing struct { func namesIn(dir string) *listing { entries, err := os.ReadDir(dir) if errors.Is(err, fs.ErrNotExist) { - return &listing{exact: map[string]struct{}{}, folded: map[string]string{}} + return &listing{exact: map[string]struct{}{}, folded: map[string][]string{}} } if err != nil { return nil } - l := &listing{exact: make(map[string]struct{}, len(entries)), folded: make(map[string]string, len(entries))} + l := &listing{exact: make(map[string]struct{}, len(entries)), folded: make(map[string][]string, len(entries))} for _, e := range entries { l.exact[e.Name()] = struct{}{} - if key := collisionKey(e.Name()); l.folded[key] == "" { - l.folded[key] = e.Name() - } + key := collisionKey(e.Name()) + l.folded[key] = append(l.folded[key], e.Name()) } return l } @@ -226,7 +232,9 @@ func namesIn(dir string) *listing { // spellings side by side and a run there is right to write its own. So the // system is asked about the path itself: Lstat finds it where the two are one // name, and not where they are two. Any answer other than "not there" counts -// as taken, the same direction as above. +// as taken, the same direction as above. The spelling named is the one that is +// the same file as what Lstat found, and none is named when no candidate is - +// the refusal then shows the path alone rather than a guess. // // What this cannot see, named rather than implied: a pair a filesystem joins // and FoldName does not. None is known - core/foldname.go records that @@ -240,14 +248,24 @@ func (l *listing) holds(path, name string) (bool, string) { if _, ok := l.exact[name]; ok { return true, "" } - onDisk, ok := l.folded[collisionKey(name)] - if !ok { + candidates := l.folded[collisionKey(name)] + if len(candidates) == 0 { return false, "" } - if _, err := os.Lstat(path); errors.Is(err, fs.ErrNotExist) { + here, err := os.Lstat(path) + if errors.Is(err, fs.ErrNotExist) { return false, "" } - return true, onDisk + if err != nil { + return true, "" + } + dir := filepath.Dir(path) + for _, onDisk := range candidates { + if there, err := os.Lstat(filepath.Join(dir, onDisk)); err == nil && os.SameFile(here, there) { + return true, onDisk + } + } + return true, "" } // tempPathFor is the name a file is written under before it is renamed into diff --git a/internal/guard/casecollision_test.go b/internal/guard/casecollision_test.go index 7de47b37..b66b3233 100644 --- a/internal/guard/casecollision_test.go +++ b/internal/guard/casecollision_test.go @@ -6,7 +6,6 @@ import ( "fmt" "os" "path/filepath" - "sort" "strings" "testing" @@ -72,7 +71,7 @@ func TestAFileWhoseNameDiffersOnlyInCaseIsNotWrittenOver(t *testing.T) { if !strings.Contains(errOut.String(), theirs) { t.Errorf("dry run %v: the refusal does not name %s, the file that is in the directory\nstderr: %s", dry, theirs, errOut.String()) } - if got := namesInDir(t, dir); strings.Join(got, " ") != theirs { + if got := namesIn(t, dir); strings.Join(got, " ") != theirs { t.Errorf("dry run %v: the directory holds %q after the refusal, expected %s alone - something was written before it", dry, got, theirs) } } else if code != cli.ExitOK { @@ -87,17 +86,46 @@ func TestAFileWhoseNameDiffersOnlyInCaseIsNotWrittenOver(t *testing.T) { } } -// namesInDir is everything the directory holds, the manifest included. -func namesInDir(t *testing.T, dir string) []string { - t.Helper() - entries, err := os.ReadDir(dir) +// The refusal names the file that is really in the way, not another one that +// folds to the same letters. +// +// core.FoldName joins pairs a filesystem keeps apart. On NTFS STRASSE.txt and +// Stra\u00dfe.txt are two files side by side, both fold to strasse.txt, and a run +// writing stra\u00dfe.txt would write over the second - measured on 2026-10-27. +// Until CodeRabbit asked on #207, preflight kept the first spelling of a fold +// and named STRASSE.txt, which is the file it would have told somebody to +// remove. A system that joins the two (APFS does) cannot hold this directory at +// all, and the guard says so rather than passing. +func TestARefusalNamesTheFileThatIsReallyInTheWay(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "STRASSE.txt"), []byte("upper case ss\n"), 0o644); err != nil { + t.Fatal(err) + } + sharp := "Stra\u00dfe.txt" + f, err := os.OpenFile(filepath.Join(dir, sharp), os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) if err != nil { + t.Skipf("this filesystem will not hold %s beside STRASSE.txt (%v), so the pair this guard is about cannot exist here", sharp, err) + } + if err := f.Close(); err != nil { t.Fatal(err) } - var out []string - for _, e := range entries { - out = append(out, e.Name()) + planned := strings.ToLower(sharp) + _, err = os.Lstat(filepath.Join(dir, planned)) + folds := err == nil + t.Logf("this directory treats %s and %s as one name: %v", sharp, planned, folds) + + var out, errOut bytes.Buffer + code := cli.Run(context.Background(), []string{"generate", "--format", "txt", "--size", "10B", "--name", planned, "--out", dir, "--dry-run"}, &out, &errOut) + if !folds { + if code != cli.ExitOK { + t.Errorf("ended with %d where %s and %s are two names\nstderr: %s", code, sharp, planned, errOut.String()) + } + return + } + if code != cli.ExitIO { + t.Fatalf("ended with %d, expected %d - %s is in the way\nstderr: %s", code, cli.ExitIO, sharp, errOut.String()) + } + if !strings.Contains(errOut.String(), "as "+sharp) || strings.Contains(errOut.String(), "STRASSE") { + t.Errorf("the refusal has to name %s, the file %s would write over, and not STRASSE.txt\nstderr: %s", sharp, planned, errOut.String()) } - sort.Strings(out) - return out } diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index 44a70300..05fdddc7 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -8293,8 +8293,8 @@ # The mutation has to be tried on one of those. "a directory that cannot be listed is read as an empty one, so a run writes over what is inside", "internal/engine/preflight.go", - "\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string]string{}}\n\t}\n\tif err != nil {\n\t\treturn nil\n\t}", - "\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string]string{}}\n\t}\n\tif err != nil {\n\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string]string{}}\n\t}", + "\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string][]string{}}\n\t}\n\tif err != nil {\n\t\treturn nil\n\t}", + "\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string][]string{}}\n\t}\n\tif err != nil {\n\t\treturn &listing{exact: map[string]struct{}{}, folded: map[string][]string{}}\n\t}", "TestADirectoryThatCannotBeListedIsStillAskedAboutEveryName", ), ( @@ -15894,8 +15894,8 @@ # entries this run cannot prove. "a name that only folds onto one in the directory is taken without asking the system", "internal/engine/preflight.go", - "if _, err := os.Lstat(path); errors.Is(err, fs.ErrNotExist) {", - "if _, err := os.Lstat(path); false && errors.Is(err, fs.ErrNotExist) {", + "here, err := os.Lstat(path)\n\tif errors.Is(err, fs.ErrNotExist) {", + "here, err := os.Lstat(path)\n\tif false && errors.Is(err, fs.ErrNotExist) {", "TestAFileWhoseNameDiffersOnlyInCaseIsNotWrittenOver", ), ( @@ -15908,4 +15908,14 @@ "name = \"context\"", "TestTheContextFinderSeesEveryShapeOfTheCall", ), + ( + # The checksum pass sees the stop and swallows it: the header gets a + # checksum of nothing and the write goes on to its next question. The + # stop placed half way through the questions catches it by count. + "the checksum pass of the padding swallows the stop it was handed", + "internal/format/opc/opc.go", + "hash never refuses a write, so this is the stop.\n\t\treturn 0, err", + "hash never refuses a write, so this is the stop.\n\t\treturn 0, nil", + "TestAnOfficePackageStopsWhileItChecksumsItsPadding", + ), ] diff --git a/internal/guard/mutationcoverage_test.go b/internal/guard/mutationcoverage_test.go index 89273602..b3f5ba04 100644 --- a/internal/guard/mutationcoverage_test.go +++ b/internal/guard/mutationcoverage_test.go @@ -136,6 +136,12 @@ var notProvenByMutation = map[string]bool{ // "proven another way" are different states and lumping them together would // send a later session to re-prove what is already proven. var provenByProbe = map[string]string{ + // Reached only where the filesystem folds case and still keeps STRASSE.txt + // apart from a sharp s spelling - NTFS does, the Linux the mutation job runs + // on keeps every case apart and APFS joins the pair, so no CI mutation of + // the branch can redden it. + "TestARefusalNamesTheFileThatIsReallyInTheWay": "broken by hand on Windows on 2026-10-27: os.SameFile(here, there) in listing.holds (preflight.go) turned into os.SameFile(there, there), so the first spelling of a fold is named. " + + "The guard went red saying the refusal named STRASSE.txt, and the file was put back and checked with md5sum -c.", // Three guards each held by two layers, so no single substitution can redden // them and each mutation of one layer scored a HOLE in the CI run of // 2026-10-15. Broken that day in a worker tree, one layer at a time and then diff --git a/internal/guard/opcstop_test.go b/internal/guard/opcstop_test.go index 61808429..a559551c 100644 --- a/internal/guard/opcstop_test.go +++ b/internal/guard/opcstop_test.go @@ -6,7 +6,6 @@ import ( "io" "sync/atomic" "testing" - "time" "github.com/donislawdev/TestingFilesGenerator/internal/format" ) @@ -19,65 +18,70 @@ import ( // once into the file. Until 2026-10-27 the first pass ran under a context of // its own, so a stop that arrived during it waited for the whole padding to be // hashed. Measured that day with a real Ctrl+Break on a 2 GB docx: 0.44 to 0.50 -// s after the signal, where a txt of the same size ended in 0.02 s. At the -// zip32 ceiling of 4 GiB it is about twice that. +// s after the signal, where a txt of the same size ended in 0.02 s. // -// The stop is placed by count rather than by clock. stopAfter answers "not -// stopped" to its first n questions and "stopped" to every one after, and the -// guard walks n upwards, so one of them lands inside the checksum pass whatever -// the number of parts in front of it - a number this guard does not have to -// know. A pass that does not ask cannot be stopped there, and hashes four -// gigabytes before the next question. -// -// Only the time is read, and the gap it has to see is three orders of -// magnitude: the first probe of a stopped pass is a 32 KiB chunk, the pass that -// ignores the stop is 4 GiB of generated bytes. docx stands for the three, as -// they reach the padding through one function in internal/format/opc. +// Counted, not timed. The first version of this guard timed a stopped write of +// four gigabytes against 250 ms, and CodeRabbit asked on #207 for a signal a +// slow runner cannot fake (docs/REVIEW-207-2026-10-27.md). The padding asks +// the context once for every chunk it generates, so two whole writes whose +// padding differs by one mebibyte differ by the chunks of that mebibyte - once +// if only the write asks, twice if the checksum pass asks as well. And a stop +// placed inside the checksum pass has to end the write at the very next +// question, or the pass carried on past it. Both go through the generator's +// Write, so the context is followed down the path a run takes. docx stands for +// the three, as they reach the padding through one function in +// internal/format/opc. func TestAnOfficePackageStopsWhileItChecksumsItsPadding(t *testing.T) { desc, err := format.Get("docx") if err != nil { t.Fatal(err) } - // Just under the zip32 ceiling, so the padding is as large as a package - // can carry and the pass that ignores the stop is as long as it can be. - const size = 4<<30 - 1<<20 - plan, err := format.PlanWith(context.Background(), desc.Generator, format.Request{Bytes: size, Seed: 1, Label: true}) - if err != nil { - t.Fatalf("planning a %d B docx: %v", size, err) + // Without the label, so the parts of the two packages are the same bytes + // and their padding differs by exactly the difference in size. + const smaller, larger = 4 << 20, 5 << 20 + questions := func(size int64, ctx *stopAfter) (int64, error) { + plan, err := format.PlanWith(context.Background(), desc.Generator, format.Request{Bytes: size, Seed: 1}) + if err != nil { + t.Fatalf("planning a %d B docx: %v", size, err) + } + err = desc.Generator.Write(ctx, io.Discard, plan) + return ctx.asked.Load(), err } - stoppedSomewhere := false - for n := int64(0); n <= 64; n++ { - ctx := &stopAfter{Context: context.Background(), after: n} - started := time.Now() - err := desc.Generator.Write(ctx, io.Discard, plan) - took := time.Since(started) - - // The state the measurement depends on: the stop really happened. - // Every n here is far below the questions a whole package asks, so a - // write that ends without the stop has written four gigabytes and - // says nothing about where the stop would have landed. - if !errors.Is(err, context.Canceled) { - t.Fatalf("with the stop after %d questions the write ended with %v rather than the stop", n, err) - } - stoppedSomewhere = true - if took > stopGrace { - t.Fatalf("with the stop after %d questions the write took %v to end - the padding was checksummed without looking at the stop", n, took) + whole := func(size int64) int64 { + n, err := questions(size, &stopAfter{Context: context.Background(), after: 1 << 62}) + if err != nil { + t.Fatalf("writing a %d B docx: %v", size, err) } + return n + } + small, large := whole(smaller), whole(larger) + const chunks = (larger - smaller) / paddingChunk + if got := large - small; got != 2*chunks { + t.Fatalf("a mebibyte more padding asked the context %d more times, expected %d - twice for each of its %d chunks, once in the checksum pass and once in the write. %d means one of the two passes does not ask", + got, 2*chunks, chunks, chunks) + } + + // Half way through a whole write's questions is inside the checksum pass: + // the parts ask once each and come first, and the padding of 4 MiB asks + // far more often than there are parts. + at := small / 2 + asked, err := questions(smaller, &stopAfter{Context: context.Background(), after: at}) + if !errors.Is(err, context.Canceled) { + t.Fatalf("with the stop after %d questions the write ended with %v rather than the stop", at, err) } - if !stoppedSomewhere { - t.Fatal("no write was stopped, so nothing was measured") + if asked != at+1 { + t.Errorf("with the stop after %d questions the write asked %d times - the checksum pass went on after it was told to stop", at, asked) } } -// stopGrace is how long a stopped write may take to end. Measured on -// 2026-10-27 on this machine: the 65 stopped writes of this guard took 0.06 s -// together, and the first write whose pass ignored the stop took 3.46 s. The -// line sits more than two hundred times away from each. -const stopGrace = 250 * time.Millisecond +// paddingChunk is how much padding is generated between two questions of the +// context - writeChunk in internal/format/opc/opc.go. If that changes, the +// arithmetic above says so in its own words rather than passing. +const paddingChunk = 32 << 10 // stopAfter is a context that is live for its first after questions and -// stopped for every one after. +// stopped for every one after, and counts them. type stopAfter struct { context.Context after int64