From faf73b45bf007b781f2ca4a790bfaa0754397af5 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 26 Oct 2026 18:03:28 +0100 Subject: [PATCH 1/7] core: move the regular-file opener down from internal/tool verify and cleanup read every file a manifest lists with a plain os.Open, which waits on a pipe and reads a device forever. The answer to that already existed as tool.OpenRegular, but internal/tool sits a layer above internal/audit, so audit could not reach it. Moved rather than copied, so there stays one way to open a file that is read. No behaviour changes in this commit. Co-Authored-By: Claude Opus 5.5 --- .../{tool/open.go => core/openregular.go} | 24 +++++++++++-------- .../open_unix.go => core/openregular_unix.go} | 2 +- .../openregular_windows.go} | 2 +- internal/guard/mutation/entries.py | 2 +- internal/guard/mutationcoverage_test.go | 2 +- internal/guard/tools_unix_test.go | 5 ++-- internal/guard/writeescape_test.go | 2 +- internal/tool/archivescan/archivescan.go | 4 ++-- internal/tool/archivescan/refusals.go | 2 +- internal/tool/checksum/read.go | 8 +++---- internal/tool/checksum/refusals.go | 2 +- 11 files changed, 30 insertions(+), 25 deletions(-) rename internal/{tool/open.go => core/openregular.go} (62%) rename internal/{tool/open_unix.go => core/openregular_unix.go} (98%) rename internal/{tool/open_windows.go => core/openregular_windows.go} (97%) diff --git a/internal/tool/open.go b/internal/core/openregular.go similarity index 62% rename from internal/tool/open.go rename to internal/core/openregular.go index 1f5484f7..87de9d73 100644 --- a/internal/tool/open.go +++ b/internal/core/openregular.go @@ -1,17 +1,21 @@ -package tool +package core import "os" -// Opening a file a tool reads. One way in for every tool, so a file named on -// its own, the thousandth file of a folder and an archive somebody was sent -// are opened with the same care - the review of #157 found a pipe swapped in -// between two looks at a name, and the answer to that is here rather than in -// each tool (docs/NARZEDZIA-SUMY-2026-09-29.md §13.4). Moved out of the -// checksum tool on 2026-10-19, when a second tool came to read files. +// Opening a file that is read rather than written. One way in for every +// reader, so a file named on its own, the thousandth file of a folder, an +// archive somebody was sent and a file a manifest lists are opened with the +// same care - the review of #157 found a pipe swapped in between two looks at a +// name, and the answer to that is here rather than in each reader +// (docs/NARZEDZIA-SUMY-2026-09-29.md §13.4). Moved out of the checksum tool on +// 2026-10-19, when a second tool came to read files, and out of internal/tool +// into core on 2026-10-26, when verify and cleanup came to need it: they sit +// a layer below the tools and could not reach it there +// (docs/STABILITY-REVIEW-2026-10-26.md, R04). -// NotRegularError is a path that names something other than a file. Each tool -// says it in its own words, wrapping this, so a check that every tool refuses -// a pipe asks for this one type. +// NotRegularError is a path that names something other than a file. Each +// reader says it in its own words, wrapping this, so a check that every reader +// refuses a pipe asks for this one type. type NotRegularError struct { Path string Directory bool diff --git a/internal/tool/open_unix.go b/internal/core/openregular_unix.go similarity index 98% rename from internal/tool/open_unix.go rename to internal/core/openregular_unix.go index a3b5e9bf..d4cb437c 100644 --- a/internal/tool/open_unix.go +++ b/internal/core/openregular_unix.go @@ -1,6 +1,6 @@ //go:build !windows -package tool +package core import ( "os" diff --git a/internal/tool/open_windows.go b/internal/core/openregular_windows.go similarity index 97% rename from internal/tool/open_windows.go rename to internal/core/openregular_windows.go index b8143393..f3b83a67 100644 --- a/internal/tool/open_windows.go +++ b/internal/core/openregular_windows.go @@ -1,6 +1,6 @@ //go:build windows -package tool +package core import "os" diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index d4dadca3..193e6783 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -13223,7 +13223,7 @@ # through - read from the source, so a file built only off Windows # is held on the Windows machine too. "the checksum tool opens its file for writing as well, and the one claim does not notice", - "internal/tool/open_unix.go", + "internal/core/openregular_unix.go", "os.O_RDONLY|syscall.O_NONBLOCK, 0)", "os.O_RDWR|syscall.O_NONBLOCK, 0)", "TestEveryFileThisToolWritesIsCreatedThroughOneClaim", diff --git a/internal/guard/mutationcoverage_test.go b/internal/guard/mutationcoverage_test.go index f7ecae88..bf66796a 100644 --- a/internal/guard/mutationcoverage_test.go +++ b/internal/guard/mutationcoverage_test.go @@ -37,7 +37,7 @@ var notProvenByMutation = map[string]bool{ // - the substitution would be applied and the test would not exist to // catch it. It runs on every Linux and macOS runner of the CI. The two // breaks it is for - asking IsDir where IsRegular stands in - // tool.OpenRegular, and an open without O_NONBLOCK in tool/open_unix.go. + // core.OpenRegular, and an open without O_NONBLOCK in core/openregular_unix.go. // The first was read, not run (2026-09-29). The second was run by hand in // a Linux container on 2026-09-30, through the tool rather than through // this test: without the flag a pipe was still waited on after five diff --git a/internal/guard/tools_unix_test.go b/internal/guard/tools_unix_test.go index f32fe65a..6c03ffa8 100644 --- a/internal/guard/tools_unix_test.go +++ b/internal/guard/tools_unix_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/tool" ) @@ -21,7 +22,7 @@ import ( // of nothing instead of the refusal. // // Since 2026-09-30 this is asked of the open file rather than of the name -// (tool.OpenRegular), which is what closed the moment between the two looks +// (core.OpenRegular), which is what closed the moment between the two looks // that a review of #157 found - so this one guard holds both halves. // // Every tool that reads a file, walked from the registry since 2026-10-19, @@ -56,7 +57,7 @@ func refusesThePipe(t *testing.T, d tool.Descriptor, pipe string) { }() select { case err := <-answered: - var notAFile *tool.NotRegularError + var notAFile *core.NotRegularError if !errors.As(err, ¬AFile) { t.Errorf("a pipe was named to %s and the answer was %v, not the refusal saying it is not a file", d.ID, err) } diff --git a/internal/guard/writeescape_test.go b/internal/guard/writeescape_test.go index 018710a7..48833bcc 100644 --- a/internal/guard/writeescape_test.go +++ b/internal/guard/writeescape_test.go @@ -133,7 +133,7 @@ func TestEveryFileThisToolWritesIsCreatedThroughOneClaim(t *testing.T) { // readingFlags are the flags of an open that only reads - os.Open with // something os.Open cannot ask for. O_NONBLOCK is why this exists: the -// tools open without waiting on a pipe (tool.OpenRegular), which +// tools open without waiting on a pipe (core.OpenRegular), which // takes os.OpenFile, and it creates nothing and writes nothing. var readingFlags = map[string]bool{ "O_RDONLY": true, "O_NONBLOCK": true, "O_CLOEXEC": true, "O_NOFOLLOW": true, "O_NOCTTY": true, diff --git a/internal/tool/archivescan/archivescan.go b/internal/tool/archivescan/archivescan.go index cc943bff..61c3212d 100644 --- a/internal/tool/archivescan/archivescan.go +++ b/internal/tool/archivescan/archivescan.go @@ -110,8 +110,8 @@ func run(ctx context.Context, in tool.Request, progress tool.Progress) (tool.Res limit, _ = core.ParseSize(raw) } path := in.Inputs[InputArchive] - f, before, err := tool.OpenRegular(path) - var not *tool.NotRegularError + f, before, err := core.OpenRegular(path) + var not *core.NotRegularError if errors.As(err, ¬) { return tool.Result{}, &NotAFileError{Path: path, Directory: not.Directory} } diff --git a/internal/tool/archivescan/refusals.go b/internal/tool/archivescan/refusals.go index bb5c9024..13e1d03a 100644 --- a/internal/tool/archivescan/refusals.go +++ b/internal/tool/archivescan/refusals.go @@ -36,7 +36,7 @@ func (e *NotAFileError) Error() string { return e.Said().String() } // Unwrap is the refusal every tool shares. func (e *NotAFileError) Unwrap() error { - return &tool.NotRegularError{Path: e.Path, Directory: e.Directory} + return &core.NotRegularError{Path: e.Path, Directory: e.Directory} } // Said is the whole refusal, for a window that says it in its own language. diff --git a/internal/tool/checksum/read.go b/internal/tool/checksum/read.go index 7a83df5c..27a46f40 100644 --- a/internal/tool/checksum/read.go +++ b/internal/tool/checksum/read.go @@ -8,7 +8,7 @@ import ( "io" "os" - "github.com/donislawdev/TestingFilesGenerator/internal/tool" + "github.com/donislawdev/TestingFilesGenerator/internal/core" ) // Reading files, for all three tools of this package. One way in, so a file @@ -67,11 +67,11 @@ func digest(ctx context.Context, path string, chosen map[string]bool, scratch [] return out, before, nil } -// openRegular is tool.OpenRegular with its refusal in the words of these +// openRegular is core.OpenRegular with its refusal in the words of these // tools - a checksum of a directory, not just a directory. func openRegular(path string) (*os.File, os.FileInfo, error) { - f, info, err := tool.OpenRegular(path) - var not *tool.NotRegularError + f, info, err := core.OpenRegular(path) + var not *core.NotRegularError if errors.As(err, ¬) { return nil, nil, &NotAFileError{Path: path, Directory: not.Directory} } diff --git a/internal/tool/checksum/refusals.go b/internal/tool/checksum/refusals.go index a2de117b..abdfa360 100644 --- a/internal/tool/checksum/refusals.go +++ b/internal/tool/checksum/refusals.go @@ -126,7 +126,7 @@ func (e *NotAFileError) Error() string { return e.Said().String() } // Unwrap is the refusal every tool shares, so a check that asks any tool // whether it turned a pipe away finds this one too. func (e *NotAFileError) Unwrap() error { - return &tool.NotRegularError{Path: e.Path, Directory: e.Directory} + return &core.NotRegularError{Path: e.Path, Directory: e.Directory} } // Said is the whole refusal, for a window that says it in its own language. From a4e611528f292d765e47400957fc6d520e6112e7 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 26 Oct 2026 18:19:55 +0100 Subject: [PATCH 2/7] audit: ask the disk again before cleanup removes a file cleanup judged every file before it removed any, so the first file judged ours then waited for the hashing of all the others, and a change made to it in that wait was removed without --force, under exit 0. An outside review reproduced it with 257 files and a change 400 ms in. Remove now looks again with os.Lstat just before os.Remove and keeps a file whose identity, size or time moved. The identity on the judged side comes from the handle it was read through: on Windows os.Stat reads it lazily by name, so two looks by name agree about a replacement. Measured on NTFS: a rewrite in place moves the time at once, even with the writer's handle still open. verify and cleanup read a listed file one way now. It opens without waiting and only a file (core.OpenRegular), asks about Ctrl+C every 256 KiB rather than only between files, and reads no more than the recorded size and one byte. A pipe put where a run left an empty file had the right size, 0, and os.Open waited on it for good. verify called every failed look "missing", and a name the system would not look up is now "unreadable" with its reason. The read pool no longer counts an item cut short by the cancellation as answered. Not closed: the moment between the second look and the removal, and a write of the same length within one tick of a coarse clock. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 14 ++ internal/audit/audit.go | 86 +++------ internal/audit/cleanup.go | 75 ++++++-- internal/audit/parallel.go | 11 +- internal/audit/read.go | 150 +++++++++++++++ internal/guard/auditpipe_unix_test.go | 71 ++++++++ internal/guard/cleanuprecheck_test.go | 252 ++++++++++++++++++++++++++ internal/guard/mutation/entries.py | 55 +++++- 8 files changed, 639 insertions(+), 75 deletions(-) create mode 100644 internal/audit/read.go create mode 100644 internal/guard/auditpipe_unix_test.go create mode 100644 internal/guard/cleanuprecheck_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e52c4e17..4b54ac1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,20 @@ because it turns other people's test suites red. `--user "$(id -u):$(id -g)"` to write there. README also shows a step that installs `tfg` on Windows and macOS runners in GitHub Actions. +### Fixed + +- **`tfg cleanup --yes` keeps a file that changed after it was checked.** + Cleanup checks every file before it removes any, and a file changed in that + time was removed without `--force`. It now looks again just before it + removes each file and keeps one that was written to or replaced, saying so. + A write that keeps the length and lands within the same tick of a coarse + clock can still go unseen - FAT32 counts in two seconds. +- **`tfg verify` and `tfg cleanup` stop part way through a large file on + Ctrl+C**, never wait on a pipe put where a listed file was, and read no more + than the size the manifest records. `verify` now reports a file it could not + even look up, for want of permission or a name too long for the system, as + `unreadable` with the reason rather than as `missing`. + ## [0.5.0] - 2026-10-20 ### Breaking diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 123a4d20..03c002d6 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -11,11 +11,7 @@ package audit import ( "context" - "crypto/sha256" - "encoding/hex" "fmt" - "io" - "os" "path" "path/filepath" "sort" @@ -38,9 +34,10 @@ const ( WrongSize Kind = "wrong-size" // WrongHash is a file of the right length and the wrong content. WrongHash Kind = "wrong-hash" - // Unreadable is a file that is there and could not be read. It is not - // agreement and it is not a mismatch, and calling it either would be a - // guess. + // Unreadable is a file that is there and could not be read, or a name the + // system would not even look up - no permission, a name too long for it. + // It is not agreement and it is not a mismatch, and calling it either + // would be a guess. Got carries the system's own reason. Unreadable Kind = "unreadable" // Leftover is a file this tool started and never finished, from a run that // was killed before it could tidy up. @@ -294,7 +291,7 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) } found, stopped := InOrder(ctx, len(claimed), func(i int, scratch []byte) Difference { - return compare(claimed[i], full[i], scratch) + return compare(ctx, claimed[i], full[i], scratch) }) for _, d := range found { if d.Kind != "" { @@ -456,31 +453,40 @@ func claimedPaths(b core.Boundary, files []manifest.File) ([]string, error) { // agreement on its own. No sentinel anybody has to remember, and no pointer // per file - a run of ten thousand would allocate ten thousand of them to say // "nothing to report" ten thousand times. -func compare(f manifest.File, full string, scratch []byte) Difference { - info, statErr := os.Stat(full) - if statErr != nil { +func compare(ctx context.Context, f manifest.File, full string, scratch []byte) Difference { + got, err := readListed(ctx, full, f.Bytes, scratch) + // Only an absent file is missing. Every failure to look used to be called + // that, so a file somebody had locked or taken the permissions off was + // reported as gone - a sentence about the wrong thing, and the one a + // person acts on (docs/STABILITY-REVIEW-2026-10-26.md, R04). cleanup has + // always told the two apart. + if absent(err) { return Difference{Kind: Missing, Path: f.Path} } + if err != nil { + return Difference{Kind: Unreadable, Path: f.Path, Got: err.Error()} + } // Size first. It is free, it catches the common failure, and it names the - // cause more precisely than a hash mismatch would. - if info.Size() != f.Bytes { + // cause more precisely than a hash mismatch would. A file that grew while + // it was read is the same answer, reached later: at least as long as what + // was read of it, and longer if it went on growing. + size := got.last.Size() + if got.grew(f.Bytes) { + size = max(size, got.read) + } + if size != f.Bytes { return Difference{ Kind: WrongSize, Path: f.Path, Want: fmt.Sprintf("%d B", f.Bytes), - Got: fmt.Sprintf("%d B", info.Size()), + Got: fmt.Sprintf("%d B", size), } } - - sum, hashErr := hashFile(full, scratch) - if hashErr != nil { - return Difference{Kind: Unreadable, Path: f.Path, Got: hashErr.Error()} - } - if sum != f.Hashes.SHA256 { + if got.sum != f.Hashes.SHA256 { return Difference{ Kind: WrongHash, Path: f.Path, Want: f.Hashes.SHA256, - Got: sum, + Got: got.sum, } } return Difference{} @@ -494,41 +500,3 @@ func compare(f manifest.File, full string, scratch []byte) Difference { // told apart, so this is the size that buys it without asking every worker for // a megabyte. const hashScratch = 256 << 10 - -// onlyRead hides everything but Read from io.CopyBuffer. -// -// This is not a wrapper for its own sake and without it the buffer below does -// nothing at all. os.File implements io.WriterTo, so io.CopyBuffer hands the -// whole job to the file and THROWS THE BUFFER AWAY - measured on 2026-09-05, -// passing 128 KiB, 256 KiB or 1 MiB with a plain file on the other end all take -// the same 167-172 ms that io.Copy takes, because all four are the same 32 KiB -// path inside. Hidden behind this, a 256 KiB buffer takes 150-161 ms. -// -// The report that suggested the buffer said it would "cut the syscall count 32 -// fold" and would have changed nothing as written. That is the whole reason -// this type exists rather than a comment saying the buffer is bigger. -type onlyRead struct{ io.Reader } - -// hashFile streams the file rather than reading it in. A run of this tool -// produces files measured in gigabytes, and verify has to survive its own -// output. -// -// scratch belongs to the caller and is reused across every file that caller -// answers for. It may be nil, and then this asks for one of its own - which is -// what a sequential caller wants and what every test that reaches here gets. -func hashFile(path string, scratch []byte) (string, error) { - f, err := os.Open(path) - if err != nil { - return "", err - } - defer func() { _ = f.Close() }() - - if scratch == nil { - scratch = make([]byte, hashScratch) - } - h := sha256.New() - if _, err := io.CopyBuffer(h, onlyRead{f}, scratch); err != nil { - return "", err - } - return hex.EncodeToString(h.Sum(nil)), nil -} diff --git a/internal/audit/cleanup.go b/internal/audit/cleanup.go index c5c5aac3..432d09d4 100644 --- a/internal/audit/cleanup.go +++ b/internal/audit/cleanup.go @@ -44,6 +44,12 @@ type Candidate struct { Path string Disposition Disposition Detail string + + // seen is the file as it was when its content was found to be ours, from + // the handle it was read through. Remove asks the disk again just before + // it deletes and leaves the file alone when the answer differs - see + // changedSince. + seen os.FileInfo } // Removable says whether this candidate is one cleanup will delete. @@ -80,36 +86,71 @@ func Inspect(ctx context.Context, dir string, m *manifest.Manifest) ([]Candidate // In order, because this list is what cleanup removes from and what it // printed to a person beforehand. return InOrder(ctx, len(claimed), func(i int, scratch []byte) Candidate { - return look(claimed[i], full[i], scratch) + return look(ctx, claimed[i], full[i], scratch) }) } // look is what one claimed file comes to for cleanup: whether it may be // removed, and if not, why not in the words a person needs. -func look(f manifest.File, full string, scratch []byte) Candidate { - info, err := os.Stat(full) - if errors.Is(err, fs.ErrNotExist) { +func look(ctx context.Context, f manifest.File, full string, scratch []byte) Candidate { + got, err := readListed(ctx, full, f.Bytes, scratch) + if absent(err) { return Candidate{Path: f.Path, Disposition: Absent} } if err != nil { return Candidate{Path: f.Path, Disposition: Unreachable, Detail: err.Error()} } - // Size before hash, so a file of obviously the wrong length does not cost - // a read of however many gigabytes it is. - if info.Size() != f.Bytes { + // Size before hash - readListed reads nothing of a file of the wrong + // length, so one of obviously the wrong length does not cost a read of + // however many gigabytes it is. + if size := got.seen.Size(); size != f.Bytes { return Candidate{Path: f.Path, Disposition: Changed, - Detail: fmt.Sprintf("it is %d B and the manifest recorded %d B", info.Size(), f.Bytes)} + Detail: fmt.Sprintf("it is %d B and the manifest recorded %d B", size, f.Bytes)} } - sum, err := hashFile(full, scratch) - if err != nil { - return Candidate{Path: f.Path, Disposition: Unreachable, Detail: err.Error()} + // Changed under the read is changed, whatever the hash came to. Part of + // what was hashed may be what this run wrote and part what somebody wrote + // since, and a match on that says nothing about what is there now. + if got.grew(f.Bytes) || got.moved() { + return Candidate{Path: f.Path, Disposition: Changed, + Detail: "it changed while it was being checked"} } - if sum != f.Hashes.SHA256 { + if got.sum != f.Hashes.SHA256 { return Candidate{Path: f.Path, Disposition: Changed, Detail: "its content is not the content this run wrote"} } - return Candidate{Path: f.Path, Disposition: Ready} + return Candidate{Path: f.Path, Disposition: Ready, seen: got.last} +} + +// changedSince says whether the file at full is no longer the file seen when +// it was checked, and why, in the words a person needs. +// +// Asked just before the file is deleted, and the reason is R01 of the review +// of 2026-10-21 (docs/STABILITY-REVIEW-2026-10-26.md). Inspect reads every +// file before Remove deletes any, so the first file was judged ours and then +// waited for the hashing of all the others - reproduced there with 257 files +// and a change 400 ms in: the changed file went, without --force, under exit +// 0. Asking again closes that wait down to the moment between this look and +// the deletion, which a file somebody is working on is very unlikely to fall +// into and which no portable call closes completely. +// +// Three questions, and each catches what the others do not. The identity is a +// file put in its place. The size is a file appended to. The time is a file +// written in place at the same length, measured on 2026-10-26 on NTFS to move +// at once, even while the writer still holds the file open. What it cannot +// see is a write in place that keeps the length and lands within the same tick +// of a coarse clock - FAT32 counts in two seconds. +func changedSince(full string, seen os.FileInfo) (string, bool) { + now, err := os.Lstat(full) + if err != nil { + // Gone, or no longer answering. Either way os.Remove would have the + // last word, and it is given it - this only stops a deletion. + return "", false + } + if !os.SameFile(seen, now) || now.Size() != seen.Size() || !now.ModTime().Equal(seen.ModTime()) { + return "it changed after it was checked, so it may not be ours. Run cleanup again to check it afresh", true + } + return "", false } // Outcome is what happened to one file. @@ -199,6 +240,14 @@ func Remove(ctx context.Context, dir string, cands []Candidate, force bool) ([]O }) continue } + // Only for a file judged ours. --force already covers a changed file, + // and a file found changed and forced has nothing to compare against. + if c.Disposition == Ready { + if why, changed := changedSince(full, c.seen); changed { + out = append(out, Outcome{Path: c.Path, Blocked: true, Reason: why}) + continue + } + } if err := os.Remove(full); err != nil { if errors.Is(err, fs.ErrNotExist) { // It went away between the two passes. That is the state the diff --git a/internal/audit/parallel.go b/internal/audit/parallel.go index 5469895a..1f14ddbe 100644 --- a/internal/audit/parallel.go +++ b/internal/audit/parallel.go @@ -133,12 +133,19 @@ func drain[T any](ctx context.Context, next *atomic.Int64, out []T, done []bool, for { i := int(next.Add(1)) - 1 // Cancellation is asked per item rather than per pass, so a Ctrl+C - // during a long hash is noticed at the next file rather than at the - // end of the run. + // between files is noticed at the next one - and verify and cleanup + // ask it inside a file as well, while they read it. if i >= len(out) || ctx.Err() != nil { return } out[i] = one(i, scratch) + // An item whose answer was cut short by the cancellation is not + // answered. A read stopped half way comes back as the context's error, + // and counting that would put "could not be read" into the prefix a + // cancelled pass reports, about a file nobody failed to read. + if ctx.Err() != nil { + return + } done[i] = true } } diff --git a/internal/audit/read.go b/internal/audit/read.go new file mode 100644 index 00000000..4d1ade61 --- /dev/null +++ b/internal/audit/read.go @@ -0,0 +1,150 @@ +package audit + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "io" + "io/fs" + "os" + "syscall" + + "github.com/donislawdev/TestingFilesGenerator/internal/core" +) + +// Reading a file the manifest lists. One way in for verify and cleanup, so +// the two cannot come to read the same file differently - and the way in +// carries three things the plain os.Open it replaced did not, each found by +// review on 2026-10-21 (docs/STABILITY-REVIEW-2026-10-26.md, R01 and R04): +// +// - The file is opened without waiting and only a file is read +// (core.OpenRegular). A name the manifest lists for an empty file and +// somebody swapped for a pipe had the right size, 0, and os.Open then +// waited on it for good. +// - Ctrl+C is asked between pieces, not only between files. A run of this +// tool writes files measured in gigabytes, and the pool asked only at the +// next file, so stopping verify meant waiting out the largest one. +// - No more is read than the manifest records, plus one byte to tell that +// there is more. A file somebody keeps appending to was read until they +// stopped. + +// listed is what reading one listed file found. +type listed struct { + // seen is the file as its handle described it before the first byte was + // read. Taken from the handle rather than the name, because on Windows + // os.Stat reads the identity of a file lazily, by name, at the moment it + // is compared - so two looks by name both describe whatever is there at + // the end and agree about a file that was replaced in between (the review + // of #157, point 4, read from stat_windows.go). Measured on 2026-10-26 on + // NTFS: this side against os.Lstat tells a replaced file of the same size + // and time apart, and a rewrite in place moves the time at once, even + // while the writer still holds the file open. + seen os.FileInfo + // last is the file as its handle described it when the reading ended, and + // the same as seen when nothing was read. + last os.FileInfo + // sum is the SHA-256 of what was read, empty when nothing was. + sum string + // read is how many bytes were read. + read int64 +} + +// absent says an open failed because nothing is at the name. +// +// One answer for every system, and that is what it is for. A listed file +// under a directory that has since become a file is ENOTDIR on Linux and +// macOS and "the path is not found" on Windows, which Go already counts as +// not existing - so without this, verify called the same directory "missing" +// on one machine and "unreadable" on the other (untouchable rule 24). +func absent(err error) bool { + return errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) +} + +// grew says more was read than the manifest records. +func (l listed) grew(want int64) bool { return l.read > want } + +// moved says the file was not the same when the reading ended as when it +// began - its size or its time changed under the read. +func (l listed) moved() bool { + return l.last.Size() != l.seen.Size() || !l.last.ModTime().Equal(l.seen.ModTime()) +} + +// readListed opens a listed file and hashes it, when its size is the size the +// manifest records. A file of another size is not read at all, so a file of +// obviously the wrong length does not cost a read of however many gigabytes it +// is. +// +// scratch belongs to the caller and is reused across every file that caller +// answers for, for the reason written at drain. It may be nil, and then this +// asks for one of its own. +// +// A cancelled read comes back as the context's error and nothing else. It is +// not an answer about the file, and the pool drops it rather than letting it +// stand as one (drain). +func readListed(ctx context.Context, path string, want int64, scratch []byte) (listed, error) { + f, seen, err := core.OpenRegular(path) + if err != nil { + return listed{}, err + } + defer func() { _ = f.Close() }() + + out := listed{seen: seen, last: seen} + if seen.Size() != want { + return out, nil + } + if scratch == nil { + scratch = make([]byte, hashScratch) + } + // One past what is recorded, so a file that is still being written to + // shows as longer rather than as the right length with the wrong content. + h := sha256.New() + if out.read, err = HashAtMost(ctx, f, want+1, h, scratch); err != nil { + return listed{}, err + } + out.sum = hex.EncodeToString(h.Sum(nil)) + + if out.last, err = f.Stat(); err != nil { + return listed{}, err + } + return out, nil +} + +// HashAtMost feeds up to limit bytes of r to h, a buffer at a time, and asks +// about Ctrl+C before every one. +// +// Exported for the guard that asks both of those of it directly. Reaching them +// through verify needs a read that is still going when the cancellation lands, +// and the only way to arrange that from outside is a clock - which is a guard +// that is slow on a slow machine and green for the wrong reason on a fast one. +// +// A loop of its own rather than io.CopyBuffer. The buffer size is measured - +// hashing a 256 MB file, median of five on 2026-09-05: io.Copy 199 ms, 64 KiB +// 182 ms, 256 KiB 161 ms, 1 MiB 162 ms - and io.CopyBuffer threw it away for an +// os.File, which implements io.WriterTo and takes the copy over with 32 KiB of +// its own. So the loop is what makes the buffer count, as well as what lets a +// read be stopped part way. +func HashAtMost(ctx context.Context, r io.Reader, limit int64, h io.Writer, scratch []byte) (int64, error) { + var read int64 + for read < limit { + if err := ctx.Err(); err != nil { + return read, err + } + piece := scratch + if left := limit - read; left < int64(len(piece)) { + piece = piece[:left] + } + n, err := r.Read(piece) + if n > 0 { + _, _ = h.Write(piece[:n]) // a hash never refuses a write + read += int64(n) + } + if errors.Is(err, io.EOF) { + return read, nil + } + if err != nil { + return read, err + } + } + return read, nil +} diff --git a/internal/guard/auditpipe_unix_test.go b/internal/guard/auditpipe_unix_test.go new file mode 100644 index 00000000..094d71f8 --- /dev/null +++ b/internal/guard/auditpipe_unix_test.go @@ -0,0 +1,71 @@ +//go:build !windows + +package guard + +import ( + "context" + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/donislawdev/TestingFilesGenerator/internal/audit" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// TestVerifyAndCleanupNeverWaitOnAPipe puts a pipe where a run left an empty +// file. The size the manifest records is 0 and a pipe's size is 0, so the size +// check let it through, and os.Open then waited on it until something wrote to +// it - which may be never (docs/STABILITY-REVIEW-2026-10-26.md, R04). Both +// commands read listed files through core.OpenRegular since 2026-10-26, which +// opens without waiting and turns away what is not a file. +// +// An empty file is the case and not a convenience: every preset that probes +// "nothing at all" lists one, and any other size the pipe fails at once. +func TestVerifyAndCleanupNeverWaitOnAPipe(t *testing.T) { + out := filepath.Join(t.TempDir(), "out") + if code, _, errOut := run(t, "generate", "--format", "txt", "--size", "0B", "--clean", "--out", out); code != 0 { + t.Fatalf("generate gave %d:\n%s", code, errOut) + } + mf := filepath.Join(out, "manifest.json") + m, err := manifest.Load(mf) + if err != nil { + t.Fatalf("loading the manifest: %v", err) + } + if len(m.Files) != 1 || m.Files[0].Bytes != 0 { + t.Fatalf("the run is not one empty file, so this is not the case it is for: %+v", m.Files) + } + full := filepath.Join(out, m.Files[0].Path) + if err := os.Remove(full); err != nil { + t.Fatalf("removing the empty file: %v", err) + } + if err := syscall.Mkfifo(full, 0o600); err != nil { + t.Fatalf("making a pipe in its place: %v", err) + } + + answersWithoutWaiting(t, "verify", func() bool { + diffs, err := audit.Verify(context.Background(), out, m, filepath.Base(mf)) + return err == nil && len(diffs) == 1 && diffs[0].Kind == audit.Unreadable + }) + answersWithoutWaiting(t, "cleanup", func() bool { + cands, err := audit.Inspect(context.Background(), out, m) + return err == nil && len(cands) == 1 && cands[0].Disposition == audit.Unreachable + }) +} + +// answersWithoutWaiting holds one command to an answer within ten seconds, and +// to the right one. +func answersWithoutWaiting(t *testing.T, name string, ask func() bool) { + t.Helper() + answered := make(chan bool, 1) + go func() { answered <- ask() }() + select { + case right := <-answered: + if !right { + t.Errorf("%s answered about the pipe, but not that it cannot read it", name) + } + case <-time.After(10 * time.Second): + t.Fatalf("%s is still waiting on the pipe after ten seconds - it opened it", name) + } +} diff --git a/internal/guard/cleanuprecheck_test.go b/internal/guard/cleanuprecheck_test.go new file mode 100644 index 00000000..d0e11292 --- /dev/null +++ b/internal/guard/cleanuprecheck_test.go @@ -0,0 +1,252 @@ +package guard + +import ( + "context" + "crypto/sha256" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/donislawdev/TestingFilesGenerator/internal/audit" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// Cleanup judges every file before it removes any, so the first file judged +// ours then waited for the hashing of all the others - and a change made to it +// in that wait was deleted without --force, under exit 0. Reproduced by an +// outside review on 2026-10-21 with 257 files and a change 400 ms in +// (docs/STABILITY-REVIEW-2026-10-26.md, R01). +// +// These ask it at the seam that wait lives in: Inspect has answered, Remove +// has not begun. No clock is involved, so the answer does not depend on how +// fast the machine hashes. + +// inspected runs a small generate and judges it for cleanup, and hands back the +// directory, the candidates and the name of the first file. +func inspected(t *testing.T) (string, []audit.Candidate, string) { + t.Helper() + out, mf := generated(t) + m, err := manifest.Load(mf) + if err != nil { + t.Fatalf("loading the manifest: %v", err) + } + victim := firstGenerated(t, out) + // Pushed an hour back before it is judged, so the write below lands on a + // later time even where the clock moves in steps of several milliseconds - + // a write in the same step is the limit changedSince names, and a guard + // that sometimes landed in it would be red for no fault. + past := time.Now().Add(-time.Hour) + if err := os.Chtimes(filepath.Join(out, victim), past, past); err != nil { + t.Fatalf("moving the time of %s back: %v", victim, err) + } + cands, err := audit.Inspect(context.Background(), out, m) + if err != nil { + t.Fatalf("inspecting: %v", err) + } + // Asserted rather than assumed: a candidate that is not ready is never + // asked again, so the question below would be answered by the old rule. + for _, c := range cands { + if c.Disposition != audit.Ready { + t.Fatalf("%s was judged %s before anything was changed, so the guard is not where it means to be", c.Path, c.Disposition) + } + } + return out, cands, victim +} + +// keptAsChanged asks that Remove left the victim on the disk with a reason that +// says why, and took the rest - the rest is what shows Remove ran at all. +func keptAsChanged(t *testing.T, out, victim string, outcomes []audit.Outcome) { + t.Helper() + if _, err := os.Stat(filepath.Join(out, victim)); err != nil { + t.Fatalf("%s changed after it was checked and cleanup removed it anyway: %v", victim, err) + } + removed := 0 + for _, o := range outcomes { + switch { + case o.Path == victim: + if !o.Blocked || !strings.Contains(o.Reason, "changed after it was checked") { + t.Errorf("%s was kept but the report does not say so and why: %+v", victim, o) + } + case o.Removed: + removed++ + } + } + if removed != len(outcomes)-1 { + t.Errorf("%d of the %d files nobody touched were removed", removed, len(outcomes)-1) + } +} + +// A file written over in place, at its own length, after it was judged ours. +func TestCleanupLeavesAFileThatChangedAfterItWasChecked(t *testing.T) { + out, cands, victim := inspected(t) + full := filepath.Join(out, victim) + + body, err := os.ReadFile(full) + if err != nil { + t.Fatalf("reading: %v", err) + } + body[0] ^= 0xFF + if err := os.WriteFile(full, body, 0o644); err != nil { + t.Fatalf("writing: %v", err) + } + + outcomes, err := audit.Remove(context.Background(), out, cands, false) + if err != nil { + t.Fatalf("removing: %v", err) + } + keptAsChanged(t, out, victim, outcomes) +} + +// A file put in the place of the one judged ours, with its size and its time. +// Only the identity tells the two apart, and on Windows only an identity read +// from the handle does - os.Stat there reads it lazily, by name, so two looks +// by name agree about whatever is there at the end (the review of #157, +// point 4). +func TestCleanupLeavesAFileReplacedAfterItWasChecked(t *testing.T) { + out, cands, victim := inspected(t) + full := filepath.Join(out, victim) + + before, err := os.Lstat(full) + if err != nil { + t.Fatalf("looking at %s: %v", victim, err) + } + body, err := os.ReadFile(full) + if err != nil { + t.Fatalf("reading: %v", err) + } + body[0] ^= 0xFF + // Made before the original goes, so the filesystem cannot hand the new + // file the number the old one had. + swap := filepath.Join(out, "swap.tmp") + if err := os.WriteFile(swap, body, 0o644); err != nil { + t.Fatalf("writing the replacement: %v", err) + } + if err := os.Chtimes(swap, before.ModTime(), before.ModTime()); err != nil { + t.Fatalf("giving the replacement the old time: %v", err) + } + if err := os.Remove(full); err != nil { + t.Fatalf("removing the original: %v", err) + } + if err := os.Rename(swap, full); err != nil { + t.Fatalf("putting the replacement in its place: %v", err) + } + // Asserted, so a replacement that differed in size or time cannot make + // this green for a reason other than the identity. + after, err := os.Lstat(full) + if err != nil { + t.Fatalf("looking at the replacement: %v", err) + } + if after.Size() != before.Size() || !after.ModTime().Equal(before.ModTime()) { + t.Fatalf("the replacement is %d B at %v and the original was %d B at %v, so this is not the case it is for", + after.Size(), after.ModTime(), before.Size(), before.ModTime()) + } + + outcomes, err := audit.Remove(context.Background(), out, cands, false) + if err != nil { + t.Fatalf("removing: %v", err) + } + keptAsChanged(t, out, victim, outcomes) +} + +// A name the system would not even look up is not a file that is missing. +// Every failure to look was called "missing" until 2026-10-26, so a file +// somebody had taken the permissions off read as gone (R04, the third of the +// further points). A name longer than any filesystem holds is the one way to +// that failure that is the same on every system: ENAMETOOLONG on Linux and +// macOS, "the syntax is incorrect" on Windows - measured on Windows the same +// day, where the build before this said missing. +func TestVerifyCallsANameItCouldNotLookUpUnreadableRatherThanMissing(t *testing.T) { + out, mf := generated(t) + m, err := manifest.Load(mf) + if err != nil { + t.Fatalf("loading the manifest: %v", err) + } + long := strings.Repeat("a", 300) + ".txt" + m.Files[0].Path, m.Files[0].Name = long, long + if err := os.Remove(mf); err != nil { + t.Fatalf("making room for the edited manifest: %v", err) + } + if err := m.Save(mf); err != nil { + t.Fatalf("saving the edited manifest: %v", err) + } + + diffs, err := audit.Verify(context.Background(), out, m, filepath.Base(mf)) + if err != nil { + t.Fatalf("verifying: %v", err) + } + for _, d := range diffs { + if d.Path != long { + continue + } + if d.Kind != audit.Unreadable || d.Got == "" { + t.Errorf("a name the system would not look up was reported as %s with %q, not as unreadable with the reason", d.Kind, d.Got) + } + return + } + t.Fatalf("the edited entry was not reported at all: %v", diffs) +} + +// cancellingReader stands for a large file. It hands out bytes for ever and +// calls the cancellation the first time it is read from. +type cancellingReader struct { + cancel func() + reads int +} + +func (r *cancellingReader) Read(p []byte) (int, error) { + r.reads++ + if r.reads == 1 { + r.cancel() + } + return len(p), nil +} + +// Ctrl+C in the middle of a large file stops the reading of it. The pool asked +// only between files, so stopping verify meant waiting out the largest one +// (R04). One read after the cancellation and no more: the question is put +// before every buffer. +func TestHashingAListedFileStopsPartWayWhenCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + r := &cancellingReader{cancel: cancel} + + _, err := audit.HashAtMost(ctx, r, 64<<20, sha256.New(), make([]byte, 256<<10)) + if !errors.Is(err, context.Canceled) { + t.Errorf("a cancelled read came back with %v, not the cancellation", err) + } + if r.reads != 1 { + t.Errorf("the file was read %d times after Ctrl+C had landed on the first read, and 256 is all of it", r.reads) + } +} + +// endlessReader stands for a file somebody keeps appending to. It refuses to go +// past a point no reasonable limit reaches, so a read with no limit ends with +// that refusal rather than never. +type endlessReader struct{ read int64 } + +func (r *endlessReader) Read(p []byte) (int, error) { + if r.read > 64<<20 { + return 0, errors.New("read past any limit this guard sets") + } + r.read += int64(len(p)) + return len(p), nil +} + +// No more is read than asked. A file that went on growing was read until it +// stopped (R04), and a listed file is asked for its recorded size and one byte +// more - the byte that says it is longer. +func TestHashingAListedFileReadsNoMoreThanItIsAskedFor(t *testing.T) { + const limit = 1000 + 1 + r := &endlessReader{} + read, err := audit.HashAtMost(context.Background(), r, limit, io.Discard, make([]byte, 256<<10)) + if err != nil { + t.Fatalf("reading: %v", err) + } + if read != limit || r.read != limit { + t.Errorf("asked for %d B, it took %d and the file handed out %d", limit, read, r.read) + } +} diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index 193e6783..25d674a4 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -1453,7 +1453,7 @@ ( "verify never looks at content, so an edited file passes", "internal/audit/audit.go", - "if sum != f.Hashes.SHA256 {", + "if got.sum != f.Hashes.SHA256 {", "if false {", "TestVerifyCatchesAMissingAnExtraAndAChangedFile", ), @@ -15542,4 +15542,57 @@ '"version": "9', "TestEveryVersionTheReadmeWritesOutIsThisOne", ), + ( + # R01 of docs/STABILITY-REVIEW-2026-10-26.md: a file judged ours and + # changed before the deletion reached it went, without --force. + "cleanup deletes a file that changed after it was judged ours, without asking the disk again", + "internal/audit/cleanup.go", + "if why, changed := changedSince(full, c.seen); changed {", + "if why, changed := changedSince(full, c.seen); false && changed {", + "TestCleanupLeavesAFileThatChangedAfterItWasChecked", + ), + ( + "a file put in the place of the one judged ours is taken for it when its size and time match", + "internal/audit/cleanup.go", + "if !os.SameFile(seen, now) || now.Size()", + "if now.Size()", + "TestCleanupLeavesAFileReplacedAfterItWasChecked", + ), + ( + "a file written over in place at its own length is still taken for the one judged ours", + "internal/audit/cleanup.go", + "now.Size() != seen.Size() || !now.ModTime().Equal(seen.ModTime()) {", + "now.Size() != seen.Size() {", + "TestCleanupLeavesAFileThatChangedAfterItWasChecked", + ), + ( + "verify calls every failure to look a missing file again", + "internal/audit/audit.go", + "if absent(err) {\n\t\treturn Difference{Kind: Missing", + "if err != nil {\n\t\treturn Difference{Kind: Missing", + "TestVerifyCallsANameItCouldNotLookUpUnreadableRatherThanMissing", + ), + ( + "a listed file is read to its end after Ctrl+C", + "internal/audit/read.go", + "if err := ctx.Err(); err != nil {\n\t\t\treturn read, err", + "if err := ctx.Err(); false && err != nil {\n\t\t\treturn read, err", + "TestHashingAListedFileStopsPartWayWhenCancelled", + ), + ( + "a listed file is read past its size and one byte, for as long as it grows", + "internal/audit/read.go", + "piece = piece[:left]", + "_ = left", + "TestHashingAListedFileReadsNoMoreThanItIsAskedFor", + ), + ( + # Unix only: a pipe exists nowhere else. The mutation runner of the CI + # is Linux, so it is judged there. + "verify and cleanup open a listed file with a plain open again, which waits on a pipe", + "internal/audit/read.go", + "f, seen, err := core.OpenRegular(path)", + "f, seen, err := func() (*os.File, os.FileInfo, error) { _ = core.OpenRegular; f, err := os.Open(path); if err != nil { return nil, nil, err }; i, err := f.Stat(); return f, i, err }()", + "TestVerifyAndCleanupNeverWaitOnAPipe", + ), ] From 0f17380ab951bbae10f6f194d95dcc98b45ba331 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 26 Oct 2026 19:53:56 +0100 Subject: [PATCH 3/7] manifest: count the record's length instead of estimating it The warning that a manifest would be too large to read back came from 786 B an entry, measured on text files in August. Measured again: an entry costs 838 B for txt, 893 for csv and 1577 for eml, so the constant was low for every format. A run of 20 500 text files wrote a manifest of 17 170 219 B with no warning and no too_large_to_read_back in it, and verify and cleanup then refused it. The manifest's length is now counted, an entry at a time so no second copy of the document is held: a dry run of a million files already peaks near three gigabytes. Save sets too_large_to_read_back from the length it writes - present exactly when the document without it is past the ceiling. engine.PredictRecord forecasts the record of a run in which every file is produced, with a placeholder hash of the real length, and the command line, the preview and the free space check all take it from there. After the save both surfaces speak from what was written, and the command line corrects its forecast when the two differ. The free space check counts the manifest and the instructions as well as the files. A disk with room for the files and not for the manifest took the files and refused the manifest, which left files nothing can remove. Rounding every file up to a cluster was not added: NTFS keeps a small file in its directory record, so it would refuse runs that fit. Cost: the forecast is a pass over every entry, about seven seconds for a million files. It is worked out once per run - the command line hands its forecast to the engine - and asks about Ctrl+C as it goes. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 + internal/cli/generate.go | 7 +- internal/cli/record.go | 58 ++++- internal/engine/engine.go | 36 ++- internal/engine/errors.go | 2 +- internal/engine/preflight.go | 14 +- internal/engine/record.go | 7 + internal/engine/recordsize.go | 118 ++++++++++ internal/guard/manifestreach_test.go | 242 ++++++++++++++++----- internal/guard/mutation/entries.py | 105 ++++++--- internal/guard/recordspace_test.go | 60 +++++ internal/guard/windowmanifestreach_test.go | 73 +++---- internal/gui/text/locale/said/en.json | 4 +- internal/gui/text/locale/said/pl.json | 2 +- internal/gui/window/run.go | 10 +- internal/gui/window/runreport.go | 27 +-- internal/manifest/manifest.go | 103 ++------- internal/manifest/reservation.go | 13 ++ internal/manifest/size.go | 183 ++++++++++++++++ 19 files changed, 811 insertions(+), 264 deletions(-) create mode 100644 internal/engine/recordsize.go create mode 100644 internal/guard/recordspace_test.go create mode 100644 internal/manifest/size.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b54ac1f..b40449ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,17 @@ because it turns other people's test suites red. than the size the manifest records. `verify` now reports a file it could not even look up, for want of permission or a name too long for the system, as `unreadable` with the reason rather than as `missing`. +- **A run is warned when its manifest will be too large to read back, + whatever the format.** The warning came from a fixed cost per file that was + low for every format, so a run of about 20 000 small files wrote a manifest + `tfg verify` and `tfg cleanup` refuse, with no warning and no + `too_large_to_read_back` in it. The manifest's length is now counted, and + after the run `generate` corrects what it said before when the two differ. + Counting adds about seven seconds to a run or preview of a million files. +- **The free space check counts the manifest and the instructions.** A disk + with room for the files and not for the manifest took the files and then + refused the manifest, which left files `tfg cleanup` cannot remove. The + refusal now says how much the run needs with its manifest. ## [0.5.0] - 2026-10-20 diff --git a/internal/cli/generate.go b/internal/cli/generate.go index 56b3ede3..8c76a9a1 100644 --- a/internal/cli/generate.go +++ b/internal/cli/generate.go @@ -423,7 +423,8 @@ func produce(ctx context.Context, targets []engine.Target, opt engine.Options, g core.ExactBytes(engine.TotalBytes(planned))) echoBoundaries(targets, planned, errOut) - echoManifestReach(planned, errOut) + forecast, warned := echoManifestReach(ctx, planned, opt, errOut) + opt.Record = forecast if g.dryRun { fmt.Fprintln(errOut, "dry run - nothing was written.") @@ -455,13 +456,13 @@ func produce(ctx context.Context, targets []engine.Target, opt engine.Options, g // Writing one would replace the record of whatever was already in the // directory, and that record is the only thing cleanup can work from. if !g.dryRun && res.Started { - saveManifest(res, opt, errOut) + saveManifest(res, opt, warned, errOut) } return classify(runErr) } if !g.dryRun { - if code := saveManifest(res, opt, errOut); code != ExitOK { + if code := saveManifest(res, opt, warned, errOut); code != ExitOK { return code } } diff --git a/internal/cli/record.go b/internal/cli/record.go index 324b9db5..69c0982a 100644 --- a/internal/cli/record.go +++ b/internal/cli/record.go @@ -80,26 +80,61 @@ func whatSurvived(runErr error, res *engine.Result) string { // today. What was missing was that nobody was told. It is printed before the // first byte and on a dry run, which is the step this project tells people to // take before anything large. -func echoManifestReach(planned []engine.PlannedFile, errOut io.Writer) { - noted := 0 - for _, f := range planned { - if len(f.Plan.Notes) > 0 { - noted++ - } +// +// It hands back the forecast, for the run to use rather than work out a second +// time, and whether it said anything, so that what the run really wrote can +// correct it afterwards (echoManifestOutcome). The forecast is the manifest of +// a run in which every file is produced - since 2026-10-26 counted entry by +// entry rather than from a constant that was low for every format, so a run of +// 20 500 text files is no longer told nothing and then left with a manifest +// verify refuses (docs/STABILITY-REVIEW-2026-10-26.md, section 2.1). +func echoManifestReach(ctx context.Context, planned []engine.PlannedFile, opt engine.Options, errOut io.Writer) (*engine.RecordSize, bool) { + record, err := engine.PredictRecord(ctx, planned, opt) + if err != nil { + // A forecast that could not be made is not a reason to stop the run + // here. The run works it out again and answers for it, and what was + // written is measured again after the save and said then. + return nil, false } - size, over := manifest.TooLargeToReadBack(len(planned), noted) - if !over { - return + if !record.TooLarge { + return &record, false } fmt.Fprintf(errOut, "note: this run will write a manifest of roughly %s and this build reads at most %s, "+ "so tfg verify and tfg cleanup will not be able to read it. Split the run to keep each manifest readable.\n", - core.HumanBytes(size), core.HumanBytes(manifest.MaxBytes)) + core.HumanBytes(record.Manifest), core.HumanBytes(manifest.MaxBytes)) + return &record, true +} + +// echoManifestOutcome says what the manifest came to, when that is not what +// the run was told before its first byte. +// +// The forecast is right for a run in which every file is produced. A file that +// fails gains a note in its entry, so the manifest can grow past the ceiling +// after the forecast said it would not - and then this is the only sentence +// that says so. The other way round, a run warned and then within the ceiling, +// is corrected too: an alarm left standing after the fact is a sentence the +// person acts on, and it would be false. +func echoManifestOutcome(written engine.RecordSize, warned bool, errOut io.Writer) { + switch { + case written.TooLarge && !warned: + fmt.Fprintf(errOut, + "note: this run wrote a manifest of %s and this build reads at most %s, "+ + "so tfg verify and tfg cleanup will not be able to read it. Split the run to keep each manifest readable.\n", + core.HumanBytes(written.Manifest), core.HumanBytes(manifest.MaxBytes)) + case !written.TooLarge && warned: + fmt.Fprintf(errOut, + "note: the manifest came to %s, within the %s this build reads, so tfg verify and tfg cleanup can read it after all.\n", + core.HumanBytes(written.Manifest), core.HumanBytes(manifest.MaxBytes)) + } } // saveManifest saves the record of a run - the manifest, and the instructions // beside it when any file was given a purpose - and says where each went. -func saveManifest(res *engine.Result, opt engine.Options, errOut io.Writer) int { +// +// warned is whether the run was told before its first byte that its manifest +// would be too large to read back - see echoManifestOutcome. +func saveManifest(res *engine.Result, opt engine.Options, warned bool, errOut io.Writer) int { // Asked of the engine rather than joined here. The engine claimed this // exact name before the first file was written, so working it out a second // way is a chance for the saver and the claim to mean different files. @@ -137,5 +172,6 @@ func saveManifest(res *engine.Result, opt engine.Options, errOut io.Writer) int "tfg: cannot write the instructions to %s: %s. The manifest was saved and holds the same facts.\n", core.Shown(rec.Missed.Path), describeError(rec.Missed.Err)) } + echoManifestOutcome(rec.Size, warned, errOut) return ExitOK } diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 9df4428f..ad84d819 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -9,14 +9,12 @@ import ( "io/fs" "os" "path/filepath" - "runtime" "strings" "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/damage" "github.com/donislawdev/TestingFilesGenerator/internal/format" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" - "github.com/donislawdev/TestingFilesGenerator/internal/version" ) // Target is one group of files to produce. @@ -147,6 +145,14 @@ type Options struct { // on a handful of files, and the real number is asked about separately. MaxPlanBytes int64 + // Record is the forecast of this run's record, when the caller has already + // worked it out for these same files with PredictRecord - the command line + // does, to warn before the first byte. A forecast is a pass over every + // entry, about seven seconds for a million files measured on 2026-10-26, + // and asked twice it doubled a dry run of that size. Nil means the run + // works it out. + Record *RecordSize + // OnProgress is called as the run advances. Nil means silence, which is // what every caller that has nobody to show it to should pass. // @@ -201,6 +207,12 @@ type Result struct { // the notes in its own language from here, and the manifest stays English. NotesSaid map[string]core.Said + // Record is how large the record of this run was worked out to be before + // its first byte, the manifest and the instructions (PredictRecord). For a + // dry run that is the answer about the run it previews. A run that is + // saved knows the real length afterwards, in Record of SaveRecord. + Record RecordSize + // reservation is the run's hold on its manifest name, taken before the // first file and handed to SaveRecord, which saves through it. Nil for a // dry run and for a run refused before it reserved anything. @@ -468,21 +480,19 @@ func PlanContext(ctx context.Context, targets []Target, opt Options) ([]PlannedF // it may start at all, and the reading back of the answers in the order the // plan lists them. func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) { - m := manifest.New( - "testing-files-generator", version.Version, - runID(opt.Seed), opt.Command, opt.Seed, - runtime.GOOS, runtime.GOARCH, - ) - m.Run.RecipeHash = opt.RecipeHash - m.Run.Overrides = opt.Overrides - m.Run.Preset = opt.Preset + m := newManifest(opt) res := &Result{Manifest: m} // Whether this run may start at all is settled before anything is written // and before a dry run returns. A dry run exists to count and show before // the disk is touched, so reporting success for a run that refuses to // start on the very next line answers the wrong question. - if err := preflight(ctx, files, opt); err != nil { + record, err := opt.forecast(ctx, files) + if err != nil { + return res, err + } + res.Record = record + if err := preflight(ctx, files, opt, record); err != nil { return res, err } @@ -495,6 +505,10 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) m.Add(res.entryFor(f, "", false, nil)) } m.Run.Complete = true + // The preview's document has no hashes and is shorter than the one the + // run writes, so what it says about being read back is said about the + // run, from the record worked out for it. + m.Summary.TooLargeToReadBack = record.TooLarge return res, nil } diff --git a/internal/engine/errors.go b/internal/engine/errors.go index f36ac80c..8a227662 100644 --- a/internal/engine/errors.go +++ b/internal/engine/errors.go @@ -214,7 +214,7 @@ func (e *SpaceError) Error() string { return e.Said().String() } // Said is the refusal, for a window that says it in its own language. func (e *SpaceError) Said() core.Said { return core.Says("engine.NoSpace", - "this run needs %d B and %s has %d B free - nothing was written. Ask for fewer files or a smaller size, or write to another disk by changing the output directory", + "this run needs %d B with its manifest and %s has %d B free - nothing was written. Ask for fewer files or a smaller size, or write to another disk by changing the output directory", core.A("Needed", e.Needed), core.A("Dir", core.Shown(e.Path)), core.A("Free", e.Available)) } diff --git a/internal/engine/preflight.go b/internal/engine/preflight.go index 668b3a56..2621bf8f 100644 --- a/internal/engine/preflight.go +++ b/internal/engine/preflight.go @@ -32,11 +32,19 @@ const DefaultManifestName = "manifest.json" // // Both checks used to sit after the dry run had already returned, so // --dry-run reported success for runs that would refuse to start. -func preflight(ctx context.Context, files []PlannedFile, opt Options) error { +func preflight(ctx context.Context, files []PlannedFile, opt Options, record RecordSize) error { // Free space first. Finding out at file five thousand of ten thousand // leaves a half written set and a full disk on a machine somebody works on. - needed := TotalBytes(files) - if available, err := opt.availableBytes(opt.OutDir); err == nil { + // + // The record counts as well as the files, since 2026-10-26. A disk with + // room for the files and not for the manifest took the files and refused + // the manifest, which left files nothing can remove - the manifest is the + // only authority over that (docs/STABILITY-REVIEW-2026-10-26.md, R08). + // What it cannot promise is the space somebody else takes while the run + // goes. Rounding every file up to a cluster is NOT added: NTFS keeps a small + // file inside its directory record, so it would refuse runs that fit. + needed, sumErr := core.AddSizes(TotalBytes(files), record.Bytes()) + if available, err := opt.availableBytes(opt.OutDir); err == nil && sumErr == nil { if available < needed { return &SpaceError{Needed: needed, Available: available, Path: opt.OutDir} } diff --git a/internal/engine/record.go b/internal/engine/record.go index 6dd375f4..3a07de53 100644 --- a/internal/engine/record.go +++ b/internal/engine/record.go @@ -19,6 +19,10 @@ type Record struct { // not failed by it - the manifest was saved and holds the same facts - and // the caller says it out loud rather than letting it pass. Missed *InstructionsError + // Size is what was written, measured on the way rather than worked out + // beforehand: Result.Record is the forecast, this is the fact. Its + // TooLarge is the field the saved manifest carries. + Size RecordSize } // InstructionsError is instructions that were due and could not be written. @@ -60,6 +64,7 @@ func SaveRecord(res *Result, opt Options) (Record, error) { } else { instructions = own rec.Instructions = path + rec.Size.Instructions = int64(len(text)) m.Run.Instructions = filepath.Base(path) } } @@ -68,9 +73,11 @@ func SaveRecord(res *Result, opt Options) (Record, error) { // The file this call wrote, and nothing that took its name since. _ = core.RemoveOwn(rec.Instructions, instructions) rec.Instructions, m.Run.Instructions = "", "" + rec.Size.Instructions = 0 } return rec, err } + rec.Size.Manifest, rec.Size.TooLarge = m.SavedBytes(), m.Summary.TooLargeToReadBack return rec, nil } diff --git a/internal/engine/recordsize.go b/internal/engine/recordsize.go new file mode 100644 index 00000000..cb3323a8 --- /dev/null +++ b/internal/engine/recordsize.go @@ -0,0 +1,118 @@ +package engine + +import ( + "context" + "path/filepath" + "runtime" + "strings" + + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" + "github.com/donislawdev/TestingFilesGenerator/internal/version" +) + +// How large the record of a run will be, worked out from the plan before a +// byte is written (docs/STABILITY-REVIEW-2026-10-26.md, R03 and R08). +// +// Two things ask it. The free space check: until 2026-10-26 it held the free +// space against the files alone, so a disk with room for the files and not +// for the manifest took the files and refused the manifest - files nothing can +// remove, because the manifest is the only authority over that (untouchable +// rule 7). And the warning that a manifest will be too large to read back, +// which came from a constant per entry that was low for every format. + +// RecordSize is the record a run will write beside its files. +type RecordSize struct { + // Manifest is the length of the manifest. + Manifest int64 + // TooLarge says this build would refuse to read that manifest back. + TooLarge bool + // Instructions is the length of the instructions, nought when no file was + // given a purpose and none are written. + Instructions int64 +} + +// Bytes is the whole record. +func (r RecordSize) Bytes() int64 { return r.Manifest + r.Instructions } + +// hashPlaceholder stands in for a hash not worked out yet. Every hash a run +// records is this long, so the document is as long as the one the run writes. +var hashPlaceholder = strings.Repeat("0", 64) + +// PredictRecord is the record a run of these files writes when every file is +// produced. A file that fails gains a note in its entry, so a run with +// failures writes a little more than this says - which is why the surfaces say +// again, from what was written, when the two disagree. +// +// Entries are counted one at a time and not kept (manifest.Projection). The +// instructions are the exception: what they say depends on every file at once, +// so for a run that gives files a purpose the document is built whole to write +// them. Runs that do are presets and recipes written by hand, not the runs of +// a million files that a whole document would weigh on. +// +// Ctrl+C is asked as it goes. It runs before the first byte, where stopping is +// expected to be immediate, and over a million files it is a pass worth +// stopping - the shape R05 of the review of 2026-10-21 found in the drawing of +// sizes, which this must not repeat. +func PredictRecord(ctx context.Context, files []PlannedFile, opt Options) (RecordSize, error) { + scratch := &Result{} + header := newManifest(opt) + header.Run.Complete = true + counted := header.Projection() + explains := false + for i, f := range files { + if i%askEvery == 0 { + if err := ctx.Err(); err != nil { + return RecordSize{}, err + } + } + e := scratch.entryFor(f, hashPlaceholder, true, nil) + explains = explains || e.Purpose != "" + counted.Add(e) + } + var out RecordSize + if explains { + header.Run.Instructions = filepath.Base(InstructionsPath(opt)) + whole := newManifest(opt) + for i, f := range files { + if i%askEvery == 0 { + if err := ctx.Err(); err != nil { + return RecordSize{}, err + } + } + whole.Add(scratch.entryFor(f, hashPlaceholder, true, nil)) + } + out.Instructions = int64(len(whole.Instructions(filepath.Base(ManifestPath(opt))))) + } + var err error + out.Manifest, out.TooLarge, err = counted.Bytes() + return out, err +} + +// forecast is the record the caller handed in, or the one worked out here +// when it handed in none. +func (o Options) forecast(ctx context.Context, files []PlannedFile) (RecordSize, error) { + if o.Record != nil { + return *o.Record, nil + } + return PredictRecord(ctx, files, o) +} + +// askEvery is how many entries go by between two questions about Ctrl+C. An +// entry costs a few microseconds, so this answers within a few milliseconds +// and asks a thousand times less often than there are entries. +const askEvery = 1024 + +// newManifest is the empty manifest of a run with these options. One place, +// so the record a run writes and the record PredictRecord measures start from +// the same header. +func newManifest(opt Options) *manifest.Manifest { + m := manifest.New( + "testing-files-generator", version.Version, + runID(opt.Seed), opt.Command, opt.Seed, + runtime.GOOS, runtime.GOARCH, + ) + m.Run.RecipeHash = opt.RecipeHash + m.Run.Overrides = opt.Overrides + m.Run.Preset = opt.Preset + return m +} diff --git a/internal/guard/manifestreach_test.go b/internal/guard/manifestreach_test.go index d1588325..b33b4c49 100644 --- a/internal/guard/manifestreach_test.go +++ b/internal/guard/manifestreach_test.go @@ -4,12 +4,14 @@ import ( "bytes" "context" "encoding/json" + "errors" "os" "path/filepath" "strings" "testing" "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) @@ -18,8 +20,7 @@ import ( // // Two ceilings were chosen independently and never held against each other. // core.MaxFilesPerRun is a million files. manifest.MaxBytes is sixteen -// megabytes, and its comment works out that this "holds a run of roughly twenty -// thousand files". +// megabytes. // // Measured on 2026-08-26: a run of 25 000 files wrote a manifest of 25 220 640 // B, and from that moment "tfg verify" and "tfg cleanup" both exited 5 on it @@ -31,19 +32,22 @@ import ( // The owner's decision on the same day was a note rather than a refusal: the // run works, and refusing it would take away something this tool does today. // What was missing was that nobody was told. -func TestARunSaysWhenItsManifestWillBeTooBigToReadBack(t *testing.T) { - // Just over the ceiling, worked out from the estimate rather than from a - // number written here - a guard carrying its own copy of a limit goes - // stale the day somebody changes the real one and says nothing while it - // does. - over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 - if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { - t.Fatalf("%d entries was not judged too large, so this proved nothing", over) - } - said := runCLI(t, "generate", "--format", "txt", "--size", "200b", - "--count", itoa(over), "--dry-run", "--out", t.TempDir()) +// inTheSilentBand is a run that was told nothing until 2026-10-26. +// +// The warning came from 786 B an entry, and a text file of one byte with no +// label costs about 838 - so 20 500 of them came to 16 113 000 B by the +// estimate, under the ceiling, and wrote 17 170 219 B, over it +// (docs/STABILITY-REVIEW-2026-10-26.md, section 2.1). A run here is a run the +// old arithmetic let through in silence, which is the run these guards are for. +var inTheSilentBand = []string{"generate", "--format", "txt", "--size", "1b", "--clean", "--count", "20500", "--dry-run"} +func TestARunSaysWhenItsManifestWillBeTooBigToReadBack(t *testing.T) { + said := runCLI(t, append(inTheSilentBand, "--out", t.TempDir())...) + + if !strings.Contains(said, "will not be able to read") { + t.Fatalf("a run whose manifest will be past the ceiling was told nothing:\n%s", firstLines(said, 6)) + } if !strings.Contains(said, "verify") || !strings.Contains(said, "cleanup") { t.Errorf("the note does not name the two commands that stop working:\n%s", said) } @@ -66,46 +70,54 @@ func TestAnOrdinaryRunSaysNothingAboutTheManifestCeiling(t *testing.T) { } } -// The estimate has to stay near what a manifest really weighs. +// The length worked out for a manifest is the length Encode writes, to the +// byte. // -// An estimate nobody checks is a number nobody measured, and this one decides -// whether the warning above appears at all. It is held to a wide band on -// purpose: the exact size moves with how long the names are and what the notes -// say, and a guard demanding the byte would go red for a reason that does not -// matter. -func TestTheManifestEstimateIsNearWhatAManifestWeighs(t *testing.T) { - dir := t.TempDir() - const files = 300 +// This replaces a guard that held an estimate to "a quarter either way", and +// a quarter was wide enough to let a constant 6% low for text and half of what +// an e-mail entry costs stay green while a run of 20 500 files went unwarned. +// The answer is counted entry by entry now, so it is held to the byte: a +// document of no entries, one entry, and many unlike each other - notes, a +// failure, words outside ASCII, characters JSON escapes, properties nested +// several deep. +func TestTheManifestSizeIsTheLengthOfTheManifest(t *testing.T) { + documents := map[string]*manifest.Manifest{"no entries": newDocument()} - runCLI(t, "generate", "--format", "txt", "--size", "200b", - "--count", itoa(files), "--out", dir) + one := newDocument() + one.Add(manifest.File{ID: "a", Path: "a.txt", Name: "a.txt", Format: "txt", Bytes: 1}) + documents["one entry"] = one - info, err := os.Stat(filepath.Join(dir, "manifest.json")) - if err != nil { - t.Fatalf("the run wrote no manifest: %v", err) - } - real := info.Size() - guess := manifest.EstimatedBytes(files, 0) + many := newDocument() + many.Run.Preset = &manifest.Preset{ID: "upload-validation", Parameters: map[string]string{"limit": "1mb"}} + many.Add(manifest.File{ID: "plain", Path: "plain.txt", Name: "plain.txt", Format: "txt", Bytes: 200, Materialized: true, + Hashes: manifest.Hashes{SHA256: strings.Repeat("ab", 32)}, TargetID: "t"}) + many.Add(manifest.File{ID: "noted", Path: "zażółć 日本.txt", Name: "zażółć 日本.txt", Format: "txt", + Notes: []manifest.Note{{Code: "label_omitted", Detail: "it said & \"left\" \\ alone"}}, + Expected: manifest.Expected{Outcome: "reject", Reason: "size_limit"}, Purpose: "a & c", Group: "g"}) + many.Add(manifest.File{ID: "deep", Path: "deep.zip", Name: "deep.zip", Format: "zip", + Properties: map[string]any{"contains": []any{map[string]any{"format": "txt", "count": 3}}, "depth": 2, "flat": true, "ratio": 0.5}}) + many.Add(manifest.File{ID: "failed", Path: "failed.png", Name: "failed.png", Format: "png", Failed: true, + Error: "a generator stopped", Damage: []manifest.Damage{{Type: "zero-head", Settings: map[string]string{"bytes": "4"}}}}) + documents["many entries"] = many - // Within a quarter either way. Wider than the measurement, narrow enough - // that the constant drifting by a factor would be caught. - low, high := real*3/4, real*5/4 - if guess < low || guess > high { - t.Errorf("the estimate for %d files is %d B and the manifest is %d B, which is outside a quarter either way", - files, guess, real) + for name, m := range documents { + counted, err := m.EncodedBytes() + if err != nil { + t.Fatalf("%s: counting: %v", name, err) + } + var written bytes.Buffer + if err := m.Encode(&written); err != nil { + t.Fatalf("%s: encoding: %v", name, err) + } + if counted != int64(written.Len()) { + t.Errorf("%s: the manifest was counted at %d B and Encode wrote %d B", name, counted, written.Len()) + } } } -// runCLI runs one command and gives back everything it said, on both channels. -// -// Both, because the line under test goes to the error channel - a report on -// stdout has to stay machine readable - and a guard reading only one of them -// would be measuring which channel it picked. -func runCLI(t *testing.T, args ...string) string { - t.Helper() - var out, errOut bytes.Buffer - cli.Run(context.Background(), args, &out, &errOut) - return out.String() + errOut.String() +// newDocument is an empty manifest with the header a run gives it. +func newDocument() *manifest.Manifest { + return manifest.New("testing-files-generator", "0.0.0-dev", "run_x", "tfg generate", 1, "linux", "amd64") } // And it says so where a script can read it, not only in prose. @@ -130,19 +142,9 @@ func runCLI(t *testing.T, args ...string) string { // produced" and point at a manifest nothing can read. Exit codes are a frozen // contract. func TestAManifestTooBigToReadBackSaysSoInTheReportAScriptReads(t *testing.T) { - over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 - - // Asked of the estimator first, so a build whose ceiling moved above this - // count fails here rather than passing the whole test by never being over - // it at all. - if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { - t.Fatalf("%d entries was not judged too large, so this proved nothing", over) - } - // stdout only. The summary line goes to the error stream, and a reader that // took both would be parsing JSON with a sentence stuck to the end of it. - code, said, errOut := run(t, "generate", "--format", "txt", "--size", "200b", - "--count", itoa(over), "--dry-run", "--json", "--out", t.TempDir()) + code, said, errOut := run(t, append(inTheSilentBand, "--json", "--out", t.TempDir())...) if code != cli.ExitOK { t.Fatalf("the run ended with %d, so there is no report to read:\n%s", code, errOut) } @@ -200,6 +202,132 @@ func TestAnOrdinaryRunSaysNothingAboutTheCeilingInItsReport(t *testing.T) { } } +// A saved manifest carries the field exactly when this build will refuse it, +// asked one byte either side of the ceiling. +// +// The field is written into the document whose length it describes, so the +// edge is where a rule that is nearly right goes wrong: a manifest of exactly +// manifest.MaxBytes is read and must not say otherwise, and one byte more is +// refused and must say so. Saved for real and read back, so the answer is about +// the bytes on the disk rather than about the arithmetic that predicted them. +func TestASavedManifestSaysItIsTooBigExactlyWhenItIs(t *testing.T) { + for _, past := range []int64{0, 1} { + m := documentOfLength(t, manifest.MaxBytes+past) + path := filepath.Join(t.TempDir(), "manifest.json") + if err := m.Save(path); err != nil { + t.Fatalf("saving: %v", err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("looking at the saved manifest: %v", err) + } + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading the saved manifest: %v", err) + } + says := bytes.Contains(raw, []byte(`"too_large_to_read_back": true`)) + _, loadErr := manifest.Load(path) + var refused *manifest.TooLargeError + isRefused := errors.As(loadErr, &refused) + + switch { + case past == 0 && (info.Size() != manifest.MaxBytes || says || isRefused): + t.Errorf("a manifest of the ceiling's length: %d B on the disk, field present %v, refused %v (%v) - it is read and must not say it is not", + info.Size(), says, isRefused, loadErr) + case past == 1 && (info.Size() <= manifest.MaxBytes || !says || !isRefused): + t.Errorf("a manifest one byte longer: %d B on the disk, field present %v, refused %v - it is refused and must say so", + info.Size(), says, isRefused) + } + if m.SavedBytes() != info.Size() { + t.Errorf("Save says it wrote %d B and the disk holds %d B", m.SavedBytes(), info.Size()) + } + } +} + +// documentOfLength is a manifest whose document, without the field, is exactly +// length bytes long - one entry's purpose takes up the difference. +func documentOfLength(t *testing.T, length int64) *manifest.Manifest { + t.Helper() + m := newDocument() + m.Add(manifest.File{ID: "a", Path: "a.txt", Name: "a.txt", Format: "txt", Purpose: "x"}) + now, err := m.EncodedBytes() + if err != nil { + t.Fatalf("counting: %v", err) + } + m.Files[0].Purpose += strings.Repeat("x", int(length-now)) + // Measured as it stands, before Save has settled the field - so this is the + // length the rule is asked about, and nothing here has set the field yet + // for Save to find already set. + if got, _ := m.EncodedBytes(); got != length || m.Summary.TooLargeToReadBack { + t.Fatalf("the document is %d B (field set: %v) and was built to be %d B without it, so the edge is not where this asks", + got, m.Summary.TooLargeToReadBack, length) + } + return m +} + +// What a run is told about its record before its first byte is what it then +// writes, when every file is produced - the manifest to the byte, and the +// instructions beside it. +// +// The forecast is what the free space is held against and what the warning +// above is said from. A forecast that drifted from the record would put both +// back where the old constant had them, wrong in a direction nobody sees. +func TestTheRecordARunIsToldBeforehandIsTheRecordItWrites(t *testing.T) { + dir := t.TempDir() + opt := engine.Options{OutDir: dir, Seed: 7, Command: "test"} + told := txtTarget("told", 3, 100) + told.Purpose, told.Group = "A file with a purpose, zażółć <&>", "with a purpose" + planned, err := engine.Plan([]engine.Target{txtTarget("plain", 300, 200), told}, opt) + if err != nil { + t.Fatalf("planning: %v", err) + } + + forecast, err := engine.PredictRecord(context.Background(), planned, opt) + if err != nil { + t.Fatalf("forecasting: %v", err) + } + res, err := engine.Run(context.Background(), planned, opt) + if err != nil { + t.Fatalf("running: %v", err) + } + rec, err := engine.SaveRecord(res, opt) + if err != nil { + t.Fatalf("saving the record: %v", err) + } + + if rec.Instructions == "" || forecast.Instructions == 0 { + t.Fatalf("no instructions were written or forecast (%q, %d B), so half of this asked nothing", rec.Instructions, forecast.Instructions) + } + onDisk := map[string]int64{} + for name, path := range map[string]string{"manifest": rec.Manifest, "instructions": rec.Instructions} { + info, err := os.Stat(path) + if err != nil { + t.Fatalf("looking at the %s: %v", name, err) + } + onDisk[name] = info.Size() + } + if forecast.Manifest != onDisk["manifest"] || rec.Size.Manifest != onDisk["manifest"] { + t.Errorf("the manifest was forecast at %d B, recorded as %d B and is %d B on the disk", + forecast.Manifest, rec.Size.Manifest, onDisk["manifest"]) + } + if forecast.Instructions != onDisk["instructions"] || rec.Size.Instructions != onDisk["instructions"] { + t.Errorf("the instructions were forecast at %d B, recorded as %d B and are %d B on the disk", + forecast.Instructions, rec.Size.Instructions, onDisk["instructions"]) + } +} + +// runCLI runs one command and gives back everything it said, on both channels. +// +// Both, because the line under test goes to the error channel - a report on +// stdout has to stay machine readable - and a guard reading only one of them +// would be measuring which channel it picked. +func runCLI(t *testing.T, args ...string) string { + t.Helper() + var out, errOut bytes.Buffer + cli.Run(context.Background(), args, &out, &errOut) + return out.String() + errOut.String() +} + // firstLines trims a long document down for a failure message. func firstLines(s string, n int) string { lines := strings.Split(s, "\n") diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index 25d674a4..fa19d8bc 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -53,30 +53,30 @@ ( "the preview stops warning that the record will be too big to read back", "internal/gui/window/run.go", - "r.say(append([]string{said}, manifestReachNote(res)...)...)", + "r.say(append([]string{said}, manifestReachNote(res.Record)...)...)", "r.say(said)", "TestTheWindowSaysWhenItsManifestWillBeTooBigToReadBack", ), ( "a finished run stops warning, so somebody who never pressed Preview is told nothing", "internal/gui/window/run.go", - "said := append([]string{outcome}, manifestReachNote(res)...)", + "said := append([]string{outcome}, manifestReachNote(saved.Size)...)", "said := []string{outcome}", "TestAFinishedRunInTheWindowSaysItsManifestIsTooBigToReadBack", ), ( "the window warns about every run, which teaches people to skip the line", "internal/gui/window/runreport.go", - "if !over {", - "if false && !over {", + "if !record.TooLarge {", + "if false && !record.TooLarge {", "TestAnOrdinaryPreviewSaysNothingAboutTheManifestCeiling", ), ( - "the document answers a different question than the plan, so one run is warned about on one surface only", - "internal/manifest/manifest.go", - "return TooLargeToReadBack(len(m.Files), m.notedFiles)", - "return TooLargeToReadBack(len(m.Files)+1000, m.notedFiles)", - "TestBothSurfacesJudgeTheSameRunTheSameWay", + "the forecast of a run's record counts a hash one character short, so it drifts from what the run writes", + "internal/engine/recordsize.go", + "strings.Repeat(\"0\", 64)", + "strings.Repeat(\"0\", 63)", + "TestTheRecordARunIsToldBeforehandIsTheRecordItWrites", ), # --- verify says whose the other files are (2026-09-07, T8) --- ( @@ -242,9 +242,9 @@ # --- a manifest too big to read back says so where a script reads (2026-09-06) --- ( "the report stops saying the manifest cannot be read back", - "internal/manifest/manifest.go", - "_, m.Summary.TooLargeToReadBack = TooLargeToReadBack(len(m.Files), m.notedFiles)", - "_, _ = TooLargeToReadBack(len(m.Files), m.notedFiles)", + "internal/engine/engine.go", + "m.Summary.TooLargeToReadBack = record.TooLarge", + "_ = record.TooLarge", "TestAManifestTooBigToReadBackSaysSoInTheReportAScriptReads", ), ( @@ -821,8 +821,8 @@ ( "a dry run skips the checks and promises a run that would be refused", "internal/engine/engine.go", - "if err := preflight(ctx, files, opt); err != nil {", - "if err := preflight(ctx, files, opt); err != nil && !opt.DryRun {", + "if err := preflight(ctx, files, opt, record); err != nil {", + "if err := preflight(ctx, files, opt, record); err != nil && !opt.DryRun {", "TestADryRunReachesTheSameVerdictAsTheRealRun", ), ( @@ -6137,16 +6137,16 @@ ( "a run stops saying that its manifest will be too big for verify and cleanup to read", "internal/cli/record.go", - "\tif !over {", - "\tif true || !over {", + "if !record.TooLarge {", + "if true || !record.TooLarge {", "TestARunSaysWhenItsManifestWillBeTooBigToReadBack", ), ( - "the manifest size estimate stops matching what a manifest really weighs", - "internal/manifest/manifest.go", - "\tBytesPerEntry = 786", - "\tBytesPerEntry = 79", - "TestTheManifestEstimateIsNearWhatAManifestWeighs", + "the counted length of a manifest leaves out the line that closes its list of files", + "internal/manifest/size.go", + "between*int64(n-1) + closing", + "between*int64(n-1)", + "TestTheManifestSizeIsTheLengthOfTheManifest", ), ( "the size-boundaries preset stops refusing a limit with no number above it", @@ -6179,8 +6179,8 @@ ( "every run starts warning that its manifest is too big to read back", "internal/cli/record.go", - "\tif !over {", - "\tif !over && false {", + "if !record.TooLarge {", + "if !record.TooLarge && false {", "TestAnOrdinaryRunSaysNothingAboutTheManifestCeiling", ), ( @@ -6254,10 +6254,10 @@ "TestAContainerRefusesASizeItsArithmeticCannotSee", ), ( - "preflight stops noticing that it was asked to stop", + "the forecast of the record and the preflight both stop noticing that they were asked to stop", "internal/engine/engine.go", - "\tif err := preflight(ctx, files, opt); err != nil {", - "\tif err := preflight(context.Background(), files, opt); err != nil {", + "record, err := opt.forecast(ctx, files)\n\tif err != nil {\n\t\treturn res, err\n\t}\n\tres.Record = record\n\tif err := preflight(ctx, files, opt, record); err != nil {", + "record, err := opt.forecast(context.Background(), files)\n\tif err != nil {\n\t\treturn res, err\n\t}\n\tres.Record = record\n\tif err := preflight(context.Background(), files, opt, record); err != nil {", "TestWorkingOutTheCostCanBeStopped", ), ( @@ -6660,8 +6660,8 @@ "the engine says something on stdout on the SUCCESS path, where the " "guard that a failed run prints nothing cannot see it", "internal/engine/engine.go", - "func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) {\n\tm := manifest.New(", - "func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) {\n\tfmt.Println(\"running\")\n\tm := manifest.New(", + "func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) {\n\tm := newManifest(opt)", + "func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) {\n\tfmt.Println(\"running\")\n\tm := newManifest(opt)", "TestNothingBelowASurfaceReachesTheConsoleDirectly", ), ( @@ -12781,8 +12781,8 @@ ( 'a manifest that could not be written leaves its empty claim behind, so the files it should have named cannot be cleaned up', 'internal/manifest/reservation.go', - '\t\t_ = core.RemoveOwn(r.tmp, r.own)\n\t\treturn err\n\t}\n\treturn nil', - '\t\treturn err\n\t}\n\treturn nil', + '\t\t_ = core.RemoveOwn(r.tmp, r.own)\n\t\treturn err\n\t}\n\tm.saved = size\n\treturn nil', + '\t\treturn err\n\t}\n\tm.saved = size\n\treturn nil', 'TestAManifestThatCouldNotBeWrittenLeavesNoManifest', ), ( @@ -15595,4 +15595,49 @@ "f, seen, err := func() (*os.File, os.FileInfo, error) { _ = core.OpenRegular; f, err := os.Open(path); if err != nil { return nil, nil, err }; i, err := f.Stat(); return f, i, err }()", "TestVerifyAndCleanupNeverWaitOnAPipe", ), + ( + # R03 of docs/STABILITY-REVIEW-2026-10-26.md: a manifest past the + # ceiling went out without the field that says so. + "a saved manifest past the ceiling no longer says it cannot be read back", + "internal/manifest/size.go", + "header.Summary.TooLargeToReadBack = true", + "header.Summary.TooLargeToReadBack = false", + "TestASavedManifestSaysItIsTooBigExactlyWhenItIs", + ), + ( + "a manifest of exactly the ceiling's length says it cannot be read back, though it is read", + "internal/manifest/size.go", + "size <= MaxBytes {", + "size < MaxBytes {", + "TestASavedManifestSaysItIsTooBigExactlyWhenItIs", + ), + ( + "Save writes the manifest without settling the field from its real length", + "internal/manifest/reservation.go", + "size, err := m.settleReadBack()", + "size, err := m.SavedBytes(), error(nil)", + "TestASavedManifestSaysItIsTooBigExactlyWhenItIs", + ), + ( + "the counted length of a manifest leaves out the indentation of the first line of every entry", + "internal/manifest/size.go", + "return int64(len(entryIndent) + len(b)), nil", + "return int64(len(b)), nil", + "TestTheManifestSizeIsTheLengthOfTheManifest", + ), + ( + # R08: the free space was held against the files alone. + "the free space is held against the files alone again, so a disk with no room for the manifest takes the files", + "internal/engine/preflight.go", + "core.AddSizes(TotalBytes(files), record.Bytes())", + "core.AddSizes(TotalBytes(files), 0)", + "TestARunWithRoomForItsFilesButNotItsRecordIsRefused", + ), + ( + "the forecast of a run's record leaves out the instructions written beside the manifest", + "internal/engine/recordsize.go", + "out.Instructions = int64(len(whole.Instructions(filepath.Base(ManifestPath(opt)))))", + "_ = whole", + "TestTheRecordARunIsToldBeforehandIsTheRecordItWrites", + ), ] diff --git a/internal/guard/recordspace_test.go b/internal/guard/recordspace_test.go new file mode 100644 index 00000000..ff50bd71 --- /dev/null +++ b/internal/guard/recordspace_test.go @@ -0,0 +1,60 @@ +package guard + +import ( + "context" + "errors" + "os" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/engine" +) + +// A disk with room for a run's files and not for its record is refused before +// the first byte. +// +// The free space was held against the files alone until 2026-10-26, so such a +// disk took every file and then refused the manifest - and a file no manifest +// lists is a file no command of this tool can remove, because the manifest is +// the only authority over that (untouchable rule 7, R08 in +// docs/STABILITY-REVIEW-2026-10-26.md). The free space is handed in, so the +// guard asks at the byte rather than filling a disk. +func TestARunWithRoomForItsFilesButNotItsRecordIsRefused(t *testing.T) { + plan := func(t *testing.T) ([]engine.PlannedFile, engine.Options) { + opt := engine.Options{OutDir: t.TempDir(), Seed: 7, Command: "test"} + planned, err := engine.Plan([]engine.Target{txtTarget("t", 3, 1000)}, opt) + if err != nil { + t.Fatalf("planning: %v", err) + } + return planned, opt + } + planned, opt := plan(t) + record, err := engine.PredictRecord(context.Background(), planned, opt) + if err != nil { + t.Fatalf("forecasting: %v", err) + } + files := engine.TotalBytes(planned) + if record.Manifest <= 0 { + t.Fatalf("the record was forecast at %d B, so there is nothing for the space to fall short of", record.Manifest) + } + + // One byte short of the files and their record. + opt.AvailableBytes = func(string) (int64, error) { return files + record.Bytes() - 1, nil } + _, err = engine.Run(context.Background(), planned, opt) + var noRoom *engine.SpaceError + if !errors.As(err, &noRoom) { + t.Fatalf("room for the files and all but one byte of the record gave %v, not the refusal about space", err) + } + if noRoom.Needed != files+record.Bytes() { + t.Errorf("the refusal says %d B are needed, and the files and their record are %d B", noRoom.Needed, files+record.Bytes()) + } + if entries, _ := os.ReadDir(opt.OutDir); len(entries) != 0 { + t.Errorf("a refused run left %d entries in the directory", len(entries)) + } + + // Exactly enough is enough - the other side of the same line. + planned, opt = plan(t) + opt.AvailableBytes = func(string) (int64, error) { return files + record.Bytes(), nil } + if _, err := engine.Run(context.Background(), planned, opt); err != nil { + t.Errorf("room for exactly the files and their record was refused: %v", err) + } +} diff --git a/internal/guard/windowmanifestreach_test.go b/internal/guard/windowmanifestreach_test.go index fc135109..cbcbd2eb 100644 --- a/internal/guard/windowmanifestreach_test.go +++ b/internal/guard/windowmanifestreach_test.go @@ -1,9 +1,11 @@ package guard import ( + "context" "strings" "testing" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" @@ -24,22 +26,38 @@ import ( // looks for, but something one surface SAYS and the other does not. // // The note was written down as a question about the manifest schema, on the -// grounds that notes are per file and this one is per run. It is not. The -// command line does not read this off the manifest either - it works it out -// from the plan and prints it before the first byte - and manifest.TooLarge- -// ToReadBack was put where it is exactly so the two surfaces could not come to -// different conclusions about one run. What was missing was a caller. - -// overTheCeiling is a file count whose manifest this build would refuse. +// grounds that notes are per file and this one is per run. It is not. Both +// surfaces take the answer from the engine - the record a run is forecast to +// write, and after it the record it wrote (engine.RecordSize) - so the two +// cannot come to different conclusions about one run. What was missing was a +// caller. + +// overTheCeiling is a file count whose manifest this build would refuse, for +// files of 200 B of text - the run the screens below are filled in for. // -// Worked out from the estimate rather than written here, for the reason the -// command line guard beside it gives: a guard carrying its own copy of a limit -// goes stale the day somebody changes the real one, and says nothing while it -// does. +// Worked out from the engine's own forecast rather than written here, for the +// reason the command line guard beside it gives: a guard carrying its own copy +// of a limit goes stale the day somebody changes the real one, and says +// nothing while it does. A thousand files past the count the forecast puts at +// the ceiling, and then asked again, so a forecast that moved cannot leave +// this under it. func overTheCeiling(t *testing.T) int { t.Helper() - over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 - if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { + forecast := func(n int) engine.RecordSize { + opt := engine.Options{OutDir: t.TempDir(), Seed: 1, Command: "test"} + planned, err := engine.Plan([]engine.Target{txtTarget("t", n, 200)}, opt) + if err != nil { + t.Fatalf("planning %d files: %v", n, err) + } + r, err := engine.PredictRecord(context.Background(), planned, opt) + if err != nil { + t.Fatalf("forecasting %d files: %v", n, err) + } + return r + } + each := forecast(2).Manifest - forecast(1).Manifest + over := int(manifest.MaxBytes/each) + 1000 + if !forecast(over).TooLarge { t.Fatalf("%d entries was not judged too large, so this guard would prove nothing", over) } return over @@ -189,32 +207,3 @@ func TestAnOrdinaryPreviewSaysNothingAboutTheManifestCeiling(t *testing.T) { t.Errorf("a hundred files drew the warning about the record being too big:\n%s", said) } } - -// Both surfaces judge the same run the same way. -// -// The window reads the answer off the document a dry run builds, and the -// command line works it out from the plan before anything is written. Two paths -// to one number, and what makes two paths acceptable is that they go through -// one predicate. Asked at the boundary, which is the only place a disagreement -// would show. -func TestBothSurfacesJudgeTheSameRunTheSameWay(t *testing.T) { - for _, entries := range []int{ - int(manifest.MaxBytes / manifest.BytesPerEntry), - int(manifest.MaxBytes/manifest.BytesPerEntry) + 1, - } { - _, fromThePlan := manifest.TooLargeToReadBack(entries, 0) - - m := manifest.New("testing-files-generator", "0.0.0-dev", "run_x", "tfg generate", 1, "linux", "amd64") - for i := 0; i < entries; i++ { - m.Add(manifest.File{Path: "f.txt", Materialized: true}) - } - _, fromTheDocument := m.ReadBackReach() - - if fromThePlan != fromTheDocument { - t.Errorf("at %d entries the plan says %v and the document says %v.\n"+ - "The command line answers from the first and the window from the second, so one run "+ - "would be warned about on one surface and not on the other", - entries, fromThePlan, fromTheDocument) - } - } -} diff --git a/internal/gui/text/locale/said/en.json b/internal/gui/text/locale/said/en.json index 5924a3df..7e36b6c8 100644 --- a/internal/gui/text/locale/said/en.json +++ b/internal/gui/text/locale/said/en.json @@ -1140,8 +1140,8 @@ }, "engine.NoSpace": { "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 {{.Needed}}, {{.Dir}}, {{.Free}}, written the way the program writes them - keep each spelled exactly that way.", - "hash": "sha256-0e6457cf7a3b", - "other": "this run needs {{.Needed}} B and {{.Dir}} has {{.Free}} B free - nothing was written. Ask for fewer files or a smaller size, or write to another disk by changing the output directory" + "hash": "sha256-064c75842c91", + "other": "this run needs {{.Needed}} B with its manifest and {{.Dir}} has {{.Free}} B free - nothing was written. Ask for fewer files or a smaller size, or write to another disk by changing the output directory" }, "engine.NotProduced": { "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.", diff --git a/internal/gui/text/locale/said/pl.json b/internal/gui/text/locale/said/pl.json index f9c3600a..42d710a3 100644 --- a/internal/gui/text/locale/said/pl.json +++ b/internal/gui/text/locale/said/pl.json @@ -226,7 +226,7 @@ "engine.NoIDWhy": { "hash": "sha256-363605924eb4", "other": "każda partia potrzebuje stałej nazwy, bo od niej zależy ziarno i to ona łączy partię z manifestem" }, "engine.NoOutDir": { "hash": "sha256-9ff19de0e58b", "other": "katalog wyjściowy jest pusty" }, "engine.NoOutDirFix": { "hash": "sha256-c14426ffc389", "other": "Podaj katalog, na przykład ./fixtures" }, - "engine.NoSpace": { "hash": "sha256-0e6457cf7a3b", "other": "ten przebieg potrzebuje {{.Needed}} B, a w {{.Dir}} jest wolne {{.Free}} B - nic nie zostało zapisane. Poproś o mniej plików albo mniejszy rozmiar albo zapisz na inny dysk, zmieniając katalog wyjściowy" }, + "engine.NoSpace": { "hash": "sha256-064c75842c91", "other": "ten przebieg potrzebuje {{.Needed}} B razem z manifestem, a w {{.Dir}} jest wolne {{.Free}} B - nic nie zostało zapisane. Poproś o mniej plików albo mniejszy rozmiar albo zapisz na inny dysk, zmieniając katalog wyjściowy" }, "engine.NotProduced": { "hash": "sha256-46fb82f6985b", "other": "Ten plik nie powstał. Przebieg poszedł dalej i skończył się kodem wyjścia oznaczającym częściowy wynik." }, "engine.OutDirIsAFile": { "hash": "sha256-55a340f0b67d", "other": "katalog wyjściowy {{.Dir}} jest plikiem, a nie katalogiem" }, "engine.OutDirIsAFileFix": { "hash": "sha256-f9e5f71f3822", "other": "Wskaż jako katalog wyjściowy katalog albo taki, którego jeszcze nie ma - zostanie utworzony" }, diff --git a/internal/gui/window/run.go b/internal/gui/window/run.go index c0266611..206bedfd 100644 --- a/internal/gui/window/run.go +++ b/internal/gui/window/run.go @@ -515,9 +515,9 @@ func roomOn(dir string) diskRoom { // previewFinished is the end of a preview, back on the interface thread. // // The result is carried across as well as the plan, and that is what lets a -// preview warn about a record too big to read back. A dry run builds the whole -// document - see manifestReachNote - so the answer is there for the asking -// rather than something the window would have to work out for itself. +// preview warn about a record too big to read back. A dry run works out the +// record the run would write - see manifestReachNote - so the answer is there +// for the asking rather than something the window works out for itself. func (r *runner) previewFinished(res *engine.Result, planned []engine.PlannedFile, opt engine.Options, room diskRoom, runErr error) { r.busy.set(false, busyFace{}) if runErr != nil { @@ -531,7 +531,7 @@ func (r *runner) previewFinished(res *engine.Result, planned []engine.PlannedFil r.line.measured(opt.OutDir, room.free) } said := r.line.said(exactly(planned), opt.OutDir) + text.AndNothingWrittenYet() - r.say(append([]string{said}, manifestReachNote(res)...)...) + r.say(append([]string{said}, manifestReachNote(res.Record)...)...) } // formatsOf is what kinds of file the run would produce, each named once. @@ -716,7 +716,7 @@ func (r *runner) runFinished(res *engine.Result, runErr, saveErr error, room dis // said only how many files - so the one thing this tool makes that others // do not was, from a window, something you found in the folder afterwards. outcome := text.SaidWithManifest(outcomeText(res, runErr), manifestNameOf(saved.Manifest)) - said := append([]string{outcome}, manifestReachNote(res)...) + said := append([]string{outcome}, manifestReachNote(saved.Size)...) // Instructions that were due and are not there are said, not failed: the // manifest was saved and holds the same facts. Silence would leave a button // missing with no reason given. diff --git a/internal/gui/window/runreport.go b/internal/gui/window/runreport.go index bafa24ea..15a73123 100644 --- a/internal/gui/window/runreport.go +++ b/internal/gui/window/runreport.go @@ -27,25 +27,20 @@ import ( // quality rather than in what the engine can do, which is the kind D1 is // easiest to lose. // -// Read off the document rather than worked out here, and off the SAME predicate -// the command line uses, which is what manifest.TooLargeToReadBack exists for. -// The two surfaces cannot come to different conclusions about one run. -// -// A preview reaches this too. engine.Run with DryRun adds an entry for every -// planned file, so the document a preview produces is the document the run -// would produce, minus the bytes on the disk. That is why one shape serves -// both, and why the window can answer before anything is written even though it -// cannot say a word in the middle of a run. -func manifestReachNote(res *engine.Result) []string { - if res == nil || res.Manifest == nil { - return nil - } - size, over := res.Manifest.ReadBackReach() - if !over { +// Read off the engine rather than worked out here, from the same count the +// command line uses, so the two surfaces cannot come to different conclusions +// about one run. A preview hands in the record worked out for the run it +// previews (engine.Result.Record), and a finished run the record it wrote +// (engine.Record.Size) - the fact, since 2026-10-26, rather than an estimate +// that was low for every format and let a run of 20 500 files end with no word +// about a manifest Verify then refused (docs/STABILITY-REVIEW-2026-10-26.md). +// A run whose record was not written hands in nothing, and nothing is said. +func manifestReachNote(record engine.RecordSize) []string { + if !record.TooLarge { return nil } return []string{text.ManifestTooLargeToRead( - text.HumanBytes(size), text.HumanBytes(manifest.MaxBytes))} + text.HumanBytes(record.Manifest), text.HumanBytes(manifest.MaxBytes))} } // progressText is the line under the bar. Bytes rather than files, because one diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index d8c98047..5b263137 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -35,15 +35,10 @@ type Manifest struct { Summary Summary `json:"summary"` Files []File `json:"files"` - // notedFiles is how many entries carry a note, which the size estimate - // needs and no reader does. - // - // Unexported, so it is not written and not read back. A manifest decoded - // from disk carries the ANSWER in Summary.TooLargeToReadBack rather than - // the working needed to reach it, which is what a reader wants and is also - // what keeps the two from disagreeing - a decoded manifest has no Add - // calls to count. - notedFiles int + // saved is how long this document was when Save wrote it, and nought + // before that or for a document read back from the disk. Unexported, so it + // is not written and not read back. + saved int64 } // Tool records what produced these bytes. Without it a hash mismatch after an @@ -188,6 +183,11 @@ type Summary struct { // written stays byte for byte what it was. An added field, which // docs/MANIFEST.md section 10 allows without moving manifest_version - // the same shape as tool.go, added for review item S1. + // + // Save sets it from the length of the document it writes, since + // 2026-10-26. It came from an estimate until then, and a manifest of + // 17 170 219 B went out without it (size.go). A preview's document carries + // it for the run it previews, from the same count. TooLargeToReadBack bool `json:"too_large_to_read_back,omitempty"` } @@ -365,6 +365,13 @@ func New(toolName, toolVersion, runID, command string, seed int64, os, arch stri // one place is what stops the two from drifting apart. func (m *Manifest) Add(f File) { m.Files = append(m.Files, f) + m.count(f) +} + +// count folds one file into the summary. Apart from Add so that a Projection +// counts a file exactly as Add does without keeping it - still one place that +// counts. +func (m *Manifest) count(f File) { m.Summary.FileCount++ m.Summary.ByFormat[f.Format]++ if f.TargetID != "" { @@ -380,14 +387,6 @@ func (m *Manifest) Add(f File) { if _, ok := m.Tool.Generators[f.Format]; !ok && f.Generator.Version != "" { m.Tool.Generators[f.Format] = f.Generator.Version } - // Kept in step here for the reason the comment above this function gives: - // counting anywhere else is a second place for the two to disagree. The - // estimate is arithmetic over the counts, so recomputing it per file costs - // nothing and is always right about the document as it stands. - if len(f.Notes) > 0 { - m.notedFiles++ - } - _, m.Summary.TooLargeToReadBack = TooLargeToReadBack(len(m.Files), m.notedFiles) } // noteExamples is how many file names a grouped note shows before it stops @@ -552,76 +551,16 @@ func (e *SchemaError) Said() core.Said { // an asymmetry rather than a decision. // // Sixteen megabytes rather than the recipe's one. A manifest is generated -// rather than typed and it grows with the run: measured on 2026-08-03, an entry -// costs about 700 B, so this holds a run of roughly twenty thousand files - -// twice the largest preset in docs/PRESETS.md and well above the ten thousand -// this tool is designed around. +// rather than typed and it grows with the run: measured on 2026-10-26, an entry +// costs from about 840 B (txt) to about 1580 B (eml), so this holds a run of +// roughly ten to twenty thousand files - above the largest preset in +// docs/PRESETS.md and around the ten thousand this tool is designed around. A +// run past it is told so, from the document's real length (size.go). // // What it does not do, stated as plainly as the recipe's: it does not defend // against a manifest that fits and is still expensive. const MaxBytes = 16 << 20 -// What an entry really costs, and what a run of a given size will come to. -// -// The comment above says "an entry costs about 700 B, so this holds a run of -// roughly twenty thousand files". Re-measured on 2026-08-26 against real runs: -// 786 B an entry for files carrying no notes, and 1009 B where every file -// carries one. So the ceiling is reached at about 21 300 files, or about 16 600 -// when every file has something to say about itself. -// -// The number matters because nothing compared it with the other ceiling. -// core.MaxFilesPerRun is a million. Measured the same day: a run of 25 000 -// files wrote a manifest of 25 220 640 B, and from that moment neither "tfg -// verify" nor "tfg cleanup" would read it - both exit 5 - so the files sat on -// the disk with nothing in this toolset able to remove them. Two ceilings were -// chosen independently and never held against each other. -// -// This is what lets a run say so before it starts rather than after. It is -// deliberately an estimate and named as one: the exact size depends on how long -// the names are and what the notes say, and a run that guessed slightly low -// would still be better off than one that said nothing. -const ( - // BytesPerEntry is a file with nothing to report. - BytesPerEntry = 786 - // BytesPerNotedEntry is the surcharge for a file that carries a note. - BytesPerNotedEntry = 223 -) - -// EstimatedBytes is roughly what the manifest of a run this shape will weigh. -func EstimatedBytes(entries, withNotes int) int64 { - return int64(entries)*BytesPerEntry + int64(withNotes)*BytesPerNotedEntry -} - -// TooLargeToReadBack says whether this build would refuse its own manifest for -// a run of this shape. -// -// Asked here rather than worked out by each surface, so the command line and -// the window cannot come to different conclusions about the same run. -func TooLargeToReadBack(entries, withNotes int) (int64, bool) { - n := EstimatedBytes(entries, withNotes) - return n, n > MaxBytes -} - -// ReadBackReach is what this document will weigh and whether this build would -// refuse to read it. -// -// The same question TooLargeToReadBack answers, asked of a manifest that -// exists rather than of a plan that has not run. It is here because the two -// callers had nowhere else to get the count of entries carrying a note from - -// that is kept while entries are added, so working it out again would mean -// walking every file a second time and getting it subtly wrong when a failed -// entry gains a note of its own. -// -// The window needs this and the command line does not. The command line says -// its piece BEFORE the first byte, from the plan, where no manifest exists yet. -// A window cannot say anything in the middle of a run - a widget touched from a -// worker is a race, and two of those were found on CI - so it says it when the -// run ends, off the document the run actually produced. Both go through -// TooLargeToReadBack, which is where that answer is settled for both surfaces. -func (m *Manifest) ReadBackReach() (int64, bool) { - return TooLargeToReadBack(len(m.Files), m.notedFiles) -} - // TooLargeError is returned for a manifest past MaxBytes. type TooLargeError struct { Path string diff --git a/internal/manifest/reservation.go b/internal/manifest/reservation.go index cc4369f7..c48cd13b 100644 --- a/internal/manifest/reservation.go +++ b/internal/manifest/reservation.go @@ -110,6 +110,14 @@ func (r *Reservation) Save(m *Manifest) error { return core.Defect(fmt.Errorf("the reservation of %s was already used or given back", core.Shown(r.final))) } r.used = true + // Whether this build will read the document back is settled from the + // length it is about to have, before the file is opened - so the field + // says what is true of the bytes below it (size.go). + size, err := m.settleReadBack() + if err != nil { + _ = core.RemoveOwn(r.tmp, r.own) + return err + } f, err := core.OpenOwn(r.tmp, r.own) if err != nil { // A reservation that could not be opened is still this run's, unless @@ -126,9 +134,14 @@ func (r *Reservation) Save(m *Manifest) error { _ = core.RemoveOwn(r.tmp, r.own) return err } + m.saved = size return nil } +// SavedBytes is how long this document was when Save wrote it, and nought for +// one not saved by this process. +func (m *Manifest) SavedBytes() int64 { return m.saved } + // Release gives the name back, for a run that reserved it and then had nothing // to record. Without it a refused run would leave its reservation behind and the // next run into that directory would be told a run is going. diff --git a/internal/manifest/size.go b/internal/manifest/size.go new file mode 100644 index 00000000..d23ef3b7 --- /dev/null +++ b/internal/manifest/size.go @@ -0,0 +1,183 @@ +package manifest + +import "encoding/json" + +// How long a manifest is, to the byte, and whether this build would read it +// back. +// +// Counted rather than estimated since 2026-10-26. Until then a constant stood +// here - 786 B an entry, measured on txt files on 2026-08-26 - and the warning +// that a manifest would be too large to read came from it. Measured again on +// 2026-10-26 (docs/STABILITY-REVIEW-2026-10-26.md, section 6.0): an entry +// costs 838 B for txt, 893 for csv and 1577 for eml, so the constant was low +// for every format, and a run of 20 500 text files wrote a manifest of +// 17 170 219 B with no warning and no too_large_to_read_back in it - which +// verify and cleanup then refused. No single number is right for every format, +// names and notes, so the document is measured instead. +// +// Measured an entry at a time rather than by encoding the whole document, so +// knowing the size costs one entry's worth of memory. A dry run of a million +// files already peaks near three gigabytes (measured the same day), and a +// second copy of the document to count it would be a large share of that. +// +// The arithmetic below is Encode's layout written out: the document with an +// empty "files" list, and then the entries in place of the brackets. A guard +// holds it to the length of Encode's output, byte for byte, for documents of +// none, one and many entries (TestTheManifestSizeIsTheLengthOfTheManifest). + +// entryIndent is what Encode puts in front of every line of an entry: the +// entry is two levels into the document. +const entryIndent = " " + +// entryBytes is how long one entry is inside the document, its indentation +// included. +func entryBytes(f File) (int64, error) { + b, err := json.MarshalIndent(f, entryIndent, " ") + if err != nil { + return 0, err + } + // MarshalIndent puts the prefix in front of every line but the first, and + // in the document the first line is indented like the rest. + return int64(len(entryIndent) + len(b)), nil +} + +// documentBytes joins an empty document and its entries. With none, the +// document is what it is - "files": [] - and with some, the two brackets +// open onto a line each, the entries are separated by a comma and a line +// break, and the list closes on a line of its own one level in. +func documentBytes(empty, entries int64, n int) int64 { + if n == 0 { + return empty + } + const open, between, closing int64 = int64(len("[\n") - len("[]")), int64(len(",\n")), int64(len("\n ]")) + return empty + open + entries + between*int64(n-1) + closing +} + +// emptyBytes is how long this document is with no entries in it. +func (m *Manifest) emptyBytes() (int64, error) { + shell := *m + if len(shell.Files) > 0 { + shell.Files = []File{} + } + var n counted + if err := shell.Encode(&n); err != nil { + return 0, err + } + return int64(n), nil +} + +// counted is a writer that keeps the count and nothing else. +type counted int64 + +func (c *counted) Write(p []byte) (int, error) { + *c += counted(len(p)) + return len(p), nil +} + +// settleReadBack sets Summary.TooLargeToReadBack from the length of the +// document as it is about to be written, and says how long that is. +// +// The field is counted in its own document, so whether it is there changes +// the length it describes. Settled the one way that cannot be wrong: present +// exactly when the document WITHOUT it is past the ceiling. Then a document +// that carries it is longer still and is refused, and one that does not +// carry it is within the ceiling and is read. +func (m *Manifest) settleReadBack() (int64, error) { + entries, err := m.entriesBytes() + if err != nil { + return 0, err + } + size, _, err := settle(m, entries, len(m.Files)) + return size, err +} + +// entriesBytes is how long the entries of this document are, together. +func (m *Manifest) entriesBytes() (int64, error) { + var entries int64 + for _, f := range m.Files { + b, err := entryBytes(f) + if err != nil { + return 0, err + } + entries += b + } + return entries, nil +} + +// settle is the rule above, for a header and the entries counted for it. +func settle(header *Manifest, entries int64, n int) (int64, bool, error) { + header.Summary.TooLargeToReadBack = false + empty, err := header.emptyBytes() + if err != nil { + return 0, false, err + } + if size := documentBytes(empty, entries, n); size <= MaxBytes { + return size, false, nil + } + header.Summary.TooLargeToReadBack = true + if empty, err = header.emptyBytes(); err != nil { + return 0, false, err + } + return documentBytes(empty, entries, n), true, nil +} + +// EncodedBytes is exactly how long Encode makes this document as it stands, +// and changes nothing in it. Save settles Summary.TooLargeToReadBack first, so +// asked of a document that is about to be saved, this can be short by the +// field. +func (m *Manifest) EncodedBytes() (int64, error) { + entries, err := m.entriesBytes() + if err != nil { + return 0, err + } + empty, err := m.emptyBytes() + if err != nil { + return 0, err + } + return documentBytes(empty, entries, len(m.Files)), nil +} + +// Projection is the length of a document a run has not written yet, counted +// as its entries are offered and none of them kept. +// +// A run says before its first byte whether its manifest will be readable, and +// a preview says what the run would write. Both have the plan and no document, +// and building the document a million entries long to measure it would hold +// it all at once. This counts each entry the moment it is offered and keeps +// only the summary, the same summary Add keeps. +type Projection struct { + shell *Manifest + entries int64 + n int + err error +} + +// Projection measures the document m would become with entries added to it. +// m is taken over as the header - its run, its tool - and must hold no files. +func (m *Manifest) Projection() *Projection { + return &Projection{shell: m} +} + +// Add counts one entry the document would hold. +func (p *Projection) Add(f File) { + if p.err != nil { + return + } + b, err := entryBytes(f) + if err != nil { + p.err = err + return + } + p.entries += b + p.n++ + p.shell.count(f) +} + +// Bytes is how long the document would be, and whether this build would refuse +// to read it back. +func (p *Projection) Bytes() (int64, bool, error) { + if p.err != nil { + return 0, false, p.err + } + return settle(p.shell, p.entries, p.n) +} From 877632a104516328043ab9ef6df523d5e7aa2746 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 26 Oct 2026 19:58:04 +0100 Subject: [PATCH 4/7] ci: raise certifi in the scanner's requirements past the emSign roots The weekly OSV scan went red on 2026-10-26 on PYSEC-2026-4320 (GHSA-6q48-6c7w-9gw2): certifi before 2026.10.19 trusts the emSign ECC Root CA - G3, whose intermediates' keys were compromised. semgrep sets no bound on certifi, so it is raised to 2026.10.23, compiled with pip-compile --upgrade-package certifi as requirements-semgrep.in describes. One line of the compiled list moved. The other finding of that scan, GHSA-xcq7-p3pf-p88h in opentelemetry-api, cannot be raised this way: semgrep 1.180.0 and 1.181.0 both ask for opentelemetry-api~=1.37.0, and the fix is in 1.45.1. Co-Authored-By: Claude Opus 5.5 --- .github/requirements-semgrep.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/requirements-semgrep.txt b/.github/requirements-semgrep.txt index 9bdf43ae..2caef892 100644 --- a/.github/requirements-semgrep.txt +++ b/.github/requirements-semgrep.txt @@ -25,7 +25,7 @@ boltons==21.0.0 # semgrep bracex==3.1 # via wcmatch -certifi==2026.7.22 +certifi==2026.10.23 # via # httpcore # httpx From a8109e47f9c09ccfd96f8391fc0ab98751009c12 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 26 Oct 2026 19:59:39 +0100 Subject: [PATCH 5/7] ci: let the OSV scan pass GHSA-xcq7 in the scanner's opentelemetry-api semgrep 1.180.0 and 1.181.0 both ask for opentelemetry-api~=1.37.0, read from their wheels, so no installable set holds the fixed 1.45.1. The advisory is about quadratic parsing of an inbound tracestate header in the instrumentation of a server, and semgrep scan receives no request. Until 2026-11-15, the same date as the mcp entry, so both come back on one reminder. The owner's decision of 2026-10-26. Co-Authored-By: Claude Opus 5.5 --- .github/osv-scanner.toml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.github/osv-scanner.toml b/.github/osv-scanner.toml index 293960de..a3384e70 100644 --- a/.github/osv-scanner.toml +++ b/.github/osv-scanner.toml @@ -25,3 +25,22 @@ id = "GHSA-84m7-p3x7-pcfv" ignoreUntil = 2026-11-15 reason = "semgrep pins mcp==1.29.0 in every release, and the advisory is about an MCP HTTP server, which semgrep scan never starts" + +# opentelemetry-api 1.37.0, in requirements-semgrep.txt for the scanner and +# nothing else. +# +# semgrep 1.180.0 and 1.181.0 both ask for opentelemetry-api~=1.37.0, read +# from their wheels on 2026-10-26, so no set pip can install holds the fixed +# 1.45.1. certifi, found by the same scan, had no such bound and was raised. +# +# The advisory is about TraceState.from_header taking quadratic time on an +# inbound tracestate header - reached through propagate.extract() in the +# instrumentation of a server receiving requests. semgrep scan sends its +# telemetry out and receives no request, on a runner that lives for one scan. +# +# The same date as the entry above, on purpose: both wait on a semgrep that +# moves its pins, and both come back on one reminder. +[[IgnoredVulns]] +id = "GHSA-xcq7-p3pf-p88h" +ignoreUntil = 2026-11-15 +reason = "semgrep pins opentelemetry-api~=1.37.0 in every release, and the advisory is about parsing an inbound tracestate header in a server, which semgrep scan never runs" From 6120236ffe492326957a38240d7ced548b5aec5e Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 26 Oct 2026 20:16:52 +0100 Subject: [PATCH 6/7] build: Go 1.27.3 govulncheck reported GO-2026-6809 in crypto/x509 of Go 1.27.2, fixed in 1.27.3, and the known vulnerabilities check went red on it on this pull request. Only the window binary links crypto/x509 (go list -deps ./cmd/tfg holds none of it). go.mod, GO_VERSION in the four workflows, README and CONTRIBUTING move together, as they did for 1.27.2. Measured under 1.27.3 before the move: TestOurOwnGeneratorsHaveNotDrifted, TestStandardLibraryOutputHasNotDrifted and TestEjectingAPresetGivesTheBytesItAlwaysGave green, so the generated bytes did not shift and D11 holds. govulncheck v1.9.0, the version ci.yml runs, finds no vulnerabilities under 1.27.3. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 2 +- .github/workflows/dev-build.yml | 2 +- .github/workflows/mutation.yml | 2 +- .github/workflows/release.yml | 2 +- CHANGELOG.md | 6 ++++++ CONTRIBUTING.md | 2 +- README.md | 2 +- go.mod | 8 +++++++- internal/guard/mutation/entries.py | 6 +++--- 9 files changed, 22 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 129f0115..11e6cdf7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ env: # The exact toolchain used for tests and releases. go.mod declares a # minimum - this is the pin. Raising it can change generated bytes, so the # byte stability guard has to be green before it moves. - GO_VERSION: "1.27.2" + GO_VERSION: "1.27.3" jobs: test: diff --git a/.github/workflows/dev-build.yml b/.github/workflows/dev-build.yml index 25b53c15..2d3a9539 100644 --- a/.github/workflows/dev-build.yml +++ b/.github/workflows/dev-build.yml @@ -51,7 +51,7 @@ concurrency: cancel-in-progress: true env: - GO_VERSION: "1.27.2" + GO_VERSION: "1.27.3" # Fourteen days rather than the default ninety. These are throwaway builds # handed to one person, and an unsigned binary should not sit for a quarter of # a year behind a link somebody can pass on as if it were official. diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index efe4759d..3851a5a3 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -42,7 +42,7 @@ concurrency: cancel-in-progress: true env: - GO_VERSION: "1.27.2" + GO_VERSION: "1.27.3" # Built as the test job builds on Linux, so a verdict here is about the # guards CI actually runs. Entries on files only a cgo build or only # Windows compiles come back UNJUDGED, said out loud in the report. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c4b14d09..a6349ea9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -29,7 +29,7 @@ concurrency: env: # The same pin CI carries. A release built on a different toolchain than the # one the byte stability guards ran under is a release nobody measured. - GO_VERSION: "1.27.2" + GO_VERSION: "1.27.3" jobs: check: diff --git a/CHANGELOG.md b/CHANGELOG.md index b40449ef..055dfea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,12 @@ because it turns other people's test suites red. refused the manifest, which left files `tfg cleanup` cannot remove. The refusal now says how much the run needs with its manifest. +### Security + +- **Built with Go 1.27.3.** It fixes a vulnerability in `crypto/x509` of the + Go standard library, which only the desktop window links. The command line + tool was not affected. Building from source now needs Go 1.27.3. + ## [0.5.0] - 2026-10-20 ### Breaking diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 042121e9..e77176b6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,7 +22,7 @@ reached yet. ## Building it -Needs Go 1.27.2 or newer, and nothing else for the command line tool. +Needs Go 1.27.3 or newer, and nothing else for the command line tool. ``` git clone https://github.com/donislawdev/TestingFilesGenerator diff --git a/README.md b/README.md index 88689af3..cff2f7ab 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ instead of writing files that disappear with the container. go install -tags noasm github.com/donislawdev/TestingFilesGenerator/cmd/tfg@latest ``` -**From source.** Needs Go 1.27.2 or newer, and nothing else: +**From source.** Needs Go 1.27.3 or newer, and nothing else: ``` git clone https://github.com/donislawdev/TestingFilesGenerator diff --git a/go.mod b/go.mod index 9eea3188..77e84f92 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ module github.com/donislawdev/TestingFilesGenerator // which version it is. Kept honest by TestTheBuildFloorIsThePinnedToolchainAndTheReadmeSaysSo, // which also holds the sentence in README.md against this number. See // docs/STACK.md. -go 1.27.2 +go 1.27.3 // How the line above got to the number it carries. There used to be a separate // "toolchain" directive here saying which compiler produces our binaries, and @@ -75,6 +75,12 @@ go 1.27.2 // guards were run under 1.27.2 the same day - all 104 golden values and every // pinned standard library path - and none shifted, so D11 holds and no major // version is owed. +// +// Raised to 1.27.3 on 2026-10-26 because govulncheck reported GO-2026-6809 in +// crypto/x509, fixed in 1.27.3, and the known vulnerabilities check of every +// pull request went red on it. Only the window binary links crypto/x509 (go +// list -deps, the same day). The byte stability guards were run under 1.27.3 +// before the move, as they were for 1.27.2. require ( fyne.io/fyne/v2 v2.8.2 diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index fa19d8bc..ae3201f9 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -30,7 +30,7 @@ ( "the contributing guide names a Go version this module does not admit", "CONTRIBUTING.md", - "Needs Go 1.27.2 or newer", + "Needs Go 1.27.3 or newer", "Needs Go 1.21 or newer", "TestTheBuildFloorIsThePinnedToolchainAndTheDocumentsSaySo", ), @@ -334,14 +334,14 @@ ( "the README goes back to naming a floor the module no longer admits", "README.md", - "**From source.** Needs Go 1.27.2 or newer, and nothing else:", + "**From source.** Needs Go 1.27.3 or newer, and nothing else:", "**From source.** Needs Go 1.26.5 or newer, and nothing else:", "TestTheBuildFloorIsThePinnedToolchainAndTheDocumentsSaySo", ), ( "CI builds with a compiler older than the floor go.mod admits", ".github/workflows/ci.yml", - 'GO_VERSION: "1.27.2"', + 'GO_VERSION: "1.27.3"', 'GO_VERSION: "1.26.8"', "TestTheBuildFloorIsThePinnedToolchainAndTheDocumentsSaySo", ), From 4ee34e0779c5dcbc4c482c2583d518c76799a84a Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 26 Oct 2026 20:42:04 +0100 Subject: [PATCH 7/7] engine: flatten PredictRecord below the nesting crowd The record forecast nested four deep (a Ctrl+C check inside a loop inside the branch that measures the instructions), which took the count of functions nesting three deep or more past its ceiling. The instructions measurement and the every-1024-entries check are now their own functions. Same checks, same lengths. Co-Authored-By: Claude Opus 5.5 --- internal/engine/recordsize.go | 41 ++++++++++++++++++++---------- internal/guard/mutation/entries.py | 4 +-- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/internal/engine/recordsize.go b/internal/engine/recordsize.go index cb3323a8..8036704a 100644 --- a/internal/engine/recordsize.go +++ b/internal/engine/recordsize.go @@ -60,34 +60,47 @@ func PredictRecord(ctx context.Context, files []PlannedFile, opt Options) (Recor counted := header.Projection() explains := false for i, f := range files { - if i%askEvery == 0 { - if err := ctx.Err(); err != nil { - return RecordSize{}, err - } + if err := askedToStop(ctx, i); err != nil { + return RecordSize{}, err } e := scratch.entryFor(f, hashPlaceholder, true, nil) explains = explains || e.Purpose != "" counted.Add(e) } var out RecordSize + var err error if explains { header.Run.Instructions = filepath.Base(InstructionsPath(opt)) - whole := newManifest(opt) - for i, f := range files { - if i%askEvery == 0 { - if err := ctx.Err(); err != nil { - return RecordSize{}, err - } - } - whole.Add(scratch.entryFor(f, hashPlaceholder, true, nil)) + if out.Instructions, err = predictInstructions(ctx, files, opt, scratch); err != nil { + return RecordSize{}, err } - out.Instructions = int64(len(whole.Instructions(filepath.Base(ManifestPath(opt))))) } - var err error out.Manifest, out.TooLarge, err = counted.Bytes() return out, err } +// predictInstructions is the length of the instructions a run of these files +// writes. They are written from the whole document, so it is built here. +func predictInstructions(ctx context.Context, files []PlannedFile, opt Options, scratch *Result) (int64, error) { + whole := newManifest(opt) + for i, f := range files { + if err := askedToStop(ctx, i); err != nil { + return 0, err + } + whole.Add(scratch.entryFor(f, hashPlaceholder, true, nil)) + } + return int64(len(whole.Instructions(filepath.Base(ManifestPath(opt))))), nil +} + +// askedToStop is Ctrl+C, asked before the first entry and then once every +// askEvery entries. +func askedToStop(ctx context.Context, i int) error { + if i%askEvery != 0 { + return nil + } + return ctx.Err() +} + // forecast is the record the caller handed in, or the one worked out here // when it handed in none. func (o Options) forecast(ctx context.Context, files []PlannedFile) (RecordSize, error) { diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index ae3201f9..7878888f 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -15636,8 +15636,8 @@ ( "the forecast of a run's record leaves out the instructions written beside the manifest", "internal/engine/recordsize.go", - "out.Instructions = int64(len(whole.Instructions(filepath.Base(ManifestPath(opt)))))", - "_ = whole", + "return int64(len(whole.Instructions(filepath.Base(ManifestPath(opt))))), nil", + "return 0, nil", "TestTheRecordARunIsToldBeforehandIsTheRecordItWrites", ), ]