diff --git a/AGENTS.md b/AGENTS.md index 5d180ed..d504c20 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -888,7 +888,8 @@ neither one's. before touching `readyack.go`: the whole-tree presence scan described above is now the WEAK tier, reached only when the entry graph cannot be resolved, and the advisory it emits is a different string that discloses - the difference. + the difference β€” and, since #258, NAMES the resolver's own reason for + falling back instead of guessing at it. See item 20. 19. **img2img sends `workflow: "txt2img"` PLUS `images[]`, requires `--ecosystem`, and uploads with NO credential β€” three things that each read @@ -1030,6 +1031,79 @@ neither one's. project shapes is exactly how the false pass above shipped, so the strength is part of the output, not an implementation detail. `TestReadyAckAdvisoriesStateTheirOwnStrength` pins both directions. + - πŸ”΄ **AND THE WEAK TIER NAMES ITS REASON RATHER THAN GUESSING AT IT β€” THE + RESOLVER ALREADY KNEW, AND THE CHECK THREW IT AWAY.** Every gap kind + writes a precise, per-reference reason into `EntryGraph.Gaps`; + `readyAckChecks` returned the CONSTANT `readyAckAdvicePresenceOnly` and + discarded the slice, so the message offered a fixed list of plausible + causes instead β€” "there is no index.html at the project root, or it holds + a reference this CLI cannot follow β€” a bundler alias, a generated file, an + off-project URL". In the canonical #206 shape, a `static` scaffold whose + `civitai-host.js` has been deleted, **not one of those is true**: the + reason is that ``, + "main.js": `import '@/civitai-host.js';` + "\n", + }) + wantGapReport(t, gapReportFor(t, dir), + "main.js", // the file holding the reference + "@/civitai-host.js", // the specifier + "bundler alias", // the reason, now earned rather than guessed + ) + }) + + t.Run("a file the resolver could not read", func(t *testing.T) { + // A stylesheet over the per-file size cap. It has to be a NON-source + // extension: the whole-tree scan would hit the same cap on a `.js` and + // report UNOBSERVABLE, which gates both tiers and emits nothing at all. + dir := ackProject(t, ackManifest(false), map[string]string{ + "index.html": ``, + "main.js": `import './huge.css';` + "\n", + }) + big := strings.Repeat("/* pad */\n.a{color:red}\n", (maxAckFileBytes/22)+64) + if err := os.WriteFile(filepath.Join(dir, "huge.css"), []byte(big), 0o600); err != nil { + t.Fatal(err) + } + wantGapReport(t, gapReportFor(t, dir), "could not read", "huge.css") + }) + + t.Run("no index.html at the project root", func(t *testing.T) { + dir := ackProject(t, ackManifest(true), map[string]string{ + "package.json": `{"dependencies": {"next": "^15.0.0"}}`, + "app/page.tsx": `export default function Page() { return null; }`, + }) + wantGapReport(t, gapReportFor(t, dir), "index.html") + }) + + t.Run("an off-project URL in a script src", func(t *testing.T) { + dir := ackProject(t, ackManifest(false), map[string]string{ + "index.html": `` + + ``, + "app.js": `document.title = 'hi';` + "\n", + }) + wantGapReport(t, gapReportFor(t, dir), "https://cdn.example.com/x.js", "could not resolve") + }) +} + +// TestGapReportIsCappedAndSaysSo pins the truncation disclosure. +// +// πŸ”΄ A SILENTLY TRUNCATED LIST READS AS "THAT WAS ALL OF THEM" β€” the same class +// of lie as the guess this report replaced. An author fixes the three references +// they were shown, re-runs, and is told about three more that were there the +// whole time. +func TestGapReportIsCappedAndSaysSo(t *testing.T) { + const refs = readyAckGapCap + 4 + var tags strings.Builder + for i := 0; i < refs; i++ { + fmt.Fprintf(&tags, ``, i) + } + dir := ackProject(t, ackManifest(false), map[string]string{ + "index.html": `` + tags.String(), + }) + report := gapReportFor(t, dir) + + // Exactly readyAckGapCap reasons are numbered, and the numbering stops there. + for i := 1; i <= readyAckGapCap; i++ { + if !strings.Contains(report, fmt.Sprintf("(%d)", i)) { + t.Errorf("the gap report is missing reason (%d):\n%s", i, report) + } + } + if strings.Contains(report, fmt.Sprintf("(%d)", readyAckGapCap+1)) { + t.Errorf("the gap report rendered more than readyAckGapCap=%d reasons:\n%s", readyAckGapCap, report) + } + // And the overflow is COUNTED, not merely hinted at. `refs` references all + // gap, so the count is exact. + want := fmt.Sprintf("and %d more", refs-readyAckGapCap) + if !strings.Contains(report, want) { + t.Errorf("the gap report truncated silently β€” it must say %q, or an author reads three reasons "+ + "as the complete list:\n%s", want, report) + } +} + +// TestGapReportUnitCap is the same rule at the function, where the input can be +// varied freely β€” the fixture above can only produce the counts a project shape +// happens to yield. +func TestGapReportUnitCap(t *testing.T) { + if got := readyAckGapReport(nil); got != "" { + t.Errorf("no gaps must render nothing, got %q", got) + } + for _, n := range []int{1, readyAckGapCap, readyAckGapCap + 1, 47} { + gaps := make([]string, n) + for i := range gaps { + gaps[i] = fmt.Sprintf("reason-%d", i) + } + got := readyAckGapReport(gaps) + shown := min(n, readyAckGapCap) + for i := 0; i < shown; i++ { + if !strings.Contains(got, fmt.Sprintf("reason-%d", i)) { + t.Errorf("n=%d: missing reason-%d in %q", n, i, got) + } + } + if shown < n { + if !strings.Contains(got, fmt.Sprintf("and %d more", n-shown)) { + t.Errorf("n=%d: overflow of %d not disclosed: %q", n, n-shown, got) + } + if strings.Contains(got, fmt.Sprintf("reason-%d", readyAckGapCap)) { + t.Errorf("n=%d: rendered past the cap: %q", n, got) + } + } else if strings.Contains(got, "more") { + t.Errorf("n=%d: claimed an overflow with nothing withheld: %q", n, got) + } + } +} + +// TestGapReportIsOneLine pins the wire contract. `Finding.Message` is a `--json` +// string field; the human layout happens at the printer (internal/cmd), and a +// newline here would break a consumer without breaking any assertion about the +// text. A gap interpolates `%v` of an OS error, which is not guaranteed +// newline-free. +func TestGapReportIsOneLine(t *testing.T) { + got := readyAckGapReport([]string{"a reason\nsplit over\r\ntwo lines", "and\ta tab"}) + if strings.ContainsAny(got, "\n\r") { + t.Fatalf("the gap report carries a line break: %q", got) + } + for _, want := range []string{"a reason split over two lines", "and a tab"} { + if !strings.Contains(got, want) { + t.Errorf("collapsing whitespace lost content: want %q in %q", want, got) + } + } +} + +// --------------------------------------------------------------------------- +// CONTROLS. Without these, "the presence tier names its reasons" is satisfied by +// a check that reports the presence tier at every project. +// --------------------------------------------------------------------------- + +// TestGapReportDoesNotLeakIntoTheStrongTiers is the structural half of item 20's +// disclosure rule, aimed at THIS change: the reachability tiers resolved the +// graph completely, so they have no gaps to report and must not acquire the +// weak tier's apparatus. +func TestGapReportDoesNotLeakIntoTheStrongTiers(t *testing.T) { + cases := []struct { + name, kind string + build func(*testing.T) string + }{ + {"unwired", "unwired", func(t *testing.T) string { + dir := renderTemplate(t, scaffold.Static) + editFile(t, dir, "index.html", ``, "") + return dir + }}, + {"missing", "missing", func(t *testing.T) string { + dir := renderTemplate(t, scaffold.Static) + editFile(t, dir, "index.html", ``, "") + if err := os.Remove(filepath.Join(dir, blockproto.ReadyAckFilename)); err != nil { + t.Fatal(err) + } + return dir + }}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + res := wantAckKind(t, c.build(t), c.kind) + for _, w := range res.Warnings { + if isPresenceOnlyAdvice(w.Message) { + t.Fatalf("the %s tier emitted a presence-tier message", c.name) + } + if strings.Contains(w.Message, readyAckGapLead) { + t.Fatalf("the %s tier carries the gap report's lead-in β€” it resolved the graph completely, "+ + "so it has nothing it could not follow, and saying otherwise blurs the two tiers:\n%s", + c.name, w.Message) + } + } + }) + } +} + +// TestGapReportNeverAppearsAtACorrectProject is the other control: the shipped +// templates resolve completely and must stay silent, gap report or not. +func TestGapReportNeverAppearsAtACorrectProject(t *testing.T) { + examined := 0 + for _, tmpl := range scaffold.AllTemplates() { + t.Run(string(tmpl), func(t *testing.T) { + wantAckKind(t, renderTemplate(t, tmpl), "") + }) + examined++ + } + if examined < 3 { + t.Fatalf("examined only %d template(s) β€” the enumeration stopped OBSERVING", examined) + } +} + +// TestPresenceAdviceHalvesBracketTheReport pins the assumption every matcher in +// this package now rests on: the head and the tail are non-empty and neither is +// a prefix of the other tiers' messages, so bracketing really identifies the +// presence tier. +// +// πŸ”΄ An empty half silently disarms `isPresenceOnlyAdvice` β€” +// `strings.HasPrefix(x, "")` is always true β€” which would classify BOTH +// reachability tiers as presence-only and make every wantAckKind in this package +// assert the wrong thing while staying green. +func TestPresenceAdviceHalvesBracketTheReport(t *testing.T) { + if readyAckAdvicePresenceOnlyHead == "" || readyAckAdvicePresenceOnlyTail == "" { + t.Fatal("a half of the presence advisory is empty; isPresenceOnlyAdvice matches everything") + } + if got := readyAckAdvicePresenceOnlyHead + readyAckAdvicePresenceOnlyTail; got != readyAckAdvicePresenceOnly { + t.Fatal("readyAckAdvicePresenceOnly is not the concatenation of its two halves β€” the ledger entry " + + "and the emitted message have drifted apart") + } + if !isPresenceOnlyAdvice(readyAckAdvicePresenceOnly) { + t.Fatal("the no-gaps presence message is not recognised as one") + } + if !isPresenceOnlyAdvice(presenceOnlyAdvice([]string{"x"})) { + t.Fatal("a presence message WITH a gap report is not recognised as one") + } + for name, other := range map[string]string{"unwired": readyAckAdviceUnwired, "missing": readyAckAdviceMissing} { + if isPresenceOnlyAdvice(other) { + t.Errorf("the %s advisory is bracketed by the presence tier's halves β€” the tiers are no longer "+ + "distinguishable, and readyAckKind reports the wrong one", name) + } + } +} + +// TestGapReportCannotSatisfyAnotherTiersStrengthAssertion is the case +// TestReadyAckAdvisoriesStateTheirOwnStrength cannot make, because that test +// operates on the FIXED bases and this change adds text at runtime. +// +// πŸ”΄ The inverse assertions there are what stop the tiers blurring together, and +// they would keep passing if the appended report happened to carry another +// tier's own literal β€” the assertion never sees an emitted message. So the +// literals are re-checked against a REAL rendered advisory here. +func TestGapReportCannotSatisfyAnotherTiersStrengthAssertion(t *testing.T) { + dir := renderTemplate(t, scaffold.Static) + if err := os.Remove(filepath.Join(dir, blockproto.ReadyAckFilename)); err != nil { + t.Fatal(err) + } + report := gapReportFor(t, dir) + // These are TestReadyAckAdvisoriesStateTheirOwnStrength's `own` literals for + // the two reachability tiers. None may appear in the presence tier's report. + for _, lit := range []string{ + "DOES contain", + "nothing index.html loads reaches it", + "orphan", + "nothing index.html loads posts it either", + } { + if strings.Contains(report, lit) { + t.Errorf("the gap report carries %q, which is another tier's whole diagnosis β€” a reader can no "+ + "longer tell which check ran:\n%s", lit, report) + } + } + // And the weak tier's own disclosure survives the splice, in the emitted + // message rather than only in the constant. + res := wantAckKind(t, dir, "presence-only") + for _, w := range res.Warnings { + if !isPresenceOnlyAdvice(w.Message) { + continue + } + for _, want := range []string{"did NOT check that the file is loaded", "will silence this warning"} { + if !strings.Contains(w.Message, want) { + t.Errorf("the emitted presence advisory lost %q β€” the disclosure is the fix, not a nicety", want) + } + } + } +} diff --git a/internal/validate/readyack_test.go b/internal/validate/readyack_test.go index a60b5ec..f75f9fe 100644 --- a/internal/validate/readyack_test.go +++ b/internal/validate/readyack_test.go @@ -92,20 +92,46 @@ func ackProject(t *testing.T, manifestJSON string, files map[string]string) stri func hasReadyAckWarning(res Result) bool { return readyAckKind(res) != "" } // readyAckKind names the tier that fired, or "". +// +// πŸ”΄ THE PRESENCE TIER IS NOT AN EQUALITY MATCH, AND MATCHING IT LIKE ONE MAKES +// THIS HELPER BLIND TO THE ONE TIER IT MOST NEEDS TO SEE. Its real message is +// `presenceOnlyAdvice(gaps)` β€” the base with the resolver's own reasons spliced +// into the middle (issue #258) β€” so `w.Message == readyAckAdvicePresenceOnly` is +// true only for a graph that recorded no reason at all, which no real project +// produces. Every wantAckKind(…, "presence-only") in this package would then +// report "" and read as "the check did not fire". It is bracketed instead. func readyAckKind(res Result) string { for _, w := range res.Warnings { - switch w.Message { - case readyAckAdviceUnwired: + switch { + case w.Message == readyAckAdviceUnwired: return "unwired" - case readyAckAdviceMissing: + case w.Message == readyAckAdviceMissing: return "missing" - case readyAckAdvicePresenceOnly: + case isPresenceOnlyAdvice(w.Message): return "presence-only" } } return "" } +// isPresenceOnlyAdvice reports whether msg is the presence tier's message, with +// or without a gap report between its two fixed halves. +func isPresenceOnlyAdvice(msg string) bool { + return strings.HasPrefix(msg, readyAckAdvicePresenceOnlyHead) && + strings.HasSuffix(msg, readyAckAdvicePresenceOnlyTail) +} + +// presenceOnlyGapReport returns the gap report spliced into msg, which must be a +// presence-tier message. It is what lets a test assert on the REASONS without +// re-stating the surrounding prose. +func presenceOnlyGapReport(t *testing.T, msg string) string { + t.Helper() + if !isPresenceOnlyAdvice(msg) { + t.Fatalf("not a presence-tier advisory:\n%s", msg) + } + return strings.TrimSuffix(strings.TrimPrefix(msg, readyAckAdvicePresenceOnlyHead), readyAckAdvicePresenceOnlyTail) +} + // wantAckKind asserts the exact tier. `want` of "" means no ready-ack warning. func wantAckKind(t *testing.T, dir, want string) Result { t.Helper()