fix(agents): classify auth HTML provider responses by martingarramon · Pull Request #79900 · openclaw/openclaw

martingarramon · 2026-05-09T17:36:32Z

Summary

Problem: classifyProviderRuntimeFailureKind returns upstream_html ("CDN or gateway blocked the request — retry") for providers that return an HTML body with HTTP 401. 401 HTML auth/login responses land here; users get retry copy when the actual remediation is re-authentication.
Why it matters: The auth_html_403 kind was added previously with the correct re-auth copy for 403s. The 401 arm was simply not branched — errors.ts:939-940 reads return status === 403 ? "auth_html_403" : "upstream_html", so any 401 + HTML body falls through.
What changed: added auth_html_401 to ProviderRuntimeFailureKind, updated the classifier branch, added formatter copy, and two regression tests.
What did NOT change: auth_html_403 formatter, upstream_html path for non-401/403 HTML bodies, isHtmlErrorResponse detection logic.

Change Type: Bug fix
Scope: Auth / tokens
Linked: Related #77394 (same classifyProviderRuntimeFailureKind function, HTML auth classification)

Classifier proof

Source trace: errors.ts:939-940 — the 401 arm was absent from the ternary; a 401 with HTML body enters upstream_html and returns "CDN or gateway blocked the request."

Regression tests assert (1) 401 <!DOCTYPE html>… → re-auth copy, (2) that input does NOT produce "CDN" or "blocked the request."

Structural proof: fix mirrors the existing auth_html_403 pattern exactly.

Real behavior proof

Behavior or issue addressed: HTTP 401 + HTML body (Cloudflare Access login page, nginx basic-auth challenge, gateway login wall) was classified as upstream_html, showing users "CDN or gateway blocked the request — retry." The actual remediation is re-authentication, not retry. This fix adds the missing 401 arm so those responses reach the correct copy.
Real environment tested: Node.js v24.14.0, Ubuntu 24.04 on WSL2. Logic exercised using the real HTML_BODY_RE / HTML_CLOSE_RE constants and the exact classifier branch from commit 128c615.
Exact steps or command run after this patch:
```
node proof-401.mjs
```
Script calls classifyProviderRuntimeFailureKind and formatAssistantErrorText with a 401 HTML body, using the real constants from src/agents/pi-embedded-helpers/errors.ts.

Evidence after fix:

$ node proof-401.mjs

=== fix/agents-html-401-auth-misroute — behavior trace ===

Input: HTTP 401 + HTML body (Cloudflare Access / nginx auth wall)
Body (first 80 chars): <!DOCTYPE html>
<html><head><title>401 Unauthorized</title></head>
<body><h1>Una...

BEFORE (original ternary: 403 → auth_html_403, else → upstream_html)
  classifyProviderRuntimeFailureKind → upstream_html
  formatAssistantErrorText            → CDN or gateway blocked the request — retry

AFTER  (fix adds 401 arm before fallback)
  classifyProviderRuntimeFailureKind → auth_html_401
  formatAssistantErrorText            → Authentication failed with an HTML 401 response from the provider. Re-authenticate and verify your provider credentials.

Observed result after fix: classifyProviderRuntimeFailureKind returns auth_html_401 for a 401 + HTML body (previously upstream_html). formatAssistantErrorText returns re-authentication copy, not CDN retry copy.
What was not tested: End-to-end through a live provider network call. The classifier receives an already-fetched response body and HTTP status code — the path from those two inputs to the returned kind is deterministic and was exercised by the above trace.

clawsweeper · 2026-05-09T17:39:56Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR maps HTTP 401/403 HTML auth provider responses to a unified auth_html runtime failure kind, updates re-authentication copy, suppresses raw auth HTML in console suffixes, and adds regression coverage plus a changelog entry.

Reproducibility: yes. from source inspection. Current main accepts 401 HTML as an HTML error response but only status 403 maps to auth copy, so 401 falls through to upstream_html; I did not run tests because this review was read-only.

PR rating
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Summary: Focused, likely mergeable bug fix with sufficient terminal proof and targeted regression coverage; latest-head CI completion remains the main merge gate.

Rank-up moves:

Confirm the in-progress required checks on head 1c5edafe2a5639ad0374cf12e0a295fb8f769bb0 complete successfully before merge.

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.

Real behavior proof
Sufficient (terminal): Sufficient terminal proof: the PR body shows deterministic post-patch classifier/formatter output changing a 401 HTML body from CDN retry copy to re-authentication copy, with private data absent from the proof.

Risk before merge

Required CI on head 1c5edafe2a5639ad0374cf12e0a295fb8f769bb0 was still in progress during this read-only review.

Maintainer options:

Decide the mitigation before merge
Land the focused classifier, formatter, raw-console suppression, and regression-test update after required CI completes successfully on the latest head.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No repair lane is needed; the branch has no concrete code finding and should proceed through maintainer review once required CI finishes.

Security
Cleared: The diff narrows auth error classification and suppresses raw auth HTML console suffixes without changing dependencies, workflows, permissions, secret handling, or code-execution surfaces.

Review details

Best possible solution:

Land the focused classifier, formatter, raw-console suppression, and regression-test update after required CI completes successfully on the latest head.

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

Yes, from source inspection. Current main accepts 401 HTML as an HTML error response but only status 403 maps to auth copy, so 401 falls through to upstream_html; I did not run tests because this review was read-only.

Is this the best way to solve the issue?

Yes. Extending the existing HTML auth branch to include 401 and centralizing auth HTML console-suffix suppression is narrower than changing provider routing, and the PR preserves upstream_html for non-auth HTML statuses.

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. Sufficient terminal proof: the PR body shows deterministic post-patch classifier/formatter output changing a 401 HTML body from CDN retry copy to re-authentication copy, with private data absent from the proof.

Label justifications:

P2: This is a focused agents/auth bug fix with clear source reproduction and limited runtime blast radius.
rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🦞 diamond lobster, patch quality is 🐚 platinum hermit, and Focused, likely mergeable bug fix with sufficient terminal proof and targeted regression coverage; latest-head CI completion remains the main merge gate.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Sufficient terminal proof: the PR body shows deterministic post-patch classifier/formatter output changing a 401 HTML body from CDN retry copy to re-authentication copy, with private data absent from the proof.
proof: sufficient: Contributor real behavior proof is sufficient. Sufficient terminal proof: the PR body shows deterministic post-patch classifier/formatter output changing a 401 HTML body from CDN retry copy to re-authentication copy, with private data absent from the proof.

What I checked:

Current main misclassifies 401 HTML auth bodies: On current main, isHtmlErrorResponse accepts 4xx HTML responses, but classifyProviderRuntimeFailureKind only maps status 403 to auth_html_403; every other HTML status, including 401, returns upstream_html. (src/agents/pi-embedded-helpers/errors.ts:978, 016c34ff1d2a)
PR diff fixes the classifier and formatter path: The latest PR diff changes the failure kind to auth_html, maps status 401 or 403 HTML responses to that kind, and updates the formatter to return credential/account re-authentication copy while preserving upstream_html for other HTML statuses. (src/agents/pi-embedded-helpers/errors.ts:976, 1c5edafe2a56)
PR adds focused regression coverage: The diff adds direct classifier/formatter tests for 401 HTML auth responses and log-suppression tests for lifecycle and failover console messages so raw HTML auth bodies do not appear in console suffixes. (src/agents/pi-embedded-helpers.isbillingerrormessage.test.ts:1510, 1c5edafe2a56)
Real behavior proof supplied in PR body: The PR body includes terminal output from a Node v24.14.0 proof script showing a 401 HTML body changing from upstream_html with CDN retry copy to an auth classification with re-authentication copy; the latest refactor unifies the kind name as auth_html but preserves the central behavior. (1c5edafe2a56)
Latest head still had CI in progress: Most check runs on head 1c5edafe2a5639ad0374cf12e0a295fb8f769bb0 were successful or skipped, but several relevant checks were still in progress during this review, including Critical Quality (network-runtime-boundary) and multiple checks-node-agentic-* lanes. (1c5edafe2a56)
Related prior HTML classification work: The merged related PR https://github.com/openclaw/openclaw/pull/67642 introduced the current broader HTML response handling, preserving 403 as auth HTML while adding upstream_html for non-403 HTML provider pages; this PR extends that behavior for 401 auth HTML responses. (src/agents/pi-embedded-helpers/errors.ts:978, e588e904a744)

Likely related people:

altaywtf: Recent main history shows embedded error observation and provider failure classification work with @altaywtf as author/coauthor/reviewer, and this branch also includes follow-up commits centralizing auth HTML console suppression. (role: recent area contributor; confidence: high; commits: 87d939be7936, 016c34ff1d2a, 1c5edafe2a56; files: src/agents/pi-embedded-error-observation.ts, src/agents/pi-embedded-runner/run/failover-observation.ts, src/agents/pi-embedded-subscribe.handlers.lifecycle.ts)
stainlu: The merged HTML classification PR added upstream_html for non-403 HTML provider pages while preserving 403 auth classification, which is the behavior this PR narrows for 401 auth HTML responses. (role: introduced related behavior; confidence: medium; commits: e588e904a744, a456f98c916e; files: src/agents/pi-embedded-helpers/errors.ts, src/agents/pi-embedded-helpers/provider-error-patterns.test.ts)
steipete: Recent file history shows repeated work on embedded agent lifecycle and error-adjacent runtime paths, making @steipete a useful routing candidate for operational copy and logging behavior. (role: recent adjacent contributor; confidence: medium; commits: f600e98e5bad, 9e9df8f2c578, f1bdfca1edc4; files: src/agents/pi-embedded-subscribe.handlers.lifecycle.ts, src/agents/pi-embedded-helpers/errors.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 016c34ff1d2a.

martingarramon · 2026-05-20T10:50:22Z

Friendly ping — anything blocking a review here?

clawsweeper · 2026-05-21T18:42:05Z

ClawSweeper PR egg

✨ Hatched: 🥚 common Cosmic Review Wisp

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: stacks clean commits.
Image traits: location flaky test forest; accessory CI status badge; palette violet, aqua, and starlight; mood bright-eyed; pose stepping out of a freshly hatched shell; shell frosted glass shell; lighting calm overcast light; background smooth stones and checkmarks.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Cosmic Review Wisp in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

…am_html A provider returning an HTML 401 body (Cloudflare Access login page, nginx basic-auth challenge, gateway login wall) was previously classified as `upstream_html` and shown the CDN-blocked retry copy to the end user. Add `auth_html_401` kind; update classifier to branch on status === 401 before the fallback `upstream_html` arm; add matching formatter copy and two regression tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

altaywtf · 2026-05-21T20:29:00Z

Merged via squash.

Prepared head SHA: b00513414d5d2361bb212a0ffb3ac68c6f94fe00
Merge commit: 7f4462e5c06d5b87c8eee4380dec1eff9fcbf691

Thanks @martingarramon!

@altaywtf