From 46dc251957fea2627e27b853fb37ad066a802edc Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 4 Nov 2026 01:51:55 +0100 Subject: [PATCH 1/3] verify and cleanup read the manifest a piece at a time A manifest is read one value at a time under a ceiling of 16 MiB on one value instead of on the whole document, so the manifest of a run of any size can be verified and cleaned up. What verify and cleanup hold of it is checked against the new --memory setting on both commands. A key named twice and anything after the document are refused, and Ctrl+C while the manifest is read stops the reading. Co-Authored-By: Claude Opus 5.5 --- internal/audit/audit.go | 46 +- internal/audit/cleanup.go | 10 +- internal/audit/neighbours.go | 55 ++- internal/cli/claims.go | 38 ++ internal/cli/cleanup.go | 16 +- internal/cli/errors.go | 12 + internal/cli/memoryflag.go | 15 +- internal/cli/verify.go | 14 +- internal/engine/engine.go | 2 +- internal/engine/processmemory.go | 8 +- internal/guard/anotherrun_test.go | 8 +- internal/guard/auditorder_test.go | 8 +- internal/guard/auditpipe_unix_test.go | 4 +- internal/guard/cleanuprecheck_test.go | 4 +- internal/guard/concurrentruns_test.go | 39 +- internal/guard/contextroots_test.go | 1 + internal/guard/manifestread_test.go | 449 +++++++++++++++++++ internal/guard/manifestreadlimits_test.go | 194 ++++++++ internal/guard/runlock_test.go | 2 +- internal/manifest/manifest.go | 100 +++-- internal/manifest/read.go | 514 ++++++++++++++++++++++ 21 files changed, 1401 insertions(+), 138 deletions(-) create mode 100644 internal/cli/claims.go create mode 100644 internal/guard/manifestread_test.go create mode 100644 internal/guard/manifestreadlimits_test.go create mode 100644 internal/manifest/read.go diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 03c002d6..f452eb89 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -185,7 +185,7 @@ func (d Difference) String() string { // EscapeError is a manifest entry that leaves the directory once the links on // the way have been followed. // -// Separate from the check manifest.Load already makes, and it has to be. That +// Separate from the check the manifest reader already makes, and it has to be. That // one reads the text of the path and this one asks the filesystem, so a path // with no climb in it - "jn/VICTIM.txt" - passes the first and is caught here // when "jn" turns out to point somewhere else. @@ -215,35 +215,15 @@ func (e *EscapeError) Error() string { // resolving it per entry impossible to write by accident: there is no directory // string in scope to resolve. See core.Boundary for what that is worth in // seconds, and observation O117 for the measurement. -func resolved(b core.Boundary, f manifest.File) (string, error) { - full := filepath.Join(b.Dir(), filepath.FromSlash(f.Path)) +func resolved(b core.Boundary, p string) (string, error) { + full := filepath.Join(b.Dir(), filepath.FromSlash(p)) if b.Escapes(full) { - return "", &EscapeError{Dir: b.Dir(), Path: f.Path} + return "", &EscapeError{Dir: b.Dir(), Path: p} } return full, nil } -// Claimed lists the files a manifest says are on the disk. -// -// Two kinds of entry are deliberately not claimed. An entry marked failed -// describes a file that was never written - reporting it missing would be a -// false alarm about the one thing the manifest was honest about. And an entry -// that was not materialised was never meant to reach the disk at all. -// -// This is the single reading of the manifest. Both commands use it, so neither -// can drift into claiming something the other does not. -func Claimed(m *manifest.Manifest) []manifest.File { - var out []manifest.File - for _, f := range m.Files { - if f.Failed || !f.Materialized { - continue - } - out = append(out, f) - } - return out -} - -// Verify compares a directory against a manifest. +// Verify compares a directory against what a manifest claims. // // skip is the name of the manifest file itself. It is never reported as extra: // the manifest usually sits in the directory it describes, and a tool that @@ -255,8 +235,8 @@ func Claimed(m *manifest.Manifest) []manifest.File { // A run that is cancelled reports what it managed to compare and says so // through the context error. Reporting "sound" on the strength of half a // directory is the one answer this must never give. -func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) ([]Difference, error) { - claimed := Claimed(m) +func Verify(ctx context.Context, dir string, c manifest.Claims, skip string) ([]Difference, error) { + claimed := c.Files boundary := core.NewBoundary(dir) present, err := walk(ctx, dir) @@ -315,7 +295,7 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) // decided. walk builds these with filepath.Rel, which returns a clean // path, so a comparablePath here is a call that cannot be wrong - // removing it left this guard green. See the comment on comparablePath. - if seen[p] || filepath.Base(p) == skip || (m.Run.Instructions != "" && filepath.Base(p) == m.Run.Instructions) { + if seen[p] || filepath.Base(p) == skip || (c.Instructions != "" && filepath.Base(p) == c.Instructions) { continue } unclaimed = append(unclaimed, p) @@ -430,14 +410,14 @@ func comparablePath(p string) string { // Shared by Verify and Inspect because both have to refuse the same manifest // for the same reason, and because both then hand the answers to workers that // must not be able to refuse anything themselves. See parallel.go. -func claimedPaths(b core.Boundary, files []manifest.File) ([]string, error) { +func claimedPaths(b core.Boundary, files []manifest.ClaimedFile) ([]string, error) { full := make([]string, len(files)) for i, f := range files { // The raw path, never the compared one. Cleaning resolves a parent step // against the text of the path rather than against the disk, and the // two differ exactly where a link sits in the middle - which is the // case core.Boundary exists for. - p, err := resolved(b, f) + p, err := resolved(b, f.Path) if err != nil { return nil, err } @@ -453,7 +433,7 @@ 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(ctx context.Context, f manifest.File, full string, scratch []byte) Difference { +func compare(ctx context.Context, f manifest.ClaimedFile, 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 @@ -482,10 +462,10 @@ func compare(ctx context.Context, f manifest.File, full string, scratch []byte) Got: fmt.Sprintf("%d B", size), } } - if got.sum != f.Hashes.SHA256 { + if got.sum != f.SHA256 { return Difference{ Kind: WrongHash, Path: f.Path, - Want: f.Hashes.SHA256, + Want: f.SHA256, Got: got.sum, } } diff --git a/internal/audit/cleanup.go b/internal/audit/cleanup.go index 432d09d4..56f46f8a 100644 --- a/internal/audit/cleanup.go +++ b/internal/audit/cleanup.go @@ -66,9 +66,9 @@ func (c Candidate) Removable(force bool) bool { // It is a separate pass because the default run of cleanup deletes nothing and // prints this list. A person gets to read what is about to disappear before it // does, and CLI.md section 9 rules out asking them interactively. -func Inspect(ctx context.Context, dir string, m *manifest.Manifest) ([]Candidate, error) { +func Inspect(ctx context.Context, dir string, c manifest.Claims) ([]Candidate, error) { boundary := core.NewBoundary(dir) - claimed := Claimed(m) + claimed := c.Files // Nothing is inspected and nothing is offered when one of these points // outside the directory. A list that points outside is not a list this tool @@ -92,7 +92,7 @@ func Inspect(ctx context.Context, dir string, m *manifest.Manifest) ([]Candidate // 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(ctx context.Context, f manifest.File, full string, scratch []byte) Candidate { +func look(ctx context.Context, f manifest.ClaimedFile, full string, scratch []byte) Candidate { got, err := readListed(ctx, full, f.Bytes, scratch) if absent(err) { return Candidate{Path: f.Path, Disposition: Absent} @@ -115,7 +115,7 @@ func look(ctx context.Context, f manifest.File, full string, scratch []byte) Can return Candidate{Path: f.Path, Disposition: Changed, Detail: "it changed while it was being checked"} } - if got.sum != f.Hashes.SHA256 { + if got.sum != f.SHA256 { return Candidate{Path: f.Path, Disposition: Changed, Detail: "its content is not the content this run wrote"} } @@ -214,7 +214,7 @@ func Remove(ctx context.Context, dir string, cands []Candidate, force bool) ([]O // separated by however long a person spends reading the preview, and // this is the one operation in the tool that destroys data - so the // question is put to the filesystem in the state it is in now. - full, err := resolved(boundary, manifest.File{Path: c.Path}) + full, err := resolved(boundary, c.Path) if err != nil { return out, err } diff --git a/internal/audit/neighbours.go b/internal/audit/neighbours.go index 550cc77d..d0b189f4 100644 --- a/internal/audit/neighbours.go +++ b/internal/audit/neighbours.go @@ -54,11 +54,22 @@ type neighbourClaims struct { // about it here and it keeps whatever verify would have called it. Guessing on // a file we could not read is how a tool starts accounting for files nobody // wrote. +// +// A record is read a piece at a time and only its claims among the candidates +// are kept, so the record of a run of a million files costs a pass over it and +// what it says about the files in front of us. It used to be read whole under +// the sixteen megabytes Load allows, and a record past that was refused - so +// the files of a run of more than about twenty thousand were called extra in +// the directory they shared with ours. func findNeighbours(ctx context.Context, dir string, candidates []string) neighbourClaims { found := neighbourClaims{ records: map[string]bool{}, claimedBy: map[string]string{}, } + unclaimed := make(map[string]bool, len(candidates)) + for _, rel := range candidates { + unclaimed[rel] = true + } for _, rel := range candidates { if ctx.Err() != nil { // A cancelled pass stops looking. The caller reports what it @@ -68,34 +79,45 @@ func findNeighbours(ctx context.Context, dir string, candidates []string) neighb if !couldBeNamedLikeARecord(rel) { continue } - full := filepath.Join(dir, filepath.FromSlash(rel)) - m, err := manifest.Load(full) - if err != nil { + claims, ok := claimsAmong(ctx, filepath.Join(dir, filepath.FromSlash(rel)), unclaimed) + if !ok { continue } found.records[rel] = true - base := filepath.Base(rel) - found.take(m, base) + found.take(claims, filepath.Base(rel)) } return found } +// claimsAmong is the files the record at full claims that are among the +// unclaimed, and whether it is a record this build reads at all. What it claims +// is kept only once the whole record has been read and found sound, because a +// record refused at its last byte is no record, whatever its first entries +// said. +func claimsAmong(ctx context.Context, full string, unclaimed map[string]bool) ([]string, bool) { + var hits []string + _, err := manifest.EachClaim(ctx, full, func(c manifest.ClaimedFile) { + // The path is put through the same spelling both sides of the + // comparison use. Without it a neighbour writing "./a.txt" would claim + // nothing, which is the fault comparablePath was written for. + if key := comparablePath(c.Path); unclaimed[key] { + hits = append(hits, key) + } + }) + return hits, err == nil +} + // take records what one neighbour says it wrote. // // Its own function rather than a loop inside a loop, which is the ceiling on // how deeply this project nests talking. It is also the better shape to read: // the caller decides WHICH files are records, and this decides what a record // accounts for. -func (n neighbourClaims) take(m *manifest.Manifest, base string) { - for _, f := range Claimed(m) { +func (n neighbourClaims) take(claims []string, base string) { + for _, key := range claims { // First record wins, and the walk that produced the candidates is in a // fixed order, so two neighbours claiming one file name the same one of // themselves every time rather than whichever was read first on the day. - // - // The path is put through the same spelling both sides of the - // comparison use. Without it a neighbour writing "./a.txt" would claim - // nothing, which is the fault comparablePath was written for. - key := comparablePath(f.Path) if _, taken := n.claimedBy[key]; !taken { n.claimedBy[key] = base } @@ -128,13 +150,14 @@ func (n neighbourClaims) take(m *manifest.Manifest, base string) { // same day. That one read the first half kilobyte of every candidate and looked // for the first key of a manifest, to keep a large JSON document that is not one // from being read in full. Nothing could make it fail: a document that got past -// it was refused by manifest.Load's schema check anyway, so removing it changed +// it was refused by the manifest's schema check anyway, so removing it changed // no answer and no test could tell. A defence nothing can redden is not a // defence, and this project takes those out rather than keeping them. // -// What is left is the name, and then manifest.Load, which has a ceiling of its -// own - so the worst an unclaimed JSON document can cost is one read of at most -// that ceiling. +// What is left is the name, and then the manifest reader, which holds at most +// one value of manifest.MaxValueBytes at a time - so the worst an unclaimed +// JSON document can cost is one pass over it, and one that is not an object +// is let go at its first byte. func couldBeNamedLikeARecord(rel string) bool { return strings.EqualFold(filepath.Ext(rel), ".json") } diff --git a/internal/cli/claims.go b/internal/cli/claims.go new file mode 100644 index 00000000..70212b77 --- /dev/null +++ b/internal/cli/claims.go @@ -0,0 +1,38 @@ +// Part of package cli. See cli.go. +package cli + +import ( + "context" + "errors" + "fmt" + "io" + + "github.com/donislawdev/TestingFilesGenerator/internal/core" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// claimsOf reads what the manifest at path claims, for verify and cleanup, +// under the memory setting, and says on errOut why not when it cannot. When ok +// is false the command ends here with code. +// +// One function for both commands, so they read a manifest under the same +// limit and refuse it in the same words. stopped is the sentence for Ctrl+C +// arriving while the manifest is read, which is the one ending the two put +// differently - one checked nothing, the other removed nothing - and both say +// so, because a manifest of a million files takes seconds to read and a person +// who stopped it wants to know what happened to their directory. +func claimsOf(ctx context.Context, path string, memory core.Memory, stopped string, errOut io.Writer) (claims manifest.Claims, code int, ok bool) { + limits := memory.On(core.MachineMemory) + engine.HandToRuntime(limits) + claims, err := manifest.ReadClaims(ctx, path, limits.Run) + if err == nil { + return claims, ExitOK, true + } + if errors.Is(err, context.Canceled) { + fmt.Fprintf(errOut, "tfg: %s\n", stopped) + return manifest.Claims{}, ExitInterrupted, false + } + fmt.Fprintf(errOut, "tfg: %s\n", describeError(err)) + return manifest.Claims{}, classify(err), false +} diff --git a/internal/cli/cleanup.go b/internal/cli/cleanup.go index 67d9747a..3c3c40f5 100644 --- a/internal/cli/cleanup.go +++ b/internal/cli/cleanup.go @@ -23,6 +23,8 @@ func cleanup(ctx context.Context, args []string, out, errOut io.Writer) int { withManifest := fs.Bool("with-manifest", false, "remove the manifest and the instructions beside it as well, once every file it lists is gone") asJSON := fs.Bool("json", false, "write the report as JSON instead of prose") against := fs.String("against", "", "directory to clean. Defaults to the directory holding the manifest") + var memory memoryValue + checkingMemoryFlag(fs, &memory) usage := func(w io.Writer) { fmt.Fprint(w, `tfg cleanup - remove the files a manifest lists. @@ -58,10 +60,9 @@ Flags: return ExitUsage } - m, err := manifest.Load(path) - if err != nil { - fmt.Fprintf(errOut, "tfg: %s\n", describeError(err)) - return classify(err) + claims, code, ok := claimsOf(ctx, path, memory.Memory, "cleanup was interrupted while reading the manifest and removed nothing.", errOut) + if !ok { + return code } dir := *against @@ -69,7 +70,7 @@ Flags: dir = filepath.Dir(path) } - cands, inspectErr := audit.Inspect(ctx, dir, m) + cands, inspectErr := audit.Inspect(ctx, dir, claims) if inspectErr != nil { // Not every refusal from the looking pass is a cancellation. It used to // be treated as one, so a manifest pointing outside the directory was @@ -97,7 +98,7 @@ Flags: // lists until 2026-09-25, and the real run then took the manifest and the // instructions beside it too. if *withManifest { - run.record, run.notOurs = recordOf(path, m) + run.record, run.notOurs = recordOf(path, claims.Instructions) } // The default run deletes nothing. A tool that removes files on the // strength of one argument is the wrong shape when the directory may hold @@ -139,9 +140,8 @@ const notOursReason = "it is not named after this manifest, so it may belong to // is the cheaper mistake: a file left to remove by hand, rather than a file // removed that was somebody else's. Refusing such a manifest outright would // have stopped verify and cleanup on every renamed one (owner's choice). -func recordOf(path string, m *manifest.Manifest) (record []string, notOurs string) { +func recordOf(path, name string) (record []string, notOurs string) { record = []string{path} - name := m.Run.Instructions if name == "" { return record, "" } diff --git a/internal/cli/errors.go b/internal/cli/errors.go index e95568f6..c9afdf89 100644 --- a/internal/cli/errors.go +++ b/internal/cli/errors.go @@ -370,6 +370,18 @@ func classifyReading(err error) (int, bool) { if errors.As(err, &manifestTooLarge) { return ExitIO, true } + // The two ways a manifest read a piece at a time is too large: one value + // past what is read at once, and more files than the memory setting lets + // a command check. Both are the document's size against a limit, the same + // answer as the ceiling the whole document had before. + var valueTooLarge *manifest.ValueTooLargeError + if errors.As(err, &valueTooLarge) { + return ExitIO, true + } + var tooMuchToHold *manifest.TooMuchToHoldError + if errors.As(err, &tooMuchToHold) { + return ExitIO, true + } // And the same class one step later: a manifest whose entries leave the // directory once the links are followed. The text of the path passed, the // filesystem did not. diff --git a/internal/cli/memoryflag.go b/internal/cli/memoryflag.go index 0b970d67..a10bbabc 100644 --- a/internal/cli/memoryflag.go +++ b/internal/cli/memoryflag.go @@ -6,7 +6,8 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/core" ) -// --memory, on every command that plans: generate, validate and preset show. +// --memory, on every command that plans: generate, validate and preset show - +// and on verify and cleanup, which read a manifest under it. // // One function registers it on all three, so the flag cannot read one way on // one command and another on the next, and core.ParseMemory reads it, so the @@ -50,3 +51,15 @@ func (m *memoryValue) Set(text string) error { func memoryFlag(fs *flag.FlagSet, v *memoryValue) { fs.Var(v, "memory", memoryHelp) } + +// checkingMemoryHelp is what --help says about the flag on verify and cleanup, +// which hold what a manifest claims rather than a plan. The same three +// answers, read by the same parser, and a sentence about what they hold. +var checkingMemoryHelp = "how much memory checking may use: auto (half of what this machine gives the program), " + + "a size such as 8gb, or off for no limit. A manifest listing more files than that can check is refused before any file is looked at. " + + "When checking gets close to this amount, the program frees memory more often, which makes it slower" + +// checkingMemoryFlag puts --memory on verify or cleanup, its value landing in v. +func checkingMemoryFlag(fs *flag.FlagSet, v *memoryValue) { + fs.Var(v, "memory", checkingMemoryHelp) +} diff --git a/internal/cli/verify.go b/internal/cli/verify.go index 5b207ac1..565b4b04 100644 --- a/internal/cli/verify.go +++ b/internal/cli/verify.go @@ -14,7 +14,6 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/audit" "github.com/donislawdev/TestingFilesGenerator/internal/core" - "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) // verify compares a directory against a manifest an earlier run wrote. @@ -23,6 +22,8 @@ func verify(ctx context.Context, args []string, out, errOut io.Writer) int { fs.SetOutput(errOut) against := fs.String("against", "", "directory to check. Defaults to the directory holding the manifest") asJSON := fs.Bool("json", false, "write the report to stdout as JSON") + var memory memoryValue + checkingMemoryFlag(fs, &memory) usage := func(w io.Writer) { fmt.Fprint(w, `tfg verify - check that a directory still matches a manifest. @@ -57,10 +58,9 @@ Flags: return ExitUsage } - m, err := manifest.Load(path) - if err != nil { - fmt.Fprintf(errOut, "tfg: %s\n", describeError(err)) - return classify(err) + claims, code, ok := claimsOf(ctx, path, memory.Memory, "verify was interrupted while reading the manifest and checked nothing.", errOut) + if !ok { + return code } // Defaulting to the directory the manifest sits in makes the common case @@ -74,8 +74,8 @@ Flags: return ExitIO } - diffs, verifyErr := audit.Verify(ctx, dir, m, filepath.Base(path)) - claimed := len(audit.Claimed(m)) + diffs, verifyErr := audit.Verify(ctx, dir, claims, filepath.Base(path)) + claimed := len(claims.Files) // A cancelled run reports what it compared and never calls the rest sound. // diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 4a5d49de..ff094b0b 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -398,7 +398,7 @@ func PlanContext(ctx context.Context, targets []Target, opt Options) ([]PlannedF // reference point, and that has to happen before any of the plan exists. limits := opt.memoryLimits() pl.budget = newPlanMemory(opt.Memory, limits) - handToRuntime(limits) + HandToRuntime(limits) // The manifest lands beside the files, so its name is a name too. A path // here would leave a manifest outside the directory the run was pointed diff --git a/internal/engine/processmemory.go b/internal/engine/processmemory.go index a6a85c72..fcb612b7 100644 --- a/internal/engine/processmemory.go +++ b/internal/engine/processmemory.go @@ -53,14 +53,16 @@ func OwnProcess() { owned = true } -// handToRuntime gives the runtime the limit for a run under limits, in a -// process that is the program's own, and does nothing anywhere else. +// HandToRuntime gives the runtime the limit for a run under limits, in a +// process that is the program's own, and does nothing anywhere else. verify +// and cleanup hand it theirs before they read a manifest, for the same reason a +// plan does. // // Called when a plan starts rather than when the writing does, because the // plan is the first thing both surfaces do for a run and the manifest is saved // after the writing has returned - a limit set and taken back inside Run would // leave both outside it. -func handToRuntime(limits core.MemoryLimits) { +func HandToRuntime(limits core.MemoryLimits) { if owned { debug.SetMemoryLimit(limits.ForRuntime(startLimit)) } diff --git a/internal/guard/anotherrun_test.go b/internal/guard/anotherrun_test.go index 49035d63..f58380a3 100644 --- a/internal/guard/anotherrun_test.go +++ b/internal/guard/anotherrun_test.go @@ -73,7 +73,7 @@ func twoRunsSharing(t *testing.T) (dir string, alpha *manifest.Manifest, alphaPa func TestVerifyNamesTheFilesOfAnotherRunRatherThanCallingThemExtra(t *testing.T) { dir, alpha, _ := twoRunsSharing(t) - diffs, err := audit.Verify(context.Background(), dir, alpha, "manifest-alpha.json") + diffs, err := audit.Verify(context.Background(), dir, alpha.Claims(), "manifest-alpha.json") if err != nil { t.Fatalf("verifying: %v", err) } @@ -128,7 +128,7 @@ func TestAStrayFileBesideAnotherRunsRecordIsStillExtra(t *testing.T) { } } - diffs, err := audit.Verify(context.Background(), dir, alpha, "manifest-alpha.json") + diffs, err := audit.Verify(context.Background(), dir, alpha.Claims(), "manifest-alpha.json") if err != nil { t.Fatalf("verifying: %v", err) } @@ -167,7 +167,7 @@ func TestARecordUnderAnotherExtensionIsStillExtra(t *testing.T) { t.Fatalf("renaming the neighbour's record: %v", err) } - diffs, err := audit.Verify(context.Background(), dir, alpha, "manifest-alpha.json") + diffs, err := audit.Verify(context.Background(), dir, alpha.Claims(), "manifest-alpha.json") if err != nil { t.Fatalf("verifying: %v", err) } @@ -181,7 +181,7 @@ func TestARecordUnderAnotherExtensionIsStillExtra(t *testing.T) { // The sentence tells a reader which run to ask. func TestTheSentenceAboutAnotherRunNamesTheRecordThatClaimsTheFile(t *testing.T) { dir, alpha, _ := twoRunsSharing(t) - diffs, err := audit.Verify(context.Background(), dir, alpha, "manifest-alpha.json") + diffs, err := audit.Verify(context.Background(), dir, alpha.Claims(), "manifest-alpha.json") if err != nil { t.Fatalf("verifying: %v", err) } diff --git a/internal/guard/auditorder_test.go b/internal/guard/auditorder_test.go index 7bbf5896..623fd66f 100644 --- a/internal/guard/auditorder_test.go +++ b/internal/guard/auditorder_test.go @@ -67,9 +67,9 @@ func TestCleanupOffersTheFilesInTheOrderTheManifestListsThem(t *testing.T) { } // A subsequence rather than an equal list, so this asks about ORDER and - // nothing else. Which entries reach the offer is audit.Claimed's rule and - // it has its own guards - repeating it here would mean two places to change - // when it moves, and a red guard naming the wrong thing. + // nothing else. Which entries reach the offer is the manifest's rule for a + // claim and it has its own guards - repeating it here would mean two places + // to change when it moves, and a red guard naming the wrong thing. at := 0 for _, f := range report.Files { found := false @@ -103,7 +103,7 @@ func pathsOf(files []struct { } // manifestPathsInOrder is every path a manifest names, in the order it names -// them - the order audit.Claimed walks and the order the offer has to keep. +// them - the order the claims are read in and the order the offer has to keep. func manifestPathsInOrder(t *testing.T, path string) []string { t.Helper() b, err := os.ReadFile(path) diff --git a/internal/guard/auditpipe_unix_test.go b/internal/guard/auditpipe_unix_test.go index 094d71f8..8074047b 100644 --- a/internal/guard/auditpipe_unix_test.go +++ b/internal/guard/auditpipe_unix_test.go @@ -45,11 +45,11 @@ func TestVerifyAndCleanupNeverWaitOnAPipe(t *testing.T) { } answersWithoutWaiting(t, "verify", func() bool { - diffs, err := audit.Verify(context.Background(), out, m, filepath.Base(mf)) + diffs, err := audit.Verify(context.Background(), out, m.Claims(), 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) + cands, err := audit.Inspect(context.Background(), out, m.Claims()) return err == nil && len(cands) == 1 && cands[0].Disposition == audit.Unreachable }) } diff --git a/internal/guard/cleanuprecheck_test.go b/internal/guard/cleanuprecheck_test.go index d0e11292..80264da1 100644 --- a/internal/guard/cleanuprecheck_test.go +++ b/internal/guard/cleanuprecheck_test.go @@ -43,7 +43,7 @@ func inspected(t *testing.T) (string, []audit.Candidate, string) { 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) + cands, err := audit.Inspect(context.Background(), out, m.Claims()) if err != nil { t.Fatalf("inspecting: %v", err) } @@ -174,7 +174,7 @@ func TestVerifyCallsANameItCouldNotLookUpUnreadableRatherThanMissing(t *testing. t.Fatalf("saving the edited manifest: %v", err) } - diffs, err := audit.Verify(context.Background(), out, m, filepath.Base(mf)) + diffs, err := audit.Verify(context.Background(), out, m.Claims(), filepath.Base(mf)) if err != nil { t.Fatalf("verifying: %v", err) } diff --git a/internal/guard/concurrentruns_test.go b/internal/guard/concurrentruns_test.go index a85c4ba7..79cc3311 100644 --- a/internal/guard/concurrentruns_test.go +++ b/internal/guard/concurrentruns_test.go @@ -247,29 +247,42 @@ func TestAnOrdinaryNameWithADotOrASpaceStillWorks(t *testing.T) { } } -// A manifest is read into memory to be compared against a directory, so its -// size is a lever somebody else holds. The recipe has had a ceiling since -// 2026-08-02 and this had none, which was an asymmetry rather than a decision. -func TestAManifestTooLargeToReadIsRefusedBeforeItIsRead(t *testing.T) { +// A manifest arrives from somebody else, so its size is a lever somebody else +// holds. The recipe has had a ceiling since 2026-08-02, and the manifest had +// one on the whole document from 2026-08-03 until 2026-11-04, when a run past +// it was found to write a manifest verify and cleanup would not read. Now the +// ceiling is on the most of it held at once - one value - and this is what +// stands between a document of any length and the memory of the machine. +// +// Both halves, because each is what makes the other safe: a single value just +// past the ceiling is refused, and a document past it whose values are small is +// read, so the ceiling cannot have quietly gone back to being the document's. +func TestOneValueOfAManifestPastTheCeilingIsRefusedAndALongManifestIsNot(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "manifest.json") - // Just over the ceiling, written as valid JSON so that the refusal cannot - // be mistaken for a parse failure. - padding := strings.Repeat("x", manifest.MaxBytes) + // Written as valid JSON so that the refusal cannot be mistaken for a + // parse failure. + padding := strings.Repeat("x", manifest.MaxValueBytes) doc := `{"manifest_version":"1.0","files":[],"run":{"command":"` + padding + `"}}` if err := os.WriteFile(path, []byte(doc), 0o644); err != nil { t.Fatalf("writing: %v", err) } - code, stdout, errOut := run(t, "verify", path) - if code != cli.ExitIO { - t.Errorf("exit %d, expected %d:\n%s", code, cli.ExitIO, errOut) - } - if !strings.Contains(errOut, "the limit is") { - t.Errorf("the refusal does not say there is a limit:\n%s", errOut) + if code != cli.ExitIO || !strings.Contains(errOut, "a single value of more than") { + t.Errorf("a manifest with one value past the ceiling ended %d, expected %d and the refusal of a value:\n%s", code, cli.ExitIO, errOut) } if stdout != "" { t.Errorf("a failed run wrote to stdout:\n%s", stdout) } + + // Longer than that in entries of a kilobyte, which is what a long run + // writes. None of them claimed, so the answer is about the reading alone. + long := longManifest(`{"path":"first.txt"}`, manifest.MaxValueBytes) + if err := os.WriteFile(path, []byte(long), 0o644); err != nil { + t.Fatalf("writing: %v", err) + } + if code, _, errOut := run(t, "verify", path); code != cli.ExitOK || !strings.Contains(errOut, "nothing to check") { + t.Errorf("a manifest of %d B in small entries ended %d, expected it read and %d:\n%s", len(long), code, cli.ExitOK, errOut) + } } diff --git a/internal/guard/contextroots_test.go b/internal/guard/contextroots_test.go index 9fa9a69d..c590906d 100644 --- a/internal/guard/contextroots_test.go +++ b/internal/guard/contextroots_test.go @@ -32,6 +32,7 @@ var contextRoots = map[string]string{ "internal/oracle/oracle.go PictureChanges": "the same instrument, a reference reader under a time limit", "internal/oracle/oracle.go Strict": "the same instrument, a reference reader under a time limit", "internal/engine/engine.go Plan": "the version without a context, for callers that have none - it is PlanContext, so it cannot disagree with it", + "internal/manifest/manifest.go Load": "reads a document of at most sixteen megabytes that is in memory already, so there is nothing long to stop - verify and cleanup read through ReadClaims, which takes the caller's", "internal/format/eml/eml.go generator.Plan": "the Generator interface's Plan for callers without a context - the engine asks PlanContext", "internal/format/htmlfile/html.go generator.Plan": "the Generator interface's Plan for callers without a context - the engine asks PlanContext", "internal/format/rtf/rtf.go generator.Plan": "the Generator interface's Plan for callers without a context - the engine asks PlanContext", diff --git a/internal/guard/manifestread_test.go b/internal/guard/manifestread_test.go new file mode 100644 index 00000000..6d42f65f --- /dev/null +++ b/internal/guard/manifestread_test.go @@ -0,0 +1,449 @@ +package guard + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// How a manifest is read, as apart from what it says. +// +// verify and cleanup read the whole document into memory and handed it to +// encoding/json, and some things about reading were true only because that is +// what encoding/json does: the keys may come in any order and any case, +// nothing may follow the document, a document with two faults is refused for +// the one that comes first in a fixed order, and a document with nothing in it +// gets the answer it always got. Nothing asked about any of them, because +// nothing could break them. A reader that takes the document a piece at a time +// (docs/MANIFEST-STREAMING-2026-11-04.md) can break every one, so each is said +// here against the reader it replaced, before the new one was written. + +// keyed is one key of a document's outermost object and its value as written. +type keyed struct { + key string + value json.RawMessage +} + +// outermost is the outermost keys of a document in the order they are +// written, which a map cannot say. +func outermost(t *testing.T, raw []byte) []keyed { + t.Helper() + dec := json.NewDecoder(bytes.NewReader(raw)) + if tok, err := dec.Token(); err != nil || tok != json.Delim('{') { + t.Fatalf("the manifest does not open with an object: %v %v", tok, err) + } + var out []keyed + for dec.More() { + tok, err := dec.Token() + if err != nil { + t.Fatalf("reading a key: %v", err) + } + var value json.RawMessage + if err := dec.Decode(&value); err != nil { + t.Fatalf("reading the value of %v: %v", tok, err) + } + key, _ := tok.(string) + out = append(out, keyed{key: key, value: value}) + } + return out +} + +// rewritten writes the manifest at path again from these keys, in this order. +func rewritten(t *testing.T, path string, keys []keyed) { + t.Helper() + var b bytes.Buffer + b.WriteByte('{') + for i, k := range keys { + if i > 0 { + b.WriteByte(',') + } + name, err := json.Marshal(k.key) + if err != nil { + t.Fatal(err) + } + b.Write(name) + b.WriteByte(':') + b.Write(k.value) + } + b.WriteByte('}') + if err := os.WriteFile(path, b.Bytes(), 0o644); err != nil { + t.Fatalf("writing the manifest again: %v", err) + } +} + +// keysOf is the outermost keys of the manifest at path, in order. +func keysOf(t *testing.T, path string) []keyed { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading the manifest: %v", err) + } + return outermost(t, raw) +} + +// textFilesIn counts the .txt files in dir, which is what generated writes. +func textFilesIn(t *testing.T, dir string) int { + t.Helper() + n := 0 + for _, name := range entryNames(t, dir) { + if strings.HasSuffix(name, ".txt") { + n++ + } + } + return n +} + +// The keys of a manifest may come in any order. +// +// This tool writes files after the version, and a reader that took that for +// granted would act on entries before it knew which schema they were written +// to - the mistake the check of the major in Load exists to rule out. A +// manifest that went through somebody's tool on the way, with its keys sorted, +// has files first. +func TestAManifestIsReadTheSameWhateverOrderItsKeysComeIn(t *testing.T) { + out, mf := generated(t) + var files, version keyed + var between []keyed + for _, k := range keysOf(t, mf) { + switch k.key { + case "files": + files = k + case "manifest_version": + version = k + default: + between = append([]keyed{k}, between...) + } + } + if files.key == "" || version.key == "" || len(between) == 0 { + t.Fatalf("the manifest a run wrote lacks files, manifest_version or anything between them") + } + rewritten(t, mf, append(append([]keyed{files}, between...), version)) + + // The state this is about, asked of the document rather than assumed. + if got := keysOf(t, mf); got[0].key != "files" || got[len(got)-1].key != "manifest_version" { + t.Fatalf("the manifest was not turned round: its keys are %v", got) + } + if code, stdout, errOut := run(t, "verify", mf); code != cli.ExitOK || !strings.Contains(stdout, "3 files checked") { + t.Errorf("verify on a manifest whose files come first ended %d:\n%s%s", code, stdout, errOut) + } + if code, _, errOut := run(t, "cleanup", mf, "--yes"); code != cli.ExitOK { + t.Errorf("cleanup on a manifest whose files come first ended %d:\n%s", code, errOut) + } + if n := textFilesIn(t, out); n != 0 { + t.Errorf("cleanup left %d of the three files a manifest whose files come first lists", n) + } +} + +// The keys of a manifest may be written in any case, which is what +// encoding/json has always accepted, so a manifest somebody's tool wrote that +// way was read and has to go on being read. +func TestAManifestIsReadTheSameWhateverCaseItsKeysAreIn(t *testing.T) { + _, mf := generated(t) + keys := keysOf(t, mf) + shouted := 0 + for i, k := range keys { + if k.key == "files" || k.key == "manifest_version" { + keys[i].key = strings.ToUpper(k.key) + shouted++ + } + } + if shouted != 2 { + t.Fatalf("expected to find files and manifest_version, found %d of them", shouted) + } + rewritten(t, mf, keys) + if code, stdout, errOut := run(t, "verify", mf); code != cli.ExitOK || !strings.Contains(stdout, "3 files checked") { + t.Errorf("verify on a manifest with FILES and MANIFEST_VERSION ended %d:\n%s%s", code, stdout, errOut) + } +} + +// Nothing may follow a manifest but white space. +// +// Two documents in one file are not a manifest, and a reader that stopped +// where the first one closed would act on half of something. encoding/json +// refuses this on its own. A reader that goes a piece at a time stops where +// the object closes unless it is made to ask what comes after. +func TestAManifestWithAnythingAfterItIsRefused(t *testing.T) { + for _, tail := range []struct{ name, text string }{ + {"a second object", "{}"}, + {"a second copy of itself", ""}, + {"a stray letter", "x"}, + {"a closing brace too many", "}"}, + } { + t.Run(tail.name, func(t *testing.T) { + out, mf := generated(t) + raw, err := os.ReadFile(mf) + if err != nil { + t.Fatal(err) + } + text := tail.text + if text == "" { + text = string(raw) + } + if err := os.WriteFile(mf, append(raw, []byte("\n"+text)...), 0o644); err != nil { + t.Fatal(err) + } + code, stdout, errOut := run(t, "cleanup", mf, "--yes") + if code != cli.ExitIO || stdout != "" || !strings.Contains(errOut, "cannot be read as a manifest") { + t.Errorf("cleanup on a manifest followed by %s ended %d, expected %d and a refusal:\n%s%s", tail.name, code, cli.ExitIO, stdout, errOut) + } + if n := textFilesIn(t, out); n != 3 { + t.Errorf("cleanup acted on a manifest followed by %s: %d of the three files are left", tail.name, n) + } + if code, _, errOut := run(t, "verify", mf); code != cli.ExitIO { + t.Errorf("verify on a manifest followed by %s ended %d, expected %d:\n%s", tail.name, code, cli.ExitIO, errOut) + } + }) + } +} + +// White space after a manifest is nothing. An editor that saves one ends it +// with a newline, and some with more than one. +func TestWhiteSpaceAfterAManifestIsNothing(t *testing.T) { + _, mf := generated(t) + raw, err := os.ReadFile(mf) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(mf, append(raw, []byte("\r\n\n\t \n")...), 0o644); err != nil { + t.Fatal(err) + } + if code, _, errOut := run(t, "verify", mf); code != cli.ExitOK { + t.Errorf("verify on a manifest followed by white space ended %d:\n%s", code, errOut) + } +} + +// claimOf writes a file into dir and gives back a manifest entry that claims +// it, its size and hash right - an entry cleanup removes the file for. +func claimOf(t *testing.T, dir, name, content string) string { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + sum := sha256.Sum256([]byte(content)) + return fmt.Sprintf(`{"path":%q,"name":%q,"materialized":true,"bytes":%d,"hashes":{"sha256":%q}}`, + name, name, len(content), hex.EncodeToString(sum[:])) +} + +// detailOf is what a refusal to read a manifest says is wrong with it - the +// part after the path, because the path of a guard's directory carries the +// name of the guard and the name may hold any word. +func detailOf(errOut string) string { + _, detail, _ := strings.Cut(errOut, "cannot be read as a manifest: ") + return detail +} + +// A manifest that names one of its keys twice is refused whole. +// +// encoding/json keeps the last of the two, so a document with two lists of +// files was checked and acted on by its second list, and a reader going a +// piece at a time would have to read the first, check it and throw it away to +// do the same. Neither is an answer about a record of one run: this tool never +// writes a key twice, so a document that does was written by something else, +// and which of its two halves it meant is a guess. The second spelling counts +// as the same key, because encoding/json reads Files as files. +func TestAManifestThatNamesAKeyTwiceIsRefused(t *testing.T) { + for _, c := range []struct{ name, doc, key string }{ + {"files twice", `{"manifest_version":"1.0","files":[],"files":[%s]}`, `"files"`}, + {"files under a second spelling", `{"manifest_version":"1.0","files":[],"Files":[%s]}`, `"Files"`}, + {"the version twice", `{"manifest_version":"9.0","manifest_version":"1.0","files":[%s]}`, `"manifest_version"`}, + } { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + mf := filepath.Join(dir, "manifest.json") + doc := fmt.Sprintf(c.doc, claimOf(t, dir, "ours.txt", "written by the run\n")) + if err := os.WriteFile(mf, []byte(doc), 0o644); err != nil { + t.Fatal(err) + } + code, stdout, errOut := run(t, "cleanup", mf, "--yes") + if code != cli.ExitIO || stdout != "" { + t.Errorf("cleanup on a manifest with %s ended %d, expected %d:\n%s%s", c.name, code, cli.ExitIO, stdout, errOut) + } + if !strings.Contains(detailOf(errOut), c.key) { + t.Errorf("the refusal does not name the key %s:\n%s", c.key, errOut) + } + if _, err := os.Stat(filepath.Join(dir, "ours.txt")); err != nil { + t.Errorf("cleanup acted on a manifest with %s: %v", c.name, err) + } + }) + } +} + +// escaping is a manifest entry whose path leaves the directory. +const escaping = `{"path":"../escaped.txt","materialized":true}` + +// twoFaultDocuments is documents with two things wrong with them, what the +// refusal of each says and what it does not, because it is about the other. +var twoFaultDocuments = []struct { + name, doc string + says []string + not []string +}{ + {"cut short after an entry that escapes", `{"manifest_version":"1.0","files":[` + escaping + `,`, + nil, []string{"escaped", "manifest_version"}}, + {"a version of the wrong type after an entry that escapes", `{"files":[` + escaping + `],"manifest_version":1}`, + []string{"cannot unmarshal"}, []string{"escaped"}}, + {"no version and an entry that escapes", `{"files":[` + escaping + `]}`, + []string{"no manifest_version"}, []string{"escaped"}}, + {"a future version after an entry that escapes", `{"files":[` + escaping + `],"manifest_version":"9.0"}`, + []string{"9.0"}, []string{"escaped"}}, + {"two entries that escape, the first in a failed entry", `{"manifest_version":"1.0","files":[{"path":"fine.txt"},{"path":"../first.txt","failed":true},{"path":"/second.txt","materialized":true}]}`, + []string{"entry 2", "../first.txt"}, []string{"second"}}, + {"instructions that escape before an entry that escapes", `{"manifest_version":"1.0","run":{"instructions":"../x.instructions.md"},"files":[` + escaping + `]}`, + []string{"escaped"}, []string{"run.instructions"}}, +} + +// A document with two faults is refused for the one that comes first, in an +// order that does not depend on where in the document each one sits. +// +// What is wrong with the document as a document first, then the version, then +// the first entry that leaves the directory, counted over every entry, then +// the instructions. A reader that answered with whichever fault it met first +// would refuse one manifest in different words depending on the order of its +// keys - and say "an entry lands outside" about a document of a future schema, +// whose entries this build does not know how to read. +func TestAManifestWithTwoFaultsIsRefusedForTheOneThatComesFirst(t *testing.T) { + for _, c := range twoFaultDocuments { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + mf := filepath.Join(dir, "manifest.json") + if err := os.WriteFile(mf, []byte(c.doc), 0o644); err != nil { + t.Fatal(err) + } + for _, command := range []string{"verify", "cleanup"} { + code, stdout, errOut := run(t, command, mf) + detail := detailOf(errOut) + if code != cli.ExitIO || stdout != "" || detail == "" { + t.Errorf("%s on a document %s ended %d, expected %d and a refusal to read it:\n%s%s", command, c.name, code, cli.ExitIO, stdout, errOut) + continue + } + for _, s := range c.says { + if !strings.Contains(detail, s) { + t.Errorf("%s refused a document %s without saying %q:\n%s", command, c.name, s, detail) + } + } + for _, s := range c.not { + if strings.Contains(detail, s) { + t.Errorf("%s refused a document %s for a fault that comes later, saying %q:\n%s", command, c.name, s, detail) + } + } + } + }) + } +} + +// nearlyEmptyDocuments is documents with nothing in them, or nothing that is a +// manifest, and the code verify ends with on each. +var nearlyEmptyDocuments = []struct { + name, doc string + code int +}{ + {"no bytes at all", ``, cli.ExitIO}, + {"an array", `[]`, cli.ExitIO}, + {"a string", `"manifest"`, cli.ExitIO}, + {"null", `null`, cli.ExitIO}, + {"an empty object", `{}`, cli.ExitIO}, + {"a byte order mark in front", "\uFEFF" + `{"manifest_version":"1.0","files":[]}`, cli.ExitIO}, + {"files that are an object", `{"manifest_version":"1.0","files":{}}`, cli.ExitIO}, + {"an entry that is a number", `{"manifest_version":"1.0","files":[1]}`, cli.ExitIO}, + {"an entry that is null", `{"manifest_version":"1.0","files":[null]}`, cli.ExitIO}, + {"an entry with no path", `{"manifest_version":"1.0","files":[{"materialized":true}]}`, cli.ExitIO}, + {"no files key", `{"manifest_version":"1.0"}`, cli.ExitOK}, + {"files that are null", `{"manifest_version":"1.0","files":null}`, cli.ExitOK}, + {"only entries the run never wrote", `{"manifest_version":"1.0","files":[{"path":"a.txt","failed":true},{"path":"b.txt"}]}`, cli.ExitOK}, +} + +// A document with nothing in it, or nothing that is a manifest, gets the +// answer it always got: refused as a document when it is not an object with a +// version, and nothing to check when it is one that lists nothing on the disk. +func TestAManifestWithNothingInItIsAnsweredAsItAlwaysWas(t *testing.T) { + for _, c := range nearlyEmptyDocuments { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + mf := filepath.Join(dir, "manifest.json") + if err := os.WriteFile(mf, []byte(c.doc), 0o644); err != nil { + t.Fatal(err) + } + code, stdout, errOut := run(t, "verify", mf) + if code != c.code { + t.Fatalf("verify on %s ended %d, expected %d:\n%s%s", c.name, code, c.code, stdout, errOut) + } + if c.code == cli.ExitIO && (stdout != "" || detailOf(errOut) == "") { + t.Errorf("verify on %s did not refuse it as a manifest it cannot read:\n%s%s", c.name, stdout, errOut) + } + if c.code == cli.ExitOK && !strings.Contains(errOut, "nothing to check") { + t.Errorf("verify on %s did not say there was nothing to check:\n%s", c.name, errOut) + } + }) + } +} + +// Another run's record is another run's record however long it is. +// +// The records of the other runs in a directory were read through the door that +// refused a manifest past sixteen megabytes, and a record that is refused is +// not a record - so the files of a run past that size were called extra in the +// directory they share, the word for files nobody asked for. Nothing said so, +// because no guard had a neighbour that large. +func TestAnotherRunsRecordIsAnotherRunsRecordHoweverLongItIs(t *testing.T) { + out, mf := generated(t) + theirs := claimOf(t, out, "theirs.bin", "the other run wrote this\n") + + // One file of its own and then enough it never wrote to pass the old + // ceiling. + record := filepath.Join(out, "other.json") + if err := os.WriteFile(record, []byte(longManifest(theirs, manifest.MaxBytes)), 0o644); err != nil { + t.Fatal(err) + } + // The state this is about: the record is longer than the old reader took. + if info, err := os.Stat(record); err != nil || info.Size() <= manifest.MaxBytes { + t.Fatalf("the neighbour's record is not past %d B: %v %v", manifest.MaxBytes, info, err) + } + + code, stdout, errOut := run(t, "verify", mf, "--json") + report := stdout + if code != cli.ExitOK { + report = errOut + t.Errorf("verify called the directory a mismatch because of another run's files, exit %d", code) + } + var got struct { + Differences []struct{ Kind, Path, Expected string } `json:"differences"` + } + if err := json.Unmarshal([]byte(report), &got); err != nil { + t.Fatalf("reading the report: %v\n%s", err, report) + } + found := false + for _, d := range got.Differences { + if d.Path == "theirs.bin" { + found = true + if d.Kind != "another-run" || d.Expected != "other.json" { + t.Errorf("the other run's file was reported as %q (%q), expected another-run claimed by other.json", d.Kind, d.Expected) + } + } + } + if !found { + t.Errorf("the other run's file is not in the report at all:\n%s", report) + } +} + +// longManifest is a manifest whose list opens with first and goes on, in +// entries of a kilobyte that claim nothing - as long as the entries of a real +// run are - until the document is longer than past. +func longManifest(first string, past int) string { + var b strings.Builder + b.WriteString(`{"manifest_version":"1.0","files":[` + first) + padding := strings.Repeat("p", 1000) + for i := 0; b.Len() <= past; i++ { + fmt.Fprintf(&b, `,{"path":"never_%07d.txt","materialized":false,"padding":%q}`, i, padding) + } + b.WriteString("]}") + return b.String() +} diff --git a/internal/guard/manifestreadlimits_test.go b/internal/guard/manifestreadlimits_test.go new file mode 100644 index 00000000..20bd38ff --- /dev/null +++ b/internal/guard/manifestreadlimits_test.go @@ -0,0 +1,194 @@ +package guard + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// The limits a manifest is read under: the ceiling on one value, the memory +// setting on what verify and cleanup hold, Ctrl+C while it is read - and the +// two doors it is read through agreeing about all of it. Beside +// manifestread_test.go, which holds what reading must keep doing the way the +// reader it replaced did. + +// Load holds the whole of a manifest and keeps the ceiling the whole document +// always had, while ReadClaims reads the same document. +// +// Load is for the guards and anything else that needs every entry, and holding +// every entry of a document of any length is what the ceiling on a document +// was for. ReadClaims holds three fields of what is claimed and has none. +func TestLoadKeepsTheCeilingOnAWholeDocumentAndReadClaimsHasNone(t *testing.T) { + path := filepath.Join(t.TempDir(), "manifest.json") + if err := os.WriteFile(path, []byte(longManifest(`{"path":"first.txt"}`, manifest.MaxBytes)), 0o644); err != nil { + t.Fatal(err) + } + var tooLarge *manifest.TooLargeError + if _, err := manifest.Load(path); !errors.As(err, &tooLarge) { + t.Errorf("Load read a document past %d B, or refused it for something else: %v", manifest.MaxBytes, err) + } + if _, err := manifest.ReadClaims(context.Background(), path, 0); err != nil { + t.Errorf("ReadClaims refused a document whose only fault is its length: %v", err) + } +} + +// The two doors a manifest is read through refuse the same documents in the +// same words, and give the same claims for the rest. +// +// Load holds the whole of a manifest and ReadClaims three fields of what it +// claims. Both are one reader underneath, and this is what keeps them so: a +// door given its own parsing later, or its own order of faults, would make +// every guard that reads a manifest through Load a guard about a reader verify +// and cleanup do not use. +func TestBothDoorsOfTheManifestRefuseTheSameDocumentsInTheSameWords(t *testing.T) { + docs := []struct{ name, doc string }{ + {"a key twice", `{"manifest_version":"1.0","files":[],"Files":[]}`}, + {"a second object after it", `{"manifest_version":"1.0","files":[]} {}`}, + {"an ordinary manifest", `{"manifest_version":"1.0","run":{"instructions":"manifest.instructions.md"},"files":[{"path":"a.txt","materialized":true,"bytes":1,"hashes":{"sha256":"00"}},{"path":"b.txt","failed":true}]}`}, + } + for _, c := range twoFaultDocuments { + docs = append(docs, struct{ name, doc string }{c.name, c.doc}) + } + for _, c := range nearlyEmptyDocuments { + docs = append(docs, struct{ name, doc string }{c.name, c.doc}) + } + refused, read := 0, 0 + for _, c := range docs { + path := filepath.Join(t.TempDir(), "manifest.json") + if err := os.WriteFile(path, []byte(c.doc), 0o644); err != nil { + t.Fatal(err) + } + whole, loadErr := manifest.Load(path) + claims, readErr := manifest.ReadClaims(context.Background(), path, 0) + switch { + case loadErr != nil && readErr != nil: + refused++ + if loadErr.Error() != readErr.Error() { + t.Errorf("%s: the two doors refuse it in different words:\n Load %v\n ReadClaims %v", c.name, loadErr, readErr) + } + case loadErr == nil && readErr == nil: + read++ + if fmt.Sprint(whole.Claims()) != fmt.Sprint(claims) { + t.Errorf("%s: the two doors claim different files:\n Load %v\n ReadClaims %v", c.name, whole.Claims(), claims) + } + default: + t.Errorf("%s: one door reads it and the other does not:\n Load %v\n ReadClaims %v", c.name, loadErr, readErr) + } + } + // The state this depends on: both kinds of answer were asked for. + if refused < len(twoFaultDocuments) || read < 2 { + t.Fatalf("%d documents refused and %d read - the corpus no longer holds both kinds", refused, read) + } +} + +// stoppedPartWay is a context cancelled at its nth question - Ctrl+C landing +// while a manifest is read, wherever the reader is when it asks. +type stoppedPartWay struct { + context.Context + cancel context.CancelFunc + asked, at int +} + +func (s *stoppedPartWay) Err() error { + s.asked++ + if s.asked >= s.at { + s.cancel() + } + return s.Context.Err() +} + +// Ctrl+C while a manifest is read stops the reading, and cleanup says it +// removed nothing. +// +// A manifest of a million files takes seconds to read, and the reader that held +// it whole asked nobody anything while it read. This document is cut short in +// its hundred and second entry, so a reader that asks only before it starts, or +// only between the keys of the document, reads on to the cut and refuses the +// document instead - only one that asks as it goes through the entries stops +// first. Cancelled at the fiftieth question, because what matters is that the +// asking goes on, not where it starts. +func TestCtrlCWhileAManifestIsReadStopsTheReadingAndRemovesNothing(t *testing.T) { + dir := t.TempDir() + var b strings.Builder + b.WriteString(`{"manifest_version":"1.0","files":[` + claimOf(t, dir, "ours.txt", "written by the run\n")) + for i := range 100 { + fmt.Fprintf(&b, `,{"path":"never_%03d.txt"}`, i) + } + b.WriteString(`,{"path":"cut sh`) + mf := filepath.Join(dir, "manifest.json") + if err := os.WriteFile(mf, []byte(b.String()), 0o644); err != nil { + t.Fatal(err) + } + // The state this depends on: read to the end, the document is refused + // rather than stopped. + if code, _, errOut := run(t, "cleanup", mf, "--yes"); code != cli.ExitIO { + t.Fatalf("cleanup on the document cut short ended %d, expected %d:\n%s", code, cli.ExitIO, errOut) + } + + base, cancel := context.WithCancel(context.Background()) + defer cancel() + stopped := &stoppedPartWay{Context: base, cancel: cancel, at: 50} + var out, errOut bytes.Buffer + code := cli.Run(stopped, []string{"cleanup", mf, "--yes"}, &out, &errOut) + if code != cli.ExitInterrupted || !strings.Contains(errOut.String(), "interrupted while reading the manifest and removed nothing") { + t.Errorf("Ctrl+C while the manifest was read ended %d, expected %d and a sentence saying so:\n%s", code, cli.ExitInterrupted, errOut.String()) + } + if _, err := os.Stat(filepath.Join(dir, "ours.txt")); err != nil { + t.Errorf("cleanup removed a file after Ctrl+C: %v", err) + } +} + +// A manifest listing more files than the memory setting lets cleanup check is +// refused before a file is looked at, and the refusal says how much it would +// take. +// +// The reader holds three fields of each claimed file and the checking a few +// times that, so a run of tens of millions of files could still ask a machine +// for more than it has. The memory setting is the person's number for that, the +// one generate plans under. The refusal comes once the whole document has been +// read and found sound, so a document with a fault of its own is refused for +// the fault rather than for its size. +func TestAManifestListingMoreThanTheMemorySettingLetsCleanupCheckIsRefused(t *testing.T) { + dir := t.TempDir() + var b strings.Builder + b.WriteString(`{"manifest_version":"1.0","files":[` + claimOf(t, dir, "ours.txt", "written by the run\n")) + for i := range 999 { + fmt.Fprintf(&b, `,{"path":"elsewhere_%03d.txt","materialized":true,"bytes":1,"hashes":{"sha256":"%064d"}}`, i, 0) + } + mf := filepath.Join(dir, "manifest.json") + if err := os.WriteFile(mf, []byte(b.String()+"]}"), 0o644); err != nil { + t.Fatal(err) + } + + code, stdout, errOut := run(t, "cleanup", mf, "--yes", "--memory", "64kb") + if code != cli.ExitIO || stdout != "" || !strings.Contains(errOut, "lists 1000 files") || !strings.Contains(errOut, "the memory setting gives") { + t.Errorf("cleanup under a setting too small for the manifest ended %d, expected %d and the refusal:\n%s%s", code, cli.ExitIO, stdout, errOut) + } + if _, err := os.Stat(filepath.Join(dir, "ours.txt")); err != nil { + t.Errorf("cleanup acted on a manifest it refused: %v", err) + } + // Room enough, or no limit, and the same document is read - what refused + // it was the setting. + for _, setting := range []string{"64mb", "off"} { + if code, _, errOut := run(t, "verify", mf, "--memory", setting); code != cli.ExitVerify { + t.Errorf("verify under --memory %s ended %d, expected the %d of a directory missing files:\n%s", setting, code, cli.ExitVerify, errOut) + } + } + + // A fault of the document comes before its size. + if err := os.WriteFile(mf, []byte(b.String()+","+escaping+"]}"), 0o644); err != nil { + t.Fatal(err) + } + code, _, errOut = run(t, "verify", mf, "--memory", "64kb") + if code != cli.ExitIO || !strings.Contains(errOut, "escaped") || strings.Contains(errOut, "memory setting") { + t.Errorf("a manifest too large for the setting and with an entry that escapes was not refused for the entry, exit %d:\n%s", code, errOut) + } +} diff --git a/internal/guard/runlock_test.go b/internal/guard/runlock_test.go index 7da1a935..dc4e29a1 100644 --- a/internal/guard/runlock_test.go +++ b/internal/guard/runlock_test.go @@ -194,7 +194,7 @@ func TestVerifyNamesTheRunLockAsOursRatherThanAsSomebodyElses(t *testing.T) { t.Fatalf("standing in for a run that was killed: %v", err) } - diffs, err := audit.Verify(context.Background(), dir, res.Manifest, "manifest.json") + diffs, err := audit.Verify(context.Background(), dir, res.Manifest.Claims(), "manifest.json") if err != nil { t.Fatalf("verifying: %v", err) } diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index 674f3ac0..68389069 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -1,7 +1,8 @@ package manifest import ( - "encoding/json" + "bytes" + "context" "io" "os" "runtime" @@ -576,38 +577,72 @@ func (e *TooLargeError) Said() core.Said { // acting on the half of it we recognise is how "verify" ends up calling a // directory sound on the strength of the part it could read. // -// One larger than MaxBytes is refused before a byte of it is read, because a -// manifest is held in memory to be compared against a directory and the file -// being read is not necessarily one of ours. +// One larger than MaxBytes is refused before a byte of it is read, because +// Load holds the whole document and every entry of it, and the file being read +// is not necessarily one of ours. verify and cleanup read through ReadClaims, +// which holds three fields of the claimed entries and has no ceiling on the +// document - Load is for a caller that needs all of it, and a manifest past +// sixteen megabytes is one such a caller should not be holding. +// +// The same reader as ReadClaims underneath (read.go), so the two refuse the +// same documents in the same words. func Load(path string) (*Manifest, error) { raw, err := readAtMost(path) if err != nil { return nil, err } var m Manifest - if err := json.Unmarshal(raw, &m); err != nil { - return nil, &SchemaError{Path: path, Detail: core.SaidOf(err)} + // Nothing long to stop: the document is in memory already and has a + // ceiling, so a context here would be one nobody could use. + err = decode(context.Background(), bytes.NewReader(raw), path, &m, func(f File) { + m.Files = append(m.Files, f) + }) + if err != nil { + return nil, err } + return &m, nil +} + +// settled is what is asked of a manifest once all of it has been read, in the +// order the answers come in: the version, then the first entry that leaves the +// directory, then the instructions. A fault of the document as a document came +// before all three and was answered while it was read. +// +// The version first, before anything is believed. A manifest from a future +// major describes fields this build does not know, and acting on the half of it +// we recognise is how "verify" ends up calling a directory sound on the +// strength of the part it could read - and an entry of it that seems to leave +// the directory is an entry this build does not know how to read. +func settled(path string, m *Manifest, escaped error) error { if m.ManifestVersion == "" { - return nil, &SchemaError{Path: path, Detail: core.Says("manifest.ItCarriesNoManifestVersionSo", "it carries no manifest_version, so it is not a manifest this tool wrote")} + return &SchemaError{Path: path, Detail: core.Says("manifest.ItCarriesNoManifestVersionSo", "it carries no manifest_version, so it is not a manifest this tool wrote")} } got, want := major(m.ManifestVersion), major(Version) if got != want { - return nil, &SchemaError{Path: path, Detail: core.Says("manifest.ItIsSchemaVersionAndThis", "it is schema version %s and this build reads %s. Use the version of tfg that wrote it", core.A("ManifestVersion", m.ManifestVersion), core.A("Version", Version))} + return &SchemaError{Path: path, Detail: core.Says("manifest.ItIsSchemaVersionAndThis", "it is schema version %s and this build reads %s. Use the version of tfg that wrote it", core.A("ManifestVersion", m.ManifestVersion), core.A("Version", Version))} } - if err := checkPaths(path, &m); err != nil { - return nil, err + if escaped != nil { + return escaped } - return &m, nil + // The instructions are a name beside the manifest and nothing else. + // cleanup --with-manifest removes the file this names, so a manifest + // carrying a path here would be a manifest that removes something the run + // never wrote. + if name := m.Run.Instructions; name != "" && !isInstructionsName(name) { + return &SchemaError{Path: path, Detail: core.Says("manifest.RunInstructionsIsAndItCan", "run.instructions is %q, and it can only be the name of a file beside the manifest ending in %s. "+ + "This tool removes that file with the manifest, so it will not act on one that names anything else. "+ + "Use the manifest the run actually wrote, or remove the key", core.A("Name", name), core.A("InstructionsSuffix", instructionsSuffix))} + } + return nil } -// checkPaths refuses a manifest whose entries point outside the directory they -// describe. +// entryEscapes refuses entry i of a manifest when its path points outside the +// directory the manifest describes, and is nil when it stays inside. // -// This is the door. Every consumer of a manifest comes through Load, so the -// check sits here rather than beside each use - "verify" and "cleanup" then -// cannot drift into disagreeing about it, and a consumer written later gets it -// without knowing it exists. +// This is the door. Every consumer of a manifest comes through the one reader, +// so the check sits there rather than beside each use - "verify" and "cleanup" +// then cannot drift into disagreeing about it, and a consumer written later +// gets it without knowing it exists. // // Measured on 2026-08-03, before this existed: an entry with the path // "../VICTIM.txt" and cleanup --yes --force removed a file one directory above @@ -621,28 +656,17 @@ func Load(path string) (*Manifest, error) { // written means the file was edited or produced by something else. Acting on // the rest of it would be trusting the half we happen to like. // -// Every entry is checked, not only the ones Claimed returns. An entry that no -// command reads today is one a command reads tomorrow. -func checkPaths(path string, m *Manifest) error { - for i, f := range m.Files { - problem := core.ContainmentProblem(f.Path) - if problem == "" { - continue - } - return &SchemaError{Path: path, Detail: core.Says("manifest.EntryHasThePathWhichLands", "entry %d has the path %q, which lands outside the directory the manifest describes - %s. "+ - "This tool never reads or removes anything outside that directory, so a manifest that asks it to is one it will not act on. "+ - "Use the manifest the run actually wrote, or correct the path to one inside the directory", core.A("I", i+1), core.A("Path", f.Path), core.A("Problem", problem))} +// Every entry is checked, not only the claimed ones. An entry that no command +// reads today is one a command reads tomorrow. i counts every entry from one, +// so the number is the one a person finds by counting the list. +func entryEscapes(path string, i int, p string) error { + problem := core.ContainmentProblem(p) + if problem == "" { + return nil } - // The instructions are a name beside the manifest and nothing else. - // cleanup --with-manifest removes the file this names, so a manifest - // carrying a path here would be a manifest that removes something the run - // never wrote. - if name := m.Run.Instructions; name != "" && !isInstructionsName(name) { - return &SchemaError{Path: path, Detail: core.Says("manifest.RunInstructionsIsAndItCan", "run.instructions is %q, and it can only be the name of a file beside the manifest ending in %s. "+ - "This tool removes that file with the manifest, so it will not act on one that names anything else. "+ - "Use the manifest the run actually wrote, or remove the key", core.A("Name", name), core.A("InstructionsSuffix", instructionsSuffix))} - } - return nil + return &SchemaError{Path: path, Detail: core.Says("manifest.EntryHasThePathWhichLands", "entry %d has the path %q, which lands outside the directory the manifest describes - %s. "+ + "This tool never reads or removes anything outside that directory, so a manifest that asks it to is one it will not act on. "+ + "Use the manifest the run actually wrote, or correct the path to one inside the directory", core.A("I", i), core.A("Path", p), core.A("Problem", problem))} } // major is the part before the first dot. A minor bump adds fields and stays diff --git a/internal/manifest/read.go b/internal/manifest/read.go new file mode 100644 index 00000000..ae0e1f51 --- /dev/null +++ b/internal/manifest/read.go @@ -0,0 +1,514 @@ +package manifest + +import ( + "bufio" + "context" + "encoding/json" + "errors" + "io" + "os" + "reflect" + "strings" + + "github.com/donislawdev/TestingFilesGenerator/internal/core" +) + +// How a manifest is read, a piece at a time. +// +// Until 2026-11-04 a manifest was read whole into memory and handed to +// encoding/json, under a ceiling of sixteen megabytes on the document - about +// twenty thousand files. A larger run wrote a manifest verify and cleanup +// refused to read, so the files of a run of six hundred thousand were files +// this tool would never remove, and the document read that way peaked at about +// four times its length (docs/MANIFEST-STREAMING-2026-11-04.md, sections 2 and +// 8). Now it is read one value at a time - the tool, the run, each entry - +// under a ceiling on one value rather than on the document, and what verify and +// cleanup keep of an entry is three fields of it. Encode writes the same +// document the same way round (encode.go). +// +// One reader for every caller. Load keeps every entry and ReadClaims three +// fields of the claimed ones, and both are this reader underneath, so they +// cannot come to disagree about which documents they refuse or in which words. + +// MaxValueBytes is the most of a manifest held at once while it is read: one +// value - one entry, the run, the tool - and never the document. +// +// The sixteen megabytes the whole document was allowed before, so a hostile +// document cannot make the reader hold more than a legal one could. An entry +// this tool writes is a kilobyte or two, and the part of it a recipe chooses +// comes from a recipe that has a ceiling of a megabyte, so nothing it writes +// comes near. Measured with tools/probes/manifestread: a document of one value +// of 200 MB, read without this, peaked at 723 MB and was accepted, and with it +// was refused after 16 MB in 0.02 s - while an ordinary document of 80 MB read +// in the same time with it and without it (section 8 of the analysis). +const MaxValueBytes = 16 << 20 + +// readBuffer is how much of the file is read at a time underneath the decoder. +const readBuffer = 64 << 10 + +// errValueTooLarge is the reader under the decoder declining to hand over more +// of one value. It comes back out of the decoder as it went in. +var errValueTooLarge = errors.New("a value runs past the ceiling on one value") + +// bounded hands the decoder the document up to a limit that is moved before +// each value. The decoder keeps what it was handed and has not used yet, so it +// never holds more than the limit past the place the value began. +type bounded struct { + r io.Reader + read int64 + limit int64 +} + +func (b *bounded) Read(p []byte) (int, error) { + room := b.limit - b.read + if room <= 0 { + return 0, errValueTooLarge + } + if int64(len(p)) > room { + p = p[:room] + } + n, err := b.r.Read(p) + b.read += int64(n) + return n, err +} + +// ValueTooLargeError is a manifest holding one value past MaxValueBytes. +type ValueTooLargeError struct { + Path string + // At is about how far into the document the value begins. + At int64 +} + +func (e *ValueTooLargeError) Error() string { return e.Said().String() } + +// Said is the refusal, for a window that says it in its own language. +func (e *ValueTooLargeError) Said() core.Said { + return core.Says("manifest.ValueTooLarge", + "%s holds a single value of more than %d B, starting about %d B in, and this tool reads at most that much of a manifest at once. "+ + "An entry of a manifest this tool wrote takes a kilobyte or two, so check that this is the manifest the run wrote", + core.A("Path", e.Path), core.A("Limit", int64(MaxValueBytes)), core.A("At", e.At)) +} + +// reading is one manifest on its way through the reader. +type reading struct { + ctx context.Context + path string + in *bounded + dec *json.Decoder + // escaped is the first entry that leaves the directory. Kept rather than + // returned, because a fault of the document further on, or a version this + // build does not read, is the answer that comes first - see settled. + escaped error +} + +// decode reads the manifest r holds into m, handing each entry to each as it +// arrives rather than keeping it. path is only what the refusals call it. +// +// each runs before the document has been found sound - an entry may arrive +// before the version does, and a fault may follow it - so a caller keeps what +// it is handed and acts on none of it unless decode returns no error. +// +// The keys of the document may come in any order and in any case, as +// encoding/json has always read them, and the entries are checked one at a +// time as they pass, so reading a manifest whose files come first holds no +// more than reading one whose files come last. +func decode(ctx context.Context, r io.Reader, path string, m *Manifest, each func(File)) error { + in := &bounded{r: r} + rd := &reading{ctx: ctx, path: path, in: in, dec: json.NewDecoder(in)} + if err := rd.document(m, each); err != nil { + return err + } + return settled(path, m, rd.escaped) +} + +// next moves the limit to one value past where the decoder stands. Called +// before every question put to the decoder, More among them - a question that +// finds the limit spent would answer for the limit rather than the document. +func (rd *reading) next() { + rd.in.limit = rd.dec.InputOffset() + MaxValueBytes +} + +// token is the next token of the document, its error in the words of the +// manifest. An end of the input part way through is a document cut short. +func (rd *reading) token() (json.Token, error) { + rd.next() + tok, err := rd.dec.Token() + if errors.Is(err, io.EOF) { + err = io.ErrUnexpectedEOF + } + return tok, rd.unreadable(err) +} + +// value decodes the next value of the document into v. +func (rd *reading) value(v any) error { + rd.next() + return rd.unreadable(rd.dec.Decode(v)) +} + +// more says whether the object or list the decoder stands in goes on. +func (rd *reading) more() bool { + rd.next() + return rd.dec.More() +} + +// unreadable is an error of the decoder in the words of a manifest. A fault of +// the document is a SchemaError, a value past the ceiling says so, and the +// rest - the disk failing, the context ending - is passed on as it came, so a +// failure to read is never called a fault of the document. +func (rd *reading) unreadable(err error) error { + var syntax *json.SyntaxError + var mistyped *json.UnmarshalTypeError + switch { + case err == nil: + return nil + case errors.Is(err, errValueTooLarge): + return &ValueTooLargeError{Path: rd.path, At: rd.in.limit - MaxValueBytes} + case errors.Is(err, io.ErrUnexpectedEOF): + return &SchemaError{Path: rd.path, Detail: core.Says("manifest.ItEndsPartWay", "it ends part way through the document, so it was cut short")} + case errors.As(err, &syntax), errors.As(err, &mistyped): + return &SchemaError{Path: rd.path, Detail: core.SaidOf(err)} + } + return err +} + +// document reads the one value a manifest is, and asks that nothing follows it. +func (rd *reading) document(m *Manifest, each func(File)) error { + rd.next() + tok, err := rd.dec.Token() + if errors.Is(err, io.EOF) { + return &SchemaError{Path: rd.path, Detail: core.Says("manifest.ItHoldsNoDocument", "it holds no JSON document at all")} + } + if err := rd.unreadable(err); err != nil { + return err + } + switch tok { + case nil: + // null, which encoding/json reads into a manifest as nothing at all - + // so it goes on to be refused for carrying no version, as it was. + case json.Delim('{'): + if err := rd.object(m, each); err != nil { + return err + } + default: + return &SchemaError{Path: rd.path, Detail: core.Says("manifest.ItIsNotAnObject", "it is a JSON %s rather than an object, so it is not a manifest this tool wrote", core.A("Kind", kindOf(tok)))} + } + return rd.end() +} + +// end asks what follows the document, which may be white space and nothing +// else. Two documents in one file are not a manifest, and the decoder on its +// own stops where the first one closes. +func (rd *reading) end() error { + rd.next() + _, err := rd.dec.Token() + if errors.Is(err, io.EOF) { + return nil + } + var syntax *json.SyntaxError + if err == nil || errors.As(err, &syntax) { + return &SchemaError{Path: rd.path, Detail: core.Says("manifest.SomethingFollowsIt", "something follows the end of the document, and a manifest is one JSON object with nothing after it but white space")} + } + return rd.unreadable(err) +} + +// object reads the keys of the outermost object, the entries one at a time +// and every other key whole. +func (rd *reading) object(m *Manifest, each func(File)) error { + seen := map[string]string{} + for rd.more() { + if err := rd.ctx.Err(); err != nil { + return err + } + tok, err := rd.token() + if err != nil { + return err + } + key, _ := tok.(string) + if name, known := outermostKey(key); known { + if first, twice := seen[name]; twice { + return rd.twice(first, key) + } + seen[name] = key + } + if strings.EqualFold(key, filesKey) { + err = rd.entries(m, each) + } else { + err = rd.field(m, key) + } + if err != nil { + return err + } + } + _, err := rd.token() + return err +} + +// twice is the refusal of a document that names one key two times. +// +// encoding/json keeps the second, so a document with two lists of files was +// acted on by its second list, and a reader going a piece at a time would read +// the first, check it and throw it away to do the same. This tool writes every +// key once, so a document with two was written or edited by something else, +// and which half of it was meant is a guess (decided by the owner on +// 2026-11-04, docs/MANIFEST-STREAMING-2026-11-04.md section 4.3). A key +// spelled another way is the same key, because encoding/json reads it as one. +func (rd *reading) twice(first, second string) error { + detail := core.Says("manifest.ItHoldsTheKeyTwice", "it holds the key %q twice", core.A("Key", first)) + if first != second { + detail = core.Says("manifest.ItHoldsTheKeyTwiceSpelled", "it holds the key %q twice, the second time spelled %q", core.A("Key", first), core.A("Second", second)) + } + return &SchemaError{Path: rd.path, Detail: core.Says("manifest.TwiceBecause", + "%s. This tool writes every key once, so the manifest was written or edited by something else, and which of the two it means is a guess. "+ + "Use the manifest the run actually wrote", core.A("Detail", detail))} +} + +// field reads one key other than files into m, through encoding/json - so the +// key is matched to a field, and the value held to the type of the field, +// exactly the way a document read whole is. A key no field has is read and +// let go, as it always was. +func (rd *reading) field(m *Manifest, key string) error { + var raw json.RawMessage + if err := rd.value(&raw); err != nil { + return err + } + name, err := json.Marshal(key) + if err != nil { + return err + } + doc := make([]byte, 0, len(name)+len(raw)+3) + doc = append(append(append(append(append(doc, '{'), name...), ':'), raw...), '}') + return rd.unreadable(json.Unmarshal(doc, m)) +} + +// entries reads the list of files an entry at a time, checking where each one +// lands as it passes and handing it on. +func (rd *reading) entries(m *Manifest, each func(File)) error { + tok, err := rd.token() + if err != nil { + return err + } + switch tok { + case nil: + // null, which encoding/json reads as no list at all. + return nil + case json.Delim('['): + default: + return &SchemaError{Path: rd.path, Detail: core.Says("manifest.FilesIsNotAList", "files is a JSON %s rather than a list", core.A("Kind", kindOf(tok)))} + } + // Present and empty until something arrives, which is how encoding/json + // leaves a list that was there - and how Encode tells [] from null. + m.Files = []File{} + for i := 1; rd.more(); i++ { + // Asked once an entry, because a manifest of a million entries takes + // seconds, and Ctrl+C in the middle of them has to be answered then + // rather than at the end. + if err := rd.ctx.Err(); err != nil { + return err + } + var f File + if err := rd.value(&f); err != nil { + return err + } + if rd.escaped == nil { + rd.escaped = entryEscapes(rd.path, i, f.Path) + } + each(f) + } + _, err = rd.token() + return err +} + +// kindOf is what a JSON token that opens or is a value is called in a refusal. +func kindOf(tok json.Token) string { + switch tok { + case json.Delim('['): + return "list" + case json.Delim('{'): + return "object" + } + switch tok.(type) { + case string: + return "string" + case float64: + return "number" + case bool: + return "true or false" + } + return "value" +} + +// filesKey is the key the entries are under. +const filesKey = "files" + +// outermostKeys is every key of the outermost object that encoding/json reads +// into a field of Manifest - taken from the type, so a field added to it is +// held to the rule about naming a key twice without anybody listing it here. +var outermostKeys = func() []string { + var keys []string + t := reflect.TypeFor[Manifest]() + for i := range t.NumField() { + f := t.Field(i) + name, _, _ := strings.Cut(f.Tag.Get("json"), ",") + if !f.IsExported() || name == "-" { + continue + } + if name == "" { + name = f.Name + } + keys = append(keys, name) + } + return keys +}() + +// outermostKey is the field a key of the outermost object is read into, under +// the same rule encoding/json matches keys by, and whether there is one. +func outermostKey(key string) (string, bool) { + for _, name := range outermostKeys { + if strings.EqualFold(key, name) { + return name, true + } + } + return "", false +} + +// ClaimedFile is one file a manifest says is on the disk, and all that verify and +// cleanup need of it. +type ClaimedFile struct { + Path string + SHA256 string + Bytes int64 +} + +// Claims is what a manifest says is on the disk, in the order it lists the +// files, and the name of the instructions it names, which both commands treat +// as the run's own. +type Claims struct { + Files []ClaimedFile + Instructions string +} + +// claimed says whether an entry is a file the manifest says is on the disk. +// +// Two kinds of entry are deliberately not claimed. An entry marked failed +// describes a file that was never written - reporting it missing would be a +// false alarm about the one thing the manifest was honest about. And an entry +// that was not materialised was never meant to reach the disk at all. +// +// This is the single reading of the manifest. Both commands come through it, +// so neither can drift into claiming something the other does not. +func (f File) claimed() bool { return !f.Failed && f.Materialized } + +func claimOf(f File) ClaimedFile { + return ClaimedFile{Path: f.Path, SHA256: f.Hashes.SHA256, Bytes: f.Bytes} +} + +// Claims is what this manifest says is on the disk - what ReadClaims gives, +// for a manifest already held whole. +func (m *Manifest) Claims() Claims { + c := Claims{Instructions: m.Run.Instructions} + for _, f := range m.Files { + if f.claimed() { + c.Files = append(c.Files, claimOf(f)) + } + } + return c +} + +// ReadClaims reads what the manifest at path says is on the disk, for verify +// and cleanup, a piece at a time. +// +// most is about how much memory checking what it claims may take, from the +// memory setting, and nought is no limit. A manifest past it is refused once +// the whole document has been read and found sound, so the refusal can say +// how much it would have taken and a document with a fault of its own is +// refused for that fault - nothing is held of it past the limit in the +// meantime. +func ReadClaims(ctx context.Context, path string, most int64) (Claims, error) { + var claims Claims + var need int64 + files := 0 + instructions, err := EachClaim(ctx, path, func(c ClaimedFile) { + files++ + need += checkingCost(c.Path, c.SHA256) + if most > 0 && need > most { + // The rest is counted and let go, so the refusal can say how + // much it would take without the reader holding it. + claims.Files = nil + return + } + claims.Files = append(claims.Files, c) + }) + if err != nil { + return Claims{}, err + } + if most > 0 && need > most { + return Claims{}, &TooMuchToHoldError{Path: path, Files: files, Need: need, Most: most} + } + claims.Instructions = instructions + return claims, nil +} + +// EachClaim reads the manifest at path a piece at a time and hands every file +// it claims to keep, holding none of them itself, and gives back the name of +// the instructions it names. +// +// keep runs before the document has been found sound - see decode - so a +// caller acts on nothing it was handed unless EachClaim returns no error. For +// a caller that keeps a few of the claims out of many, which is what the +// records of the other runs in a directory are to verify. +func EachClaim(ctx context.Context, path string, keep func(ClaimedFile)) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", err + } + defer func() { _ = f.Close() }() + + var m Manifest + err = decode(ctx, bufio.NewReaderSize(f, readBuffer), path, &m, func(e File) { + if e.claimed() { + keep(claimOf(e)) + } + }) + if err != nil { + return "", err + } + return m.Run.Instructions, nil +} + +// checkingCost is about how much memory checking one claimed file takes in +// verify or cleanup: the claim held, and what the checking makes of it. +// +// The claim is a ClaimedFile and its two strings - measured at 134 B for a path of +// 17 characters, and 48 B and the two lengths come to 129 (section 8 of the +// analysis). The checking holds about as much again for each, and twice that is +// allowed for the collector's room. +func checkingCost(path, sum string) int64 { + return checkingFactor * (claimBytes + int64(len(path)) + int64(len(sum))) +} + +// claimBytes is what a claim takes besides the bytes of its two strings. +const claimBytes = 48 + +// checkingFactor is how many times its claims checking a manifest takes. +const checkingFactor = 3 + +// TooMuchToHoldError is a manifest listing more files than verify or cleanup +// may hold under the memory setting. +type TooMuchToHoldError struct { + Path string + Files int + // Need is about how much checking the files takes, and Most what the + // memory setting gives the command. + Need, Most int64 +} + +func (e *TooMuchToHoldError) Error() string { return e.Said().String() } + +// Said is the refusal, for a window that says it in its own language. +func (e *TooMuchToHoldError) Said() core.Said { + return core.Says("manifest.TooMuchToHold", + "%s lists %s, and checking them takes about %s of memory, more than the %s the memory setting gives this command. "+ + "Give it more through the memory setting, a size such as 8gb or off for no limit, or split the run it describes", + core.A("Path", e.Path), core.A("Files", core.SaysN("manifest.Files", "%d file", "%d files", core.A("Count", e.Files))), + core.A("Need", core.Bytes(e.Need)), core.A("Most", core.Bytes(e.Most))) +} From 5ede9e8bad8533ad339323f7d116de0380ea01c6 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 4 Nov 2026 08:44:34 +0100 Subject: [PATCH 2/3] The manifest past 16 MB is verified and cleaned up, and the read-back warning goes With no ceiling on the length of the manifest left, nothing is too large to read back: summary.too_large_to_read_back is no longer written, and the warning about it is gone from the command line and the window. The forecast the command line made only for that warning goes with it. generate --json writes the manifest out a part at a time instead of holding a second copy of it in memory. The cost a claimed file is counted at against --memory is measured: verify and cleanup --yes read the manifest of 600 000 files in a container of 1 GB. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 32 ++- internal/cli/generate.go | 20 +- internal/cli/record.go | 123 ++++------ internal/engine/engine.go | 21 +- internal/engine/record.go | 5 +- internal/engine/recordsize.go | 20 +- internal/guard/manifestreach_test.go | 256 +++++--------------- internal/guard/manifeststream_test.go | 59 +++++ internal/guard/mutation/entries.py | 267 ++++++++++++--------- internal/guard/windowmanifestreach_test.go | 209 ---------------- internal/gui/text/locale/en.json | 5 - internal/gui/text/locale/pl.json | 1 - internal/gui/text/locale/said/en.json | 50 ++++ internal/gui/text/locale/said/pl.json | 10 + internal/gui/text/text.go | 17 -- internal/gui/window/run.go | 18 +- internal/gui/window/runreport.go | 27 --- internal/manifest/manifest.go | 58 ++--- internal/manifest/read.go | 35 ++- internal/manifest/reservation.go | 7 +- internal/manifest/size.go | 62 ++--- 21 files changed, 492 insertions(+), 810 deletions(-) delete mode 100644 internal/guard/windowmanifestreach_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 578b8169..5def03a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,12 @@ because it turns other people's test suites red. licences are the same. `THIRD-PARTY-NOTICES.md` names the new files and their sums. +- **`tfg verify` and `tfg cleanup` refuse a manifest that names a key twice + or has anything after it but white space.** This tool never writes either. + A manifest with two lists of files used to be acted on by the second one, + and now nothing in it is acted on. The refusal names the key, and ends with + exit code 5 like any manifest that cannot be read. + ### Fixed - **Cancel answers at once, and closing the window during a run no longer @@ -124,13 +130,25 @@ 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. +- **`tfg verify` and `tfg cleanup` read the manifest of a run of any size.** + They refused a manifest larger than 16 MB, about 20 000 files, so the files + of a larger run were files `tfg cleanup` could never remove. The manifest is + now read a part at a time: one value in it, such as one file's entry, may + take at most 16 MB, and the document as a whole has no limit. In a + container of 1 GB, `verify` and `cleanup --yes` work through the manifest of + 600 000 files in 10 and 24 seconds, holding at most about 280 MB. Both + commands take `--memory`, the same setting `generate` takes, and a + manifest listing more files than it lets them check is refused before any + file is looked at, with the number of files and how much they would take. + Ctrl+C while the manifest is read stops at once. The records of other runs + in the same directory are read the same way, so the files of a large + neighbour are no longer reported as `extra`. With nothing left that is too + large to read back, the warning about it is gone from both the command line + and the window, and new manifests no longer carry + `summary.too_large_to_read_back`. +- **`generate --json` no longer holds a second copy of the manifest in memory + to print it.** It is written out a part at a time, the same bytes as the + saved manifest. - **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 diff --git a/internal/cli/generate.go b/internal/cli/generate.go index 6d05f946..a0fe3e87 100644 --- a/internal/cli/generate.go +++ b/internal/cli/generate.go @@ -428,8 +428,6 @@ func produce(ctx context.Context, targets []engine.Target, opt engine.Options, g core.ExactBytes(engine.TotalBytes(planned))) echoBoundaries(targets, planned, errOut) - forecast, warned := echoManifestReach(ctx, planned, opt, errOut) - opt.Record = forecast echoHold(planned, opt, errOut) if g.dryRun { @@ -462,31 +460,21 @@ 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, warned, errOut) + saveManifest(res, opt, errOut) } return classify(runErr) } if !g.dryRun { - if code := saveManifest(res, opt, warned, errOut); code != ExitOK { + if code := saveManifest(res, opt, errOut); code != ExitOK { return code } } // Data goes to standard output and only on success. if g.asJSON { - var buf bytes.Buffer - if err := res.Manifest.Encode(&buf); err != nil { - fmt.Fprintf(errOut, "tfg: cannot render the manifest: %s\n", describeError(err)) - return ExitRuntime - } - // The error is read rather than dropped. A closed pipe is the ordinary - // way this fails - "tfg generate --json | head" - and a run that could - // not deliver what was asked for has not succeeded, whatever happened - // on the disk. - if _, err := out.Write(buf.Bytes()); err != nil { - fmt.Fprintf(errOut, "tfg: the files were written and the manifest could not be sent to standard output: %s\n", describeError(err)) - return ExitIO + if code := sendManifest(res, g.dryRun, out, errOut); code != ExitOK { + return code } } diff --git a/internal/cli/record.go b/internal/cli/record.go index d387067e..a42e51ae 100644 --- a/internal/cli/record.go +++ b/internal/cli/record.go @@ -2,8 +2,8 @@ package cli // What a run leaves about itself once the files are written: the manifest and -// the instructions beside it, and what is said when a run stopped or its record -// will be too large to read back. Out of generate.go on 2026-09-25, when the +// the instructions beside it, what is said when a run stopped, and the +// manifest --json sends. Out of generate.go on 2026-09-25, when the // instructions made that file the largest in the tree. import ( @@ -14,7 +14,6 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/engine" - "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) // whatSurvived says what a stopped run left behind, for the sentence produce @@ -66,53 +65,14 @@ func whatSurvived(runErr error, res *engine.Result) string { core.Count(len(res.Manifest.Files), "file", "files")) } -// echoManifestReach says so when a run will write a manifest this build cannot -// read back. -// -// Measured on 2026-08-26: a run of 25 000 files wrote a manifest of 25 220 640 -// B against a read ceiling of 16 777 216 B, and from that point "tfg verify" -// and "tfg cleanup" both exited 5 on it. The files stayed on the disk and -// nothing in this toolset could remove them, because the manifest is the only -// authority over what may be deleted. -// -// A note rather than a refusal, by the owner's decision on the same day: the -// run itself works, and refusing it would take away something this tool does -// 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. -// -// 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 - } - 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(record.Manifest), core.HumanBytes(manifest.MaxBytes)) - return &record, true -} - // echoHold says so when a run will hold more than its memory setting gives it // (O296, engine.PredictHold says why and what was measured). // // Before the first byte, and that is the point of it: a run the system ends // prints nothing afterwards, so the sentence has to be in the log already. A // note rather than a refusal, because the same run finishes where there is swap -// to carry it. And on a dry run, for the reason echoManifestReach gives. +// to carry it. And on a dry run, which is the step this project tells people +// to take before anything large. func echoHold(planned []engine.PlannedFile, opt engine.Options, errOut io.Writer) { hold := engine.PredictHold(len(planned), opt) if !hold.Over { @@ -125,35 +85,9 @@ func echoHold(planned []engine.PlannedFile, opt engine.Options, errOut io.Writer core.HumanBytes(hold.Holds), core.HumanBytes(hold.Room)) } -// 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. -// -// 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 { +func saveManifest(res *engine.Result, opt engine.Options, 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. @@ -191,6 +125,51 @@ func saveManifest(res *engine.Result, opt engine.Options, warned bool, errOut io "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 } + +// sendManifest puts the manifest on standard output for --json, a part at a +// time as Encode writes it. +// +// It used to be encoded whole into memory a second time, beside every entry +// the run still held, and handed over in one write - for a run whose manifest +// was already on the disk (observation O300, +// docs/MANIFEST-STREAMING-2026-11-04.md section 9). Encoded again rather than +// copied from the disk, because Encode streams and writes the same bytes the +// saved file holds, and a copy would be a second way for the two to differ - +// a file somebody replaced in between would be sent as this run's. +func sendManifest(res *engine.Result, dryRun bool, out, errOut io.Writer) int { + sent := &watched{w: out} + err := res.Manifest.Encode(sent) + switch { + case err == nil: + return ExitOK + case sent.err == nil: + fmt.Fprintf(errOut, "tfg: cannot render the manifest: %s\n", describeError(err)) + return ExitRuntime + case dryRun: + fmt.Fprintf(errOut, "tfg: the manifest could not be sent to standard output: %s\n", describeError(err)) + return ExitIO + } + // The error is read rather than dropped. A closed pipe is the ordinary + // way this fails - "tfg generate --json | head" - and a run that could + // not deliver what was asked for has not succeeded, whatever happened + // on the disk. + fmt.Fprintf(errOut, "tfg: the files were written and the manifest could not be sent to standard output: %s\n", describeError(err)) + return ExitIO +} + +// watched passes writes on and keeps the first that failed, so a failure to +// write can be told from a failure to render. +type watched struct { + w io.Writer + err error +} + +func (s *watched) Write(p []byte) (int, error) { + n, err := s.w.Write(p) + if err != nil && s.err == nil { + s.err = err + } + return n, err +} diff --git a/internal/engine/engine.go b/internal/engine/engine.go index ff094b0b..2ebcb8e0 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -156,14 +156,6 @@ type Options struct { // two gigabytes without owning one. MachineMemory func() (int64, error) - // 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. // @@ -225,12 +217,6 @@ type Result struct { // with the machine would record something other than the files beside it. RunNotes []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 - // Hold is what this run was worked out to hold at its end, against what // its memory setting gives it (PredictHold), before its first byte. A // window says it after a preview and after a run, and the command line @@ -518,11 +504,10 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) // 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. - record, err := opt.forecast(ctx, files) + record, err := PredictRecord(ctx, files, opt) if err != nil { return res, err } - res.Record = record if err := preflight(ctx, files, opt, record); err != nil { return res, err } @@ -539,10 +524,6 @@ 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/record.go b/internal/engine/record.go index 3a07de53..5bfce7f2 100644 --- a/internal/engine/record.go +++ b/internal/engine/record.go @@ -20,8 +20,7 @@ type Record struct { // 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. + // beforehand (PredictRecord is the forecast, this is the fact). Size RecordSize } @@ -77,7 +76,7 @@ func SaveRecord(res *Result, opt Options) (Record, error) { } return rec, err } - rec.Size.Manifest, rec.Size.TooLarge = m.SavedBytes(), m.Summary.TooLargeToReadBack + rec.Size.Manifest = m.SavedBytes() return rec, nil } diff --git a/internal/engine/recordsize.go b/internal/engine/recordsize.go index 8036704a..ac504751 100644 --- a/internal/engine/recordsize.go +++ b/internal/engine/recordsize.go @@ -17,15 +17,13 @@ import ( // 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. +// rule 7). And, until 2026-11-04, the warning that a manifest would be too +// large to read back, which went with the ceiling it was about. // 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 @@ -40,8 +38,7 @@ 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. +// failures writes a little more than this says. // // 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, @@ -75,7 +72,7 @@ func PredictRecord(ctx context.Context, files []PlannedFile, opt Options) (Recor return RecordSize{}, err } } - out.Manifest, out.TooLarge, err = counted.Bytes() + out.Manifest, err = counted.Bytes() return out, err } @@ -101,15 +98,6 @@ func askedToStop(ctx context.Context, i int) error { 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) { - 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. diff --git a/internal/guard/manifestreach_test.go b/internal/guard/manifestreach_test.go index b7ca6f99..f4d43617 100644 --- a/internal/guard/manifestreach_test.go +++ b/internal/guard/manifestreach_test.go @@ -4,9 +4,8 @@ import ( "bytes" "context" "encoding/json" - "errors" + "fmt" "os" - "path/filepath" "strings" "testing" @@ -16,57 +15,69 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) -// A run whose manifest this build cannot read back has to say so. +// The manifest of a run past the ceiling the whole document once had is one +// verify and cleanup read. // -// Two ceilings were chosen independently and never held against each other. -// core.MaxFilesPerRun is a million files. manifest.MaxBytes is sixteen -// megabytes. +// From 2026-08-03 until 2026-11-04 a manifest was read whole under sixteen +// megabytes, so a run of more than about twenty thousand files wrote one both +// commands refused - measured on 2026-08-26 at 25 000 files, generate ended 0 +// and verify and cleanup 5, and the files were files nothing in this toolset +// could remove. For two months the answer was a note saying so, in both +// surfaces and in a field of the manifest (O184). Now the reader takes the +// document a piece at a time, the note and the field are gone, and this is the +// run they were about: the manifest a run wrote, past the old ceiling, read back +// by both commands. // -// 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 -// with "is 25220640 B and the limit is 16777216 B". The files stayed on the -// disk and nothing in this toolset could remove them - the manifest is the only -// authority over what may be deleted, so a manifest that cannot be read is a -// set of files with no owner. -// -// 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. - -// 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)) +// The run is three files and the length is copies of its own entries, each +// marked as never written, because writing twenty thousand files takes the +// better part of a minute on Windows and the length is what is asked about. +func TestTheManifestOfARunPastTheOldCeilingIsVerifiedAndCleanedUp(t *testing.T) { + out, mf := generated(t) + keys := keysOf(t, mf) + lengthened := false + for i, k := range keys { + if k.key != "files" { + continue + } + var entries []map[string]any + if err := json.Unmarshal(k.value, &entries); err != nil || len(entries) == 0 { + t.Fatalf("the run's list of files does not read as entries: %v", err) + } + model, err := json.Marshal(entries[0]) + if err != nil { + t.Fatal(err) + } + for n := 0; n <= manifest.MaxBytes/len(model); n++ { + copied := map[string]any{} + for field, value := range entries[0] { + copied[field] = value + } + name := fmt.Sprintf("never_%07d.txt", n) + copied["id"], copied["path"], copied["name"], copied["materialized"] = name, name, name, false + entries = append(entries, copied) + } + if keys[i].value, err = json.Marshal(entries); err != nil { + t.Fatal(err) + } + lengthened = true } - 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) + if !lengthened { + t.Fatal("the run's manifest has no list of files to lengthen") } - if !strings.Contains(said, "manifest") { - t.Errorf("the note does not say what will be too big:\n%s", said) + rewritten(t, mf, keys) + // The state this is about: past the length the old reader refused. + if info, err := os.Stat(mf); err != nil || info.Size() <= manifest.MaxBytes { + t.Fatalf("the manifest is not past %d B: %v %v", manifest.MaxBytes, info, err) } -} - -// And a run comfortably under it stays quiet. -// -// Without this the test above passes on a build that warns about every run, -// which teaches people to skip the line - the same reason the site probe skips -// pages carrying noindex rather than reporting noise. -func TestAnOrdinaryRunSaysNothingAboutTheManifestCeiling(t *testing.T) { - said := runCLI(t, "generate", "--format", "txt", "--size", "200b", - "--count", "100", "--dry-run", "--out", t.TempDir()) - if strings.Contains(said, "too big") || strings.Contains(said, "will not be able to read") { - t.Errorf("a hundred files drew a warning about the manifest ceiling:\n%s", said) + if code, stdout, errOut := run(t, "verify", mf); code != cli.ExitOK || !strings.Contains(stdout, "3 files checked") { + t.Errorf("verify on the manifest of a run past the old ceiling ended %d:\n%s%s", code, stdout, errOut) + } + if code, _, errOut := run(t, "cleanup", mf, "--yes"); code != cli.ExitOK { + t.Errorf("cleanup on the manifest of a run past the old ceiling ended %d:\n%s", code, errOut) + } + if n := textFilesIn(t, out); n != 0 { + t.Errorf("cleanup left %d of the three files the manifest lists", n) } } @@ -127,158 +138,13 @@ 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. -// -// The note above is correct, printed first, and was the whole of the answer -// until 2026-09-06. Two measurements say why that was not enough. On stderr it -// was one line among 25 003, carrying the same "note:" prefix as 25 000 -// repetitions of a sentence about labels - grouped since, so it is now one of -// two. And in the machine readable report it was ABSENT ENTIRELY: a run of -// 25 000 files answered exit 0 with a clean --json and no trace of the warning -// anywhere in it. -// -// This tool plugs into CI. A fact that only a person reading prose can learn is -// a fact a pipeline cannot act on, and the thing it cannot act on here is that -// the directory it just filled can never be verified or cleaned up by this -// build - untouchable rule 7 makes the manifest the only authority over what -// may be deleted. -// -// NOT AN EXIT CODE, and that was decided rather than skipped. docs/CLI.md -// defines 8 as "run finished, but not everything was produced", and here -// everything was produced. The sentence would read "0 files could not be -// produced" and point at a manifest nothing can read. Exit codes are a frozen -// contract. -func TestAManifestTooBigToReadBackSaysSoInTheReportAScriptReads(t *testing.T) { - // 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, 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) - } - - // Read out of the document a consumer receives rather than off the struct. - // Decoding into manifest.Summary would pass by construction the moment the - // type changed, which is the trap manifestShape in recipe_test.go exists - // against. - var report struct { - Summary map[string]any `json:"summary"` - } - if err := json.Unmarshal([]byte(said), &report); err != nil { - t.Fatalf("the report is not readable as JSON: %v\n%s", err, firstLines(said, 4)) - } - flag, present := report.Summary["too_large_to_read_back"] - if !present { - t.Fatalf("the report carries no too_large_to_read_back at all, so a script has "+ - "no way to learn that this run cannot be verified or cleaned up:\nsummary: %v", - report.Summary) - } - if flag != true { - t.Errorf("too_large_to_read_back is %v on a run that is over the ceiling", flag) - } -} - -// An ordinary run does not carry the field at all. -// -// The sharp half. A build writing "too_large_to_read_back": false into every -// manifest would satisfy the guard above and change the bytes of every document -// this tool has ever written, for a fact that is almost never worth saying. It -// is absent rather than false, which is what keeps existing manifests byte for -// byte what they were - the same choice recipe_hash, overrides and preset made. -func TestAnOrdinaryRunSaysNothingAboutTheCeilingInItsReport(t *testing.T) { - code, said, errOut := run(t, "generate", "--format", "txt", "--size", "200b", - "--count", "20", "--dry-run", "--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) - } - - var report struct { - Summary map[string]any `json:"summary"` - } - if err := json.Unmarshal([]byte(said), &report); err != nil { - t.Fatalf("the report is not readable as JSON: %v\n%s", err, firstLines(said, 4)) - } - // Asserted rather than assumed, so an empty summary cannot pass this by - // having no keys at all. - if len(report.Summary) == 0 { - t.Fatal("the report carries no summary, so this guard checked nothing") - } - if _, present := report.Summary["too_large_to_read_back"]; present { - t.Errorf("an ordinary run carries too_large_to_read_back in its manifest.\n" + - "Absent rather than false is what keeps every manifest already written " + - "byte for byte what it was.") - } -} - -// 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. +// The forecast is what the free space is held against. A forecast that +// drifted from the record would put that back where the old constant had +// it, wrong in a direction nobody sees. func TestTheRecordARunIsToldBeforehandIsTheRecordItWrites(t *testing.T) { dir := t.TempDir() opt := engine.Options{OutDir: dir, Seed: 7, Command: "test"} diff --git a/internal/guard/manifeststream_test.go b/internal/guard/manifeststream_test.go index 0003892a..32c6ef14 100644 --- a/internal/guard/manifeststream_test.go +++ b/internal/guard/manifeststream_test.go @@ -2,11 +2,15 @@ package guard import ( "bytes" + "context" "encoding/json" "fmt" + "os" + "path/filepath" "strings" "testing" + "github.com/donislawdev/TestingFilesGenerator/internal/cli" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) @@ -74,6 +78,61 @@ func (w *writeSizes) Write(p []byte) (int, error) { return len(p), nil } +// keptSizes is writeSizes that also keeps what it was handed. +type keptSizes struct { + writeSizes + kept bytes.Buffer +} + +func (w *keptSizes) Write(p []byte) (int, error) { + _, _ = w.writeSizes.Write(p) + return w.kept.Write(p) +} + +// generate --json sends the manifest a part at a time too, and after a run what +// it sends is the file the run saved. +// +// It used to encode the whole document into memory a second time to send it, +// beside every entry the run still held, for a run whose manifest was on the +// disk already - observation O300. Both halves: a dry run, which saves nothing, +// and a real run, whose manifest is on the disk - and what that one sends is +// the saved file to the byte, so what a script reads is what verify reads. +func TestGenerateSendsItsManifestAPartAtATime(t *testing.T) { + for _, dry := range []bool{true, false} { + dir := t.TempDir() + args := []string{"generate", "--format", "txt", "--size", "1b", "--count", "800", "--json", "--out", dir} + if dry { + args = append(args, "--dry-run") + } + var sent keptSizes + var errOut bytes.Buffer + if code := cli.Run(context.Background(), args, &sent, &errOut); code != cli.ExitOK { + t.Fatalf("dry run %v: generate ended %d:\n%s", dry, code, errOut.String()) + } + // The question only means something for a document many writes long. + if sent.total < 512<<10 { + t.Fatalf("dry run %v: the manifest sent is only %d B - too short to tell a whole write from a streamed one", dry, sent.total) + } + if sent.largest*8 > sent.total { + t.Errorf("dry run %v: a manifest of %d B reached standard output in %d writes, the largest %d B - held whole to be sent", + dry, sent.total, sent.writes, sent.largest) + } + if dry { + if !json.Valid(sent.kept.Bytes()) { + t.Errorf("the manifest a dry run sent is not a JSON document") + } + continue + } + saved, err := os.ReadFile(filepath.Join(dir, "manifest.json")) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(saved, sent.kept.Bytes()) { + t.Errorf("the manifest sent to standard output (%d B) is not the file the run saved (%d B)", sent.total, len(saved)) + } + } +} + // encodingJSON is the manifest as encoding/json renders it in one piece, with // the indent Encode has always used. func encodingJSON(t *testing.T, m *manifest.Manifest) []byte { diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index 5f91e058..9abb8097 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -48,29 +48,6 @@ "and nothing else for the command line tool;", "TestProseInTheRepositoryUsesFlatHyphensAndNoSemicolons", ), - # --- the window says what the command line says about the manifest ceiling - # (2026-09-07, O184) --- - ( - "the preview stops warning that the record will be too big to read back", - "internal/gui/window/run.go", - "r.say(append(append(append([]string{said}, manifestReachNote(res.Record)...), holdNote(res)...), runNotesOf(res)...)...)", - "r.say(append(append([]string{said}, holdNote(res)...), runNotesOf(res)...)...)", - "TestTheWindowSaysWhenItsManifestWillBeTooBigToReadBack", - ), - ( - "a finished run stops warning, so somebody who never pressed Preview is told nothing", - "internal/gui/window/run.go", - "said := append(append([]string{outcome}, manifestReachNote(saved.Size)...), holdNote(res)...)", - "said := append([]string{outcome}, holdNote(res)...)", - "TestAFinishedRunInTheWindowSaysItsManifestIsTooBigToReadBack", - ), - ( - "the window warns about every run, which teaches people to skip the line", - "internal/gui/window/runreport.go", - "if !record.TooLarge {", - "if false && !record.TooLarge {", - "TestAnOrdinaryPreviewSaysNothingAboutTheManifestCeiling", - ), ( "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", @@ -117,8 +94,8 @@ ( "the attribution stops naming which run, so the reader is told somebody else's and not whose", "internal/audit/neighbours.go", - "base := filepath.Base(rel)", - 'base := "a run"', + "found.take(claims, filepath.Base(rel))", + 'found.take(claims, "a run")', "TestTheSentenceAboutAnotherRunNamesTheRecordThatClaimsTheFile", ), ( @@ -239,21 +216,6 @@ "go install github.com/donislawdev/TestingFilesGenerator/cmd/tfg@latest", "TestTheInstallInstructionsCarryTheBuildTags", ), - # --- 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/engine/engine.go", - "m.Summary.TooLargeToReadBack = record.TooLarge", - "_ = record.TooLarge", - "TestAManifestTooBigToReadBackSaysSoInTheReportAScriptReads", - ), - ( - "every manifest carries the ceiling field, changing the bytes of all of them", - "internal/manifest/manifest.go", - 'TooLargeToReadBack bool `json:"too_large_to_read_back,omitempty"`', - 'TooLargeToReadBack bool `json:"too_large_to_read_back"`', - "TestAnOrdinaryRunSaysNothingAboutTheCeilingInItsReport", - ), # --- a stopped run says so in our own words (2026-09-06) --- ( "a cancelled run goes back to printing Go's own vocabulary", @@ -1299,12 +1261,10 @@ "TestOurOwnGeneratorsHaveNotDrifted", ), ( - # Untouchable rule 7. If this one ever goes green, the tool deletes - # files nobody listed, on somebody else's disk, with no undo. "cleanup walks the directory instead of the manifest", - "internal/audit/cleanup.go", - "claimed := Claimed(m)", - "claimed := m.Files", + "internal/manifest/read.go", + "func (f File) claimed() bool { return !f.Failed && f.Materialized }", + "func (f File) claimed() bool { return true }", "TestCleanupDoesNotRemoveAFileTheRunNeverPutOnTheDisk", ), ( @@ -1453,7 +1413,7 @@ ( "verify never looks at content, so an edited file passes", "internal/audit/audit.go", - "if got.sum != f.Hashes.SHA256 {", + "if got.sum != f.SHA256 {", "if false {", "TestVerifyCatchesAMissingAnExtraAndAChangedFile", ), @@ -1465,19 +1425,19 @@ "TestVerifyCatchesAMissingAnExtraAndAChangedFile", ), ( - "verify counts a file that was never written as missing", # A failed entry describes a file the run reported as not produced. # Claiming it would turn the manifest's own honesty into a false alarm. - "internal/audit/audit.go", - "if f.Failed || !f.Materialized {", - "if false {", + "verify counts a file that was never written as missing", + "internal/manifest/read.go", + "func (f File) claimed() bool { return !f.Failed && f.Materialized }", + "func (f File) claimed() bool { return f.Materialized }", "TestVerifyDoesNotGoLookingForFilesTheRunSaidItNeverWrote", ), ( "verify treats the manifest itself as a file nobody asked for", "internal/audit/audit.go", - "if seen[p] || filepath.Base(p) == skip || (m.Run.Instructions != \"\" && filepath.Base(p) == m.Run.Instructions) {", - "if seen[p] || (m.Run.Instructions != \"\" && filepath.Base(p) == m.Run.Instructions) {", + "if seen[p] || filepath.Base(p) == skip || (c.Instructions != \"\" && filepath.Base(p) == c.Instructions) {", + "if seen[p] || (c.Instructions != \"\" && filepath.Base(p) == c.Instructions) {", "TestVerifyDoesNotReportTheManifestAsAnExtraFile", ), ( @@ -1681,9 +1641,9 @@ # "tfg generate --json | head" is the ordinary way this happens, and # the error was dropped on the floor. "a manifest that never reached standard output is reported as a success", - "internal/cli/generate.go", - "if _, err := out.Write(buf.Bytes()); err != nil {", - "if _, err := out.Write(buf.Bytes()); err != nil && false {", + "internal/cli/record.go", + "\tcase err == nil:\n\t\treturn ExitOK\n", + "\tcase err == nil || sent.err != nil:\n\t\treturn ExitOK\n", "TestAManifestThatCannotReachStandardOutputIsNotASuccess", ), ( @@ -2082,8 +2042,8 @@ ( "verify reports a clean directory instead of refusing an entry that leaves it", "internal/audit/audit.go", - "p, err := resolved(b, f)\n\t\tif err != nil {\n\t\t\treturn nil, err\n\t\t}", - "p, err := resolved(b, f)\n\t\tif err != nil {\n\t\t\treturn full, nil\n\t\t}", + "p, err := resolved(b, f.Path)\n\t\tif err != nil {\n\t\t\treturn nil, err\n\t\t}", + "p, err := resolved(b, f.Path)\n\t\tif err != nil {\n\t\t\treturn full, nil\n\t\t}", "TestVerifyDoesNotFollowALinkOutOfTheDirectory", ), ( @@ -2468,8 +2428,8 @@ # compiles - a mutation that does not build proves nothing. "the boundary is worked out again for every entry", "internal/audit/audit.go", - " p, err := resolved(b, f)", - " p, err := resolved(core.NewBoundary(b.Dir()), f)", + " p, err := resolved(b, f.Path)", + " p, err := resolved(core.NewBoundary(b.Dir()), f.Path)", "TestTheBoundaryIsWorkedOutOncePerPassAndNotOncePerEntry", ), ( @@ -2504,11 +2464,11 @@ "TestAnOrdinaryNameWithADotOrASpaceStillWorks", ), ( - "a manifest of any size is read into memory", + "Load holds a manifest of any size whole", "internal/manifest/manifest.go", "if int64(len(raw)) <= MaxBytes {", "if len(raw) >= 0 {", - "TestAManifestTooLargeToReadIsRefusedBeforeItIsRead", + "TestLoadKeepsTheCeilingOnAWholeDocumentAndReadClaimsHasNone", ), ( # Found by audit on 2026-08-03. Measuring an archive meant building it, @@ -2613,8 +2573,8 @@ ( "the manifest is read without asking where its entries point", "internal/manifest/manifest.go", - "if err := checkPaths(path, &m); err != nil {", - "if err := checkPaths(path, &m); err != nil && false {", + "\tif escaped != nil {\n\t\treturn escaped\n\t}\n", + "\tif escaped != nil && false {\n\t\treturn escaped\n\t}\n", "TestLoadingAManifestRefusesAnEntryThatClimbsOut", ), ( @@ -2638,12 +2598,12 @@ "TestAManifestPathMayStillNameASubdirectory", ), ( - # The other direction of the door in manifest.Load, so shutting it on - # everything is not a way to pass. + # The other direction of the door, so shutting it on everything is not + # a way to pass. "the door refuses a manifest naming a file in a subdirectory", "internal/manifest/manifest.go", - "problem := core.ContainmentProblem(f.Path)", - 'problem := core.ContainmentProblem("../" + f.Path)', + "problem := core.ContainmentProblem(p)", + "problem := core.ContainmentProblem(\"../\" + p)", "TestLoadingAManifestAcceptsAnEntryInASubdirectory", ), ( @@ -6125,13 +6085,6 @@ "\tif true || p.seen < p.nextAt {", "TestAPlanTooBigToHoldIsRefusedWhileItIsBuilt", ), - ( - "a run stops saying that its manifest will be too big for verify and cleanup to read", - "internal/cli/record.go", - "if !record.TooLarge {", - "if true || !record.TooLarge {", - "TestARunSaysWhenItsManifestWillBeTooBigToReadBack", - ), ( "the counted length of a manifest leaves out the line that closes its list of files", "internal/manifest/size.go", @@ -6167,13 +6120,6 @@ "\tif asked >= maxEntries {", "TestTheEntryCeilingStillAcceptsTheNumberBelowIt", ), - ( - "every run starts warning that its manifest is too big to read back", - "internal/cli/record.go", - "if !record.TooLarge {", - "if !record.TooLarge && false {", - "TestAnOrdinaryRunSaysNothingAboutTheManifestCeiling", - ), ( "free space too large to count wraps into a negative number again", "internal/core/diskspace.go", @@ -6247,8 +6193,8 @@ ( "the forecast of the record and the preflight both stop noticing that they were asked to stop", "internal/engine/engine.go", - "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 {", + "record, err := PredictRecord(ctx, files, opt)\n\tif err != nil {\n\t\treturn res, err\n\t}\n\tif err := preflight(ctx, files, opt, record); err != nil {", + "record, err := PredictRecord(context.Background(), files, opt)\n\tif err != nil {\n\t\treturn res, err\n\t}\n\tif err := preflight(context.Background(), files, opt, record); err != nil {", "TestWorkingOutTheCostCanBeStopped", ), ( @@ -10796,7 +10742,7 @@ ( "verify reports the instructions beside the manifest as a file nobody asked for", "internal/audit/audit.go", - "\t\tif seen[p] || filepath.Base(p) == skip || (m.Run.Instructions != \"\" && filepath.Base(p) == m.Run.Instructions) {\n", + "\t\tif seen[p] || filepath.Base(p) == skip || (c.Instructions != \"\" && filepath.Base(p) == c.Instructions) {\n", "\t\tif seen[p] || filepath.Base(p) == skip {\n", "TestTheInstructionsGoWhereTheManifestGoes", ), @@ -15586,29 +15532,6 @@ "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", @@ -16327,7 +16250,7 @@ ( "a plan no longer hands its memory setting to the runtime", "internal/engine/engine.go", - "\thandToRuntime(limits)\n", + "\tHandToRuntime(limits)\n", "\t_ = limits\n", "TestTheCommandLineHoldsTheWholeRunToItsMemorySetting", ), @@ -16448,8 +16371,8 @@ ( "a preview in the window no longer says that the run will hold too much", "internal/gui/window/run.go", - "manifestReachNote(res.Record)...), holdNote(res)...), runNotesOf", - "manifestReachNote(res.Record)...), []string(nil)...), runNotesOf", + "r.say(append(append([]string{said}, holdNote(res)...), runNotesOf(res)...)...)", + "r.say(append(append([]string{said}, []string(nil)...), runNotesOf(res)...)...)", "TestTheWindowSaysWhenARunWillHoldMoreThanItsMemory", ), ( @@ -16457,8 +16380,8 @@ # sentence after the run is for. "a finished run in the window no longer says that it held too much", "internal/gui/window/run.go", - "manifestReachNote(saved.Size)...), holdNote(res)...)", - "manifestReachNote(saved.Size)...), []string(nil)...)", + "said := append([]string{outcome}, holdNote(res)...)", + "said := append([]string{outcome}, []string(nil)...)", "TestTheWindowSaysWhenARunWillHoldMoreThanItsMemory", ), ( @@ -16683,4 +16606,124 @@ "\t\t\t_ = screen\n", "TestClosingTheWindowDuringARunStopsItAndWaitsForIt", ), + # --- the manifest read a piece at a time, docs/MANIFEST-STREAMING-2026-11-04.md (2026-11-04) --- + ( + "one value of a manifest may be as long as it likes", + "internal/manifest/read.go", + "rd.in.limit = rd.dec.InputOffset() + MaxValueBytes", + "rd.in.limit = 1 << 62", + "TestOneValueOfAManifestPastTheCeilingIsRefusedAndALongManifestIsNot", + ), + ( + "a second document after the manifest is let through", + "internal/manifest/read.go", + "\trd.next()\n\t_, err := rd.dec.Token()\n\tif errors.Is(err, io.EOF) {\n\t\treturn nil\n\t}\n", + "\trd.next()\n\t_, err := rd.dec.Token()\n\tif true {\n\t\treturn nil\n\t}\n", + "TestAManifestWithAnythingAfterItIsRefused", + ), + ( + "white space after a manifest is taken for something more", + "internal/manifest/read.go", + "\tif errors.Is(err, io.EOF) {\n\t\treturn nil\n\t}\n\tvar syntax", + "\tif false {\n\t\treturn nil\n\t}\n\tvar syntax", + "TestWhiteSpaceAfterAManifestIsNothing", + ), + ( + "a key named twice is read, the second time winning", + "internal/manifest/read.go", + "if first, twice := seen[name]; twice {", + "if first, twice := seen[name]; twice && false {", + "TestAManifestThatNamesAKeyTwiceIsRefused", + ), + ( + "Ctrl+C is not asked while the entries are read", + "internal/manifest/read.go", + "\t\tif err := rd.ctx.Err(); err != nil {\n\t\t\treturn err\n\t\t}\n\t\tvar f File\n", + "\t\tvar f File\n", + "TestCtrlCWhileAManifestIsReadStopsTheReadingAndRemovesNothing", + ), + ( + "an entry that escapes is refused before the version is known", + "internal/manifest/read.go", + "\t\tif rd.escaped == nil {\n\t\t\trd.escaped = entryEscapes(rd.path, i, f.Path)\n\t\t}\n", + "\t\tif err := entryEscapes(rd.path, i, f.Path); err != nil {\n\t\t\treturn err\n\t\t}\n", + "TestAManifestWithTwoFaultsIsRefusedForTheOneThatComesFirst", + ), + ( + "the entries are kept only once the version has been read", + "internal/manifest/read.go", + "\t\teach(f)\n", + "\t\tif m.ManifestVersion != \"\" {\n\t\t\teach(f)\n\t\t}\n", + "TestAManifestIsReadTheSameWhateverOrderItsKeysComeIn", + ), + ( + "FILES is read as a field rather than as the entries", + "internal/manifest/read.go", + "\t\tif strings.EqualFold(key, filesKey) {", + "\t\tif key == filesKey {", + "TestAManifestIsReadTheSameWhateverCaseItsKeysAreIn", + ), + ( + "files that are null are refused rather than read as no list", + "internal/manifest/read.go", + "\tcase nil:\n\t\t// null, which encoding/json reads as no list at all.\n\t\treturn nil\n\tcase json.Delim('['):", + "\tcase json.Delim('['):", + "TestAManifestWithNothingInItIsAnsweredAsItAlwaysWas", + ), + ( + "checking a manifest has no limit whatever the memory setting", + "internal/manifest/read.go", + "\tif most > 0 && need > most {\n\t\treturn Claims{}, &TooMuchToHoldError", + "\tif false {\n\t\treturn Claims{}, &TooMuchToHoldError", + "TestAManifestListingMoreThanTheMemorySettingLetsCleanupCheckIsRefused", + ), + ( + "verify and cleanup read a manifest under no memory setting at all", + "internal/cli/claims.go", + "claims, err := manifest.ReadClaims(ctx, path, limits.Run)", + "claims, err := manifest.ReadClaims(ctx, path, 0)", + "TestAManifestListingMoreThanTheMemorySettingLetsCleanupCheckIsRefused", + ), + ( + "Ctrl+C while the manifest is read is not said as such", + "internal/cli/claims.go", + "\tif errors.Is(err, context.Canceled) {\n\t\tfmt.Fprintf(errOut, \"tfg: %s\\n\", stopped)\n\t\treturn manifest.Claims{}, ExitInterrupted, false\n\t}\n", + "\tif errors.Is(err, context.Canceled) && false {\n\t\tfmt.Fprintf(errOut, \"tfg: %s\\n\", stopped)\n\t\treturn manifest.Claims{}, ExitInterrupted, false\n\t}\n", + "TestCtrlCWhileAManifestIsReadStopsTheReadingAndRemovesNothing", + ), + ( + "Load keeps the last entry rather than every one", + "internal/manifest/manifest.go", + "\t\tm.Files = append(m.Files, f)\n\t})", + "\t\tm.Files = append(m.Files[:0], f)\n\t})", + "TestBothDoorsOfTheManifestRefuseTheSameDocumentsInTheSameWords", + ), + ( + "another run's record is let go once it is past the old ceiling", + "internal/manifest/read.go", + "err = decode(ctx, bufio.NewReaderSize(f, readBuffer), path, &m, func(e File) {", + "err = decode(ctx, bufio.NewReaderSize(io.LimitReader(f, MaxBytes), readBuffer), path, &m, func(e File) {", + "TestAnotherRunsRecordIsAnotherRunsRecordHoweverLongItIs", + ), + ( + "the manifest of a run past the old ceiling is cut short where the old reader stopped", + "internal/manifest/read.go", + "err = decode(ctx, bufio.NewReaderSize(f, readBuffer), path, &m, func(e File) {", + "err = decode(ctx, bufio.NewReaderSize(io.LimitReader(f, MaxBytes+1), readBuffer), path, &m, func(e File) {", + "TestTheManifestOfARunPastTheOldCeilingIsVerifiedAndCleanedUp", + ), + ( + "another run's record no longer claims the files in front of us", + "internal/audit/neighbours.go", + "if key := comparablePath(c.Path); unclaimed[key] {", + "if key := comparablePath(c.Path); false && unclaimed[key] {", + "TestVerifyNamesTheFilesOfAnotherRunRatherThanCallingThemExtra", + ), + ( + "generate --json holds the manifest whole again to send it", + "internal/cli/generate.go", + "\t\tif code := sendManifest(res, g.dryRun, out, errOut); code != ExitOK {", + "\t\tvar whole bytes.Buffer\n\t\tcode := sendManifest(res, g.dryRun, &whole, errOut)\n\t\t_, _ = out.Write(whole.Bytes())\n\t\tif code != ExitOK {", + "TestGenerateSendsItsManifestAPartAtATime", + ), ] diff --git a/internal/guard/windowmanifestreach_test.go b/internal/guard/windowmanifestreach_test.go deleted file mode 100644 index cbcbd2eb..00000000 --- a/internal/guard/windowmanifestreach_test.go +++ /dev/null @@ -1,209 +0,0 @@ -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" -) - -// The window says the same thing the command line says about a record too big -// to read back. -// -// Observation O184, measured on 2026-09-06: TooLargeToReadBack had two callers, -// both in internal/cli and internal/manifest, and NOT ONE in internal/gui. A -// person who generated 25 000 files from the window was told nothing at all, -// and was left with a directory that tfg verify and tfg cleanup both refuse - -// the manifest is the only authority over what may be removed, so a manifest -// that cannot be read is a set of files with no owner. -// -// It is the kind of parity gap D1 loses most easily. Not something the engine -// can do from one surface and not the other, which is what the parity guard -// 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. 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 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() - 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 -} - -// previewOf presses Preview for a run of count files and gives back what the -// screen said. -// -// It REFUSES to return a refusal, and that is the whole reason it exists. The -// first version of the pair below set a size the default format will not take - -// the window opens on the first format in the registry, which is avif, and -// 200 B is far under what a picture needs. Both previews were turned down, so -// the negative half passed while proving nothing: a screen that says "check the -// settings marked above" says nothing about a manifest ceiling either. -func previewOf(t *testing.T, count string) string { - t.Helper() - content, w, host := screenInAWindowWithHost(t, text.TabOneTarget()) - - // txt rather than whatever the window opens on, for two reasons. The size - // below has to be one the format takes, and planning twenty two thousand - // pictures would encode twenty two thousand pictures - png, jpg, gif and - // avif all do that while planning. - picker, ok := controlUnder(content, text.FieldFormat()).(*parts.Chooser) - if !ok { - t.Fatal("the format field is not a list to choose from, so this guard read the wrong tree") - } - picker.SetSelected("txt") - - // Small files, because what is being asked about is the number of ENTRIES - // rather than the number of bytes. A preview writes nothing either way. - fill(t, content, text.FieldSize(), "200b") - fill(t, content, text.FieldCount(), count) - - press(t, content, text.ButtonPreview()) - // This preview is accepted, so it answers from a worker. Joined before the - // status line is read - see join. - join(host) - settle(content, w) - - _, status := runMessages(content) - if status == nil { - t.Fatal("the screen has no status line, so this guard read the wrong tree") - } - // Matched on the tail the preview puts on its own line, the way the - // action bar guard does it, so this cannot be satisfied by a refusal. - if !strings.Contains(status.Text, text.AndNothingWrittenYet()) { - t.Fatalf("the preview of %s files was not accepted, so nothing here was asked about the manifest.\nIt said:\n%s", - count, status.Text) - } - return status.Text -} - -// runOf presses Generate rather than Preview, and gives back what the screen -// said when it finished. -// -// It exists because the preview is OPTIONAL. Somebody who presses Generate -// straight away never sees the preview's answer, and that person is exactly the -// one observation O184 is about - they end up with a directory nothing in this -// toolset can read or clean. The window cannot say anything in the middle of a -// run, so the end of the run is the only place left. -// -// It writes files, which is why this is the one guard here that does. Twenty -// two thousand of them at 200 B, into a directory that goes away with the test. -func runOf(t *testing.T, count string) string { - t.Helper() - content, w, host := screenInAWindowWithHost(t, text.TabOneTarget()) - - picker, ok := controlUnder(content, text.FieldFormat()).(*parts.Chooser) - if !ok { - t.Fatal("the format field is not a list to choose from, so this guard read the wrong tree") - } - picker.SetSelected("txt") - fill(t, content, text.FieldSize(), "200b") - fill(t, content, text.FieldCount(), count) - fill(t, content, text.FieldOutputDir(), t.TempDir()) - - press(t, content, text.ButtonGenerate()) - join(host) - settle(content, w) - - _, status := runMessages(content) - if status == nil { - t.Fatal("the screen has no status line, so this guard read the wrong tree") - } - // The run has to have HAPPENED. A refused run says nothing about a - // manifest either, and a guard that cannot tell those apart is the shape - // this project has recorded as passing without reaching the code. - if !strings.Contains(status.Text, text.Written(0)[strings.LastIndex(text.Written(0), " ")+1:]) { - t.Fatalf("the run of %s files did not finish, so nothing here was asked about the manifest.\nIt said:\n%s", - count, status.Text) - } - return status.Text -} - -// warningAbout is the fixed half of the sentence, without the two numbers. -// -// Taken from the text package rather than typed here, so a reworded warning -// does not quietly stop being checked. -func warningAbout(t *testing.T) string { - t.Helper() - marker := text.ManifestTooLargeToRead("SIZE", "LIMIT") - at := strings.Index(marker, "SIZE") - if at < 0 { - t.Fatal("the warning does not carry the size it was given, so this guard cannot find its fixed half") - } - return marker[:at] -} - -func TestTheWindowSaysWhenItsManifestWillBeTooBigToReadBack(t *testing.T) { - said := previewOf(t, itoa(overTheCeiling(t))) - if !strings.Contains(said, warningAbout(t)) { - t.Errorf("a preview of %d files said nothing about the record being too big to read back.\n"+ - "The command line has said this since 2026-08-26. Somebody who does the same from the window "+ - "gets a directory that neither Verify nor Clean up can read, and no warning.\nIt said:\n%s", - overTheCeiling(t), said) - } -} - -// And a run that was never previewed says it too, which is the case that -// matters most. -// -// The preview is a button somebody may not press. The warning has to reach the -// person who pressed Generate and nothing else, because they are the one left -// with the directory. -func TestAFinishedRunInTheWindowSaysItsManifestIsTooBigToReadBack(t *testing.T) { - said := runOf(t, itoa(overTheCeiling(t))) - if !strings.Contains(said, warningAbout(t)) { - t.Errorf("a finished run of %d files said nothing about the record being too big to read back.\n"+ - "That directory now has a manifest neither Verify nor Clean up can read, and nobody was told.\nIt said:\n%s", - overTheCeiling(t), said) - } - // The line somebody pressed the button for stays first. The room for these - // messages is a ceiling and the message scrolls inside it. - if first := strings.SplitN(said, "\n", 2)[0]; strings.Contains(first, warningAbout(t)) { - t.Errorf("the warning took the first line from the outcome:\n%s", said) - } -} - -// A run this build CAN read back stays quiet. -// -// Without this the guard above passes on a window that warns about every run, -// which teaches somebody to stop reading the line - the same reason the command -// line has this pair rather than only the first half. -func TestAnOrdinaryPreviewSaysNothingAboutTheManifestCeiling(t *testing.T) { - said := previewOf(t, "100") - if strings.Contains(said, warningAbout(t)) { - t.Errorf("a hundred files drew the warning about the record being too big:\n%s", said) - } -} diff --git a/internal/gui/text/locale/en.json b/internal/gui/text/locale/en.json index 3538abda..160fa008 100644 --- a/internal/gui/text/locale/en.json +++ b/internal/gui/text/locale/en.json @@ -631,11 +631,6 @@ "hash": "sha256-bb722052cb9e", "other": "the files were written and the manifest could not be saved to {{.Path}}" }, - "ManifestTooLargeToRead": { - "description": "Shown in the window. Carries these values, each of which has to stay spelled exactly that way: {{.Size}}, {{.Limit}}.", - "hash": "sha256-0b5d14c5fc58", - "other": "this run's record is about {{.Size}} and this build reads at most {{.Limit}}, so Verify and Clean up will not be able to read it. Split the run to keep each record readable." - }, "MemoryAutoIs": { "description": "Shown in the window. Carries one value, {{.Size}}, which has to stay spelled exactly that way.", "hash": "sha256-3a039b136e1c", diff --git a/internal/gui/text/locale/pl.json b/internal/gui/text/locale/pl.json index 7edb2fed..894a9668 100644 --- a/internal/gui/text/locale/pl.json +++ b/internal/gui/text/locale/pl.json @@ -125,7 +125,6 @@ "ListNothingMatches": { "hash": "sha256-7b2c3b9853ac", "other": "Żaden format nie pasuje - wyczyść pole, żeby zobaczyć wszystkie" }, "ManifestNamed": { "hash": "sha256-f9540bb6e465", "other": "Manifest: {{.Name}}" }, "ManifestNotSaved": { "hash": "sha256-bb722052cb9e", "other": "pliki zostały zapisane, a manifestu nie udało się zapisać do {{.Path}}" }, - "ManifestTooLargeToRead": { "hash": "sha256-0b5d14c5fc58", "other": "zapis tego przebiegu ma około {{.Size}}, a ta wersja czyta najwyżej {{.Limit}}, więc polecenia verify i cleanup nie zdołają go odczytać. Podziel przebieg, żeby każdy zapis dał się odczytać." }, "MemoryAutoIs": { "hash": "sha256-3a039b136e1c", "other": "Przebiegi mogą wziąć {{.Size}}, połowę pamięci, którą ten komputer daje programowi." }, "MemoryAutoUnknown": { "hash": "sha256-c1b8c2b64c84", "other": "Nie udało się odczytać pamięci tego komputera, więc przebiegi biorą {{.Size}}, czyli najwięcej, ile brał każdy przebieg przed tym ustawieniem." }, "MemoryCustomEmpty": { "hash": "sha256-f021f2e8d953", "other": "Wpisz rozmiar, na przykład 8gb. Dopóki go nie ma, przebiegi używają ustawienia Automatycznie." }, diff --git a/internal/gui/text/locale/said/en.json b/internal/gui/text/locale/said/en.json index 9247585c..21fb972e 100644 --- a/internal/gui/text/locale/said/en.json +++ b/internal/gui/text/locale/said/en.json @@ -2256,11 +2256,41 @@ "one": "{{.Count}} file", "other": "{{.Count}} files" }, + "manifest.FilesIsNotAList": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Kind}}, written the way the program writes them - keep each spelled exactly that way.", + "hash": "sha256-45d3b81b6af0", + "other": "files is a JSON {{.Kind}} rather than a list" + }, "manifest.ItCarriesNoManifestVersionSo": { "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run.", "hash": "sha256-00841a615039", "other": "it carries no manifest_version, so it is not a manifest this tool wrote" }, + "manifest.ItEndsPartWay": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run.", + "hash": "sha256-cd8b50653d2c", + "other": "it ends part way through the document, so it was cut short" + }, + "manifest.ItHoldsNoDocument": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run.", + "hash": "sha256-73ba6f86a219", + "other": "it holds no JSON document at all" + }, + "manifest.ItHoldsTheKeyTwice": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Key}}, written the way the program writes them - keep each spelled exactly that way.", + "hash": "sha256-d13535e53697", + "other": "it holds the key {{.Key}} twice" + }, + "manifest.ItHoldsTheKeyTwiceSpelled": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Key}}, {{.Second}}, written the way the program writes them - keep each spelled exactly that way.", + "hash": "sha256-5a62ea453435", + "other": "it holds the key {{.Key}} twice, the second time spelled {{.Second}}" + }, + "manifest.ItIsNotAnObject": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Kind}}, written the way the program writes them - keep each spelled exactly that way.", + "hash": "sha256-1bd780d90f55", + "other": "it is a JSON {{.Kind}} rather than an object, so it is not a manifest this tool wrote" + }, "manifest.ItIsSchemaVersionAndThis": { "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.ManifestVersion}}, {{.Version}}, written the way the program writes them - keep each spelled exactly that way.", "hash": "sha256-57828974a0e1", @@ -2291,16 +2321,36 @@ "hash": "sha256-1c92a916a2de", "other": "run.instructions is {{.Name}}, and it can only be the name of a file beside the manifest ending in {{.InstructionsSuffix}}. This tool removes that file with the manifest, so it will not act on one that names anything else. Use the manifest the run actually wrote, or remove the key" }, + "manifest.SomethingFollowsIt": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run.", + "hash": "sha256-ed8ba11add7e", + "other": "something follows the end of the document, and a manifest is one JSON object with nothing after it but white space" + }, "manifest.TooLarge": { "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Path}}, {{.Bytes}}, {{.Limit}}, written the way the program writes them - keep each spelled exactly that way.", "hash": "sha256-e12fe7ac5071", "other": "{{.Path}} is {{.Bytes}} B and the limit is {{.Limit}} B. A manifest is read into memory to be compared against a directory, so an unbounded one is a way to exhaust it. Check that this is a manifest this tool wrote, or split the run it describes" }, + "manifest.TooMuchToHold": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Path}}, {{.Files}}, {{.Need}}, {{.Most}}, written the way the program writes them - keep each spelled exactly that way.", + "hash": "sha256-629eb46d9a66", + "other": "{{.Path}} lists {{.Files}}, and checking them takes about {{.Need}} of memory, more than the {{.Most}} the memory setting gives this command. Give it more through the memory setting, a size such as 8gb or off for no limit, or split the run it describes" + }, + "manifest.TwiceBecause": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Detail}}, written the way the program writes them - keep each spelled exactly that way.", + "hash": "sha256-1ae6ad202098", + "other": "{{.Detail}}. This tool writes every key once, so the manifest was written or edited by something else, and which of the two it means is a guess. Use the manifest the run actually wrote" + }, "manifest.Unreadable": { "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Path}}, {{.Detail}}, written the way the program writes them - keep each spelled exactly that way.", "hash": "sha256-bb90fafec850", "other": "{{.Path}} cannot be read as a manifest: {{.Detail}}" }, + "manifest.ValueTooLarge": { + "description": "Said by internal/manifest - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Path}}, {{.Limit}}, {{.At}}, written the way the program writes them - keep each spelled exactly that way.", + "hash": "sha256-65f9e8304926", + "other": "{{.Path}} holds a single value of more than {{.Limit}} B, starting about {{.At}} B in, and this tool reads at most that much of a manifest at once. An entry of a manifest this tool wrote takes a kilobyte or two, so check that this is the manifest the run wrote" + }, "mp4.MinimumReasonPictures": { "description": "Said by internal/format/mp4 - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Duration}}, {{.FPS}}, {{.Interval}}, {{.Width}}, {{.Height}}, {{.Change}}, {{.Bytes}}, {{.Free}}, written the way the program writes them - keep each spelled exactly that way.", "hash": "sha256-2d553380ec9d", diff --git a/internal/gui/text/locale/said/pl.json b/internal/gui/text/locale/said/pl.json index 663c1ae0..2b3308dd 100644 --- a/internal/gui/text/locale/said/pl.json +++ b/internal/gui/text/locale/said/pl.json @@ -449,15 +449,25 @@ "log.ShapeNoSeverity": { "hash": "sha256-24f6ca4ac678", "other": "kształt {{.Shape}} nie niesie poziomu ważności" }, "manifest.EntryHasThePathWhichLands": { "hash": "sha256-79cbc1e59f3f", "other": "wpis {{.I}} ma ścieżkę {{.Path}}, która wychodzi poza katalog opisany przez manifest - {{.Problem}}. To narzędzie nigdy nie czyta ani nie usuwa niczego poza tym katalogiem, więc nie wykona manifestu, który o to prosi. Użyj manifestu, który przebieg naprawdę zapisał, albo popraw ścieżkę na leżącą wewnątrz katalogu" }, "manifest.Files": { "hash": "sha256-83589d643302", "one": "{{.Count}} plik", "few": "{{.Count}} pliki", "many": "{{.Count}} plików", "other": "{{.Count}} pliku" }, + "manifest.FilesIsNotAList": { "hash": "sha256-45d3b81b6af0", "other": "files jest wartością JSON typu {{.Kind}}, a nie listą" }, "manifest.ItCarriesNoManifestVersionSo": { "hash": "sha256-00841a615039", "other": "nie ma manifest_version, więc to nie jest manifest zapisany przez to narzędzie" }, + "manifest.ItEndsPartWay": { "hash": "sha256-cd8b50653d2c", "other": "urywa się w połowie dokumentu, więc został ucięty" }, + "manifest.ItHoldsNoDocument": { "hash": "sha256-73ba6f86a219", "other": "nie zawiera żadnego dokumentu JSON" }, + "manifest.ItHoldsTheKeyTwice": { "hash": "sha256-d13535e53697", "other": "zawiera klucz {{.Key}} dwa razy" }, + "manifest.ItHoldsTheKeyTwiceSpelled": { "hash": "sha256-5a62ea453435", "other": "zawiera klucz {{.Key}} dwa razy, za drugim razem zapisany jako {{.Second}}" }, + "manifest.ItIsNotAnObject": { "hash": "sha256-1bd780d90f55", "other": "jest wartością JSON typu {{.Kind}}, a nie obiektem, więc to nie jest manifest zapisany przez to narzędzie" }, "manifest.ItIsSchemaVersionAndThis": { "hash": "sha256-57828974a0e1", "other": "ma wersję schematu {{.ManifestVersion}}, a ta wersja czyta {{.Version}}. Użyj wersji tfg, która go zapisała" }, "manifest.NoteMany": { "hash": "sha256-c4c8340dd2e0", "other": "{{.Files}}: {{.Note}} Wymienione: {{.Named}}. Pozostałe, których tu nie wymieniono: {{.Hidden}}." }, "manifest.NoteOne": { "hash": "sha256-9d548db5c58a", "other": "{{.File}}: {{.Note}}" }, "manifest.NoteSeveral": { "hash": "sha256-d0803401c505", "other": "{{.Files}}: {{.Note}} Wymienione: {{.Named}}." }, "manifest.NoteText": { "hash": "sha256-e75b463d20f0", "other": "{{.Text}}" }, "manifest.RunInstructionsIsAndItCan": { "hash": "sha256-1c92a916a2de", "other": "run.instructions to {{.Name}}, a może to być tylko nazwa pliku obok manifestu kończąca się na {{.InstructionsSuffix}}. To narzędzie usuwa ten plik razem z manifestem, więc nie wykona takiego, który wskazuje cokolwiek innego. Użyj manifestu, który przebieg naprawdę zapisał, albo usuń ten klucz" }, + "manifest.SomethingFollowsIt": { "hash": "sha256-ed8ba11add7e", "other": "po końcu dokumentu jest coś jeszcze, a manifest to jeden obiekt JSON, po którym mogą być tylko białe znaki" }, "manifest.TooLarge": { "hash": "sha256-e12fe7ac5071", "other": "{{.Path}} ma {{.Bytes}} B, a limit to {{.Limit}} B. Manifest jest wczytywany do pamięci, żeby porównać go z katalogiem, więc nieograniczony byłby sposobem na jej wyczerpanie. Sprawdź, czy to manifest zapisany przez to narzędzie, albo podziel przebieg, który opisuje" }, + "manifest.TooMuchToHold": { "hash": "sha256-629eb46d9a66", "other": "{{.Path}} wymienia {{.Files}}, a sprawdzenie ich zajmie około {{.Need}} pamięci, więcej niż {{.Most}}, które temu poleceniu daje ustawienie pamięci. Daj mu więcej w ustawieniu pamięci, rozmiar taki jak 8gb albo off bez limitu, albo podziel przebieg, który opisuje" }, + "manifest.TwiceBecause": { "hash": "sha256-1ae6ad202098", "other": "{{.Detail}}. To narzędzie zapisuje każdy klucz raz, więc manifest zapisało albo zmieniło coś innego, a to, którą z dwóch wartości ma na myśli, to zgadywanie. Użyj manifestu, który faktycznie zapisał przebieg" }, "manifest.Unreadable": { "hash": "sha256-bb90fafec850", "other": "{{.Path}} nie daje się odczytać jako manifest: {{.Detail}}" }, + "manifest.ValueTooLarge": { "hash": "sha256-65f9e8304926", "other": "{{.Path}} zawiera pojedynczą wartość dłuższą niż {{.Limit}} B, zaczynającą się mniej więcej {{.At}} B od początku, a to narzędzie czyta z manifestu naraz najwyżej tyle. Wpis manifestu zapisanego przez to narzędzie zajmuje kilobajt lub dwa, więc sprawdź, czy to manifest zapisany przez przebieg" }, "mp4.MinimumReasonPictures": { "hash": "sha256-2d553380ec9d", "other": "film o długości {{.Duration}} przy {{.FPS}} klatkach na sekundę z klatką kluczową co {{.Interval}} i nowym obrazem {{.Width}}x{{.Height}} co {{.Change}} zajmuje {{.Bytes}} B, a plik zawsze niesie pudełko free, które kosztuje {{.Free}} B nawet puste" }, "pdf.Carrying": { "hash": "sha256-83e9af0cb805", "other": " z {{.What}}" }, "pdf.ChooseOneSize": { "hash": "sha256-61da6826cc02", "other": "wybierz jeden rozmiar, na przykład a4" }, diff --git a/internal/gui/text/text.go b/internal/gui/text/text.go index 111d2d61..6e99791f 100644 --- a/internal/gui/text/text.go +++ b/internal/gui/text/text.go @@ -221,23 +221,6 @@ func DirectoryWithFreeSpace(dir, free string) string { map[string]any{"Directory": dir, "Free": free}) } -// ManifestTooLargeToRead is said when a run will write a record this build -// cannot read back. -// -// The command line has printed this since 2026-08-26 and the window said -// nothing at all, which is the parity gap observation O184 names. A person who -// generates 25 000 files from a window gets a directory that tfg verify and -// tfg cleanup both refuse - and the manifest is the only authority over what -// may be deleted, so nothing in this toolset can ever remove those files. -// -// A note rather than a refusal, which is the owner's decision from that day and -// is unchanged here. The run works. What was missing was that nobody was told. -func ManifestTooLargeToRead(size, limit string) string { - return sayf("ManifestTooLargeToRead", - "this run's record is about {{.Size}} and this build reads at most {{.Limit}}, so Verify and Clean up will not be able to read it. Split the run to keep each record readable.", - map[string]any{"Size": size, "Limit": limit}) -} - // RunHoldsMoreThanItsMemory is said when a run will hold more than its memory // setting gives it - the window's half of the note the command line prints // before the first byte (O296, engine.PredictHold). diff --git a/internal/gui/window/run.go b/internal/gui/window/run.go index 1cdd716a..10b2e6ba 100644 --- a/internal/gui/window/run.go +++ b/internal/gui/window/run.go @@ -502,9 +502,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 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. +// preview say what the run would hold and what it would note. A dry run works +// those out for the run it previews, 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 { @@ -518,7 +518,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(append(append([]string{said}, manifestReachNote(res.Record)...), holdNote(res)...), runNotesOf(res)...)...) + r.say(append(append([]string{said}, holdNote(res)...), runNotesOf(res)...)...) } // formatsOf is what kinds of file the run would produce, each named once. @@ -685,11 +685,9 @@ func (r *runner) runFinished(res *engine.Result, runErr, saveErr error, room dis // answer in a terminal and an instruction to open a file with ten thousand // entries in a window. // - // The warning about a record too big to read back comes SECOND, ahead of - // the per file notes, and that order is the same lesson the command line - // learned on 2026-09-06: it is the one line standing between somebody and a - // directory nothing in this toolset can ever clean up, and it was being - // buried under notes about a label that did not fit. + // The note about memory comes SECOND, ahead of the per file notes - the + // lesson the command line learned on 2026-09-06, when the one line about + // the run was buried under notes about a label that did not fit. // // The record is named on the same line as the outcome rather than on one of // its own, because it is part of the same fact: what this run produced. The @@ -697,7 +695,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(append([]string{outcome}, manifestReachNote(saved.Size)...), holdNote(res)...) + said := append([]string{outcome}, holdNote(res)...) // 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 a356d888..62cc4fcd 100644 --- a/internal/gui/window/runreport.go +++ b/internal/gui/window/runreport.go @@ -6,7 +6,6 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/engine" "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" - "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) // What a run tells the person while it goes and when it ends. @@ -17,32 +16,6 @@ import ( // number. The split is by subject: everything here turns what the engine // reports into the sentence under the buttons, and nothing here drives a run. -// manifestReachNote is the window's half of the warning the command line prints -// before the first byte. -// -// Observation O184: the command line has said this since 2026-08-26 and the -// window said nothing at all, so a person generating 25 000 files from a window -// was left with a directory that neither Verify nor Clean up can read - and the -// manifest is the only authority over what may be removed. A parity gap in -// quality rather than in what the engine can do, which is the kind D1 is -// easiest to lose. -// -// 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(record.Manifest), text.HumanBytes(manifest.MaxBytes))} -} - // holdNote is the window's half of the note the command line prints before the // first byte when a run will hold more than its memory setting gives it. // diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index 68389069..db3059df 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -155,41 +155,6 @@ type Summary struct { // The cheapest form of the question O97 is about - a person checking by // eye, or a script asserting a shape, does not have to walk the entries. ByTarget map[string]int `json:"by_target"` - - // TooLargeToReadBack says this build would refuse to read this document - // back, so "tfg verify" and "tfg cleanup" cannot work from it. - // - // The run itself succeeds and its files are correct. What is gone is the - // only authority over what may be deleted - untouchable rule 7 makes the - // manifest that authority, so a manifest this build will not read is a set - // of files nothing in this toolset can remove. Measured on 2026-08-26 and - // again on 2026-09-06: 25 000 files write about 25.9 MB against a ceiling - // of 16 MB, generate exits 0, and verify and cleanup both exit 5. - // - // The owner decided on 2026-08-26 that this is a note rather than a - // refusal, because refusing would take away something the tool does. What - // was missing was that nobody was told, and the telling was prose on - // stderr - measured on 2026-09-06 as one line among 25 003, and ABSENT - // ENTIRELY from the machine readable report. A tool that plugs into CI has - // to be able to say this to a script. - // - // Not an exit code, and that was decided rather than skipped. Exit 8 means - // "run finished, but not everything was produced" (docs/CLI.md), and here - // everything was produced - res.Failures is nought, so the sentence would - // read "0 files could not be produced" and point at a manifest that cannot - // be read. Exit codes are a frozen contract and redefining one is a major - // version, not a stopgap. - // - // Absent rather than false on an ordinary run, so every manifest already - // 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"` } // File is one entry. @@ -407,11 +372,12 @@ const noteExamples = 3 // same sentence about the label not fitting. The advice was per file for a // decision that is per target. // -// The line that matters was buried under them. A run whose manifest will be -// too big for this build to read back says so first, in a "note:" line -// typographically identical to the 25 000 that follow it - and that one is the -// only thing standing between somebody and a directory neither verify nor -// cleanup can ever read. See echoManifestReach in cli/generate.go. +// The line that mattered was buried under them. A run whose manifest would +// be too big for this build to read back said so first, in a "note:" line +// typographically identical to the 25 000 that followed it - and that one was +// the only thing standing between somebody and a directory neither verify nor +// cleanup could read. That note went on 2026-11-04 with the ceiling it was +// about (read.go), and the grouping stays for every note that is left. // // The same reasoning was already applied to the progress bar, which is // throttled to 10 Hz and silent when stderr is not a terminal, on the grounds @@ -536,7 +502,14 @@ func (e *SchemaError) Said() core.Said { return core.Says("manifest.Unreadable", "%s cannot be read as a manifest: %s", core.A("Path", e.Path), core.A("Detail", e.Detail)) } -// MaxBytes is the largest manifest this build will read. +// MaxBytes is the largest manifest Load will hold whole. +// +// Load's alone since 2026-11-04. verify and cleanup read through ReadClaims, +// which takes a document a piece at a time under MaxValueBytes on one value and +// has no ceiling on the document - a run past this one wrote a manifest they +// then refused, so its files were files this tool could never remove +// (read.go). Load holds every entry, so for it the length of the document is +// still the memory it takes, and what is below is still its reason. // // A manifest arrives from outside the same way a recipe does - it travels with // a fixture set, it turns up in a pull request, it is handed to "verify" by @@ -548,8 +521,7 @@ func (e *SchemaError) Said() core.Said { // 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). +// docs/PRESETS.md and around the ten thousand this tool is designed around. // // 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. diff --git a/internal/manifest/read.go b/internal/manifest/read.go index ae0e1f51..d4b1020e 100644 --- a/internal/manifest/read.go +++ b/internal/manifest/read.go @@ -224,11 +224,8 @@ func (rd *reading) object(m *Manifest, each func(File)) error { return err } key, _ := tok.(string) - if name, known := outermostKey(key); known { - if first, twice := seen[name]; twice { - return rd.twice(first, key) - } - seen[name] = key + if err := rd.once(seen, key); err != nil { + return err } if strings.EqualFold(key, filesKey) { err = rd.entries(m, each) @@ -243,6 +240,22 @@ func (rd *reading) object(m *Manifest, each func(File)) error { return err } +// once remembers a key of the outermost object that is read into a field, and +// refuses it when the field was named before under any spelling - see twice. A +// key no field has is not remembered: it is read and let go, and a document of +// a million different keys would otherwise be a map with no ceiling. +func (rd *reading) once(seen map[string]string, key string) error { + name, known := outermostKey(key) + if !known { + return nil + } + if first, twice := seen[name]; twice { + return rd.twice(first, key) + } + seen[name] = key + return nil +} + // twice is the refusal of a document that names one key two times. // // encoding/json keeps the second, so a document with two lists of files was @@ -480,8 +493,14 @@ func EachClaim(ctx context.Context, path string, keep func(ClaimedFile)) (string // // The claim is a ClaimedFile and its two strings - measured at 134 B for a path of // 17 characters, and 48 B and the two lengths come to 129 (section 8 of the -// analysis). The checking holds about as much again for each, and twice that is -// allowed for the collector's room. +// analysis). The checking holds more of each, and that was measured too: in a +// container of 1 GB on 2026-11-04, a manifest of 600 000 files with paths of +// 16 characters, verify held at most 206 MB alive and cleanup --yes 282 MB - +// 343 and 470 B a file, 2.6 and 3.5 times what the claims take +// (tools/probes/manifestread/container.py). Four times covers both. The path +// is held about four times over as well - claimed, joined to the directory, +// folded for spelling, and in the list of what the directory holds - so a +// longer path is counted at about the rate it costs. func checkingCost(path, sum string) int64 { return checkingFactor * (claimBytes + int64(len(path)) + int64(len(sum))) } @@ -490,7 +509,7 @@ func checkingCost(path, sum string) int64 { const claimBytes = 48 // checkingFactor is how many times its claims checking a manifest takes. -const checkingFactor = 3 +const checkingFactor = 4 // TooMuchToHoldError is a manifest listing more files than verify or cleanup // may hold under the memory setting. diff --git a/internal/manifest/reservation.go b/internal/manifest/reservation.go index c48cd13b..de74625f 100644 --- a/internal/manifest/reservation.go +++ b/internal/manifest/reservation.go @@ -110,10 +110,9 @@ 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() + // Counted before the file is opened, from the layout Encode writes + // (size.go), and what SavedBytes says afterwards. + size, err := m.EncodedBytes() if err != nil { _ = core.RemoveOwn(r.tmp, r.own) return err diff --git a/internal/manifest/size.go b/internal/manifest/size.go index eca557dd..8a242583 100644 --- a/internal/manifest/size.go +++ b/internal/manifest/size.go @@ -2,8 +2,7 @@ package manifest import "encoding/json" -// How long a manifest is, to the byte, and whether this build would read it -// back. +// How long a manifest is, to the byte. // // 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 @@ -29,6 +28,12 @@ import "encoding/json" // (TestTheManifestSizeIsTheLengthOfTheManifest), and another holds Encode's // bytes to encoding/json's own, so a layout both got wrong the same way is // still caught. +// +// Until 2026-11-04 the count also settled whether this build would read the +// document back, into a field of the summary. The ceiling it was held against +// was on the whole document, and it went when the reader learned to take a +// document a piece at a time (read.go) - the field went with it, and what is +// left is the length: what the free space is held against. // entryIndent is what Encode puts in front of every line of an entry: the // entry is two levels into the document. @@ -79,23 +84,6 @@ func (c *counted) Write(p []byte) (int, error) { 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 @@ -109,27 +97,8 @@ func (m *Manifest) entriesBytes() (int64, error) { 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. +// and changes nothing in it. func (m *Manifest) EncodedBytes() (int64, error) { entries, err := m.entriesBytes() if err != nil { @@ -145,7 +114,7 @@ func (m *Manifest) EncodedBytes() (int64, error) { // 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 run holds the free space against its manifest before its first byte, 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 @@ -178,11 +147,14 @@ func (p *Projection) Add(f File) { 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) { +// Bytes is how long the document would be. +func (p *Projection) Bytes() (int64, error) { if p.err != nil { - return 0, false, p.err + return 0, p.err + } + empty, err := p.shell.emptyBytes() + if err != nil { + return 0, err } - return settle(p.shell, p.entries, p.n) + return documentBytes(empty, p.entries, p.n), nil } From d9e2c22744918d21b98b160c59cc618aeeda967c Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 4 Nov 2026 09:44:09 +0100 Subject: [PATCH 3/3] Another run's record is held once a file, and the value ceiling is held from both sides - verify keeps each file another run's record claims once, however many times the record lists it, so what it holds grows with the directory rather than with the record. A guard reads two records of 50 000 entries and compares what each allocated. - One value a kilobyte under the 16 MB ceiling is read, so the ceiling cannot drop below the one the refusal names without a guard noticing. - The guard on generate --json asks that the first piece of the manifest reaches standard output from inside Encode. A document encoded whole and then sent in small pieces gave the same piece sizes and passed. - The changelog entry for reading a manifest of any size is cut under the hundred words, with --memory on verify and cleanup under Added and the read-back warning under Removed. - The reader's internal signal for a value past the ceiling is a Defect, the one-key document is built by append alone, and the race weights no longer name the window guard this branch removed. Co-Authored-By: Claude Opus 5.5 --- .github/race-weights | 1 - CHANGELOG.md | 33 ++++++----- internal/audit/neighbours.go | 8 ++- internal/guard/concurrentruns_test.go | 24 ++++++++ internal/guard/manifestreadlimits_test.go | 69 +++++++++++++++++++++++ internal/guard/manifeststream_test.go | 40 ++++++++++++- internal/guard/mutation/entries.py | 27 ++++++++- internal/manifest/read.go | 11 ++-- 8 files changed, 188 insertions(+), 25 deletions(-) diff --git a/.github/race-weights b/.github/race-weights index 8bb69854..0d4a7a67 100644 --- a/.github/race-weights +++ b/.github/race-weights @@ -54,7 +54,6 @@ unlisted 0.58 16.2 TestABuildWithNoWindowInItSaysSoAndKeepsStandardOutputEmpty 16.0 TestEveryLanguageDescribesEverythingTheProgramCanProduce 15.4 TestAvifStillFitsWhenTheQualityIsNotTheOneTheCeilingsWereMeasuredAt -15.2 TestAFinishedRunInTheWindowSaysItsManifestIsTooBigToReadBack 15.1 TestABuildWithoutTheBuildTagsRefusesAndSaysWhy 14.9 TestAShortcutCannotStartASecondRunDuringTheFirst 14.6 TestTheLicenceCommandListsWhatTheBinaryLinks diff --git a/CHANGELOG.md b/CHANGELOG.md index 5def03a2..386ae971 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,11 @@ because it turns other people's test suites red. The same run finished where the container had swap, so it is warned rather than refused. +- **`tfg verify` and `tfg cleanup` take `--memory`**, the setting `generate` + takes. A manifest listing more files than it lets them check is refused + before any file is looked at, with the number of files and how much memory + checking them takes. + - **A Docker image.** `ghcr.io/donislawdev/tfg` holds the Linux command line of a release and nothing else, for x64 and Arm, and runs with no network. It is made from the published archive once its checksum and attestation check @@ -56,6 +61,13 @@ because it turns other people's test suites red. and now nothing in it is acted on. The refusal names the key, and ends with exit code 5 like any manifest that cannot be read. +### Removed + +- **The warning that a manifest will be too large to read back**, on the + command line and in the window, and `summary.too_large_to_read_back` in new + manifests. `tfg verify` and `tfg cleanup` read a manifest of any size now. + The field was optional, and `manifest_version` does not change. + ### Fixed - **Cancel answers at once, and closing the window during a run no longer @@ -131,21 +143,12 @@ because it turns other people's test suites red. even look up, for want of permission or a name too long for the system, as `unreadable` with the reason rather than as `missing`. - **`tfg verify` and `tfg cleanup` read the manifest of a run of any size.** - They refused a manifest larger than 16 MB, about 20 000 files, so the files - of a larger run were files `tfg cleanup` could never remove. The manifest is - now read a part at a time: one value in it, such as one file's entry, may - take at most 16 MB, and the document as a whole has no limit. In a - container of 1 GB, `verify` and `cleanup --yes` work through the manifest of - 600 000 files in 10 and 24 seconds, holding at most about 280 MB. Both - commands take `--memory`, the same setting `generate` takes, and a - manifest listing more files than it lets them check is refused before any - file is looked at, with the number of files and how much they would take. - Ctrl+C while the manifest is read stops at once. The records of other runs - in the same directory are read the same way, so the files of a large - neighbour are no longer reported as `extra`. With nothing left that is too - large to read back, the warning about it is gone from both the command line - and the window, and new manifests no longer carry - `summary.too_large_to_read_back`. + They refused one larger than 16 MB, about 20 000 files, so `tfg cleanup` + could never remove the files of a larger run. One value in it, such as one + file's entry, may still take at most 16 MB. In a container of 1 GB both + read the manifest of 600 000 files holding at most about 280 MB. Ctrl+C + while it is read stops at once. The files of a large run sharing the + directory are no longer called `extra`. - **`generate --json` no longer holds a second copy of the manifest in memory to print it.** It is written out a part at a time, the same bytes as the saved manifest. diff --git a/internal/audit/neighbours.go b/internal/audit/neighbours.go index d0b189f4..c68fd73e 100644 --- a/internal/audit/neighbours.go +++ b/internal/audit/neighbours.go @@ -94,13 +94,19 @@ func findNeighbours(ctx context.Context, dir string, candidates []string) neighb // is kept only once the whole record has been read and found sound, because a // record refused at its last byte is no record, whatever its first entries // said. +// +// Each file once, however many times the record lists it, so what is kept is +// never more than the files in front of us. A record of a million entries +// naming one of them would otherwise be a list a million long. func claimsAmong(ctx context.Context, full string, unclaimed map[string]bool) ([]string, bool) { var hits []string + kept := map[string]bool{} _, err := manifest.EachClaim(ctx, full, func(c manifest.ClaimedFile) { // The path is put through the same spelling both sides of the // comparison use. Without it a neighbour writing "./a.txt" would claim // nothing, which is the fault comparablePath was written for. - if key := comparablePath(c.Path); unclaimed[key] { + if key := comparablePath(c.Path); unclaimed[key] && !kept[key] { + kept[key] = true hits = append(hits, key) } }) diff --git a/internal/guard/concurrentruns_test.go b/internal/guard/concurrentruns_test.go index 79cc3311..d99da00a 100644 --- a/internal/guard/concurrentruns_test.go +++ b/internal/guard/concurrentruns_test.go @@ -286,3 +286,27 @@ func TestOneValueOfAManifestPastTheCeilingIsRefusedAndALongManifestIsNot(t *test t.Errorf("a manifest of %d B in small entries ended %d, expected it read and %d:\n%s", len(long), code, cli.ExitOK, errOut) } } + +// And one value just short of the ceiling is read, so the ceiling is the one +// the refusal names. The guard above holds it from above, and nothing there +// would notice the reader refusing at half of it - a manifest this tool reads +// today turned away. +// +// Short by a kilobyte rather than a byte. What the reader counts against the +// ceiling is the value and the bytes either side of it - the colon before it, +// and the byte after it that says where it ends - so the exact edge moves by a +// byte or two with the shape of the value, and no reader of a manifest needs +// it to the byte. +func TestOneValueOfAManifestJustUnderTheCeilingIsRead(t *testing.T) { + path := filepath.Join(t.TempDir(), "manifest.json") + value := `{"command":""}` + padding := strings.Repeat("x", manifest.MaxValueBytes-1024-len(value)) + doc := `{"manifest_version":"1.0","files":[],"run":{"command":"` + padding + `"}}` + if err := os.WriteFile(path, []byte(doc), 0o644); err != nil { + t.Fatalf("writing: %v", err) + } + if code, _, errOut := run(t, "verify", path); code != cli.ExitOK || !strings.Contains(errOut, "nothing to check") { + t.Errorf("a manifest with one value of %d B, a kilobyte under the ceiling of %d B, ended %d, expected it read and %d:\n%s", + len(value)+len(padding), manifest.MaxValueBytes, code, cli.ExitOK, errOut) + } +} diff --git a/internal/guard/manifestreadlimits_test.go b/internal/guard/manifestreadlimits_test.go index 20bd38ff..0eb59d55 100644 --- a/internal/guard/manifestreadlimits_test.go +++ b/internal/guard/manifestreadlimits_test.go @@ -7,9 +7,11 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "testing" + "github.com/donislawdev/TestingFilesGenerator/internal/audit" "github.com/donislawdev/TestingFilesGenerator/internal/cli" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) @@ -192,3 +194,70 @@ func TestAManifestListingMoreThanTheMemorySettingLetsCleanupCheckIsRefused(t *te t.Errorf("a manifest too large for the setting and with an entry that escapes was not refused for the entry, exit %d:\n%s", code, errOut) } } + +// Another run's record that lists one file over and over holds it once. +// +// What verify keeps of another run's record is what it says about the files in +// front of it, so what it holds grows with the directory and not with the +// record. A record naming one of those files in each of its entries was kept +// as a list as long as the record. Nothing in the report tells the two apart - +// the file is another run's either way - so this asks the memory. Two records +// of the same length are read the same way, one naming the file in every entry +// and one naming files that are not there, and what reading each allocated is +// compared. Kept once, the two cost about the same. Kept every time, the first +// costs at least the sixteen bytes of a string for each entry. +// +// Not in parallel, so no other guard allocates while it counts. +func TestAnotherRunsRecordListingOneFileOverAndOverHoldsItOnce(t *testing.T) { + const entries = 50_000 + read := func(path func(int) string) (int64, []audit.Difference) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "a0000000.txt"), []byte("the other run wrote this\n"), 0o644); err != nil { + t.Fatal(err) + } + var b strings.Builder + b.WriteString(`{"manifest_version":"1.0","files":[`) + for i := range entries { + if i > 0 { + b.WriteByte(',') + } + fmt.Fprintf(&b, `{"path":%q,"materialized":true}`, path(i)) + } + if err := os.WriteFile(filepath.Join(dir, "other.json"), []byte(b.String()+"]}"), 0o644); err != nil { + t.Fatal(err) + } + runtime.GC() + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + diffs, err := audit.Verify(context.Background(), dir, manifest.Claims{}, "manifest.json") + runtime.ReadMemStats(&after) + if err != nil { + t.Fatalf("verifying: %v", err) + } + return int64(after.TotalAlloc - before.TotalAlloc), diffs + } + same, sameDiffs := read(func(int) string { return "a0000000.txt" }) + apart, apartDiffs := read(func(i int) string { return fmt.Sprintf("b%07d.txt", i) }) + + // The state this is about: both records were read to the end and taken + // for records, and only the first one claims the file. + for _, c := range []struct { + name string + diffs []audit.Difference + file audit.Kind + }{{"naming the file", sameDiffs, audit.AnotherRun}, {"naming other files", apartDiffs, audit.Extra}} { + kinds := map[string]audit.Kind{} + for _, d := range c.diffs { + kinds[d.Path] = d.Kind + } + if kinds["other.json"] != audit.AnotherRun || kinds["a0000000.txt"] != c.file { + t.Fatalf("the record %s was not read as another run's record claiming what it lists: %v", c.name, c.diffs) + } + } + + t.Logf("reading the record naming the file allocated %d B, and the record naming other files %d B", same, apart) + if grew := same - apart; grew > entries { + t.Errorf("a record naming one file in each of its %d entries took %d B more to read than one naming none of the files, "+ + "%.1f B an entry - the file was kept once for every entry rather than once", entries, grew, float64(grew)/entries) + } +} diff --git a/internal/guard/manifeststream_test.go b/internal/guard/manifeststream_test.go index 32c6ef14..c4a310a3 100644 --- a/internal/guard/manifeststream_test.go +++ b/internal/guard/manifeststream_test.go @@ -7,6 +7,8 @@ import ( "fmt" "os" "path/filepath" + "reflect" + "runtime" "strings" "testing" @@ -78,17 +80,41 @@ func (w *writeSizes) Write(p []byte) (int, error) { return len(p), nil } -// keptSizes is writeSizes that also keeps what it was handed. +// keptSizes is writeSizes that also keeps what it was handed, and whether the +// first piece arrived while Encode was still writing the document. type keptSizes struct { writeSizes - kept bytes.Buffer + kept bytes.Buffer + whileEncoding bool } func (w *keptSizes) Write(p []byte) (int, error) { + if w.writes == 0 { + w.whileEncoding = calledFrom(encodeName) + } _, _ = w.writeSizes.Write(p) return w.kept.Write(p) } +// encodeName is the name Encode goes by on a stack, taken from the method +// rather than written out, so renaming it renames this. +var encodeName = runtime.FuncForPC(reflect.ValueOf((*manifest.Manifest).Encode).Pointer()).Name() + +// calledFrom says whether the function named is among the callers. +func calledFrom(name string) bool { + pcs := make([]uintptr, 128) + frames := runtime.CallersFrames(pcs[:runtime.Callers(1, pcs)]) + for { + f, more := frames.Next() + if f.Function == name { + return true + } + if !more { + return false + } + } +} + // generate --json sends the manifest a part at a time too, and after a run what // it sends is the file the run saved. // @@ -97,6 +123,12 @@ func (w *keptSizes) Write(p []byte) (int, error) { // disk already - observation O300. Both halves: a dry run, which saves nothing, // and a real run, whose manifest is on the disk - and what that one sends is // the saved file to the byte, so what a script reads is what verify reads. +// +// The size of the pieces says the document was not handed over in one, and +// not that it was never held: encoded whole and then sent a little at a time +// gives the same pieces. What tells the two apart is when the first piece +// comes - from inside Encode, while the rest of the document is still to be +// written. func TestGenerateSendsItsManifestAPartAtATime(t *testing.T) { for _, dry := range []bool{true, false} { dir := t.TempDir() @@ -117,6 +149,10 @@ func TestGenerateSendsItsManifestAPartAtATime(t *testing.T) { t.Errorf("dry run %v: a manifest of %d B reached standard output in %d writes, the largest %d B - held whole to be sent", dry, sent.total, sent.writes, sent.largest) } + if !sent.whileEncoding { + t.Errorf("dry run %v: the manifest reached standard output after Encode had written it rather than while - "+ + "held whole to be sent, whatever the size of the pieces it went in", dry) + } if dry { if !json.Valid(sent.kept.Bytes()) { t.Errorf("the manifest a dry run sent is not a JSON document") diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index 9abb8097..ed9f20a9 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -16715,8 +16715,8 @@ ( "another run's record no longer claims the files in front of us", "internal/audit/neighbours.go", - "if key := comparablePath(c.Path); unclaimed[key] {", - "if key := comparablePath(c.Path); false && unclaimed[key] {", + "if key := comparablePath(c.Path); unclaimed[key] && !kept[key] {", + "if key := comparablePath(c.Path); false && unclaimed[key] && !kept[key] {", "TestVerifyNamesTheFilesOfAnotherRunRatherThanCallingThemExtra", ), ( @@ -16726,4 +16726,27 @@ "\t\tvar whole bytes.Buffer\n\t\tcode := sendManifest(res, g.dryRun, &whole, errOut)\n\t\t_, _ = out.Write(whole.Bytes())\n\t\tif code != ExitOK {", "TestGenerateSendsItsManifestAPartAtATime", ), + ( + "the ceiling on one value of a manifest is halved, so a value this tool reads today is refused", + "internal/manifest/read.go", + "rd.in.limit = rd.dec.InputOffset() + MaxValueBytes", + "rd.in.limit = rd.dec.InputOffset() + MaxValueBytes/2", + "TestOneValueOfAManifestJustUnderTheCeilingIsRead", + ), + ( + "another run's record listing one file over and over keeps it once for every entry", + "internal/audit/neighbours.go", + "unclaimed[key] && !kept[key] {", + "unclaimed[key] {", + "TestAnotherRunsRecordListingOneFileOverAndOverHoldsItOnce", + ), + ( + "generate --json holds the manifest whole and sends it a small piece at a time", + "internal/cli/generate.go", + "\t\tif code := sendManifest(res, g.dryRun, out, errOut); code != ExitOK {", + "\t\tvar whole bytes.Buffer\n\t\tcode := sendManifest(res, g.dryRun, &whole, errOut)\n" + "\t\tfor p := whole.Bytes(); len(p) > 0; p = p[min(len(p), 4096):] {\n\t\t\t_, _ = out.Write(p[:min(len(p), 4096)])\n\t\t}\n" + "\t\tif code != ExitOK {", + "TestGenerateSendsItsManifestAPartAtATime", + ), ] diff --git a/internal/manifest/read.go b/internal/manifest/read.go index d4b1020e..ed50a01b 100644 --- a/internal/manifest/read.go +++ b/internal/manifest/read.go @@ -47,8 +47,10 @@ const MaxValueBytes = 16 << 20 const readBuffer = 64 << 10 // errValueTooLarge is the reader under the decoder declining to hand over more -// of one value. It comes back out of the decoder as it went in. -var errValueTooLarge = errors.New("a value runs past the ceiling on one value") +// of one value. It comes back out of the decoder as it went in, and unreadable +// says it as ValueTooLargeError - so a person reading it would mean a fault +// here, which is what Defect marks. +var errValueTooLarge = core.Defect(errors.New("a value runs past the ceiling on one value")) // bounded hands the decoder the document up to a limit that is moved before // each value. The decoder keeps what it was handed and has not used yet, so it @@ -288,8 +290,9 @@ func (rd *reading) field(m *Manifest, key string) error { if err != nil { return err } - doc := make([]byte, 0, len(name)+len(raw)+3) - doc = append(append(append(append(append(doc, '{'), name...), ':'), raw...), '}') + // Sized by append. raw is at most MaxValueBytes, and lengths added up + // front are what a scanner of the code reads as a size that may overflow. + doc := append(append(append(append([]byte{'{'}, name...), ':'), raw...), '}') return rd.unreadable(json.Unmarshal(doc, m)) }