fix(stack): atomic env-var merge — no lost updates under concurrency (bug-bash #10)

[mastermanas805]

Summary

PATCH /stacks/:slug/env did a non-atomic read-modify-write: GetStackEnvVars (load) → merge the delta in Go → UpdateStackEnvVars (blind full overwrite). Two concurrent PATCHes — PATCH{A=1} and PATCH{B=2} — both read {} and one overwrote the other, silently dropping a key.

This PR replaces that sequence with a single atomic models.MergeStackEnvVars: one transaction that takes a SELECT ... FOR UPDATE row lock, merges inside the tx, checks the 64KiB cap before the write, and commits. Concurrent PATCHes now serialize — the second blocks on the lock and reads the first's committed result instead of a stale snapshot.

• Handler UpdateEnv now calls MergeStackEnvVars once. Error mapping preserved: ErrStackEnvVarsTooLarge→413 env_too_large, ErrStackNotFound→404 not_found, else 503 persist_failed. Audit keys_set = len(body.Env) - deletes math preserved (a delete is counted only when the key was present, matching the old loop's effective behavior on the merged set).
• GetStackEnvVars + UpdateStackEnvVars are kept (other call sites: promote, custom-domain, log streaming, redeploy).

Coverage block

Symptom:        PATCH /stacks/:slug/env loses a key when two PATCHes race
                (non-atomic load→merge→overwrite; lost update).
Enumeration:    rg -F 'models.UpdateStackEnvVars' / 'GetStackEnvVars'  → single mutate site (UpdateEnv);
                other GetStackEnvVars readers (promote x2, redeploy) are read-only, not in the race.
Sites found:    1  (handler UpdateEnv mutate path)
Sites touched:  1  (rewired to one atomic MergeStackEnvVars call)
Coverage test:  models.TestMergeStackEnvVars_ConcurrentPatchesNoLostUpdate — 16 concurrent
                disjoint-key merges vs a real seeded row; asserts all 16 survive (fails on the
                old non-atomic path, passes under -race). Plus models.TestMergeStackEnvVars_Branches
                (sqlmock, 100% of merge arms: begin/select/unmarshal/too-large-rollback/update/
                commit/happy + present-only delete counting).
Live verified:  DB-gated tests run in CI (TEST_DATABASE_URL). Locally proven against
                postgres://localhost:5432/instant_dev_test with -race: merge tests green;
                full UpdateEnv + stack mid-handler-503 fault suite green.

Test-harness note (the fault-injection gotcha)

The atomic merge collapses the old two DB-error surfaces (fetch_failed for the read, persist_failed for the write) into one persist_failed surface. I chose option (b) — repurpose the two stack-env fault tests rather than rewire the shared faultConn — because it is lower blast radius (no change to the harness that ~15 other fault tests share).

Notably, the merge's tx-internal SELECT ... FOR UPDATE and UPDATE do still fault through the shared faultConn: lib/pq's driver.Tx implements neither QueryerContext nor ExecerContext, so database/sql routes sql.Tx.QueryContext/ExecContext back through the conn-level faultConn methods, which honor the failAfter budget. So:

• TestStackFinal2_UpdateEnv_MergeSelectFailed (failAfter=2) — faults the tx SELECT → persist_failed.
• TestStackFinal2_UpdateEnv_PersistFailed (failAfter=3) — faults the tx UPDATE → persist_failed.
• TestStackFinal2_UpdateEnv_MergeRowVanished_404 (new) — a purpose-built local forUpdateVanish driver returns zero rows only for the FOR UPDATE query, so GetStackBySlug finds the row but the merge sees none → ErrStackNotFound → 404. Covers the handler's NotFound arm (a TOCTOU that can't be timed against a live row).
• TestStackUpdateEnv_MidHandler503 (sweep) and all other stack mid-handler-503 fault tests stay green.

I added an accurate comment to faultConn.BeginTx documenting why in-tx queries remain fault-injectable.

🤖 Generated with Claude Code