refactor: enforce Effective Handle uniqueness in Postgres (generated column + index)

[pachev]

Split from #27 (item 6). This one needs a migration + ADR amendment, so it does not belong in #27's "clean slice."

Settled in a /domain-plan session. ADR 0008 has the full amendment ("Effective Handle uniqueness moves to the database"); CONTEXT.md gains the Effective Handle term. This issue is the implementation.

Why

Effective Handle uniqueness is documented as an invariant ("two Releases on a Server sharing a handle: neither wins"), but it is enforced only by Mast.Fleet.Release.taken?/3 — an in-memory Repo.all + Enum.any? scan that is (a) racy under concurrent inserts and (b) the source of an N+1 read shared with Fleet.get_release/2. The partial (server_id, name) index is blind to the derived-basename case (name=foo vs release_command=/opt/foo/bin/foo both → handle foo).

The invariant (unchanged)

No two Releases on one Server may share an Effective Handle. Logs-only Releases (no Name, no release_command) have no handle (nil) and never collide.

Changes

1. Migration (forward-only)

Add a stored generated column and swap the index:

# add column
execute """
ALTER TABLE releases ADD COLUMN effective_handle text
GENERATED ALWAYS AS (
  NULLIF(
    COALESCE(
      NULLIF(name, ''),
      regexp_replace(regexp_replace(release_command, '/+$', ''), '^.*/', '')
    ),
  '')
) STORED
""", "ALTER TABLE releases DROP COLUMN effective_handle"

create unique_index(:releases, [:server_id, :effective_handle])
drop unique_index(:releases, [:server_id, :name], where: "name IS NOT NULL")

• Double regexp_replace strips trailing slashes before basename, matching Path.basename/1 exactly (a naive ^.*/ diverges on trailing slashes: /opt/foo/ → "" vs Elixir "foo").
• Outer NULLIF makes an empty derived basename = NULL = no handle.
• Postgres NULL-distinctness lets multiple logs-only Releases coexist — no partial WHERE needed.
• The effective_handle index subsumes the old partial name index; drop the old one.

2. Schema (lib/mast/fleet/release.ex)

• Add field :effective_handle, :string, read_after_writes: true (generated column, never cast).
• Add unique_constraint([:server_id, :effective_handle], name: :releases_server_id_effective_handle_index, ...) as the race backstop, generic message. Keep the existing [:server_id, :name] unique_constraint mapping pointed at the new index OR remove it — only one DB constraint exists now.
• Tighten validate_release_command/1: also reject a trailing slash (String.ends_with?(path, "/")) — bin/<release> is a file, a trailing slash is meaningless.
• Align effective_handle/1: return nil (not "") when the derived basename is empty, so app == column.

3. Layered enforcement (do NOT delete taken?/3)

taken?/3 stays as the UX layer — it fires first in the common case and gives the situation-aware message ("set an explicit name"). Rewrite it from Repo.all + Enum.any? to:

defp taken?(server_id, handle, current_id) do
  q = from r in __MODULE__,
        where: r.server_id == ^server_id and r.effective_handle == ^handle
  q = if current_id, do: where(q, [r], r.id != ^current_id), else: q
  Mast.Repo.exists?(q)
end

The DB index is the invariant guarantee under concurrency; taken?/3 is the friendly pre-check. Layered, not replaced.

4. Read path (lib/mast/fleet.ex)

get_release/2: replace Repo.all + Enum.find(effective_handle == handle) with Repo.one filtered on effective_handle in SQL.

Leave list_releases' Enum.sort_by(&effective_handle/1) alone — in-memory sort of an already-loaded small list, not worth a query change.

TDD notes

• RED first for the collision the old index missed: insert name: "foo", then insert release_command: "/opt/foo/bin/foo" (no name) on the same server → must be rejected. This fails today (passes the insert).
• Trailing-slash equivalence: a row with release_command: "/a/b/foo" and one with "/a/b/foo/" (if it could exist) compute the same handle — covered by the column expression; assert via the generated value after insert.
• Concurrency backstop is hard to unit-test deterministically; assert the unique_constraint is mapped (changeset error on duplicate) rather than racing threads.

Done =

• Migration adds column + unique index, drops old partial name index; mix ecto.migrate clean
• Derived-basename collision is DB-rejected (test)
• effective_handle/1 returns nil for empty basename; matches column
• validate_release_command/1 rejects trailing slash
• taken?/3 is Repo.exists?; get_release/2 is SQL-filtered Repo.one
• ADR 0008 amendment + CONTEXT.md Effective Handle term land in the same PR
• mix precommit green