ci(indexnow): treat an edge 403 on the key file as deployed - #11204
Conversation
The readiness loop treated every non-200 as "not deployed yet"; a runner that Cloudflare's bot management answers with 403 would have slept the full eight minutes on every run. Mirrors the sister repo's review. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu
There was a problem hiding this comment.
🟡 Changes recommended
Transport failures produce a malformed 000000 status, and the changelog entry lacks its required PR reference.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates IndexNow deployment readiness handling to avoid unnecessary waits on Cloudflare 403 responses.
Changes:
- Treats HTTP 403 as evidence the key file is deployed.
- Reports the last observed status in timeout warnings.
- Documents the fix in the changelog.
File summaries
| File | Description |
|---|---|
.github/workflows/indexnow-submit.yml |
Updates key-file readiness probing. |
CHANGELOG.md |
Records the IndexNow fix. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The first sitemap-scope run (33665718870) never reached IndexNow: a
full-sitemap body is ~260 KB and a single command-line argument is capped
at 128 KB on Linux, so curl failed to exec ("Argument list too long") and
the run reported HTTP 000 for 4,593 URLs. The body now goes through
`--data @body.json`.
The retry by hand then answered 403 SiteVerificationNotCompleted: IndexNow
verifies a new key file asynchronously. That is a warning, not an error —
the next push resubmits its URLs and scope=sitemap is a re-run away.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu
…gelog ref
Review feedback: curl prints 000 for %{http_code} on a transport failure,
so '|| echo 000' inside the substitution recorded 000000.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu
There was a problem hiding this comment.
🟡 Changes recommended
Transport failures produce an invalid status, and rejected verification-pending batches can incorrectly leave the workflow green.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
.github/workflows/indexnow-submit.yml:127
- On a transport failure,
curl -walready prints000before returning nonzero, and theecho 000fallback appends another value. This makesstatusbecome000000(or, after a partial response, values such as200000), so the new “last status” diagnostic is inaccurate and a received 200 may not stop the wait. Assign the fallback after the command instead, as the submission request below already does.
CHANGELOG.md:48 - Add this PR’s
(#…)reference to the changelog bullet. Changelog entries consistently carry their PR reference (for example, the adjacent entries at lines 40 and 57), and leaving this one unlinked makes the release record incomplete.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The retry budget can exceed the job timeout, and rejected changed-URL batches can finish successfully without automatic resubmission.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/indexnow-submit.yml:160
- This retry budget can exceed the job's 10-minute timeout. After the readiness loop's 15 sleeps (450 seconds), four 60-second attempts plus retry delays can take another 255 seconds, so Actions can terminate the job before this branch finishes; slow readiness probes make the overrun larger. Please reduce the combined wait/retry budget or increase the job timeout so the intended warning/submission path can complete.
.github/workflows/indexnow-submit.yml:174
- This leaves the job green even though the batch was not accepted. For
changedruns, later pushes calculate a new diff and do not resubmit these URLs, so the batch is lost unless someone notices the warning and manually runs the sitemap scope; it also contradictsdocs/reference/seo.md:523-524, which says a 4xx fails the run. Keep this failure visible or add an automatic retry/backfill mechanism, and align the documentation and changelog with that policy.
echo "::warning::IndexNow has not finished verifying the key file yet (403 SiteVerificationNotCompleted); ${n} URL(s) not accepted — re-run with scope=sitemap later"
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
…f-trigger Review feedback: a green warning on 403 SiteVerificationNotCompleted would drop a `changed` run's URLs for good, because later pushes submit only their own diffs, and the docs promise visible 4xx failures. The case is now retried once a minute for up to ten minutes and then fails the run, which keeps it re-runnable with the same URL set. A push that changes this workflow file triggers it with the full list, so a fix runs before the next plot merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu
There was a problem hiding this comment.
🟡 Changes recommended
The ten-minute job timeout cannot accommodate the newly added retry and readiness waits.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
The job kept `timeout-minutes: 10` while the step can now wait 8 min for the key file and another 10 min for key verification, so GitHub would have cancelled the run before the final 403 error. The timeout covers both bounded waits, the verification loop no longer sleeps after its last try, and the workflow docs (seo.md, workflows/overview.md) now mention that a change to the workflow file submits the full list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu
There was a problem hiding this comment.
🟡 Changes recommended
The timeout cannot cover the implemented retry bounds, and several deployment and timing claims are inaccurate.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
.github/workflows/indexnow-submit.yml:56
- The 25-minute job limit does not cover the coded worst case. The readiness loop can take
16 × 15s + 15 × 30s = 11.5m, and the verification loop can take10 × 60s + 9 × 60s = 19m, already exceeding 30 minutes before setup or curl retries. GitHub can therefore cancel the job before the explicit rejection path preserves the run as re-runnable. Use elapsed-time deadlines/reduced request timeouts, or raise the job limit above the actual bound.
# Worst case: 8 min waiting for the key file + 10 min of verification
# retries + the curl timeouts, with room to spare.
timeout-minutes: 25
.github/workflows/indexnow-submit.yml:159
- This is an independent payload-size fix, while the diff also adds full-list self-triggering and submission-response retries; the PR summary and test plan describe only the key-file probe's 403 handling. Update the PR description and validation plan to cover these substantive behaviors, or split them, so the reviewed scope matches the proposed changes.
# The body goes through a file: a full-sitemap batch is ~250 KB,
# and a single command-line argument is capped at 128 KB on Linux
# — the first sitemap-scope run (33665718870) failed to exec curl
# at all and reported HTTP 000 for 4,593 URLs.
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
…ployed Ten attempts with 60-second requests and 60-second gaps could take about 19 minutes, not the ten the docs promise; the loop now retries until a ten-minute elapsed deadline has passed and the job timeout covers the real worst case. An edge 403 on the key-file probe is answered before the origin is asked, so the notice, the comment and the changelog no longer call it "deployed": the wait is skipped and IndexNow's own fetch decides. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu
There was a problem hiding this comment.
🔵 Needs a closer look
The PR title and summary still incorrectly characterize an inconclusive 403 as proof of deployment.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/indexnow-submit.yml:146
- The implementation now correctly treats a 403 as inconclusive, but the PR title says it is treated “as deployed,” and the summary still says a 403 means the file is served. Update the PR title and summary to state that the wait is skipped and IndexNow performs the authoritative deployment check.
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
curl -f, so a 403 from Cloudflare's bot management looked like "not deployed yet" and a blocked runner would sleep the full 8 minutes on every run.200and403both end the wait (a 403 means the file is served but this runner is blocked at the edge; Bing's own fetch is not), anything else keeps waiting, and the final warning names the last status seen.Plan
N/A
Test plan
bash -n.changedrun after the next pipeline merge logs eitherkey file reachableor the 403 notice within one probe.🤖 Generated with Claude Code
https://claude.ai/code/session_01SrKzcwZBnref1sWYtdXynu