From 8ffafe9b0119bac5ebe59f42dac4fa4d8396c249 Mon Sep 17 00:00:00 2001 From: ZacxDev Date: Fri, 7 Aug 2026 15:13:18 -0500 Subject: [PATCH 1/4] fix(scaffold): refuse to mangle a non-ASCII name into a blockId, and echo the one we chose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blockId a scaffold mints is the app's PERMANENT public identity — the hostname it is served at (https://.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 `. 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) --- internal/cmd/app_create.go | 22 +- internal/cmd/app_create_cmd_test.go | 9 +- internal/cmd/app_init.go | 111 +++++++-- internal/cmd/app_init_identity_test.go | 332 +++++++++++++++++++++++++ internal/cmd/cmd_test.go | 7 +- internal/scaffold/slug.go | 78 +++++- internal/scaffold/slug_lossy_test.go | 208 ++++++++++++++++ 7 files changed, 737 insertions(+), 30 deletions(-) create mode 100644 internal/cmd/app_init_identity_test.go create mode 100644 internal/scaffold/slug_lossy_test.go diff --git a/internal/cmd/app_create.go b/internal/cmd/app_create.go index 6e5f98a..ecca4f3 100644 --- a/internal/cmd/app_create.go +++ b/internal/cmd/app_create.go @@ -14,6 +14,7 @@ func newAppCreateCmd() *cobra.Command { var fromSlug string var dirFlag string var nameFlag string + var slugFlag string var noInput bool cmd := &cobra.Command{ @@ -25,8 +26,9 @@ This is the friendly happy path: a thin superset of "civitai app init" that defaults to the rich page-money template — a Vite + React + TypeScript full-page app wired to the published App SDK (estimate -> consent -> submit -> poll -> Buzz spend), with a mock-host dev harness and a unit test. The scaffold is -immediately runnable (npm install && npm run dev:harness), test-green, and -validates clean. +immediately runnable (npm install && npm run dev:harness) and test-green. +"civitai app validate" passes once you have run "npm install" — until then it +correctly reports the package-lock.json the platform build installs from. The default scaffold ships a runnable txt2img money path AND a Comfy on Civitai (customComfy) sample that runs a server-registered recipe (invite-only beta) — @@ -50,6 +52,12 @@ Templates (override with --template): The display name can be free-form ("My Cool Block"); it is slugified for the blockId. A slug-shaped name is used verbatim. +The blockId is your app's PERMANENT public identity — it is the hostname your app +is served at and the argument every later command takes — so derivation refuses +rather than guesses when the name carries characters a blockId cannot hold +("Café Del Mar", "ÜberApp", any non-Latin name). Pass --slug to choose one +yourself; it bypasses derivation entirely. + By default the project is created in ./. Override the output directory with a positional [dir] or --dir ; override the display name independently with --name (so name, slug, and directory can all differ). @@ -67,19 +75,23 @@ the AI Services scopes: ` + spendCredentialRoutes + `.`, civitai app create my-block --template static # Custom output directory (slug stays my-block; created in ./apps/foo). - civitai app create my-block --dir ./apps/foo`, + civitai app create my-block --dir ./apps/foo + + # A name derivation cannot slugify: choose the blockId yourself. + civitai app create "Café Del Mar" --slug cafe-del-mar`, Args: cobra.MaximumNArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - return runAppScaffold(cmd, args, templateFlag, fromSlug, dirFlag, nameFlag, noInput) + return runAppScaffold(cmd, args, templateFlag, fromSlug, dirFlag, nameFlag, slugFlag, noInput) }, } // create defaults to the batteries-included page-money template; every // other flag matches init exactly. cmd.Flags().StringVarP(&templateFlag, "template", "t", string(scaffold.PageMoney), "project template: static | page-vite | page-money") - cmd.Flags().StringVar(&fromSlug, "from", "", "fork from an existing published app slug (not yet wired)") + cmd.Flags().StringVar(&fromSlug, "from", "", fromFlagUsage) cmd.Flags().StringVar(&dirFlag, "dir", "", "output directory (default ./)") cmd.Flags().StringVar(&nameFlag, "name", "", "display name (default derived from the name argument)") + cmd.Flags().StringVar(&slugFlag, "slug", "", slugFlagUsage) cmd.Flags().BoolVarP(&noInput, "yes", "y", false, "non-interactive: never prompt (use flags/defaults; fail if a name is missing)") return cmd } diff --git a/internal/cmd/app_create_cmd_test.go b/internal/cmd/app_create_cmd_test.go index 43083d5..ce045bc 100644 --- a/internal/cmd/app_create_cmd_test.go +++ b/internal/cmd/app_create_cmd_test.go @@ -233,15 +233,18 @@ func TestAppCreateRefusesNonEmptyDir(t *testing.T) { } } +// TestAppCreateFromIsNotWired pins the `--from` refusal for `app create`; the +// message contract itself (no internal TODO, an actionable next command) is +// asserted for BOTH commands in TestScaffoldFromErrorShipsNoEngineeringNote. func TestAppCreateFromIsNotWired(t *testing.T) { tmp := t.TempDir() dest := filepath.Join(tmp, "out") _, errOut, err := run(t, "app", "create", "my-block", dest, "--from", "some-slug") if err == nil { - t.Fatal("expected --from to error (not yet wired)") + t.Fatal("expected --from to error (not available yet)") } - if !strings.Contains(err.Error()+errOut, "not yet wired") { - t.Errorf("--from should report it is not wired: err=%v stderr=%s", err, errOut) + if !strings.Contains(err.Error()+errOut, "--from is not available yet") { + t.Errorf("--from should report it is unavailable: err=%v stderr=%s", err, errOut) } } diff --git a/internal/cmd/app_init.go b/internal/cmd/app_init.go index 4a0bcbe..68d9cec 100644 --- a/internal/cmd/app_init.go +++ b/internal/cmd/app_init.go @@ -20,6 +20,7 @@ func newAppInitCmd() *cobra.Command { var fromSlug string var dirFlag string var nameFlag string + var slugFlag string var noInput bool cmd := &cobra.Command{ @@ -36,6 +37,12 @@ Templates: The display name can be free-form ("My Cool Block"); it is slugified for the blockId. A slug-shaped name is used verbatim. +The blockId is your app's PERMANENT public identity — it is the hostname your app +is served at and the argument every later command takes — so derivation refuses +rather than guesses when the name carries characters a blockId cannot hold +("Café Del Mar", "ÜberApp", any non-Latin name). Pass --slug to choose one +yourself; it bypasses derivation entirely. + By default the project is created in ./. Override the output directory with a positional [dir] or --dir ; override the display name independently with --name (so name, slug, and directory can all differ).`, @@ -48,38 +55,60 @@ a positional [dir] or --dir ; override the display name independently with # Custom output directory (slug stays my-block; created in ./apps/foo). civitai app init my-block --dir ./apps/foo + # A name derivation cannot slugify: choose the blockId yourself. + civitai app init "Café Del Mar" --slug cafe-del-mar + # Name, slug, and dir all independent. civitai app init my-block ./apps/foo --name "My Block"`, Args: cobra.MaximumNArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - return runAppScaffold(cmd, args, templateFlag, fromSlug, dirFlag, nameFlag, noInput) + return runAppScaffold(cmd, args, templateFlag, fromSlug, dirFlag, nameFlag, slugFlag, noInput) }, } cmd.Flags().StringVarP(&templateFlag, "template", "t", string(scaffold.Static), "project template: static | page-vite | page-money") - cmd.Flags().StringVar(&fromSlug, "from", "", "fork from an existing published app slug (not yet wired)") + cmd.Flags().StringVar(&fromSlug, "from", "", fromFlagUsage) cmd.Flags().StringVar(&dirFlag, "dir", "", "output directory (default ./)") cmd.Flags().StringVar(&nameFlag, "name", "", "display name (default derived from the name argument)") + cmd.Flags().StringVar(&slugFlag, "slug", "", slugFlagUsage) cmd.Flags().BoolVarP(&noInput, "yes", "y", false, "non-interactive: never prompt (use flags/defaults; fail if a name is missing)") return cmd } +// slugFlagUsage / fromFlagUsage are shared by `app init` and `app create` so the +// two commands cannot drift on the same flag. +const ( + slugFlagUsage = "explicit blockId (bypasses derivation from the name; 3-40 chars, starts with a letter, lowercase a-z/0-9/hyphens)" + // The flag stays discoverable — it is a real roadmap item and hiding it + // would only move the surprise to a user who read about it elsewhere — but + // the usage string says up front that it cannot work yet, so the failure is + // predictable BEFORE the command is run rather than after. + fromFlagUsage = "fork from an existing published app slug (NOT AVAILABLE YET — the CLI cannot fetch app source)" +) + // runAppScaffold is the shared scaffold body behind both `app init` and // `app create`. The two commands differ only in the default template; all of // the slug/display derivation, dir resolution, rendering, self-validation, and // next-steps output lives here so there is a single code path. -func runAppScaffold(cmd *cobra.Command, args []string, templateFlag, fromSlug, dirFlag, nameFlag string, noInput bool) error { +func runAppScaffold(cmd *cobra.Command, args []string, templateFlag, fromSlug, dirFlag, nameFlag, slugFlag string, noInput bool) error { out := cmd.OutOrStdout() + // TODO(server): expose a read endpoint that returns a published app's + // canonical source tree by slug, then --from can scaffold from it. Until + // then the flag has a 100% failure rate, so the message a user sees is a + // plain actionable CLI error — the engineering context stays HERE. An + // end-user error is not the place to ship an internal ticket. if fromSlug != "" { - return fmt.Errorf(`--from is not yet wired up. - -Forking an existing published app requires fetching its source from the -server, which this CLI cannot do yet (no programmatic app-source endpoint). + return fmt.Errorf("--from is not available yet: this CLI cannot fetch a published app's source. Scaffold a plain project with `%s ` and copy the upstream files in by hand", cmd.CommandPath()) + } -TODO(server): expose a read endpoint that returns a published app's canonical -source tree by slug, then --from can scaffold from it. For now, run a plain -init and copy the upstream files in manually`) + // An explicit --slug bypasses derivation entirely, so it is checked against + // the same server contract a derived slug is. A bad VALUE is a usage error + // (exit 2), same class as a bad --template. + if slugFlag != "" { + if err := scaffold.ValidateSlug(slugFlag); err != nil { + return asUsageError(err) + } } name := "" @@ -92,7 +121,10 @@ init and copy the upstream files in manually`) // stdin (not a TTY), or --yes all SKIP huh — so scripted invocations never // block on a prompt (huh requires a TTY). The prompt fills in the name + // template; everything downstream is identical to the flag-driven path. - if name == "" && !noInput && stdinIsTTY() { + // --slug supplies the one thing the prompt exists to collect (an identity we + // can build a project around), so it also suppresses the prompt: the display + // name falls back to the slug's title case. + if name == "" && slugFlag == "" && !noInput && stdinIsTTY() { inputs, err := scaffoldPromptFn(cmd, templateFlag) if err != nil { return err @@ -112,7 +144,7 @@ init and copy the upstream files in manually`) return asUsageError(err) } - if name == "" { + if name == "" && slugFlag == "" { return asUsageError(fmt.Errorf("provide a project name: %s ", cmd.CommandPath())) } @@ -130,18 +162,33 @@ init and copy the upstream files in manually`) targetDir = posDir } - // Derive slug + display name. If the name is already a valid slug - // use it verbatim; otherwise slugify and title-case for display. + // Derive slug + display name. An explicit --slug wins outright; otherwise a + // name that is already a valid slug is used verbatim, and anything else is + // slugified (which REFUSES rather than dropping characters — see + // scaffold.LossyRunes). + nameIsSlug := name != "" && scaffold.ValidateSlug(name) == nil var slug, display string - if err := scaffold.ValidateSlug(name); err == nil { + switch { + case slugFlag != "": + slug = slugFlag + case nameIsSlug: slug = name - display = scaffold.TitleFromSlug(name) - } else { + default: // An unusable argument is a bad VALUE, same class as --template. slug, err = scaffold.Slugify(name) if err != nil { return asUsageError(err) } + } + + // Display name: title-cased from whichever identifier we have when the user + // typed one that reads as an id, verbatim when they typed prose. + switch { + case name == "": + display = scaffold.TitleFromSlug(slug) + case nameIsSlug: + display = scaffold.TitleFromSlug(name) + default: display = name } @@ -190,12 +237,38 @@ init and copy the upstream files in manually`) return nil } +// installStepFmt is next-step 1 for the templates that install. It is shared by +// both npm branches so they cannot drift. +// +// 🔴 THE SECOND LINE IS THE FIX, NOT DECORATION. `civitai app validate` FAILS on +// a freshly-scaffolded page-money / page-vite project until `npm install` has +// written package-lock.json — the platform build installs strictly from the +// committed lockfile, so the check is right and its message is good. What was +// wrong was the PROMISE: `app create --help` said the scaffold "validates +// clean", and validating before installing ~103 MB of node_modules is the +// natural first instinct. Saying it here (the last screen before the author +// starts) and in the help text is what closes that minute-3 surprise. `static` +// ships no package.json, installs nothing, and does validate clean — which is +// why this line lives on the install branches only (issue #260). +const installStepFmt = " 1. cd %s && npm install # writes package-lock.json — COMMIT it\n" + + " (`civitai app validate` fails until you do — it needs that lockfile)\n" + // printScaffoldResult prints a one-line summary (with a file count, not the // full tree) followed by a numbered, template-tailored "Next steps" sequence. func printScaffoldResult(out io.Writer, display, slug string, tmpl scaffold.Template, destDir, abs string, written []string) { // One scannable line: name, template, where, and how many files — no tree. fmt.Fprintln(out, ui.Success(fmt.Sprintf("Created App %q (%s) · %s/ · %d files", display, tmpl, destDir, len(written)))) + // 🔴 ECHO THE blockId ALWAYS, not only when it was derived or only when --dir + // was passed. It is the app's PERMANENT public identity — the hostname it is + // served at, and the argument `app status` / `app metrics` / `app listing` + // all take — and `slug` was a DEAD PARAMETER here: no line of this output + // named it. With the default directory you can infer it from the directory + // name; with --dir it is invisible unless you open block.manifest.json. So + // the case where derivation is most likely to surprise you was the case + // where it was least visible (issue #259). + fmt.Fprintln(out, ui.Dim(fmt.Sprintf(" blockId: %s — permanent public id: https://%s.civit.ai/ · `civitai app status %s`", slug, slug, slug))) + // page-money ships a runnable txt2img money path and a Comfy on Civitai // (customComfy) sample, with body builders for BOTH customComfy arms — the // server-registered recipe, and an inline graph the app ships itself. Surface @@ -230,7 +303,7 @@ func printScaffoldResult(out io.Writer, display, slug string, tmpl scaffold.Temp // The two beta surfaces are still listed submit-first, but that is a // PRESENTATIONAL grouping (publish path, then preview path) — not a // dependency. Do not re-derive an ordering requirement from it. - fmt.Fprintf(out, " 1. cd %s && npm install # writes package-lock.json — COMMIT it\n", destDir) + fmt.Fprintf(out, installStepFmt, destDir) fmt.Fprintln(out, " 2. npm run dev:harness # mock host, no Buzz — works today") fmt.Fprintln(out, " 3. edit src/App.tsx and iterate") fmt.Fprintln(out) @@ -239,7 +312,7 @@ func printScaffoldResult(out io.Writer, display, slug string, tmpl scaffold.Temp fmt.Fprintln(out, " npm run dev:tunnel # in another terminal: serve your app for the tunnel") fmt.Fprintln(out, " civitai app dev-tunnel # your LOCAL app INSIDE the real host, prod-fidelity — no submit needed") case tmpl == scaffold.PageVite: - fmt.Fprintf(out, " 1. cd %s && npm install # writes package-lock.json — COMMIT it\n", destDir) + fmt.Fprintf(out, installStepFmt, destDir) fmt.Fprintln(out, " 2. npm run dev # preview locally (your UI only — see below)") fmt.Fprintln(out, " 3. civitai app submit # validate + submit for review") default: diff --git a/internal/cmd/app_init_identity_test.go b/internal/cmd/app_init_identity_test.go new file mode 100644 index 0000000..b240542 --- /dev/null +++ b/internal/cmd/app_init_identity_test.go @@ -0,0 +1,332 @@ +package cmd + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +// manifestBlockID / manifestName read what the scaffold actually WROTE. The +// manifest is the authority on the app's identity; the printed line is a claim +// ABOUT it, so the tests below assert both and never infer one from the other. +// (readBlockID already exists in app_dev_token_rename_test.go — reused here.) +func manifestBlockID(t *testing.T, dir string) string { + t.Helper() + got := readBlockID(t, dir) + if got == "" { + t.Fatalf("manifest in %s has no blockId", dir) + } + return got +} + +func manifestName(t *testing.T, dir string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(dir, "block.manifest.json")) + if err != nil { + t.Fatalf("read manifest: %v", err) + } + var m struct { + Name string `json:"name"` + } + if err := json.Unmarshal(b, &m); err != nil { + t.Fatalf("decode manifest: %v", err) + } + return m.Name +} + +// TestScaffoldEchoesTheDerivedSlugWithADir is the half of issue #259 that makes +// the derivation bug INVISIBLE. `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 the +// only copy is inside block.manifest.json. +// +// The assertion is deliberately not "the output contains the slug somewhere": +// the slug here (`ueber-app`) shares no substring with the display name or the +// directory, so a green cannot be earned by an unrelated line. +func TestScaffoldEchoesTheDerivedSlugWithADir(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "somewhere-else") + + stdout, _, err := run(t, "app", "create", "Uber App", "--slug", "ueber-app", "--dir", dest, "--template", "static") + if err != nil { + t.Fatalf("app create: %v\n%s", err, stdout) + } + if got := manifestBlockID(t, dest); got != "ueber-app" { + t.Fatalf("manifest blockId = %q, want ueber-app", got) + } + if !strings.Contains(stdout, "ueber-app") { + t.Errorf("output must echo the blockId (the app's permanent public id):\n%s", stdout) + } + if !strings.Contains(stdout, "blockId") { + t.Errorf("output must LABEL the id, not just happen to contain the string:\n%s", stdout) + } + // The directory name is the one place the slug is normally readable; here it + // is not, which is what makes this the load-bearing case. + if strings.Contains(filepath.Base(dest), "ueber-app") { + t.Fatal("fixture is broken: the --dir name must NOT contain the slug") + } +} + +// TestScaffoldEchoesTheSlugWithTheDefaultDir is the same claim for the default +// directory. It is a weaker observation on its own (the directory name is the +// slug), so it asserts the LABEL too. +func TestScaffoldEchoesTheSlugWithTheDefaultDir(t *testing.T) { + tmp := t.TempDir() + chdir(t, tmp) + + stdout, _, err := run(t, "app", "create", "My Cool Block", "--template", "static") + if err != nil { + t.Fatalf("app create: %v\n%s", err, stdout) + } + if got := manifestBlockID(t, filepath.Join(tmp, "my-cool-block")); got != "my-cool-block" { + t.Fatalf("manifest blockId = %q, want my-cool-block", got) + } + if !strings.Contains(stdout, "blockId: my-cool-block") { + t.Errorf("output must echo the blockId even with the default dir:\n%s", stdout) + } +} + +// TestSlugFlagIsTheEscapeHatchForARefusedName is the reason the refusal in +// scaffold.Slugify is shippable at all: "Café App" no longer derives (it used to +// silently mint `caf-app`), and --slug is how the author says what they want +// instead. +func TestSlugFlagIsTheEscapeHatchForARefusedName(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "out") + + // Without --slug the name is refused, as a usage error (exit 2). + if _, _, err := run(t, "app", "create", "Café App", "--dir", filepath.Join(tmp, "refused"), "--template", "static"); err == nil { + t.Fatal("expected a non-slugifiable name to be refused") + } else if !errors.Is(err, ErrUsage) { + t.Errorf("a refused name is a usage error: errors.Is(err, ErrUsage) = false (%v)", err) + } + + // With --slug it succeeds and the manifest carries the explicit slug. + stdout, _, err := run(t, "app", "create", "Café App", "--slug", "cafe-app", "--dir", dest, "--template", "static") + if err != nil { + t.Fatalf("app create with --slug: %v\n%s", err, stdout) + } + if got := manifestBlockID(t, dest); got != "cafe-app" { + t.Errorf("manifest blockId = %q, want cafe-app", got) + } + if got := manifestName(t, dest); got != "Café App" { + t.Errorf("display name = %q, want the name the author typed (Café App)", got) + } + if !strings.Contains(stdout, "cafe-app") { + t.Errorf("output must echo the explicit blockId too:\n%s", stdout) + } +} + +// TestSlugFlagRejectsAnInvalidValue: --slug bypasses derivation, so it is the +// only thing standing between a typo and an invalid manifest. It goes through +// the same ValidateSlug the server contract is written from, and a bad VALUE is +// a usage error — pinned with errors.Is, never message text (AGENTS item 7). +func TestSlugFlagRejectsAnInvalidValue(t *testing.T) { + for _, bad := range []string{ + "Cafe-App", // uppercase + "1st-app", // must start with a letter + "ab", // under the 3-char floor + "cafe app", // space + "cafe-", // must end alphanumeric + strings.Repeat("a", 41), // over the 40-char cap + } { + t.Run(bad, func(t *testing.T) { + tmp := t.TempDir() + _, _, err := run(t, "app", "create", "my-block", "--slug", bad, "--dir", filepath.Join(tmp, "out"), "--template", "static") + if err == nil { + t.Fatalf("--slug %q should be refused", bad) + } + if !errors.Is(err, ErrUsage) { + t.Errorf("--slug %q must be a usage error: %v", bad, err) + } + if _, statErr := os.Stat(filepath.Join(tmp, "out")); statErr == nil { + t.Errorf("--slug %q was refused but a project was still written", bad) + } + }) + } +} + +// TestNameSlugAndDirAreFullyIndependent. The help text has promised name/dir +// independence for a while; --slug makes the third axis real. All three values +// are pairwise distinct AND share no substring, so no assertion here can be +// satisfied by the wrong one. +func TestNameSlugAndDirAreFullyIndependent(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "zzz-output-dir") + + stdout, _, err := run(t, "app", "create", "ignored-positional", + "--name", "Widget Machine", + "--slug", "qqq-block-id", + "--dir", dest, + "--template", "static") + if err != nil { + t.Fatalf("app create: %v\n%s", err, stdout) + } + if got := manifestBlockID(t, dest); got != "qqq-block-id" { + t.Errorf("blockId = %q, want qqq-block-id", got) + } + if got := manifestName(t, dest); got != "Widget Machine" { + t.Errorf("display name = %q, want Widget Machine", got) + } + if _, err := os.Stat(filepath.Join(dest, "block.manifest.json")); err != nil { + t.Errorf("project should be written to --dir: %v", err) + } + if _, err := os.Stat(filepath.Join(tmp, "qqq-block-id")); err == nil { + t.Error("--dir was given, so nothing may be written to ./") + } + for _, want := range []string{"qqq-block-id", "Widget Machine", "zzz-output-dir"} { + if !strings.Contains(stdout, want) { + t.Errorf("output should name all three of slug/display/dir; missing %q:\n%s", want, stdout) + } + } +} + +// TestSlugFlagAloneNeedsNoName — the axes really are independent: --slug on its +// own is enough to scaffold, with the display name title-cased from it. +func TestSlugFlagAloneNeedsNoName(t *testing.T) { + tmp := t.TempDir() + chdir(t, tmp) + + stdout, _, err := run(t, "app", "create", "--slug", "solo-block", "--template", "static") + if err != nil { + t.Fatalf("app create --slug (no name): %v\n%s", err, stdout) + } + if got := manifestBlockID(t, filepath.Join(tmp, "solo-block")); got != "solo-block" { + t.Errorf("blockId = %q, want solo-block", got) + } + if got := manifestName(t, filepath.Join(tmp, "solo-block")); got != "Solo Block" { + t.Errorf("display name = %q, want Solo Block (title-cased from the slug)", got) + } +} + +// TestAsciiDerivationIsUnchanged is the CONTROL for the refusal: every name that +// derived a slug before must derive the byte-identical slug now, through the +// real command. These rows are expected to have been green before the change — +// they are an invariant guard, not regression coverage, and their job is to fail +// if the refusal predicate ever widens to swallow ordinary punctuation. +func TestAsciiDerivationIsUnchanged(t *testing.T) { + cases := map[string]string{ + "My Cool Block": "my-cool-block", + "my-block": "my-block", + " Spaced Out ": "spaced-out", + "Foo/Bar_Baz.Qux!": "foo-bar-baz-qux", + "Widget (v2) — Pro": "widget-v2-pro", // an em dash is a separator, not content + "a & b + c = d": "a-b-c-d", + "Trailing punctuation.": "trailing-punctuation", + } + for name, want := range cases { + t.Run(name, func(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "out") + stdout, _, err := run(t, "app", "create", name, "--dir", dest, "--template", "static") + if err != nil { + t.Fatalf("app create %q: %v\n%s", name, err, stdout) + } + if got := manifestBlockID(t, dest); got != want { + t.Errorf("blockId for %q = %q, want %q", name, got, want) + } + }) + } +} + +// TestScaffoldHelpDoesNotPromiseAValidateCleanScaffold — issue #260 item 1. +// `app create --help` said 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. +func TestScaffoldHelpDoesNotPromiseAValidateCleanScaffold(t *testing.T) { + help, _, err := run(t, "app", "create", "--help") + if err != nil { + t.Fatalf("app create --help: %v", err) + } + if strings.Contains(help, "validates clean") { + t.Errorf("help must not claim the scaffold `validates clean` — it does not until `npm install` runs:\n%s", help) + } + // And it must say what the real precondition is, rather than going silent. + for _, want := range []string{"civitai app validate", "npm install"} { + if !strings.Contains(help, want) { + t.Errorf("help should name the validate/install relationship; missing %q:\n%s", want, help) + } + } +} + +// TestNextStepsSayValidateNeedsInstall — the same fact at the other surface the +// author actually reads: the numbered next-steps block printed right after the +// scaffold is written. The `static` template installs nothing and DOES validate +// clean, so it must NOT carry the caveat; that pair is what stops the assertion +// from passing on a line printed unconditionally. +func TestNextStepsSayValidateNeedsInstall(t *testing.T) { + t.Run("page-money warns", func(t *testing.T) { + tmp := t.TempDir() + stdout, _, err := run(t, "app", "create", "my-block", "--dir", filepath.Join(tmp, "out")) + if err != nil { + t.Fatalf("app create: %v\n%s", err, stdout) + } + if !strings.Contains(stdout, "npm install") { + t.Fatalf("expected an npm install step:\n%s", stdout) + } + if !strings.Contains(stdout, "civitai app validate` fails until you do") { + t.Errorf("next steps must say validate fails until npm install has run:\n%s", stdout) + } + }) + t.Run("page-vite warns", func(t *testing.T) { + tmp := t.TempDir() + stdout, _, err := run(t, "app", "create", "my-block", "--dir", filepath.Join(tmp, "out"), "--template", "page-vite") + if err != nil { + t.Fatalf("app create: %v\n%s", err, stdout) + } + if !strings.Contains(stdout, "civitai app validate` fails until you do") { + t.Errorf("next steps must say validate fails until npm install has run:\n%s", stdout) + } + }) + t.Run("static does not (it really does validate clean)", func(t *testing.T) { + tmp := t.TempDir() + stdout, _, err := run(t, "app", "create", "my-block", "--dir", filepath.Join(tmp, "out"), "--template", "static") + if err != nil { + t.Fatalf("app create: %v\n%s", err, stdout) + } + if strings.Contains(stdout, "fails until you do") { + t.Errorf("static installs nothing and validates clean — it must not carry the caveat:\n%s", stdout) + } + }) +} + +// TestScaffoldFromErrorShipsNoEngineeringNote — issue #260 item 6. `--from` used +// to answer with a multi-line internal note ("TODO(server): expose a read +// endpoint …") that buried the one useful sentence. The context belongs in a +// source comment; the user gets an ordinary actionable CLI error. +func TestScaffoldFromErrorShipsNoEngineeringNote(t *testing.T) { + for _, sub := range []string{"create", "init"} { + t.Run(sub, func(t *testing.T) { + tmp := t.TempDir() + _, errOut, err := run(t, "app", sub, "forked-app", "--from", "some-published-app", "--dir", filepath.Join(tmp, "out")) + if err == nil { + t.Fatal("expected --from to fail") + } + blob := err.Error() + errOut + for _, leaked := range []string{"TODO(", "TODO", "endpoint that returns"} { + if strings.Contains(blob, leaked) { + t.Errorf("user-facing error leaks an engineering note (%q):\n%s", leaked, blob) + } + } + if !strings.Contains(blob, "--from is not available yet") { + t.Errorf("error should say plainly that the flag is unavailable:\n%s", blob) + } + // Actionable: it names what to do instead, with the real command path. + if !strings.Contains(blob, "civitai app "+sub+" ") { + t.Errorf("error should name the command to run instead:\n%s", blob) + } + if strings.Count(blob, "\n") > 1 { + t.Errorf("error should be a single line, not a multi-line note:\n%s", blob) + } + // Unchanged classification: an unavailable feature is not a + // malformed invocation, so it stays generic (exit 1), not exit 2. + if errors.Is(err, ErrUsage) { + t.Error("--from is a real flag with a real value: it must not be tagged ErrUsage") + } + }) + } +} diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 5b56ddf..4180bfe 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -268,6 +268,9 @@ func TestAppInitRequiresName(t *testing.T) { } } +// TestAppInitFromIsNotWired pins the `--from` refusal for `app init`; the +// message contract itself (no internal TODO, an actionable next command) is +// asserted for BOTH commands in TestScaffoldFromErrorShipsNoEngineeringNote. func TestAppInitFromIsNotWired(t *testing.T) { tmp := t.TempDir() chdir(t, tmp) @@ -275,8 +278,8 @@ func TestAppInitFromIsNotWired(t *testing.T) { if err == nil { t.Fatal("expected --from to be reported as not wired") } - if !strings.Contains(err.Error(), "not yet wired") { - t.Errorf("error should say not yet wired: %v", err) + if !strings.Contains(err.Error(), "--from is not available yet") { + t.Errorf("error should say --from is not available yet: %v", err) } } diff --git a/internal/scaffold/slug.go b/internal/scaffold/slug.go index 458e80e..8d590da 100644 --- a/internal/scaffold/slug.go +++ b/internal/scaffold/slug.go @@ -5,6 +5,8 @@ import ( "fmt" "regexp" "strings" + "unicode" + "unicode/utf8" ) // slugSuffixAlphabet is lowercase-alphanumeric only (no hyphens) so a generated @@ -66,9 +68,83 @@ var slugPattern = regexp.MustCompile(`^[a-z][a-z0-9-]*[a-z0-9]$`) var nonSlugChars = regexp.MustCompile(`[^a-z0-9]+`) +// LossyRunes returns, in first-appearance order and deduplicated, the runes in +// name that slug derivation would DROP rather than fold into a hyphen. +// +// 🔴 The point is the ASYMMETRY between the two things a non-`[a-z0-9]` rune can +// be. A SEPARATOR (space, `_`, `.`, `/`, `-`, `!`, `&`, an em dash …) carries no +// information of its own: turning it into a hyphen is what the author meant, and +// that is why `"My Cool Block"` → `my-cool-block` is correct. A LETTER, DIGIT or +// MARK the ASCII slug alphabet cannot carry is the opposite: it is content the +// author typed, and there is no lossless place to put it. Dropping it silently +// mints a DIFFERENT permanent public identity — `"ÜberApp Ω"` → `berapp` (the +// leading `Ü` becomes a hyphen and is then trimmed), `"Café Del Mar"` → +// `caf-del-mar` (a spurious word boundary mid-name). Neither is a truncation the +// author can recognise, and a blockId is not renameable, so the caller refuses +// and asks for an explicit slug instead. +// +// Transliteration was considered and rejected: `Ω → o` vs `omega` is a +// locale-dependent judgement, a general transliterator means a new +// `golang.org/x/text` dependency, and it produces nothing usable for CJK, +// Cyrillic or Arabic — which is most of the population this refusal serves. +// +// 🔴 EVERY ASCII RUNE IS EXEMPT BY CONSTRUCTION, and that is load-bearing rather +// than lazy: it is what makes the ASCII derivations that work today +// byte-identical, including the two dead ends whose existing messages are good +// (`"123 Numbers"` → starts with a digit; `"!!!"` → nothing left). Above ASCII +// the classification is by Unicode category, so an em dash or a `»` is still an +// ordinary separator. +func LossyRunes(name string) []rune { + var lossy []rune + seen := map[rune]bool{} + for _, r := range strings.ToLower(strings.TrimSpace(name)) { + if r < utf8.RuneSelf { + // ASCII: either a slug character or a separator. Byte-identical to + // the derivation this CLI has always done. + continue + } + if unicode.IsSpace(r) || unicode.IsPunct(r) || unicode.IsSymbol(r) { + // A separator with no content of its own — becomes a hyphen. + continue + } + if seen[r] { + continue + } + seen[r] = true + lossy = append(lossy, r) + } + return lossy +} + +// maxNamedRunes bounds how many offending characters the refusal spells out. A +// fully non-Latin name can carry dozens of distinct runes and an error that +// re-prints the whole name back as a quoted list stops being readable; the first +// few are enough to make the refusal recognisable, and the name itself is quoted +// in the same sentence. +const maxNamedRunes = 6 + +// quoteRunes renders runes as a comma-separated list of quoted characters, for +// an error that has to NAME what it is refusing. +func quoteRunes(rs []rune) string { + more := "" + if len(rs) > maxNamedRunes { + more = fmt.Sprintf(" (and %d more)", len(rs)-maxNamedRunes) + rs = rs[:maxNamedRunes] + } + parts := make([]string, len(rs)) + for i, r := range rs { + parts[i] = fmt.Sprintf("%q", string(r)) + } + return strings.Join(parts, ", ") + more +} + // Slugify converts an arbitrary name into a valid block slug, or returns an -// error if it cannot produce one within the length bounds. +// error if it cannot produce one within the length bounds — or if deriving one +// would silently DISCARD characters the author typed (see LossyRunes). func Slugify(name string) (string, error) { + if lossy := LossyRunes(name); len(lossy) > 0 { + return "", fmt.Errorf("cannot derive a slug from %q: %s cannot appear in a blockId (lowercase a-z, 0-9 and hyphens only), and dropping them would give your app a different permanent public id than you typed — choose one yourself with --slug ", name, quoteRunes(lossy)) + } s := strings.ToLower(strings.TrimSpace(name)) s = nonSlugChars.ReplaceAllString(s, "-") s = strings.Trim(s, "-") diff --git a/internal/scaffold/slug_lossy_test.go b/internal/scaffold/slug_lossy_test.go new file mode 100644 index 0000000..e24e313 --- /dev/null +++ b/internal/scaffold/slug_lossy_test.go @@ -0,0 +1,208 @@ +package scaffold + +import ( + "fmt" + "strings" + "testing" +) + +// TestSlugifyRefusesRatherThanDroppingCharacters is the headline of issue #259. +// +// 🔴 EACH ROW PINS THE OLD OUTPUT IT MUST NOT PRODUCE, not merely "an error". +// "an error came back" is satisfied by a refusal for the wrong reason and by a +// refusal that happens to fire everywhere; naming `berapp` / `caf-del-mar` +// is what makes the row evidence about THIS defect. +func TestSlugifyRefusesRatherThanDroppingCharacters(t *testing.T) { + cases := []struct { + name string + // mustNotProduce is what Slugify returned BEFORE the fix — a silently + // different permanent public identity. + mustNotProduce string + // names are the characters the refusal has to point at. + names []string + }{ + { + // The leading Ü becomes a hyphen and is then trimmed, so the app's + // public id loses its first letter outright. + name: "ÜberApp Ω", + mustNotProduce: "berapp", + names: []string{"ü", "ω"}, + }, + { + // Worse than dropping: mid-string it INSERTS a word boundary that + // the author never typed. + name: "Café Del Mar", + mustNotProduce: "caf-del-mar", + names: []string{"é"}, + }, + { + name: "日本語アプリ", + mustNotProduce: "", + names: []string{"日", "本", "語"}, + }, + { + name: "Приложение", + mustNotProduce: "", + names: []string{"п", "р"}, + }, + { + name: "تطبيق", + mustNotProduce: "", + names: []string{"ت", "ط"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := Slugify(tc.name) + if err == nil { + t.Fatalf("Slugify(%q) = %q, want a refusal", tc.name, got) + } + if got != "" { + t.Errorf("a refused Slugify must return no slug, got %q", got) + } + // The pre-fix output must be unreachable, not merely unlikely. + if tc.mustNotProduce != "" { + if s, e := Slugify(tc.name); e == nil && s == tc.mustNotProduce { + t.Errorf("Slugify(%q) still produces the mangled %q", tc.name, tc.mustNotProduce) + } + } + msg := err.Error() + for _, r := range tc.names { + if !strings.Contains(msg, r) { + t.Errorf("refusal must NAME the offending character %q: %s", r, msg) + } + } + // Actionable: it points at the escape hatch, and states the rule. + if !strings.Contains(msg, "--slug") { + t.Errorf("refusal must point at --slug: %s", msg) + } + if !strings.Contains(msg, "a-z") || !strings.Contains(msg, "hyphens") { + t.Errorf("refusal must state the blockId constraint: %s", msg) + } + }) + } +} + +// TestSlugifyAsciiDerivationIsByteIdentical is the CONTROL. These rows were +// green before the refusal existed and must stay green: the predicate exempts +// every ASCII rune by construction, so ordinary punctuation and multiple spaces +// still fold to a single hyphen. An invariant guard, not regression coverage — +// its job is to fail loudly if the refusal ever widens. +func TestSlugifyAsciiDerivationIsByteIdentical(t *testing.T) { + cases := map[string]string{ + "My Cool Block": "my-cool-block", + "my-block": "my-block", + " Spaced Out ": "spaced-out", + "Foo/Bar_Baz.Qux!": "foo-bar-baz-qux", + "a & b + c = d": "a-b-c-d", + "Trailing punctuation.": "trailing-punctuation", + "UPPER case NAME": "upper-case-name", + "app2 v3": "app2-v3", + "a---b": "a-b", + } + for in, want := range cases { + got, err := Slugify(in) + if err != nil { + t.Errorf("Slugify(%q) errored, want %q: %v", in, want, err) + continue + } + if got != want { + t.Errorf("Slugify(%q) = %q, want %q", in, got, want) + } + } +} + +// TestSlugifyNonAsciiSeparatorsStaySeparators: above ASCII the classification is +// by Unicode category, so a punctuation or symbol rune is still a separator with +// no content of its own. Getting this wrong would refuse names that derive +// perfectly well — the false-refusal direction. +func TestSlugifyNonAsciiSeparatorsStaySeparators(t *testing.T) { + cases := map[string]string{ + "Widget — Pro": "widget-pro", // em dash (Pd) + "Widget « Pro »": "widget-pro", // guillemets (Pi/Pf) + "Widget © Pro": "widget-pro", // copyright sign (So) + "Widget Pro": "widget-pro", // no-break space (Zs) + } + for in, want := range cases { + got, err := Slugify(in) + if err != nil { + t.Errorf("Slugify(%q) errored, want %q: %v", in, want, err) + continue + } + if got != want { + t.Errorf("Slugify(%q) = %q, want %q", in, got, want) + } + } +} + +// TestSlugifyExistingDeadEndsKeepTheirMessages. Both of these are ASCII, so the +// new refusal must not intercept them — the issue itself praises their messages, +// and stealing them would ALSO make the "names the offending characters" claim +// false (there are none to name). +func TestSlugifyExistingDeadEndsKeepTheirMessages(t *testing.T) { + cases := map[string]string{ + "123 Numbers": "must start with a letter", + "!!!": "need ≥3 chars", + } + for in, want := range cases { + _, err := Slugify(in) + if err == nil { + t.Errorf("Slugify(%q) should still fail", in) + continue + } + if !strings.Contains(err.Error(), want) { + t.Errorf("Slugify(%q) message changed: got %q, want it to contain %q", in, err, want) + } + if strings.Contains(err.Error(), "--slug ") { + t.Errorf("Slugify(%q) was intercepted by the lossy-rune refusal: %v", in, err) + } + } +} + +// TestLossyRunesIsDeduplicatedAndOrdered — the message lists characters, so a +// name repeating one must not repeat it in the error. +func TestLossyRunesIsDeduplicatedAndOrdered(t *testing.T) { + got := LossyRunes("ÜÜber Ω Über") + want := []rune{'ü', 'ω'} + if len(got) != len(want) { + t.Fatalf("LossyRunes = %q, want %q", string(got), string(want)) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("LossyRunes = %q, want %q (first-appearance order)", string(got), string(want)) + } + } + if len(LossyRunes("My Cool Block")) != 0 { + t.Error("an ASCII name has no lossy runes") + } +} + +// TestSlugifyRefusalBoundsTheCharacterList — a fully non-Latin name can carry +// dozens of distinct runes; the refusal names the first few and counts the rest +// rather than echoing the whole name back as a quoted list. +func TestSlugifyRefusalBoundsTheCharacterList(t *testing.T) { + long := "日本語アプリケーションのテスト" + lossy := LossyRunes(long) + if len(lossy) <= maxNamedRunes { + t.Fatalf("fixture is broken: %q has %d distinct lossy runes, need > %d", long, len(lossy), maxNamedRunes) + } + _, err := Slugify(long) + if err == nil { + t.Fatal("expected a refusal") + } + msg := err.Error() + // The bounded prefix is named … + for _, r := range lossy[:maxNamedRunes] { + if !strings.Contains(msg, string(r)) { + t.Errorf("refusal should name %q: %s", string(r), msg) + } + } + // … and the remainder is COUNTED, not silently dropped. + if !strings.Contains(msg, fmt.Sprintf("(and %d more)", len(lossy)-maxNamedRunes)) { + t.Errorf("refusal should count the runes it did not spell out: %s", msg) + } + // The quoted-list is bounded: exactly maxNamedRunes quoted single characters. + if got := strings.Count(msg, `", "`); got != maxNamedRunes-1 { + t.Errorf("expected %d separators in the quoted list, got %d: %s", maxNamedRunes-1, got, msg) + } +} From f94e3d10fed4798cc8d59f17dcb5c87ba53e40b4 Mon Sep 17 00:00:00 2001 From: ZacxDev Date: Fri, 7 Aug 2026 16:55:13 -0500 Subject: [PATCH 2/4] fix(scaffold): name the character the author typed, and stop --slug eating the template prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 "ＡＢＣ" reported "ａ","ｂ","ｃ". 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://.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) --- internal/cmd/app_create.go | 12 +- internal/cmd/app_init.go | 108 +++++--- internal/cmd/app_init_echo_test.go | 248 ++++++++++++++++++ internal/cmd/scaffold_prompt_test.go | 8 +- internal/scaffold/slug.go | 192 ++++++++++---- internal/scaffold/slug_lossy_test.go | 374 +++++++++++++++++++++++---- 6 files changed, 807 insertions(+), 135 deletions(-) create mode 100644 internal/cmd/app_init_echo_test.go diff --git a/internal/cmd/app_create.go b/internal/cmd/app_create.go index ecca4f3..814342a 100644 --- a/internal/cmd/app_create.go +++ b/internal/cmd/app_create.go @@ -52,11 +52,13 @@ Templates (override with --template): The display name can be free-form ("My Cool Block"); it is slugified for the blockId. A slug-shaped name is used verbatim. -The blockId is your app's PERMANENT public identity — it is the hostname your app -is served at and the argument every later command takes — so derivation refuses -rather than guesses when the name carries characters a blockId cannot hold -("Café Del Mar", "ÜberApp", any non-Latin name). Pass --slug to choose one -yourself; it bypasses derivation entirely. +The blockId is your app's PERMANENT public identity — the hostname your app will +be served at once it is approved, and the argument every later command takes — so +derivation refuses rather than guesses when the name carries LETTERS a blockId +cannot hold ("Café Del Mar", "ÜberApp", any non-Latin name). Punctuation, symbols +and emoji still fold to a hyphen, as they always have ("Rocket 🚀 App" -> +rocket-app). Pass --slug to choose the blockId yourself; it bypasses +derivation entirely. By default the project is created in ./. Override the output directory with a positional [dir] or --dir ; override the display name independently with diff --git a/internal/cmd/app_init.go b/internal/cmd/app_init.go index 68d9cec..bd58aa1 100644 --- a/internal/cmd/app_init.go +++ b/internal/cmd/app_init.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "unicode/utf8" "github.com/charmbracelet/huh" "github.com/civitai/cli/internal/scaffold" @@ -37,11 +38,13 @@ Templates: The display name can be free-form ("My Cool Block"); it is slugified for the blockId. A slug-shaped name is used verbatim. -The blockId is your app's PERMANENT public identity — it is the hostname your app -is served at and the argument every later command takes — so derivation refuses -rather than guesses when the name carries characters a blockId cannot hold -("Café Del Mar", "ÜberApp", any non-Latin name). Pass --slug to choose one -yourself; it bypasses derivation entirely. +The blockId is your app's PERMANENT public identity — the hostname your app will +be served at once it is approved, and the argument every later command takes — so +derivation refuses rather than guesses when the name carries LETTERS a blockId +cannot hold ("Café Del Mar", "ÜberApp", any non-Latin name). Punctuation, symbols +and emoji still fold to a hyphen, as they always have ("Rocket 🚀 App" -> +rocket-app). Pass --slug to choose the blockId yourself; it bypasses +derivation entirely. By default the project is created in ./. Override the output directory with a positional [dir] or --dir ; override the display name independently with @@ -121,11 +124,18 @@ func runAppScaffold(cmd *cobra.Command, args []string, templateFlag, fromSlug, d // stdin (not a TTY), or --yes all SKIP huh — so scripted invocations never // block on a prompt (huh requires a TTY). The prompt fills in the name + // template; everything downstream is identical to the flag-driven path. - // --slug supplies the one thing the prompt exists to collect (an identity we - // can build a project around), so it also suppresses the prompt: the display - // name falls back to the slug's title case. - if name == "" && slugFlag == "" && !noInput && stdinIsTTY() { - inputs, err := scaffoldPromptFn(cmd, templateFlag) + // + // 🔴 --slug SUPPRESSES THE NAME FIELD, NOT THE PROMPT. An earlier version + // added `&& slugFlag == ""` to this condition, reasoning that --slug + // "supplies the one thing the prompt exists to collect". It does not: the + // form collects a name AND a TEMPLATE, so `civitai app create --slug my-app` + // on a TTY silently took page-money with no template choice offered — a + // question the user was asked before and is no longer asked. The identity is + // the only thing --slug settles, so it drops that one field and the template + // select still runs; the display name then falls back to the slug's title + // case. --yes remains the way to skip the prompt entirely. + if name == "" && !noInput && stdinIsTTY() { + inputs, err := scaffoldPromptFn(cmd, templateFlag, slugFlag == "") if err != nil { return err } @@ -135,6 +145,19 @@ func runAppScaffold(cmd *cobra.Command, args []string, templateFlag, fromSlug, d } } + // The display name is written VERBATIM into block.manifest.json. JSON is + // UTF-8 by definition, so an invalid-UTF-8 name produces a manifest no + // conforming parser can read — and the platform is the one that finds out. + // scaffold.Slugify refuses the same bytes for the blockId (see its (a) + // residual); this is the other half, and it is needed because --slug + // bypasses derivation entirely, so the name reaches the manifest unchecked. + // A bad flag/arg VALUE is a usage error (exit 2), same class as --template. + for _, v := range []struct{ what, val string }{{"the name argument", name}, {"--name", nameFlag}} { + if v.val != "" && !utf8.ValidString(v.val) { + return asUsageError(fmt.Errorf("%s is not valid UTF-8: %q — the display name is written into block.manifest.json verbatim, and JSON must be UTF-8", v.what, v.val)) + } + } + // A bad --template VALUE is a usage error (exit 2), exactly like a bad flag // NAME. scaffold cannot tag it itself — ErrUsage lives here, and internal/cmd // already imports internal/scaffold — so the tag is attached at the call site. @@ -165,7 +188,9 @@ func runAppScaffold(cmd *cobra.Command, args []string, templateFlag, fromSlug, d // Derive slug + display name. An explicit --slug wins outright; otherwise a // name that is already a valid slug is used verbatim, and anything else is // slugified (which REFUSES rather than dropping characters — see - // scaffold.LossyRunes). + // scaffold.LossyChars, and the residuals in scaffold.Slugify's header: + // derivation still folds symbols/emoji to a hyphen and still lowers the two + // runes above ASCII that lower INTO ASCII). nameIsSlug := name != "" && scaffold.ValidateSlug(name) == nil var slug, display string switch { @@ -267,7 +292,18 @@ func printScaffoldResult(out io.Writer, display, slug string, tmpl scaffold.Temp // name; with --dir it is invisible unless you open block.manifest.json. So // the case where derivation is most likely to surprise you was the case // where it was least visible (issue #259). - fmt.Fprintln(out, ui.Dim(fmt.Sprintf(" blockId: %s — permanent public id: https://%s.civit.ai/ · `civitai app status %s`", slug, slug, slug))) + // + // 🔴 THE URL IS A FUTURE TENSE, AND THAT IS NOT A STYLE CHOICE. The first + // version printed the bare `https://.civit.ai/` as the app's + // "permanent public id" at scaffold time — a URL that is guaranteed to 404 + // at that exact moment, because the subdomain is only programmed on + // approval + deploy (README: "Before approval, https://.civit.ai/ + // 404s"). That is the same false-promise class as the "validates clean" + // claim two lines up in this very output block, and `app status` already + // gets it right ("Not live yet — … only serves after the app is approved and + // deployed"). Keep the two surfaces saying the same thing. + fmt.Fprintln(out, ui.Dim(fmt.Sprintf(" blockId: %s — your app's permanent public id (it cannot be renamed later)", slug))) + fmt.Fprintln(out, ui.Dim(fmt.Sprintf(" Will be served at https://%s.civit.ai/ — only after the app is approved and deployed · `civitai app status %s`", slug, slug))) // page-money ships a runnable txt2img money path and a Comfy on Civitai // (customComfy) sample, with body builders for BOTH customComfy arms — the @@ -377,29 +413,35 @@ var scaffoldPromptFn = runScaffoldForm // runScaffoldForm runs the huh form that collects a missing app name + template. // defaultTemplate pre-selects the template (the command's default). The form // renders to stderr (status stream) and reads the command's stdin. -func runScaffoldForm(cmd *cobra.Command, defaultTemplate string) (scaffoldInputs, error) { +// +// askName is false when --slug already settled the identity: the TEMPLATE select +// still runs, because that is a second, independent question the prompt exists +// to ask and --slug says nothing about it. +func runScaffoldForm(cmd *cobra.Command, defaultTemplate string, askName bool) (scaffoldInputs, error) { in := scaffoldInputs{template: defaultTemplate} + var fields []huh.Field + if askName { + fields = append(fields, huh.NewInput(). + Title("App name"). + Description(`Free-form ("My Cool Block") — slugified for the blockId.`). + Value(&in.name). + Validate(func(s string) error { + if strings.TrimSpace(s) == "" { + return fmt.Errorf("a name is required") + } + return nil + })) + } + fields = append(fields, huh.NewSelect[string](). + Title("Template"). + Options( + huh.NewOption("static — no-build page app (index.html + a tiny JS)", string(scaffold.Static)), + huh.NewOption("page-vite — Vite + React page app", string(scaffold.PageVite)), + huh.NewOption("page-money — Vite + React + TS SDK money-path app", string(scaffold.PageMoney)), + ). + Value(&in.template)) form := huh.NewForm( - huh.NewGroup( - huh.NewInput(). - Title("App name"). - Description(`Free-form ("My Cool Block") — slugified for the blockId.`). - Value(&in.name). - Validate(func(s string) error { - if strings.TrimSpace(s) == "" { - return fmt.Errorf("a name is required") - } - return nil - }), - huh.NewSelect[string](). - Title("Template"). - Options( - huh.NewOption("static — no-build page app (index.html + a tiny JS)", string(scaffold.Static)), - huh.NewOption("page-vite — Vite + React page app", string(scaffold.PageVite)), - huh.NewOption("page-money — Vite + React + TS SDK money-path app", string(scaffold.PageMoney)), - ). - Value(&in.template), - ), + huh.NewGroup(fields...), ).WithInput(cmd.InOrStdin()).WithOutput(cmd.ErrOrStderr()) if err := form.Run(); err != nil { return in, err diff --git a/internal/cmd/app_init_echo_test.go b/internal/cmd/app_init_echo_test.go new file mode 100644 index 0000000..977c973 --- /dev/null +++ b/internal/cmd/app_init_echo_test.go @@ -0,0 +1,248 @@ +package cmd + +import ( + "errors" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + "unicode/utf8" + + "github.com/spf13/cobra" +) + +// serveURLRe finds the `https://.civit.ai/` the scaffold echoes. The +// capture group is the whole thing this file exists to assert on. +var serveURLRe = regexp.MustCompile(`https://([^\s]+)\.civit\.ai/`) + +// TestScaffoldServingURLIsBuiltFromTheSlugNotTheDisplayName pins the OPERAND of +// the echoed URL, which nothing did before. +// +// 🔴 SWAPPING `slug` FOR `display` IN THAT `Fprintf` SURVIVED THE WHOLE SUITE. +// Every existing assertion was a `strings.Contains(stdout, slug)`, and the FIRST +// `%s` on that line still carried the slug — so the mutant printed +// `https://Widget Machine.civit.ai/` as the app's "permanent public id" and no +// test could see it. The fixture keeps slug, display name and directory pairwise +// distinct AND substring-disjoint, so no assertion here can be satisfied by the +// wrong value. +func TestScaffoldServingURLIsBuiltFromTheSlugNotTheDisplayName(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "zzz-output-dir") + + stdout, _, err := run(t, "app", "create", "ignored-positional", + "--name", "Widget Machine", + "--slug", "qqq-block-id", + "--dir", dest, + "--template", "static") + if err != nil { + t.Fatalf("app create: %v\n%s", err, stdout) + } + + m := serveURLRe.FindAllStringSubmatch(stdout, -1) + if len(m) != 1 { + t.Fatalf("expected exactly one https://.civit.ai/ in the output, got %d:\n%s", len(m), stdout) + } + if got := m[0][1]; got != "qqq-block-id" { + t.Errorf("the serving URL must be built from the blockId, got host %q (want qqq-block-id):\n%s", got, stdout) + } + // The display name must not appear inside the URL at all — the mutation this + // test exists for produces `https://Widget Machine.civit.ai/`. + if strings.Contains(stdout, "https://Widget") { + t.Errorf("the display name leaked into the serving URL:\n%s", stdout) + } +} + +// TestScaffoldDoesNotPromiseTheURLServesYet — issue #260, same class as the +// "validates clean" promise two lines up in the SAME output block. +// +// 🔴 THE URL IS GUARANTEED TO 404 AT THE MOMENT IT IS PRINTED. The subdomain is +// only programmed on approval + deploy (README: "Before approval, +// https://.civit.ai/ 404s"), and `app status` already says so ("Not +// live yet — … only serves after the app is approved and deployed"). Printing it +// bare as the app's "permanent public id" walks a first-time author straight +// into a 404 they were told to expect to work. The qualifier must sit on the +// SAME LINE as the URL, or a later edit can move the URL out from under it. +func TestScaffoldDoesNotPromiseTheURLServesYet(t *testing.T) { + tmp := t.TempDir() + stdout, _, err := run(t, "app", "create", "my-block", "--dir", filepath.Join(tmp, "out"), "--template", "static") + if err != nil { + t.Fatalf("app create: %v\n%s", err, stdout) + } + + var urlLine string + for _, line := range strings.Split(stdout, "\n") { + if strings.Contains(line, ".civit.ai/") { + urlLine = line + break + } + } + if urlLine == "" { + t.Fatalf("expected the scaffold to echo a serving URL:\n%s", stdout) + } + // The wording deliberately matches `app status`'s "approved and deployed", + // so the two surfaces tell the author the same thing (app_status.go). + if !strings.Contains(urlLine, "approved and deployed") { + t.Errorf("the serving URL must be qualified on its own line — it 404s until approval:\n%s", urlLine) + } + // And the blockId is still labelled, on a line of its own. + if !strings.Contains(stdout, "blockId: my-block") { + t.Errorf("output must still LABEL the blockId:\n%s", stdout) + } +} + +// promptRecorder captures what the scaffold asked the interactive form for. +type promptRecorder struct { + calls int + askedName bool + defaultTemplate string +} + +func stubPrompt(t *testing.T, rec *promptRecorder, reply scaffoldInputs) { + t.Helper() + origTTY, origPrompt := stdinIsTTY, scaffoldPromptFn + t.Cleanup(func() { stdinIsTTY = origTTY; scaffoldPromptFn = origPrompt }) + stdinIsTTY = func() bool { return true } + scaffoldPromptFn = func(_ *cobra.Command, defaultTemplate string, askName bool) (scaffoldInputs, error) { + rec.calls++ + rec.askedName = askName + rec.defaultTemplate = defaultTemplate + return reply, nil + } +} + +// TestSlugFlagDropsTheNameFieldButKeepsTheTemplatePrompt — the regression this +// PR introduced and this test closes. +// +// 🔴 `--slug` USED TO SUPPRESS THE WHOLE PROMPT, on the reasoning that it +// "supplies the one thing the prompt exists to collect". `runScaffoldForm` +// collects a name AND a TEMPLATE, so a TTY user running +// `civitai app create --slug my-app` silently got page-money with no template +// choice — a question they were asked before the flag existed. The mutant +// deleting `slugFlag == ""` from that guard survived with ZERO failures because +// nothing covered the suppression at all. +// +// The assertions are structural, not "some prompt happened": the prompt must be +// CALLED, it must be told NOT to ask for a name, and the template it returns +// must be the one the project is built from — which is only observable because +// the reply (page-vite) differs from `app create`'s default (page-money). +func TestSlugFlagDropsTheNameFieldButKeepsTheTemplatePrompt(t *testing.T) { + var rec promptRecorder + stubPrompt(t, &rec, scaffoldInputs{name: "", template: "page-vite"}) + + tmp := t.TempDir() + chdir(t, tmp) + stdout, _, err := run(t, "app", "create", "--slug", "solo-block") + if err != nil { + t.Fatalf("app create --slug on a TTY: %v\n%s", err, stdout) + } + + if rec.calls != 1 { + t.Fatalf("--slug must NOT suppress the prompt: it was called %d time(s), want 1", rec.calls) + } + if rec.askedName { + t.Error("--slug settles the identity, so the NAME field must be dropped from the form") + } + if rec.defaultTemplate != "page-money" { + t.Errorf("the form should be pre-selected with the command's default template, got %q", rec.defaultTemplate) + } + // The prompted template is what got built — the whole point of still asking. + if !strings.Contains(stdout, "(page-vite)") { + t.Errorf("the template chosen at the prompt must be the one scaffolded:\n%s", stdout) + } + if got := readBlockID(t, filepath.Join(tmp, "solo-block")); got != "solo-block" { + t.Errorf("blockId = %q, want solo-block", got) + } +} + +// TestNoSlugFlagStillAsksForTheName is the other direction, and it is what stops +// the test above from being satisfied by a form that never asks for a name at +// all. Without --slug there is no identity yet, so both fields must be collected. +func TestNoSlugFlagStillAsksForTheName(t *testing.T) { + var rec promptRecorder + stubPrompt(t, &rec, scaffoldInputs{name: "prompted-block", template: "static"}) + + tmp := t.TempDir() + chdir(t, tmp) + if stdout, _, err := run(t, "app", "create"); err != nil { + t.Fatalf("app create on a TTY: %v\n%s", err, stdout) + } + if rec.calls != 1 { + t.Fatalf("the prompt must run when no name and no --slug are supplied (calls=%d)", rec.calls) + } + if !rec.askedName { + t.Error("with no --slug the form still has to collect the name") + } +} + +// TestScaffoldRefusesAnInvalidUTF8Name — the manifest half of the silent-loss +// class #259 is about. `civitai app create $'caf\xe9 app'` used to derive the +// blockId `caf-app` with rc 0 AND write a block.manifest.json holding the raw +// 0xE9, which is not valid UTF-8 and which `validate.ManifestOnly` accepted. +// scaffold.Slugify now refuses the derivation; this pins the other route in, +// because --slug bypasses derivation entirely and the name still lands in the +// manifest verbatim. +// +// Classification is asserted with errors.Is, never message text (AGENTS item 7). +func TestScaffoldRefusesAnInvalidUTF8Name(t *testing.T) { + const badName = "caf\xe9 app" + if utf8.ValidString(badName) { + t.Fatal("fixture is broken: badName must NOT be valid UTF-8") + } + + t.Run("as the positional name, with --slug", func(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "out") + _, _, err := run(t, "app", "create", badName, "--slug", "cafe-app", "--dir", dest, "--template", "static") + if err == nil { + t.Fatal("an invalid-UTF-8 display name must be refused") + } + if !errors.Is(err, ErrUsage) { + t.Errorf("a bad name VALUE is a usage error: %v", err) + } + if _, statErr := os.Stat(dest); statErr == nil { + t.Error("refused, but a project was still written") + } + }) + + t.Run("as --name", func(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "out") + _, _, err := run(t, "app", "create", "my-block", "--name", badName, "--dir", dest, "--template", "static") + if err == nil { + t.Fatal("an invalid-UTF-8 --name must be refused") + } + if !errors.Is(err, ErrUsage) { + t.Errorf("a bad --name VALUE is a usage error: %v", err) + } + }) + + t.Run("derivation refuses it too, with no --slug", func(t *testing.T) { + tmp := t.TempDir() + _, _, err := run(t, "app", "create", badName, "--dir", filepath.Join(tmp, "out"), "--template", "static") + if err == nil { + t.Fatal("invalid UTF-8 must not derive a slug") + } + if !errors.Is(err, ErrUsage) { + t.Errorf("a bad name VALUE is a usage error: %v", err) + } + }) + + // POSITIVE CONTROL: the same shape with VALID UTF-8 succeeds and writes a + // manifest that is valid UTF-8 — so the rows above are evidence about the + // bytes, not about a build that refuses everything. + t.Run("a valid non-ASCII name still scaffolds with --slug", func(t *testing.T) { + tmp := t.TempDir() + dest := filepath.Join(tmp, "out") + if _, _, err := run(t, "app", "create", "Café App", "--slug", "cafe-app", "--dir", dest, "--template", "static"); err != nil { + t.Fatalf("a valid UTF-8 name must still work: %v", err) + } + b, readErr := os.ReadFile(filepath.Join(dest, "block.manifest.json")) + if readErr != nil { + t.Fatalf("read manifest: %v", readErr) + } + if !utf8.Valid(b) { + t.Error("the written manifest must be valid UTF-8") + } + }) +} diff --git a/internal/cmd/scaffold_prompt_test.go b/internal/cmd/scaffold_prompt_test.go index e51f64b..701a795 100644 --- a/internal/cmd/scaffold_prompt_test.go +++ b/internal/cmd/scaffold_prompt_test.go @@ -24,7 +24,7 @@ func TestMain(m *testing.M) { func TestScaffoldNonInteractiveNoPrompt(t *testing.T) { orig := scaffoldPromptFn t.Cleanup(func() { scaffoldPromptFn = orig }) - scaffoldPromptFn = func(_ *cobra.Command, _ string) (scaffoldInputs, error) { + scaffoldPromptFn = func(_ *cobra.Command, _ string, _ bool) (scaffoldInputs, error) { t.Fatal("huh prompt must NOT run on a non-TTY stdin") return scaffoldInputs{}, nil } @@ -44,7 +44,7 @@ func TestScaffoldYesSkipsPromptEvenOnTTY(t *testing.T) { t.Cleanup(func() { stdinIsTTY = origTTY; scaffoldPromptFn = origPrompt }) stdinIsTTY = func() bool { return true } - scaffoldPromptFn = func(_ *cobra.Command, _ string) (scaffoldInputs, error) { + scaffoldPromptFn = func(_ *cobra.Command, _ string, _ bool) (scaffoldInputs, error) { t.Fatal("huh prompt must NOT run when --yes is set") return scaffoldInputs{}, nil } @@ -65,7 +65,7 @@ func TestScaffoldInteractiveUsesPromptValues(t *testing.T) { t.Cleanup(func() { stdinIsTTY = origTTY; scaffoldPromptFn = origPrompt }) stdinIsTTY = func() bool { return true } - scaffoldPromptFn = func(_ *cobra.Command, _ string) (scaffoldInputs, error) { + scaffoldPromptFn = func(_ *cobra.Command, _ string, _ bool) (scaffoldInputs, error) { return scaffoldInputs{name: "prompted-block", template: "static"}, nil } @@ -91,7 +91,7 @@ func TestScaffoldNameArgSkipsPrompt(t *testing.T) { t.Cleanup(func() { stdinIsTTY = origTTY; scaffoldPromptFn = origPrompt }) stdinIsTTY = func() bool { return true } - scaffoldPromptFn = func(_ *cobra.Command, _ string) (scaffoldInputs, error) { + scaffoldPromptFn = func(_ *cobra.Command, _ string, _ bool) (scaffoldInputs, error) { t.Fatal("huh prompt must NOT run when a name is supplied") return scaffoldInputs{}, nil } diff --git a/internal/scaffold/slug.go b/internal/scaffold/slug.go index 8d590da..fa8fdb2 100644 --- a/internal/scaffold/slug.go +++ b/internal/scaffold/slug.go @@ -68,82 +68,176 @@ var slugPattern = regexp.MustCompile(`^[a-z][a-z0-9-]*[a-z0-9]$`) var nonSlugChars = regexp.MustCompile(`[^a-z0-9]+`) -// LossyRunes returns, in first-appearance order and deduplicated, the runes in -// name that slug derivation would DROP rather than fold into a hyphen. +// isCombiningMark reports whether r is a Unicode Mark (Mn/Mc/Me) — a character +// that has no standalone appearance and renders on top of the one before it. +func isCombiningMark(r rune) bool { return unicode.Is(unicode.M, r) } + +// isLossyBase classifies ONE base rune: true when derivation would DROP it +// rather than fold it into a hyphen. +// +// 🔴 THE CLASSIFICATION IS ON THE LOWERED RUNE AND THE REPORT IS ON THE ORIGINAL +// — the two must not be collapsed. Derivation lowercases first, so lowering is +// what decides whether a rune survives; but a message that quotes the LOWERED +// rune names a character the user never typed. Measured on the first version of +// this code, which classified and reported off `strings.ToLower(name)`: +// `"ẞE App"` reported `"ß"`, a rune ABSENT from the input, and `"ＡＢＣ"` +// reported `"ａ", "ｂ", "ｃ"`. Someone searching their own name for the quoted +// character finds nothing. +// +// 🔴 The rule itself is the ASYMMETRY between the two things a non-`[a-z0-9]` +// rune can be. A SEPARATOR (space, `_`, `.`, `/`, `-`, `!`, `&`, an em dash …) +// carries no information of its own: turning it into a hyphen is what the author +// meant, and that is why `"My Cool Block"` → `my-cool-block` is correct. A +// LETTER, DIGIT or MARK the ASCII slug alphabet cannot carry is the opposite: it +// is content the author typed, and there is no lossless place to put it. +// Dropping it silently mints a DIFFERENT permanent public identity — +// `"ÜberApp Ω"` → `berapp` (the leading `Ü` becomes a hyphen and is then +// trimmed), `"Café Del Mar"` → `caf-del-mar` (a spurious word boundary +// mid-name). Neither is a truncation the author can recognise, and a blockId is +// not renameable, so the caller refuses and asks for an explicit slug instead. +// +// 🔴 EVERY RUNE THAT LOWERS INTO ASCII IS EXEMPT BY CONSTRUCTION, and that is +// load-bearing rather than lazy: it is what makes the ASCII derivations that +// work today byte-identical, including the two dead ends whose existing messages +// are good (`"123 Numbers"` → starts with a digit; `"!!!"` → nothing left). It +// is also the source of the (b) exception in the Slugify header: exactly two +// runes above ASCII lower INTO ASCII. Do not "improve" the exemption into a +// character allowlist — an allowlist re-decides every ASCII derivation this CLI +// has ever produced, and the whole point of the boundary is that it decides +// none of them. +func isLossyBase(r rune) bool { + lower := unicode.ToLower(r) + if lower < utf8.RuneSelf { + // Lowers into ASCII: either a slug character or a separator. + // Byte-identical to the derivation this CLI has always done. + return false + } + // A separator with no content of its own — becomes a hyphen. + return !unicode.IsSpace(lower) && !unicode.IsPunct(lower) && !unicode.IsSymbol(lower) +} + +// dottedCircle is the Unicode convention for displaying a combining mark that +// has no base character to sit on (U+25CC DOTTED CIRCLE). +const dottedCircle = "◌" + +// LossyChars returns, in first-appearance order and deduplicated, the characters +// in name that slug derivation would DROP rather than fold into a hyphen — each +// as the printable string the AUTHOR typed, never a lowered or decomposed +// substitute. // -// 🔴 The point is the ASYMMETRY between the two things a non-`[a-z0-9]` rune can -// be. A SEPARATOR (space, `_`, `.`, `/`, `-`, `!`, `&`, an em dash …) carries no -// information of its own: turning it into a hyphen is what the author meant, and -// that is why `"My Cool Block"` → `my-cool-block` is correct. A LETTER, DIGIT or -// MARK the ASCII slug alphabet cannot carry is the opposite: it is content the -// author typed, and there is no lossless place to put it. Dropping it silently -// mints a DIFFERENT permanent public identity — `"ÜberApp Ω"` → `berapp` (the -// leading `Ü` becomes a hyphen and is then trimmed), `"Café Del Mar"` → -// `caf-del-mar` (a spurious word boundary mid-name). Neither is a truncation the -// author can recognise, and a blockId is not renameable, so the caller refuses -// and asks for an explicit slug instead. +// 🔴 A COMBINING MARK IS REPORTED WITH ITS BASE, because on its own it renders +// as an accent floating over nothing. macOS paths and some paste routes deliver +// NFD, so `"Café App"` arrives as `e` + U+0301 and the first version of this +// message read `"́" cannot appear in a blockId` — the same VISIBLE name yielding +// two different messages, one of them pointing at nothing the user can see. The +// cluster is what gets quoted, so NFC and NFD both report `"é"`. This changes +// only the RENDERING: a mark is non-ASCII and is neither space, punct nor +// symbol, so it was already refused, and the set of refused names is unchanged. +// A mark with no base at all (a name STARTING with one) is shown on a dotted +// circle rather than bare. // // Transliteration was considered and rejected: `Ω → o` vs `omega` is a // locale-dependent judgement, a general transliterator means a new // `golang.org/x/text` dependency, and it produces nothing usable for CJK, // Cyrillic or Arabic — which is most of the population this refusal serves. // -// 🔴 EVERY ASCII RUNE IS EXEMPT BY CONSTRUCTION, and that is load-bearing rather -// than lazy: it is what makes the ASCII derivations that work today -// byte-identical, including the two dead ends whose existing messages are good -// (`"123 Numbers"` → starts with a digit; `"!!!"` → nothing left). Above ASCII -// the classification is by Unicode category, so an em dash or a `»` is still an -// ordinary separator. -func LossyRunes(name string) []rune { - var lossy []rune - seen := map[rune]bool{} - for _, r := range strings.ToLower(strings.TrimSpace(name)) { - if r < utf8.RuneSelf { - // ASCII: either a slug character or a separator. Byte-identical to - // the derivation this CLI has always done. - continue +// name MUST be valid UTF-8; Slugify gates that before calling here. Ranging over +// invalid bytes yields U+FFFD, which is a Symbol and would be waved through as a +// separator. +func LossyChars(name string) []string { + runes := []rune(strings.TrimSpace(name)) + var lossy []string + seen := map[string]bool{} + add := func(s string) { + if seen[s] { + return } - if unicode.IsSpace(r) || unicode.IsPunct(r) || unicode.IsSymbol(r) { - // A separator with no content of its own — becomes a hyphen. - continue + seen[s] = true + lossy = append(lossy, s) + } + for i := 0; i < len(runes); { + base := runes[i] + j := i + 1 + for j < len(runes) && isCombiningMark(runes[j]) { + j++ } - if seen[r] { - continue + switch { + case isCombiningMark(base): + // Only reachable at the very start of the name: every other mark is + // consumed by the base before it. + add(dottedCircle + string(runes[i:j])) + case j > i+1 || isLossyBase(base): + // Either the cluster carries a mark (content, dropped by + // derivation) or the base itself is lossy. Report the cluster. + add(string(runes[i:j])) } - seen[r] = true - lossy = append(lossy, r) + i = j } return lossy } -// maxNamedRunes bounds how many offending characters the refusal spells out. A -// fully non-Latin name can carry dozens of distinct runes and an error that +// maxNamedChars bounds how many offending characters the refusal spells out. A +// fully non-Latin name can carry dozens of distinct characters and an error that // re-prints the whole name back as a quoted list stops being readable; the first // few are enough to make the refusal recognisable, and the name itself is quoted // in the same sentence. -const maxNamedRunes = 6 +const maxNamedChars = 6 -// quoteRunes renders runes as a comma-separated list of quoted characters, for -// an error that has to NAME what it is refusing. -func quoteRunes(rs []rune) string { +// quoteChars renders characters as a comma-separated list of quoted characters, +// for an error that has to NAME what it is refusing. +func quoteChars(cs []string) string { more := "" - if len(rs) > maxNamedRunes { - more = fmt.Sprintf(" (and %d more)", len(rs)-maxNamedRunes) - rs = rs[:maxNamedRunes] + if len(cs) > maxNamedChars { + more = fmt.Sprintf(" (and %d more)", len(cs)-maxNamedChars) + cs = cs[:maxNamedChars] } - parts := make([]string, len(rs)) - for i, r := range rs { - parts[i] = fmt.Sprintf("%q", string(r)) + parts := make([]string, len(cs)) + for i, c := range cs { + parts[i] = fmt.Sprintf("%q", c) } return strings.Join(parts, ", ") + more } // Slugify converts an arbitrary name into a valid block slug, or returns an // error if it cannot produce one within the length bounds — or if deriving one -// would silently DISCARD characters the author typed (see LossyRunes). +// would silently DISCARD characters the author typed (see LossyChars). +// +// 🔴 THIS NARROWS ISSUE #259, IT DOES NOT CLOSE IT. Three classes of input still +// derive a slug that quietly differs from the name, and all three are stated +// here rather than implied away — a residual nobody writes down is +// indistinguishable from a bug nobody noticed: +// +// - (a) INVALID UTF-8 — CLOSED. `caf\xe9 app` used to derive `caf-app` with +// rc 0, because `for _, r := range` yields U+FFFD per bad byte and U+FFFD is +// a Symbol, i.e. the separator branch. Worse, the manifest the scaffold then +// wrote held the raw `0xE9` and was not valid UTF-8 — and `validate` accepted +// it. The `utf8.ValidString` guard below refuses it. (The manifest half is +// closed at the call site, which also refuses an invalid-UTF-8 DISPLAY name; +// `internal/validate` still has no UTF-8 check of its own.) +// - (b) THE TWO RUNES ABOVE ASCII THAT LOWER INTO ASCII — a deliberate, +// ENUMERATED exception, not an oversight. All of Unicode was walked: exactly +// two exist, `İ` U+0130 → `i` and `K` U+212A KELVIN SIGN → `k`. So +// `"İstanbul App"` derives `istanbul-app` with rc 0. That is arguably the +// nicest transliteration available and it costs nothing, so it stays — but +// it means the exemption below is "lowers into ASCII", never "is ASCII", +// and any absolute claim that every non-ASCII letter is refused is false. +// - (c) SYMBOLS, EMOJI AND NON-ASCII PUNCTUATION fold to a hyphen, so +// `"Rocket 🚀 App"` derives `rocket-app` with rc 0. Census over printable +// non-ASCII runes: 8,580 take the separator branch, 140,321 the refuse +// branch. This is the asymmetry working as designed for `—` and `»`, and +// arguably NOT what an author means by an emoji — but an emoji has no +// lossless ASCII form either, so refusing would only trade a silent drop for +// a dead end. Left as-is deliberately, tracked separately. func Slugify(name string) (string, error) { - if lossy := LossyRunes(name); len(lossy) > 0 { - return "", fmt.Errorf("cannot derive a slug from %q: %s cannot appear in a blockId (lowercase a-z, 0-9 and hyphens only), and dropping them would give your app a different permanent public id than you typed — choose one yourself with --slug ", name, quoteRunes(lossy)) + // 🔴 BEFORE anything ranges over the string. `for _, r := range` silently + // substitutes U+FFFD for each invalid byte, and U+FFFD classifies as a + // Symbol — the separator branch — so a mojibake name derived a slug that + // dropped the bad bytes AND wrote them raw into block.manifest.json. + if !utf8.ValidString(name) { + return "", fmt.Errorf("cannot derive a slug from %q: the name is not valid UTF-8, so the characters it holds cannot be read (a blockId is lowercase a-z, 0-9 and hyphens only), and deriving one anyway would give your app a different permanent public id than you typed — choose one yourself with --slug ", name) + } + if lossy := LossyChars(name); len(lossy) > 0 { + return "", fmt.Errorf("cannot derive a slug from %q: %s cannot appear in a blockId (lowercase a-z, 0-9 and hyphens only), and dropping them would give your app a different permanent public id than you typed — choose one yourself with --slug ", name, quoteChars(lossy)) } s := strings.ToLower(strings.TrimSpace(name)) s = nonSlugChars.ReplaceAllString(s, "-") diff --git a/internal/scaffold/slug_lossy_test.go b/internal/scaffold/slug_lossy_test.go index e24e313..0864929 100644 --- a/internal/scaffold/slug_lossy_test.go +++ b/internal/scaffold/slug_lossy_test.go @@ -2,76 +2,166 @@ package scaffold import ( "fmt" + "regexp" "strings" "testing" + "unicode" + "unicode/utf8" ) +// runesThatLowerIntoASCII walks ALL of Unicode for runes above ASCII whose +// unicode.ToLower lands back inside ASCII. It is a walk rather than a literal +// pair so the "exactly two" claim in Slugify's header tracks the Go unicode +// tables instead of somebody's memory of them. +func runesThatLowerIntoASCII() []rune { + var out []rune + for r := rune(utf8.RuneSelf); r <= unicode.MaxRune; r++ { + if !utf8.ValidRune(r) { + continue + } + if unicode.ToLower(r) < utf8.RuneSelf { + out = append(out, r) + } + } + return out +} + +// legacySlugify is the derivation EXACTLY as it stood before the refusal landed +// (`e800129`-era `Slugify`, minus the two new guards). It exists so a +// `mustNotProduce` row is a MEASUREMENT of the old behaviour rather than a +// remembered string: a row naming an output the old algorithm never produced +// would be evidence about nothing, and nothing else in the suite could tell. +func legacySlugify(name string) (string, error) { + s := strings.ToLower(strings.TrimSpace(name)) + s = regexp.MustCompile(`[^a-z0-9]+`).ReplaceAllString(s, "-") + s = strings.Trim(s, "-") + for strings.Contains(s, "--") { + s = strings.ReplaceAll(s, "--", "-") + } + if len(s) < 3 { + return "", fmt.Errorf("cannot derive a valid slug from %q (need ≥3 chars)", name) + } + if len(s) > 40 { + s = strings.Trim(s[:40], "-") + } + if !slugPattern.MatchString(s) { + return "", fmt.Errorf("derived slug %q is invalid", s) + } + return s, nil +} + // TestSlugifyRefusesRatherThanDroppingCharacters is the headline of issue #259. // // 🔴 EACH ROW PINS THE OLD OUTPUT IT MUST NOT PRODUCE, not merely "an error". // "an error came back" is satisfied by a refusal for the wrong reason and by a -// refusal that happens to fire everywhere; naming `berapp` / `caf-del-mar` -// is what makes the row evidence about THIS defect. +// refusal that happens to fire everywhere; naming `berapp` / `caf-del-mar` is +// what makes the row evidence about THIS defect. +// +// 🔴 AND THE ASSERTION HAS TO EXECUTE. The first version put that check behind a +// `t.Fatalf("want a refusal")` that had already aborted the subtest whenever +// `err == nil`, so the `mustNotProduce` block was UNREACHABLE — measured: +// deleting it left the package green while a positive control reddened 1, i.e. +// the harness could go red and this assertion simply never ran. The PR body's +// headline claim rested on it. It now lives INSIDE the `err == nil` branch, +// where it is the only thing that can distinguish "derivation came back" from +// "derivation came back with the exact #259 mangling". func TestSlugifyRefusesRatherThanDroppingCharacters(t *testing.T) { cases := []struct { name string // mustNotProduce is what Slugify returned BEFORE the fix — a silently - // different permanent public identity. + // different permanent public identity. "" means the pre-fix derivation + // bottomed out in one of the two ASCII dead ends instead. mustNotProduce string - // names are the characters the refusal has to point at. - names []string + // chars are the characters the refusal has to point at, AS TYPED. + chars []string }{ { // The leading Ü becomes a hyphen and is then trimmed, so the app's // public id loses its first letter outright. name: "ÜberApp Ω", mustNotProduce: "berapp", - names: []string{"ü", "ω"}, + chars: []string{"Ü", "Ω"}, }, { // Worse than dropping: mid-string it INSERTS a word boundary that // the author never typed. name: "Café Del Mar", mustNotProduce: "caf-del-mar", - names: []string{"é"}, + chars: []string{"é"}, + }, + { + // 🔴 THE ROW THAT PINS "REPORT THE ORIGINAL, NOT THE LOWERED RUNE". + // U+1E9E LATIN CAPITAL LETTER SHARP S lowers to U+00DF ß — a + // DIFFERENT character, and one absent from this input. The first + // version classified and reported off strings.ToLower(name) and so + // quoted "ß" at a user who never typed it. + name: "\u1e9eE App", + mustNotProduce: "e-app", + chars: []string{"\u1e9e"}, + }, + { + // Same class, fullwidth: Ａ lowers to ａ, so the pre-fix message + // named "ａ", "ｂ", "ｃ" instead of what was typed. + name: "\uff21\uff22\uff23 Widget", + mustNotProduce: "widget", + chars: []string{"\uff21", "\uff22", "\uff23"}, }, { name: "日本語アプリ", mustNotProduce: "", - names: []string{"日", "本", "語"}, + chars: []string{"日", "本", "語"}, }, { name: "Приложение", mustNotProduce: "", - names: []string{"п", "р"}, + chars: []string{"П", "р"}, }, { name: "تطبيق", mustNotProduce: "", - names: []string{"ت", "ط"}, + chars: []string{"ت", "ط"}, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { + // Positive control on the FIXTURE: mustNotProduce must be what the + // pre-fix derivation really produced, or the row below is asserting + // against a string nothing ever emitted. + legacy, legacyErr := legacySlugify(tc.name) + if tc.mustNotProduce == "" { + if legacyErr == nil { + t.Fatalf("fixture is broken: the pre-fix derivation produced %q for %q, so mustNotProduce must name it", legacy, tc.name) + } + } else if legacyErr != nil || legacy != tc.mustNotProduce { + t.Fatalf("fixture is broken: the pre-fix derivation gave (%q, %v) for %q, want %q", legacy, legacyErr, tc.name, tc.mustNotProduce) + } + got, err := Slugify(tc.name) if err == nil { + // 🔴 This is the reachable form of "the pre-fix output must be + // unreachable". A build that reintroduces the mangling fails + // HERE, naming it, rather than in the generic branch below. + if tc.mustNotProduce != "" && got == tc.mustNotProduce { + t.Fatalf("Slugify(%q) = %q — that is the exact pre-fix mangling issue #259 is about, a different permanent public id than the author typed", tc.name, got) + } t.Fatalf("Slugify(%q) = %q, want a refusal", tc.name, got) } if got != "" { t.Errorf("a refused Slugify must return no slug, got %q", got) } - // The pre-fix output must be unreachable, not merely unlikely. - if tc.mustNotProduce != "" { - if s, e := Slugify(tc.name); e == nil && s == tc.mustNotProduce { - t.Errorf("Slugify(%q) still produces the mangled %q", tc.name, tc.mustNotProduce) - } - } msg := err.Error() - for _, r := range tc.names { - if !strings.Contains(msg, r) { - t.Errorf("refusal must NAME the offending character %q: %s", r, msg) + for _, c := range tc.chars { + if !strings.Contains(msg, c) { + t.Errorf("refusal must NAME the offending character %q AS TYPED: %s", c, msg) } } + // 🔴 The refusal quotes the INPUT. Nothing asserted this, so a + // mutant replacing the interpolated name with a fixed literal + // survived — an error naming somebody else's name is worse than one + // naming none. + if want := fmt.Sprintf("%q", tc.name); !strings.Contains(msg, want) { + t.Errorf("refusal must quote the name the author typed (%s): %s", want, msg) + } // Actionable: it points at the escape hatch, and states the rule. if !strings.Contains(msg, "--slug") { t.Errorf("refusal must point at --slug: %s", msg) @@ -83,11 +173,137 @@ func TestSlugifyRefusesRatherThanDroppingCharacters(t *testing.T) { } } +// TestSlugifyReportsTheCharacterTheAuthorTyped is the same claim as the "ẞE App" +// row above, stated directly against LossyChars so it cannot be satisfied by any +// other part of the refusal sentence. Each row's lowered form is a character +// that is NOT in the input — that is what makes the row discriminating. +func TestSlugifyReportsTheCharacterTheAuthorTyped(t *testing.T) { + cases := []struct { + in string + want []string + mustNotReport []string + }{ + {in: "\u1e9eE App", want: []string{"\u1e9e"}, mustNotReport: []string{"\u00df"}}, + {in: "\uff21\uff22\uff23", want: []string{"\uff21", "\uff22", "\uff23"}, mustNotReport: []string{"\uff41", "\uff42", "\uff43"}}, + {in: "ÜberApp Ω", want: []string{"Ü", "Ω"}, mustNotReport: []string{"ü", "ω"}}, + } + for _, tc := range cases { + t.Run(tc.in, func(t *testing.T) { + got := strings.Join(LossyChars(tc.in), "") + for _, w := range tc.want { + if !strings.Contains(got, w) { + t.Errorf("LossyChars(%q) = %q, must report the typed %q", tc.in, got, w) + } + } + for _, bad := range tc.mustNotReport { + if strings.Contains(got, bad) { + t.Errorf("LossyChars(%q) = %q — %q is the LOWERED form and does not appear in the input", tc.in, got, bad) + } + } + // And the same through the real message, so the fix cannot be + // correct in the predicate and lost at the printer. + _, err := Slugify(tc.in) + if err == nil { + t.Fatalf("Slugify(%q) should refuse", tc.in) + } + for _, bad := range tc.mustNotReport { + if strings.Contains(err.Error(), bad) { + t.Errorf("refusal names the lowered %q, absent from the input: %v", bad, err) + } + } + }) + } +} + +// TestSlugifyRefusesInvalidUTF8 — residual (a) of the Slugify header, now +// closed. `for _, r := range` yields U+FFFD per invalid byte and U+FFFD is a +// Symbol, i.e. the SEPARATOR branch, so mojibake derived a slug with rc 0 AND +// the scaffold wrote the raw bytes into block.manifest.json. +func TestSlugifyRefusesInvalidUTF8(t *testing.T) { + // Measured pre-fix outputs, i.e. exactly the #259 shape. + cases := map[string]string{ + "caf\xe9 app": "caf-app", + "\xff\xfeWidget": "widget", + "Sm\xc3rt Widget": "sm-rt-widget", + } + for in, legacy := range cases { + t.Run(fmt.Sprintf("%q", in), func(t *testing.T) { + // Positive control on the fixture: the bytes really are invalid and + // the pre-fix derivation really produced the named slug. + if got, err := legacySlugify(in); err != nil || got != legacy { + t.Fatalf("fixture is broken: pre-fix derivation gave (%q, %v), want %q", got, err, legacy) + } + got, err := Slugify(in) + if err == nil { + t.Fatalf("Slugify(%q) = %q, want a refusal — invalid UTF-8 silently loses bytes", in, got) + } + if got != "" { + t.Errorf("a refused Slugify must return no slug, got %q", got) + } + if !strings.Contains(err.Error(), "UTF-8") { + t.Errorf("the refusal should say what is wrong with the name: %v", err) + } + if !strings.Contains(err.Error(), "--slug") { + t.Errorf("refusal must point at the escape hatch: %v", err) + } + }) + } +} + +// TestLossyCharsReportsACombiningMarkWithItsBase — F5. macOS paths and some +// paste routes deliver NFD, so the SAME VISIBLE NAME arrives as two different +// byte sequences. Before this, NFC reported "é" and NFD reported a bare +// combining acute rendered over nothing. The refusal SET is identical either +// way (a mark was always lossy); only the rendering differed. +func TestLossyCharsReportsACombiningMarkWithItsBase(t *testing.T) { + const ( + nfc = "Caf\u00e9 App" // e-acute as ONE rune (U+00E9) + nfd = "Cafe\u0301 App" // 'e' + U+0301 COMBINING ACUTE ACCENT + bare = "\u0301Widget name" // a combining mark with no base at all + ) + // Fixture control. A character that RENDERS identically can be a different + // code point, so assert the bytes rather than trusting the source text. + if nfc == nfd { + t.Fatal("fixture is broken: the NFC and NFD forms must be different byte sequences") + } + if !strings.ContainsRune(nfd, '\u0301') || strings.ContainsRune(nfc, '\u0301') { + t.Fatalf("fixture is broken: only the NFD form may carry U+0301 (nfc=% x nfd=% x)", nfc, nfd) + } + + for _, tc := range []struct{ in, want string }{ + {nfc, "\u00e9"}, + {nfd, "e\u0301"}, + } { + got := LossyChars(tc.in) + if len(got) != 1 || got[0] != tc.want { + t.Fatalf("LossyChars(% x) = %q, want the single cluster [%q] the author actually sees", tc.in, got, tc.want) + } + // READABILITY is the whole point: the reported token must lead with a + // base character, never a lone combining mark floating over nothing. + if []rune(got[0])[0] == '\u0301' { + t.Errorf("LossyChars(% x) reported a bare combining mark %q", tc.in, got[0]) + } + if _, err := Slugify(tc.in); err == nil { + t.Fatalf("Slugify(% x) should refuse", tc.in) + } + } + + // A leading mark has no base to attach to, so it is shown on a dotted + // circle (the Unicode convention) rather than bare. + got := LossyChars(bare) + if len(got) == 0 { + t.Fatal("a leading combining mark is still content derivation would drop") + } + if !strings.HasPrefix(got[0], dottedCircle) { + t.Errorf("a base-less combining mark must be shown on a dotted circle, got %q", got[0]) + } +} + // TestSlugifyAsciiDerivationIsByteIdentical is the CONTROL. These rows were // green before the refusal existed and must stay green: the predicate exempts -// every ASCII rune by construction, so ordinary punctuation and multiple spaces -// still fold to a single hyphen. An invariant guard, not regression coverage — -// its job is to fail loudly if the refusal ever widens. +// every rune that LOWERS INTO ASCII by construction, so ordinary punctuation and +// multiple spaces still fold to a single hyphen. An invariant guard, not +// regression coverage — its job is to fail loudly if the refusal ever widens. func TestSlugifyAsciiDerivationIsByteIdentical(t *testing.T) { cases := map[string]string{ "My Cool Block": "my-cool-block", @@ -112,6 +328,76 @@ func TestSlugifyAsciiDerivationIsByteIdentical(t *testing.T) { } } +// TestSlugifyAsciiExemptionBoundary pins the `< utf8.RuneSelf` comparison from +// BOTH sides. Widening it to `<=` was a surviving mutant: U+0080 is a C1 control +// nobody types, so no behavioural row saw the change — but the doc comment calls +// that boundary load-bearing, and an unpinned boundary is a claim nothing holds. +func TestSlugifyAsciiExemptionBoundary(t *testing.T) { + // U+007F DEL is the last ASCII rune: exempt, folded to a hyphen like any + // other non-slug ASCII character. + if got, err := Slugify("delimiter app"); err != nil || got != "del-imiter-app" { + t.Errorf("U+007F is ASCII and must stay exempt: Slugify = (%q, %v), want del-imiter-app", got, err) + } + // U+0080 is the first rune ABOVE ASCII. It is a control (Cc) — not space, + // punct or symbol — so it is content with nowhere to go, and refusing it is + // what the boundary says. + got, err := Slugify("ctrl€name app") + if err == nil { + t.Errorf("U+0080 is above ASCII and must be refused, got %q", got) + } + if len(LossyChars("ctrl€name")) != 1 { + t.Errorf("LossyChars must report U+0080: %q", LossyChars("ctrl€name")) + } +} + +// TestSlugifyLowersIntoAsciiIsADocumentedException — residual (b). Exactly two +// runes above ASCII lower INTO ASCII, and both derive rather than refuse. This +// is deliberate (it is the nicest transliteration available and costs nothing), +// so it is pinned as BEHAVIOUR rather than left as an accident of the boundary: +// a future "tighten the exemption to `r < utf8.RuneSelf` on the ORIGINAL rune" +// would silently start refusing İstanbul. +func TestSlugifyLowersIntoAsciiIsADocumentedException(t *testing.T) { + cases := map[string]string{ + "\u0130stanbul App": "istanbul-app", // U+0130 LATIN CAPITAL LETTER I WITH DOT ABOVE -> i + "Temp Kelvin": "temp-kelvin", // U+212A KELVIN SIGN -> k + } + for in, want := range cases { + got, err := Slugify(in) + if err != nil { + t.Errorf("Slugify(%q) errored, want the documented %q: %v", in, want, err) + continue + } + if got != want { + t.Errorf("Slugify(%q) = %q, want %q", in, got, want) + } + } + // The claim is that these are the ONLY two. If a future Go unicode table + // grows a third, the header's "exactly two" stops being true and this fails. + if n := len(runesThatLowerIntoASCII()); n != 2 { + t.Errorf("the header claims exactly 2 runes above ASCII lower into ASCII; found %d", n) + } +} + +// TestSlugifySymbolsAndEmojiStillFoldToAHyphen — residual (c), pinned as the +// deliberate behaviour it is rather than left undescribed. If someone decides to +// close it, this test is what says the change was intentional. +func TestSlugifySymbolsAndEmojiStillFoldToAHyphen(t *testing.T) { + cases := map[string]string{ + "Rocket \U0001F680 App": "rocket-app", + "Widget \u2764 Pro": "widget-pro", + } + for in, want := range cases { + got, err := Slugify(in) + if err != nil { + t.Errorf("Slugify(%q) errored, want the documented %q: %v", in, want, err) + continue + } + if got != want { + t.Errorf("Slugify(%q) = %q, want %q — symbols are a documented exception", in, got, want) + } + } +} + // TestSlugifyNonAsciiSeparatorsStaySeparators: above ASCII the classification is // by Unicode category, so a punctuation or symbol rune is still a separator with // no content of its own. Getting this wrong would refuse names that derive @@ -121,7 +407,7 @@ func TestSlugifyNonAsciiSeparatorsStaySeparators(t *testing.T) { "Widget — Pro": "widget-pro", // em dash (Pd) "Widget « Pro »": "widget-pro", // guillemets (Pi/Pf) "Widget © Pro": "widget-pro", // copyright sign (So) - "Widget Pro": "widget-pro", // no-break space (Zs) + "Widget Pro": "widget-pro", // no-break space (Zs) } for in, want := range cases { got, err := Slugify(in) @@ -159,32 +445,32 @@ func TestSlugifyExistingDeadEndsKeepTheirMessages(t *testing.T) { } } -// TestLossyRunesIsDeduplicatedAndOrdered — the message lists characters, so a +// TestLossyCharsIsDeduplicatedAndOrdered — the message lists characters, so a // name repeating one must not repeat it in the error. -func TestLossyRunesIsDeduplicatedAndOrdered(t *testing.T) { - got := LossyRunes("ÜÜber Ω Über") - want := []rune{'ü', 'ω'} +func TestLossyCharsIsDeduplicatedAndOrdered(t *testing.T) { + got := LossyChars("ÜÜber Ω Über") + want := []string{"Ü", "Ω"} if len(got) != len(want) { - t.Fatalf("LossyRunes = %q, want %q", string(got), string(want)) + t.Fatalf("LossyChars = %q, want %q", got, want) } for i := range want { if got[i] != want[i] { - t.Fatalf("LossyRunes = %q, want %q (first-appearance order)", string(got), string(want)) + t.Fatalf("LossyChars = %q, want %q (first-appearance order)", got, want) } } - if len(LossyRunes("My Cool Block")) != 0 { - t.Error("an ASCII name has no lossy runes") + if len(LossyChars("My Cool Block")) != 0 { + t.Error("an ASCII name has no lossy characters") } } // TestSlugifyRefusalBoundsTheCharacterList — a fully non-Latin name can carry -// dozens of distinct runes; the refusal names the first few and counts the rest -// rather than echoing the whole name back as a quoted list. +// dozens of distinct characters; the refusal names the first few and counts the +// rest rather than echoing the whole name back as a quoted list. func TestSlugifyRefusalBoundsTheCharacterList(t *testing.T) { long := "日本語アプリケーションのテスト" - lossy := LossyRunes(long) - if len(lossy) <= maxNamedRunes { - t.Fatalf("fixture is broken: %q has %d distinct lossy runes, need > %d", long, len(lossy), maxNamedRunes) + lossy := LossyChars(long) + if len(lossy) <= maxNamedChars { + t.Fatalf("fixture is broken: %q has %d distinct lossy characters, need > %d", long, len(lossy), maxNamedChars) } _, err := Slugify(long) if err == nil { @@ -192,17 +478,17 @@ func TestSlugifyRefusalBoundsTheCharacterList(t *testing.T) { } msg := err.Error() // The bounded prefix is named … - for _, r := range lossy[:maxNamedRunes] { - if !strings.Contains(msg, string(r)) { - t.Errorf("refusal should name %q: %s", string(r), msg) + for _, c := range lossy[:maxNamedChars] { + if !strings.Contains(msg, c) { + t.Errorf("refusal should name %q: %s", c, msg) } } // … and the remainder is COUNTED, not silently dropped. - if !strings.Contains(msg, fmt.Sprintf("(and %d more)", len(lossy)-maxNamedRunes)) { - t.Errorf("refusal should count the runes it did not spell out: %s", msg) + if !strings.Contains(msg, fmt.Sprintf("(and %d more)", len(lossy)-maxNamedChars)) { + t.Errorf("refusal should count the characters it did not spell out: %s", msg) } - // The quoted-list is bounded: exactly maxNamedRunes quoted single characters. - if got := strings.Count(msg, `", "`); got != maxNamedRunes-1 { - t.Errorf("expected %d separators in the quoted list, got %d: %s", maxNamedRunes-1, got, msg) + // The quoted-list is bounded: exactly maxNamedChars quoted characters. + if got := strings.Count(msg, `", "`); got != maxNamedChars-1 { + t.Errorf("expected %d separators in the quoted list, got %d: %s", maxNamedChars-1, got, msg) } } From 395cefcef9a0f9fdcb61af4c35a43543555c5548 Mon Sep 17 00:00:00 2001 From: ZacxDev Date: Fri, 7 Aug 2026 16:59:57 -0500 Subject: [PATCH 3/4] docs: record the blockId derivation decision and announce the --slug break MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- AGENTS.md | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++- README.md | 36 +++++++++++++++++- 2 files changed, 144 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5d180ed..8ec64b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -169,7 +169,9 @@ EXISTING app is missing the item-11 handshake (20 is the reachability repair to `field` every `--json` consumer groups on; and item 24 covers the ONE transport-vs-filesystem predicate now shared by the CLI-wide exit-code classifier (which every command's published exit code funnels through) and -`pkg/civitai`'s read-GET retry loop. +`pkg/civitai`'s read-GET retry loop; and item 25 covers blockId derivation — +the one identity this CLI mints that can NEVER be renamed, and the residuals the +refusal knowingly ships with. The durable fix for the mirroring is a server-side `civitai app validate` endpoint that calls the real `BlockManifestValidator` — until that exists, vendoring is on purpose. @@ -1865,6 +1867,112 @@ neither one's. full type resolution because `golang.org/x/tools/go/packages` would be a new dependency, which is an "ask first" below. +25. **The blockId derivation REFUSES rather than transliterates, and the + exemption that makes refusing safe is "LOWERCASES INTO ASCII" — never a + character allowlist.** `scaffold.Slugify` used to lowercase the name and + replace every run of non-`[a-z0-9]` with a hyphen, which silently DROPPED + content: `"Café App"` minted the blockId `caf-app` and `"ÜberApp Ω"` minted + `berapp` (measured), at exit 0, for an identity that **cannot be renamed + afterwards** — it is the hostname the app is served at and the argument every + later command takes. It now refuses and names the offending characters, with + `--slug` as the escape hatch (#259). + - 🔴 **THE EXEMPTION IS THE LOAD-BEARING PART, AND IT IS A PREDICATE ON THE + LOWERED RUNE, NOT A LIST.** `isLossyBase` asks `unicode.ToLower(r) < + utf8.RuneSelf` and, above that, whether the lowered rune is a + space/punct/symbol. Because lowercasing is what derivation does first, the + exemption re-decides NOTHING: every ASCII derivation this CLI has ever + produced is byte-identical, including the two dead ends whose existing + messages are good (`"123 Numbers"`, `"!!!"`). **Do not "improve" it into an + allowlist of permitted characters.** An allowlist is a second, hand- + maintained mirror of the slug alphabet that re-opens every one of those + derivations to a typo, and it buys nothing the boundary does not already + give. The `<` was a surviving mutant (only U+0080 changes hands, a C1 + control nobody types) and is now pinned from both sides precisely because + an unpinned boundary is a claim nothing holds. + - 🔴 **IT NARROWS #259; IT DOES NOT CLOSE IT — and the first write-up said + otherwise.** An audit measured three classes of input still producing the + exact `#259` shape after the refusal shipped. State the residuals with the + claim, or the next reader inherits a closure that was never delivered: + - **(a) INVALID UTF-8 — now CLOSED.** `app create $'caf\xe9 app'` derived + `caf-app` at rc 0, because `for _, r := range` yields U+FFFD per bad byte + and U+FFFD is a Symbol, i.e. the SEPARATOR branch. The written + `block.manifest.json` also held the raw `0xE9` and was not valid UTF-8, + and `validate.ManifestOnly` accepted it. `Slugify` now gates on + `utf8.ValidString` BEFORE anything ranges over the string, and + `runAppScaffold` refuses an invalid-UTF-8 DISPLAY name too — that second + guard is not redundant, because `--slug` bypasses derivation entirely and + the name still reaches the manifest verbatim. `internal/validate` still + has no UTF-8 check of its own; that gap is real and is not this item's. + - **(b) THE TWO RUNES ABOVE ASCII THAT LOWER INTO ASCII — a deliberate, + ENUMERATED exception.** All of Unicode was walked: exactly two exist, + `İ` U+0130 → `i` and `K` U+212A KELVIN SIGN → `k`. So `"İstanbul App"` + derives `istanbul-app` at rc 0. That is the nicest transliteration + available and it costs nothing, so it STAYS — but it is why any absolute + claim that every non-ASCII letter is refused is false, and why the + exemption is spelled "lowers into ASCII" rather than "is ASCII". + `TestSlugifyLowersIntoAsciiIsADocumentedException` re-walks Unicode and + fails if a future Go table grows a third. + - **(c) SYMBOLS, EMOJI AND NON-ASCII PUNCTUATION fold to a hyphen**, so + `"Rocket 🚀 App"` derives `rocket-app` at rc 0. Census over printable + non-ASCII runes: **8,580** take the separator branch, **140,321** the + refuse branch. This is the asymmetry working as designed for `—` and `»`, + and arguably not what an author means by an emoji — but an emoji has no + lossless ASCII form either, so refusing would trade a silent drop for a + dead end. **The product decision is made and the behaviour stays: + civitai/cli#272.** Do not re-argue it in a doc comment; point at the + issue. + - 🔴 **A COMBINING MARK IS REPORTED WITH ITS BASE, because the same VISIBLE + name arrives as two different byte sequences.** macOS paths and some paste + routes deliver NFD, so `"Café App"` arrives as `e` + U+0301 and the first + message read `"́" cannot appear in a blockId` — an accent rendered over + nothing, while the NFC form of the same name said `"é"`. `LossyChars` + clusters base+marks, so both forms now report `"é"`; a mark with no base at + all is shown on a dotted circle (U+25CC). This changed only the RENDERING — + a mark is non-ASCII and is neither space, punct nor symbol, so it was + already refused, and the set of refused names is unchanged. + - 🔴 **CLASSIFY ON THE LOWERED RUNE, REPORT THE ORIGINAL. The first version + did both on the lowered one and quoted characters the user never typed.** + Measured: `"ẞE App"` reported `"ß"` — a rune ABSENT from the input, because + U+1E9E lowers to U+00DF — and `"ＡＢＣ"` reported `"ａ", "ｂ", "ｃ"`. Someone + searching their own name for the quoted character finds nothing, which is + worse than a generic message. The two axes are independent and the code + keeps them apart; `TestSlugifyReportsTheCharacterTheAuthorTyped` pins it + with rows whose lowered form is provably not in the input. + - 🔴 **`--slug` SUPPRESSES THE NAME FIELD, NOT THE PROMPT — and getting that + wrong is a silent capability loss, not a cosmetic one.** `--slug` was first + 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 **zero** failures, because nothing covered the + suppression at all. Whenever a flag is made to skip a prompt, enumerate + what ELSE that prompt collects. + - 🔴 **THE ECHOED URL IS FUTURE TENSE, and that is not style.** The scaffold + echoes the blockId always (it was a DEAD PARAMETER of + `printScaffoldResult` — no line of output named the app's permanent id, and + with `--dir` the only copy was inside `block.manifest.json`). But the first + version printed the bare `https://.civit.ai/` as the "permanent + public id" at scaffold time — a URL **guaranteed to 404 at that exact + moment**, since the subdomain is only programmed on approval + deploy (the + README says so, and `app status` already says "Not live yet — … only serves + after the app is approved and deployed"). That is the same false-promise + class as the "validates clean" claim two lines up in the SAME output block. + Keep the two surfaces on the same words. + - 🔴 **A `mustNotProduce` ROW THAT CANNOT RUN IS NOT COVERAGE, AND A GREEN + SUITE CANNOT TELL YOU.** The refusal test claimed 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 while a positive control reddened 1 — the harness + could go red; the assertion simply never executed. 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. **When a test's headline + claim is "it pins the exact old value", check that the line can be + reached.** + **When you change a validation rule, keep all four vendored mirrors in sync with the server — `schema/`, the ported Go checks in `internal/validate/` (including the slot registry), the Vite dotenv resolution behind the dev-tunnel parent-origin diff --git a/README.md b/README.md index 3108320..f170379 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ README. For the end-to-end walkthrough, see | `civitai login [--scopes ] [--token []] [--no-browser]` | Browser OAuth device login by default (stores auto-refreshing tokens). The default scope set grants identity + Apps submit + dev-tunnel and **not** Buzz-spend; `--scopes generate` additively grants generation + Buzz **spend** (needed by `civitai generate` and money-path `dev:live`). `--token ` stores a personal API key instead (not combinable with `--scopes`). `--token` with **no value** prints where to create a personal key (`civitai.com/user/account`) and how to re-run — handy when you know you want a personal key but haven't minted one yet. Config at `~/.config/civitai/config.yaml`, 0600. Also reads `CIVITAI_TOKEN`. | | `civitai whoami [--scopes] [--json]` | Verify the stored token; print the authenticated user **and a Capabilities section** — credential type (**OAuth login** vs **personal API key**), **Read Buzz balance**, and **Spend Buzz** — decoded from the token's scope, so a money-path dead end (a default OAuth login can't spend) is visible before `dev:live` — and when it can't, the output names the fix for that credential (`login --scopes generate` for an OAuth login, a full-scope key otherwise). `--scopes` also lists every granted scope; `--json` emits the user + `credentialType`/`canReadBalance`/`canSpend`/`scopes` (scriptable). | | `civitai buzz [--json]` | Show your spendable Buzz balance (**blue / green / yellow**, plus a **total**). Needs the BuzzRead scope — a full-scope personal API key or `civitai login --scopes generate`; a **default** OAuth login token can't read it, and gets a clear message naming both fixes. `--json` emits `{blue,green,yellow,total}` (scriptable — handy for before/after diffing a `dev:live` spend). | -| `civitai app create [name] [dir] [--template static\|page-vite\|page-money] [--dir ] [--name ]` | **The friendly happy path.** Scaffold a ready-to-build App, defaulting to the batteries-included `page-money` SDK template (default dir `./`). | +| `civitai app create [name] [dir] [--template static\|page-vite\|page-money] [--dir ] [--name ] [--slug ]` | **The friendly happy path.** Scaffold a ready-to-build App, defaulting to the batteries-included `page-money` SDK template (default dir `./`). `--slug` sets the **blockId** explicitly instead of deriving it from the name — required for a name derivation refuses (see [The blockId](#the-blockid)). | | `civitai app init [name] [dir] [...]` | Same scaffolder as `create` with a no-build `static` default (back-compat alias). | | `civitai app dev-token [--env] [--spend] [--budget ]` | **Mint a short-lived (~4h) dev block token for `npm run dev:live`** — calls the invite-gated mint route with your stored credential, reading scopes from your local `block.manifest.json` (so it works on an unsubmitted slug). `--spend` explicitly REQUESTS `ai:write:budgeted` (real Buzz); omit it and that scope is **filtered out** of the request — the CLI never asks for budgeted spend implicitly, even when your manifest declares it (the scaffolded money app does, so a live run that used to generate now needs `--spend`). Prints the token (`--env` prints `VITE_LIVE_BLOCK_TOKEN=`, paste-ready); warns at mint time if the token is read-only (can't spend). See [Local dev loop](#local-dev-loop-harness-mock-vs-live). | | `civitai app dev-tunnel [blockId] [--port] [--tunnel-endpoint] [--idle-timeout]` | **(Pre-GA / invite-gated)** Preview your **local** dev server inside the **real** Civitai host at `civitai.com/apps/dev/` — a prod-fidelity inner-dev-loop. Mints an **ephemeral in-memory ssh keypair**, opens a reverse tunnel from your dev port (start `npm run dev:tunnel` first) to the Civitai tunnel endpoint, prints the URL to open, and tears everything down on Ctrl-C or an idle timeout. Before minting it also **pre-flights whether the host can actually embed your dev server** — the host iframes it sandboxed (opaque `null` origin), so a dev server missing `Access-Control-Allow-Origin: *`, missing the `.civit.ai` entry in `allowedHosts`, or sending a framing header that excludes `civitai.com` loads as a blank iframe with no error anywhere. Those are printed as warnings (never fatal) **twice** — once the moment the checks run, so you still get them if you Ctrl-C the DNS wait, and again just above the URL, with the `vite.config.ts` fix. Apps scaffolded by `civitai app init --template page-money` already satisfy all of it. The tunnel endpoint (`sish.civitai.com:2224`) is **live**; access is gated behind an Apps-author invite **and** a server kill-switch flag, so if you are not enrolled the mint reports **"not available"** — ask to be added to the cohort. Publishing the tunnel host through external-dns + Cloudflare usually takes 1–3 min (occasionally longer); the command waits for it and prints the elapsed time. | @@ -232,6 +232,40 @@ README. For the end-to-end walkthrough, see Run `civitai help`, `civitai app --help`, or `civitai --help` for the full details and examples. +### The blockId + +The **blockId** is your app's permanent public identity: the hostname it will be +served at once approved (`https://.civit.ai/`) and the argument every +later command takes (`app status`, `app metrics`, `app listing`, `app dev-token`, +`app dev-tunnel`). **It cannot be renamed afterwards.** `app create` / `app init` +echo the one they chose, so it is on screen before you commit anything. + +By default it is derived from the name: `"My Cool Block"` → `my-cool-block`. +Pass **`--slug `** to choose it yourself — it bypasses derivation entirely, +so name, blockId and directory are three fully independent axes. + +> **Breaking change.** Derivation used to lowercase the name and replace every +> run of non-`[a-z0-9]` with a hyphen, which silently **dropped characters**: +> `civitai app create "Café App"` minted the blockId **`caf-app`**, and +> `"ÜberApp"` minted **`berapp`** — a different permanent public id than the +> author typed, with no warning and exit 0. Derivation now **refuses** and names +> the offending characters, exiting **2** and asking for `--slug`. **If you have +> a script passing a non-ASCII name, it must now pass `--slug `.** The old +> output was wrong, so the break is the point — but it is a break. + +What derivation refuses is **letters, digits and marks** above ASCII that the +slug alphabet cannot carry. Three things still derive rather than refuse, and +they are deliberate: + +| input | blockId | why | +| --- | --- | --- | +| `"Rocket 🚀 App"` | `rocket-app` | Symbols, emoji and non-ASCII punctuation are **separators** — that is what makes `"Widget — Pro"` → `widget-pro` right. An emoji has no lossless ASCII form either, so refusing would only trade a silent drop for a dead end. Tracked as [#272](https://github.com/civitai/cli/issues/272). | +| `"İstanbul App"` | `istanbul-app` | Exactly two runes above ASCII lowercase **into** ASCII — `İ` (U+0130) and `K` (U+212A). Lowercasing is what decides whether a character survives, so these transliterate for free. | +| `"My Cool___Block"` | `my-cool-block` | ASCII is exempt by construction — every derivation that worked before still produces the byte-identical blockId. | + +A name that is **not valid UTF-8** is refused outright (it used to lose the bad +bytes from the blockId *and* write them into `block.manifest.json`). + ### Templates - **`static`** — a no-build page app (`index.html` + a tiny `app.js`, From cb0cd51cbe84c0309c85c1342457bc264bee253d Mon Sep 17 00:00:00 2001 From: ZacxDev Date: Fri, 7 Aug 2026 17:20:01 -0500 Subject: [PATCH 4/4] fix(test): spell the two control-character fixtures as Go escapes, not raw bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- internal/scaffold/slug_lossy_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/scaffold/slug_lossy_test.go b/internal/scaffold/slug_lossy_test.go index 0864929..e62b93c 100644 --- a/internal/scaffold/slug_lossy_test.go +++ b/internal/scaffold/slug_lossy_test.go @@ -335,18 +335,18 @@ func TestSlugifyAsciiDerivationIsByteIdentical(t *testing.T) { func TestSlugifyAsciiExemptionBoundary(t *testing.T) { // U+007F DEL is the last ASCII rune: exempt, folded to a hyphen like any // other non-slug ASCII character. - if got, err := Slugify("delimiter app"); err != nil || got != "del-imiter-app" { + if got, err := Slugify("del\x7fimiter app"); err != nil || got != "del-imiter-app" { t.Errorf("U+007F is ASCII and must stay exempt: Slugify = (%q, %v), want del-imiter-app", got, err) } // U+0080 is the first rune ABOVE ASCII. It is a control (Cc) — not space, // punct or symbol — so it is content with nowhere to go, and refusing it is // what the boundary says. - got, err := Slugify("ctrl€name app") + got, err := Slugify("ctrl\u0080name app") if err == nil { t.Errorf("U+0080 is above ASCII and must be refused, got %q", got) } - if len(LossyChars("ctrl€name")) != 1 { - t.Errorf("LossyChars must report U+0080: %q", LossyChars("ctrl€name")) + if len(LossyChars("ctrl\u0080name")) != 1 { + t.Errorf("LossyChars must report U+0080: %q", LossyChars("ctrl\u0080name")) } }