Skip to content

fix(scaffold): refuse to mangle a non-ASCII name into a blockId, add --slug, and echo the derived id - #267

Open
ZacxDev wants to merge 5 commits into
mainfrom
fix/259-slug-derivation-and-echo
Open

fix(scaffold): refuse to mangle a non-ASCII name into a blockId, add --slug, and echo the derived id#267
ZacxDev wants to merge 5 commits into
mainfrom
fix/259-slug-derivation-and-echo

Conversation

@ZacxDev

@ZacxDev ZacxDev commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Revised after an independent audit (see the maintainer's comment below).
Two things this description used to claim are corrected here rather than
quietly edited away: it does not close #259, and its headline test claim
was resting on an assertion that could not execute. Both are fixed in
f94e3d1; the audit trail is in this section and in AGENTS.md item 25.

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 blockId caf-app.
It now exits 2 with a message naming é and pointing at the new --slug
flag. 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.mdThe blockId, and recorded as
a 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:

rune class verdict why
anything whose unicode.ToLower is < 0x80 never lossy exempt by construction — this is what keeps every ASCII derivation byte-identical
non-ASCII IsSpace || IsPunct || IsSymbol (on the lowered rune) separator an em dash, «», ©, NBSP carry no content of their own; they fold to a hyphen exactly like a space
everything else non-ASCII (letters, digits, marks) lossy → refuse content the author typed, with nowhere lossless to put it

Silent-character-loss classes, and their status after this PR:

class example before now
non-ASCII letters/digits/marks "Café App" caf-app, rc 0 CLOSED — refuses, rc 2, names é
invalid UTF-8 $'caf\xe9 app' caf-app, rc 0, and a manifest that was not valid UTF-8 CLOSED — refuses, rc 2
runes that lowercase into ASCII "İstanbul App" istanbul-app, rc 0 OPEN, deliberately — enumerated exception, exactly 2 runes exist (İ U+0130, K U+212A)
symbols / emoji "Rocket 🚀 App" rocket-app, rc 0 OPEN, deliberately — product decision, tracked as #272

The last two are documented in scaffold.Slugify's header and in the README
table, 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 TestSlugifyLowersIntoAsciiIsADocumentedException re-walks Unicode
so 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. LossyRunes
classified and reported off strings.ToLower(name). Measured: "ẞE App"
reported "ß" — a rune absent from the input, because U+1E9E lowers to
U+00DF — and "ABC" reported "a","b","c". Someone searching their own name
for 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. LossyRunesLossyChars ([]string), since the reported unit is
no longer always one rune.

2. The mustNotProduce block was UNREACHABLE, and this description's headline
claim 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'". The
block sat after a t.Fatalf that had already aborted the subtest whenever
err == nil, so its e == nil condition never held. Measured: deleting the
whole block left internal/scaffold green.

It now lives inside the err == nil branch, and each row's expected pre-fix
string 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:

# refusal disabled:
slug_lossy_test.go:145: Slugify("ÜberApp Ω") = "berapp" — that is the exact
  pre-fix mangling issue #259 is about, a different permanent public id …
slug_lossy_test.go:145: Slugify("Café Del Mar") = "caf-del-mar" — …

# refusal disabled AND the mustNotProduce block deleted:
slug_lossy_test.go:144: Slugify("ÜberApp Ω") = "berapp", want a refusal

The message degrades from the specific #259 claim to the generic one, which is
what shows the block executes and carries the claim.

3. REGRESSION this PR introduced: --slug silently skipped template
selection.
--slug was wired into the stdinIsTTY guard on the reasoning that
it "supplies the one thing the prompt exists to collect". runScaffoldForm
collects a name and a template, so civitai app create --slug my-app on a
TTY silently took page-money with no template choice — a question the user was
asked before the flag existed. The mutant deleting slugFlag == "" from that
guard survived with 0 failures; nothing covered the suppression at all.
--slug now drops only the name field; the template select still runs.

4. Invalid UTF-8 (both halves). for _, r := range yields U+FFFD per bad
byte, and U+FFFD is a Symbol — the separator branch. Slugify now gates on
utf8.ValidString before anything ranges over the string. runAppScaffold
separately refuses an invalid-UTF-8 display name, which is not redundant:
--slug bypasses derivation entirely, so the name reached
block.manifest.json unchecked. The two guards are independently pinned — M5
reddens only internal/scaffold rows, M6 only internal/cmd rows.
(internal/validate still 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 as e + U+0301 and the message read
"́" cannot appear in a blockId — an accent rendered over nothing — while the
NFC form of the same visible name said "é". Combining marks are now reported
with their base, so both forms report "é"; a base-less mark is shown on a
dotted 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
display survived because every assertion was a strings.Contains and the
first %s still carried the slug — the mutant printed
https://Widget Machine.civit.ai/ as the "permanent public id"); the refusal
quoting 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 public
id" at scaffold time — a URL that 404s until approval + deploy, as
README.md says and as app status already 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:

✓ Created App "My Cool Block" (static)  ·  ./my-cool-block/  ·  7 files
  blockId: my-cool-block  —  your app's permanent public id (it cannot be renamed later)
  Will be served at https://my-cool-block.civit.ai/ — only after the app is approved and deployed · `civitai app status my-cool-block`

Docs (395cefc)

  • README — the app create row now carries --slug, plus a new
    The blockId section: what the id is, that it can never be renamed, the
    breaking change (announced the way the --json field-notation break was), and
    the table of what still derives rather than refuses.
  • AGENTS.md 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 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 be
numbered 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; rewritten
internal/scaffold/slug_lossy_test.go; updated
internal/cmd/app_init_identity_test.go.

Controls that must stay green: 9 ASCII derivations through Slugify plus 7
through the real command, 4 non-ASCII separator rows (em dash / guillemets /
© / NBSP still derive widget-pro), both existing dead-end messages, and the
two documented exceptions. Exit codes are asserted with errors.Is, never
message text (AGENTS item 7). Every non-ASCII fixture is written with explicit
\uXXXX escapes and byte-asserted in the test itself — NFC and NFD render
identically, 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 one
command. Leaf --- FAIL lines are counted from the output, never an exit code.

# mutant leaf subtests reddened
M1 report the lowered rune, not the one typed 5
M2 revert the refusal entirely (= pre-#259; produces berapp) 15
M3 --slug suppresses the whole prompt again 1
M4 the form runs but is told to ask for the name anyway (half-fix) 1
M5 drop the utf8.ValidString guard in Slugify 4
M6 drop the display-name UTF-8 guard (manifest half) 3
M7 build the serving URL from the display name 1
M8 the refusal quotes a fixed literal, not the input 8
M9 widen the ASCII exemption boundary <<= 1
M10 report a combining mark bare, detached from its base 1
M11 print the serving URL bare, as a currently-valid id 1
M12 make symbols/emoji refuse (invert the #272 decision) 2
M13 classify on the original rune (removes the İ/K exception) 1
M14 drop the dedupe 1
M15 null mutant (comment only) 0 — SURVIVES, as required

Live end-to-end against the built binary, reproducing the original symptoms:
"ẞE App" → rc 2 naming "ẞ" (not "ß"); $'caf\xe9 app' → rc 2 (was
caf-app rc 0); NFD and NFC "Café App" → the identical message naming
"é"; "İstanbul App"istanbul-app rc 0; "Rocket 🚀 App"rocket-app
rc 0; "My Cool Block"my-cool-block rc 0.

make ci on the merged tree: 18 packages ok, 0 --- FAIL, 0
build failed, 0 timeout panics
; gofmt -s -l . clean over 303 .go files.

Note on history

04c5c57 merges origin/main in to resolve a README.md conflict caused by
#268 (which added a TOC and two new command-table rows adjacent to the
app create row this PR edits). Control run: the audited tip 8ffafe9 merges
cleanly with current main, HEAD did not — so the conflict came from this PR's
own README edit, not from drift. Nothing was rebased or force-pushed; 8ffafe9
is still in the history.

cb0cd51 respells two control-character test fixtures as Go escapes (\x7f,
\u0080) after staticcheck ST1018 flagged the raw bytes. Worth recording as a
process note: make ci is tidy + vet + test + build and does NOT run lint,
so a clean local make ci says nothing about the lint job — on a box without
golangci-lint, nix-shell -p golangci-lint --run "golangci-lint run" gets the
same 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 blockId
section, 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

…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>
@ZacxDev

ZacxDev commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

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 (berapp, caf-del-mar) rather than merely 'an error came back'." That claim is false. An independent audit found the mustNotProduce block in slug_lossy_test.go:64-68 is unreachable: the t.Fatalf at line 58 aborts the subtest whenever err == nil, so the e == nil branch at line 65 never executes. Measured — deleting the entire block leaves internal/scaffold green, while a positive control (corrupting an ASCII control row) reddens 1, so the harness can go red; the assertion simply never runs. The rows really are "an error came back".

The PR is not unprotected — the over-refusal guarantee comes from separate controls (TestSlugifyAsciiDerivationIsByteIdentical, …NonAsciiSeparatorsStaySeparators, TestAsciiDerivationIsUnchanged), which do bite: inverting the predicate reddens 12, narrowing the ASCII exemption reddens 18. But it is not protected by the thing the description advertises.

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 caf- shape #259 is about — all byte-identical on main, so pre-existing rather than introduced here:

  1. Invalid UTF-8app create $'caf\xe9 app' → blockId caf-app, rc 0. The written block.manifest.json also contains the raw 0xE9, is not valid UTF-8, and validate.ManifestOnly accepts it.
  2. Two runes ≥ 0x80 that lower to ASCIIİ (U+0130) and K (U+212A), found by enumerating all of Unicode. app create "İstanbul App" → blockId istanbul-app, rc 0. The doc comment's absolute rule is false for exactly these two.
  3. Symbols and emoji fold to a hyphen"Rocket 🚀 App"rocket-app, rc 0 (8,580 runes take the separator branch, 140,323 the refuse branch).

A fix round is in progress: (1) gets a utf8.ValidString guard, (2) becomes a documented enumerated exception, (3) is filed separately as a product decision. The doc comment and this description will be rewritten to state the residuals rather than claim closure.

ZacxDev and others added 4 commits August 7, 2026 16:55
…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>
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

slug derivation silently drops non-ASCII characters, and --dir never echoes the derived blockId

1 participant