Fix-Runde 1: Vorabpruefung vor dem Unique-Index, zwei Praezisierungen, ein fehlender Test

Migration: die Reihenfolge in up() stellte den unwiderruflichsten Schritt
(app_settings loeschen, dns_name-Spalte weg) VOR den einzigen Schritt, der an
vorhandenen Daten scheitern kann (unique('name') - hosts.name trug noch nie
einen eindeutigen Index). Schlug der auf MariaDB fehl, war der Schaden nicht
mehr rueckgaengig zu machen: DDL committet dort implizit, die Migration gilt
aber mangels Eintrag in der Migrationstabelle als nicht gelaufen, und ein
zweiter up()-Versuch stirbt an der bereits fehlenden dns_name-Spalte. Jetzt
steht eine reine Vorabpruefung ganz am Anfang, die simuliert, was die
Uebertragung schreiben wuerde, und mit einer RuntimeException abbricht, bevor
irgendetwas angefasst ist; das Loeschen der app_settings-Zeilen steht jetzt
hinter dem Unique-Index, nicht davor. Gegen echtes MariaDB geprueft,
einschliesslich eines Laufs mit zwei absichtlich kollidierenden Hostnamen.

HostStepsTest: Titel und ein Kommentar praezisiert - der Schritt vergibt den
Namen nicht mehr, er veroeffentlicht ihn nur noch.

HostNamingTest: ungenutzten Import entfernt (Pint), Rueckfall-Test fuer
HostName::label() bei einem Code ohne gueltige Zeichen ergaenzt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

database/migrations/2026_08_04_090000_clupilot_vergibt_die_hostnamen.php

@@ -28,6 +28,43 @@ return new class extends Migration
 {
     public function up(): void
     {
+        // Vorabprüfung, bevor irgendetwas angefasst wird — auch vor der
+        // Spalte weiter unten, die selbst schon DDL ist. `hosts.name` trug
+        // noch nie einen eindeutigen Index (siehe create_hosts_table): ein
+        // Host, der die DNS-Registrierung nie erreicht hat, behält seinen
+        // getippten Namen, und ein zweiter Anlauf nach einem gescheiterten
+        // Anlegen ist genau die Lage, in der derselbe Name zweimal getippt
+        // wird. Bricht `unique('name')` weiter unten deswegen ab, ist es zu
+        // spät: MariaDB committet DDL sofort und implizit, egal ob die
+        // Migration insgesamt als gelaufen gilt. Ohne Eintrag in der
+        // Migrationstabelle ruft niemand `down()` auf, und ein zweiter
+        // `up()`-Versuch stirbt sofort an der Spalte, die im ersten
+        // (fehlgeschlagenen) Anlauf schon angelegt wurde — Rettung dann nur
+        // noch von Hand per SQL.
+        //
+        // Deshalb wird hier SIMULIERT, was die Übertragung gleich schreiben
+        // würde (`dns_name`, wo gesetzt, sonst der unveränderte `name`),
+        // statt real zu schreiben und erst danach nachzusehen. Das deckt
+        // beide Quellen von Doppelten ab — zwei Hosts, die schon denselben
+        // getippten Namen tragen, UND ein getippter Name, der zufällig einem
+        // fremden `dns_name` gleicht — und lässt die Datenbank unangetastet,
+        // wenn es einen Treffer gibt: eine reine SELECT-Prüfung vor der
+        // ersten Schreiboperation, beliebig oft wiederholbar.
+        $duplicates = DB::table('hosts')
+            ->selectRaw("(CASE WHEN dns_name IS NOT NULL AND dns_name <> '' THEN dns_name ELSE name END) as future_name")
+            ->pluck('future_name')
+            ->countBy()
+            ->filter(fn (int $count) => $count > 1)
+            ->keys();
+
+        if ($duplicates->isNotEmpty()) {
+            throw new RuntimeException(
+                'Mehrere Hosts würden nach dieser Migration denselben Namen tragen: '
+                .$duplicates->implode(', ').'. Nichts wurde verändert — '
+                .'die betroffenen Hosts von Hand umbenennen oder entfernen und die Migration erneut ausführen.'
+            );
+        }
+
         Schema::table('datacenters', function (Blueprint $table) {
             $table->unsignedInteger('next_host_number')->default(1)->after('code');
         });
@@ -54,7 +91,10 @@ return new class extends Migration
             // Und was der alte Zähler schon ausgegeben HATTE. Ohne diesen Wert
             // ginge die Zusage „niemals wiederverwendet" beim Umzug verloren:
             // ein Host, der angelegt und wieder entfernt wurde, steht in keiner
-            // Zeile mehr, aber sein Name steht noch in den Protokollen.
+            // Zeile mehr, aber sein Name steht noch in den Protokollen. Die
+            // Zeile steht hier noch, weil `app_settings` erst ganz am Ende
+            // geleert wird (siehe dort) — an dieser Stelle ist sie also
+            // garantiert noch da.
             $carried = (int) (json_decode(
                 (string) DB::table('app_settings')->where('key', 'dns.sequence.'.$label)->value('value'),
                 true,
@@ -64,10 +104,6 @@ return new class extends Migration
                 ->update(['next_host_number' => max($inUse, $carried) + 1]);
         }
 
-        // Der alte Zähler geht mit. Eine tote Einstellung, die noch wie eine
-        // Quelle aussieht, ist genau das Problem, das diese Migration behebt.
-        DB::table('app_settings')->where('key', 'like', 'dns.sequence.%')->delete();
-
         // Getrennte Aufrufe: Index löschen, Spalte löschen und Index anlegen in
         // einem Blueprint bringt SQLite (Testlauf) durcheinander.
         Schema::table('hosts', function (Blueprint $table) {
@@ -78,10 +114,21 @@ return new class extends Migration
             $table->dropColumn('dns_name');
         });
 
-        // Ein Riegel, kein Ersatz für den Zähler.
+        // Der Riegel — dank der Vorabprüfung oben ohne Überraschung, aber
+        // trotzdem der riskanteste Schritt dieser Migration: der einzige, der
+        // an vorhandenen DATEN scheitern kann statt nur am Schema. Alles, was
+        // danach noch aussteht (die Zeile unten), ist reine Aufräumarbeit an
+        // etwas, das nichts mehr referenziert — sie steht deshalb ZULETZT.
         Schema::table('hosts', function (Blueprint $table) {
             $table->unique('name');
         });
+
+        // Der alte Zähler geht mit. Eine tote Einstellung, die noch wie eine
+        // Quelle aussieht, ist genau das Problem, das diese Migration behebt.
+        // Ganz am Ende, NACH dem Unique-Index: das ist der letzte wirklich
+        // zerstörerische Schritt hier, und er gehört hinter jeden Schritt, der
+        // noch scheitern könnte — nicht davor.
+        DB::table('app_settings')->where('key', 'like', 'dns.sequence.%')->delete();
     }
 
     public function down(): void

tests/Feature/Admin/HostNamingTest.php

@@ -4,8 +4,9 @@ use App\Actions\StartHostOnboarding;
 use App\Livewire\Admin\HostCreate;
 use App\Models\Datacenter;
 use App\Models\Host;
-use App\Provisioning\Jobs\PurgeHost;
 use App\Support\HostName;
+use App\Support\HostTakeoverCommand;
+use Illuminate\Database\QueryException;
 use Illuminate\Support\Facades\Queue;
 use Livewire\Livewire;
 
@@ -106,7 +107,7 @@ it('lässt zwei Hosts nicht denselben Namen tragen', function () {
     onboard();
 
     expect(fn () => Host::factory()->create(['name' => 'fsn-01']))
-        ->toThrow(Illuminate\Database\QueryException::class);
+        ->toThrow(QueryException::class);
 });
 
 it('gibt zwei ähnlich geschriebenen Rechenzentrums-Codes nicht denselben Namen', function () {
@@ -120,6 +121,10 @@ it('gibt zwei ähnlich geschriebenen Rechenzentrums-Codes nicht denselben Namen'
         ->and(onboard('eu-west')->name)->toBe('eu-west-02');
 });
 
+it('fällt auf eine gültige Bezeichnung zurück, wenn der Code keine hergibt', function () {
+    expect(HostName::label('---'))->toBe('node');
+});
+
 it('zählt ab hundert ohne Sonderfall weiter', function () {
     Datacenter::query()->where('code', 'fsn')->update(['next_host_number' => 100]);
 
@@ -132,6 +137,6 @@ it('nennt den Host im DNS so, wie die Konsole ihn nennt', function () {
     $host = onboard();
 
     expect(HostName::fqdn($host->name))->toBe('fsn-01.node.clupilot.com')
-        ->and(App\Support\HostTakeoverCommand::fqdnFor($host))
+        ->and(HostTakeoverCommand::fqdnFor($host))
         ->toBe(HostName::fqdn($host->name));
 });

tests/Feature/Provisioning/HostStepsTest.php

@@ -1326,7 +1326,7 @@ it('marks the host active on completion', function () {
         ->and($host->fresh()->status)->toBe('active');
 });
 
-it('gives the host a name that points at the tunnel, not at its public address', function () {
+it('publishes the name it was given, and points it at the tunnel, not at the public address', function () {
     // The zone is pinned here rather than read from the machine's own .env. It
     // used to assert the literal clupilot.com, which was true until somebody
     // set CLUPILOT_DNS_ZONE to clupilot.cloud on their installation — and then
@@ -1347,6 +1347,13 @@ it('gives the host a name that points at the tunnel, not at its public address',
     $result = (new RegisterHostDns($s['hostDns']))->execute($run);
 
     expect($result->type)->toBe('advance')
+        // Kein Beweis für den Schritt selbst — der Name steht schon vor
+        // dem Aufruf fest, also bliebe diese Zeile auch grün, wenn der
+        // Schritt ihn einfach nur unangetastet ließe. Sie ist ein
+        // Stolperdraht: bekäme RegisterHostDns wieder ein `$host->update([
+        // 'name' => ...])` verpasst (wie früher `dns_name`), müsste ein
+        // ANDERER Wert hier auffallen, sobald jemand den Namen tatsächlich
+        // ändert.
         ->and($host->fresh()->name)->toBe('fsn-01');
 
     // The management address: publishing a Proxmox host's public IP would hand