Give the scaffolder's foreign-file edits one whitespace rule

.changeset/append-never-insert.md

@@ -0,0 +1,30 @@
+---
+"create-eigen-game": patch
+---
+
+Give the three edits into files the scaffolder does not own one shared whitespace
+rule, and write down the doctrine they follow.
+
+`enableAndroidCoreLibraryDesugaring`, `enableAndroidReleaseSigning` and
+`configureLauncherIconsAndSplash` each append a block to a file `flutter create`
+produced. All three normalised newlines themselves, with two different answers,
+so the desugaring block got a blank line before it and the other two did not: in
+a generated project the two Gradle blocks ended up flush against each other.
+`appendBlock` now owns that and nothing else, one blank line before and one
+newline after, with the block constants carrying neither, and the scaffold test
+pins the separation rather than only the contents.
+
+No behavioural change beyond the whitespace of generated `build.gradle.kts` and
+`pubspec.yaml`. A comment claiming the release-signing block still prepends a
+Kotlin `import` was also stale, describing an edit removed when it broke under
+AGP 9, and said the opposite of the rule it sat next to.
+
+MAINTAINERS.md gains **Editing files the scaffolder does not own**: append never
+insert, why every block is recognised by a content probe on its own payload, and
+why FlutterFire's `// START:`/`// END:` marker comments are deliberately not
+copied. Those markers are attribution rather than machinery, never read back by
+`flutterfire_cli` itself, and an append needs no anchor; the one condition that
+would earn a marker pair is a block whose content must be found and replaced on a
+later run, which is an upgrade command's problem and not one that exists yet. It
+also records that configuration values are assigned into declared slots instead,
+and the JSON-versus-JSONC rule that governs how.

MAINTAINERS.md

@@ -382,6 +382,111 @@ line-crossing release.
 defeat the purpose: a real resolution against pub.dev is the only thing that
 makes the pin mean anything.
 
+### Editing files the scaffolder does not own
+
+Most of what `create-eigen-game` writes, it owns: four template trees copied
+whole and token-substituted by `renderTree`. Three edits are different. They
+modify files `flutter create` produced, in a directory `flutterfire configure`
+will edit next, and they have one rule.
+
+> **Append, never insert.**
+
+| Function | File | Identified by |
+|---|---|---|
+| `enableAndroidCoreLibraryDesugaring` | `android/app/build.gradle.kts` | `isCoreLibraryDesugaringEnabled` |
+| `enableAndroidReleaseSigning` | same file | `releaseKeyProperties` |
+| `configureLauncherIconsAndSplash` | `app/pubspec.yaml` | `flutter_launcher_icons:` |
+
+An append cannot compete for position with whatever Flutter or FlutterFire
+writes above it. Both Android edits are second top-level `android { }` blocks,
+which is legal because Gradle merges repeated extension-configuration blocks in
+one file; the pubspec edit is two top-level YAML keys. A second top-level
+`plugins { }` block is *not* legal, one per script, which is why the Crashlytics
+plugin registration is left to `flutterfire configure` and not attempted here.
+
+The rule is not a preference. The one edit that ever needed a position,
+prepending `import java.util.Properties` because Kotlin requires imports before
+every other top-level declaration, was broken by an AGP upgrade, and was fixed by
+removing the need for it rather than by chasing the new layout. Parsing
+`key.properties` with the Kotlin stdlib needs no import. Reach for a positional
+edit and you have signed up for that maintenance again.
+
+Each block is recognised by **a content probe on its own payload**, chosen to be
+a string only that block contains. `includes("signingConfigs")` would not do:
+Flutter's own template already contains it, via
+`signingConfig = signingConfigs.getByName("debug")`, so the guard would
+short-circuit forever and nothing would ever be appended. Two of the three also
+assert on the *shape* of what they found and throw: an Android application module
+must be there, and a half-written desugaring configuration is an error rather
+than something to append alongside.
+
+`appendBlock` owns the whitespace, and only the whitespace: one blank line before
+the block, one newline after, block constants carrying neither. It is one
+function because it was three, with two different answers, so the second Gradle
+block landed flush against the first in every generated project.
+
+#### Why FlutterFire's marker comments are not copied
+
+`flutterfire configure` brackets its Gradle insertions:
+
+```kotlin
+// START: FlutterFire Configuration
+id("com.google.gms.google-services") version("4.4.4") apply false
+// END: FlutterFire Configuration
+```
+
+**Those markers are attribution, not machinery.** In flutterfire_cli 1.4.1 the
+two constants appear in ten places and every one is string construction; nothing
+ever reads them back. Its idempotency comes from a separate content probe,
+`contains(_googleServicesPluginClassPath)`, which is the same mechanism the table
+above uses. FlutterFire needs a *regex anchor* because it must inject into the
+single permitted `plugins { }` block, and the comment rides along; an append
+needs no anchor, so a marker would add text without adding capability.
+
+Copying them would also be actively misleading in one respect: FlutterFire uses
+one marker text for all its insertions, so it could not replace them selectively
+even if it read them.
+
+**A marker pair earns its place under exactly one condition: the block's content
+changes between runs and the old copy must be found and replaced.** That is what
+a content probe cannot do. `includes("releaseKeyProperties")` answers "is mine
+here?" but never "where does mine end?", so it can only skip, never update, and
+`desugar_jdk_libs:2.1.4` is frozen into every project already generated.
+
+That condition arrives with an upgrade command, not before. When it does, add
+markers as a genuinely parsed region: **one pair per block**, versioned in the
+marker text so a future writer knows what it is replacing, and guarded on
+finding *exactly one* properly nested pair. Anything else, and they are
+decoration.
+
+Never put markers in `templates/`. Those files are wholly owned and rendered
+whole, and a marker there would promise a tool that revisits them.
+
+#### Configuration values are assigned, not appended
+
+Engine configuration is a separate case from the three edits above, and does not
+use them. `FIREBASE_PROJECT_ID`, `WEB_APP_ORIGIN`, `GOOGLE_WEB_CLIENT_ID`,
+`API_BASE_URL` and the VAPID key all have declared, empty slots in
+`templates/worker/wrangler.jsonc` and `templates/app-overlay/app-config.json`.
+Filling a slot beats appending a block: the key is visible in the template before
+anything runs, the wrangler schema knows about it, and an absent or ambiguous
+slot is a refusal rather than a guess.
+
+`configure_firebase` in eigen-flutter does that filling, at scaffold time and on
+every later re-run, so there is one implementation rather than two that drift.
+Its rule, in `lib/src/codegen/firebase_link.dart`:
+
+| File | Method | Why |
+|---|---|---|
+| `app-config.json` | decode, merge, re-encode | Plain JSON; nothing to preserve |
+| `wrangler.jsonc` | rewrite the one assignment in place | Comments. Re-encoding would delete every one, and Cloudflare recommends JSONC for new projects |
+
+The surgical path's safety is the **match count, not the pattern**: a key found
+anywhere other than exactly once, commented out, duplicated, or nested under some
+other object, means this is not the file it knows how to edit, so nothing is
+written and the caller reports it. Guessing which of two matches was meant is the
+only outcome worth avoiding.
+
 ## Downstream documentation
 
 `eigen-web` commits generated copies of:

packages/create-eigen-game/src/index.ts

@@ -373,6 +373,32 @@ function packageCommand(manager: PackageManager, operation: "install" | "contrac
   return [manager, ["run", operation]];
 }
 
+/**
+ * Appends `block` to a file this scaffolder does not own, as its own paragraph.
+ *
+ * **Append, never insert.** That is the rule the three callers below follow, and
+ * it is what keeps them from competing with `flutter create` and `flutterfire
+ * configure` for position in the same files. A pure append cannot collide with
+ * whatever those tools write above it; the one edit here that ever needed a
+ * position, prepending a Kotlin `import`, was broken by an AGP upgrade and has
+ * since been rewritten away. See MAINTAINERS.md for the whole doctrine, including
+ * why FlutterFire's `// START:`/`// END:` marker comments are deliberately not
+ * copied.
+ *
+ * `contents` is passed in rather than read here because every caller has already
+ * read the file to decide whether its block is present, and two of them assert
+ * on the shape of what they found. This owns the whitespace and nothing else.
+ *
+ * That is worth one function because it was previously written three times with
+ * two different answers, so the desugaring block got a blank line before it and
+ * the other two did not, and the two Gradle blocks ended up flush against each
+ * other in every generated project. Separation belongs to the append, so the
+ * block constants carry no leading or trailing newline of their own.
+ */
+function appendBlock(path: string, contents: string, block: string): void {
+  appendFileSync(path, `${contents.endsWith("\n") ? "" : "\n"}\n${block}\n`);
+}
+
 const androidDesugaring = `// flutter_local_notifications requires desugaring in the application module.
 // A library plugin cannot enable this compiler setting transitively.
 android {
@@ -397,7 +423,7 @@ function enableAndroidCoreLibraryDesugaring(appRoot: string): void {
   if (settingExists || dependencyExists) {
     throw new Error(`Flutter created an incomplete Android core library desugaring configuration: ${gradlePath}`);
   }
-  appendFileSync(gradlePath, `${gradle.endsWith("\n") ? "\n" : "\n\n"}${androidDesugaring}\n`);
+  appendBlock(gradlePath, gradle, androidDesugaring);
 }
 
 // A second top-level `android { }` block is valid Gradle Kotlin DSL, since Gradle
@@ -407,10 +433,12 @@ function enableAndroidCoreLibraryDesugaring(appRoot: string): void {
 // script), which is why the Crashlytics Gradle plugin registration,
 // `flutterfire configure`'s own territory, is deliberately left alone here.
 //
-// `Properties` needs a real `import`, prepended separately below. A
-// fully-qualified `java.util.Properties()` reference here was tried first,
-// to avoid needing to touch the top of the file at all, but fails to
-// resolve under AGP 9's Kotlin DSL script compilation
+// Parsed with the Kotlin stdlib rather than `java.util.Properties`, which is
+// what keeps this appendable. `Properties` would need a real `import`, and
+// Kotlin requires imports before every other top-level declaration, so using it
+// meant also reaching into the very start of a file this scaffolder does not
+// own. A fully-qualified `java.util.Properties()` reference was tried to avoid
+// that, and does not resolve under AGP 9's Kotlin DSL script compilation
 // (`flutter create`'s current default): "Unresolved reference 'util'".
 // Confirmed by actually building a scaffolded project, not just reading it.
 const androidReleaseSigning = `val releaseKeyProperties: Map<String, String> =
@@ -455,13 +483,9 @@ function enableAndroidReleaseSigning(appRoot: string): void {
   // so that check would always short-circuit and never append anything.
   if (gradle.includes("releaseKeyProperties")) return;
   // A pure append. This previously also PREPENDED `import java.util.Properties`,
-  // because Kotlin requires imports before every other top-level declaration,
-  // which meant reaching into the very start of a file this scaffolder does
-  // not own, the most position-dependent edit here and the one already broken
-  // once by an AGP upgrade. Parsing `key.properties` with the Kotlin stdlib
-  // needs no import, so the edit is now append-only and cannot collide with
-  // whatever Flutter or `flutterfire configure` writes above it.
-  appendFileSync(gradlePath, `${gradle.endsWith("\n") ? "" : "\n"}${androidReleaseSigning}\n`);
+  // the most position-dependent edit here and the one already broken once by an
+  // AGP upgrade; see the block above for what replaced it.
+  appendBlock(gradlePath, gradle, androidReleaseSigning);
 }
 
 // `flutter_launcher_icons`/`flutter_native_splash` read these as plain
@@ -475,8 +499,7 @@ function enableAndroidReleaseSigning(appRoot: string): void {
 // reversed, light-on-dark variant, so an ink background is what makes it
 // legible. Rebranding a game means replacing the four PNGs *and* these
 // colours together. See https://eigeninteractive.com/docs/ship-it/branding.
-const launcherIconsAndSplashConfig = `
-flutter_launcher_icons:
+const launcherIconsAndSplashConfig = `flutter_launcher_icons:
   android: true
   web:
     generate: true
@@ -499,14 +522,13 @@ flutter_native_splash:
     image_dark: assets/icon/splash_dark.png
     icon_background_color: "#F4F1EA"
     icon_background_color_dark: "#1B1E24"
-  web: true
-`;
+  web: true`;
 
 function configureLauncherIconsAndSplash(appRoot: string): void {
   const pubspecPath = resolve(appRoot, "pubspec.yaml");
   const pubspec = readFileSync(pubspecPath, "utf8");
   if (pubspec.includes("flutter_launcher_icons:")) return;
-  appendFileSync(pubspecPath, `${pubspec.endsWith("\n") ? "" : "\n"}${launcherIconsAndSplashConfig}`);
+  appendBlock(pubspecPath, pubspec, launcherIconsAndSplashConfig);
 }
 
 export interface AddContinuousIntegrationOptions {

packages/create-eigen-game/test/scaffold.spec.ts

@@ -224,12 +224,27 @@ describe("scaffoldGame", () => {
     expect(androidGradle).toContain('coreLibraryDesugaring("com.android.tools:desugar_jdk_libs:2.1.4")');
     expect(androidGradle).toContain("signingConfigs");
     expect(androidGradle).toContain("proguardFiles(");
+    // Each appended block is separated from what it follows by exactly one blank
+    // line, including from the other block this scaffolder appends to the same
+    // file. Newline normalisation used to be written per call site with two
+    // different answers, so the second Gradle block landed flush against the
+    // first in every generated project. This is generated code someone reads.
+    expect(androidGradle).toContain("}\n\n// flutter_local_notifications requires desugaring");
+    expect(androidGradle).toContain("}\n\nval releaseKeyProperties");
+    expect(androidGradle).not.toMatch(/\n\n\n/);
+    expect(androidGradle.endsWith("}\n")).toBe(true);
 
     expect(readFileSync(resolve(root, "app/fastlane/Appfile"), "utf8")).toContain('package_name("games.example.chess")');
 
     const appPubspec = readFileSync(resolve(root, "app/pubspec.yaml"), "utf8");
     expect(appPubspec).toContain("flutter_launcher_icons:");
     expect(appPubspec).toContain("flutter_native_splash:");
+    // Same separation rule as the Gradle appends above, and here it is load
+    // bearing rather than cosmetic: these are top-level YAML keys, so a block
+    // that landed without its own line break would be read as part of whatever
+    // `flutter create` wrote last.
+    expect(appPubspec).toContain("sdk: flutter\n\nflutter_launcher_icons:");
+    expect(appPubspec.endsWith("  web: true\n")).toBe(true);
     // `scaffoldGame` only creates `--platforms android,web`, so `ios`/`macos`
     // keys would make `flutter_launcher_icons` error on a platform directory
     // that doesn't exist.