What review found in the port machinery, fixed

Three faults, one file split. All from reading, all verified to bite.

Unassign now releases the module's ports. ReleasePorts existed, said
"for when it is unassigned" in its own comment, and was called by
nothing — so a fixed port stayed claimed in the name of a module that
was gone, and the next module needing it was refused by a ghost.
Kept-once-chosen is a promise about a module that is still here.

MachineSide reads addressed mappings. "127.0.0.1:8080:80" was split at
the first colon, "127.0.0.1" failed to parse as a port, and the mapping
was silently skipped — putting the filter back on the declared port,
the exact fault the function was written to end. The machine side is
the second-from-last part, which is the reading the host already
applies, and the substrate bundle writes that shape today.

An allocation race answers in the mesh's words. Two concurrent picks of
the same port used to surface as a Postgres constraint violation,
verbatim. The table has two keys, so the collision is one of two facts:
the racer was this same assignment — then its answer is the answer,
kept-once-chosen does not care who chose — or another module took the
machine port, and an unfixed pick is simply made again against the
moved free list. A fixed port that lost the race is refused by name.
Told apart by re-reading the row, not by the constraint's name, so this
does not couple to the migration's spelling.

And the artifact-store cycle tests moved to bootstrap_cycle_test.go;
machineside_test.go had quietly become three subjects.

internal/inventory/catalogue.go

@@ -283,7 +283,11 @@ func (i *Inventory) Unassign(ctx context.Context, nodeName, module string) error
 	if tag.RowsAffected() == 0 {
 		return fmt.Errorf("%s is not assigned to %s", module, nodeName)
 	}
-	return nil
+	// And its ports go back. Kept-once-chosen is a promise about a module that is still here —
+	// held past unassignment, a fixed port stays claimed in the name of something that is gone,
+	// and the next module needing it is refused by a ghost. The same argument that lets a machine
+	// take a carried port back: a set that only grows keeps a port reserved for nothing.
+	return i.ReleasePorts(ctx, nodeName, module)
 }
 
 // Assigned is what a person put on this node, which is not the same as what it runs: resolution

internal/inventory/ports.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 
 	"github.com/jackc/pgx/v5"
+	"github.com/jackc/pgx/v5/pgconn"
 )
 
 // Which port a machine uses for what a module needs reachable.
@@ -163,6 +164,38 @@ func (i *Inventory) assignPort(
 		`insert into port_assignment (node, module, wanted, machine, fixed)
 		 values ($1, $2, $3, $4, $5)`,
 		nodeID, module, wanted, machine, fixed)
+	// Two allocations at once can both pick the same lowest free port; the unique index lets one
+	// through and hands the other a constraint violation in SQL. Said in the mesh's words instead
+	// — and for a port the mesh chose, simply chosen again: the free list has moved, the retry
+	// reads it fresh, and the caller never learns the race happened.
+	var collided *pgconn.PgError
+	if errors.As(err, &collided) && collided.Code == "23505" {
+		// Which race decides what happens next. The table has two keys, so this is one of two
+		// collisions: the racer was *this same assignment* (the primary key), in which case its
+		// answer is the answer — kept-once-chosen does not care who did the choosing — or it was
+		// another module taking the machine port (the unique index), in which case the free list
+		// has moved and an unfixed pick is simply made again. Asking the table tells them apart;
+		// branching on the constraint's name would couple this to the migration's spelling.
+		var held Assigned
+		reread := i.store.Pool().QueryRow(ctx,
+			`select machine, fixed from port_assignment
+			  where node = $1 and module = $2 and wanted = $3`,
+			nodeID, module, wanted).Scan(&held.Machine, &held.Fixed)
+		if reread == nil {
+			held.Module, held.Wanted = module, wanted
+			return held, nil
+		}
+		if !errors.Is(reread, pgx.ErrNoRows) {
+			return Assigned{}, reread
+		}
+		if !fixed {
+			return i.assignPort(ctx, nodeID, node, module, wanted, false)
+		}
+		return Assigned{}, fmt.Errorf(
+			"%w: %s needs %d on %s and something else was given it at the same moment — "+
+				"two assignments raced, and the port the protocol fixes went to the other one",
+			ErrPortTaken, module, wanted, node)
+	}
 	if err != nil {
 		return Assigned{}, err
 	}

internal/inventory/ports_test.go

@@ -211,3 +211,28 @@ func TestWhatAMachineNoLongerHoldsIsAvailableAgain(t *testing.T) {
 		t.Errorf("a port the machine gave back was still reserved: got %d", got.Machine)
 	}
 }
+
+// Unassigning a module gives its ports back — the fixed ones are what make this matter.
+//
+// Held past unassignment, port 25 stays claimed in the name of a mail system that is gone, and
+// every mail system after it is refused by a ghost. Found in review: ReleasePorts existed, was
+// documented "for when it is unassigned", and was called by nothing.
+func TestUnassigningReleasesTheModulesPorts(t *testing.T) {
+	inv, node := aNodeWithModules(t, "mailu", "other-mail")
+	ctx := t.Context()
+	if err := inv.Assign(ctx, node, "mailu"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := inv.PortFor(ctx, node, "mailu", 25, true); err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.Unassign(ctx, node, "mailu"); err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.Assign(ctx, node, "other-mail"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := inv.PortFor(ctx, node, "other-mail", 25, true); err != nil {
+		t.Fatalf("port 25 is still held in the name of a module that was unassigned: %v", err)
+	}
+}