Skip to content

fix: stop /check from reporting same-origin 522/523 link failures as critical broken links - #76

Closed
nish3451 wants to merge 4 commits into
mainfrom
fix/check-same-origin-522-523-not-broken
Closed

nish3451 wants to merge 4 commits into
mainfrom
fix/check-same-origin-522-523-not-broken

Conversation

@nish3451

@nish3451 nish3451 commented Aug 9, 2026 •

Copy link
Copy Markdown
Owner

What

The public anonymous one-page check (POST /api/public-check, page at /check) runs the shared audit engine. When the checked site's origin behind Cloudflare temporarily fails, every same-origin internal link returns 522 (connection timed out) or 523 (origin unreachable) at once — Cloudflare-edge errors for the site's own origin. The engine classified those as broken, so /check reported "Broken internal links" as critical, confidence=verified findings and inflated the critical count: inventing a defect the page does not have (the links work as soon as the origin recovers, and Google backs off and retries 5xx rather than dropping URLs).

Change

shared/audit-engine.js (the engine /check runs through — no route changes needed):

  • New isSameOriginInfraFailure(check) predicate: an internal (same-origin) link check returning 522/523 is transient infrastructure, not a broken link.
  • isBrokenResource now excludes those, so they never become critical findings, never inflate summary.critical, and stay out of per-page broken-link counts and the repair brief.
  • External targets' 522/523 remain reportable: that is a real observation about the referenced site, not our own footprint. Same-origin only.
  • Same family as the existing throttled-statuses guard (408/425/429/503) that stopped reporting our own crawl rate as the customer's broken links.

Tests

shared/audit-engine.test.mjs:

  1. Unit: 522/523 internal → infra failure; external/image 522/523 → still reportable; 404/502 internal → still broken.
  2. End-to-end: a page whose same-origin links return 522 and 523 while an external link returns 523 → zero criticals, no broken-internal-link finding, external failure still evidenced.

Validation

  • npm run test:audit-engine — 23/23 pass
  • npm run test:public-check — 5/5 pass
  • npm run test:worker-dispatch — 10/10 pass
  • Full npm run check (18 suites + build) — exit 0, zero failures

Summary by CodeRabbit

  • Bug Fixes
    • Improved link auditing by excluding transient Cloudflare infrastructure failures for same-origin resources from broken-link findings.
    • Continued reporting external failures and genuine issues such as missing pages, image failures, and server errors.

…critical broken links

522 (connection timed out) and 523 (origin unreachable) are Cloudflare-edge
errors returned when the checked site's own origin temporarily cannot be
reached. While it is down, every same-origin link fails at once and recovers
with it; Google backs off and retries 5xx rather than treating the URLs as
dead. The public one-page check was reporting a scan-time origin hiccup as
critical, confidence=verified 'Broken internal links' findings -- inventing a
defect the page does not have (same family as the 429/503 throttle fix).

Same-origin (internal) 522/523 checks now classify as transient infra
failures instead of broken links, so they never become critical findings or
inflate the critical count. External targets' 522/523 stay reportable: that
is a real observation about the referenced site, not our own footprint.

Two regression tests: the classifier (internal vs external vs image vs real
failures) and an end-to-end audit where same-origin links return 522/523 but
an external link returns 523, pinning the report to zero criticals while the
external failure is still evidenced.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4c9684fb-a8cf-4578-8bb8-b964722d5b4d

📥 Commits

Reviewing files that changed from the base of the PR and between 120c699 and 81d3ddf.

📒 Files selected for processing (2)
  • shared/audit-engine.js
  • shared/audit-engine.test.mjs

📝 Walkthrough

Walkthrough

The audit engine now treats same-origin HTTP 522 and 523 responses as transient infrastructure failures. It excludes these responses from broken-resource findings while continuing to report equivalent failures for external resources.

Changes

Same-origin infrastructure failure handling

Layer / File(s) Summary
Classify same-origin Cloudflare failures
shared/audit-engine.js
Adds the exported isSameOriginInfraFailure helper. Same-origin 522 and 523 responses no longer produce broken-resource findings. External responses remain reportable.
Validate classification and audit findings
shared/audit-engine.test.mjs
Adds unit and end-to-end tests for same-origin 522/523 exclusions, external 523 reporting, unrelated resource types, 404 responses, and 502 responses.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • nish3451/seo-fix-kit#55: Both changes classify transient same-origin HTTP failures in shared/audit-engine.js, but they cover different helpers and status codes.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change to exclude same-origin 522/523 failures from critical broken-link reports.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/check-same-origin-522-523-not-broken

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 81d3ddf89b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shared/audit-engine.js
Comment on lines +2419 to +2421
check.kind === "internal" &&
check.status &&
CLOUDFLARE_ORIGIN_ERRORS.has(check.status)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reclassify redirected checks by their final origin

When a same-origin link redirects to another origin and that external destination returns 522/523, kind remains "internal" because it is assigned before fetchResource follows redirects, while status comes from the final hop. This predicate consequently suppresses the broken-link finding for an external target that the change explicitly intends to keep reportable; only treat the response as a same-origin infrastructure failure when the final URL is still on the checked origin.

Useful? React with 👍 / 👎.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 41674d2a17

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shared/audit-engine.js

function isBrokenResource(check) {
if (isThrottledResource(check)) return false;
if (isSameOriginInfraFailure(check)) return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Apply the infra-failure rule to page proof counts

When a same-origin link returns 522/523, this predicate removes it from findings and backend page summaries, but the saved /check report's PageProof in src/App.jsx still counts every !check.ok or status ≥400 as a broken link. The resulting report can show zero critical findings while its raw page evidence still labels those same transient checks as broken; reuse the same classification when computing that displayed count.

Useful? React with 👍 / 👎.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@nish3451

Copy link
Copy Markdown
Owner Author

Superseded by #91 (fresh branch from current main). This branch's merge history would also revert shipped work from #83/#85/#90 (URL-scheme validation, policy-page CTAs, methodology CTA), so it could not be merged as-is; #91 carries only the same-origin 522/523 classifier fix and its regression tests, cleanly based on origin/main.

@nish3451 nish3451 closed this Aug 10, 2026
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