fix(scaffold): refuse to mangle a non-ASCII name into a blockId, add --slug, and echo the derived id - #267
fix(scaffold): refuse to mangle a non-ASCII name into a blockId, add --slug, and echo the derived id#267ZacxDev wants to merge 5 commits into
Conversation
…echo the one we chose The blockId a scaffold mints is the app's PERMANENT public identity — the hostname it is served at (https://<blockId>.civit.ai/) and the argument every later command takes. Two defects met there (#259), plus two false messages on the same screen (#260 items 1 and 6). 1. Derivation silently DROPPED characters. `Slugify` lowercases and then replaces every run of `[^a-z0-9]+` with one hyphen, so "ÜberApp Ω" derived `berapp` (measured) — the leading Ü became a hyphen and was trimmed. Mid-name it is worse than dropping: "Café Del Mar" derived `caf-del-mar` (measured), inserting a word boundary the author never typed. Neither is a truncation the author can recognise, and the id cannot be renamed afterwards. Slugify now REFUSES when the name carries a rune derivation would lose, and names the offending characters. The predicate is an asymmetry, not a character list: a separator (space, `_`, `.`, `/`, `-`, `!`, an em dash) has no content of its own and still folds to a hyphen, while a letter/digit/mark the ASCII slug alphabet cannot carry is content with nowhere lossless to go. Every ASCII rune is exempt by construction, which is what keeps today's ASCII derivations byte-identical — including the two dead ends whose existing messages are good ("123 Numbers", "!!!"). Transliteration (the issue's preferred fix) was evaluated and rejected: Ω → o vs omega is a locale-dependent judgement, a general transliterator means a new golang.org/x/text dependency (an "ask first" per AGENTS.md), and it produces nothing usable for CJK/Cyrillic/Arabic. 2. `--slug` is the escape hatch that makes refusal viable, and there was none. It bypasses derivation entirely, is checked with the existing ValidateSlug, and also unblocks "123 Numbers" / "!!!". Name, slug and dir are now genuinely independent — `--slug` alone is enough to scaffold. 3. `printScaffoldResult` took `slug` and never used it — a dead parameter — so no line of output named the blockId. With the default directory you can infer it from the directory name; with `--dir` it was invisible unless you opened block.manifest.json, i.e. the case where derivation is most likely to surprise you was the case where it was least visible. It is now echoed always. 4. `app create --help` claimed the scaffold "validates clean". A fresh page-money / page-vite project FAILS `civitai app validate` until `npm install` writes the lockfile the platform build installs from. The check is right; the promise was wrong. Help text and next-step 1 both say so now (`static` ships no package.json and really does validate clean, so it does not carry the caveat). 5. `--from` printed a multi-line internal note ("TODO(server): expose a read endpoint …") that buried its one useful sentence. Users get an ordinary actionable one-line error; the engineering context is a source comment. Exit code unchanged (1 — an unavailable feature is not a malformed invocation). BREAKING CHANGE: `civitai app create "Café App"` used to silently produce the blockId `caf-app` and now exits 2 asking for an explicit `--slug`. Any script relying on a non-ASCII name deriving a slug must pass `--slug <slug>`. The old output was wrong (a different permanent public id than the author typed), so the break is the point — but it is a break. Mutation matrix (12 mutants, targeted at both the predicate and its call sites; each edit checksum-gated so an unapplied edit cannot read as a survivor): reverting the refusal reddens 8 leaf subtests; dropping the ASCII exemption 18; dropping the non-ASCII separator exemption 3; deleting the blockId echo 4; echoing it only under --dir (the half-fix) 1; ignoring --slug in derivation 4; skipping ValidateSlug on --slug 7; restoring the "validates clean" promise 1; restoring the old next-step-1 line 3; restoring the TODO(server) note 3; unbounding the character list 1; the comment-only null mutant SURVIVES. make ci: 18 packages ok, 0 `--- FAIL`, 0 `build failed`, gofmt -s clean. Refs #259, #260 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Correction to this PR's description, posted publicly rather than by editing the body — a reviewer may already have read the original. The description claims each refusal row "pins the exact pre-fix output it must not produce ( The PR is not unprotected — the over-refusal guarantee comes from separate controls ( Second correction: this PR narrows issue #259, it does not close it. Three classes of input still silently lose a character the author typed, each reproducing the exact
A fix round is in progress: (1) gets a |
…ating the template prompt Audit fixes on top of 8ffafe9. The refusal that PR added narrows #259; it does not close it, and three of its own claims did not hold. 1. The refusal named characters the user never typed. LossyRunes classified AND reported off strings.ToLower(name), so "ẞE App" reported "ß" — a rune absent from the input — and "ABC" reported "a","b","c". Classification stays on the LOWERED rune (that is what keeps ASCII derivations byte-identical); the REPORT is now the original. LossyRunes -> LossyChars ([]string), because the reported unit is no longer always one rune. 2. The `mustNotProduce` block was UNREACHABLE: the t.Fatalf above it aborted the subtest whenever err == nil, so `e == nil` never held. Measured: deleting the whole block left internal/scaffold green. The PR body's headline claim rested on it. It now lives inside the err == nil branch, and each row's expected pre-fix output is verified against a `legacySlugify` copy of the pre-refusal derivation, so a row cannot name a string the old code never emitted. 3. REGRESSION the PR introduced: `--slug` suppressed the whole interactive prompt, but runScaffoldForm collects a name AND a TEMPLATE. A TTY user running `civitai app create --slug my-app` silently got page-money with no template choice. --slug now drops only the NAME field; the template select still runs. The mutant deleting `slugFlag == ""` from that guard previously survived with 0 failures. 4. Invalid UTF-8 silently lost bytes: `app create $'caf\xe9 app'` derived `caf-app` rc 0 (range yields U+FFFD per bad byte; U+FFFD is So, the separator branch) and wrote a block.manifest.json that was not valid UTF-8. Slugify now refuses it, and the scaffold refuses an invalid-UTF-8 DISPLAY name too — --slug bypasses derivation, so the name reached the manifest unchecked. 5. NFD input produced an unreadable message: "Café App" in NFD named the bare combining acute, rendering over nothing, while NFC named "é" — the same VISIBLE name, two messages. Combining marks are now reported with their base, so both forms report "é". A base-less mark is shown on a dotted circle. The refusal SET is unchanged; only the rendering moved. 6. The echoed URL promised something guaranteed false at that moment: it printed https://<blockId>.civit.ai/ bare as the "permanent public id" while the README says that 404s before approval and `app status` already says so. It is now future-tense and carries the same "approved and deployed" qualifier app_status.go uses. 7. Three mutation survivors closed: the URL operand (slug -> display survived because every assertion was a Contains and the first %s still carried the slug), the refusal quoting the input name, and the ASCII-exemption boundary (< vs <=). The two exceptions that remain are now enumerated in Slugify's header rather than contradicted by it: the exactly-two runes above ASCII that lower INTO ASCII (İ U+0130, K U+212A), and symbols/emoji folding to a hyphen (issue #272). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…break
README
- The `app create` command-table row was still missing `--slug` entirely, and
no repo test enforces README<->flag parity, so it was not going to happen by
itself.
- New "The blockId" section: what the id is, that it can never be renamed, and
the BREAKING CHANGE — a non-ASCII name used to mint a silently different
permanent public id ("Café App" -> caf-app) and now exits 2 asking for
--slug. Announced the way the `--json` field-notation break was: inline, in
the section a reader is already in, with "update your scripts".
- A table of the three inputs that still DERIVE rather than refuse, so the
section documents the residuals instead of implying closure.
AGENTS.md
- New item 25. The durable claim is the exemption: derivation is safe to refuse
only because it exempts every rune that LOWERCASES INTO ASCII, which is what
keeps every pre-existing derivation byte-identical — do NOT "improve" that
into a character allowlist. Plus the four residuals stated as residuals
(invalid UTF-8 now closed; the İ/K pair; symbols/emoji per #272; NFD
rendering), and the four process lessons this round produced: classify on the
lowered rune but report the original, a flag that skips a prompt must
enumerate what else the prompt collects, the echoed URL is future tense, and
a mustNotProduce row placed after a t.Fatalf is not coverage.
- Indexed it in the preamble clause paragraph, per the file's own rule that an
item nothing points at is unreachable navigation.
NUMBERING: this takes item 25, not 26. PR #265 is adding an item concurrently
and was expected to take 25 — but parseAgentsItems in agents_xrefs_test.go
ENFORCES CONTIGUITY (the idx-th heading must be numbered idx+1), so skipping to
26 fails that guard unconditionally, today, on a 24-item file. AGENTS.md's own
maintenance rule covers the collision: the PR merging SECOND renumbers its own
new items. Whichever of #265 / #267 lands second renumbers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # README.md
…t raw bytes golangci-lint's staticcheck ST1018 flagged four string literals in slug_lossy_test.go holding raw U+007F / U+0080 rather than `\x7f` / `�`. The bytes were correct and the assertions were doing their job — the boundary mutant (`<` -> `<=`) is still killed after this change, re-measured — but a raw control character in source is invisible in every diff, review and grep, which is precisely the failure mode the surrounding test exists to guard against. Two process notes, because this is how it got through: - `make ci` is tidy + vet + test + build. It does NOT run lint, so a clean local `make ci` says nothing about the `lint` CI job. Run `golangci-lint run` too; on a machine without it, `nix-shell -p golangci-lint --run "golangci-lint run"` gets the same v2.12.2 CI uses. - The local lint run was instrument-checked before its "0 issues" was believed: reintroducing one raw U+007F reddened it with the ST1018 message, and removing it returned 0. A zero from an unvalidated scanner is indistinguishable from a scanner wired to nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This NARROWS #259; it does not close it. It also closes items 1 and 6 of
#260 — everything on the one screen where an app's permanent public identity
is minted.
🔴 BREAKING CHANGE, read first
civitai app create "Café App"used to silently produce the blockIdcaf-app.It now exits 2 with a message naming
éand pointing at the new--slugflag. Any script passing a non-ASCII name must pass
--slug <slug>.The old output was wrong — a different permanent public id than the author
typed, on an identifier that cannot be renamed — so this is the correct break,
but it is a break. Announced in
README.md→ The blockId, and recorded asa
BREAKING CHANGE:paragraph in the commit body (there is no CHANGELOG.md;goreleaser builds release notes from conventional-commit subjects).
What this closes, and what it knowingly does not
The refusal predicate is an asymmetry, not a character list:
unicode.ToLoweris< 0x80IsSpace || IsPunct || IsSymbol(on the lowered rune)«»,©, NBSP carry no content of their own; they fold to a hyphen exactly like a spaceSilent-character-loss classes, and their status after this PR:
"Café App"caf-app, rc 0é$'caf\xe9 app'caf-app, rc 0, and a manifest that was not valid UTF-8"İstanbul App"istanbul-app, rc 0İU+0130,KU+212A)"Rocket 🚀 App"rocket-app, rc 0The last two are documented in
scaffold.Slugify's header and in the READMEtable, not implied away. The whole-of-Unicode census behind them: 8,580
printable non-ASCII runes take the separator branch, 140,321 the refuse
branch; and
TestSlugifyLowersIntoAsciiIsADocumentedExceptionre-walks Unicodeso a future Go table growing a third such rune fails the build.
Fixes made in this round (
f94e3d1)1. The refusal named characters the user never typed.
LossyRunesclassified and reported off
strings.ToLower(name). Measured:"ẞE App"reported
"ß"— a rune absent from the input, because U+1E9E lowers toU+00DF — and
"ABC"reported"a","b","c". Someone searching their own namefor the quoted character finds nothing. Classification stays on the lowered
rune (that is what makes the ASCII exemption work); the report is now the
original.
LossyRunes→LossyChars([]string), since the reported unit isno longer always one rune.
2. The
mustNotProduceblock was UNREACHABLE, and this description's headlineclaim rested on it. It said each row "pins the exact pre-fix output it must not
produce (
berapp,caf-del-mar) rather than merely 'an error came back'". Theblock sat after a
t.Fatalfthat had already aborted the subtest whenevererr == nil, so itse == nilcondition never held. Measured: deleting thewhole block left
internal/scaffoldgreen.It now lives inside the
err == nilbranch, and each row's expected pre-fixstring is verified against
legacySlugify, a copy of the pre-refusal derivation— so a row cannot name an output the old code never emitted. Proven by mutation,
both directions:
The message degrades from the specific
#259claim to the generic one, which iswhat shows the block executes and carries the claim.
3. REGRESSION this PR introduced:
--slugsilently skipped templateselection.
--slugwas wired into thestdinIsTTYguard on the reasoning thatit "supplies the one thing the prompt exists to collect".
runScaffoldFormcollects a name and a template, so
civitai app create --slug my-appon aTTY silently took
page-moneywith no template choice — a question the user wasasked before the flag existed. The mutant deleting
slugFlag == ""from thatguard survived with 0 failures; nothing covered the suppression at all.
--slugnow drops only the name field; the template select still runs.4. Invalid UTF-8 (both halves).
for _, r := rangeyields U+FFFD per badbyte, and U+FFFD is a Symbol — the separator branch.
Slugifynow gates onutf8.ValidStringbefore anything ranges over the string.runAppScaffoldseparately refuses an invalid-UTF-8 display name, which is not redundant:
--slugbypasses derivation entirely, so the name reachedblock.manifest.jsonunchecked. The two guards are independently pinned — M5reddens only
internal/scaffoldrows, M6 onlyinternal/cmdrows.(
internal/validatestill has no UTF-8 check of its own; out of scope, stated.)5. NFD produced an unreadable message. macOS paths and some paste routes
deliver NFD, so
"Café App"arrives ase+ U+0301 and the message read"́" cannot appear in a blockId— an accent rendered over nothing — while theNFC form of the same visible name said
"é". Combining marks are now reportedwith their base, so both forms report
"é"; a base-less mark is shown on adotted circle. The set of refused names is unchanged — a mark was always
lossy — only the rendering moved.
6. Three mutation survivors closed. The URL operand (swapping
slug→displaysurvived because every assertion was astrings.Containsand thefirst
%sstill carried the slug — the mutant printedhttps://Widget Machine.civit.ai/as the "permanent public id"); the refusalquoting the input name; and the ASCII-exemption boundary (
<vs<=).7. The echoed URL promised something guaranteed false at that moment. It
printed the bare
https://<blockId>.civit.ai/as the app's "permanent publicid" at scaffold time — a URL that 404s until approval + deploy, as
README.mdsays and asapp statusalready tells people ("Not live yet — …only serves after the app is approved and deployed"). That is the same
false-promise class this PR fixes for "validates clean" two lines up in the
same output block. Now:
Docs (
395cefc)app createrow now carries--slug, plus a newThe blockId section: what the id is, that it can never be renamed, the
breaking change (announced the way the
--jsonfield-notation break was), andthe table of what still derives rather than refuses.
safe to refuse only because it exempts every rune that lowercases into
ASCII, which is what keeps every pre-existing derivation byte-identical — do
not "improve" that into a character allowlist. Plus the four residuals and
the four process lessons.
🔴 Numbering: this took item 25, not 26. #265 is adding an item
concurrently and was expected to take 25 — but
parseAgentsItems(
agents_xrefs_test.go) enforces contiguity (the i-th heading must benumbered i), so skipping to 26 fails that guard unconditionally on a 24-item
file. AGENTS.md's own maintenance rule covers the collision: whichever of
#265 / #267 merges second renumbers its own new items.
Testing
New:
internal/cmd/app_init_echo_test.go; rewritteninternal/scaffold/slug_lossy_test.go; updatedinternal/cmd/app_init_identity_test.go.Controls that must stay green: 9 ASCII derivations through
Slugifyplus 7through the real command, 4 non-ASCII separator rows (em dash / guillemets /
©/ NBSP still derivewidget-pro), both existing dead-end messages, and thetwo documented exceptions. Exit codes are asserted with
errors.Is, nevermessage text (AGENTS item 7). Every non-ASCII fixture is written with explicit
\uXXXXescapes and byte-asserted in the test itself — NFC and NFD renderidentically, so the fixtures verify their own bytes.
Mutation matrix — 15 mutants, re-measured on the merged tree (
04c5c57).Each edit is checksum-gated (target must occur exactly once; file hash must
change) so an edit that silently failed to apply cannot read as a survivor, and
restore is via
git checkout --so an interruption is diagnosable in onecommand. Leaf
--- FAILlines are counted from the output, never an exit code.berapp)--slugsuppresses the whole prompt againutf8.ValidStringguard inSlugify<→<=İ/Kexception)Live end-to-end against the built binary, reproducing the original symptoms:
"ẞE App"→ rc 2 naming"ẞ"(not"ß");$'caf\xe9 app'→ rc 2 (wascaf-apprc 0); NFD and NFC"Café App"→ the identical message naming"é";"İstanbul App"→istanbul-apprc 0;"Rocket 🚀 App"→rocket-apprc 0;
"My Cool Block"→my-cool-blockrc 0.make cion the merged tree: 18 packagesok, 0--- FAIL, 0build failed, 0 timeout panics;gofmt -s -l .clean over 303.gofiles.Note on history
04c5c57mergesorigin/mainin to resolve aREADME.mdconflict caused by#268 (which added a TOC and two new command-table rows adjacent to the
app createrow this PR edits). Control run: the audited tip8ffafe9mergescleanly with current
main, HEAD did not — so the conflict came from this PR'sown README edit, not from drift. Nothing was rebased or force-pushed;
8ffafe9is still in the history.
cb0cd51respells two control-character test fixtures as Go escapes (\x7f,\u0080) after staticcheck ST1018 flagged the raw bytes. Worth recording as aprocess note:
make ciis tidy + vet + test + build and does NOT run lint,so a clean local
make cisays nothing about thelintjob — on a box withoutgolangci-lint,
nix-shell -p golangci-lint --run "golangci-lint run"gets thesame v2.12.2 CI uses. That local run was instrument-checked before its
"0 issues" was believed: reintroducing one raw U+007F reddened it with the
ST1018 message, removing it returned 0. The boundary mutant is still killed
after the respelling (re-measured).
One deviation from the brief worth flagging: it said not to touch the nav/TOC
sections. I added a single TOC sub-bullet for my own new
### The blockIdsection, because #268 has already merged (so there is no live sibling owning
that region) and every other
###under Command reference is listed there —leaving mine out would make the README's own navigation incomplete. Happy to
drop that one line if you'd rather.
🤖 Generated with Claude Code