fix(sell inference): emit registration spec + honor operator overrides - #485

Closed
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration
Closed

fix(sell inference): emit registration spec + honor operator overrides#485
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Summary

obol sell inference produced ServiceOffers that were silently missing the ERC-8004 registration block, so /.well-known/agent-registration.json was never routed for inference-typed sells. Surfaced today on spark2 while probing https://inference.v1337.org/.well-known/agent-registration.json — got the Traefik fall-through 404 with a Next.js HTML body. Manual kubectl patch enabled registration but exposed a second, controller-side defect where any operator-supplied description was clobbered on the way out.

Four related defects in one cohesive fix:

#LayerWhat
Acmd/obol/sell.go sellInferenceCommandNo --register-* flags exposed
Bcmd/obol/sell.go buildInferenceServiceOfferSpecNever wrote spec.registration; hardcoded spec.model.name="ollama" regardless of --model
Ccmd/obol/sell.go buildInferenceServiceOfferSpec call siteNo plumbing from the operator's flags into the spec
Dinternal/serviceoffercontroller/render.gobuildActiveRegistrationDocument unconditionally overwrote operator Spec.Registration.Description for inference offers

Test coverage that would have caught this

Adding tests was the asked-for half of the work — each one pins a specific defect so a regression has to fight every layer.

  • TestSellInference_Flags now requires --no-register, --register-name, --register-description, --register-image, --register-skills, --register-domains, --register-metadata. Their absence on main was defect A.
  • TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault — defaults produce spec.registration.enabled = true and name = <offer name>.
  • TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration--no-register opt-out path.
  • TestBuildInferenceServiceOfferSpec_OperatorOverridesWin — operator-supplied name/description/image/skills/domains all survive into spec.registration verbatim.
  • TestBuildInferenceServiceOfferSpec_ModelNameNotHardcodedspec.model.name reflects --model, not the historical "ollama" literal.
  • TestBuildActiveRegistrationDocument_KeepsOperatorDescription — controller-side: operator description survives into the published AgentRegistration document (defect D).
  • TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference — the other branch: empty operator description on inference offers still gets the model-aware default, not the generic one.

Run:

$ go test ./cmd/obol ./internal/serviceoffercontroller -count=1
ok cmd/obol 3.624s
ok internal/serviceoffercontroller 10.259s

Reverse-validation that the tests catch the bugs

Each new test maps to a specific line that was wrong on main:

  • The flag-list assertion fails before adding cmd/obol/sell.go:188216 (the new flag block).
  • The _RegistrationEnabledByDefault / _OperatorOverridesWin tests fail before changing buildInferenceServiceOfferSpec's signature to accept the registration map.
  • The _ModelNameNotHardcoded test fails on the name: "ollama" literal at the old line.
  • The controller test fails on the if owner.IsInference() && owner.Spec.Model.Name != "" unconditional overwrite at render.go:589.

Drive-by

TestSellInference_Flags was already failing on main after #470 removed the --price default but didn't update the assertStringDefault(t, flags, "price", "0.001") assertion. Bumped to assert "" with a comment explaining the post-#470 contract. Not the headline fix but rolling it in keeps make test green.

Notes for the reviewer

  • The rename sellHTTPRegistrationInput → sellRegistrationInput (and the matching helper) is mechanical and contained: only sell.go + sell_test.go reference these names. Did it because the helper is now shared between inference and http call sites.
  • Live end-to-end validation on spark2 was attempted; the dev k3d cluster had wound down between checks. The unit-test coverage above pins the three layers independently; happy to re-do the live test against a refreshed cluster if reviewers want belt-and-suspenders.

Generated with Claude Code

bussyjd added 3 commits May 12, 2026 17:31
`obol sell inference` was producing ServiceOffers with three latent
defects that together stopped /.well-known/agent-registration.json
from ever being routed for inference-typed sells:
1. The inference subcommand never exposed `--register-*` flags
(compare with `obol sell http` which does), even though the help
text on `--register` says "registration is enabled by default".
2. `buildInferenceServiceOfferSpec` never wrote `spec.registration`
onto the offer at all, so the controller's
`reconcileRegistrationStatus` saw `Enabled=false` (zero value) and
emitted `Registered=True Disabled` with no RegistrationRequest CR
and no /.well-known HTTPRoute.
3. `buildInferenceServiceOfferSpec` hardcoded
`spec.model.name = "ollama"` regardless of the actual `--model`
value, so anything downstream that keyed off the model id (the
controller's description default included) was looking at the
wrong string.
Surfaced today on spark2 while trying to fetch
`https://inference.v1337.org/.well-known/agent-registration.json` —
got the Traefik fall-through 404. Manual `kubectl patch` enabled
registration, exposed a fourth defect:
4. `buildActiveRegistrationDocument` in the serviceoffer-controller
unconditionally overwrote `Spec.Registration.Description` for
inference offers with `"<model.name> inference via x402
micropayments"`, even when the operator had supplied an explicit
description at sell time.
This PR fixes all four:
- Add `--no-register`, `--register-name`, `--register-description`,
`--register-image`, `--register-skills`, `--register-domains`,
`--register-metadata` to `obol sell inference`.
- Rename `sellHTTPRegistrationInput` / `buildSellHTTPRegistrationConfig`
to the unqualified `sellRegistrationInput` /
`buildSellRegistrationConfig` since they now serve both inference
and http call sites.
- Extend `buildInferenceServiceOfferSpec` to accept the resolved
model name and the registration block, write
`spec.model.name = <real model id>`, and merge the registration
block into `spec.registration` when non-empty.
- In the controller's `buildActiveRegistrationDocument`, only fall
back to the model-aware description string when the operator left
`Spec.Registration.Description` empty.
Tests that would have caught the regression earlier:
- `TestSellInference_Flags` now requires the six registration flags;
their absence on `main` was the bug.
- `TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault`
pins that defaults produce `spec.registration.enabled = true` and
the offer name as `spec.registration.name`.
- `TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration`
pins the `--no-register` opt-out.
- `TestBuildInferenceServiceOfferSpec_OperatorOverridesWin` pins
that operator-supplied name/description/image/skills/domains all
survive into the spec verbatim.
- `TestBuildInferenceServiceOfferSpec_ModelNameNotHardcoded` pins
that `spec.model.name` reflects `--model`, not the historical
"ollama" literal.
- `TestBuildActiveRegistrationDocument_KeepsOperatorDescription`
pins the controller-side fix: an operator description survives
the buildActiveRegistrationDocument pass.
- `TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference`
pins the other branch — inference offers with no operator
description still get the model-aware default, not the generic
one.
Drive-by: `TestSellInference_Flags` was failing on `main` after #470
removed the `--price` default but didn't update the corresponding
assertion. Updated to assert `--price` default = "" with a comment
explaining the contract.
The first revision of #485 stopped at "produce a ServiceOffer with a
populated spec.registration block". That made
/.well-known/agent-registration.json get routed, but left the offer in
Registered=False AwaitingExternalRegistration until someone manually
ran `obol sell register`. The serviceoffer-controller's storefront
filter (buildServiceCatalogJSON in render.go) requires Ready=True,
which transitively requires Registered=True, so the offer was silently
excluded from /api/services.json — the very feed the operator's own
storefront UI consumes.
obol sell http already auto-registers on the same code path. Mirror
that here so the inference path reaches the same end state without a
follow-up obol sell register step.
Changes:
- Extract shouldAutoRegisterSell(spec, tunnelURL) as the shared
decision predicate. The same gate now drives both the http and
inference auto-register call sites; defensively returns false when
the registration block is missing, disabled, malformed, or the
tunnel URL is empty.
- sellInferenceCommand Action: after kubectlApply + EnsureTunnelForSell
succeed, if shouldAutoRegisterSell says yes, call
autoRegisterServiceOffer with the resolved name/description/wallet
pulled from the spec. Surface failures as warnings + a re-run hint
rather than aborting the gateway start, because the underlying call
needs gas on the target chain and we do not want a one-off RPC
hiccup to block local dev.
Test coverage that would have caught the regression:
- TestShouldAutoRegisterSell - table-driven over six scenarios
including the defensive cases (registration not a map,
registration.enabled not a bool). Both call sites use the helper.
- TestSellInferenceAction_InvokesAutoRegister - source-level guard
that scans the sellInferenceCommand body for shouldAutoRegisterSell
and autoRegisterServiceOffer. The bug we just fixed was "Action
calls neither"; an innocent refactor could remove the calls without
any unit-level signal otherwise.
Operational note: the agent's remote-signer wallet needs a small ETH
balance on the target chain (~0.20-0.50 USD typical) for the on-chain
register tx. If the wallet is unfunded the warning fires and the
offer stays in AwaitingExternalRegistration; the operator can fund
the wallet and re-run obol sell register to finish.
…straint)
The autoRegisterServiceOffer pre-flight check rejected any registration
whose signer didn't match the offer's payTo wallet:
registration signer 0xA... does not match the payment wallet 0xB...
Use a matching signer, omit --wallet so the remote-signer wallet is
used, or pass --no-register
The error wording read like an ERC-8004 limitation but isn't. ERC-8004
treats the agent OWNER (msg.sender at register time) and the agent
WALLET (settable post-mint via setAgentWallet) as independent
addresses. x402 settlement honors the offer's spec.payment.payTo
directly — buyers pay that address regardless of what the registry's
getAgentWallet returns. The "hot signer, cold/multisig payee" split is
the canonical pattern.
The historic guard existed because the obol CLI never exposed
setAgentWallet, so a mismatched registration left operators with no
in-CLI recovery path. This change instead surfaces the split as an
informational note + adds `obol sell update <name> --pay-to <new>` as
the recovery surface (already in tree; just needed test coverage and
the connection wired into the diagnostic).
Changes:
- signerPayeeDelegationNote(signer, payTo) returns a human-readable
note when the two diverge (case-insensitive, whitespace-tolerant,
empty on either side) and "" otherwise. Used by
autoRegisterServiceOffer instead of the previous early-return.
- buildSellUpdatePatch(payTo, chain, price) extracted from the inline
sellUpdateCommand Action so the patch shape — the thing that
actually hits the cluster — is testable without a live offer.
Action calls the helper instead of inlining the same logic.
Tests:
- TestSignerPayeeDelegationNote — 6-case table: match,
case-insensitive, whitespace, empty payTo, empty signer (defensive),
true mismatch (assertions name the addresses + advise sell update).
- TestAutoRegister_AllowsSignerPayeeMismatch — source-level guard
asserting the banned error wording is gone from
autoRegisterServiceOffer and the soft-notice path is wired. Anyone
re-introducing the check has to delete this test too, which forces
them to read the rationale.
- TestBuildSellUpdatePatch_PayToOnly — `obol sell update <name>
--pay-to 0xBooB` builds a patch that touches only
spec.payment.payTo, not network or price.
- TestBuildSellUpdatePatch_PriceSwitchNullsOldKeys — table over
perRequest/perMTok/perHour: the unused keys are explicitly null so
a switch (e.g. perRequest → perMTok) doesn't leave the previous key
fighting through merge semantics.
- TestBuildSellUpdatePatch_NoFieldsErrors — error fires when no
fields are set, and the error names the flags the operator should
pass.
- TestSellUpdate_PayToFlagSurface — `obol sell update` exposes
--pay-to (with --wallet/--recipient/-w aliases via payToFlag), and
--namespace is Required.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Superseded by #487 — all three commits (4708407, b92b78e, dbf6c89) folded into the umbrella branch verbatim, plus the resume + storefront work that grew out of them. Reviewing #487 reviews these changes too.

@bussyjdbussyjd closed this May 12, 2026
@OisinKyne
OisinKyne deleted the fix/sell-inference-registration branch July 1, 2026 12:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(sell inference): emit registration spec + honor operator overrides - #485

Closed
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration
Closed

fix(sell inference): emit registration spec + honor operator overrides#485
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Summary

obol sell inference produced ServiceOffers that were silently missing the ERC-8004 registration block, so /.well-known/agent-registration.json was never routed for inference-typed sells. Surfaced today on spark2 while probing https://inference.v1337.org/.well-known/agent-registration.json — got the Traefik fall-through 404 with a Next.js HTML body. Manual kubectl patch enabled registration but exposed a second, controller-side defect where any operator-supplied description was clobbered on the way out.

Four related defects in one cohesive fix:

#LayerWhat
Acmd/obol/sell.go sellInferenceCommandNo --register-* flags exposed
Bcmd/obol/sell.go buildInferenceServiceOfferSpecNever wrote spec.registration; hardcoded spec.model.name="ollama" regardless of --model
Ccmd/obol/sell.go buildInferenceServiceOfferSpec call siteNo plumbing from the operator's flags into the spec
Dinternal/serviceoffercontroller/render.gobuildActiveRegistrationDocument unconditionally overwrote operator Spec.Registration.Description for inference offers

Test coverage that would have caught this

Adding tests was the asked-for half of the work — each one pins a specific defect so a regression has to fight every layer.

  • TestSellInference_Flags now requires --no-register, --register-name, --register-description, --register-image, --register-skills, --register-domains, --register-metadata. Their absence on main was defect A.
  • TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault — defaults produce spec.registration.enabled = true and name = <offer name>.
  • TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration--no-register opt-out path.
  • TestBuildInferenceServiceOfferSpec_OperatorOverridesWin — operator-supplied name/description/image/skills/domains all survive into spec.registration verbatim.
  • TestBuildInferenceServiceOfferSpec_ModelNameNotHardcodedspec.model.name reflects --model, not the historical "ollama" literal.
  • TestBuildActiveRegistrationDocument_KeepsOperatorDescription — controller-side: operator description survives into the published AgentRegistration document (defect D).
  • TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference — the other branch: empty operator description on inference offers still gets the model-aware default, not the generic one.

Run:

$ go test ./cmd/obol ./internal/serviceoffercontroller -count=1
ok cmd/obol 3.624s
ok internal/serviceoffercontroller 10.259s

Reverse-validation that the tests catch the bugs

Each new test maps to a specific line that was wrong on main:

  • The flag-list assertion fails before adding cmd/obol/sell.go:188216 (the new flag block).
  • The _RegistrationEnabledByDefault / _OperatorOverridesWin tests fail before changing buildInferenceServiceOfferSpec's signature to accept the registration map.
  • The _ModelNameNotHardcoded test fails on the name: "ollama" literal at the old line.
  • The controller test fails on the if owner.IsInference() && owner.Spec.Model.Name != "" unconditional overwrite at render.go:589.

Drive-by

TestSellInference_Flags was already failing on main after #470 removed the --price default but didn't update the assertStringDefault(t, flags, "price", "0.001") assertion. Bumped to assert "" with a comment explaining the post-#470 contract. Not the headline fix but rolling it in keeps make test green.

Notes for the reviewer

  • The rename sellHTTPRegistrationInput → sellRegistrationInput (and the matching helper) is mechanical and contained: only sell.go + sell_test.go reference these names. Did it because the helper is now shared between inference and http call sites.
  • Live end-to-end validation on spark2 was attempted; the dev k3d cluster had wound down between checks. The unit-test coverage above pins the three layers independently; happy to re-do the live test against a refreshed cluster if reviewers want belt-and-suspenders.

Generated with Claude Code

bussyjd added 3 commits May 12, 2026 17:31
`obol sell inference` was producing ServiceOffers with three latent
defects that together stopped /.well-known/agent-registration.json
from ever being routed for inference-typed sells:
1. The inference subcommand never exposed `--register-*` flags
(compare with `obol sell http` which does), even though the help
text on `--register` says "registration is enabled by default".
2. `buildInferenceServiceOfferSpec` never wrote `spec.registration`
onto the offer at all, so the controller's
`reconcileRegistrationStatus` saw `Enabled=false` (zero value) and
emitted `Registered=True Disabled` with no RegistrationRequest CR
and no /.well-known HTTPRoute.
3. `buildInferenceServiceOfferSpec` hardcoded
`spec.model.name = "ollama"` regardless of the actual `--model`
value, so anything downstream that keyed off the model id (the
controller's description default included) was looking at the
wrong string.
Surfaced today on spark2 while trying to fetch
`https://inference.v1337.org/.well-known/agent-registration.json` —
got the Traefik fall-through 404. Manual `kubectl patch` enabled
registration, exposed a fourth defect:
4. `buildActiveRegistrationDocument` in the serviceoffer-controller
unconditionally overwrote `Spec.Registration.Description` for
inference offers with `"<model.name> inference via x402
micropayments"`, even when the operator had supplied an explicit
description at sell time.
This PR fixes all four:
- Add `--no-register`, `--register-name`, `--register-description`,
`--register-image`, `--register-skills`, `--register-domains`,
`--register-metadata` to `obol sell inference`.
- Rename `sellHTTPRegistrationInput` / `buildSellHTTPRegistrationConfig`
to the unqualified `sellRegistrationInput` /
`buildSellRegistrationConfig` since they now serve both inference
and http call sites.
- Extend `buildInferenceServiceOfferSpec` to accept the resolved
model name and the registration block, write
`spec.model.name = <real model id>`, and merge the registration
block into `spec.registration` when non-empty.
- In the controller's `buildActiveRegistrationDocument`, only fall
back to the model-aware description string when the operator left
`Spec.Registration.Description` empty.
Tests that would have caught the regression earlier:
- `TestSellInference_Flags` now requires the six registration flags;
their absence on `main` was the bug.
- `TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault`
pins that defaults produce `spec.registration.enabled = true` and
the offer name as `spec.registration.name`.
- `TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration`
pins the `--no-register` opt-out.
- `TestBuildInferenceServiceOfferSpec_OperatorOverridesWin` pins
that operator-supplied name/description/image/skills/domains all
survive into the spec verbatim.
- `TestBuildInferenceServiceOfferSpec_ModelNameNotHardcoded` pins
that `spec.model.name` reflects `--model`, not the historical
"ollama" literal.
- `TestBuildActiveRegistrationDocument_KeepsOperatorDescription`
pins the controller-side fix: an operator description survives
the buildActiveRegistrationDocument pass.
- `TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference`
pins the other branch — inference offers with no operator
description still get the model-aware default, not the generic
one.
Drive-by: `TestSellInference_Flags` was failing on `main` after #470
removed the `--price` default but didn't update the corresponding
assertion. Updated to assert `--price` default = "" with a comment
explaining the contract.
The first revision of #485 stopped at "produce a ServiceOffer with a
populated spec.registration block". That made
/.well-known/agent-registration.json get routed, but left the offer in
Registered=False AwaitingExternalRegistration until someone manually
ran `obol sell register`. The serviceoffer-controller's storefront
filter (buildServiceCatalogJSON in render.go) requires Ready=True,
which transitively requires Registered=True, so the offer was silently
excluded from /api/services.json — the very feed the operator's own
storefront UI consumes.
obol sell http already auto-registers on the same code path. Mirror
that here so the inference path reaches the same end state without a
follow-up obol sell register step.
Changes:
- Extract shouldAutoRegisterSell(spec, tunnelURL) as the shared
decision predicate. The same gate now drives both the http and
inference auto-register call sites; defensively returns false when
the registration block is missing, disabled, malformed, or the
tunnel URL is empty.
- sellInferenceCommand Action: after kubectlApply + EnsureTunnelForSell
succeed, if shouldAutoRegisterSell says yes, call
autoRegisterServiceOffer with the resolved name/description/wallet
pulled from the spec. Surface failures as warnings + a re-run hint
rather than aborting the gateway start, because the underlying call
needs gas on the target chain and we do not want a one-off RPC
hiccup to block local dev.
Test coverage that would have caught the regression:
- TestShouldAutoRegisterSell - table-driven over six scenarios
including the defensive cases (registration not a map,
registration.enabled not a bool). Both call sites use the helper.
- TestSellInferenceAction_InvokesAutoRegister - source-level guard
that scans the sellInferenceCommand body for shouldAutoRegisterSell
and autoRegisterServiceOffer. The bug we just fixed was "Action
calls neither"; an innocent refactor could remove the calls without
any unit-level signal otherwise.
Operational note: the agent's remote-signer wallet needs a small ETH
balance on the target chain (~0.20-0.50 USD typical) for the on-chain
register tx. If the wallet is unfunded the warning fires and the
offer stays in AwaitingExternalRegistration; the operator can fund
the wallet and re-run obol sell register to finish.
…straint)
The autoRegisterServiceOffer pre-flight check rejected any registration
whose signer didn't match the offer's payTo wallet:
registration signer 0xA... does not match the payment wallet 0xB...
Use a matching signer, omit --wallet so the remote-signer wallet is
used, or pass --no-register
The error wording read like an ERC-8004 limitation but isn't. ERC-8004
treats the agent OWNER (msg.sender at register time) and the agent
WALLET (settable post-mint via setAgentWallet) as independent
addresses. x402 settlement honors the offer's spec.payment.payTo
directly — buyers pay that address regardless of what the registry's
getAgentWallet returns. The "hot signer, cold/multisig payee" split is
the canonical pattern.
The historic guard existed because the obol CLI never exposed
setAgentWallet, so a mismatched registration left operators with no
in-CLI recovery path. This change instead surfaces the split as an
informational note + adds `obol sell update <name> --pay-to <new>` as
the recovery surface (already in tree; just needed test coverage and
the connection wired into the diagnostic).
Changes:
- signerPayeeDelegationNote(signer, payTo) returns a human-readable
note when the two diverge (case-insensitive, whitespace-tolerant,
empty on either side) and "" otherwise. Used by
autoRegisterServiceOffer instead of the previous early-return.
- buildSellUpdatePatch(payTo, chain, price) extracted from the inline
sellUpdateCommand Action so the patch shape — the thing that
actually hits the cluster — is testable without a live offer.
Action calls the helper instead of inlining the same logic.
Tests:
- TestSignerPayeeDelegationNote — 6-case table: match,
case-insensitive, whitespace, empty payTo, empty signer (defensive),
true mismatch (assertions name the addresses + advise sell update).
- TestAutoRegister_AllowsSignerPayeeMismatch — source-level guard
asserting the banned error wording is gone from
autoRegisterServiceOffer and the soft-notice path is wired. Anyone
re-introducing the check has to delete this test too, which forces
them to read the rationale.
- TestBuildSellUpdatePatch_PayToOnly — `obol sell update <name>
--pay-to 0xBooB` builds a patch that touches only
spec.payment.payTo, not network or price.
- TestBuildSellUpdatePatch_PriceSwitchNullsOldKeys — table over
perRequest/perMTok/perHour: the unused keys are explicitly null so
a switch (e.g. perRequest → perMTok) doesn't leave the previous key
fighting through merge semantics.
- TestBuildSellUpdatePatch_NoFieldsErrors — error fires when no
fields are set, and the error names the flags the operator should
pass.
- TestSellUpdate_PayToFlagSurface — `obol sell update` exposes
--pay-to (with --wallet/--recipient/-w aliases via payToFlag), and
--namespace is Required.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Superseded by #487 — all three commits (4708407, b92b78e, dbf6c89) folded into the umbrella branch verbatim, plus the resume + storefront work that grew out of them. Reviewing #487 reviews these changes too.

@bussyjdbussyjd closed this May 12, 2026
@OisinKyne
OisinKyne deleted the fix/sell-inference-registration branch July 1, 2026 12:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(sell inference): emit registration spec + honor operator overrides - #485

Closed
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration
Closed

fix(sell inference): emit registration spec + honor operator overrides#485
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Summary

obol sell inference produced ServiceOffers that were silently missing the ERC-8004 registration block, so /.well-known/agent-registration.json was never routed for inference-typed sells. Surfaced today on spark2 while probing https://inference.v1337.org/.well-known/agent-registration.json — got the Traefik fall-through 404 with a Next.js HTML body. Manual kubectl patch enabled registration but exposed a second, controller-side defect where any operator-supplied description was clobbered on the way out.

Four related defects in one cohesive fix:

#LayerWhat
Acmd/obol/sell.go sellInferenceCommandNo --register-* flags exposed
Bcmd/obol/sell.go buildInferenceServiceOfferSpecNever wrote spec.registration; hardcoded spec.model.name="ollama" regardless of --model
Ccmd/obol/sell.go buildInferenceServiceOfferSpec call siteNo plumbing from the operator's flags into the spec
Dinternal/serviceoffercontroller/render.gobuildActiveRegistrationDocument unconditionally overwrote operator Spec.Registration.Description for inference offers

Test coverage that would have caught this

Adding tests was the asked-for half of the work — each one pins a specific defect so a regression has to fight every layer.

  • TestSellInference_Flags now requires --no-register, --register-name, --register-description, --register-image, --register-skills, --register-domains, --register-metadata. Their absence on main was defect A.
  • TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault — defaults produce spec.registration.enabled = true and name = <offer name>.
  • TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration--no-register opt-out path.
  • TestBuildInferenceServiceOfferSpec_OperatorOverridesWin — operator-supplied name/description/image/skills/domains all survive into spec.registration verbatim.
  • TestBuildInferenceServiceOfferSpec_ModelNameNotHardcodedspec.model.name reflects --model, not the historical "ollama" literal.
  • TestBuildActiveRegistrationDocument_KeepsOperatorDescription — controller-side: operator description survives into the published AgentRegistration document (defect D).
  • TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference — the other branch: empty operator description on inference offers still gets the model-aware default, not the generic one.

Run:

$ go test ./cmd/obol ./internal/serviceoffercontroller -count=1
ok cmd/obol 3.624s
ok internal/serviceoffercontroller 10.259s

Reverse-validation that the tests catch the bugs

Each new test maps to a specific line that was wrong on main:

  • The flag-list assertion fails before adding cmd/obol/sell.go:188216 (the new flag block).
  • The _RegistrationEnabledByDefault / _OperatorOverridesWin tests fail before changing buildInferenceServiceOfferSpec's signature to accept the registration map.
  • The _ModelNameNotHardcoded test fails on the name: "ollama" literal at the old line.
  • The controller test fails on the if owner.IsInference() && owner.Spec.Model.Name != "" unconditional overwrite at render.go:589.

Drive-by

TestSellInference_Flags was already failing on main after #470 removed the --price default but didn't update the assertStringDefault(t, flags, "price", "0.001") assertion. Bumped to assert "" with a comment explaining the post-#470 contract. Not the headline fix but rolling it in keeps make test green.

Notes for the reviewer

  • The rename sellHTTPRegistrationInput → sellRegistrationInput (and the matching helper) is mechanical and contained: only sell.go + sell_test.go reference these names. Did it because the helper is now shared between inference and http call sites.
  • Live end-to-end validation on spark2 was attempted; the dev k3d cluster had wound down between checks. The unit-test coverage above pins the three layers independently; happy to re-do the live test against a refreshed cluster if reviewers want belt-and-suspenders.

Generated with Claude Code

bussyjd added 3 commits May 12, 2026 17:31
`obol sell inference` was producing ServiceOffers with three latent
defects that together stopped /.well-known/agent-registration.json
from ever being routed for inference-typed sells:
1. The inference subcommand never exposed `--register-*` flags
(compare with `obol sell http` which does), even though the help
text on `--register` says "registration is enabled by default".
2. `buildInferenceServiceOfferSpec` never wrote `spec.registration`
onto the offer at all, so the controller's
`reconcileRegistrationStatus` saw `Enabled=false` (zero value) and
emitted `Registered=True Disabled` with no RegistrationRequest CR
and no /.well-known HTTPRoute.
3. `buildInferenceServiceOfferSpec` hardcoded
`spec.model.name = "ollama"` regardless of the actual `--model`
value, so anything downstream that keyed off the model id (the
controller's description default included) was looking at the
wrong string.
Surfaced today on spark2 while trying to fetch
`https://inference.v1337.org/.well-known/agent-registration.json` —
got the Traefik fall-through 404. Manual `kubectl patch` enabled
registration, exposed a fourth defect:
4. `buildActiveRegistrationDocument` in the serviceoffer-controller
unconditionally overwrote `Spec.Registration.Description` for
inference offers with `"<model.name> inference via x402
micropayments"`, even when the operator had supplied an explicit
description at sell time.
This PR fixes all four:
- Add `--no-register`, `--register-name`, `--register-description`,
`--register-image`, `--register-skills`, `--register-domains`,
`--register-metadata` to `obol sell inference`.
- Rename `sellHTTPRegistrationInput` / `buildSellHTTPRegistrationConfig`
to the unqualified `sellRegistrationInput` /
`buildSellRegistrationConfig` since they now serve both inference
and http call sites.
- Extend `buildInferenceServiceOfferSpec` to accept the resolved
model name and the registration block, write
`spec.model.name = <real model id>`, and merge the registration
block into `spec.registration` when non-empty.
- In the controller's `buildActiveRegistrationDocument`, only fall
back to the model-aware description string when the operator left
`Spec.Registration.Description` empty.
Tests that would have caught the regression earlier:
- `TestSellInference_Flags` now requires the six registration flags;
their absence on `main` was the bug.
- `TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault`
pins that defaults produce `spec.registration.enabled = true` and
the offer name as `spec.registration.name`.
- `TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration`
pins the `--no-register` opt-out.
- `TestBuildInferenceServiceOfferSpec_OperatorOverridesWin` pins
that operator-supplied name/description/image/skills/domains all
survive into the spec verbatim.
- `TestBuildInferenceServiceOfferSpec_ModelNameNotHardcoded` pins
that `spec.model.name` reflects `--model`, not the historical
"ollama" literal.
- `TestBuildActiveRegistrationDocument_KeepsOperatorDescription`
pins the controller-side fix: an operator description survives
the buildActiveRegistrationDocument pass.
- `TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference`
pins the other branch — inference offers with no operator
description still get the model-aware default, not the generic
one.
Drive-by: `TestSellInference_Flags` was failing on `main` after #470
removed the `--price` default but didn't update the corresponding
assertion. Updated to assert `--price` default = "" with a comment
explaining the contract.
The first revision of #485 stopped at "produce a ServiceOffer with a
populated spec.registration block". That made
/.well-known/agent-registration.json get routed, but left the offer in
Registered=False AwaitingExternalRegistration until someone manually
ran `obol sell register`. The serviceoffer-controller's storefront
filter (buildServiceCatalogJSON in render.go) requires Ready=True,
which transitively requires Registered=True, so the offer was silently
excluded from /api/services.json — the very feed the operator's own
storefront UI consumes.
obol sell http already auto-registers on the same code path. Mirror
that here so the inference path reaches the same end state without a
follow-up obol sell register step.
Changes:
- Extract shouldAutoRegisterSell(spec, tunnelURL) as the shared
decision predicate. The same gate now drives both the http and
inference auto-register call sites; defensively returns false when
the registration block is missing, disabled, malformed, or the
tunnel URL is empty.
- sellInferenceCommand Action: after kubectlApply + EnsureTunnelForSell
succeed, if shouldAutoRegisterSell says yes, call
autoRegisterServiceOffer with the resolved name/description/wallet
pulled from the spec. Surface failures as warnings + a re-run hint
rather than aborting the gateway start, because the underlying call
needs gas on the target chain and we do not want a one-off RPC
hiccup to block local dev.
Test coverage that would have caught the regression:
- TestShouldAutoRegisterSell - table-driven over six scenarios
including the defensive cases (registration not a map,
registration.enabled not a bool). Both call sites use the helper.
- TestSellInferenceAction_InvokesAutoRegister - source-level guard
that scans the sellInferenceCommand body for shouldAutoRegisterSell
and autoRegisterServiceOffer. The bug we just fixed was "Action
calls neither"; an innocent refactor could remove the calls without
any unit-level signal otherwise.
Operational note: the agent's remote-signer wallet needs a small ETH
balance on the target chain (~0.20-0.50 USD typical) for the on-chain
register tx. If the wallet is unfunded the warning fires and the
offer stays in AwaitingExternalRegistration; the operator can fund
the wallet and re-run obol sell register to finish.
…straint)
The autoRegisterServiceOffer pre-flight check rejected any registration
whose signer didn't match the offer's payTo wallet:
registration signer 0xA... does not match the payment wallet 0xB...
Use a matching signer, omit --wallet so the remote-signer wallet is
used, or pass --no-register
The error wording read like an ERC-8004 limitation but isn't. ERC-8004
treats the agent OWNER (msg.sender at register time) and the agent
WALLET (settable post-mint via setAgentWallet) as independent
addresses. x402 settlement honors the offer's spec.payment.payTo
directly — buyers pay that address regardless of what the registry's
getAgentWallet returns. The "hot signer, cold/multisig payee" split is
the canonical pattern.
The historic guard existed because the obol CLI never exposed
setAgentWallet, so a mismatched registration left operators with no
in-CLI recovery path. This change instead surfaces the split as an
informational note + adds `obol sell update <name> --pay-to <new>` as
the recovery surface (already in tree; just needed test coverage and
the connection wired into the diagnostic).
Changes:
- signerPayeeDelegationNote(signer, payTo) returns a human-readable
note when the two diverge (case-insensitive, whitespace-tolerant,
empty on either side) and "" otherwise. Used by
autoRegisterServiceOffer instead of the previous early-return.
- buildSellUpdatePatch(payTo, chain, price) extracted from the inline
sellUpdateCommand Action so the patch shape — the thing that
actually hits the cluster — is testable without a live offer.
Action calls the helper instead of inlining the same logic.
Tests:
- TestSignerPayeeDelegationNote — 6-case table: match,
case-insensitive, whitespace, empty payTo, empty signer (defensive),
true mismatch (assertions name the addresses + advise sell update).
- TestAutoRegister_AllowsSignerPayeeMismatch — source-level guard
asserting the banned error wording is gone from
autoRegisterServiceOffer and the soft-notice path is wired. Anyone
re-introducing the check has to delete this test too, which forces
them to read the rationale.
- TestBuildSellUpdatePatch_PayToOnly — `obol sell update <name>
--pay-to 0xBooB` builds a patch that touches only
spec.payment.payTo, not network or price.
- TestBuildSellUpdatePatch_PriceSwitchNullsOldKeys — table over
perRequest/perMTok/perHour: the unused keys are explicitly null so
a switch (e.g. perRequest → perMTok) doesn't leave the previous key
fighting through merge semantics.
- TestBuildSellUpdatePatch_NoFieldsErrors — error fires when no
fields are set, and the error names the flags the operator should
pass.
- TestSellUpdate_PayToFlagSurface — `obol sell update` exposes
--pay-to (with --wallet/--recipient/-w aliases via payToFlag), and
--namespace is Required.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Superseded by #487 — all three commits (4708407, b92b78e, dbf6c89) folded into the umbrella branch verbatim, plus the resume + storefront work that grew out of them. Reviewing #487 reviews these changes too.

@bussyjdbussyjd closed this May 12, 2026
@OisinKyne
OisinKyne deleted the fix/sell-inference-registration branch July 1, 2026 12:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(sell inference): emit registration spec + honor operator overrides - #485

Closed
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration
Closed

fix(sell inference): emit registration spec + honor operator overrides#485
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Summary

obol sell inference produced ServiceOffers that were silently missing the ERC-8004 registration block, so /.well-known/agent-registration.json was never routed for inference-typed sells. Surfaced today on spark2 while probing https://inference.v1337.org/.well-known/agent-registration.json — got the Traefik fall-through 404 with a Next.js HTML body. Manual kubectl patch enabled registration but exposed a second, controller-side defect where any operator-supplied description was clobbered on the way out.

Four related defects in one cohesive fix:

#LayerWhat
Acmd/obol/sell.go sellInferenceCommandNo --register-* flags exposed
Bcmd/obol/sell.go buildInferenceServiceOfferSpecNever wrote spec.registration; hardcoded spec.model.name="ollama" regardless of --model
Ccmd/obol/sell.go buildInferenceServiceOfferSpec call siteNo plumbing from the operator's flags into the spec
Dinternal/serviceoffercontroller/render.gobuildActiveRegistrationDocument unconditionally overwrote operator Spec.Registration.Description for inference offers

Test coverage that would have caught this

Adding tests was the asked-for half of the work — each one pins a specific defect so a regression has to fight every layer.

  • TestSellInference_Flags now requires --no-register, --register-name, --register-description, --register-image, --register-skills, --register-domains, --register-metadata. Their absence on main was defect A.
  • TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault — defaults produce spec.registration.enabled = true and name = <offer name>.
  • TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration--no-register opt-out path.
  • TestBuildInferenceServiceOfferSpec_OperatorOverridesWin — operator-supplied name/description/image/skills/domains all survive into spec.registration verbatim.
  • TestBuildInferenceServiceOfferSpec_ModelNameNotHardcodedspec.model.name reflects --model, not the historical "ollama" literal.
  • TestBuildActiveRegistrationDocument_KeepsOperatorDescription — controller-side: operator description survives into the published AgentRegistration document (defect D).
  • TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference — the other branch: empty operator description on inference offers still gets the model-aware default, not the generic one.

Run:

$ go test ./cmd/obol ./internal/serviceoffercontroller -count=1
ok cmd/obol 3.624s
ok internal/serviceoffercontroller 10.259s

Reverse-validation that the tests catch the bugs

Each new test maps to a specific line that was wrong on main:

  • The flag-list assertion fails before adding cmd/obol/sell.go:188216 (the new flag block).
  • The _RegistrationEnabledByDefault / _OperatorOverridesWin tests fail before changing buildInferenceServiceOfferSpec's signature to accept the registration map.
  • The _ModelNameNotHardcoded test fails on the name: "ollama" literal at the old line.
  • The controller test fails on the if owner.IsInference() && owner.Spec.Model.Name != "" unconditional overwrite at render.go:589.

Drive-by

TestSellInference_Flags was already failing on main after #470 removed the --price default but didn't update the assertStringDefault(t, flags, "price", "0.001") assertion. Bumped to assert "" with a comment explaining the post-#470 contract. Not the headline fix but rolling it in keeps make test green.

Notes for the reviewer

  • The rename sellHTTPRegistrationInput → sellRegistrationInput (and the matching helper) is mechanical and contained: only sell.go + sell_test.go reference these names. Did it because the helper is now shared between inference and http call sites.
  • Live end-to-end validation on spark2 was attempted; the dev k3d cluster had wound down between checks. The unit-test coverage above pins the three layers independently; happy to re-do the live test against a refreshed cluster if reviewers want belt-and-suspenders.

Generated with Claude Code

bussyjd added 3 commits May 12, 2026 17:31
`obol sell inference` was producing ServiceOffers with three latent
defects that together stopped /.well-known/agent-registration.json
from ever being routed for inference-typed sells:
1. The inference subcommand never exposed `--register-*` flags
(compare with `obol sell http` which does), even though the help
text on `--register` says "registration is enabled by default".
2. `buildInferenceServiceOfferSpec` never wrote `spec.registration`
onto the offer at all, so the controller's
`reconcileRegistrationStatus` saw `Enabled=false` (zero value) and
emitted `Registered=True Disabled` with no RegistrationRequest CR
and no /.well-known HTTPRoute.
3. `buildInferenceServiceOfferSpec` hardcoded
`spec.model.name = "ollama"` regardless of the actual `--model`
value, so anything downstream that keyed off the model id (the
controller's description default included) was looking at the
wrong string.
Surfaced today on spark2 while trying to fetch
`https://inference.v1337.org/.well-known/agent-registration.json` —
got the Traefik fall-through 404. Manual `kubectl patch` enabled
registration, exposed a fourth defect:
4. `buildActiveRegistrationDocument` in the serviceoffer-controller
unconditionally overwrote `Spec.Registration.Description` for
inference offers with `"<model.name> inference via x402
micropayments"`, even when the operator had supplied an explicit
description at sell time.
This PR fixes all four:
- Add `--no-register`, `--register-name`, `--register-description`,
`--register-image`, `--register-skills`, `--register-domains`,
`--register-metadata` to `obol sell inference`.
- Rename `sellHTTPRegistrationInput` / `buildSellHTTPRegistrationConfig`
to the unqualified `sellRegistrationInput` /
`buildSellRegistrationConfig` since they now serve both inference
and http call sites.
- Extend `buildInferenceServiceOfferSpec` to accept the resolved
model name and the registration block, write
`spec.model.name = <real model id>`, and merge the registration
block into `spec.registration` when non-empty.
- In the controller's `buildActiveRegistrationDocument`, only fall
back to the model-aware description string when the operator left
`Spec.Registration.Description` empty.
Tests that would have caught the regression earlier:
- `TestSellInference_Flags` now requires the six registration flags;
their absence on `main` was the bug.
- `TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault`
pins that defaults produce `spec.registration.enabled = true` and
the offer name as `spec.registration.name`.
- `TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration`
pins the `--no-register` opt-out.
- `TestBuildInferenceServiceOfferSpec_OperatorOverridesWin` pins
that operator-supplied name/description/image/skills/domains all
survive into the spec verbatim.
- `TestBuildInferenceServiceOfferSpec_ModelNameNotHardcoded` pins
that `spec.model.name` reflects `--model`, not the historical
"ollama" literal.
- `TestBuildActiveRegistrationDocument_KeepsOperatorDescription`
pins the controller-side fix: an operator description survives
the buildActiveRegistrationDocument pass.
- `TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference`
pins the other branch — inference offers with no operator
description still get the model-aware default, not the generic
one.
Drive-by: `TestSellInference_Flags` was failing on `main` after #470
removed the `--price` default but didn't update the corresponding
assertion. Updated to assert `--price` default = "" with a comment
explaining the contract.
The first revision of #485 stopped at "produce a ServiceOffer with a
populated spec.registration block". That made
/.well-known/agent-registration.json get routed, but left the offer in
Registered=False AwaitingExternalRegistration until someone manually
ran `obol sell register`. The serviceoffer-controller's storefront
filter (buildServiceCatalogJSON in render.go) requires Ready=True,
which transitively requires Registered=True, so the offer was silently
excluded from /api/services.json — the very feed the operator's own
storefront UI consumes.
obol sell http already auto-registers on the same code path. Mirror
that here so the inference path reaches the same end state without a
follow-up obol sell register step.
Changes:
- Extract shouldAutoRegisterSell(spec, tunnelURL) as the shared
decision predicate. The same gate now drives both the http and
inference auto-register call sites; defensively returns false when
the registration block is missing, disabled, malformed, or the
tunnel URL is empty.
- sellInferenceCommand Action: after kubectlApply + EnsureTunnelForSell
succeed, if shouldAutoRegisterSell says yes, call
autoRegisterServiceOffer with the resolved name/description/wallet
pulled from the spec. Surface failures as warnings + a re-run hint
rather than aborting the gateway start, because the underlying call
needs gas on the target chain and we do not want a one-off RPC
hiccup to block local dev.
Test coverage that would have caught the regression:
- TestShouldAutoRegisterSell - table-driven over six scenarios
including the defensive cases (registration not a map,
registration.enabled not a bool). Both call sites use the helper.
- TestSellInferenceAction_InvokesAutoRegister - source-level guard
that scans the sellInferenceCommand body for shouldAutoRegisterSell
and autoRegisterServiceOffer. The bug we just fixed was "Action
calls neither"; an innocent refactor could remove the calls without
any unit-level signal otherwise.
Operational note: the agent's remote-signer wallet needs a small ETH
balance on the target chain (~0.20-0.50 USD typical) for the on-chain
register tx. If the wallet is unfunded the warning fires and the
offer stays in AwaitingExternalRegistration; the operator can fund
the wallet and re-run obol sell register to finish.
…straint)
The autoRegisterServiceOffer pre-flight check rejected any registration
whose signer didn't match the offer's payTo wallet:
registration signer 0xA... does not match the payment wallet 0xB...
Use a matching signer, omit --wallet so the remote-signer wallet is
used, or pass --no-register
The error wording read like an ERC-8004 limitation but isn't. ERC-8004
treats the agent OWNER (msg.sender at register time) and the agent
WALLET (settable post-mint via setAgentWallet) as independent
addresses. x402 settlement honors the offer's spec.payment.payTo
directly — buyers pay that address regardless of what the registry's
getAgentWallet returns. The "hot signer, cold/multisig payee" split is
the canonical pattern.
The historic guard existed because the obol CLI never exposed
setAgentWallet, so a mismatched registration left operators with no
in-CLI recovery path. This change instead surfaces the split as an
informational note + adds `obol sell update <name> --pay-to <new>` as
the recovery surface (already in tree; just needed test coverage and
the connection wired into the diagnostic).
Changes:
- signerPayeeDelegationNote(signer, payTo) returns a human-readable
note when the two diverge (case-insensitive, whitespace-tolerant,
empty on either side) and "" otherwise. Used by
autoRegisterServiceOffer instead of the previous early-return.
- buildSellUpdatePatch(payTo, chain, price) extracted from the inline
sellUpdateCommand Action so the patch shape — the thing that
actually hits the cluster — is testable without a live offer.
Action calls the helper instead of inlining the same logic.
Tests:
- TestSignerPayeeDelegationNote — 6-case table: match,
case-insensitive, whitespace, empty payTo, empty signer (defensive),
true mismatch (assertions name the addresses + advise sell update).
- TestAutoRegister_AllowsSignerPayeeMismatch — source-level guard
asserting the banned error wording is gone from
autoRegisterServiceOffer and the soft-notice path is wired. Anyone
re-introducing the check has to delete this test too, which forces
them to read the rationale.
- TestBuildSellUpdatePatch_PayToOnly — `obol sell update <name>
--pay-to 0xBooB` builds a patch that touches only
spec.payment.payTo, not network or price.
- TestBuildSellUpdatePatch_PriceSwitchNullsOldKeys — table over
perRequest/perMTok/perHour: the unused keys are explicitly null so
a switch (e.g. perRequest → perMTok) doesn't leave the previous key
fighting through merge semantics.
- TestBuildSellUpdatePatch_NoFieldsErrors — error fires when no
fields are set, and the error names the flags the operator should
pass.
- TestSellUpdate_PayToFlagSurface — `obol sell update` exposes
--pay-to (with --wallet/--recipient/-w aliases via payToFlag), and
--namespace is Required.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Superseded by #487 — all three commits (4708407, b92b78e, dbf6c89) folded into the umbrella branch verbatim, plus the resume + storefront work that grew out of them. Reviewing #487 reviews these changes too.

@bussyjdbussyjd closed this May 12, 2026
@OisinKyne
OisinKyne deleted the fix/sell-inference-registration branch July 1, 2026 12:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(sell inference): emit registration spec + honor operator overrides - #485

Closed
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration
Closed

fix(sell inference): emit registration spec + honor operator overrides#485
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Summary

obol sell inference produced ServiceOffers that were silently missing the ERC-8004 registration block, so /.well-known/agent-registration.json was never routed for inference-typed sells. Surfaced today on spark2 while probing https://inference.v1337.org/.well-known/agent-registration.json — got the Traefik fall-through 404 with a Next.js HTML body. Manual kubectl patch enabled registration but exposed a second, controller-side defect where any operator-supplied description was clobbered on the way out.

Four related defects in one cohesive fix:

#LayerWhat
Acmd/obol/sell.go sellInferenceCommandNo --register-* flags exposed
Bcmd/obol/sell.go buildInferenceServiceOfferSpecNever wrote spec.registration; hardcoded spec.model.name="ollama" regardless of --model
Ccmd/obol/sell.go buildInferenceServiceOfferSpec call siteNo plumbing from the operator's flags into the spec
Dinternal/serviceoffercontroller/render.gobuildActiveRegistrationDocument unconditionally overwrote operator Spec.Registration.Description for inference offers

Test coverage that would have caught this

Adding tests was the asked-for half of the work — each one pins a specific defect so a regression has to fight every layer.

  • TestSellInference_Flags now requires --no-register, --register-name, --register-description, --register-image, --register-skills, --register-domains, --register-metadata. Their absence on main was defect A.
  • TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault — defaults produce spec.registration.enabled = true and name = <offer name>.
  • TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration--no-register opt-out path.
  • TestBuildInferenceServiceOfferSpec_OperatorOverridesWin — operator-supplied name/description/image/skills/domains all survive into spec.registration verbatim.
  • TestBuildInferenceServiceOfferSpec_ModelNameNotHardcodedspec.model.name reflects --model, not the historical "ollama" literal.
  • TestBuildActiveRegistrationDocument_KeepsOperatorDescription — controller-side: operator description survives into the published AgentRegistration document (defect D).
  • TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference — the other branch: empty operator description on inference offers still gets the model-aware default, not the generic one.

Run:

$ go test ./cmd/obol ./internal/serviceoffercontroller -count=1
ok cmd/obol 3.624s
ok internal/serviceoffercontroller 10.259s

Reverse-validation that the tests catch the bugs

Each new test maps to a specific line that was wrong on main:

  • The flag-list assertion fails before adding cmd/obol/sell.go:188216 (the new flag block).
  • The _RegistrationEnabledByDefault / _OperatorOverridesWin tests fail before changing buildInferenceServiceOfferSpec's signature to accept the registration map.
  • The _ModelNameNotHardcoded test fails on the name: "ollama" literal at the old line.
  • The controller test fails on the if owner.IsInference() && owner.Spec.Model.Name != "" unconditional overwrite at render.go:589.

Drive-by

TestSellInference_Flags was already failing on main after #470 removed the --price default but didn't update the assertStringDefault(t, flags, "price", "0.001") assertion. Bumped to assert "" with a comment explaining the post-#470 contract. Not the headline fix but rolling it in keeps make test green.

Notes for the reviewer

  • The rename sellHTTPRegistrationInput → sellRegistrationInput (and the matching helper) is mechanical and contained: only sell.go + sell_test.go reference these names. Did it because the helper is now shared between inference and http call sites.
  • Live end-to-end validation on spark2 was attempted; the dev k3d cluster had wound down between checks. The unit-test coverage above pins the three layers independently; happy to re-do the live test against a refreshed cluster if reviewers want belt-and-suspenders.

Generated with Claude Code

bussyjd added 3 commits May 12, 2026 17:31
`obol sell inference` was producing ServiceOffers with three latent
defects that together stopped /.well-known/agent-registration.json
from ever being routed for inference-typed sells:
1. The inference subcommand never exposed `--register-*` flags
(compare with `obol sell http` which does), even though the help
text on `--register` says "registration is enabled by default".
2. `buildInferenceServiceOfferSpec` never wrote `spec.registration`
onto the offer at all, so the controller's
`reconcileRegistrationStatus` saw `Enabled=false` (zero value) and
emitted `Registered=True Disabled` with no RegistrationRequest CR
and no /.well-known HTTPRoute.
3. `buildInferenceServiceOfferSpec` hardcoded
`spec.model.name = "ollama"` regardless of the actual `--model`
value, so anything downstream that keyed off the model id (the
controller's description default included) was looking at the
wrong string.
Surfaced today on spark2 while trying to fetch
`https://inference.v1337.org/.well-known/agent-registration.json` —
got the Traefik fall-through 404. Manual `kubectl patch` enabled
registration, exposed a fourth defect:
4. `buildActiveRegistrationDocument` in the serviceoffer-controller
unconditionally overwrote `Spec.Registration.Description` for
inference offers with `"<model.name> inference via x402
micropayments"`, even when the operator had supplied an explicit
description at sell time.
This PR fixes all four:
- Add `--no-register`, `--register-name`, `--register-description`,
`--register-image`, `--register-skills`, `--register-domains`,
`--register-metadata` to `obol sell inference`.
- Rename `sellHTTPRegistrationInput` / `buildSellHTTPRegistrationConfig`
to the unqualified `sellRegistrationInput` /
`buildSellRegistrationConfig` since they now serve both inference
and http call sites.
- Extend `buildInferenceServiceOfferSpec` to accept the resolved
model name and the registration block, write
`spec.model.name = <real model id>`, and merge the registration
block into `spec.registration` when non-empty.
- In the controller's `buildActiveRegistrationDocument`, only fall
back to the model-aware description string when the operator left
`Spec.Registration.Description` empty.
Tests that would have caught the regression earlier:
- `TestSellInference_Flags` now requires the six registration flags;
their absence on `main` was the bug.
- `TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault`
pins that defaults produce `spec.registration.enabled = true` and
the offer name as `spec.registration.name`.
- `TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration`
pins the `--no-register` opt-out.
- `TestBuildInferenceServiceOfferSpec_OperatorOverridesWin` pins
that operator-supplied name/description/image/skills/domains all
survive into the spec verbatim.
- `TestBuildInferenceServiceOfferSpec_ModelNameNotHardcoded` pins
that `spec.model.name` reflects `--model`, not the historical
"ollama" literal.
- `TestBuildActiveRegistrationDocument_KeepsOperatorDescription`
pins the controller-side fix: an operator description survives
the buildActiveRegistrationDocument pass.
- `TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference`
pins the other branch — inference offers with no operator
description still get the model-aware default, not the generic
one.
Drive-by: `TestSellInference_Flags` was failing on `main` after #470
removed the `--price` default but didn't update the corresponding
assertion. Updated to assert `--price` default = "" with a comment
explaining the contract.
The first revision of #485 stopped at "produce a ServiceOffer with a
populated spec.registration block". That made
/.well-known/agent-registration.json get routed, but left the offer in
Registered=False AwaitingExternalRegistration until someone manually
ran `obol sell register`. The serviceoffer-controller's storefront
filter (buildServiceCatalogJSON in render.go) requires Ready=True,
which transitively requires Registered=True, so the offer was silently
excluded from /api/services.json — the very feed the operator's own
storefront UI consumes.
obol sell http already auto-registers on the same code path. Mirror
that here so the inference path reaches the same end state without a
follow-up obol sell register step.
Changes:
- Extract shouldAutoRegisterSell(spec, tunnelURL) as the shared
decision predicate. The same gate now drives both the http and
inference auto-register call sites; defensively returns false when
the registration block is missing, disabled, malformed, or the
tunnel URL is empty.
- sellInferenceCommand Action: after kubectlApply + EnsureTunnelForSell
succeed, if shouldAutoRegisterSell says yes, call
autoRegisterServiceOffer with the resolved name/description/wallet
pulled from the spec. Surface failures as warnings + a re-run hint
rather than aborting the gateway start, because the underlying call
needs gas on the target chain and we do not want a one-off RPC
hiccup to block local dev.
Test coverage that would have caught the regression:
- TestShouldAutoRegisterSell - table-driven over six scenarios
including the defensive cases (registration not a map,
registration.enabled not a bool). Both call sites use the helper.
- TestSellInferenceAction_InvokesAutoRegister - source-level guard
that scans the sellInferenceCommand body for shouldAutoRegisterSell
and autoRegisterServiceOffer. The bug we just fixed was "Action
calls neither"; an innocent refactor could remove the calls without
any unit-level signal otherwise.
Operational note: the agent's remote-signer wallet needs a small ETH
balance on the target chain (~0.20-0.50 USD typical) for the on-chain
register tx. If the wallet is unfunded the warning fires and the
offer stays in AwaitingExternalRegistration; the operator can fund
the wallet and re-run obol sell register to finish.
…straint)
The autoRegisterServiceOffer pre-flight check rejected any registration
whose signer didn't match the offer's payTo wallet:
registration signer 0xA... does not match the payment wallet 0xB...
Use a matching signer, omit --wallet so the remote-signer wallet is
used, or pass --no-register
The error wording read like an ERC-8004 limitation but isn't. ERC-8004
treats the agent OWNER (msg.sender at register time) and the agent
WALLET (settable post-mint via setAgentWallet) as independent
addresses. x402 settlement honors the offer's spec.payment.payTo
directly — buyers pay that address regardless of what the registry's
getAgentWallet returns. The "hot signer, cold/multisig payee" split is
the canonical pattern.
The historic guard existed because the obol CLI never exposed
setAgentWallet, so a mismatched registration left operators with no
in-CLI recovery path. This change instead surfaces the split as an
informational note + adds `obol sell update <name> --pay-to <new>` as
the recovery surface (already in tree; just needed test coverage and
the connection wired into the diagnostic).
Changes:
- signerPayeeDelegationNote(signer, payTo) returns a human-readable
note when the two diverge (case-insensitive, whitespace-tolerant,
empty on either side) and "" otherwise. Used by
autoRegisterServiceOffer instead of the previous early-return.
- buildSellUpdatePatch(payTo, chain, price) extracted from the inline
sellUpdateCommand Action so the patch shape — the thing that
actually hits the cluster — is testable without a live offer.
Action calls the helper instead of inlining the same logic.
Tests:
- TestSignerPayeeDelegationNote — 6-case table: match,
case-insensitive, whitespace, empty payTo, empty signer (defensive),
true mismatch (assertions name the addresses + advise sell update).
- TestAutoRegister_AllowsSignerPayeeMismatch — source-level guard
asserting the banned error wording is gone from
autoRegisterServiceOffer and the soft-notice path is wired. Anyone
re-introducing the check has to delete this test too, which forces
them to read the rationale.
- TestBuildSellUpdatePatch_PayToOnly — `obol sell update <name>
--pay-to 0xBooB` builds a patch that touches only
spec.payment.payTo, not network or price.
- TestBuildSellUpdatePatch_PriceSwitchNullsOldKeys — table over
perRequest/perMTok/perHour: the unused keys are explicitly null so
a switch (e.g. perRequest → perMTok) doesn't leave the previous key
fighting through merge semantics.
- TestBuildSellUpdatePatch_NoFieldsErrors — error fires when no
fields are set, and the error names the flags the operator should
pass.
- TestSellUpdate_PayToFlagSurface — `obol sell update` exposes
--pay-to (with --wallet/--recipient/-w aliases via payToFlag), and
--namespace is Required.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Superseded by #487 — all three commits (4708407, b92b78e, dbf6c89) folded into the umbrella branch verbatim, plus the resume + storefront work that grew out of them. Reviewing #487 reviews these changes too.

@bussyjdbussyjd closed this May 12, 2026
@OisinKyne
OisinKyne deleted the fix/sell-inference-registration branch July 1, 2026 12:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(sell inference): emit registration spec + honor operator overrides - #485

Closed
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration
Closed

fix(sell inference): emit registration spec + honor operator overrides#485
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Summary

obol sell inference produced ServiceOffers that were silently missing the ERC-8004 registration block, so /.well-known/agent-registration.json was never routed for inference-typed sells. Surfaced today on spark2 while probing https://inference.v1337.org/.well-known/agent-registration.json — got the Traefik fall-through 404 with a Next.js HTML body. Manual kubectl patch enabled registration but exposed a second, controller-side defect where any operator-supplied description was clobbered on the way out.

Four related defects in one cohesive fix:

#LayerWhat
Acmd/obol/sell.go sellInferenceCommandNo --register-* flags exposed
Bcmd/obol/sell.go buildInferenceServiceOfferSpecNever wrote spec.registration; hardcoded spec.model.name="ollama" regardless of --model
Ccmd/obol/sell.go buildInferenceServiceOfferSpec call siteNo plumbing from the operator's flags into the spec
Dinternal/serviceoffercontroller/render.gobuildActiveRegistrationDocument unconditionally overwrote operator Spec.Registration.Description for inference offers

Test coverage that would have caught this

Adding tests was the asked-for half of the work — each one pins a specific defect so a regression has to fight every layer.

  • TestSellInference_Flags now requires --no-register, --register-name, --register-description, --register-image, --register-skills, --register-domains, --register-metadata. Their absence on main was defect A.
  • TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault — defaults produce spec.registration.enabled = true and name = <offer name>.
  • TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration--no-register opt-out path.
  • TestBuildInferenceServiceOfferSpec_OperatorOverridesWin — operator-supplied name/description/image/skills/domains all survive into spec.registration verbatim.
  • TestBuildInferenceServiceOfferSpec_ModelNameNotHardcodedspec.model.name reflects --model, not the historical "ollama" literal.
  • TestBuildActiveRegistrationDocument_KeepsOperatorDescription — controller-side: operator description survives into the published AgentRegistration document (defect D).
  • TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference — the other branch: empty operator description on inference offers still gets the model-aware default, not the generic one.

Run:

$ go test ./cmd/obol ./internal/serviceoffercontroller -count=1
ok cmd/obol 3.624s
ok internal/serviceoffercontroller 10.259s

Reverse-validation that the tests catch the bugs

Each new test maps to a specific line that was wrong on main:

  • The flag-list assertion fails before adding cmd/obol/sell.go:188216 (the new flag block).
  • The _RegistrationEnabledByDefault / _OperatorOverridesWin tests fail before changing buildInferenceServiceOfferSpec's signature to accept the registration map.
  • The _ModelNameNotHardcoded test fails on the name: "ollama" literal at the old line.
  • The controller test fails on the if owner.IsInference() && owner.Spec.Model.Name != "" unconditional overwrite at render.go:589.

Drive-by

TestSellInference_Flags was already failing on main after #470 removed the --price default but didn't update the assertStringDefault(t, flags, "price", "0.001") assertion. Bumped to assert "" with a comment explaining the post-#470 contract. Not the headline fix but rolling it in keeps make test green.

Notes for the reviewer

  • The rename sellHTTPRegistrationInput → sellRegistrationInput (and the matching helper) is mechanical and contained: only sell.go + sell_test.go reference these names. Did it because the helper is now shared between inference and http call sites.
  • Live end-to-end validation on spark2 was attempted; the dev k3d cluster had wound down between checks. The unit-test coverage above pins the three layers independently; happy to re-do the live test against a refreshed cluster if reviewers want belt-and-suspenders.

Generated with Claude Code

bussyjd added 3 commits May 12, 2026 17:31
`obol sell inference` was producing ServiceOffers with three latent
defects that together stopped /.well-known/agent-registration.json
from ever being routed for inference-typed sells:
1. The inference subcommand never exposed `--register-*` flags
(compare with `obol sell http` which does), even though the help
text on `--register` says "registration is enabled by default".
2. `buildInferenceServiceOfferSpec` never wrote `spec.registration`
onto the offer at all, so the controller's
`reconcileRegistrationStatus` saw `Enabled=false` (zero value) and
emitted `Registered=True Disabled` with no RegistrationRequest CR
and no /.well-known HTTPRoute.
3. `buildInferenceServiceOfferSpec` hardcoded
`spec.model.name = "ollama"` regardless of the actual `--model`
value, so anything downstream that keyed off the model id (the
controller's description default included) was looking at the
wrong string.
Surfaced today on spark2 while trying to fetch
`https://inference.v1337.org/.well-known/agent-registration.json` —
got the Traefik fall-through 404. Manual `kubectl patch` enabled
registration, exposed a fourth defect:
4. `buildActiveRegistrationDocument` in the serviceoffer-controller
unconditionally overwrote `Spec.Registration.Description` for
inference offers with `"<model.name> inference via x402
micropayments"`, even when the operator had supplied an explicit
description at sell time.
This PR fixes all four:
- Add `--no-register`, `--register-name`, `--register-description`,
`--register-image`, `--register-skills`, `--register-domains`,
`--register-metadata` to `obol sell inference`.
- Rename `sellHTTPRegistrationInput` / `buildSellHTTPRegistrationConfig`
to the unqualified `sellRegistrationInput` /
`buildSellRegistrationConfig` since they now serve both inference
and http call sites.
- Extend `buildInferenceServiceOfferSpec` to accept the resolved
model name and the registration block, write
`spec.model.name = <real model id>`, and merge the registration
block into `spec.registration` when non-empty.
- In the controller's `buildActiveRegistrationDocument`, only fall
back to the model-aware description string when the operator left
`Spec.Registration.Description` empty.
Tests that would have caught the regression earlier:
- `TestSellInference_Flags` now requires the six registration flags;
their absence on `main` was the bug.
- `TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault`
pins that defaults produce `spec.registration.enabled = true` and
the offer name as `spec.registration.name`.
- `TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration`
pins the `--no-register` opt-out.
- `TestBuildInferenceServiceOfferSpec_OperatorOverridesWin` pins
that operator-supplied name/description/image/skills/domains all
survive into the spec verbatim.
- `TestBuildInferenceServiceOfferSpec_ModelNameNotHardcoded` pins
that `spec.model.name` reflects `--model`, not the historical
"ollama" literal.
- `TestBuildActiveRegistrationDocument_KeepsOperatorDescription`
pins the controller-side fix: an operator description survives
the buildActiveRegistrationDocument pass.
- `TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference`
pins the other branch — inference offers with no operator
description still get the model-aware default, not the generic
one.
Drive-by: `TestSellInference_Flags` was failing on `main` after #470
removed the `--price` default but didn't update the corresponding
assertion. Updated to assert `--price` default = "" with a comment
explaining the contract.
The first revision of #485 stopped at "produce a ServiceOffer with a
populated spec.registration block". That made
/.well-known/agent-registration.json get routed, but left the offer in
Registered=False AwaitingExternalRegistration until someone manually
ran `obol sell register`. The serviceoffer-controller's storefront
filter (buildServiceCatalogJSON in render.go) requires Ready=True,
which transitively requires Registered=True, so the offer was silently
excluded from /api/services.json — the very feed the operator's own
storefront UI consumes.
obol sell http already auto-registers on the same code path. Mirror
that here so the inference path reaches the same end state without a
follow-up obol sell register step.
Changes:
- Extract shouldAutoRegisterSell(spec, tunnelURL) as the shared
decision predicate. The same gate now drives both the http and
inference auto-register call sites; defensively returns false when
the registration block is missing, disabled, malformed, or the
tunnel URL is empty.
- sellInferenceCommand Action: after kubectlApply + EnsureTunnelForSell
succeed, if shouldAutoRegisterSell says yes, call
autoRegisterServiceOffer with the resolved name/description/wallet
pulled from the spec. Surface failures as warnings + a re-run hint
rather than aborting the gateway start, because the underlying call
needs gas on the target chain and we do not want a one-off RPC
hiccup to block local dev.
Test coverage that would have caught the regression:
- TestShouldAutoRegisterSell - table-driven over six scenarios
including the defensive cases (registration not a map,
registration.enabled not a bool). Both call sites use the helper.
- TestSellInferenceAction_InvokesAutoRegister - source-level guard
that scans the sellInferenceCommand body for shouldAutoRegisterSell
and autoRegisterServiceOffer. The bug we just fixed was "Action
calls neither"; an innocent refactor could remove the calls without
any unit-level signal otherwise.
Operational note: the agent's remote-signer wallet needs a small ETH
balance on the target chain (~0.20-0.50 USD typical) for the on-chain
register tx. If the wallet is unfunded the warning fires and the
offer stays in AwaitingExternalRegistration; the operator can fund
the wallet and re-run obol sell register to finish.
…straint)
The autoRegisterServiceOffer pre-flight check rejected any registration
whose signer didn't match the offer's payTo wallet:
registration signer 0xA... does not match the payment wallet 0xB...
Use a matching signer, omit --wallet so the remote-signer wallet is
used, or pass --no-register
The error wording read like an ERC-8004 limitation but isn't. ERC-8004
treats the agent OWNER (msg.sender at register time) and the agent
WALLET (settable post-mint via setAgentWallet) as independent
addresses. x402 settlement honors the offer's spec.payment.payTo
directly — buyers pay that address regardless of what the registry's
getAgentWallet returns. The "hot signer, cold/multisig payee" split is
the canonical pattern.
The historic guard existed because the obol CLI never exposed
setAgentWallet, so a mismatched registration left operators with no
in-CLI recovery path. This change instead surfaces the split as an
informational note + adds `obol sell update <name> --pay-to <new>` as
the recovery surface (already in tree; just needed test coverage and
the connection wired into the diagnostic).
Changes:
- signerPayeeDelegationNote(signer, payTo) returns a human-readable
note when the two diverge (case-insensitive, whitespace-tolerant,
empty on either side) and "" otherwise. Used by
autoRegisterServiceOffer instead of the previous early-return.
- buildSellUpdatePatch(payTo, chain, price) extracted from the inline
sellUpdateCommand Action so the patch shape — the thing that
actually hits the cluster — is testable without a live offer.
Action calls the helper instead of inlining the same logic.
Tests:
- TestSignerPayeeDelegationNote — 6-case table: match,
case-insensitive, whitespace, empty payTo, empty signer (defensive),
true mismatch (assertions name the addresses + advise sell update).
- TestAutoRegister_AllowsSignerPayeeMismatch — source-level guard
asserting the banned error wording is gone from
autoRegisterServiceOffer and the soft-notice path is wired. Anyone
re-introducing the check has to delete this test too, which forces
them to read the rationale.
- TestBuildSellUpdatePatch_PayToOnly — `obol sell update <name>
--pay-to 0xBooB` builds a patch that touches only
spec.payment.payTo, not network or price.
- TestBuildSellUpdatePatch_PriceSwitchNullsOldKeys — table over
perRequest/perMTok/perHour: the unused keys are explicitly null so
a switch (e.g. perRequest → perMTok) doesn't leave the previous key
fighting through merge semantics.
- TestBuildSellUpdatePatch_NoFieldsErrors — error fires when no
fields are set, and the error names the flags the operator should
pass.
- TestSellUpdate_PayToFlagSurface — `obol sell update` exposes
--pay-to (with --wallet/--recipient/-w aliases via payToFlag), and
--namespace is Required.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Superseded by #487 — all three commits (4708407, b92b78e, dbf6c89) folded into the umbrella branch verbatim, plus the resume + storefront work that grew out of them. Reviewing #487 reviews these changes too.

@bussyjdbussyjd closed this May 12, 2026
@OisinKyne
OisinKyne deleted the fix/sell-inference-registration branch July 1, 2026 12:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(sell inference): emit registration spec + honor operator overrides - #485

Closed
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration
Closed

fix(sell inference): emit registration spec + honor operator overrides#485
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Summary

obol sell inference produced ServiceOffers that were silently missing the ERC-8004 registration block, so /.well-known/agent-registration.json was never routed for inference-typed sells. Surfaced today on spark2 while probing https://inference.v1337.org/.well-known/agent-registration.json — got the Traefik fall-through 404 with a Next.js HTML body. Manual kubectl patch enabled registration but exposed a second, controller-side defect where any operator-supplied description was clobbered on the way out.

Four related defects in one cohesive fix:

#LayerWhat
Acmd/obol/sell.go sellInferenceCommandNo --register-* flags exposed
Bcmd/obol/sell.go buildInferenceServiceOfferSpecNever wrote spec.registration; hardcoded spec.model.name="ollama" regardless of --model
Ccmd/obol/sell.go buildInferenceServiceOfferSpec call siteNo plumbing from the operator's flags into the spec
Dinternal/serviceoffercontroller/render.gobuildActiveRegistrationDocument unconditionally overwrote operator Spec.Registration.Description for inference offers

Test coverage that would have caught this

Adding tests was the asked-for half of the work — each one pins a specific defect so a regression has to fight every layer.

  • TestSellInference_Flags now requires --no-register, --register-name, --register-description, --register-image, --register-skills, --register-domains, --register-metadata. Their absence on main was defect A.
  • TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault — defaults produce spec.registration.enabled = true and name = <offer name>.
  • TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration--no-register opt-out path.
  • TestBuildInferenceServiceOfferSpec_OperatorOverridesWin — operator-supplied name/description/image/skills/domains all survive into spec.registration verbatim.
  • TestBuildInferenceServiceOfferSpec_ModelNameNotHardcodedspec.model.name reflects --model, not the historical "ollama" literal.
  • TestBuildActiveRegistrationDocument_KeepsOperatorDescription — controller-side: operator description survives into the published AgentRegistration document (defect D).
  • TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference — the other branch: empty operator description on inference offers still gets the model-aware default, not the generic one.

Run:

$ go test ./cmd/obol ./internal/serviceoffercontroller -count=1
ok cmd/obol 3.624s
ok internal/serviceoffercontroller 10.259s

Reverse-validation that the tests catch the bugs

Each new test maps to a specific line that was wrong on main:

  • The flag-list assertion fails before adding cmd/obol/sell.go:188216 (the new flag block).
  • The _RegistrationEnabledByDefault / _OperatorOverridesWin tests fail before changing buildInferenceServiceOfferSpec's signature to accept the registration map.
  • The _ModelNameNotHardcoded test fails on the name: "ollama" literal at the old line.
  • The controller test fails on the if owner.IsInference() && owner.Spec.Model.Name != "" unconditional overwrite at render.go:589.

Drive-by

TestSellInference_Flags was already failing on main after #470 removed the --price default but didn't update the assertStringDefault(t, flags, "price", "0.001") assertion. Bumped to assert "" with a comment explaining the post-#470 contract. Not the headline fix but rolling it in keeps make test green.

Notes for the reviewer

  • The rename sellHTTPRegistrationInput → sellRegistrationInput (and the matching helper) is mechanical and contained: only sell.go + sell_test.go reference these names. Did it because the helper is now shared between inference and http call sites.
  • Live end-to-end validation on spark2 was attempted; the dev k3d cluster had wound down between checks. The unit-test coverage above pins the three layers independently; happy to re-do the live test against a refreshed cluster if reviewers want belt-and-suspenders.

Generated with Claude Code

bussyjd added 3 commits May 12, 2026 17:31
`obol sell inference` was producing ServiceOffers with three latent
defects that together stopped /.well-known/agent-registration.json
from ever being routed for inference-typed sells:
1. The inference subcommand never exposed `--register-*` flags
(compare with `obol sell http` which does), even though the help
text on `--register` says "registration is enabled by default".
2. `buildInferenceServiceOfferSpec` never wrote `spec.registration`
onto the offer at all, so the controller's
`reconcileRegistrationStatus` saw `Enabled=false` (zero value) and
emitted `Registered=True Disabled` with no RegistrationRequest CR
and no /.well-known HTTPRoute.
3. `buildInferenceServiceOfferSpec` hardcoded
`spec.model.name = "ollama"` regardless of the actual `--model`
value, so anything downstream that keyed off the model id (the
controller's description default included) was looking at the
wrong string.
Surfaced today on spark2 while trying to fetch
`https://inference.v1337.org/.well-known/agent-registration.json` —
got the Traefik fall-through 404. Manual `kubectl patch` enabled
registration, exposed a fourth defect:
4. `buildActiveRegistrationDocument` in the serviceoffer-controller
unconditionally overwrote `Spec.Registration.Description` for
inference offers with `"<model.name> inference via x402
micropayments"`, even when the operator had supplied an explicit
description at sell time.
This PR fixes all four:
- Add `--no-register`, `--register-name`, `--register-description`,
`--register-image`, `--register-skills`, `--register-domains`,
`--register-metadata` to `obol sell inference`.
- Rename `sellHTTPRegistrationInput` / `buildSellHTTPRegistrationConfig`
to the unqualified `sellRegistrationInput` /
`buildSellRegistrationConfig` since they now serve both inference
and http call sites.
- Extend `buildInferenceServiceOfferSpec` to accept the resolved
model name and the registration block, write
`spec.model.name = <real model id>`, and merge the registration
block into `spec.registration` when non-empty.
- In the controller's `buildActiveRegistrationDocument`, only fall
back to the model-aware description string when the operator left
`Spec.Registration.Description` empty.
Tests that would have caught the regression earlier:
- `TestSellInference_Flags` now requires the six registration flags;
their absence on `main` was the bug.
- `TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault`
pins that defaults produce `spec.registration.enabled = true` and
the offer name as `spec.registration.name`.
- `TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration`
pins the `--no-register` opt-out.
- `TestBuildInferenceServiceOfferSpec_OperatorOverridesWin` pins
that operator-supplied name/description/image/skills/domains all
survive into the spec verbatim.
- `TestBuildInferenceServiceOfferSpec_ModelNameNotHardcoded` pins
that `spec.model.name` reflects `--model`, not the historical
"ollama" literal.
- `TestBuildActiveRegistrationDocument_KeepsOperatorDescription`
pins the controller-side fix: an operator description survives
the buildActiveRegistrationDocument pass.
- `TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference`
pins the other branch — inference offers with no operator
description still get the model-aware default, not the generic
one.
Drive-by: `TestSellInference_Flags` was failing on `main` after #470
removed the `--price` default but didn't update the corresponding
assertion. Updated to assert `--price` default = "" with a comment
explaining the contract.
The first revision of #485 stopped at "produce a ServiceOffer with a
populated spec.registration block". That made
/.well-known/agent-registration.json get routed, but left the offer in
Registered=False AwaitingExternalRegistration until someone manually
ran `obol sell register`. The serviceoffer-controller's storefront
filter (buildServiceCatalogJSON in render.go) requires Ready=True,
which transitively requires Registered=True, so the offer was silently
excluded from /api/services.json — the very feed the operator's own
storefront UI consumes.
obol sell http already auto-registers on the same code path. Mirror
that here so the inference path reaches the same end state without a
follow-up obol sell register step.
Changes:
- Extract shouldAutoRegisterSell(spec, tunnelURL) as the shared
decision predicate. The same gate now drives both the http and
inference auto-register call sites; defensively returns false when
the registration block is missing, disabled, malformed, or the
tunnel URL is empty.
- sellInferenceCommand Action: after kubectlApply + EnsureTunnelForSell
succeed, if shouldAutoRegisterSell says yes, call
autoRegisterServiceOffer with the resolved name/description/wallet
pulled from the spec. Surface failures as warnings + a re-run hint
rather than aborting the gateway start, because the underlying call
needs gas on the target chain and we do not want a one-off RPC
hiccup to block local dev.
Test coverage that would have caught the regression:
- TestShouldAutoRegisterSell - table-driven over six scenarios
including the defensive cases (registration not a map,
registration.enabled not a bool). Both call sites use the helper.
- TestSellInferenceAction_InvokesAutoRegister - source-level guard
that scans the sellInferenceCommand body for shouldAutoRegisterSell
and autoRegisterServiceOffer. The bug we just fixed was "Action
calls neither"; an innocent refactor could remove the calls without
any unit-level signal otherwise.
Operational note: the agent's remote-signer wallet needs a small ETH
balance on the target chain (~0.20-0.50 USD typical) for the on-chain
register tx. If the wallet is unfunded the warning fires and the
offer stays in AwaitingExternalRegistration; the operator can fund
the wallet and re-run obol sell register to finish.
…straint)
The autoRegisterServiceOffer pre-flight check rejected any registration
whose signer didn't match the offer's payTo wallet:
registration signer 0xA... does not match the payment wallet 0xB...
Use a matching signer, omit --wallet so the remote-signer wallet is
used, or pass --no-register
The error wording read like an ERC-8004 limitation but isn't. ERC-8004
treats the agent OWNER (msg.sender at register time) and the agent
WALLET (settable post-mint via setAgentWallet) as independent
addresses. x402 settlement honors the offer's spec.payment.payTo
directly — buyers pay that address regardless of what the registry's
getAgentWallet returns. The "hot signer, cold/multisig payee" split is
the canonical pattern.
The historic guard existed because the obol CLI never exposed
setAgentWallet, so a mismatched registration left operators with no
in-CLI recovery path. This change instead surfaces the split as an
informational note + adds `obol sell update <name> --pay-to <new>` as
the recovery surface (already in tree; just needed test coverage and
the connection wired into the diagnostic).
Changes:
- signerPayeeDelegationNote(signer, payTo) returns a human-readable
note when the two diverge (case-insensitive, whitespace-tolerant,
empty on either side) and "" otherwise. Used by
autoRegisterServiceOffer instead of the previous early-return.
- buildSellUpdatePatch(payTo, chain, price) extracted from the inline
sellUpdateCommand Action so the patch shape — the thing that
actually hits the cluster — is testable without a live offer.
Action calls the helper instead of inlining the same logic.
Tests:
- TestSignerPayeeDelegationNote — 6-case table: match,
case-insensitive, whitespace, empty payTo, empty signer (defensive),
true mismatch (assertions name the addresses + advise sell update).
- TestAutoRegister_AllowsSignerPayeeMismatch — source-level guard
asserting the banned error wording is gone from
autoRegisterServiceOffer and the soft-notice path is wired. Anyone
re-introducing the check has to delete this test too, which forces
them to read the rationale.
- TestBuildSellUpdatePatch_PayToOnly — `obol sell update <name>
--pay-to 0xBooB` builds a patch that touches only
spec.payment.payTo, not network or price.
- TestBuildSellUpdatePatch_PriceSwitchNullsOldKeys — table over
perRequest/perMTok/perHour: the unused keys are explicitly null so
a switch (e.g. perRequest → perMTok) doesn't leave the previous key
fighting through merge semantics.
- TestBuildSellUpdatePatch_NoFieldsErrors — error fires when no
fields are set, and the error names the flags the operator should
pass.
- TestSellUpdate_PayToFlagSurface — `obol sell update` exposes
--pay-to (with --wallet/--recipient/-w aliases via payToFlag), and
--namespace is Required.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Superseded by #487 — all three commits (4708407, b92b78e, dbf6c89) folded into the umbrella branch verbatim, plus the resume + storefront work that grew out of them. Reviewing #487 reviews these changes too.

@bussyjdbussyjd closed this May 12, 2026
@OisinKyne
OisinKyne deleted the fix/sell-inference-registration branch July 1, 2026 12:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(sell inference): emit registration spec + honor operator overrides - #485

Closed
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration
Closed

fix(sell inference): emit registration spec + honor operator overrides#485
bussyjd wants to merge 3 commits into
mainfrom
fix/sell-inference-registration

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Summary

obol sell inference produced ServiceOffers that were silently missing the ERC-8004 registration block, so /.well-known/agent-registration.json was never routed for inference-typed sells. Surfaced today on spark2 while probing https://inference.v1337.org/.well-known/agent-registration.json — got the Traefik fall-through 404 with a Next.js HTML body. Manual kubectl patch enabled registration but exposed a second, controller-side defect where any operator-supplied description was clobbered on the way out.

Four related defects in one cohesive fix:

#LayerWhat
Acmd/obol/sell.go sellInferenceCommandNo --register-* flags exposed
Bcmd/obol/sell.go buildInferenceServiceOfferSpecNever wrote spec.registration; hardcoded spec.model.name="ollama" regardless of --model
Ccmd/obol/sell.go buildInferenceServiceOfferSpec call siteNo plumbing from the operator's flags into the spec
Dinternal/serviceoffercontroller/render.gobuildActiveRegistrationDocument unconditionally overwrote operator Spec.Registration.Description for inference offers

Test coverage that would have caught this

Adding tests was the asked-for half of the work — each one pins a specific defect so a regression has to fight every layer.

  • TestSellInference_Flags now requires --no-register, --register-name, --register-description, --register-image, --register-skills, --register-domains, --register-metadata. Their absence on main was defect A.
  • TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault — defaults produce spec.registration.enabled = true and name = <offer name>.
  • TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration--no-register opt-out path.
  • TestBuildInferenceServiceOfferSpec_OperatorOverridesWin — operator-supplied name/description/image/skills/domains all survive into spec.registration verbatim.
  • TestBuildInferenceServiceOfferSpec_ModelNameNotHardcodedspec.model.name reflects --model, not the historical "ollama" literal.
  • TestBuildActiveRegistrationDocument_KeepsOperatorDescription — controller-side: operator description survives into the published AgentRegistration document (defect D).
  • TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference — the other branch: empty operator description on inference offers still gets the model-aware default, not the generic one.

Run:

$ go test ./cmd/obol ./internal/serviceoffercontroller -count=1
ok cmd/obol 3.624s
ok internal/serviceoffercontroller 10.259s

Reverse-validation that the tests catch the bugs

Each new test maps to a specific line that was wrong on main:

  • The flag-list assertion fails before adding cmd/obol/sell.go:188216 (the new flag block).
  • The _RegistrationEnabledByDefault / _OperatorOverridesWin tests fail before changing buildInferenceServiceOfferSpec's signature to accept the registration map.
  • The _ModelNameNotHardcoded test fails on the name: "ollama" literal at the old line.
  • The controller test fails on the if owner.IsInference() && owner.Spec.Model.Name != "" unconditional overwrite at render.go:589.

Drive-by

TestSellInference_Flags was already failing on main after #470 removed the --price default but didn't update the assertStringDefault(t, flags, "price", "0.001") assertion. Bumped to assert "" with a comment explaining the post-#470 contract. Not the headline fix but rolling it in keeps make test green.

Notes for the reviewer

  • The rename sellHTTPRegistrationInput → sellRegistrationInput (and the matching helper) is mechanical and contained: only sell.go + sell_test.go reference these names. Did it because the helper is now shared between inference and http call sites.
  • Live end-to-end validation on spark2 was attempted; the dev k3d cluster had wound down between checks. The unit-test coverage above pins the three layers independently; happy to re-do the live test against a refreshed cluster if reviewers want belt-and-suspenders.

Generated with Claude Code

bussyjd added 3 commits May 12, 2026 17:31
`obol sell inference` was producing ServiceOffers with three latent
defects that together stopped /.well-known/agent-registration.json
from ever being routed for inference-typed sells:
1. The inference subcommand never exposed `--register-*` flags
(compare with `obol sell http` which does), even though the help
text on `--register` says "registration is enabled by default".
2. `buildInferenceServiceOfferSpec` never wrote `spec.registration`
onto the offer at all, so the controller's
`reconcileRegistrationStatus` saw `Enabled=false` (zero value) and
emitted `Registered=True Disabled` with no RegistrationRequest CR
and no /.well-known HTTPRoute.
3. `buildInferenceServiceOfferSpec` hardcoded
`spec.model.name = "ollama"` regardless of the actual `--model`
value, so anything downstream that keyed off the model id (the
controller's description default included) was looking at the
wrong string.
Surfaced today on spark2 while trying to fetch
`https://inference.v1337.org/.well-known/agent-registration.json` —
got the Traefik fall-through 404. Manual `kubectl patch` enabled
registration, exposed a fourth defect:
4. `buildActiveRegistrationDocument` in the serviceoffer-controller
unconditionally overwrote `Spec.Registration.Description` for
inference offers with `"<model.name> inference via x402
micropayments"`, even when the operator had supplied an explicit
description at sell time.
This PR fixes all four:
- Add `--no-register`, `--register-name`, `--register-description`,
`--register-image`, `--register-skills`, `--register-domains`,
`--register-metadata` to `obol sell inference`.
- Rename `sellHTTPRegistrationInput` / `buildSellHTTPRegistrationConfig`
to the unqualified `sellRegistrationInput` /
`buildSellRegistrationConfig` since they now serve both inference
and http call sites.
- Extend `buildInferenceServiceOfferSpec` to accept the resolved
model name and the registration block, write
`spec.model.name = <real model id>`, and merge the registration
block into `spec.registration` when non-empty.
- In the controller's `buildActiveRegistrationDocument`, only fall
back to the model-aware description string when the operator left
`Spec.Registration.Description` empty.
Tests that would have caught the regression earlier:
- `TestSellInference_Flags` now requires the six registration flags;
their absence on `main` was the bug.
- `TestBuildInferenceServiceOfferSpec_RegistrationEnabledByDefault`
pins that defaults produce `spec.registration.enabled = true` and
the offer name as `spec.registration.name`.
- `TestBuildInferenceServiceOfferSpec_NoRegisterOmitsRegistration`
pins the `--no-register` opt-out.
- `TestBuildInferenceServiceOfferSpec_OperatorOverridesWin` pins
that operator-supplied name/description/image/skills/domains all
survive into the spec verbatim.
- `TestBuildInferenceServiceOfferSpec_ModelNameNotHardcoded` pins
that `spec.model.name` reflects `--model`, not the historical
"ollama" literal.
- `TestBuildActiveRegistrationDocument_KeepsOperatorDescription`
pins the controller-side fix: an operator description survives
the buildActiveRegistrationDocument pass.
- `TestBuildActiveRegistrationDocument_FallsBackToModelDescriptionForInference`
pins the other branch — inference offers with no operator
description still get the model-aware default, not the generic
one.
Drive-by: `TestSellInference_Flags` was failing on `main` after #470
removed the `--price` default but didn't update the corresponding
assertion. Updated to assert `--price` default = "" with a comment
explaining the contract.
The first revision of #485 stopped at "produce a ServiceOffer with a
populated spec.registration block". That made
/.well-known/agent-registration.json get routed, but left the offer in
Registered=False AwaitingExternalRegistration until someone manually
ran `obol sell register`. The serviceoffer-controller's storefront
filter (buildServiceCatalogJSON in render.go) requires Ready=True,
which transitively requires Registered=True, so the offer was silently
excluded from /api/services.json — the very feed the operator's own
storefront UI consumes.
obol sell http already auto-registers on the same code path. Mirror
that here so the inference path reaches the same end state without a
follow-up obol sell register step.
Changes:
- Extract shouldAutoRegisterSell(spec, tunnelURL) as the shared
decision predicate. The same gate now drives both the http and
inference auto-register call sites; defensively returns false when
the registration block is missing, disabled, malformed, or the
tunnel URL is empty.
- sellInferenceCommand Action: after kubectlApply + EnsureTunnelForSell
succeed, if shouldAutoRegisterSell says yes, call
autoRegisterServiceOffer with the resolved name/description/wallet
pulled from the spec. Surface failures as warnings + a re-run hint
rather than aborting the gateway start, because the underlying call
needs gas on the target chain and we do not want a one-off RPC
hiccup to block local dev.
Test coverage that would have caught the regression:
- TestShouldAutoRegisterSell - table-driven over six scenarios
including the defensive cases (registration not a map,
registration.enabled not a bool). Both call sites use the helper.
- TestSellInferenceAction_InvokesAutoRegister - source-level guard
that scans the sellInferenceCommand body for shouldAutoRegisterSell
and autoRegisterServiceOffer. The bug we just fixed was "Action
calls neither"; an innocent refactor could remove the calls without
any unit-level signal otherwise.
Operational note: the agent's remote-signer wallet needs a small ETH
balance on the target chain (~0.20-0.50 USD typical) for the on-chain
register tx. If the wallet is unfunded the warning fires and the
offer stays in AwaitingExternalRegistration; the operator can fund
the wallet and re-run obol sell register to finish.
…straint)
The autoRegisterServiceOffer pre-flight check rejected any registration
whose signer didn't match the offer's payTo wallet:
registration signer 0xA... does not match the payment wallet 0xB...
Use a matching signer, omit --wallet so the remote-signer wallet is
used, or pass --no-register
The error wording read like an ERC-8004 limitation but isn't. ERC-8004
treats the agent OWNER (msg.sender at register time) and the agent
WALLET (settable post-mint via setAgentWallet) as independent
addresses. x402 settlement honors the offer's spec.payment.payTo
directly — buyers pay that address regardless of what the registry's
getAgentWallet returns. The "hot signer, cold/multisig payee" split is
the canonical pattern.
The historic guard existed because the obol CLI never exposed
setAgentWallet, so a mismatched registration left operators with no
in-CLI recovery path. This change instead surfaces the split as an
informational note + adds `obol sell update <name> --pay-to <new>` as
the recovery surface (already in tree; just needed test coverage and
the connection wired into the diagnostic).
Changes:
- signerPayeeDelegationNote(signer, payTo) returns a human-readable
note when the two diverge (case-insensitive, whitespace-tolerant,
empty on either side) and "" otherwise. Used by
autoRegisterServiceOffer instead of the previous early-return.
- buildSellUpdatePatch(payTo, chain, price) extracted from the inline
sellUpdateCommand Action so the patch shape — the thing that
actually hits the cluster — is testable without a live offer.
Action calls the helper instead of inlining the same logic.
Tests:
- TestSignerPayeeDelegationNote — 6-case table: match,
case-insensitive, whitespace, empty payTo, empty signer (defensive),
true mismatch (assertions name the addresses + advise sell update).
- TestAutoRegister_AllowsSignerPayeeMismatch — source-level guard
asserting the banned error wording is gone from
autoRegisterServiceOffer and the soft-notice path is wired. Anyone
re-introducing the check has to delete this test too, which forces
them to read the rationale.
- TestBuildSellUpdatePatch_PayToOnly — `obol sell update <name>
--pay-to 0xBooB` builds a patch that touches only
spec.payment.payTo, not network or price.
- TestBuildSellUpdatePatch_PriceSwitchNullsOldKeys — table over
perRequest/perMTok/perHour: the unused keys are explicitly null so
a switch (e.g. perRequest → perMTok) doesn't leave the previous key
fighting through merge semantics.
- TestBuildSellUpdatePatch_NoFieldsErrors — error fires when no
fields are set, and the error names the flags the operator should
pass.
- TestSellUpdate_PayToFlagSurface — `obol sell update` exposes
--pay-to (with --wallet/--recipient/-w aliases via payToFlag), and
--namespace is Required.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Superseded by #487 — all three commits (4708407, b92b78e, dbf6c89) folded into the umbrella branch verbatim, plus the resume + storefront work that grew out of them. Reviewing #487 reviews these changes too.

@bussyjdbussyjd closed this May 12, 2026
@OisinKyne
OisinKyne deleted the fix/sell-inference-registration branch July 1, 2026 12:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd