Skip to content

fix: abort hung Hermit content-rights proxy fetches - #3579

Open
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/hermit-content-rights-timeout
Open

fix: abort hung Hermit content-rights proxy fetches#3579
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/hermit-content-rights-timeout

Conversation

@SebTardif

@SebTardif SebTardif commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where staff using the admin content-rights proxy would hang the Convex action when forms.openclaw.ai (or HERMIT_CONTENT_RIGHTS_BASE_URL) accepted the TCP connection and never finished the GET case or POST correspondence response. The proxy called fetch with no abort signal. A stalled Hermit host held the action until the platform limit.

The same hang class was already closed for public OG metadata fetches in PR #3471 and for registry discovery in PR #3378. The admin Hermit proxy did not get that deadline.

Why This Change Was Made

Both Hermit fetch calls now pass AbortSignal.timeout(10_000). Ten seconds is long enough for an admin form POST with attachments, and short enough that a silent Hermit host cannot pin a Convex action. The existing catch path still returns HTTP 502 Hermit content rights service unavailable. Missing CLAWHUB_BAN_APPEALS_TOKEN is still 503 and never contacts Hermit.

User Impact

Staff content-rights GET and correspondence POST fail closed after 10 seconds when Hermit stalls, instead of sitting on the action until Convex kills it. Successful Hermit replies are unchanged.

Evidence

Live bun on Darwin arm64, Node v26.7.0, worktree /tmp/clawhub-F005 at 6a6c06597ae17451c8aa47254e6a967119df2ab4. Script /tmp/clawhub-f005-proof.mjs starts a local node:http server that never writes a response, then:

  1. fetch(hangUrl) with no signal against an 800ms harness
  2. fetch(hangUrl, { signal: AbortSignal.timeout(400) })
  3. Production proxyHermitContentRightsRequest with a never-settling fetch that rejects only when init.signal aborts
$ bun /tmp/clawhub-f005-proof.mjs
timeout_ms 10000
hang_url http://127.0.0.1:61333/hang
unpatched_fetch Error HARNESS_TIMEOUT elapsed_ms 808
timeout_fetch TimeoutError The operation timed out. elapsed_ms 405
proxy_status 502 proxy_body Hermit content rights service unavailable elapsed_ms 10004

Without a signal, the mute origin is still waiting when the 800ms harness fires. AbortSignal.timeout(400) aborts in 405ms. The product proxy, using the 10s constant, returns 502 at 10004ms instead of hanging.

The untimeouted fetches landed in 3c9d4f12109bfd9b3ef8a42033228770f1bbe1d1 (origin/main blame on convex/httpApiV1/contentRightsV1.ts GET around line 48 and POST around line 58), shipped today in #3493.

Real behavior proof

  • Behavior or issue addressed: Admin content-rights Hermit proxy now aborts a stalled forms.openclaw.ai fetch after 10 seconds and returns 502 instead of holding the Convex action.
  • Real environment tested: macOS Darwin 25.6.0 arm64, Node v26.7.0, bun 1.3.14, worktree /tmp/clawhub-F005 on 6a6c06597ae17451c8aa47254e6a967119df2ab4. Proof used a local mute node:http server plus production proxyHermitContentRightsRequest.
  • Exact steps or command run after this patch: From /tmp/clawhub-F005, ran bun /tmp/clawhub-f005-proof.mjs. That process listened on an ephemeral port, issued an unbounded fetch against the mute server, issued fetch with AbortSignal.timeout(400), then called the production proxy with a fetch that only rejects when the abort signal fires.
  • Evidence after fix: terminal output copied above. Unbounded fetch hit HARNESS_TIMEOUT at 808ms. AbortSignal.timeout(400) printed TimeoutError The operation timed out. at 405ms. The product proxy printed proxy_status 502 and Hermit content rights service unavailable at 10004ms.
  • Observed result after fix: A mute Hermit-style origin no longer holds the proxy. The 10s AbortSignal.timeout fires, and staff get HTTP 502 from the existing unavailable path.
  • What was not tested: A live stall on forms.openclaw.ai. Windows. A real admin token against production Hermit.

Notes

Same hang class as OG metadata in PR #3471 (1.5s). This admin proxy uses 10s because correspondence POST can carry attachments.

Allow edits from maintainers is enabled.

Tracker

Ref #3671

That issue stays open if this PR is closed without landing on main.

@SebTardif
SebTardif requested review from a team and Patrick-Erichsen as code owners September 2, 2026 19:30
@clawsweeper

clawsweeper Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@SebTardif is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 2, 2026
@clawsweeper

clawsweeper Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed September 11, 2026, 10:08 AM ET / 14:08 UTC (Revision 28).

ClawSweeper review

What this changes

Adds a 10-second timeout to staff content-rights case reads and correspondence uploads, with a mocked stalled-request regression test.

Merge readiness

Blocked before merge - 8 items remain

The fix remains necessary: current main and v0.23.3 still leave Hermit requests unbounded. The three prior findings remain unresolved, and the supplied terminal evidence only partially exercises the changed production path.

Priority: P2
Reviewed head: ad060dd43844c774a20bab8d970e5aa2749b3d69
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and useful, but unresolved error handling, upload compatibility, and partial runtime evidence prevent merge readiness.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The macOS/Bun trace separately demonstrates native fetch cancellation and a 502 from the production helper using mocked transport. It does not connect real transport to the Convex HTTP action or demonstrate successful attachment uploads and body-stall recovery, as applicable Convex guidance requires. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 3 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The macOS/Bun trace separately demonstrates native fetch cancellation and a 502 from the production helper using mocked transport. It does not connect real transport to the Convex HTTP action or demonstrate successful attachment uploads and body-stall recovery, as applicable Convex guidance requires. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 9 items Current main still needs the fix: Both outbound fetches lack a signal on the fetched main revision; the handler supplies native fetch after API-token authentication and an admin check.
Release comparison: The supplied latest release, v0.23.3, also contains both unbounded fetches.
Response-body rejection escapes error handling: The introduced signals can abort response.text(), but both branches return proxyResponse without awaiting it inside the try block. A headers-then-body stall therefore rejects the HTTP action instead of returning the intended 502.
Findings 3 actionable findings [P1] Use an upload-compatible correspondence deadline
[P2] Await response-body conversion inside the 502 boundary
[P3] Record the proxy timeout and recovery contract in specs
Security None None.

How this fits together

ClawHub’s admin HTTP API authenticates staff requests and forwards case reads and correspondence attachments to Hermit. It then converts Hermit’s response into the response returned to the admin client.

flowchart LR
  A[Admin CLI] --> B[Authenticate and authorize staff]
  B --> C[Content-rights proxy]
  C --> D[Hermit case service]
  D --> E[Read response body]
  C --> F[Deadline expires]
  E --> G[Admin response]
  F --> G
Loading

Decision needed

Question Recommendation
What bounded deadline should correspondence uploads use, given the existing client's 120-second upload allowance? Use separate read and upload budgets: Retain the short GET deadline and select a longer bounded correspondence deadline validated against representative attachments and Convex limits.

Why: Source proves that the new 10-second cutoff can reject previously successful uploads, but neither representative upload evidence nor an approved service latency contract establishes the right replacement budget.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The macOS/Bun trace separately demonstrates native fetch cancellation and a 502 from the production helper using mocked transport. It does not connect real transport to the Convex HTTP action or demonstrate successful attachment uploads and body-stall recovery, as applicable Convex guidance requires. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Use an upload-compatible correspondence deadline (P1) - The new shared 10-second signal also cuts off attachment uploads. The existing admin command uses apiRequestForm with a 120-second upload allowance, so a healthy Hermit request completing after 10 seconds now fails. Use a separately justified upload budget and validate representative attachments before narrowing this existing workflow.
  • Await response-body conversion inside the 502 boundary (P2) - If Hermit sends headers and then stalls, the new timeout aborts response.text() inside proxyResponse. Both branches return that promise without awaiting it, so its rejection bypasses this try/catch and rejects the HTTP action instead of returning the promised 502. Await proxyResponse in both branches and cover a headers-then-body stall.
  • Record the proxy timeout and recovery contract in specs (P3) - This introduces a timeout policy for content-rights reads and correspondence recording, but the branch changes only source and tests. Root AGENTS.md requires changed subsystem intent to live in specs/. Add a focused note covering the approved budgets, complete response-body handling, and verification before retrying an uncertain correspondence result.
  • Resolve merge risk (P1) - Existing correspondence uploads that need more than 10 seconds on the Hermit leg will now fail; representative attachment latency and the intended upload budget remain unproven.
  • Resolve merge risk (P2) - An aborted correspondence request can leave the client uncertain whether Hermit already recorded it; the timeout recovery contract should specify verification before retrying.
  • Complete next step (P2) - Resolve the three retained findings and provide real Convex HTTP proof with native transport. Redact tokens, private endpoints, IP addresses, and case details from posted evidence.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P1] Use an upload-compatible correspondence deadline — convex/httpApiV1/contentRightsV1.ts:66
  • [P2] Await response-body conversion inside the 502 boundary — convex/httpApiV1/contentRightsV1.ts:50-54
  • [P3] Record the proxy timeout and recovery contract in specs — convex/httpApiV1/contentRightsV1.ts:17-18
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch size +77/-2 across 2 files; production +5/-0, tests +72/-2 Production growth is narrowly justified by the deadline, while the new timeout regression covers only a mocked pre-header GET stall.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3671
Summary: This PR is the candidate implementation for the open Hermit timeout tracker; neighboring timeout fixes affect different request owners.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Validate a correspondence-specific budget (recommended)
    Separate the read and upload deadlines, obtain approval for the upload budget, and demonstrate successful attachment recording under it.
  2. Accept the tighter upload contract
    Explicitly accept that uploads exceeding 10 seconds will fail and document verification before retries.

Technical review

Best possible solution:

Bound complete Hermit responses inside the 502 error boundary, preserve an explicitly approved attachment-upload budget, and document how staff verify an uncertain correspondence result before retrying.

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

Yes, from source: current main has no outbound deadline, and the patch’s body-read rejection escapes its catch block. No runtime reproduction was executed during this read-only review.

Is this the best way to solve the issue?

Not yet: the proxy is the right repair location, but complete body consumption must be awaited and the upload budget resolved. A CLI-only timeout would leave the Convex action running; the unrelated OG helper is not a substitute for fixing this owner.

Full review comments:

  • [P1] Use an upload-compatible correspondence deadline — convex/httpApiV1/contentRightsV1.ts:66
    The new shared 10-second signal also cuts off attachment uploads. The existing admin command uses apiRequestForm with a 120-second upload allowance, so a healthy Hermit request completing after 10 seconds now fails. Use a separately justified upload budget and validate representative attachments before narrowing this existing workflow.
    Confidence: 0.93
  • [P2] Await response-body conversion inside the 502 boundary — convex/httpApiV1/contentRightsV1.ts:50-54
    If Hermit sends headers and then stalls, the new timeout aborts response.text() inside proxyResponse. Both branches return that promise without awaiting it, so its rejection bypasses this try/catch and rejects the HTTP action instead of returning the promised 502. Await proxyResponse in both branches and cover a headers-then-body stall.
    Confidence: 0.99
  • [P3] Record the proxy timeout and recovery contract in specs — convex/httpApiV1/contentRightsV1.ts:17-18
    This introduces a timeout policy for content-rights reads and correspondence recording, but the branch changes only source and tests. Root AGENTS.md requires changed subsystem intent to live in specs/. Add a focused note covering the approved budgets, complete response-body handling, and verification before retrying an uncertain correspondence result.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against cbfee7343ddc.

Labels

Label justifications:

  • P2: This fixes a bounded staff API availability problem without evidence of a current broad outage.
  • merge-risk: 🚨 compatibility: The new 10-second outbound deadline can reject attachment uploads that previously completed within the existing client's longer allowance.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The macOS/Bun trace separately demonstrates native fetch cancellation and a 502 from the production helper using mocked transport. It does not connect real transport to the Convex HTTP action or demonstrate successful attachment uploads and body-stall recovery, as applicable Convex guidance requires. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Current main still needs the fix: Both outbound fetches lack a signal on the fetched main revision; the handler supplies native fetch after API-token authentication and an admin check. (convex/httpApiV1/contentRightsV1.ts:48, cbfee7343ddc)
  • Release comparison: The supplied latest release, v0.23.3, also contains both unbounded fetches. (convex/httpApiV1/contentRightsV1.ts:48, 87ca030c30f3)
  • Response-body rejection escapes error handling: The introduced signals can abort response.text(), but both branches return proxyResponse without awaiting it inside the try block. A headers-then-body stall therefore rejects the HTTP action instead of returning the intended 502. (convex/httpApiV1/contentRightsV1.ts:50, ad060dd43844)
  • Fetch cancellation contract: The patch directly uses AbortSignal with fetch. MDN documents that aborting after headers have arrived can reject response-body consumption: AbortSignal reference.
  • Existing attachment workflow: The admin correspondence command sends attachments through apiRequestForm with retries disabled. That client uses the 120,000ms upload budget in packages/clawhub/src/http.ts; the PR introduces a separate 10,000ms cutoff on the outbound Hermit leg. (packages/clawhub-admin/src/commands/contentRights.ts:105, ad060dd43844)
  • Captured proof and review continuity: The supplied body at sourceRevision 401fd84394114473f10eaf9d9c92cb1f34bd9c4356862822d0fdb0002f38a583 reports native fetch against a mute local server separately from the production proxy using an injected abort-aware fetch. The latter returns 502 after 10004ms. This is useful partial evidence, but does not exercise native transport through the Convex HTTP action or attachment/body-stall behavior. The prior completed review covered the same head and named all three retained findings. The inspected downloaded images concern the unrelated traffic-signals PR. (ad060dd43844)

Likely related people:

  • Patrick Erichsen: Raw commit 89bb8fe adds convex/httpApiV1/contentRightsV1.ts:47 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 89bb8fe93864; files: convex/httpApiV1/contentRightsV1.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Await both complete response conversions and add headers-then-body-stall regression coverage.
  • Approve an attachment-compatible deadline and record the timeout and retry-verification contract in specs.
  • Add redacted real-Convex HTTP evidence using native transport for successful and stalled GET and attachment POST requests, including body stalls; update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (27 earlier review cycles; latest 8 shown)
  • reviewed 2026-09-04T04:52:03.915Z sha ad060dd :: needs real behavior proof before merge. :: [P2] Await forwarded response bodies inside the 502 boundary | [P3] Record the Hermit timeout invariant in specs
  • reviewed 2026-09-04T05:23:49.719Z sha ad060dd :: needs real behavior proof before merge. :: [P1] Preserve the correspondence upload timeout | [P2] Await forwarded response bodies inside the 502 boundary | [P3] Record the Hermit timeout invariant in specs
  • reviewed 2026-09-04T06:57:17.917Z sha ad060dd :: needs real behavior proof before merge. :: [P1] Use an upload-compatible correspondence deadline | [P2] Await forwarded response bodies inside the 502 boundary | [P3] Document the Hermit timeout contract in specs
  • reviewed 2026-09-04T09:56:34.606Z sha ad060dd :: needs real behavior proof before merge. :: [P1] Preserve correspondence upload completion | [P2] Keep response-body reads inside the error boundary | [P3] Record the Hermit timeout contract in specs
  • reviewed 2026-09-04T11:23:42.481Z sha ad060dd :: needs real behavior proof before merge. :: [P1] Preserve correspondence uploads beyond the GET timeout | [P2] Await the forwarded body inside the 502 boundary | [P3] Record the Hermit timeout contract in specs
  • reviewed 2026-09-04T13:11:37.332Z sha ad060dd :: needs real behavior proof before merge. :: [P1] Use an upload-compatible correspondence deadline | [P2] Await forwarded body reads inside 502 handling | [P3] Record the Hermit timeout contract in specs
  • reviewed 2026-09-04T15:04:50.276Z sha ad060dd :: needs real behavior proof before merge. :: [P1] Use an upload-compatible correspondence deadline | [P2] Await proxied response bodies inside the 502 boundary | [P3] Record the Hermit timeout contract in specs
  • reviewed 2026-09-05T01:05:59.307Z sha ad060dd :: needs real behavior proof before merge. :: [P1] Use an upload-compatible correspondence deadline | [P2] Await response-body conversion inside the 502 boundary | [P3] Record the proxy timeout contract in specs

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Sep 3, 2026
The admin content-rights proxy called fetch with no AbortSignal.
A stalled forms.openclaw.ai host could hold the Convex action
until the platform limit. Pass AbortSignal.timeout on both
Hermit fetches.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@SebTardif
SebTardif force-pushed the fix/hermit-content-rights-timeout branch from 6a6c065 to ad060dd Compare September 3, 2026 16:33
@SebTardif

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. P2 Normal backlog priority with limited blast radius. labels Sep 3, 2026
@clawsweeper clawsweeper Bot added P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal backlog priority with limited blast radius. and removed P1 High-priority user-facing bug, regression, or broken workflow. labels Sep 4, 2026
@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. label Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant