Fetch Logpush dataset pages from middlecache

[soheiokamoto]

Summary

Fetch and validate generated Logpush dataset pages from middlecache during docs builds without affecting development startup.

• Fetch and atomically validate a fresh archive for each build.
• Validate archive structure and generated frontmatter before replacing pages.
• Sync generated Markdown only within existing dataset scopes and remove stale generated pages.
• Reject archives that omit a scope or remove more than 25% of a scope's pages.
• Protect local edits while recognizing output from the previous successful synchronization.
• Stage replacements and recover interrupted swaps without partially updating checked-in pages.

Screenshots (optional)

Not applicable.

Documentation checklist

• The change adheres to the documentation style guide.

[cloudflare-docs-bot]

Review

💡 2 suggestions found in commit dab4ffd.

👉 Fix in your agent 👈

Fix the following review findings in PR #33248 (https://github.com/cloudflare/cloudflare-docs/pull/33248).

Before making changes, review each finding and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g. the fix requires disproportionate effort for minimal benefit,
  or the finding is factually incorrect), explain why
- If you need clarification before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

After triaging, follow this order:
1. Post a comment on this PR for any findings you are skipping, with the finding ID and your reasoning.
2. Then commit the fixes for the legitimate findings.

The comment must come before the commit — the bot reads PR comments when a new
push triggers a review, so skip comments posted after the push will be missed.

---

## Code Review

### Suggestions (2)

#### CR-f7b9071876b4 · Redundant archive listing / pointless async
- **File:** `bin/fetch-logpush-datasets.ts` line 127
- **Issue:** `validateArchive` is declared `async` but contains no awaited operation (only synchronous `spawnSync`), and the identical `tar -tzf` member listing is performed again inside `extractTarGz` at line 180, so every archive is fully listed twice per run.
- **Fix:** Drop the `async` modifier (or actually await something), and consider whether the duplicate `tar -tzf` pass in `extractTarGz` makes the pre-download validation redundant — at minimum note that the listing runs twice.

#### CR-0c78590fe25a · Silently ignored archive files
- **File:** `bin/fetch-logpush-datasets.ts` line 190
- **Issue:** Only flat `scope/page.md` paths in unseeded scopes produce the `skippedArchiveScopes` warning. Top-level `.md` files and `.md` files nested inside unseeded scopes (e.g. `newscope/sub/page.md`) are filtered out with no warning at all, so an unexpected change to the archive layout can silently drop content from the sync.
- **Fix:** Extend the warning to cover all ignored archive `.md` files (or fail loudly for unexpected top-level/nested entries), so a layout change in the middlecache archive is visible instead of silent.

Code Review

This code review is in beta and may not always be helpful — use your judgment.

Suggestions (2)

File	Issue
bin/fetch-logpush-datasets.ts line 127	Redundant archive listing / pointless async — validateArchive is declared async but contains no awaited operation (only synchronous spawnSync), and the identical tar -tzf member listing is performed again inside extractTarGz at line 180, so every archive is fully listed twice per run. Fix: Drop the async modifier (or actually await something), and consider whether the duplicate tar -tzf pass in extractTarGz makes the pre-download validation redundant — at minimum note that the listing runs twice.
bin/fetch-logpush-datasets.ts line 190	Silently ignored archive files — Only flat scope/page.md paths in unseeded scopes produce the skippedArchiveScopes warning. Top-level .md files and .md files nested inside unseeded scopes (e.g. newscope/sub/page.md) are filtered out with no warning at all, so an unexpected change to the archive layout can silently drop content from the sync. Fix: Extend the warning to cover all ignored archive .md files (or fail loudly for unexpected top-level/nested entries), so a layout change in the middlecache archive is visible instead of silent.

Conventions

_{No convention issues found.}

Style Guide Review

_{No style-guide issues found.}

Commands

Only codeowners can run commands. Post a comment with the command to trigger it.

Command	Description
/review	Runs a review now. Incremental if a prior review exists, full if not.
/full-review	Re-reviews the entire PR diff from scratch, ignoring incremental history. Useful after a rebase, when you want a fresh review, or if the bot gets out of sync and reports issues that no longer exist.
/ignore-review-limit	Permanently lifts the 2-review automatic limit for this PR. Future pushes will trigger reviews as normal.
/disable-auto-review	Stops automatic reviews from triggering on future pushes to this PR. Codeowners can still run /review or /full-review manually.
/rebase	Rebases the PR branch against production. On conflict, attempts to resolve automatically using AI. Stops with an explanation if confidence is not high enough.

[github-actions]

This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:

Pattern	Owners
*	@cloudflare/product-owners
*.ts	@cloudflare/content-engineering
package.json	@cloudflare/content-engineering

[cloudflare-workers-and-pages]

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://sohei-ds-17240-fetch-logpush-docs.previews.developers.cloudflare.com (commit dab4ffd)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status	Deployment URL	Commit	Updated (UTC)	See this deployment's details
Build: Success ✅ Deployment: Success ✅ View logs ↗	https://4ad386da.previews.developers.cloudflare.com	dab4ffd	2026-09-11T20:52:10.341Z	Visit the dashboard ↗
Build: Failed ❌ View logs ↗		2c15d32	2026-09-11T18:59:39.774Z	View logs ↗
Build: Failed ❌ View logs ↗		2188729	2026-09-11T10:37:23.246Z	View logs ↗
Build: Failed ❌ View logs ↗		86ce2a6	2026-09-11T09:51:21.828Z	View logs ↗
Build: Failed ❌ View logs ↗		cbe1049	2026-09-11T08:37:13.274Z	View logs ↗
Build: Failed ❌ View logs ↗		d8c053e	2026-09-11T08:21:30.744Z	View logs ↗
Build: Failed ❌ View logs ↗		5c30d11	2026-09-11T08:00:18.963Z	View logs ↗
Build: Failed ❌ View logs ↗		5e9a1e4	2026-09-11T07:46:40.513Z	View logs ↗
Build: Failed ❌ View logs ↗		c58ab5a	2026-09-11T03:02:07.754Z	View logs ↗
Build: Failed ❌ View logs ↗		1f8a233	2026-09-11T02:04:34.489Z	View logs ↗

View all previews: View all previews ↗

[github-actions]

Preview URL: https://e131148c.preview.developers.cloudflare.com
Preview Branch URL: https://sohei-ds-17240-fetch-logpush-docs.preview.developers.cloudflare.com

[soheiokamoto]

Review triage before the next push:

• CR-773782c2b6a8: No further change. Failures after mutation begins are fatal, so a mixed working tree cannot be deployed. Full rollback would not improve publish safety.
• CR-cff72d2e658a: No change. These Markdown files are generated pages intentionally replaced during builds; tracked copies provide the fetch-failure fallback.
• CR-9358915e6c5d: No change. The production-build soft fallback is intentional so archive availability does not block unrelated documentation builds.
• CR-8486ea85020a: No change. The archive contains file entries only and therefore cannot emit an empty scope directory.
• CV-c3eb79385248: No change. The only functional AGENTS.md update registers the new loader; the remaining diff aligns existing tree comments.
• CR-c970353aceac: Will fix by forcing a fresh archive download in both development and build hooks.

[soheiokamoto]

Review triage:

• CR-5eee759da2f6: Skipping. The dedicated producer validates that the non-empty archive exactly matches its generated source tree and contains only regular <scope>/<page>.md files before publication. A deletion-ratio threshold would be arbitrary and could reject legitimate dataset removals; the consumer retains a focused safeguard against removing every managed page.
• CR-40a40712138a: Skipping. The soft failure is intentional so temporary middlecache availability does not block unrelated documentation builds. Checked-in dataset pages are retained as the fallback.
• CR-472aab9ac652: Skipping. Both supported package-script entry points invoke the loader with --force; stale cache reuse only applies to a direct manual invocation without that flag.
• CR-405d2bd4083d: Skipping. Fetching fresh dataset pages on each development run is intentional, while --soft ensures a failed network request does not block local startup.

No changes are planned for these findings.

[soheiokamoto]

Review triage for commit 6c13530:

• CR-795021323ab2: Skipping. The scripts share low-level download and extraction utilities, but their orchestration differs: the Skills loader handles a manifest, existing-directory caching, direct replacement, and a missing-content fallback, while the Logpush loader refreshes an archive and computes a deferred copy/removal sync. Extracting their flag, failure, and mutation control flow after only two consumers would add abstraction and broaden this change without removing meaningful duplication.
• CR-02d41ba38188: Agreed. I will correct the repository tree to reference src/content/directory/.

[soheiokamoto]

Skipping CR-3e27227ebdae: refreshing the Logpush dataset archive on every development and build run is intentional. These generated pages can change independently of this repository, so reusing the persistent cache would silently serve stale documentation. The fetch remains soft-failing so network availability does not block local development or unrelated builds.

[soheiokamoto]

Skipping CR-1313933d2d68: these dataset pages are generator-owned output and are marked Code generator. DO NOT EDIT. Middlecache is the source of truth; preserving direct edits to generated pages would defeat the purpose of refreshing them for development and preview builds.

Skipping CR-a571aa59202a: this finding describes the pre-fix implementation. Commit 4059864 validates every destination scope and rejects an archive that would remove more than 25% of any scope's pages before it stages or swaps any content.

[soheiokamoto]

Skipping CR-4fe29a0643e0: the checked-in dataset pages are intentionally the production fallback. Logpush archive availability or validation failures must not block unrelated documentation builds; warnings remain visible in build logs.

Skipping CR-d143f1f7060b: the supported predev and fetch:assets entry points execute this loader sequentially. Cross-process locking plus stale-lock recovery would add disproportionate complexity for unsupported overlapping manual invocations.

Skipping CR-74db237246b6: fresh download and replacement on every development start are intentional because these generator-owned pages can change independently of this repository. The behavior is also called out in the PR summary.

[github-actions]

CI run failed: build logs

[soheiokamoto]

Skipping CR-bf7e983e9dc6: portable replacement of a non-empty directory requires two renames. The script keeps the original tree as a backup, rolls back ordinary installation failures, and restores an interrupted swap on the next run; a process-kill window cannot be eliminated without a substantially different symlink-based content layout.

Skipping CR-c5b3b5c14339: the managed scopes are the established account and zone collections, not arbitrary one-to-three-page scopes. Rejecting an archive that omits either scope is intentional protection against partial publication.

Skipping CR-0b49dd8a5f0c: fetching fresh generator-owned pages on each development start is intentional because their source changes independently of this repository. Reusing the persistent archive cache would silently serve stale content.

[soheiokamoto]

Skipping CR-da8feac5680d: strict <scope>/<page>.md validation is intentional and was added to resolve CR-c048db27e7c2. Ignoring nested Markdown would silently freeze generated pages if the producer changes layout, while failing loudly exposes the contract change before tracked content is replaced.

Skipping CR-adb72d2cd6ef: this repeats the acknowledged build-only synchronization decision from CR-24ffc0a41b6e. The Logpush fetcher updates tracked pages, so adding it to predev would dirty unrelated contributors’ worktrees; development intentionally serves checked-in pages.

[soheiokamoto]

Review disposition for commit 77735de:

• CR-f32cae5dd593 is resolved in the next push. Transaction staging and backup directories now live under the repository’s gitignored .tmp directory. The existing startup recovery restores the backup if an interruption leaves the tracked dataset route missing.
• CR-da8feac5680d is intentionally not changed. The archive contract is <scope>/<page>.md; rejecting nested Markdown is deliberate and directly resolves CR-c048db27e7c2, which warned that silently ignoring nested pages could freeze generated content.
• CR-adb72d2cd6ef is intentionally not changed. Logpush synchronization is build-only because it updates tracked documentation pages. Running it from predev would dirty unrelated contributors’ worktrees and recreate the behavior reported in CR-5ae485f8a23a.

Please treat the two documented design decisions as acknowledged, not as warnings or blockers in subsequent reviews.

[soheiokamoto]

Review disposition for commit c58ab5a:

• CR-1f095b9e199d is valid and will be fixed in the next push by applying strict layout validation only to archive pages whose first path segment is a managed destination scope.
• CR-79075bce8815 is valid and will be fixed by warning when valid archive scopes are not yet seeded in the checked-in documentation.
• CR-182ed8359c7a is intentionally skipped. Build-time synchronization is required to fail hard when fresh generated documentation cannot be validated or fetched; silently shipping stale generated fields from CI is not acceptable. --soft remains an explicit manual recovery mode rather than normal production wiring.
• CR-cddbb91657c2 is intentionally skipped and repeats the acknowledged build-only decision. predev must not run this fetcher because it updates tracked documentation and would dirty unrelated contributors’ worktrees.

Please treat the two intentional wiring decisions as acknowledged rather than warnings or blockers in subsequent reviews.

[soheiokamoto]

Review disposition for commit 5c30d11:

• CR-f5946039e388: the fail-closed behavior is intentional. If .tmp state is gone, generated output is indistinguishable from user edits; automatically treating a dirty tree as the baseline would disable the data-loss guard. The next push will make the error explicitly name the state path and retain the existing commit-or-restore recovery guidance.
• CR-e12624fc050b is valid. The next push will retain a valid stale archive as a transaction backup while refreshing, restore it after download failure or an interrupted prior refresh, and remove it only after a replacement validates.
• CR-bf806430786c is intentionally skipped and repeats the acknowledged strict-layout decision. Nested Markdown inside a managed scope indicates producer contract drift and must fail loudly; silently skipping it could freeze generated pages.
• CR-dbe3121d0012 is intentionally skipped and repeats the acknowledged build-only decision. predev must not invoke a fetcher that updates tracked documentation pages.

Please preserve these two recurring design decisions as acknowledged in subsequent reviews rather than raising new equivalent findings.

[soheiokamoto]

Review disposition for commit d8c053e:

• CR-720cf276a9b7 is intentionally skipped as a behavior change. Restoring .stale after a failed refresh preserves the last valid artifact for future retries and interrupted-run recovery; it is not intended to let the current hard-mode build continue with stale generated data. Hard mode deliberately requires a fresh archive. The next push will add an explicit code comment so this preservation policy is not mistaken for runtime fallback.
• CR-8c0684151ab2 is valid. The next push will make a missing dataset directory unconditionally fatal because --soft can only continue when checked-in pages actually exist.

[soheiokamoto]

Skipping CR-b3f6c7486b47 because its requested behavior conflicts directly with acknowledged safety requirements and prior finding CR-8c0684151ab2.

--soft may exit successfully only when usable checked-in pages remain at DATASETS_DIR. If that directory is missing, there are no checked-in pages to preserve, so success would hide broken routes. Likewise, if a swap failure leaves the original tree only at BACKUP_DIR, the routed dataset directory is absent; exiting 0 would leave the build in a broken state. Both cases must remain hard failures. This is intentionally stricter than fetch-skills.ts because Logpush pages are tracked routed content, not an optional gitignored artifact.

Please treat this safety behavior as acknowledged and do not raise missing-target or missing-route soft-mode success as a blocker in subsequent reviews.

[soheiokamoto]

/review

[soheiokamoto]

Review disposition for commit cbe1049:

• CR-f41028a891b8 is valid. The next push will remove the redundant startup .stale restoration; the download-failure path already restores the valid archive, while an interrupted refresh can retain .stale until the next download attempt.
• CR-2cf2fae2520b is intentionally skipped. Supported repository wiring invokes this fetcher once and sequentially within fetch:assets; concurrent cross-process execution is not part of the script contract. Adding a filesystem lock would require stale-lock ownership and crash recovery, increasing risk and complexity for an unsupported invocation pattern.
• CR-65cac596d1a9 is intentionally skipped and repeats the acknowledged build-only decision. predev must not invoke this fetcher because it updates tracked documentation pages and would dirty unrelated contributors’ worktrees.

Please treat the concurrency scope and build-only wiring as acknowledged design constraints in subsequent reviews.

[soheiokamoto]

Pre-push simplification note:

• Removed the unused --soft and --force modes. The fetcher is build-only and always fails closed.
• Removed the unused destination override, keyed transaction paths, TTL/cache branching, and redundant replacement state.
• Every build now fetches a fresh archive. The prior structurally valid archive remains available until download, semantic validation, deletion checks, dirty-tree checks, and synchronization all succeed; any failure restores it for a later retry while the current build still fails.
• Retained strict managed-scope layout validation, unseeded-scope warnings, missing-scope and deletion thresholds, local-edit protection, pending-state recovery, staged replacement, rollback, and the acknowledged sequential-invocation contract.
• Minimized the AGENTS.md change to listing the new fetcher only.

This is a reduction-only pass. It does not change the acknowledged build-only wiring or strict managed-layout policy.

[soheiokamoto]

/review

[soheiokamoto]

/review

[soheiokamoto]

Skipping CR-9e1cfe82104b because restoring the previous archive on downstream failure is intentional semantic rollback, not a download retry optimization.

downloadToDotTempIfNotPresent establishes only that the new file is a valid tar archive. Frontmatter validation, managed-scope checks, deletion thresholds, dirty-tree protection, and the staged swap determine whether that archive is acceptable for synchronization. If any of those checks fail, retaining the fresh archive would allow a semantically rejected artifact to replace the last accepted archive on the next refresh. Restoring the previous archive preserves that accepted artifact for recovery and diagnostics while the current build still fails hard; it is never used as a stale-data fallback.

Please treat rollback after any pre-success failure as an acknowledged transaction boundary rather than limiting restoration to transport failures.

[soheiokamoto]

/review

[soheiokamoto]

Skipping CR-57f7127f099d because this repeats the acknowledged build-only synchronization decision.

Logpush synchronization updates tracked documentation pages, unlike the gitignored assets refreshed by the existing soft predev fetchers. Adding it to predev would unexpectedly dirty unrelated contributors’ worktrees whenever generated content changes. Local development intentionally serves the committed generated pages; CI and explicit builds perform the authoritative hard-failing refresh.

This behavior was previously documented for equivalent findings CR-24ffc0a41b6e, CR-adb72d2cd6ef, CR-cddbb91657c2, CR-dbe3121d0012, and CR-65cac596d1a9. Please treat it as an acknowledged design constraint.

[soheiokamoto]

/review

[soheiokamoto]

Superseding my earlier disposition for CR-9e1cfe82104b: the final state-machine audit found that the archive rollback cannot reliably distinguish an accepted archive from an interrupted candidate, and the retained archive is never used as a stale fallback.

The next push resolves the suggestion by removing previousArchivePath, archiveRefreshStarted, and the archive rollback catch path. Each build deletes the cached archive and downloads a fresh, atomically validated archive. Dataset staging, pending-state recovery, dirty-tree protection, and hard-fail behavior remain unchanged.

[soheiokamoto]

/review

[soheiokamoto]

Closing this PR in favor of the already merged one: #33401

This PR had some prevention of stale or malformed generated documentation during future archive changes or partial failures, but will create a follow-up PR if needed.