Clean up share links when accepted

internal/core/shares.go

@@ -197,9 +197,9 @@ func (s *Handlers) handleAcceptInvite(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Consume the link, grant the share, and seed default commands atomically, so a
-	// failure can never spend the link without granting access. The used_at guard
-	// inside makes it the single-use gate against concurrent accepts.
+	// Grant the share, seed its commands, and delete the link atomically, so a
+	// failure can never delete the link without granting access. A row lock inside
+	// makes it the single-use gate against concurrent accepts.
 	if _, err := s.Store.AcceptInvite(r.Context(), token, uid); errors.Is(err, store.ErrNotFound) {
 		s.Render(w, r, "invite.html", map[string]any{"State": "invalid"})
 		return

internal/core/templates/share.html

@@ -16,17 +16,21 @@
 <div class="row row-cards mt-1">
 <div class="col-lg-7">
 <div class="card">
-  <div class="card-body">
+  <div class="card-header">
     <h3 class="card-title">Share links</h3>
-    <p class="text-secondary">
+    <div class="card-actions">
+      <button type="button" class="btn btn-primary" data-testid="new-invite"
+              data-bs-toggle="modal" data-bs-target="#invite-modal"
+              hx-get="/repeaters/{{.Repeater.PublicID}}/share/link/new"
+              hx-target="#invite-modal-content" hx-swap="innerHTML">{{template "icon-plus" "me-1"}}Create link</button>
+    </div>
+  </div>
+  <div class="card-body">
+    <p class="text-secondary mb-0">
       Invite specific people with single-use links: the recipient signs in and accepts to control this
-      repeater from their own browser-connected KISS modem. Each link works once — mint one per person,
-      label it, and choose what they can do.
+      repeater from their own browser-connected KISS modem. Each link works once and disappears when it's
+      used — mint one per person, label it, and choose what they can do.
     </p>
-    <button type="button" class="btn btn-primary" data-testid="new-invite"
-            data-bs-toggle="modal" data-bs-target="#invite-modal"
-            hx-get="/repeaters/{{.Repeater.PublicID}}/share/link/new"
-            hx-target="#invite-modal-content" hx-swap="innerHTML">Create single-use link</button>
 
     {{if .Invites}}
     <div class="list-group list-group-flush mt-3">
@@ -34,23 +38,21 @@
       <div class="list-group-item px-0">
         <div class="d-flex align-items-center gap-2 mb-1">
           <span class="fw-bold">{{if .Description}}{{.Description}}{{else}}(no label){{end}}</span>
-          {{if .UsedAt}}<span class="badge bg-success-lt">Used by {{.UsedByName}}</span>{{else}}<span class="badge bg-yellow-lt">Unused</span>{{end}}
+          <span class="badge bg-yellow-lt">Unused</span>
         </div>
-        {{if not .UsedAt}}
         <div class="input-group mb-2">
           <code class="form-control font-monospace" id="link-{{.ID}}">{{$.BaseURL}}/invite/{{.Token}}</code>
           <button type="button" class="btn" data-copy-target="#link-{{.ID}}">{{template "icon-copy" ""}}Copy</button>
         </div>
-        {{end}}
         <form method="post" action="/repeaters/{{$.Repeater.PublicID}}/share/link/delete" class="m-0">
           <input type="hidden" name="invite_id" value="{{.ID}}">
-          <button type="submit" class="btn btn-sm btn-ghost-danger">{{if .UsedAt}}Remove from list{{else}}Revoke link{{end}}</button>
+          <button type="submit" class="btn btn-sm btn-ghost-danger">Revoke link</button>
         </form>
       </div>
       {{end}}
     </div>
     {{else}}
-    <p class="text-secondary mt-2">No links yet.</p>
+    <p class="text-secondary mt-3 mb-0">No pending links.</p>
     {{end}}
   </div>
 </div>

internal/store/migrations/0039_drop_invite_used.sql

@@ -0,0 +1,13 @@
+-- +goose Up
+-- Single-use links are now deleted the instant they're redeemed (see AcceptInvite):
+-- a consumed link is redundant with the recipient who then appears in the
+-- People-with-access list, so there's no "used" state to track. Remove links that
+-- were already consumed (their recipients already hold a share), then drop the
+-- consumed-state columns.
+DELETE FROM repeater_invites WHERE used_at IS NOT NULL;
+ALTER TABLE repeater_invites DROP COLUMN used_at;
+ALTER TABLE repeater_invites DROP COLUMN used_by;
+
+-- +goose Down
+ALTER TABLE repeater_invites ADD COLUMN used_at TIMESTAMPTZ;
+ALTER TABLE repeater_invites ADD COLUMN used_by BIGINT REFERENCES users(id) ON DELETE SET NULL;

internal/store/shares.go

@@ -170,15 +170,13 @@ func (s *Store) ListStewards(ctx context.Context, repeaterID int64) ([]ShareInfo
 
 // --- share links (single-use invites) ---
 
-// Invite is a single-use share link. Used invites retain who consumed them and
-// when, as an audit trail.
+// Invite is a pending single-use share link. Redeemed links are deleted (see
+// AcceptInvite), so every Invite is unused.
 type Invite struct {
 	ID          int64
 	Token       string
 	Description string
 	CreatedAt   time.Time
-	UsedAt      *time.Time
-	UsedByName  *string // display name or username of the consumer, if used
 }
 
 // CreateInvite mints a new single-use share link for a repeater, returning its
@@ -213,21 +211,19 @@ func (s *Store) CreateInvite(ctx context.Context, repeaterID int64, description
 	return token, nil
 }
 
-// ListInvites returns all invites for a repeater (pending and used), newest first.
+// ListInvites returns a repeater's pending (unredeemed) share links, newest first.
 func (s *Store) ListInvites(ctx context.Context, repeaterID int64) ([]Invite, error) {
 	rows, err := s.pool.Query(ctx, `
-		SELECT i.id, i.token, i.description, i.created_at, i.used_at,
-		       COALESCE(NULLIF(u.display_name, ''), u.username)
-		FROM repeater_invites i
-		LEFT JOIN users u ON u.id = i.used_by
-		WHERE i.repeater_id = $1
-		ORDER BY i.created_at DESC`, repeaterID)
+		SELECT id, token, description, created_at
+		FROM repeater_invites
+		WHERE repeater_id = $1
+		ORDER BY created_at DESC`, repeaterID)
 	if err != nil {
 		return nil, fmt.Errorf("list invites: %w", err)
 	}
 	return collectRows(rows, func(r pgx.Row) (Invite, error) {
 		var inv Invite
-		err := r.Scan(&inv.ID, &inv.Token, &inv.Description, &inv.CreatedAt, &inv.UsedAt, &inv.UsedByName)
+		err := r.Scan(&inv.ID, &inv.Token, &inv.Description, &inv.CreatedAt)
 		return inv, err
 	})
 }
@@ -243,12 +239,12 @@ func (s *Store) DeleteInvite(ctx context.Context, repeaterID, inviteID int64) er
 	return nil
 }
 
-// RepeaterByInviteToken resolves a *valid* (unused) share-link token to its
-// repeater, or ErrNotFound if the token is unknown, revoked, or already used.
-// queryUserID is used only to populate the Shared flag and owner display.
+// RepeaterByInviteToken resolves a valid share-link token to its repeater, or
+// ErrNotFound if the token is unknown, revoked, or already redeemed (redemption
+// deletes the link). queryUserID only populates the Shared flag and owner display.
 func (s *Store) RepeaterByInviteToken(ctx context.Context, queryUserID int64, token string) (*Repeater, error) {
 	row := s.pool.QueryRow(ctx, repeaterSelect+`
-		WHERE r.id = (SELECT repeater_id FROM repeater_invites WHERE token = $2 AND used_at IS NULL)`,
+		WHERE r.id = (SELECT repeater_id FROM repeater_invites WHERE token = $2)`,
 		queryUserID, token)
 	r, err := scanRepeater(row)
 	if err != nil {
@@ -257,25 +253,23 @@ func (s *Store) RepeaterByInviteToken(ctx context.Context, queryUserID int64, to
 	return r, nil
 }
 
-// AcceptInvite redeems a single-use share link for userID, doing all three steps
-// in one transaction so they are all-or-nothing: it consumes the link, grants the
-// share, and seeds the command set the owner chose for this link (invite_commands).
-// If any step fails the whole thing rolls back, so a failure can never spend the
-// link without granting access (or grant access without spending the link).
+// AcceptInvite redeems a single-use share link for userID in one transaction so
+// the steps are all-or-nothing: it grants the share, seeds the command set the
+// owner chose for this link (invite_commands), and deletes the link — a redeemed
+// link doesn't stick around. If any step fails the whole thing rolls back, so a
+// failure can never delete the link without granting access (or grant access
+// without deleting the link, leaving it redeemable again).
 //
 // It returns whether a new share was created (false if the user already had one)
-// and ErrNotFound if the token is unknown, revoked, or already used — the
-// single-use guard, safe against concurrent accepts.
+// and ErrNotFound if the token is unknown, revoked, or already redeemed. The row
+// lock is the single-use gate: concurrent accepts serialize on it, and the winner
+// deletes the row, so losers find nothing.
 func (s *Store) AcceptInvite(ctx context.Context, token string, userID int64) (added bool, err error) {
 	err = s.inTx(ctx, func(tx pgx.Tx) error {
 		var inviteID, repeaterID int64
-		// Consume the link. The used_at IS NULL guard makes this the single-use
-		// gate: only one accept can flip it, so a concurrent (or repeat) accept
-		// matches no row and falls through to ErrNotFound.
 		if err := tx.QueryRow(ctx,
-			`UPDATE repeater_invites SET used_at = now(), used_by = $2
-			 WHERE token = $1 AND used_at IS NULL
-			 RETURNING id, repeater_id`, token, userID).Scan(&inviteID, &repeaterID); err != nil {
+			`SELECT id, repeater_id FROM repeater_invites WHERE token = $1 FOR UPDATE`,
+			token).Scan(&inviteID, &repeaterID); err != nil {
 			return notFoundOr(err, "consume invite")
 		}
 		tag, err := tx.Exec(ctx,
@@ -285,16 +279,19 @@ func (s *Store) AcceptInvite(ctx context.Context, token string, userID int64) (a
 			return fmt.Errorf("add share: %w", err)
 		}
 		added = tag.RowsAffected() > 0
-		if !added {
-			return nil // already shared: nothing to seed
+		if added {
+			// Seed the new share with the command set the owner chose for this link,
+			// before the delete below cascades invite_commands away.
+			if _, err := tx.Exec(ctx, `
+				INSERT INTO share_commands (repeater_id, user_id, command_id)
+				SELECT $1, $2, command_id FROM invite_commands WHERE invite_id = $3
+				ON CONFLICT DO NOTHING`, repeaterID, userID, inviteID); err != nil {
+				return fmt.Errorf("seed share commands: %w", err)
+			}
 		}
-		// Seed the new share with exactly the command set the owner chose for this
-		// link; the owner can adjust it afterwards.
-		if _, err := tx.Exec(ctx, `
-			INSERT INTO share_commands (repeater_id, user_id, command_id)
-			SELECT $1, $2, command_id FROM invite_commands WHERE invite_id = $3
-			ON CONFLICT DO NOTHING`, repeaterID, userID, inviteID); err != nil {
-			return fmt.Errorf("seed share commands: %w", err)
+		// Consume the single-use link by deleting it (cascades its invite_commands).
+		if _, err := tx.Exec(ctx, `DELETE FROM repeater_invites WHERE id = $1`, inviteID); err != nil {
+			return fmt.Errorf("consume invite: %w", err)
 		}
 		return nil
 	})

internal/store/shares_test.go

@@ -61,15 +61,10 @@ func TestAcceptInvite(t *testing.T) {
 	if len(cmds) != 1 || cmds[0] != grant {
 		t.Fatalf("seeded commands = %v, want exactly [%d] (the chosen grant)", cmds, grant)
 	}
+	// The redeemed link is deleted, not retained — nothing left to show.
 	invites, err := st.ListInvites(ctx, rep.ID)
-	if err != nil || len(invites) != 1 {
-		t.Fatalf("ListInvites = %d invites, %v; want 1", len(invites), err)
-	}
-	if invites[0].UsedAt == nil {
-		t.Fatalf("invite not marked consumed after accept")
-	}
-	if invites[0].UsedByName == nil || *invites[0].UsedByName != "invitee" {
-		t.Fatalf("invite UsedByName = %v, want invitee", invites[0].UsedByName)
+	if err != nil || len(invites) != 0 {
+		t.Fatalf("ListInvites = %d invites, %v; want 0 (redeemed link deleted)", len(invites), err)
 	}
 
 	// Single-use: a second accept (by anyone) must fail and grant nothing.
@@ -86,10 +81,10 @@ func TestAcceptInvite(t *testing.T) {
 }
 
 // TestAcceptInviteRollsBack is the atomicity regression: if any step of the
-// redemption fails, the whole thing must roll back — the link stays unspent so it
+// redemption fails, the whole thing must roll back — the link stays undeleted so it
 // can still be redeemed, rather than being consumed with no access granted (the
 // reported bug). We trigger a failure by redeeming for a non-existent user, which
-// violates the used_by/user_id foreign keys inside the transaction.
+// violates the repeater_shares.user_id foreign key inside the transaction.
 func TestAcceptInviteRollsBack(t *testing.T) {
 	t.Parallel()
 	st, ctx := orgTestStore(t)
@@ -115,13 +110,10 @@ func TestAcceptInviteRollsBack(t *testing.T) {
 		t.Fatalf("AcceptInvite for non-existent user succeeded, want error")
 	}
 
-	// The link must NOT have been consumed by the failed attempt.
+	// The link must NOT have been deleted by the failed attempt — still redeemable.
 	invites, err := st.ListInvites(ctx, rep.ID)
 	if err != nil || len(invites) != 1 {
-		t.Fatalf("ListInvites = %d, %v; want 1", len(invites), err)
-	}
-	if invites[0].UsedAt != nil {
-		t.Fatalf("link was consumed despite the accept failing (non-atomic)")
+		t.Fatalf("ListInvites = %d, %v; want 1 (link survives a failed accept)", len(invites), err)
 	}
 
 	// And a real user can still redeem it afterwards.