Skip to content

check-live-surfaces: bounded concurrency, timeouts as failed checks - #51

Merged
next-devin merged 3 commits into
mainfrom
live-check-hardening
Sep 4, 2026
Merged

check-live-surfaces: bounded concurrency, timeouts as failed checks#51
next-devin merged 3 commits into
mainfrom
live-check-hardening

Conversation

@next-devin

Copy link
Copy Markdown
Contributor

Follow-up to #50. The first production run of scripts/check-live-surfaces.mjs crashed: the map-link sweep fired ~190 fetches at once and an unhandled connect timeout killed the process before the report.

  • Link checks run 8 at a time; every fetch has a 20 s timeout.
  • A network error is a failed check (status 0, error code in the message), not a crash.

Rerun against production after #50 and docs#34 deployed: 132 passed, 0 failed, including every capability-map link and the deterministic half of the smoke set.

Decision needed from: none.

🤖 Generated with Claude Code

…s, not crashes

The first production run opened ~190 connections at once for the map-link
sweep and died on an unhandled connect timeout. Link checks now run 8 at a
time with a 20 s timeout each; a network error is reported as status 0 on
that check instead of aborting the run. Against production after #50 and
docs#34 deployed: 132 passed, 0 failed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 3, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
developer-docs 9b93789 Commit Preview URL

Branch Preview URL
Sep 03 2026, 04:21 PM

@next-devin
next-devin marked this pull request as ready for review September 3, 2026 15:58
Comment thread scripts/check-live-surfaces.mjs Outdated
@kilo-code-bot

kilo-code-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The previous review's only SUGGESTION (forEachLimit not isolating per-item failures) is now fixed: each worker wraps its fn(item) in a try/catch that pushes check for <item> threw: <message> to failures instead of letting Promise.all reject the whole sweep. The item is captured before await fn so the index race the previous shape had cannot happen here either. No new issues on the changed lines.

Files Reviewed (1 file)
  • scripts/check-live-surfaces.mjs
Previous Review Summaries (2 snapshots, latest commit 833e51e)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 833e51e)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1

The previous review's SUGGESTION about the unused results array is fixed — mapLimit is now forEachLimit with no return value, matching its single side-effect call site.

The only open finding is a future-risk note on forEachLimit itself.

Issue Details (click to expand)

SUGGESTION

File Line Issue
scripts/check-live-surfaces.mjs 56 forEachLimit does not isolate per-item failures; if fn rejects, Promise.all rejects and the whole sweep crashes, which is asymmetric with the PR's stated goal that a network error is a failed check, not a crash. Not blocking today because the only caller's fn is fully safe (calls fetchText, which already swallows errors).
Files Reviewed (1 file)
  • scripts/check-live-surfaces.mjs - 1 issue

Fix these issues in Kilo Cloud

Previous review (commit 25f5eda)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1

The change correctly addresses the reported crash: the map-link sweep is now bounded (concurrency 8) and fetchText swallows network errors as status 0 with the cause in the message, so a connect timeout no longer aborts the run. AbortSignal.timeout is fine given engines.node >= 22, and the broken += 1 / failures.push mutations are safe under the bounded concurrency since they happen synchronously between awaits.

Issue Details (click to expand)

SUGGESTION

File Line Issue
scripts/check-live-surfaces.mjs 62 mapLimit returns a results array that no caller reads; the only call site (line 118) uses side effects only. The allocation and return results are dead state.
Files Reviewed (1 file)
  • scripts/check-live-surfaces.mjs - 1 issue

Fix these issues in Kilo Cloud


Reviewed by minimax-m3 · Input: 23.7K · Output: 1.3K · Cached: 126.7K

…esults array

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread scripts/check-live-surfaces.mjs Outdated
… sweep

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@next-devin
next-devin merged commit cba7600 into main Sep 4, 2026
3 checks passed
@next-devin
next-devin deleted the live-check-hardening branch September 4, 2026 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant