Skip to content

fix(selfhost): bound s3-blob-store fetch calls with a per-attempt timeout (#8362)#8442

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
joaovictor91123:fix/s3-blob-store-fetch-timeout-8362
Jul 24, 2026
Merged

fix(selfhost): bound s3-blob-store fetch calls with a per-attempt timeout (#8362)#8442
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
joaovictor91123:fix/s3-blob-store-fetch-timeout-8362

Conversation

@joaovictor91123

Copy link
Copy Markdown
Contributor

Summary

  • This repo's self-host backend adapters follow a bounded-fetch convention: every external network call carries an explicit timeout (see src/selfhost/qdrant-vectorize.ts's QDRANT_FETCH_TIMEOUT_MS / signal: AbortSignal.timeout(...), and src/selfhost/ai.ts). src/selfhost/s3-blob-store.ts's three client.fetch() calls (get, put, delete) carried no timeout at all — a misconfigured or unreachable S3-compatible endpoint could hang each call indefinitely, which in turn hangs the screenshot/visual-review pipeline that depends on this store. Added a S3_FETCH_TIMEOUT_MS constant and passed signal: AbortSignal.timeout(S3_FETCH_TIMEOUT_MS) to all three calls, mirroring the qdrant-vectorize pattern exactly.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Closes #8362

Validation

  • git diff --check
  • npm run typecheck
  • npx vitest run test/unit/selfhost-s3-blob-store.test.ts --coverage --coverage.include="src/selfhost/s3-blob-store.ts" — all 17 tests pass (14 pre-existing + 3 new), 100% branch coverage on the changed file
  • npm run actionlint
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • This change touches only src/selfhost/s3-blob-store.ts and its unit test (no UI/MCP/worker/OpenAPI surface touched), so the UI/MCP/workers/OpenAPI-specific checks above were not run locally; they are unaffected by this diff and are still exercised by the full CI gate.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no auth/session/CORS code touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — internal self-host blob-store adapter only, no external API/OpenAPI/MCP surface.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI touched.)
  • Visible UI changes include a UI Evidence section below. (N/A — no visible UI change.)
  • Public docs/changelogs are updated where needed. (N/A — no docs/changelog change needed.)

UI Evidence

N/A — this is a backend adapter change with no visible UI surface.

Notes

  • Only the timeout/signal wiring changed; aws4fetch's retries: 3 option and all other client configuration are untouched, per the issue's own scope note.

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.59%. Comparing base (2494517) to head (43b561b).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8442      +/-   ##
==========================================
- Coverage   92.42%   89.59%   -2.84%     
==========================================
  Files         791       98     -693     
  Lines       79294    22728   -56566     
  Branches    23950     3877   -20073     
==========================================
- Hits        73291    20363   -52928     
+ Misses       4866     2187    -2679     
+ Partials     1137      178     -959     
Flag Coverage Δ
shard-1 100.00% <100.00%> (+42.23%) ⬆️
shard-2 ?
shard-3 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/selfhost/s3-blob-store.ts 100.00% <100.00%> (ø)

... and 693 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 24, 2026
@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-24 13:47:26 UTC

2 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds an AbortSignal.timeout(15_000) to all three s3-blob-store fetch calls (get/put/delete), mirroring the existing bounded-fetch convention used in qdrant-vectorize.ts. The change is small, correctly scoped, and consistent with the stated pattern; get's existing try/catch already treats an abort as a caught error and returns null, while put/delete let the abort propagate to the caller's own catch, matching each method's existing fail-safe contract. Tests added directly verify the signal is attached and exercise both the get-degrades-to-null and put-rejects behaviors on abort.

Nits — 4 non-blocking
  • src/selfhost/s3-blob-store.ts: delete() doesn't have an explicit abort test (only get and put are covered in the new describe block), though its fetch call shares the same code path as put so the risk is low.
  • S3_FETCH_TIMEOUT_MS is a fixed constant (15_000) with no override — consider whether this should be configurable via S3BlobStoreConfig like the qdrant timeout likely is, for parity with operator-tunable settings.
  • Add a delete-abort test alongside the get/put ones in test/unit/selfhost-s3-blob-store.test.ts for full parity across all three methods.
  • If qdrant-vectorize.ts's QDRANT_FETCH_TIMEOUT_MS is operator-configurable, consider the same for S3_FETCH_TIMEOUT_MS for consistency.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8362
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 176 registered-repo PR(s), 81 merged, 5 issue(s).
Contributor context ✅ Confirmed Gittensor contributor joaovictor91123; Gittensor profile; 176 PR(s), 5 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
All three client.fetch() calls in s3-blob-store.ts now pass signal: AbortSignal.timeout(S3_FETCH_TIMEOUT_MS) via a single named constant, exactly mirroring the qdrant-vectorize pattern and leaving retries: 3 untouched, and new tests assert all three calls carry an AbortSignal plus verify get/put's abort-handling behavior.

Review context
  • Author: joaovictor91123
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, JavaScript, MDX, TypeScript, C++, CSS, Rust
  • Official Gittensor activity: 176 PR(s), 5 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 8dffb03 into JSONbored:main Jul 24, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

s3-blob-store.ts's fetch calls have no timeout, breaking the repo's bounded-fetch convention

1 participant