Skip to content

fix(auth): suppress recovery hint for format failures#95779

Merged
steipete merged 2 commits into
openclaw:mainfrom
Pick-cat:fix-format-auth-profile-recovery-hint
Jun 26, 2026
Merged

fix(auth): suppress recovery hint for format failures#95779
steipete merged 2 commits into
openclaw:mainfrom
Pick-cat:fix-format-auth-profile-recovery-hint

Conversation

@Pick-cat

Copy link
Copy Markdown
Contributor

What Problem This Solves

format failover reasons represent request/schema rejections, not an auth state the user can repair by logging in again. formatAuthProfileFailureMessage() still routed unknown/default reasons through shouldIncludeRecoveryHint()'s default: true, so a format failure could append provider auth recovery guidance such as login/configure commands.

Why This Change Was Made

The auth-profile failure copy already suppresses recovery hints for transient and non-actionable reasons (rate_limit, overloaded, timeout, server_error, model_not_found). format belongs with that non-actionable group: it is a request-shape/schema problem, and resolveAuthProfileFailureReason() already avoids persisting format failures as shared auth-profile health.

Related / Duplicate Check

Searched open and closed PRs for shouldIncludeRecoveryHint, format auth profile recovery hint, failure-copy, and schema rejection auth-profiles. No overlapping open or closed PR was found.

Expected rating: diamond-lobster level. The patch is the smallest owner-boundary fix (+1 source line), has focused regression coverage, direct production-function proof, main-fail/fix-pass evidence, and no config/protocol/API surface change.

User Impact

Users no longer see misleading auth recovery commands for format/schema rejection failures. Actionable auth, session, billing, and login failures still include the provider recovery hint.

Evidence

  • Reproduced the bug against main behavior by keeping the new regression assertion while removing the production fix: node scripts/run-vitest.mjs src/agents/auth-profiles/failure-copy.test.ts failed because reason=format output contained <<login-hint-for-provider>>:openai-codex.
  • After the fix: node scripts/run-vitest.mjs src/agents/auth-profiles/failure-copy.test.ts passes, 8/8.
  • Direct production-function proof after the fix: node --import tsx -e "import { formatAuthProfileFailureMessage } from './src/agents/auth-profiles/failure-copy.ts'; const message = formatAuthProfileFailureMessage({ reason: 'format', provider: 'openai-codex', allInCooldown: true }); console.log(message); if (/models auth login|openclaw configure/.test(message)) process.exit(1);" prints Couldn't reach openai-codex with any of your saved logins right now. and exits 0.
  • git diff --check -- src/agents/auth-profiles/failure-copy.ts src/agents/auth-profiles/failure-copy.test.ts passes.
  • .agents/skills/autoreview/scripts/autoreview --mode local reported no accepted/actionable findings.

@Pick-cat
Pick-cat requested a review from a team as a code owner June 22, 2026 12:56
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Jun 22, 2026
@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 7:23 PM ET / 23:23 UTC.

Summary
The PR adds format to auth-profile failover reasons that suppress login/configure recovery hints and extends formatter plus auto-reply regression coverage.

PR surface: Source +1, Tests +27. Total +28 across 3 files.

Reproducibility: yes. source-reproducible. Current main lets reason: "format" fall through shouldIncludeRecoveryHint to the default recovery-hint path while callers pass typed FailoverError metadata into that formatter.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

  • [P2] No repair lane is needed because the open PR has no actionable findings; the remaining action is ordinary maintainer merge review and exact-head gate completion.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes failure-copy branching and focused tests only.

Review details

Best possible solution:

Land this narrow formatter gate after normal maintainer review and exact-head merge gates, keeping recovery hints for actionable auth, billing, and session failures only.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible. Current main lets reason: "format" fall through shouldIncludeRecoveryHint to the default recovery-hint path while callers pass typed FailoverError metadata into that formatter.

Is this the best way to solve the issue?

Yes. The formatter is the final user-facing recovery-hint gate, and changing broader failover classification or auth-profile cooldown policy would touch more behavior than this copy bug requires.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against bdd365a34882.

Label changes

Label justifications:

  • P2: The PR fixes a narrow user-facing auth guidance bug with limited blast radius and no config, protocol, migration, or API surface change.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output against the production formatter plus before/after focused Vitest evidence for this non-visual copy path.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output against the production formatter plus before/after focused Vitest evidence for this non-visual copy path.
Evidence reviewed

PR surface:

Source +1, Tests +27. Total +28 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 1 0 +1
Tests 2 30 3 +27
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 31 3 +28

What I checked:

  • Repository policy read: Root AGENTS.md and scoped src/agents/AGENTS.md were read; their high-confidence PR review, auth/provider compatibility, and agent-test guidance were applied. (AGENTS.md:1, bdd365a34882)
  • Current main behavior: On current main, shouldIncludeRecoveryHint returns false for transient/model-not-found reasons but omits format, so format reaches the default true branch and can append provider auth recovery guidance. (src/agents/auth-profiles/failure-copy.ts:74, bdd365a34882)
  • Runtime caller: Auto-reply passes typed FailoverError.reason, provider, cooldown state, and cause into formatAuthProfileFailureMessage, making this helper the user-facing copy gate for typed auth-profile failures. (src/auto-reply/reply/agent-runner-execution.ts:920, bdd365a34882)
  • Sibling auth-profile policy: The adjacent auth-profile failure policy explicitly treats request-shape format rejections as session-local and not shared credential/profile health, supporting no login/configure recovery hint for that reason. (src/agents/embedded-agent-runner/run/auth-profile-failure-policy.ts:20, bdd365a34882)
  • PR diff: The PR adds case "format" to the no-recovery branch and adds focused formatter plus auto-reply assertions that the format diagnostic remains while auth recovery commands are absent. (src/agents/auth-profiles/failure-copy.ts:86, 663e2c3869dd)
  • Proof and checks: The PR body includes before/fix-pass focused test evidence and terminal output against the production formatter; live GitHub status also shows the Real behavior proof check passing on the current head. (663e2c3869dd)

Likely related people:

  • RomneyDa: Merged PR fix(agents): actionable copy for exhausted auth-profile failover #85798 introduced the auth-profile failure-copy presenter and recovery-hint composition that this PR narrows. (role: introduced behavior; confidence: high; commits: 64a946ac21d1; files: src/agents/auth-profiles/failure-copy.ts, src/agents/auth-profiles/failure-copy.test.ts)
  • zhangguiping-xydt: Current main blame attributes the present failure-copy.ts lines to commit e9f9a68 after a recent merge that carried the file into the current tree. (role: current-main blame carrier; confidence: medium; commits: e9f9a68d68c9; files: src/agents/auth-profiles/failure-copy.ts, src/agents/embedded-agent-runner/run/auth-profile-failure-policy.ts)
  • openperf: Recent auth cooldown isolation work touched the policy path that already classifies format as session-local rather than shared auth-profile health. (role: adjacent auth-profile policy contributor; confidence: medium; commits: 5b21384ab625; files: src/agents/embedded-agent-runner/run/auth-profile-failure-policy.ts, src/agents/embedded-agent-runner/run/auth-profile-failure-policy.test.ts)
  • steipete: Recent failover-helper documentation and this PR branch's focused auto-reply coverage make this a useful routing candidate for the final merge review. (role: recent adjacent contributor; confidence: medium; commits: fa6be505efc9, 663e2c3869dd; files: src/agents/failover-error.ts, src/auto-reply/reply/agent-runner-execution.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jun 23, 2026
@steipete steipete self-assigned this Jun 26, 2026
@steipete

steipete commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Land-ready verification complete.

  • Confirmed the typed failure-copy owner is the narrowest boundary: format failures keep their diagnostic and omit login/configure recovery guidance; auth-capable failures are unchanged.
  • Focused proof: node scripts/run-vitest.mjs src/agents/auth-profiles/failure-copy.test.ts src/auto-reply/reply/agent-runner-execution.test.ts — 215 tests passed.
  • Fresh autoreview: no accepted/actionable findings.
  • Repository exact-head hosted CI/Testbox gates passed for 663e2c3869dd3dbcd49044179eb034d323250c40 after rebasing onto current main.

No known proof gaps.

@steipete
steipete force-pushed the fix-format-auth-profile-recovery-hint branch from 90748af to 663e2c3 Compare June 26, 2026 23:15
@steipete
steipete merged commit 6db4624 into openclaw:main Jun 26, 2026
93 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Merged via squash.

wangmiao0668000666 pushed a commit to wangmiao0668000666/openclaw that referenced this pull request Jun 27, 2026
* fix(auth): suppress recovery hint for format failures

* test(auth): cover format failure recovery copy

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 27, 2026
* fix(auth): suppress recovery hint for format failures

* test(auth): cover format failure recovery copy

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
xydigit-zt pushed a commit to xydigit-zt/xydigit-zt-openclaw that referenced this pull request Jun 28, 2026
* fix(auth): suppress recovery hint for format failures

* test(auth): cover format failure recovery copy

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
* fix(auth): suppress recovery hint for format failures

* test(auth): cover format failure recovery copy

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants