From 6481ac9636af53aafff894472dccc76a89a094cb Mon Sep 17 00:00:00 2001 From: ZacxDev Date: Fri, 7 Aug 2026 15:07:01 -0500 Subject: [PATCH 1/2] fix(validate): the ready-ack advisory names the reason it fell back, and wraps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The presence-tier advisory GUESSED at why the entry graph had not resolved — "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" — while `EntryGraph.Gaps` already held the real, per-reference reason and readyack.go discarded it. In the canonical #206 shape (a `static` scaffold whose `civitai-host.js` has been deleted) none of the guesses is true: index.html plainly references a file that is not there. A five-file no-build app sent its author hunting for a bundler alias that cannot exist in it. Issue #258. The gaps are now surfaced GENERALLY rather than special-casing the dangling reference, so all six gap kinds reach the author at once — capped at 3 with the overflow counted out loud, because a silently truncated list reads as "that was all of them". The tiering is unchanged: AGENTS.md item 20's judgement that a missing target is a GAP rather than a decided absence stands. The message was the defect. Second half: `app validate` printed the ~2 kB advisory as ONE 1938-character line. Layout now happens at the printer (internal/cmd/validate_print.go), which fixes every finding rather than the one that provoked it. Wrapping inside the message would have corrupted `--json` — the inverse of item 23: the field comes from the producer, the layout does not. Co-Authored-By: Claude Opus 5 (1M context) --- internal/blockproto/entrygraph.go | 20 +- internal/cmd/app_validate.go | 4 +- internal/cmd/app_validate_lockfile_test.go | 7 +- internal/cmd/app_validate_wrap_test.go | 176 ++++++++++ internal/cmd/validate_print.go | 58 ++++ internal/validate/readyack.go | 146 +++++++- internal/validate/readyack_gaps_test.go | 385 +++++++++++++++++++++ internal/validate/readyack_test.go | 34 +- 8 files changed, 804 insertions(+), 26 deletions(-) create mode 100644 internal/cmd/app_validate_wrap_test.go create mode 100644 internal/cmd/validate_print.go create mode 100644 internal/validate/readyack_gaps_test.go diff --git a/internal/blockproto/entrygraph.go b/internal/blockproto/entrygraph.go index 615ef6e..e753479 100644 --- a/internal/blockproto/entrygraph.go +++ b/internal/blockproto/entrygraph.go @@ -176,6 +176,16 @@ type EntryGraph struct { // must never be read as evidence that something is absent. Complete bool // Gaps explains, in author-readable terms, why Complete is false. + // + // 🔴 THESE STRINGS ARE PRINTED TO AUTHORS, not just to a test failure + // message. `internal/validate`'s presence-tier advisory renders them + // (issue #258: it used to GUESS at why it had fallen back — "a bundler + // alias, a generated file, an off-project URL" — while the real reason sat + // here and was discarded, so a five-file no-build app was sent hunting for + // a bundler alias that cannot exist in it). Word a new gap for the author + // who has to fix it: name the referencing file, the specifier, and the + // edit. Each one must be a SINGLE LINE — the advisory rides in a + // `--json` message field. Gaps []string // Trace lists every reference considered and what it resolved to, for // error messages that name the near-miss instead of saying "nothing". @@ -327,7 +337,15 @@ func ResolveEntryGraph(dir string, opts EntryGraphOptions) *EntryGraph { // the project. That is a GAP, not "the browser 404s it": treating // it as the latter would let a mis-modelled project produce a // confident finding. - g.gap("%s %q resolves to %s, which does not exist — this resolver's model of the project is incomplete", + // + // 🔴 THE WORDING IS AUTHOR-FACING, because `Gaps` is rendered into + // the ready-ack advisory an author reads (issue #258). This is the + // gap the canonical #206 shape produces — a `static` scaffold whose + // `civitai-host.js` was deleted — so it must name the referencing + // file, the specifier and the missing target, and say what to do. + // "this resolver's model of the project is incomplete", the tail it + // used to carry, is a fact about US that an author cannot act on. + g.gap("%s %q points at %s, which does not exist — restore that file or fix the reference", p.what, p.spec, relTo(dir, resolved)) continue } diff --git a/internal/cmd/app_validate.go b/internal/cmd/app_validate.go index 7335b42..dfe4686 100644 --- a/internal/cmd/app_validate.go +++ b/internal/cmd/app_validate.go @@ -99,7 +99,7 @@ Defaults to the current directory.`, if !res.OK() { fmt.Fprintln(errw, ui.For(errw).ErrorMsg(fmt.Sprintf("%d validation error(s) in %s:", len(res.Errors), dir))) for _, e := range res.Errors { - fmt.Fprintf(errw, " - %s\n", e.Message) + printFinding(errw, e.Message) } // Surface warnings too — they're useful context even on a failure. printWarnings(errw, res) @@ -158,6 +158,6 @@ func printWarnings(w io.Writer, res validate.Result) { } fmt.Fprintln(w, ui.For(w).Warn(fmt.Sprintf("%d warning(s):", len(res.Warnings)))) for _, warn := range res.Warnings { - fmt.Fprintf(w, " - %s\n", warn.Message) + printFinding(w, warn.Message) } } diff --git a/internal/cmd/app_validate_lockfile_test.go b/internal/cmd/app_validate_lockfile_test.go index d04b35b..982951b 100644 --- a/internal/cmd/app_validate_lockfile_test.go +++ b/internal/cmd/app_validate_lockfile_test.go @@ -45,7 +45,12 @@ func TestAppValidateFailsOnPnpmLockWithNpmBuildCommand(t *testing.T) { `"outputDir"`, "npm install", } { - if !strings.Contains(stderr, want) { + // unwrapFinding because the printer WRAPS a finding to the terminal + // width (validate_print.go) — the message is one line on the wire and + // several on screen, so a raw substring test here is really asserting + // where the layout chose to break. `"buildCommand": "pnpm run build"` + // straddled a break the moment wrapping landed. + if !strings.Contains(unwrapFinding(stderr), want) { t.Errorf("validate stderr missing %q:\n%s", want, stderr) } } diff --git a/internal/cmd/app_validate_wrap_test.go b/internal/cmd/app_validate_wrap_test.go new file mode 100644 index 0000000..dc5c4e2 --- /dev/null +++ b/internal/cmd/app_validate_wrap_test.go @@ -0,0 +1,176 @@ +package cmd + +// app_validate_wrap_test.go pins the OTHER half of issue #258: the ready-ack +// advisory is a ~2 kB paragraph, and `app validate` printed it as ONE +// 1938-character line. +// +// 🔴 THE TWO SURFACES HAVE OPPOSITE REQUIREMENTS, AND ONE FIX CANNOT SERVE BOTH +// FROM THE PRODUCER. `--json` needs the message to stay a single line — it is a +// string field a consumer reads — while the terminal needs it broken to a width +// the producer cannot know. So the layout lives at the printer +// (validate_print.go) and the message stays flat, which is the inverse of +// AGENTS.md item 23's rule for a finding's `Field`. Both directions are asserted +// here, because either alone is satisfied by a broken fix: wrap in the message +// and the text test passes while `--json` is corrupted; wrap nowhere and the +// `--json` test passes while the terminal is unreadable. + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/civitai/cli/internal/blockproto" + "github.com/civitai/cli/internal/scaffold" +) + +// prefixScaffold renders `static` and deletes the emitter — the canonical #206 +// project, and the one that produces the longest message this CLI emits. +func prefixScaffold(t *testing.T) string { + t.Helper() + dir := filepath.Join(t.TempDir(), "block") + if _, err := scaffold.Render(scaffold.Static, dir, scaffold.Data{Slug: "wrap-block", Name: "Wrap Block"}); err != nil { + t.Fatalf("render: %v", err) + } + if err := os.Remove(filepath.Join(dir, blockproto.ReadyAckFilename)); err != nil { + t.Fatal(err) + } + return dir +} + +// TestValidateJSONMessagesAreOneLine is the wire contract. +// +// 🔴 IT DECODES. Per AGENTS.md item 23, a `strings.Contains(out, …)` over raw +// `--json` stdout cannot tell a real newline inside a message from the `\n` +// escape `encoding/json` writes for one — the escape is what a naive text test +// sees, and it looks identical either way. So the payload is unmarshalled and +// the decoded Go string is checked. +func TestValidateJSONMessagesAreOneLine(t *testing.T) { + dir := prefixScaffold(t) + stdout, _, err := run(t, "app", "validate", dir, "--json") + if err != nil { + t.Fatalf("validate --json: %v\n%s", err, stdout) + } + var payload map[string]any + if jerr := json.Unmarshal([]byte(stdout), &payload); jerr != nil { + t.Fatalf("--json output is not valid JSON: %v\n%s", jerr, stdout) + } + + checked := 0 + for _, key := range []string{"errors", "warnings"} { + list, _ := payload[key].([]any) + for _, item := range list { + m, ok := item.(map[string]any) + if !ok { + t.Fatalf("%s[] element is not an object: %#v", key, item) + } + msg, ok := m["message"].(string) + if !ok { + t.Fatalf("%s[] element has no string message: %#v", m, m) + } + if strings.ContainsAny(msg, "\n\r") { + t.Errorf("a %s message carries a line break — wrapping belongs at the printer, not in the "+ + "message, or every --json consumer gets a multi-line string field:\n%q", key, msg) + } + checked++ + } + } + // POSITIVE CONTROL. A zero here is indistinguishable from a project that + // produced no findings at all, and this fixture must produce the ready-ack + // advisory — the longest message the CLI emits, and the one that provoked + // this test. + if checked == 0 { + t.Fatal("the fixture produced NO findings, so every assertion above is vacuous — a `static` scaffold " + + "with its emitter deleted must warn") + } + if !strings.Contains(stdout, "BLOCK_READY") { + t.Fatalf("the ready-ack advisory is absent from --json; this test is checking the wrong findings:\n%s", stdout) + } +} + +// TestValidateTextOutputIsWrapped is the terminal half. +func TestValidateTextOutputIsWrapped(t *testing.T) { + dir := prefixScaffold(t) + stdout, stderr, err := run(t, "app", "validate", dir) + if err != nil { + t.Fatalf("validate: %v\n%s\n%s", err, stdout, stderr) + } + + longest, lines := 0, 0 + advisoryLines := 0 + inAdvisory := false + for _, line := range strings.Split(strings.TrimRight(stderr, "\n"), "\n") { + lines++ + if n := len([]rune(line)); n > longest { + longest = n + } + if strings.HasPrefix(line, findingBullet) { + inAdvisory = strings.Contains(line, "page") + } + if inAdvisory && strings.HasPrefix(line, " ") { + advisoryLines++ + } + } + if lines == 0 { + t.Fatal("validate printed nothing to stderr — the fixture is not warning and this test is vacuous") + } + if longest > findingWrapWidth { + t.Errorf("a stderr line is %d runes wide, over the %d-rune budget — the finding was not wrapped:\n%s", + longest, findingWrapWidth, stderr) + } + // 🔴 The WIDTH assertion alone is satisfied by a build that prints nothing + // long, so require the advisory to have actually been BROKEN UP. It is ~2 kB; + // at this width that is tens of lines, and any single-digit count means the + // message shrank rather than wrapped. + if advisoryLines < 10 { + t.Errorf("the ready-ack advisory occupies only %d line(s) — it is ~2 kB, so this is not a wrapped "+ + "paragraph:\n%s", advisoryLines, stderr) + } + + // The content survives the layout: unwrapping reproduces the logical message, + // including the issue-#258 gap report naming the reference that broke. + flat := unwrapFinding(stderr) + for _, want := range []string{ + "did NOT check that the file is loaded", + `index.html `, + "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() From c3ddc6b014fc2f21d1179abdb06faf56c2f4b6ef Mon Sep 17 00:00:00 2001 From: ZacxDev Date: Fri, 7 Aug 2026 15:20:28 -0500 Subject: [PATCH 2/2] test(validate): read the WHOLE message for the guess, not just the gap report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A mutation found the hole: restoring the shipped speculation to `readyAckAdvicePresenceOnlyHead` while KEEPING the real reasons — the most likely way #258 regresses — reddened 0 subtests, because the absence assertion was scoped to the gap report. `TestPresenceAdviceNoLongerSpeculates` now reads the emitted message at a fixture where every quoted phrase is provably impossible, with a positive control so "says none of the wrong things" cannot be satisfied by a message that says nothing. Records the measurement, the tiering-unchanged boundary and the mutation matrix in AGENTS.md items 18 and 20. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 76 ++++++++++++++++++++++++- internal/validate/readyack_gaps_test.go | 62 +++++++++++++++++--- 2 files changed, 129 insertions(+), 9 deletions(-) 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 `