Repository navigation
Conversation
Classify prepared-owner not-published and publication-superseded errors as coordination so fallback cannot rotate to an unrelated provider or surface an exhausted OAuth candidate as a quota warning. Fixes openclaw#156975 Signed-off-by: GokayAI <[email protected]>
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P1
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 24, 2026, 9:34 AM ET / 13:34 UTC (Revision 2). ClawSweeper reviewWhat this changesThe branch makes OpenClaw stop model fallback when preparation reports an unpublished or superseded runtime owner, and adds classification and fallback tests. Regression provenancePossible regression — probable (reviewed change; failure trace). No predecessor PR is attributed. Merge readiness✅ Ready for maintainer review The classification gap remains on current main. The new before-and-after publication-owner trace addresses the prior proof request at the changed fallback boundary, and the separate Gateway control shows ordinary provider fallback still works. A second open PR addresses the same issue, but neither has merged. Priority: P1 Review scores
Verification
How this fits togetherThe agent runner prepares a runtime before trying a selected model and its configured fallback models. Its error classifier decides whether a failure belongs to a provider or to OpenClaw's own runtime coordination, which determines whether the next provider is called. flowchart LR
A[Agent turn] --> B[Prepared runtime]
B --> C{Failure type}
C -->|Provider failure| D[Fallback model]
C -->|Runtime coordination| E[Preserve internal error]
D --> F[Provider response]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource +12, Tests +82. Total +94 across 3 files. View PR surface stats
Review metricsNone. Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Technical reviewBest possible solution: Keep runtime-owner reacquisition in the lease lifecycle and let the fallback classifier preserve any ownership error that escapes it; resolve the two open fix PRs through normal review without combining their scopes. Do we have a high-confidence way to reproduce the issue? Yes, for the changed boundary: the captured before-and-after race invokes the real publication owner from the fallback runner and records the different provider requests and returned errors. The original Windows OAuth Gateway sequence remains nondeterministic. Is this the best way to solve the issue? Yes. Classifying this typed internal ownership error where fallback decisions are made is a narrow repair; the existing lease owner remains responsible for reacquisition. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 3948a0fd1831. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
Collaborator
|
@clawsweeper re-review Added real publication-owner race evidence and a real isolated Gateway HTTP-429 fallback control to the PR body. Two competing actual preparations produce the typed supersession inside the real publication owner; the real fallback runner preserves it without rotating, while the replacement publishes successfully. The Gateway config-reload experiment and its reproduction limit are stated explicitly; this is not claimed as an OAuth Gateway race reproduction. |
Contributor
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
Collaborator
|
Landed as 23f89ee. Thanks @gokay-ai for the fix and @WG-Mojo for the report. Before: two real competing runtime publications on pinned main sent the superseded primary into fallback, made an HTTP 402 request, and returned an aggregate billing error. After: the same publication-owner race returned the original typed internal error, made no fallback request, and left the replacement runtime published/current. A separate isolated built Gateway control with primary HTTP 429 still reached its fallback and returned FALLBACK_OK. The original Windows OAuth Gateway race was not recreated; that limit is documented in the PR body. Exact-head CI was green and hosted ClawSweeper marked the evidence sufficient. |
This was referenced Sep 24, 2026
2 of 8 tasks
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 25, 2026
…openclaw#157218) Thanks for the clear write-up on openclaw#156975. ## What Problem This Solves Fixes: a superseded prepared-model-runtime publication is treated as an unknown provider failure, so fallback rotates to the next candidate and an exhausted OAuth fallback can surface as a quota or rate-limit warning. Fixes openclaw#156975 ## User Impact User impact: that internal ownership miss stays internal. OpenClaw does not rotate to an unrelated provider, and operators are not sent looking at OpenAI quota. ## Why This Change Was Made Failover now classifies prepared-owner not-published and publication-superseded errors as coordination. The existing candidate runner already stops rotation for that kind. ## Evidence Focused Vitest (`node scripts/run-vitest.mjs <file> --maxWorkers=1`): - `src/agents/failover-error.test.ts`: 12.3s wall (83 tests). Direct and wrapped owner-not-published / publication-superseded errors classify as coordination, not billing/rate-limit. - `src/agents/model-fallback.terminal-boundary.test.ts`: 17.0s wall (96 tests). Forcing the typed error on the primary candidate does not rotate to `xai/grok-4.7` and does not surface the exhausted-fallback billing path. - `git diff --check`: clean. ## Additional runtime evidence Verified on `964bb1eba653f2188970d353b99cfeb4ff3544fb`: - **Real publication-owner race (3.06s wall):** called `publishModelRuntimeSnapshot` twice without awaiting between calls, with the same agent/workspace identity and shared owner/build-completion maps, from the primary callback of the real `runWithModelFallback`. Both calls executed actual preparation in static catalog mode with plugins disabled. The second publication replaced the first registered owner; the first rejected from `prepared-model-runtime.owner.ts:704` with `PreparedModelRuntimePublicationSupersededError`. The runner attempted only `proof-primary`, returned that unchanged internal error (no quota/billing text), and never invoked `proof-fallback`. The second publication completed and was the sole current owner, with its snapshot published, no pending gate, and `needsRefresh=false`. No mocked publication, injected error, classifier replacement, or production modification was used in this proof. - **Real Gateway negative control:** an isolated built Gateway, launched using `pnpm openclaw gateway`, called a scripted loopback OpenAI-compatible primary returning HTTP 429. After the existing same-model retry policy, it made one request to the separately configured fallback endpoint and returned `FALLBACK_OK` through `/v1/chat/completions`. This protects real provider failover, not only classifier behavior. - **Gateway reproduction limit:** changing `agents.defaults.model.fallbacks` during that in-flight turn produced `config hot reload applied`, but did not produce publication supersession. Delaying model HTTP responses does not hold the static runtime preparation boundary: the model call happens after admission. Current Gateway admission joins replacement publication and retries a superseded acquisition when owner/generation facts advance (`prepared-model-runtime-lease.ts:114-145,363-378`); the publication queue also joins newer refreshes when requested. Thus an ordinary concurrent reload is not a deterministic trigger for the escaping error. The Windows OAuth production race has not been recreated; the deterministic proof above exercises the real lower-level publication owner and the changed fallback boundary instead. No claim that this patch repairs the underlying publication race. The intended behavior at this boundary is to preserve the internal error, not rotate providers or invent an arbitrary retry. Reacquisition remains owned by the existing lease/publication lifecycle. ### Matched real-owner before/after Ran the **same portable owner-race script** against a full source snapshot of pinned `origin/main` (`fd4d10858d5d2a187cf6f6d95d782234b4fde6d6`, its own frozen-lockfile dependencies) and the contributor head (`964bb1eba653f2188970d353b99cfeb4ff3544fb`). No module/classifier substitution was used. The fallback callback makes a real HTTP request to a loopback provider returning a synthetic HTTP 402 billing response. | Observation | Pinned main (11.00s wall) | Candidate (5.29s wall) | | --- | --- | --- | | Primary preparation | Real owner throws `PreparedModelRuntimePublicationSupersededError` | Same | | Candidates attempted | `proof-primary`, `proof-fallback` | `proof-primary` only | | Fallback HTTP request | One; status 402 | None | | Returned failure | `FailoverError`: `All models failed (2)`, supersession `(unknown)` followed by synthetic exhausted balance `(billing)` | Original `PreparedModelRuntimePublicationSupersededError`, no billing/quota advice | | Replacement publication | Published, sole current owner, no pending gate | Same | The separately exercised Gateway HTTP 429 control above still returned `FALLBACK_OK`. These results distinguish internal publication coordination from actual provider failures. <details> <summary>Observed owner-race output (paths redacted)</summary> ```json [ { "revision": "fd4d10858d5d2a187cf6f6d95d782234b4fde6d6", "variant": "baseline", "attempted": [ "proof-primary", "proof-fallback" ], "primaryErrorName": "PreparedModelRuntimePublicationSupersededError", "errorName": "FailoverError", "message": "All models failed (2): proof-primary/model: prepared model runtime publication was superseded for <isolated-agent-dir> (unknown) | proof-fallback/model: {\"error\":{\"message\":\"synthetic usage balance exhausted\",\"type\":\"billing_error\"}} (billing)", "providerStatus": 402, "replacementPublished": true, "replacementCurrent": true }, { "revision": "964bb1eba653f2188970d353b99cfeb4ff3544fb", "variant": "candidate", "attempted": [ "proof-primary" ], "primaryErrorName": "PreparedModelRuntimePublicationSupersededError", "errorName": "PreparedModelRuntimePublicationSupersededError", "message": "prepared model runtime publication was superseded for <isolated-agent-dir>", "replacementPublished": true, "replacementCurrent": true } ] ``` </details> Co-authored-by: Ayaan Zaidi <[email protected]>
This was referenced Oct 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for the clear write-up on #156975.
What Problem This Solves
Fixes: a superseded prepared-model-runtime publication is treated as an unknown provider failure, so fallback rotates to the next candidate and an exhausted OAuth fallback can surface as a quota or rate-limit warning.
Fixes #156975
User Impact
User impact: that internal ownership miss stays internal. OpenClaw does not rotate to an unrelated provider, and operators are not sent looking at OpenAI quota.
Why This Change Was Made
Failover now classifies prepared-owner not-published and publication-superseded errors as coordination. The existing candidate runner already stops rotation for that kind.
Evidence
Focused Vitest (
node scripts/run-vitest.mjs <file> --maxWorkers=1):src/agents/failover-error.test.ts: 12.3s wall (83 tests). Direct and wrapped owner-not-published / publication-superseded errors classify as coordination, not billing/rate-limit.src/agents/model-fallback.terminal-boundary.test.ts: 17.0s wall (96 tests). Forcing the typed error on the primary candidate does not rotate toxai/grok-4.7and does not surface the exhausted-fallback billing path.git diff --check: clean.Additional runtime evidence
Verified on
964bb1eba653f2188970d353b99cfeb4ff3544fb:publishModelRuntimeSnapshottwice without awaiting between calls, with the same agent/workspace identity and shared owner/build-completion maps, from the primary callback of the realrunWithModelFallback. Both calls executed actual preparation in static catalog mode with plugins disabled. The second publication replaced the first registered owner; the first rejected fromprepared-model-runtime.owner.ts:704withPreparedModelRuntimePublicationSupersededError. The runner attempted onlyproof-primary, returned that unchanged internal error (no quota/billing text), and never invokedproof-fallback. The second publication completed and was the sole current owner, with its snapshot published, no pending gate, andneedsRefresh=false. No mocked publication, injected error, classifier replacement, or production modification was used in this proof.pnpm openclaw gateway, called a scripted loopback OpenAI-compatible primary returning HTTP 429. After the existing same-model retry policy, it made one request to the separately configured fallback endpoint and returnedFALLBACK_OKthrough/v1/chat/completions. This protects real provider failover, not only classifier behavior.agents.defaults.model.fallbacksduring that in-flight turn producedconfig hot reload applied, but did not produce publication supersession. Delaying model HTTP responses does not hold the static runtime preparation boundary: the model call happens after admission. Current Gateway admission joins replacement publication and retries a superseded acquisition when owner/generation facts advance (prepared-model-runtime-lease.ts:114-145,363-378); the publication queue also joins newer refreshes when requested. Thus an ordinary concurrent reload is not a deterministic trigger for the escaping error. The Windows OAuth production race has not been recreated; the deterministic proof above exercises the real lower-level publication owner and the changed fallback boundary instead. No claim that this patch repairs the underlying publication race.The intended behavior at this boundary is to preserve the internal error, not rotate providers or invent an arbitrary retry. Reacquisition remains owned by the existing lease/publication lifecycle.
Matched real-owner before/after
Ran the same portable owner-race script against a full source snapshot of pinned
origin/main(9e007615ece5436bc5d88d0d21ba0c4c62b408ec, its own frozen-lockfile dependencies) and the contributor head (964bb1eba653f2188970d353b99cfeb4ff3544fb). No module/classifier substitution was used. The fallback callback makes a real HTTP request to a loopback provider returning a synthetic HTTP 402 billing response.PreparedModelRuntimePublicationSupersededErrorproof-primary,proof-fallbackproof-primaryonlyFailoverError:All models failed (2), supersession(unknown)followed by synthetic exhausted balance(billing)PreparedModelRuntimePublicationSupersededError, no billing/quota adviceThe separately exercised Gateway HTTP 429 control above still returned
FALLBACK_OK. These results distinguish internal publication coordination from actual provider failures.Observed owner-race output (paths redacted)
[ { "revision": "9e007615ece5436bc5d88d0d21ba0c4c62b408ec", "variant": "baseline", "attempted": [ "proof-primary", "proof-fallback" ], "primaryErrorName": "PreparedModelRuntimePublicationSupersededError", "errorName": "FailoverError", "message": "All models failed (2): proof-primary/model: prepared model runtime publication was superseded for <isolated-agent-dir> (unknown) | proof-fallback/model: {\"error\":{\"message\":\"synthetic usage balance exhausted\",\"type\":\"billing_error\"}} (billing)", "providerStatus": 402, "replacementPublished": true, "replacementCurrent": true }, { "revision": "964bb1eba653f2188970d353b99cfeb4ff3544fb", "variant": "candidate", "attempted": [ "proof-primary" ], "primaryErrorName": "PreparedModelRuntimePublicationSupersededError", "errorName": "PreparedModelRuntimePublicationSupersededError", "message": "prepared model runtime publication was superseded for <isolated-agent-dir>", "replacementPublished": true, "replacementCurrent": true } ]