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