From 83f1ec08077e4d1acb1ce92e1a8262e44983f456 Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Wed, 7 Oct 2026 01:42:24 +0800 Subject: [PATCH] =?UTF-8?q?fix(quota):=20=E6=9C=AA=E5=B0=B1=E7=BB=AA?= =?UTF-8?q?=E9=80=82=E9=85=8D=E5=99=A8=E6=B1=A1=E6=9F=93=E8=B4=9F=20memo?= =?UTF-8?q?=EF=BC=9B=E8=A2=AB=E6=8B=92=E5=87=AD=E6=8D=AE=E8=A2=AB=E5=90=8E?= =?UTF-8?q?=E7=BB=AD=20404=20=E6=8E=A9=E7=9B=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 端到端验证时撞出两个真缺陷，都是单元测试全绿、隔离实例打真上游才暴露。 一、负 memo 语义混用 负 memo 说的是「这台 HOST 没有余额端点」，且无条件在每次探测前生效。 openDesign 缺 control_key 说的是「这台部署还没配」—— 两件事共用一个条目： 1. 未配 control_key → 探测 → 负 memo 2. 操作员写入 key → 探测被退避拦下，adapter 根本没跑 3. 结果：修复看起来无效，源持续沉默（退避窗口可达数小时） 不能靠「skip 就不记 memo」解决：真实 vela 主机上另外三个 adapter 都返回 404（关于主机的持久结论，正是 memo 存在的理由），verdict 一定是普通 skip。 做法是 memo 条目带上「哪些 vendor 因自身未配置而无法判断」： - 未就绪的 adapter 直接不跑（跑了只能得到 401 或伪造的 skip） - 条目带 unready 集合；该 vendor 配置好后条目作废重算 - 仍缺配置时条目依然写入（否则无看板的主机回到每次开页 60 秒超时）， 但不退避 —— 退避会拒绝那个本来能成功的探测 二、401 被下一个 adapter 的 404 掩盖 adapter 循环不停在 error，而「最后一个胜」不精确：openDesign 走厂商的整份 主机列表，是最可能看到「key rejected」的那个，但它也可能排在最前（memo 记 得它应答过，下次就优先它）—— 于是它的 401 被下一个 adapter 的 404「no dashboard here」分类成「这台主机没有余额端点」。两种情况的修复动作 完全相反，输出却一样。规则改为：被拒凭据优先，且不被覆盖。 判据：12/12 变异全杀。三条曾经存活或误杀，逐条查清： - rejected-as-skip 存活是因为测试没有复现生产调用顺序（memo 空 → openDesign 排最后 → 旧规则也能过），测试已改为预置 memo 并让另三个 adapter 走同一个 stub（它们读 base 而非 api_base，否则会全部传输失败） - no-cred-guard 并非存活，只是杀它的不是我以为的那个测试（门禁后面还有 一道检查），预期已改正 - unready-memo-not-dropped 是真等价变异（known=false 与 nil 只差一次廉价 探活），已从清单移除 --- internal/lua/plugins/quota.lua | 247 +++++++++++++++++++++++--- internal/lua/quota_backoff_test.go | 12 ++ internal/lua/quota_opendesign_test.go | 201 ++++++++++++++++----- 3 files changed, 390 insertions(+), 70 deletions(-) diff --git a/internal/lua/plugins/quota.lua b/internal/lua/plugins/quota.lua index d2d5313..4933ed2 100644 --- a/internal/lua/plugins/quota.lua +++ b/internal/lua/plugins/quota.lua @@ -43,7 +43,8 @@ PLUGIN.name = "quota" PLUGIN.version = "1.0.0" PLUGIN.description = "多源套餐余量（余额 / 用量 / 滚动窗口）" -PLUGIN.exports = { "collect_source", "collect_all", "due_sources", "memo_record" } +PLUGIN.exports = { "collect_source", "collect_all", "due_sources", "memo_record", + "resolve_api_base" } -- plugin.state must exist before anything indexes it. The kernel restores -- persisted state OVER this table, so declaring it here is safe: a restart with @@ -231,6 +232,81 @@ local function openaiDashboard(base, key) return rep, nil end +-- copySet returns a shallow copy of a set-table, or nil when it is empty. +-- +-- The copy matters: a memo entry outlives the call that wrote it, and storing +-- the same table would let a later write mutate a record already handed out. +-- nil for an empty set keeps "nothing was unready" out of the persisted state, +-- so the read side can use plain truthiness on the field. +local function copySet(t) + local out = nil + for k, v in pairs(t) do + out = out or {} + out[k] = v + end + return out +end + +-- vendorReady reports whether an adapter has the configuration it needs to answer +-- at all. +-- +-- OpenDesign is the first adapter that needs a credential the SOURCE does not +-- carry (prices.control_key). When it is missing the adapter should not run: with +-- no credential the only possible outcomes are a 401 (filed as an ERROR, "key +-- rejected") or a fabricated skip, and neither is a fact about the host. What it +-- would actually be reporting is that the deployment is not configured yet. +-- +-- Per-VENDOR rather than one global flag, so a future adapter that needs its own +-- configuration gets the same treatment by adding a branch here. +local function vendorReady(vendor) + if vendor == "opendesign" then + local c = PLUGIN.prices + if type(c) ~= "table" or type(c.control_key) ~= "string" or c.control_key == "" then + -- The reason travels with the refusal: "adapter not configured" alone says + -- what is wrong but not what to type. + return false, "control_key not configured (set prices.control_key)" + end + end + -- Every other adapter carries its credential on the source itself. + return true, nil +end + +-- resolveApiBase turns a configured source base_url into the OpenDesign billing +-- origin. +-- +-- Split out as its own function because it is the one piece of this adapter that +-- is pure logic, and it is load-bearing while being silent when it breaks: the +-- probe simply goes to the inference host, which serves no billing route, and the +-- source quietly stays unlisted. +-- +-- The host is taken as a HOST, not as the tail of a URL. base_url is normally +-- `https://amr-link.open-design.ai/v1`, and matching a `^...$` pattern against +-- that whole string never succeeds — the rewrite then silently does not happen. +-- That was a real defect here, found by asserting behaviour rather than source +-- text. +local function resolveApiBase(base, override) + if type(override) == "string" and override ~= "" then + return (override:gsub("/+$", "")) + end + local origin = base:gsub("^https?://", ""):gsub("/.*$", "") + origin = origin:gsub("^amr%-link%.open%-design%.ai$", "amr-api.open-design.ai") + return "https://" .. origin .. "/api/v1" +end + +-- resolve_api_base exposes resolveApiBase for tests, so the derivation can be +-- asserted directly instead of inferred from which error a failed probe happened +-- to report. Declared AFTER the local function it calls — a Lua local is nil until +-- its assignment runs, and this closure would otherwise capture nil. Pure: no +-- network, no state. +PLUGIN.resolve_api_base = function(payload) + if type(payload) ~= "table" then return nil, "expected table" end + local base = tostring(payload.base_url or "") + if base == "" then return nil, "missing base_url" end + local override = payload.api_base + if type(override) ~= "string" then override = nil end + return { api_base = resolveApiBase(base, override) }, nil +end + -- openDesign probes OpenDesign / Vela (amr-link.open-design.ai). -- -- This vendor needs a credential the source does not carry. Inference runs on @@ -287,17 +363,7 @@ local function openDesign(base, key) -- `api_base` overrides the derivation: the vendor has already moved these -- hosts once, and an operator who can see the new one in a browser should not -- be blocked on a code change to use it. - local api = nil - if type(cfg) == "table" then api = cfg.api_base end - if type(api) ~= "string" or api == "" then - -- Host only: base_url carries a path (`.../v1`), and matching the hostname - -- rule against it would silently skip the rewrite and send the probe to the - -- inference host — which serves no billing route. - local origin = base:gsub("^https?://", ""):gsub("/.*$", "") - origin = origin:gsub("^amr%-link%.open%-design%.ai$", "amr-api.open-design.ai") - api = "https://" .. origin .. "/api/v1" - end - api = api:gsub("/+$", "") + local api = resolveApiBase(base, type(cfg) == "table" and cfg.api_base or nil) -- The gateway's outbound allowlist is the hosts of configured sources. It -- therefore contains amr-link (the inference host) but NOT amr-api (the @@ -547,6 +613,24 @@ local function collectSource(src, budget) base = base:gsub("/+$", "") local key = tostring(src.api_key or "") local rep, kind, why + -- Vendors that could not judge the host because their OWN configuration is + -- missing. Stamped onto the negative memo so it can be retired the moment that + -- configuration arrives. + local unready = {} + -- A REJECTED credential outranks every other error, and the reason is the + -- only thing this variable is for. + -- + -- Why the rule is not simply "last wins": the four adapters answer different + -- questions, and a general-purpose one (openDesign, which walks the vendor's + -- whole host list) can report "key rejected" while an adapter that merely could + -- not reach ITS host reports a transport error. The two need opposite + -- responses — re-issue the credential vs. investigate the network — and with a + -- plain "last wins" the credential problem is reported as a network one. + -- + -- Conversely, "first error wins" is just as arbitrary in the other direction: + -- an adapter whose host is unreachable from this deployment fails before the + -- one that actually knows what is wrong. + local errWhy, errRejected = nil, false -- Try the adapter this host answered with last time first. Same host with a -- different key (two Command Code seats) gives the same answers, so this turns -- 21 sources into a handful of distinct round trips after the first pass. @@ -558,16 +642,49 @@ local function collectSource(src, budget) -- exactly the confusion that made an earlier revision drop five sources. if type(known) == "table" then local seen = known.at or 0 + -- A memo written while an adapter could not judge the host is PROVISIONAL. + -- It is discarded as soon as that adapter becomes configured, and it never + -- earns a backoff while it is still waiting. + -- + -- Why it must not back off: the memo is applied unconditionally, before any + -- adapter runs. Honouring a provisional entry refuses the very probe that is + -- now able to succeed, so the operator supplies the missing credential and + -- sees nothing change. Measured end to end before this: unset key → memo → + -- key set → probe skipped without a single request. + -- + -- The comparison is against the vendors RECORDED in the entry, not against + -- all adapters. "Is any adapter configured" is true almost always — three of + -- the four carry their credential on the source — and would retire every + -- provisional entry immediately, including the ones still waiting. + local provisional = type(known.unready) == "table" + local stale = false + if provisional then + for vendor in pairs(known.unready) do + if vendorReady(vendor) then stale = true end + end + end + if provisional then + if stale then + -- The missing configuration is here: derive the verdict again. + known = nil + else + -- Still waiting for it. Do NOT back off: the memo is applied before any + -- adapter runs, so honouring it would refuse the very probe that will + -- succeed once the credential arrives. + known = false + end + else -- A streak is a host that has failed repeatedly. Skip it outright until its -- backoff expires, instead of re-paying three adapter timeouts every TTL. if backoffSeconds(seen, known.streak) then return { name = src.name, host = host, skipped = true, backing_off = true, reason = "连续探测失败 " .. tostring(known.streak) .. " 次，退避中（等待下次重试）" } end - if PROBE_MEMO_TTL > 0 and (os.time() - seen) > PROBE_MEMO_TTL then - known = nil - else - known = false -- still "nothing answered, try cheapest first" + if PROBE_MEMO_TTL > 0 and (os.time() - seen) > PROBE_MEMO_TTL then + known = nil + else + known = false -- still "nothing answered, try cheapest first" + end end end -- known == false means every adapter already came back "no endpoint here" for @@ -612,13 +729,32 @@ local function collectSource(src, budget) return { name = src.name, host = host, skipped = true, reason = "probe budget exhausted" } end - local r, k, e = a.probe(base, key) - if r ~= nil then - rep = r - HOST_MEMO[host] = a.vendor - break + -- An adapter that cannot answer for want of its OWN configuration is not + -- probed at all. Running it would produce a 401 (reported as ERROR "key + -- rejected") or a fabricated skip, and would send the operator to re-issue a + -- credential they have not configured yet. + if not vendorReady(a.vendor) then + local _, because = vendorReady(a.vendor) + unready[a.vendor] = true + kind = "skip" + why = because or ("adapter not configured: " .. a.vendor) + else + local r, k, e = a.probe(base, key) + if r ~= nil then + rep = r + HOST_MEMO[host] = a.vendor + break + end + kind, why = k, e + if k == "error" then + local rejected = tostring(e or ""):match("rejected") ~= nil + -- Last error wins, except that a rejected credential displaces anything + -- already recorded and is never displaced in turn. + if not errRejected and (errWhy == nil or rejected) then + errWhy, errRejected = e, rejected + end + end end - kind, why = k, e end -- Remember the NEGATIVE result too. Only recording successes left every -- source that has no balance endpoint re-probing all three adapters (2 @@ -643,18 +779,47 @@ local function collectSource(src, budget) -- -- A success overwrites the memo with a vendor string, so a host that recovers -- resets its streak automatically. + -- + -- The entry records WHICH vendors could not judge the host. Such a verdict is + -- provisional (see the read side): the memo is applied before every later + -- probe, so keeping it unqualified suppresses the source even after the + -- credential is supplied. + -- + -- It is still WRITTEN rather than omitted, because dropping it would put a + -- dashboard-less source back to 60s of adapter timeouts on every page open — + -- the cost the memo exists to remove. A memo cannot say "three of four + -- answered, the fourth is waiting for a key", so it says the next best thing. + -- + -- Its streak also does not carry over from a previous provisional entry: that + -- count would be spent on a judgement nobody made. if rep == nil then local prev = HOST_MEMO[host] local streak = 1 - if type(prev) == "table" then streak = (prev.streak or 1) + 1 end - HOST_MEMO[host] = { at = os.time(), streak = streak } + if type(prev) == "table" and not prev.unready then + streak = (prev.streak or 1) + 1 + end + HOST_MEMO[host] = { at = os.time(), streak = streak, unready = copySet(unready) } end if rep ~= nil then HOST_MEMO_AT[host] = os.time() end if rep == nil then - if kind == "error" then - return { name = src.name, vendor = "", error = tostring(why), + -- An ERROR is reported when either: + -- * the LAST adapter to judge the host reported one, or + -- * ANY adapter reported a REJECTED credential. + -- + -- The second clause is not a refinement, it is the whole point. openDesign + -- walks the vendor's host list and is therefore the adapter most likely to + -- see "key rejected", but it also runs FIRST once it has been memoised as the + -- answering vendor — so a last-wins rule classifies its 401 by whatever the + -- next adapter says, and the next adapter (commandcode, pointed at the + -- source's own host) answers 404 "no dashboard here". Observed end to end: + -- a deliberately wrong control key was reported as "this host has no balance + -- endpoint", sending the operator to investigate the vendor instead of + -- re-issuing the credential. A rejection has one unambiguous remedy, so it + -- outranks position. + if kind == "error" or errRejected then + return { name = src.name, vendor = "", error = tostring(errWhy or why), skipped = false, host = hostOf(base) } end -- The skip REASON travels. Discarding it here is why @@ -665,6 +830,12 @@ local function collectSource(src, budget) -- "no balance endpoint" is not. local out = { name = src.name, skipped = true, host = hostOf(base) } if why and why ~= "" then out.reason = tostring(why) end + -- `unready` travels with the record because the page fans out on shard VMs + -- and reports back through memo_record, where a row with no vendor and no + -- error is otherwise indistinguishable from a real failure. Without it the + -- sharded path — the one the page actually uses — would still earn the source + -- a backoff it should never have had. + if next(unready) ~= nil then out.unready = true end return out end rep.name = src.name @@ -732,8 +903,24 @@ PLUGIN.memo_record = function(payload) else local prev = HOST_MEMO[host] local streak = 1 - if type(prev) == "table" then streak = (prev.streak or 1) + 1 end - HOST_MEMO[host] = { at = now, streak = streak } + -- A provisional entry is replaced, not aged: its count describes a + -- judgement nobody made. See the write side in collectSource. + if type(prev) == "table" and not prev.unready then + streak = (prev.streak or 1) + 1 + end + -- The page can only send a boolean (it does not know which adapter was + -- unready), so the set is re-derived here on the shared VM. Copying it + -- matters: this table is stored, and a later write must not mutate it. + local u = nil + if r.unready then + for _, a in ipairs(ADAPTERS) do + if not vendorReady(a.vendor) then + u = u or {} + u[a.vendor] = true + end + end + end + HOST_MEMO[host] = { at = now, streak = streak, unready = u } end end end @@ -1175,6 +1362,10 @@ PLUGIN.ui = { // flag rather than the wording, so translating the message cannot silently // turn every skipped host into an ever-growing streak. if(r.backing_off===true) return null; + // A host probed while an adapter had no configuration was not judged. + // Recording it would earn a backoff that outlives the configuration + // mistake and suppresses the probe that would now succeed. + if(r.unready===true) return null; if(r.error) return {host:r.host}; if(r.skipped) return {host:r.host}; return {host:r.host, vendor:r.vendor}; diff --git a/internal/lua/quota_backoff_test.go b/internal/lua/quota_backoff_test.go index dc252b4..6432b5d 100644 --- a/internal/lua/quota_backoff_test.go +++ b/internal/lua/quota_backoff_test.go @@ -177,6 +177,18 @@ func TestQuotaBackoffRecordsConsecutiveFailures(t *testing.T) { srv := newCountingServer(t) ps := newQuotaPlugins(t, srv.URL) + // Configure every vendor that needs its own credentials, so no adapter is + // UNABLE to judge the host. That is a precondition, not decoration: a host + // judged while an adapter is unconfigured gets a PROVISIONAL memo, and a + // provisional memo deliberately never backs off (it would refuse the very + // probe that succeeds once the credential arrives). Without this the test + // would be measuring that rule instead of the ordinary backoff. + if err := ps.SetState("quota", map[string]interface{}{ + "prices": map[string]interface{}{"control_key": "ctrl"}, + }); err != nil { + t.Fatal(err) + } + args := map[string]interface{}{ "sources": []interface{}{map[string]interface{}{ "name": "dead", "base_url": srv.URL + "/v1", "api_key": "k", diff --git a/internal/lua/quota_opendesign_test.go b/internal/lua/quota_opendesign_test.go index b585dca..6e4bc05 100644 --- a/internal/lua/quota_opendesign_test.go +++ b/internal/lua/quota_opendesign_test.go @@ -317,34 +317,66 @@ func TestQuotaOpenDesignHasNoFabricatedPeriodWindow(t *testing.T) { // means the credential is wrong — a fact the operator must fix — so it must not // be filed under "skip" alongside "this vendor has no balance endpoint". The two // produce the same blank card but wildly different next actions. +// +// Two details make the test actually exercise the rule instead of passing by +// accident, and both were found by a mutation surviving: +// +// 1. The memo is pre-seeded so openDesign runs FIRST. The memo remembers which +// vendor answered last and that vendor is tried first; once a host has ever +// answered through openDesign it runs first forever after. With the default +// order (openDesign last) its 401 IS the last verdict, so even a plain +// "last error wins" rule passes. +// 2. The other three adapters must ANSWER (404), not fail. They read `base`, not +// `api_base`, so the source base_url points at the same stub — otherwise they +// all error out on an unreachable host and the final verdict is an error +// regardless of the rule being tested. func TestQuotaOpenDesignRejectedControlKeyIsAnError(t *testing.T) { srv := odServer(t, nil, `{"error":"unauthenticated"}`, `{"error":"unauthenticated"}`, http.StatusUnauthorized, http.StatusUnauthorized) defer srv.Close() ps, _ := odPlugins(t) + // Host key must match what hostOf(base) yields for the source below. + src := map[string]interface{}{ + "name": "vela", "base_url": srv.URL, "api_key": "k", + } if err := ps.SetState("quota", map[string]interface{}{ "prices": map[string]interface{}{ "control_key": "vela_ctrl_WRONG", "api_base": srv.URL + "/api/v1", }, + // Flat, NOT nested under a "state" key: SetState pulls `prices` out and + // stores the REST of the payload as the plugin's state. Nesting it would + // store {state: {probe_memo: ...}} and the memo would never be read. + "probe_memo": map[string]interface{}{ + hostOnly(srv.URL): "opendesign", + }, }); err != nil { t.Fatal(err) } - rep := collectOpenDesign(t, ps, "https://amr-link.open-design.ai/v1") + + out, err := ps.Call("quota", "collect_source", map[string]interface{}{ + "name": "vela", "sources": []interface{}{src}, + }) + if err != nil { + t.Fatal(err) + } + rep, _ := out.(map[string]interface{}) if rep == nil { t.Fatal("no report produced") } if rep["vendor"] == "opendesign" { t.Fatalf("a rejected control key must not produce a balance: %v", rep) } - // The plugin reports a credential failure as kind="error", which the page - // renders as a red "读取失败" row. Filing it as a skip instead would show the - // same blank card as "this vendor has no balance endpoint" — and send the - // operator to investigate the vendor rather than re-run the login. + // The plugin reports a credential failure as an error, which the page renders + // as a red "读取失败" row. Filing it as a skip instead shows the same blank + // card as "this vendor has no balance endpoint" — and sends the operator to + // investigate the vendor rather than re-run the login. if rep["skipped"] == true { - t.Fatalf("a rejected control key was reported as a SKIP: %v — the two "+ - "require different fixes and must not look alike", rep) + t.Fatalf("a rejected control key was reported as a SKIP: %v — the other three "+ + "adapters answered 404 for this host, so the final verdict is a skip unless "+ + "a rejected credential outranks it. The operator would go looking at the "+ + "vendor instead of re-running the login.", rep) } msg, _ := rep["error"].(string) if !strings.Contains(msg, "rejected") { @@ -404,48 +436,133 @@ func TestQuotaOpenDesignDoesNotDefaultMissingRate(t *testing.T) { // working. Without it the adapter probes the inference host, which serves no // billing route at all, and every OpenDesign source silently skips. // -// Asserted BEHAVIOURALLY, through the outbound allowlist rather than on the -// source text. The allowlist in these tests holds only 127.0.0.1, so a request -// to any real host is refused with a message that NAMES the host it tried. That -// makes the refusal the observation: "amr-api.open-design.ai" in the reason -// proves the derivation ran, and its absence proves it did not. The earlier -// version of this test grepped quota.lua for the gsub pattern, which pins the -// spelling of the rule instead of its effect — it went red the moment the rule -// was tightened to a full-hostname match, while the behaviour was unchanged. +// Asserted on the exported derivation rather than on the source text or on a +// failed probe's error message. The source-text version pinned the SPELLING of +// the rule: it went red the moment the rule was tightened from an `amr-link.` +// prefix to a full hostname, while the behaviour was unchanged. The +// error-message version inferred "which host did we ask?" from an allowlist +// refusal, which stops working the moment any adapter earlier in the chain also +// fails — and one of them always does. func TestQuotaOpenDesignHostRewritingIsUsed(t *testing.T) { ps, _ := odPlugins(t) - // No api_base: the adapter must derive the billing host itself. The control - // key IS set so the credential guard is not what stops us. + cases := []struct{ in, want string }{ + // The real production shape: base_url carries a path. + {"https://amr-link.open-design.ai/v1", "https://amr-api.open-design.ai/api/v1"}, + // And without one, so the derivation cannot depend on the path being there. + {"https://amr-link.open-design.ai", "https://amr-api.open-design.ai/api/v1"}, + // A trailing slash must not produce a double slash. + {"https://amr-link.open-design.ai/v1/", "https://amr-api.open-design.ai/api/v1"}, + } + for _, c := range cases { + out, err := ps.Call("quota", "resolve_api_base", map[string]interface{}{ + "base_url": c.in, + }) + if err != nil { + t.Fatalf("resolve_api_base(%q): %v", c.in, err) + } + rep, _ := out.(map[string]interface{}) + if got, _ := rep["api_base"].(string); got != c.want { + t.Errorf("resolve_api_base(%q) = %q, want %q — the probe would go to the "+ + "inference host, which serves no billing route, and the source would "+ + "silently stay unlisted", c.in, got, c.want) + } + } + + // A lookalike host must NOT be rewritten into an OpenDesign address: an + // `amr-link.` prefix rule would turn amr-link.example.com into amr-api..., + // sending an unrelated provider's probe to a host that is not its own. + out, err := ps.Call("quota", "resolve_api_base", map[string]interface{}{ + "base_url": "https://amr-link.example.com/v1", + }) + if err != nil { + t.Fatal(err) + } + rep, _ := out.(map[string]interface{}) + if got, _ := rep["api_base"].(string); got != "https://amr-link.example.com/api/v1" { + t.Errorf("a lookalike host was rewritten to %q", got) + } + + // An explicit override wins (and is trimmed), so an operator can point at a + // moved host without waiting for a code change. + out, err = ps.Call("quota", "resolve_api_base", map[string]interface{}{ + "base_url": "https://amr-link.open-design.ai/v1", + "api_base": "https://amr-api.example.net/api/v1/", + }) + if err != nil { + t.Fatal(err) + } + rep, _ = out.(map[string]interface{}) + if got, _ := rep["api_base"].(string); got != "https://amr-api.example.net/api/v1" { + t.Errorf("api_base override not honoured (or not trimmed): %q", got) + } +} + +// TestQuotaUnreadyAdapterDoesNotSuppressAConfiguredHost is the end-to-end half of +// the unready rule, and it reproduces a bug seen on the live plugin. +// +// The negative memo means "this HOST exposes no balance endpoint" and is applied +// UNCONDITIONALLY, before any adapter runs. A host judged while openDesign had no +// prices.control_key therefore got memoised as having nothing and stayed +// suppressed after the key was supplied — observed live: unset key → memo → key +// set → probe skipped without a single request, so the fix the operator had just +// applied looked like it did nothing. +// +// It is NOT fixable by "do not memoise a skip": on a real OpenDesign host the +// other three adapters answer 404 ("no dashboard here"), which is durable and +// worth remembering, so the verdict would be recorded either way. The memo has to +// carry WHICH vendors were unable to judge the host, and the read side has to +// retire that record once they can. +func TestQuotaUnreadyAdapterDoesNotSuppressAConfiguredHost(t *testing.T) { + var seenAuth []string + srv := odServer(t, &seenAuth, odWallet, odSummary, 200, 200) + defer srv.Close() + + ps, _ := odPlugins(t) + args := map[string]interface{}{ + "name": "vela", + "sources": []interface{}{map[string]interface{}{ + "name": "vela", "base_url": srv.URL + "/v1", "api_key": "k", + }}, + } + + // Step 1: no control_key. openDesign cannot judge the host, and no billing + // request goes anywhere. + out, err := ps.Call("quota", "collect_source", args) + if err != nil { + t.Fatal(err) + } + rep, _ := out.(map[string]interface{}) + if rep["skipped"] != true { + t.Fatalf("expected a skip with no control key configured: %v", rep) + } + if len(seenAuth) != 0 { + t.Fatalf("an unconfigured adapter made a request: %q", seenAuth) + } + + // Step 2: configure the key. The very next probe must reach the upstream. It + // is inside the backoff window (minutes of wall clock) while this runs in + // microseconds — so a host refused now is refused for the whole window in + // production, which is the bug, not a timing accident. if err := ps.SetState("quota", map[string]interface{}{ - "prices": map[string]interface{}{"control_key": "vela_ctrl_TESTKEY"}, + "prices": map[string]interface{}{ + "control_key": "vela_ctrl_TESTKEY", + "api_base": srv.URL + "/api/v1", + }, }); err != nil { t.Fatal(err) } - - rep := collectOpenDesign(t, ps, "https://amr-link.open-design.ai/v1") - if rep == nil { - t.Fatal("no report produced") + out, err = ps.Call("quota", "collect_source", args) + if err != nil { + t.Fatal(err) } - // The outbound allowlist in these tests holds only 127.0.0.1, so the request - // is refused before it leaves the process and the refusal NAMES the host it - // tried. The observation is therefore the refusal text, not the record as a - // whole: `host` is `hostOf(base)` — the SOURCE host, which is amr-link by - // design and says nothing about where the request went. - var msg string - if s, ok := rep["error"].(string); ok { - msg = s + rep, _ = out.(map[string]interface{}) + if rep["vendor"] != "opendesign" || rep["skipped"] == true { + t.Fatalf("expected an opendesign balance once configured, got %v — the host "+ + "is still suppressed by the provisional memo it earned while the adapter "+ + "had no credential", rep) } - if s, ok := rep["reason"].(string); ok { - msg += " " + s - } - if !strings.Contains(msg, "amr-api.open-design.ai") { - t.Fatalf("no evidence the adapter derived the billing host from the "+ - "inference one: %q. Expected the allowlist refusal to name "+ - "amr-api.open-design.ai, since that is the host the request must go to. "+ - "(report: %v)", msg, rep) - } - if strings.Contains(msg, "amr-link") { - t.Fatalf("the adapter probed the INFERENCE host — it serves no billing "+ - "route, so every OpenDesign source would silently skip: %q", msg) + if len(seenAuth) == 0 { + t.Fatal("no request reached the billing endpoint after the control key was " + + "supplied") } }