fix(connection): settle legacy reservations and end failed grants

internal/apidocs/openapi.yaml

@@ -94,7 +94,7 @@ components:
         request_id: {type: string, format: uuid}
         status:
           type: string
-          enum: [open, claimed, confirmed, provisioning, released, rejected, lapsed]
+          enum: [open, claimed, confirmed, provisioning, released, rejected, lapsed, failed]
         vta_did: {type: string}
         operation: {type: string, enum: [provision_vta, grant_acl]}
         expires_at: {type: string, format: date-time}
@@ -110,7 +110,9 @@ components:
         candidate_did:
           type: string
           description: Proof-bound candidate administrator DID; present only while confirmation is required.
-        error: {type: string}
+        error:
+          type: string
+          description: Why adding the administrator has not completed, or why it failed. A failed request is final; generate a new QR code to try again.
     MobileConnectionState:
       type: object
       required: [enabled, server_time, connection]
@@ -259,7 +261,7 @@ components:
           description: True only for an unclaimed session that is prebuilt and waiting or whose preparation failed
         claim_state:
           type: string
-          enum: [open, claimed, confirmed, provisioning, released, rejected, lapsed]
+          enum: [open, claimed, confirmed, provisioning, released, rejected, lapsed, failed]
       required: [session_id, user_id, vta_name, status, url, claimed, cleanable]
 
     PoolBatch:

internal/connection/store.go

@@ -459,6 +459,16 @@ func MarkReleased(ctx context.Context, db *gorm.DB, requestID string) error {
 	})
 }
 
+// MarkFailed ends an additional-administrator request whose grant did not
+// complete. The status must be final: anything else keeps the request active,
+// which blocks every new QR code for the session and has the recovery queue
+// retry the grant, restarting the VTA each time.
+func MarkFailed(ctx context.Context, db *gorm.DB, requestID, reason string) error {
+	return db.WithContext(ctx).Model(&model.MobileConnection{}).
+		Where("id = ? AND operation = ? AND status IN ?", requestID, OperationGrantACL, []string{"confirmed", "provisioning"}).
+		Updates(map[string]any{"status": "failed", "provision_error": reason}).Error
+}
+
 // Reconcile expires unconfirmed reservations and marks initial provisioning as
 // released once the existing setup pipeline reports the VTA running.
 func Reconcile(ctx context.Context, db *gorm.DB, r *model.MobileConnection, s *model.SetupSession) error {

internal/connection/store_test.go

@@ -3,11 +3,13 @@ package connection
 import (
 	"context"
 	"errors"
+	"os"
 	"strings"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/google/uuid"
 	"github.com/ic3software/vtafarm-api/internal/didkey"
 	"github.com/ic3software/vtafarm-api/internal/model"
 	"github.com/ic3software/vtafarm-api/internal/testutil"
@@ -257,3 +259,98 @@ func TestTeardownRejectsClaim(t *testing.T) {
 		t.Fatalf("claim after teardown: %v", err)
 	}
 }
+
+func TestFailedGrantFreesSession(t *testing.T) {
+	db := testutil.Postgres(t)
+	ctx := context.Background()
+	s := testutil.SetupSession(t, db, model.ModeVtaOnly)
+	if err := db.Model(&s).Update("status", "running").Error; err != nil {
+		t.Fatal(err)
+	}
+	r := mustRequest(t, db, s)
+	if _, err := Claim(ctx, db, *r.ClaimHandle, newDID(t)); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := Confirm(ctx, db, s.ID, r.ID); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := StartProvisioning(ctx, db, r.ID); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := Change(ctx, db, s.ID, "refresh", r.ID); !errors.Is(err, ErrConflict) {
+		t.Fatalf("refresh during grant: %v", err)
+	}
+	if err := MarkFailed(ctx, db, r.ID, "grant failed"); err != nil {
+		t.Fatal(err)
+	}
+	var failed model.MobileConnection
+	db.First(&failed, "id = ?", r.ID)
+	if failed.Status != "failed" || failed.ProvisionError != "grant failed" {
+		t.Fatalf("not failed: %+v", failed)
+	}
+	if err := db.Model(&failed).Update("created_at", failed.CreatedAt.Add(-time.Minute)).Error; err != nil {
+		t.Fatal(err)
+	}
+	next, err := Change(ctx, db, s.ID, "refresh", r.ID)
+	if err != nil {
+		t.Fatalf("refresh after failure: %v", err)
+	}
+	if next.ID == r.ID || next.Status != "open" || next.Operation != OperationGrantACL {
+		t.Fatalf("no fresh reservation: %+v", next)
+	}
+}
+
+func TestLegacyReservationsSettled(t *testing.T) {
+	db := testutil.Postgres(t)
+	ctx := context.Background()
+	pending := testutil.SetupSession(t, db, model.ModeVtaOnly)
+	accepted := testutil.SetupSession(t, db, model.ModeVtaOnly)
+	connectedAt := time.Now().Add(-time.Hour).UTC().Truncate(time.Microsecond)
+	for _, row := range []struct {
+		id        string
+		session   model.SetupSession
+		status    string
+		connected *time.Time
+	}{
+		{uuid.NewString(), pending, "pending", nil},
+		{uuid.NewString(), accepted, "accepted", &connectedAt},
+	} {
+		if err := db.Model(&row.session).Update("status", "running").Error; err != nil {
+			t.Fatal(err)
+		}
+		if err := db.Exec(`INSERT INTO mobile_connections (id, session_id, vta_did, status, created_at, expires_at, connected_at)
+			VALUES (?, ?, ?, ?, now() - interval '2 days', now() - interval '1 day', ?)`, row.id, row.session.ID, row.session.VtaDid, row.status, row.connected).Error; err != nil {
+			t.Fatal(err)
+		}
+		if _, err := Change(ctx, db, row.session.ID, "create", ""); !errors.Is(err, ErrConflict) {
+			t.Fatalf("%s row before migration: %v", row.status, err)
+		}
+	}
+
+	up, err := os.ReadFile("../../migrations/000044_settle_legacy_mobile_connections.up.sql")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := db.Exec(string(up)).Error; err != nil {
+		t.Fatal(err)
+	}
+
+	for _, want := range []struct {
+		session model.SetupSession
+		status  string
+	}{{pending, "lapsed"}, {accepted, "released"}} {
+		legacy, err := Current(db, want.session.ID)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if legacy.Status != want.status {
+			t.Fatalf("legacy row = %s, want %s", legacy.Status, want.status)
+		}
+		if want.status == "released" && (legacy.ReleasedAt == nil || !legacy.ReleasedAt.Equal(connectedAt)) {
+			t.Fatalf("released_at = %v, want %v", legacy.ReleasedAt, connectedAt)
+		}
+		if _, err := Change(ctx, db, want.session.ID, "create", ""); err != nil {
+			t.Fatalf("%s row after migration: %v", want.status, err)
+		}
+	}
+}

internal/handler/mobile_additional.go

@@ -122,9 +122,7 @@ func (h *SetupHandler) runAdditionalMobile(ctx context.Context, requestID string
 
 func (h *SetupHandler) failAdditionalMobile(ctx context.Context, requestID string, err error) {
 	log.Printf("[mobile-connection] additional request %s failed: %v", requestID, err)
-	if updateErr := h.db.WithContext(ctx).Model(&model.MobileConnection{}).
-		Where("id = ? AND operation = ? AND status IN ?", requestID, connection.OperationGrantACL, []string{"confirmed", "provisioning"}).
-		Update("provision_error", additionalMobileFailure).Error; updateErr != nil {
+	if updateErr := connection.MarkFailed(ctx, h.db, requestID, additionalMobileFailure); updateErr != nil {
 		log.Printf("[mobile-connection] could not record failure for request %s: %v", requestID, updateErr)
 	}
 }

internal/handler/mobile_connection.go

@@ -124,7 +124,7 @@ func (h *MobileConnectionHandler) view(r *model.MobileConnection) *mobileView {
 		v.ClaimCheck = connection.ClaimCheck(r.AdminDid)
 		v.CandidateDID = r.AdminDid
 	}
-	if r.ProvisionError != "" && (r.Status == "confirmed" || r.Status == "provisioning") {
+	if r.ProvisionError != "" && (r.Status == "confirmed" || r.Status == "provisioning" || r.Status == "failed") {
 		v.Error = r.ProvisionError
 	}
 	return v

migrations/000044_settle_legacy_mobile_connections.down.sql

@@ -0,0 +1,4 @@
+-- Nothing to undo: the settled rows can no longer be told apart from ones the
+-- claim flow wrote, and 000041's down maps lapsed and released back to the
+-- 000039 statuses anyway.
+SELECT 1;

migrations/000044_settle_legacy_mobile_connections.up.sql

@@ -0,0 +1,12 @@
+-- 000041 added the claim-reservation statuses but left rows written by the
+-- 000039 flow as they were. The reservation code knows neither 'pending' nor
+-- 'accepted' and treats an unknown latest row as still active, so creating or
+-- refreshing a QR code on such a session answered 409 forever.
+UPDATE mobile_connections
+SET status = 'lapsed'
+WHERE status = 'pending';
+
+UPDATE mobile_connections
+SET status = 'released',
+    released_at = COALESCE(connected_at, provisioned_at, accepted_at, created_at)
+WHERE status = 'accepted';