Skip to content

fix: time out well-known agent-skills upstream fetch - #3621

Open
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/well-known-agent-skills-timeout
Open

fix: time out well-known agent-skills upstream fetch#3621
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/well-known-agent-skills-timeout

Conversation

@SebTardif

@SebTardif SebTardif commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where users or installers requesting /{owner}/skills/{slug}/.well-known/agent-skills/index.json would hang when the Convex /api/v1/agent-skills/.../index.json upstream accepted the connection but never answered. The TanStack server handler fetched that URL with no abort signal, so the Node request stayed open until the platform limit.

Why This Change Was Made

The GET and HEAD handlers now pass AbortSignal.timeout(10_000) on the upstream fetch. That matches the 10s outbound deadline already used for changelog and embeddings fetches. The change is only the missing deadline; response header filtering is unchanged.

User Impact

A stalled Convex discovery response now fails instead of holding the well-known skill page request until the host times out. Successful lookups are unchanged.

Evidence

Live bun import of fetchAgentSkillsDiscovery from this branch. Fetch hung until the abort signal fired. The call rejected with TimeoutError in 84ms (proof used an 80ms abort stand-in for the 10s product timeout). The handler still requested 10000ms.

$ bun well-known-timeout-proof.mjs
requestedTimeoutMs=10000
productTimeoutMs=10000
proofBudgetMs=80
elapsedMs=84
errorName=TimeoutError
OK: hanging upstream fetch aborted inside the proof budget

Unfixed main fetchAgentSkillsDiscovery has no signal on the Convex fetch. Sibling OG helpers already abort hung public API reads. CLI registry discovery already times out (#3378).

Real behavior proof

  • Behavior or issue addressed: The well-known Agent Skills discovery proxy now aborts a hung Convex fetch after 10s instead of holding the Node request until the platform limit.
  • Real environment tested: Windows 11, Bun, worktree fix/well-known-agent-skills-timeout on origin/main d3bde70e, live bun importing fetchAgentSkillsDiscovery with a hung fetch.
  • Exact steps or command run after this patch: bun well-known-timeout-proof.mjs from the patched worktree (proof helper not committed).
  • Evidence after fix: terminal output copied below.
$ bun well-known-timeout-proof.mjs
requestedTimeoutMs=10000
productTimeoutMs=10000
proofBudgetMs=80
elapsedMs=84
errorName=TimeoutError
OK: hanging upstream fetch aborted inside the proof budget
  • Observed result after fix: The hung upstream fetch aborted. Outcome is TimeoutError. Elapsed time is 84ms, not the platform request limit. Product timeout remains 10000ms.
  • What was not tested: A live production Convex outage during a real installer lookup.

Summary

This hang has been present since 7aff40d2 in #3233 (2026-07-22). Related same-repo timeouts: #3471 (OG), #3378 (CLI registry discovery), #3579 (Hermit), #3615 (changelog OpenAI).

Tracker

Ref #3676

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

GET/HEAD /{owner}/skills/{slug}/.well-known/agent-skills/index.json
fetched Convex with no AbortSignal, so a stalled upstream held the
Node request until the platform limit.

Pass AbortSignal.timeout(10s) on the upstream fetch.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@vercel

vercel Bot commented Sep 7, 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 commented Sep 7, 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.

@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 7, 2026
@clawsweeper

clawsweeper Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

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

ClawSweeper review

What this changes

Adds a ten-second upstream deadline to the skill discovery endpoint’s GET and HEAD requests, with tests for signal propagation and cancellation.

Merge readiness

Blocked before merge - 2 items remain

The fix remains necessary: current main and v0.23.3 still lack this timeout. No blocking code findings; the previously requested real-transport proof remains outstanding.

Priority: P2
Reviewed head: b17c30c301b6e931dc1f8867ab22310ffbb176d6

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused patch has no blocking correctness findings, but the supplied trace does not yet satisfy real-transport proof.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: The Windows/Bun trace exercises the production discovery helper and records accelerated timeout rejection, but does not establish a real HTTP transport request or successful GET/HEAD proxy behavior. The prior request for a redacted production-helper trace against a stalled local origin remains applicable; a production Convex outage is unnecessary. 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 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The Windows/Bun trace exercises the production discovery helper and records accelerated timeout rejection, but does not establish a real HTTP transport request or successful GET/HEAD proxy behavior. The prior request for a redacted production-helper trace against a stalled local origin remains applicable; a production Convex outage is unnecessary. 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 8 items Current main still needs the fix: Both handlers use the same upstream fetch without an abort signal; GET subsequently buffers the response body.
Latest release also lacks the timeout: The route in v0.23.3 contains the same fetch without a signal.
Verified introduced scope: The pinned delta contains only the route timeout and its tests: production +8/-1 and tests +92/-2. Response filtering, URL construction, authorization, dependencies, and workflows are unchanged.
Findings None None.
Security None None.

How this fits together

ClawHub’s skill discovery endpoint lets installers retrieve metadata through a skill-page URL. The website proxies requests to its Convex public API and returns selected headers and the discovery body.

flowchart TD
  A[Installer requests skill metadata] --> B[Website discovery endpoint]
  B --> C[Convex API fetch]
  C --> D{Completes within ten seconds?}
  D -->|Yes| E[Forward status and selected headers]
  E --> F[GET body or empty HEAD response]
  D -->|No| G[Abort and fail request]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The Windows/Bun trace exercises the production discovery helper and records accelerated timeout rejection, but does not establish a real HTTP transport request or successful GET/HEAD proxy behavior. The prior request for a redacted production-helper trace against a stalled local origin remains applicable; a production Convex outage is unnecessary. 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.
  • Complete next step (P2) - Add real-transport proof using the production helper, covering stalled-origin timeout and successful GET/HEAD behavior. Terminal output or logs are sufficient; a terminal screenshot or recording is welcome. Redact private addresses, credentials, and endpoints. Update the PR body to trigger re-review; if it does not, ask a maintainer to comment @clawsweeper re-review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Diff size +100/-3 across 2 files The change stays within one route and its regression tests.
Production versus test LOC Production net +7; tests net +90 Production growth adds the deadline and exposes the existing helper for focused testing.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3676
Summary: This PR is the candidate fix for the paired discovery-timeout tracker; adjacent timeout changes address separate fetches.

Members:

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

Technical review

Best possible solution:

Keep the deadline local to the discovery fetch, preserving successful GET bodies, HEAD semantics, and selected response headers.

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

Yes, from source: both discovery methods await an upstream fetch without an application deadline on current main. A silent HTTP origin exercises that path; this read-only review did not execute it.

Is this the best way to solve the issue?

Yes, this is the narrowest fix for the identified fetch. Reusing the OG helper would import a different timeout policy, while changing CLI discovery would leave this website request unbounded.

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P2: This is a bounded reliability fix for discovery requests when the upstream stalls.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The Windows/Bun trace exercises the production discovery helper and records accelerated timeout rejection, but does not establish a real HTTP transport request or successful GET/HEAD proxy behavior. The prior request for a redacted production-helper trace against a stalled local origin remains applicable; a production Convex outage is unnecessary. 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 handlers use the same upstream fetch without an abort signal; GET subsequently buffers the response body. ([src/routes/$owner/skills/$slug/[.]well-known/agent-skills/index[.]json.ts:17](https://github.com/openclaw/clawhub/blob/cbfee7343ddc867316dd9b3de6fa8856730f9f41/src/routes/%24owner/skills/%24slug/%5B.%5Dwell-known/agent-skills/index%5B.%5Djson.ts#L17), cbfee7343ddc867316dd9b3de6fa8856730f9f41)
  • Latest release also lacks the timeout: The route in v0.23.3 contains the same fetch without a signal. ([src/routes/$owner/skills/$slug/[.]well-known/agent-skills/index[.]json.ts:17](https://github.com/openclaw/clawhub/blob/87ca030c30f3cfb78ab15c8e66b5ff1469c8f9c8/src/routes/%24owner/skills/%24slug/%5B.%5Dwell-known/agent-skills/index%5B.%5Djson.ts#L17), 87ca030c30f3cfb78ab15c8e66b5ff1469c8f9c8)
  • Verified introduced scope: The pinned delta contains only the route timeout and its tests: production +8/-1 and tests +92/-2. Response filtering, URL construction, authorization, dependencies, and workflows are unchanged. ([src/routes/$owner/skills/$slug/[.]well-known/agent-skills/index[.]json.ts:26](https://github.com/openclaw/clawhub/blob/b17c30c301b6e931dc1f8867ab22310ffbb176d6/src/routes/%24owner/skills/%24slug/%5B.%5Dwell-known/agent-skills/index%5B.%5Djson.ts#L26), b17c30c301b6e931dc1f8867ab22310ffbb176d6)
  • Feature history: Comparison against the raw recorded parent shows this route was added by the commit associated with feat: support npx skills installs from skill pages #3233. Blame connects the retained GET handler to that commit. ([src/routes/$owner/skills/$slug/[.]well-known/agent-skills/index[.]json.ts:9](https://github.com/openclaw/clawhub/blob/7aff40d26a898b934dc9553a35683e52f9aa20c7/src/routes/%24owner/skills/%24slug/%5B.%5Dwell-known/agent-skills/index%5B.%5Djson.ts#L9), 7aff40d26a898b934dc9553a35683e52f9aa20c7)
  • Proof inspected and continuity preserved: The complete captured body reports a Windows/Bun import of the production helper, requesting 10000ms with an 80ms timeout stand-in and observing TimeoutError after 84ms. It supplies no helper source or HTTP request trace establishing real fetch against a stalled origin. The live body matches that account; the earlier review requested precisely this transport evidence. The reviewed head is unchanged. (b17c30c301b6)
  • Canonical tracker and distinct adjacent fixes: Agent Skills well-known proxy fetch has no timeout #3676 remains open and explicitly pairs this PR with stalled-origin and successful-response acceptance criteria. The merged fixes at fix: abort registry discovery fetch after a timeout #3378 and fix: abort hung OG metadata fetches and archive metrics #3471 concern CLI registry discovery and OG metadata respectively, not this website route.

Likely related people:

  • Patrick Erichsen: Raw commit 7aff40d adds src/routes/$owner/skills/$slug/[.]well-known/agent-skills/index[.]json.ts:7 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 7aff40d26a89; files: src/routes/$owner/skills/$slug/[.]well-known/agent-skills/index[.]json.ts)
  • SebTardif: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

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

  • Provide redacted output showing the production helper using real fetch against a stalled HTTP origin, with observed timeout and successful GET/HEAD responses.

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 (1 earlier review cycle)
  • reviewed 2026-09-07T21:56:48.121Z sha b17c30c :: needs real behavior proof before merge. :: none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal backlog priority with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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