From 556b9440b81c0d56649e5ee51e1b50154249a329 Mon Sep 17 00:00:00 2001
From: prosolis <5590409+prosolis@users.noreply.github.com>
Date: Fri, 24 Jul 2026 22:36:35 -0700
Subject: [PATCH] adventure: don't strand an order on a database blip, don't
 misroute a tap

Two things a code review turned up in the W9 seams.

The verdict handler answered 400 for everything ResolveAdvOrder could fail
with, not just a bad verdict. gogobee's contract says a 400 means "park this
row for a human", so a SQLite busy or a disk hiccup permanently stranded an
extract or a bout that was perfectly resolvable. Split the two apart with
ErrBadAdvVerdict: a verdict outside the terminal set is still 400, because
gogobee will never send it successfully, and a genuine storage failure is now
500 and comes back on the next poll.

The push URL builders concatenated the guid and the run id raw, while every
other builder beside them path-escapes because these values arrive over a wire.
A guid carrying a slash sent the notification tap to a different page.
---
 internal/storage/orders.go     |  8 +++++++-
 internal/web/orders.go         | 12 ++++++++++--
 internal/web/push_adventure.go |  8 ++++++--
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/internal/storage/orders.go b/internal/storage/orders.go
index 1f2f670..d0c0b27 100644
--- a/internal/storage/orders.go
+++ b/internal/storage/orders.go
@@ -148,6 +148,12 @@ func validAdvVerdict(status string) bool {
 
 var ErrNoSuchAdvOrder = errors.New("orders: no such order")
 
+// ErrBadAdvVerdict is a verdict outside the terminal set. It is kept distinct
+// from a storage failure so the web seam can answer 400 (gogobee sent something
+// it will never be able to send successfully) rather than parking a perfectly
+// resolvable order on a transient database error.
+var ErrBadAdvVerdict = errors.New("orders: bad verdict")
+
 // InsertAdvOrder records a fresh, pending order and returns it with a new guid.
 // The guid is minted here so the owner has a stable reference the instant they
 // click, before gogobee has heard of it. The caller has already proved the signed-
@@ -218,7 +224,7 @@ func PendingAdvOrders(limit int) ([]AdvOrder, error) {
 // missing row all take one path.
 func ResolveAdvOrder(guid, status, detail string) (AdvOrder, error) {
 	if !validAdvVerdict(status) {
-		return AdvOrder{}, fmt.Errorf("orders: bad verdict %q", status)
+		return AdvOrder{}, fmt.Errorf("%w %q", ErrBadAdvVerdict, status)
 	}
 	now := nowUnix()
 	if _, err := Get().Exec(
diff --git a/internal/web/orders.go b/internal/web/orders.go
index ff52ca2..f58222c 100644
--- a/internal/web/orders.go
+++ b/internal/web/orders.go
@@ -336,11 +336,19 @@ func (s *Server) handleAdvOrderVerdict(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "no such order", http.StatusBadRequest)
 		return
 	}
-	if err != nil {
-		slog.Error("orders: resolve", "guid", v.GUID, "status", v.Status, "err", err)
+	if errors.Is(err, storage.ErrBadAdvVerdict) {
+		slog.Error("orders: verdict outside the terminal set", "guid", v.GUID, "status", v.Status)
 		http.Error(w, "bad verdict", http.StatusBadRequest)
 		return
 	}
+	if err != nil {
+		// A storage failure, not a bad request. 400 here would park a perfectly
+		// resolvable order forever on a transient database error; 500 gets it
+		// retried on gogobee's next poll.
+		slog.Error("orders: resolve", "guid", v.GUID, "status", v.Status, "err", err)
+		http.Error(w, "internal error", http.StatusInternalServerError)
+		return
+	}
 	slog.Info("orders: action resolved", "guid", order.GUID, "action", order.Action, "status", order.Status)
 	writeJSON(w, order)
 }
diff --git a/internal/web/push_adventure.go b/internal/web/push_adventure.go
index 260d942..0548a49 100644
--- a/internal/web/push_adventure.go
+++ b/internal/web/push_adventure.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log/slog"
+	"net/url"
 	"strings"
 	"time"
 
@@ -344,13 +345,16 @@ func orPlace(zone string) string {
 // what they have always done.
 func advRunOrStoryURL(ev storage.AdvEvent) string {
 	if ev.RunID != "" {
-		return "/adventure/run/" + ev.RunID
+		return runReportPath(ev.RunID)
 	}
 	return advStoryURL(ev.GUID)
 }
 
+// advStoryURL is advPermalink's relative half, and it escapes for the same
+// reason: the guid arrives over a wire, and one that grew a slash would send the
+// notification somewhere else entirely.
 func advStoryURL(guid string) string {
-	return "/adventure/" + guid
+	return "/adventure/" + url.PathEscape(guid)
 }
 
 // buildAdvPayload renders the notification JSON the service worker expects. The