From ef3cb2a304b945c87b5db3945232cb2b19b2610b Mon Sep 17 00:00:00 2001
From: Sergei Poljanski <me@asxp.io>
Date: Thu, 2 Jul 2026 17:31:19 +0400
Subject: [PATCH] invoices: numeric number allocation, retry create on
 collision

---
 CLAUDE.md      |  2 +-
 asxpio.rb      | 24 ++++++++++++++++++------
 lib/invoice.rb |  9 +++++----
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index 2c0f31c..4e135c2 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -105,7 +105,7 @@ Admin pages use a shared dark palette extended in `public/style.css` (search for
 
 ### Caveats
 
-- The `INV-{YYYY}-{NNNN}` allocator is `SELECT MAX(...) + 1`. Single-process deploy means no contention; if Puma ever runs multiple workers or we add a second container, switch to a DB-side sequence per year.
+- The `INV-{YYYY}-{NNNN}` allocator is `SELECT MAX(...) + 1` over the numeric suffix. Concurrent creates (Puma threads) that collide on the unique constraint are retried — the whole build→render→upload→save sequence, since the PDF embeds the number; a losing attempt orphans its two S3 objects. If we ever add a second container, switch to a DB-side sequence per year.
 - Anyone with the UUID can fetch the PDF. UUIDs are 122-bit random so not enumerable, but they aren't access-controlled. If a stronger gate is ever needed (email confirmation, expiry on paid, etc.) it goes in the `/i/:uuid` and `/i/:uuid/pdf` handlers.
 - The MinIO bucket is **private**; only the app's scoped service account can put/get. Presigned URLs use `S3_PUBLIC_ENDPOINT=https://s3.asxp.io` so the signed host matches what the browser fetches.
 
diff --git a/asxpio.rb b/asxpio.rb
index 5d6d7cb..a99a732 100644
--- a/asxpio.rb
+++ b/asxpio.rb
@@ -266,13 +266,25 @@ class AsxpioWeb < Sinatra::Base
       return erb :'admin/invoices/new'
     end
 
-    invoice = Invoice.build(@form_values)
-    # Base key (no suffix); the two status variants get -pending/-paid appended.
-    invoice.pdf_key = "invoices/#{invoice.number}-#{invoice.uuid}.pdf"
-    %w[pending paid].each do |st|
-      S3.put(invoice.pdf_key_for(st), InvoicePdf.render(invoice, status: st))
+    # The whole build→render→upload→save sequence retries on a duplicate
+    # number: two concurrent creates (Puma threads) can both compute MAX+1.
+    # The PDF embeds the number, so a retry must re-render, not just re-save.
+    # A losing attempt orphans its two S3 objects; harmless and near-impossible
+    # with a single operator.
+    attempts = 0
+    begin
+      invoice = Invoice.build(@form_values)
+      # Base key (no suffix); the two status variants get -pending/-paid appended.
+      invoice.pdf_key = "invoices/#{invoice.number}-#{invoice.uuid}.pdf"
+      %w[pending paid].each do |st|
+        S3.put(invoice.pdf_key_for(st), InvoicePdf.render(invoice, status: st))
+      end
+      invoice.save_changes
+    rescue Sequel::UniqueConstraintViolation
+      attempts += 1
+      raise if attempts > 2
+      retry
     end
-    invoice.save_changes
 
     redirect "/admin/invoices/#{invoice.uuid}"
   end
diff --git a/lib/invoice.rb b/lib/invoice.rb
index a323d93..3eb6961 100644
--- a/lib/invoice.rb
+++ b/lib/invoice.rb
@@ -60,13 +60,14 @@ class Invoice < Sequel::Model(:invoices)
   end
 
   class << self
+    # Max is taken over the numeric suffix, not the string: past 9999 the
+    # 5-digit numbers would sort below INV-YYYY-9999 lexically and the
+    # allocator would hand out duplicates.
     def allocate_number(year = Date.today.year)
       prefix = "INV-#{year}-"
       last   = where(Sequel.like(:number, "#{prefix}%"))
-                 .order(Sequel.desc(:number))
-                 .get(:number)
-      n = last ? last.split('-').last.to_i + 1 : 1
-      format("#{prefix}%04d", n)
+                 .max(Sequel.cast(Sequel.function(:split_part, :number, '-', 3), Integer))
+      format("#{prefix}%04d", (last || 0) + 1)
     end
 
     def build(params)