diff --git a/cmd/obol/sell.go b/cmd/obol/sell.go index 28618b2d..2df0f18a 100644 --- a/cmd/obol/sell.go +++ b/cmd/obol/sell.go @@ -747,8 +747,10 @@ Examples: Value: 8080, }, &cli.StringFlag{ - Name: "health-path", - Usage: "Upstream health check path", + Name: "health-path", + Usage: "Upstream health check path. When left at the default, --upstream litellm " + + "defaults to /health/readiness and --upstream ollama defaults to / — " + + "neither serves a usable /health.", Value: "/health", }, &cli.StringFlag{ @@ -969,11 +971,9 @@ Examples: upstreamSvc := cmd.String("upstream") healthPath := cmd.String("health-path") - // LiteLLM's /health requires the master key (401 unauthenticated). - // Its readiness/liveliness probes are public and return 2xx — the - // only useful default once UpstreamHealthy requires 2xx. - if strings.EqualFold(upstreamSvc, "litellm") && (healthPath == "" || healthPath == "/health") { - healthPath = "/health/readiness" + if defaulted := defaultHealthPathForUpstream(upstreamSvc, healthPath); defaulted != healthPath { + healthPath = defaulted + u.Dim(fmt.Sprintf(" --upstream %s: no usable /health, defaulting --health-path to %q", upstreamSvc, healthPath)) } spec := map[string]any{ "type": "http", @@ -3394,6 +3394,29 @@ Examples: } } +// defaultHealthPathForUpstream fills in a working health-check path for +// upstreams whose generic "/health" default (the --health-path flag's zero +// value) never returns 2xx, which would permanently fail UpstreamHealthy's +// 2xx-only probe gate. Returns healthPath unchanged for any explicit, +// non-default value or an upstream with no special case. +func defaultHealthPathForUpstream(upstreamSvc, healthPath string) string { + if healthPath != "" && healthPath != "/health" { + return healthPath + } + switch { + case strings.EqualFold(upstreamSvc, "litellm"): + // LiteLLM's /health requires the master key (401 unauthenticated). + // Its readiness/liveliness probes are public and return 2xx. + return "/health/readiness" + case strings.EqualFold(upstreamSvc, "ollama"): + // Ollama serves neither /health nor /health/readiness — only "/" + // returns 2xx. + return "/" + default: + return healthPath + } +} + // normalizeAgentOrigin validates that raw is a bare storefront origin // (scheme://host[:port], no path) and returns it with any trailing slash // stripped. The controller only ever publishes agent-registration.json at @@ -3408,17 +3431,32 @@ func normalizeAgentOrigin(raw string) (string, error) { if err != nil || parsed.Host == "" { return "", fmt.Errorf("--origin must be a bare host with no path (got %q): expected scheme://host[:port]", raw) } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return "", fmt.Errorf("--origin must use http or https (got %q): expected scheme://host[:port]", raw) + } if parsed.Path != "" && parsed.Path != "/" { return "", fmt.Errorf("--origin must be a bare host with no path (got %q); agent-registration.json is served at the storefront root, not a service path", raw) } return strings.TrimRight(parsed.Scheme+"://"+parsed.Host, "/"), nil } +// autoRegisterOptOutAnnotation lets an offer opt out of +// enableRegistrationOnOffers' cluster-wide sweep permanently — e.g. an +// operator who deliberately created an offer with --no-register and wants it +// to stay unlisted even after a later `obol sell register` succeeds on its +// network. Not set by --no-register itself (that would defeat the sweep for +// every ordinary --no-register offer); it's an explicit escape hatch. +const autoRegisterOptOutAnnotation = "obol.stack/no-auto-register" + // offerEligibleForAutoEnable reports whether an offer should have -// registration.enabled flipped on: it must still be disabled and pinned to -// one of the networks the triggering `obol sell register` call actually -// succeeded on (registeredNetworks, keyed by erc8004.NetworkConfig.Name). +// registration.enabled flipped on: it must still be disabled, pinned to one +// of the networks the triggering `obol sell register` call actually +// succeeded on (registeredNetworks, keyed by erc8004.NetworkConfig.Name), +// and not carrying the autoRegisterOptOutAnnotation opt-out. func offerEligibleForAutoEnable(o monetizeapi.ServiceOffer, registeredNetworks map[string]bool) bool { + if o.Annotations[autoRegisterOptOutAnnotation] == "true" { + return false + } return !o.Spec.Registration.Enabled && registeredNetworks[o.Spec.Payment.Network] } @@ -3707,6 +3745,15 @@ func isTransientRegistrationError(err error) bool { "connection reset", "eof", "too many requests", + // registerWithRecovery pins the nonce for the whole call and reuses + // it across retries, so a broadcast-timeout retry can resubmit the + // exact tx the node already has: these three responses mean no new + // tx was accepted (no double-mint risk), and retrying gives + // recoverRegistrationByOwnerAndURI more time to see the original + // broadcast land instead of reporting a landed mint as failed. + "already known", + "nonce too low", + "replacement transaction underpriced", } { if strings.Contains(msg, needle) { return true diff --git a/cmd/obol/sell_test.go b/cmd/obol/sell_test.go index d20102af..40e5efeb 100644 --- a/cmd/obol/sell_test.go +++ b/cmd/obol/sell_test.go @@ -804,6 +804,11 @@ func TestNormalizeAgentOrigin(t *testing.T) { {in: "https://store.example.com/.well-known/agent-registration.json", errPart: "no path"}, {in: "not a url", errPart: "no path"}, {in: "", errPart: "no path"}, + // Scheme-less and non-HTTP origins must be rejected, not silently + // accepted — a bare "//host" or "ftp://host" would mint a malformed, + // unfetchable on-chain agentURI. + {in: "//store.example.com", errPart: "http or https"}, + {in: "ftp://store.example.com", errPart: "http or https"}, } { got, err := normalizeAgentOrigin(tc.in) if tc.errPart != "" { @@ -837,6 +842,9 @@ func TestOfferEligibleForAutoEnable(t *testing.T) { } } + optedOut := offer("base", false) + optedOut.Annotations = map[string]string{autoRegisterOptOutAnnotation: "true"} + tests := []struct { name string o monetizeapi.ServiceOffer @@ -846,6 +854,7 @@ func TestOfferEligibleForAutoEnable(t *testing.T) { {"disabled offer on a different, unregistered network", offer("ethereum", false), false}, {"already-enabled offer on a just-registered network", offer("base", true), false}, {"disabled offer on an unregistered network stays disabled", offer("polygon", false), false}, + {"opt-out annotation exempts an otherwise-eligible offer", optedOut, false}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -857,6 +866,34 @@ func TestOfferEligibleForAutoEnable(t *testing.T) { } } +// TestDefaultHealthPathForUpstream pins the --upstream-specific health-path +// defaults: ollama serves 2xx only at "/" (neither /health nor +// /health/readiness), so under the 2xx-only UpstreamHealthy gate the plain +// "/health" default would leave the offer permanently un-Ready. +func TestDefaultHealthPathForUpstream(t *testing.T) { + tests := []struct { + name string + upstream string + healthPath string + want string + }{ + {"ollama gets / when health-path unset", "ollama", "", "/"}, + {"ollama gets / when health-path left at the flag default", "ollama", "/health", "/"}, + {"ollama case-insensitive", "Ollama", "/health", "/"}, + {"ollama explicit override is respected", "ollama", "/custom-health", "/custom-health"}, + {"litellm still defaults to /health/readiness", "litellm", "/health", "/health/readiness"}, + {"other upstream keeps the flag default", "my-svc", "/health", "/health"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := defaultHealthPathForUpstream(tc.upstream, tc.healthPath); got != tc.want { + t.Errorf("defaultHealthPathForUpstream(%q, %q) = %q, want %q", + tc.upstream, tc.healthPath, got, tc.want) + } + }) + } +} + func TestSellPricing_Flags(t *testing.T) { cfg := newTestConfig(t) cmd := sellCommand(cfg) @@ -933,6 +970,16 @@ func TestIsTransientRegistrationError(t *testing.T) { {name: "rpc 500", err: errors.New("erc8004: register tx: 500 Internal Server Error"), want: true}, {name: "timeout", err: errors.New("context deadline exceeded while waiting for headers"), want: true}, {name: "revert", err: errors.New("erc8004: register tx: execution reverted"), want: false}, + // registerWithRecovery pins and reuses the same nonce across a + // broadcast-timeout retry, so these three responses mean no new tx + // was accepted (no double-mint risk) — they must be transient so the + // retry loop keeps giving recoverRegistrationByOwnerAndURI time to + // see the original broadcast land, instead of reporting an + // already-landed mint as failed. + {name: "already known", err: errors.New("already known"), want: true}, + {name: "nonce too low", err: errors.New("nonce too low"), want: true}, + {name: "replacement underpriced", err: errors.New("replacement transaction underpriced"), want: true}, + {name: "nonce error uppercase variant", err: errors.New("Nonce Too Low"), want: true}, } for _, tt := range tests { diff --git a/docs/guides/monetize-inference.md b/docs/guides/monetize-inference.md index 70b794bf..7cc6a85d 100644 --- a/docs/guides/monetize-inference.md +++ b/docs/guides/monetize-inference.md @@ -156,6 +156,12 @@ obol sell http my-qwen \ --port 11434 ``` +Ollama doesn't serve `/health` or `/health/readiness` — only `/` returns +2xx — so `obol sell http` automatically defaults `--health-path` to `/` when +`--upstream ollama` is left at its default `/health`. No extra flag is +needed for the example above; pass `--health-path` yourself only if you're +fronting a different health endpoint. + By default this also registers the seller agent on ERC-8004. Use `--no-register` only for local or private-only testing where on-chain discovery is intentionally skipped. diff --git a/flows/flow-04-agent.sh b/flows/flow-04-agent.sh index 94c75d2c..70b0406d 100755 --- a/flows/flow-04-agent.sh +++ b/flows/flow-04-agent.sh @@ -249,8 +249,8 @@ for i in $(seq 1 15); do dash_root=$(dash_code "$HERMES_DASHBOARD_URL/") dash_root_final=$(dash_code_follow "$HERMES_DASHBOARD_URL/") if [ "$dash_status" = "200" ] && [ "$dash_protected" = "401" ] && \ - [ "$dash_login" != "500" ] && [ -n "$dash_login" ] && [ "$dash_login" != "000" ] && \ - [ "$dash_root" != "500" ] && [ -n "$dash_root" ] && [ "$dash_root" != "000" ] && \ + { [ "$dash_login" = "200" ] || [ "$dash_login" = "401" ]; } && \ + [ "$dash_root" = "302" ] && \ [ "$dash_root_final" != "500" ] && [ -n "$dash_root_final" ] && [ "$dash_root_final" != "000" ]; then pass "Hermes dashboard up + auth-gated + root/login ok (status=$dash_status protected=$dash_protected login=$dash_login root=$dash_root final=$dash_root_final)" break @@ -260,11 +260,11 @@ done if [ "$dash_status" != "200" ] || [ "$dash_protected" != "401" ]; then fail "Hermes dashboard check failed — status=$dash_status protected=$dash_protected" fi -if [ -z "$dash_login" ] || [ "$dash_login" = "000" ] || [ "$dash_login" = "500" ]; then - fail "Hermes dashboard password-login path failed — expected non-500 HTML login page, got HTTP $dash_login" +if [ "$dash_login" != "200" ] && [ "$dash_login" != "401" ]; then + fail "Hermes dashboard password-login path failed — expected HTTP 200 or 401, got HTTP $dash_login" fi -if [ -z "$dash_root" ] || [ "$dash_root" = "000" ] || [ "$dash_root" = "500" ]; then - fail "Hermes dashboard root failed — expected edge 302 (or non-500) to password-login, got HTTP $dash_root" +if [ "$dash_root" != "302" ]; then + fail "Hermes dashboard root failed — expected edge 302 redirect to password-login, got HTTP $dash_root" fi if [ -z "$dash_root_final" ] || [ "$dash_root_final" = "000" ] || [ "$dash_root_final" = "500" ]; then fail "Hermes dashboard root follow failed — expected non-500 after redirect, got HTTP $dash_root_final" diff --git a/flows/flow-06-sell-setup.sh b/flows/flow-06-sell-setup.sh index 11a03e50..5e662c6b 100755 --- a/flows/flow-06-sell-setup.sh +++ b/flows/flow-06-sell-setup.sh @@ -15,9 +15,17 @@ else SELL_MODEL_RUNTIME="${SELL_MODEL_RUNTIME:-ollama}" fi +# LiteLLM: unauthenticated readiness probe. /health requires the master key +# (401) and fails UpstreamHealthy under the 2xx-only probe gate. Ollama +# serves neither /health nor /health/readiness — only "/" returns 2xx — +# matching sell.go's --upstream ollama default. +if [ "$SELL_UPSTREAM_SERVICE" = "ollama" ]; then + SELL_HEALTH_PATH="${SELL_HEALTH_PATH:-/}" +else + SELL_HEALTH_PATH="${SELL_HEALTH_PATH:-/health/readiness}" +fi + apply_flow_qwen_inference_offer() { - # LiteLLM: unauthenticated readiness probe. /health requires the master - # key (401) and fails UpstreamHealthy under the 2xx-only probe gate. "$OBOL" sell http flow-qwen --namespace llm --from-json - <