fix(#260): a schema pattern error names its rule, a bad manifest names its line, a busy dir names its remedy

[ZacxDev]

Closes three of the four remaining code items under #260 § 7. All three were terse rather than wrong, so every fix APPENDS to the existing message and none replaces it.

Base: 8ec3cb0 (rebased onto the current origin/main twice — it moved under this branch mid-session).

What this closes, and what remains

#260 § 7 bullet	state
Schema pattern errors are raw where semantic errors are excellent	closed here
Malformed JSON gives no location	closed here
<dir> is not empty carries no remedy	closed here
civitai app package does not exist	recommendation below, no code
--from ships a TODO(server): note	already closed by #267 (merged, 01c486e)
app create --help "validates clean"	already closed by #267
unknown flag offers no next step	already fixed on main — verified live
app validate's exit code undocumented	already fixed on main — see caveat below
app metrics recommends a login that will be refused	already fixed on main — verified

Umbrella items 1–6 of #260 (README TOC, undocumented list/view/pull, listing-media on-ramp) are untouched.

Verification of the three "already done" claims

• unknown flag hint — confirmed live: civitai app validate --stict → Error: unknown flag: --stict / Run 'civitai app validate --help' for the available flags, rc 2.
• app metrics names the personal API key — confirmed in source and pinned by internal/cmd/app_metrics_credential_test.go: appMetricsCredentialRoute = "a full-scope personal API key …", and the no-token error says a browser login is 403-refused there.
• app validate's exit code — confirmed, but it is documented in civitai --help (the root exit-code taxonomy from exitCodeDocs), not in civitai app validate --help. app validate --help still only says "Warnings do NOT fail validation (exit 0) unless --strict is passed" and never states that an invalid manifest exits 1. Deliberately left alone here — it is a docs decision, not a defect, and the contract is published. Flagging it so the umbrella can record it as closed-with-a-caveat rather than closed.

---

1. A pattern violation now names its rule and shows a valid value

Before (main):

  - blockId: 'My First App!' does not match pattern '^[a-z][a-z0-9-]*[a-z0-9]$'
  - contentRating: value must be one of 'g', 'pg', 'pg13', 'r', 'x'

After:

  - blockId: 'My First App!' does not match pattern '^[a-z][a-z0-9-]*[a-z0-9]$'
    — must be lowercase letters, digits and hyphens only, starting with a
    letter and ending with a letter or digit (example: "my-first-app")
  - contentRating: value must be one of 'g', 'pg', 'pg13', 'r', 'x'

internal/validate/pattern.go. Three decisions that will attract a "fix":

• 🔴 The table is keyed on the REGEX SOURCE, not the field. Keying on the field is a second hand-maintained map of which fields carry a pattern — wrong the moment the schema moves one, and blind to a pattern reached through $ref or reused on two fields. One gloss covers all seven patterns wherever the schema applies them.
• 🔴 It is APPENDED, never substituted. The library's sentence carries the offending value and the authoritative regex — the only parts of that message that are not our opinion.
• 🔴 It FAILS SOFT. An unglossed pattern emits exactly today's message. That is what makes it safe in front of a vendored mirror (AGENTS item 1): a schema/ sync that adds a pattern degrades to terse, never to a stale English claim. TestPatternRulesCoverTheVendoredSchema is a bidirectional ledger that keeps the two in step anyway — it fails when the schema grows a pattern with no gloss and when a gloss names a pattern the schema no longer has.

Residual, stated rather than hidden: outputDir's four not: {"pattern": …} sub-schemas are deliberately unglossed and cannot be glossed — a failing not surfaces as kind.Not, whose message is the bare not failed with no keyword path, no regex and no value. buildCoherence covers the two shapes it models (leading /, ..); outputDir: not failed still reaches the author for a backslash separator and a Windows drive prefix. Fixing that needs the schema's $comment surfaced through the library, which it is not.

2. A malformed manifest names its line and column

Before:

  - block.manifest.json is not valid JSON: invalid character '}' looking for
    beginning of object key string

After:

  - block.manifest.json is not valid JSON at line 4, column 1: invalid
    character '}' looking for beginning of object key string

internal/manifest/jsonloc.go, one composer (invalidJSON) that every manifest decode goes through — Load, both of LoadRaw's decodes, and SetBlockID's fallback. Both offset-bearing decoder errors are handled (*json.SyntaxError and *json.UnmarshalTypeError, the latter reachable because LoadRaw decodes the same bytes twice).

🔴 The offset is a BYTE count and a column is a CHARACTER count. Slice by bytes, count runes. The two agree on every ASCII manifest, so a single fixture cannot see the bug — it appears the moment an author writes an accented display name, an em dash or an emoji, and a rune-indexed slice moves the line, not just the column. Measured live, aaaaaaaa vs Café — 🚀 (8 runes each, 8 vs 14 bytes) with the defect after them on line 3:

ASCII      → block.manifest.json is not valid JSON at line 3, column 21: …
multi-byte → block.manifest.json is not valid JSON at line 3, column 21: …

3. The non-empty-directory refusal names its remedy

Before:

Error: /tmp/…/m is not empty — refusing to overwrite

After (identical for app create and app init, rc 1 in both, unchanged):

Error: /tmp/…/m is not empty — refusing to overwrite. Scaffold somewhere else
(`--dir <new path>`, or a different name), or remove the directory first —
there is no --force

scaffold.NotEmptyRemedy, a named constant so the message, the README bullet and the guards cannot drift. The exit code does not move (a real directory is not item 26's usage error) and that is asserted with errors.Is(err, ErrUsage) per AGENTS item 7, across both scaffold verbs.

---

Mutation matrix

15 mutations + a null mutant, each checksum-gated (an edit that did not land reports BROKEN rather than reading as a survivor) and each run against a clean-tree gate before and after. Leaf --- FAIL subtests counted from -v output, never an exit code. Run over internal/{validate,cmd,manifest,scaffold}, re-run after make fmt, and re-run again on the rebased tree — numbers below are from the final tree.

#	mutation	leaf FAILs
M1	gloss DELETED (schemaErrors stops appending)	9
M2	gloss SUBSTITUTED for the library sentence	10
M3	two gloss rows SWAPPED (blockId ↔ version)	4
M4	blockId gloss row DELETED — ledger, growth direction	6
M5	a STALE gloss row the schema does not have — ledger, shrink direction	3
M6	an EXAMPLE that does not satisfy its own pattern	4
M7	gloss EVERY schema kind, not just kind.Pattern	2
M8	JSON: never attach a location	13
M9	JSON: count the column in BYTES — the byte-vs-rune bug	3
M10	JSON: treat the byte offset as a RUNE index	5
M11	JSON: do not step back past the offending byte	15
M12	JSON: wire invalidJSON into only SOME decode sites	1
M13	scaffold: drop the remedy	3
M14	scaffold: drop the "no --force" clause	3
M15	the refusal becomes a USAGE error (exit 2)	2
NULL	comment-only null mutant	0 (must survive)

🔴 Two rows were fixed because the matrix found them resting on a single subtest — the "a battery rested on one row" shape AGENTS items 24 and 26 record:

• M9 reddened exactly 1 leaf on the first pass. The internal/cmd pair that looked like a second backstop put its multi-byte text on line 2 and its defect on a bare } on line 4, where the column is 1 either way — it read like a discriminator and observed nothing. Both packages now carry a second pair whose defect sits after the multi-byte run on the same line, plus a count floor. M9 → 3 leaves across 2 packages.
• M3 left TestPatternFindingsCarryTheRuleAndAnExample entirely GREEN. Its expectation is derived from patternRules, so it moved with the mutation — and so did its cross-row denial. TestPatternGlossesAreTheRightWayRound now spells each rule fragment and example independently, from what the regex means. M3 → 4 leaves.
• M15 reddened exactly 1 leaf (only app create was driven). app init — the verb the README's Troubleshooting entry names — is now driven too.

Verification

• make ci counted on the rebased tree: 18 ok packages, 0 --- FAIL, 0 build failed, 0 no test files, 0 timeout panics, 0 ^FAIL.
• gofmt -s -l . silent. Positive control: a deliberately-unformatted file dropped in the tree was listed, then removed and the tree re-checked clean — so the silence is "all clean", not "scanned nothing".
• make lint — golangci-lint is not on this box (the Makefile errors rather than falling back), so it was run under nix-shell -p golangci-lint: v2.12.2, 0 issues. Positive control: a probe file with an unused func and a bad Sprintf produced 2 issues, then was removed.
• All three symptoms reproduced end-to-end against the built binary before and after (output above). Exit codes: pattern 1, malformed JSON 1, non-empty dir 1 — all unchanged.
• The "before" measurements were taken at e34f598; git diff e34f598 origin/main -- internal/validate/validate.go internal/manifest/ internal/scaffold/scaffold.go is empty, so they are still valid for this base.

---

Recommendation 1 — --force: do not add it

Not implemented, per instruction, and I would not implement it.

The refusal is not an inconvenience with a missing escape hatch — it is the only thing standing between a mistyped --dir and an unrecoverable overwrite of a directory the CLI knows nothing about. The two remedies now printed cover every case a --force would: scaffold elsewhere, or delete first (which is at least an explicit, reviewable act). What was actually broken was that the CLI never said so, which is what this PR fixes.

If it is ever added, the shape that would be defensible is not a blanket --force: it would have to refuse unless the target already holds a block.manifest.json at its root, so it can only ever re-scaffold an app the author already has — never an arbitrary directory. scaffold.NotEmptyRemedy is the single place the "there is no --force" claim lives, so that change is one constant plus a guard.

Recommendation 2 — civitai app package: add a hint, not a command and not an alias

Not implemented, per instruction. My recommendation, in order of preference:

Add a "moved verb" hint to the existing unknownSubcommandError seam (internal/cmd/root.go), a static map from verbs a newcomer will try (package, zip, bundle, build, publish, deploy) to the real invocation. Today:

$ civitai app package
Error: unknown command "package" for "civitai app"
Run 'civitai app --help' for the available subcommands.

Cobra's Levenshtein suggester is no help — package is nowhere near submit.

Why a hint and not the alternatives:

• A real app package command duplicates a pipeline that must stay in lockstep with submit's — the item-26 resolveProjectDir gate, validate, the item-20 ready-ack advisory print, pkgzip, --out, --skip-validate. Two entry points into one pipeline is the "one rule, one place" hazard, and submit's is the money-adjacent copy.
• An alias has the same drift at the flag level. Which of submit's flags does package accept? "All of them" makes civitai app package --yes a thing that either submits (dangerous) or errors confusingly; a curated subset is a second flag set to maintain.
• Docs alone do not fix it. The user typed app package because they had not found it in the docs. That said, the README's app submit row should also spell the word "package" so grep package README.md lands — it currently only says --package-only inside the flag list.
• The actual harm is one round-trip of discoverability. A hint costs ~10 lines at a seam that already exists, is deterministic (no prose heuristics), fails soft (an unlisted verb behaves exactly as today), and adds no second entry point.

If you would rather do nothing, that is defensible too — the cost is genuinely small and --package-only is documented. What I would not do is add the command or the alias.

---

Note for whoever owns AGENTS.md: it is 236 bytes from its own ceiling

This PR does not touch AGENTS.md, and that is a forced choice worth surfacing.

I wrote an item 28 for these three decisions. agents_size_test.go (new, from #290) failed and named it: origin/main at 8ec3cb0 is 67,799 bytes against a 68,000-byte cap — 236 bytes of headroom. Even a one-line item does not fit.

The eviction playbook does not apply to a new item: it requires moving the body verbatim to claudedocs/decisions/NN-*.md and pinning sha256 of its non-blank lines at agentsSplitBase (c5c3817), and a born-split item does not exist at that commit. Adding a splitItems row for it would assert a verbatim move that never happened, and TestSplitDigestsAreTheBaseCommitsText would fail anyway.

So the full rationale lives in the doc comments of internal/validate/pattern.go, internal/manifest/jsonloc.go, internal/scaffold/scaffold.go and each guard's header — which is where someone editing that code will read it. The next person who wants an AGENTS item will hit the same wall. Unblocking it means evicting an existing large in-file item first (items 19 and 10 are the biggest, ~7.8 kB each, and both are at agentsSplitBase so the playbook works for them) — a separate, measurable change I deliberately did not fold in here.

🤖 Generated with Claude Code