From 2b5f7324826a8742db4e76a804c05e5eea627fae Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 7 Sep 2026 07:14:47 +0200 Subject: [PATCH] fix: a manifest too big to read back says so in the report a script reads A run of 25 000 files writes a manifest of about 25.9 MB against a 16 MB reading limit, so verify and cleanup both exit 5 on it. The run itself ends 0, correctly - every file was produced and every one is right. What is gone is the only authority over what may be deleted, because untouchable rule 7 makes the manifest that authority. The warning existed and was correct: a sentence on standard error, printed first. Two measurements say why that was not enough. It was one line among 25 003, carrying the same "note:" prefix as 25 000 repetitions of a sentence about labels. And the machine readable report carried no trace of it at all - --json came back clean, exit 0, nothing. This tool plugs into CI, so a fact only a person reading prose can learn is a fact a pipeline cannot act on. Not an exit code, and that was settled with a citation rather than a preference. docs/CLI.md defines 8 as "run finished, but not everything was produced", and here everything was produced: res.Failures is nought, so the sentence would read "0 files could not be produced. The manifest says which ones" - false twice over and pointing at a manifest nothing can read. Exit codes are a frozen contract. Refusing the run was considered and had already been decided against, by the owner on 2026-08-26. It is written in the comment on the guard beside this one: "a note rather than a refusal... What was missing was that nobody was told." The field is absent on an ordinary run rather than false, which is why manifest_version stays 1.0. Measured against the previous binary: an ordinary manifest is identical and all 20 files are byte for byte the same. Third case under "the schema grows by adding fields", after tool.go and by_target. Two mutations, both caught. The second takes off omitempty, because a build writing the field into every manifest would satisfy the first guard while changing the bytes of every document this tool has ever written. Found while doing this and recorded as O184, not fixed here: the window does not warn about the ceiling at all. TooLargeToReadBack has zero hits in internal/gui, so somebody generating 25 000 files from the window is told nothing. Closing it properly means deciding where a run level note lives, and notes are per file today. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 15 ++++ internal/guard/manifestreach_test.go | 102 +++++++++++++++++++++++++++ internal/manifest/manifest.go | 48 +++++++++++++ 3 files changed, 165 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53c1533..1c29406 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,6 +299,21 @@ because it turns other people's test suites red. ### Fixed +- **A manifest too big for this build to read back now says so in the report a + script reads.** A run of 25 000 files writes a manifest of about 25.9 MB + against a 16 MB reading limit, so `tfg verify` and `tfg cleanup` both refuse + it - and the manifest is the only authority over what may be deleted. The run + still ends `0`, because the files are correct and complete. + + Until now the only warning was a sentence on standard error. `--json` carried + no trace of it, so a CI job had no way to learn that the directory it had just + filled could never be verified or cleaned up by this build. The manifest now + carries `summary.too_large_to_read_back`. + + The field is **absent** on an ordinary run rather than `false`, so every + manifest already written is byte for byte what it was, and + `manifest_version` stays `1.0`. + - **A stopped run says what happened and what survived, instead of `context canceled`.** Pressing Ctrl+C, or a CI job running out of time, printed six characters of Go vocabulary and left you to work out whether the directory was diff --git a/internal/guard/manifestreach_test.go b/internal/guard/manifestreach_test.go index 65ca971..d158832 100644 --- a/internal/guard/manifestreach_test.go +++ b/internal/guard/manifestreach_test.go @@ -3,6 +3,7 @@ package guard import ( "bytes" "context" + "encoding/json" "os" "path/filepath" "strings" @@ -106,3 +107,104 @@ func runCLI(t *testing.T, args ...string) string { cli.Run(context.Background(), args, &out, &errOut) return out.String() + errOut.String() } + +// 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) { + over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 + + // Asked of the estimator first, so a build whose ceiling moved above this + // count fails here rather than passing the whole test by never being over + // it at all. + if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { + t.Fatalf("%d entries was not judged too large, so this proved nothing", over) + } + + // stdout only. The summary line goes to the error stream, and a reader that + // took both would be parsing JSON with a sentence stuck to the end of it. + code, said, errOut := run(t, "generate", "--format", "txt", "--size", "200b", + "--count", itoa(over), "--dry-run", "--json", "--out", t.TempDir()) + 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.") + } +} + +// firstLines trims a long document down for a failure message. +func firstLines(s string, n int) string { + lines := strings.Split(s, "\n") + if len(lines) > n { + lines = lines[:n] + } + return strings.Join(lines, "\n") +} diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index 9e534bc..a2b5744 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -38,6 +38,16 @@ type Manifest struct { Summary Summary `json:"summary"` Files []File `json:"files"` + + // notedFiles is how many entries carry a note, which the size estimate + // needs and no reader does. + // + // Unexported, so it is not written and not read back. A manifest decoded + // from disk carries the ANSWER in Summary.TooLargeToReadBack rather than + // the working needed to reach it, which is what a reader wants and is also + // what keeps the two from disagreeing - a decoded manifest has no Add + // calls to count. + notedFiles int } // Tool records what produced these bytes. Without it a hash mismatch after an @@ -142,6 +152,36 @@ 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. + TooLargeToReadBack bool `json:"too_large_to_read_back,omitempty"` } // File is one entry. @@ -298,6 +338,14 @@ func (m *Manifest) Add(f File) { if _, ok := m.Tool.Generators[f.Format]; !ok && f.Generator.Version != "" { m.Tool.Generators[f.Format] = f.Generator.Version } + // Kept in step here for the reason the comment above this function gives: + // counting anywhere else is a second place for the two to disagree. The + // estimate is arithmetic over the counts, so recomputing it per file costs + // nothing and is always right about the document as it stands. + if len(f.Notes) > 0 { + m.notedFiles++ + } + _, m.Summary.TooLargeToReadBack = TooLargeToReadBack(len(m.Files), m.notedFiles) } // noteExamples is how many file names a grouped note shows before it stops