Skip to content

fix(skill-store): bound discovery requests with an HTTP timeout - #459

Open
polylane[bot] wants to merge 1 commit into
mainfrom
polylane/autofix/wktrws6btn5d
Open

polylane[bot] wants to merge 1 commit into
mainfrom
polylane/autofix/wktrws6btn5d

Conversation

@polylane

@polylane polylane Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Fixes: Skill Store discovery requests have no HTTP timeout

Skill Store search, the popular list, and skill details load over an HTTP client that had no request timeout. When the upstream accepts the connection and then never answers, the request never returns, so those screens stay busy and show no error. After this change each request gives up after eight seconds and reports a normal connection error, so the store can recover.

flowchart LR
  UI["Skill Store UI (busy state)"] -->|invoke| CMD["Tauri: search_skills / get_popular_skills / get_skill_details"]
  CMD -->|no timeout wrapper| API["api::* then client_for"]
  API -->|pre-fix: Client::new, no timeout| UP["skills.sh or the proxy"]
  UP -.->|accepts, never replies| API
  API -->|fix: Client::builder().timeout 8s| UP
  BG["skill_install_counts (background)"] -->|tokio::time::timeout 8s| UP
Loading
Detection evidence

Affected: int_0c37b2126001vkbz84c78wfn

Why this fix

The client behind every Skill Store discovery request was built with no timeout, while the same crate bounds the identical background fetch at eight seconds for the stated reason that a hung connection must not hold a lookup open. A server that accepts a connection and never replies reproduced the hang: without the change the request never returns, with it the request fails at eight seconds. The eight-second bound matches the existing background fetches, and those responses are short JSON, so it does not cut off a healthy response.

Fix walkthrough

Watch the fix walkthrough

Watch the fix explained (1:06). Recorded for commit 482f88e7555242e64ed24afa3798e9f0222ea98c.

Out of scope
  • github_skill_listing.rs#HttpGithubApi::get_json also builds a client with no timeout, but it serves the GitHub-skill listing flow, not Skill Store discovery. Recorded as a follow-up on the issue.
  • The background skill_install_counts fetches already carry their own bound, so they were left as they are.
Causal chain
  • Signal (alert): Skill Studio: Skill Store discovery fetches have no HTTP timeout
  • Surfacing site: sergical/skill-studio desktop app at sergical/skill-studio:apps/desktop/src-tauri/src/skills/api.rs#client_for
  • Mechanism: client_for built reqwest::Client::new(), whose default is no request timeout. When skills.sh or the proxy accepts the TCP connection and never sends a response, the request future never completes, so the Tauri command the UI awaits never resolves.
  • Producer: The Skill Store discovery HTTP client in the desktop app, at sergical/skill-studio:apps/desktop/src-tauri/src/skills/api.rs#client_for
  • Trigger: A stalled upstream (skills.sh in Direct mode, or the Skill Studio server in Server mode) that accepts the connection and never answers.
  • What happens to the failed unit today: The crate already bounds the background install-count fetches with tokio::time::timeout(FETCH_TIMEOUT, ...) at 8 s, and telemetry.rs bounds its client; the interactive discovery commands had no bound.
  • Blast radius: unknown other resource(s), unknown other tenant(s); data at risk: none named
  • Producer evidence:
    • api.rs#client_for returned reqwest::Client::new() with no timeout
    • The three Tauri commands in commands.rs call api:: with no tokio::time::timeout wrapper
    • skill_install_counts.rs bounds the same class of fetch at 8 s with the reason that a hung connection must not hold a lookup open
1 file changed (+64/-2)
  • apps/desktop/src-tauri/src/skills/api.rs: modified, +64/-2

View thread View autofix


Generated by Polylane. You can ask follow-ups by mentioning @polylane in a comment.

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
@polylane
polylane Bot requested a review from sergical as a code owner October 21, 2026 01:39
@polylane polylane Bot added polylane severity:medium Polylane autofix severity: medium labels Oct 21, 2026
@polylane

polylane Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Warning

Polylane could not verify the production impact of this pull request.

The added 8 s bound matches the 8 s bound this crate already puts on the same upstream's background fetches, and 72 h of errors and spans for the four skill-studio Sentry projects returned empty. I found no regression path.

View the full analysis →

Also considered · 2 refuted
  • Refuted · 8 s discovery bound cuts off a healthy but slow response · The crate already bounds the same class of fetch to the same host at 8 s (skill_install_counts.rs:36), the discovery responses are short JSON rather than file bodies, and the serg.tech read for skill-studio-desktop-rust returned zero error events and zero spans over 72 h, so no observed response approaches the bound.
  • Refuted · Timeout failures amplify load on the upstream · A transport failure returns Err from the Tauri command with no retry, backoff or resend path in the resulting code, so an aborted request is not re-issued and adds no upstream load.
Analysed against 1 integration and 1 repository

View in Polylane Pause assessments

Polylane has no cloud accounts connected for this workspace, so it cannot see what this repository deploys to. Connect a cloud and every pull request will be checked against your production resources.

Connect a cloud

Polylane analysed 482f88e for production impact. You can ask follow-ups by mentioning @polylane in a comment.

Did this help? React 👍 or 👎 so the next review is sharper.

@polylane

polylane Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Suggested observability improvements

apps/desktop/src-tauri/src/skills/api.rs:223

  • Gap: connection_error maps every transport failure to the same message, so a request that a stalled upstream abandons at the new 8 s bound is reported exactly like a refused connection, and neither the toast nor any log distinguishes the two.
  • Change: Branch on e.is_timeout() in connection_error and return a message that names the 8 s request bound as the cause.
  • Answers: How many Skill Store discovery failures are stalls at the 8 s bound rather than connection refusals?

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

Labels

polylane severity:medium Polylane autofix severity: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant