Skip to content

ci(docs): publish mdbook to GitHub Pages#331

Open
MegaRedHand wants to merge 8 commits intomainfrom
ci/mdbook-gh-pages
Open

ci(docs): publish mdbook to GitHub Pages#331
MegaRedHand wants to merge 8 commits intomainfrom
ci/mdbook-gh-pages

Conversation

@MegaRedHand
Copy link
Copy Markdown
Collaborator

Summary

  • Adds .github/workflows/pr-main_mdbook.yml to build and publish the mdBook docs site to GitHub Pages.
  • Workflow runs on docs/** and book.toml changes:
    • Build with peaceiris/actions-mdbook@v2 (mdBook 0.5.2, matching Makefile).
    • Link check via lycheeverse/lychee-action@v2 (excludes localhost and 127.0.0.1, which appear in operations docs).
    • Deploy to the gh-pages branch on push to main using peaceiris/actions-gh-pages@v4.
  • Modeled after ethrex's workflow, simplified for ethlambda (no CNAME, no preserved subdirectories).

Closes #123.

Notes

  • After the first successful main run, GitHub Pages must be enabled manually in Settings → Pages with source set to the gh-pages branch.
  • Build output goes to book/html/ because book.toml declares both [output.html] and [output.linkcheck2]. mdBook auto-generates a .nojekyll file there.
  • Verified locally: make docs builds cleanly with mdBook 0.5.2 and produces book/html/.nojekyll.

Test plan

  • PR run executes the Build and Link Check jobs successfully.
  • After merging, the Deploy job runs and creates/updates the gh-pages branch.
  • Repo admin enables GitHub Pages from gh-pages branch.
  • https://lambdaclass.github.io/ethlambda/ serves the rendered book.

Adds a workflow that builds the mdbook on docs/book.toml changes,
runs lychee link-check on PRs, and deploys to the gh-pages branch
on push to main. Closes #123.

Note: GitHub Pages must be manually configured in repo settings to
serve from the gh-pages branch after the first successful run.
@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Here is my review of PR #331:


PR #331 — CI: Publish mdBook to GitHub Pages

What it does: Adds a GitHub Actions workflow that builds, link-checks, and deploys the mdBook docs site to GitHub Pages. Triggered on changes to docs/**, book.toml, or the workflow file itself, with the deploy step gated to pushes to main.


Correctness & Functionality

  • The three-job pipeline (Build → Link Check + Deploy) is logically sound. Build runs first, then link-check and deploy run in parallel, both depending on a successful build. The artifact upload/download path (book/html) correctly matches the book.toml output directory for the [output.html] renderer.

  • actions/checkout@v6 on lines 28 and 50 — This version should be verified. As of mid-2025 the latest stable was v4. If v6 does not exist or is not yet a stable floating tag, all jobs that use checkout will fail immediately. Recommend confirming the version or pinning to a known-good tag (e.g. v4).

  • The if: ${{ github.ref == 'refs/heads/main' }} guard on the artifact upload (line 36) and the deploy job (line 61) is correct — PRs build and check but never deploy. Good.

  • The link-check job (line 47) operates on the raw docs/ source directory, not the built HTML. This catches broken links in Markdown but misses any link issues introduced by mdBook templating/preprocessing. This is an acceptable trade-off and is clearly intentional.


Security & Permissions

  • permissions: contents: write is set at the workflow level (line 19), which means every job — including link-check — inherits write access to repository contents. Only the deploy job actually needs write access (to push the gh-pages branch). The other two jobs only need contents: read. Recommend scoping permissions per-job:
# Remove the top-level permissions block, then add per-job:

jobs:
  build:
    permissions:
      contents: read

  link-check:
    permissions:
      contents: read

  deploy:
    permissions:
      contents: write

This follows the principle of least privilege, especially relevant since link-check runs on untrusted PR code.


Concurrency

  • Both build (line 24) and deploy (line 65) define the same concurrency group ${{ github.workflow }}-${{ github.ref }} without cancel-in-progress: true. This means concurrent pushes queue rather than cancel stale runs. For a docs deployment this is acceptable, but for PRs it means outdated builds sit in the queue. Consider adding cancel-in-progress: true to the build job's concurrency block so stale PR builds are dropped.

  • Sharing the same concurrency group between build and deploy creates a subtle ordering effect: if a new push arrives while a prior deploy is still running, the new build queues behind it. This is probably fine for docs, but worth noting.


Design Notes

  • The link-check job declares needs: build (line 49) but doesn't consume the build artifact — it independently checks out the repo and runs lychee on the source. The needs dependency effectively makes link-check a gated step (only runs after a successful build), which is a reasonable quality gate. This intent should be clear to future maintainers; a brief inline comment would help.

  • retention-days: 1 on the artifact (line 41) is appropriate — the artifact only needs to survive from build to deploy within the same run.

  • The workflow_dispatch trigger (line 16) enables manual re-deployment, which is useful for recovering from a failed deploy without a code change.


Summary

The workflow is well-structured and follows the ethrex reference model appropriately. Two items warrant fixes before merge:

  1. Verify actions/checkout@v6 exists — if it doesn't, all jobs fail on checkout.
  2. Scope permissions: contents: write to the deploy job only — the other jobs should use contents: read to follow least-privilege.

The cancel-in-progress and needs: build comments are lower-priority suggestions.


Automated review by Claude (Anthropic) · sonnet · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Kimi Code Review

This PR adds a GitHub Actions workflow for publishing mdBook documentation. This is infrastructure code and does not modify consensus-critical logic (fork choice, XMSS, SSZ, state transition), so the review focuses on CI/CD correctness and security.

Critical Issues

  1. Invalid Action Version (Workflow Failure)
    • File: .github/workflows/pr-main_mdbook.yml
    • Lines: 19, 46
    • Issue: uses: actions/checkout@v6 references a non-existent version. The latest stable release is v4 (as of late 2024).
    • Fix: Change to actions/checkout@v4.

Security & Permissions

  1. Overly Permissive Top-Level Permissions

    • File: .github/workflows/pr-main_mdbook.yml
    • Lines: 16-17
    • Issue: permissions: contents: write is granted to all jobs, including link-check which only needs read access.
    • Fix: Move permissions: contents: write to the deploy job only. Set top-level permissions to contents: read or remove the global block and specify per-job.
  2. Supply Chain Security (Recommended)

    • Issue: Actions are pinned to mutable version tags (@v4, @v2) rather than specific commit SHAs.
    • Fix: Pin to specific commit hashes (e.g., actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2) and use Dependabot or Renovate to update them.

Logic & Performance

  1. Unnecessary Job Dependency

    • File: .github/workflows/pr-main_mdbook.yml
    • Line: 44
    • Issue: link-check declares needs: build but checks out the repository fresh and scans the docs/ source directory. It does not consume the build artifact.
    • Impact: Serializes jobs unnecessarily, increasing CI latency.
    • Fix: Remove needs: build from link-check to allow parallel execution, or merge the link check into the build job.
  2. Deployment Not Gated on Link Check

    • File: .github/workflows/pr-main_mdbook.yml
    • Line: 60
    • Issue: The deploy job only needs: build, meaning broken links will not prevent deployment to GitHub Pages.
    • Fix: If the intent is to block deployment on broken links, change to needs: [build, link-check].
  3. Missing Concurrency Control

    • File: .github/workflows/pr-main_mdbook.yml
    • Issue: The link-check job lacks the concurrency block present in build and deploy. This can lead to resource waste if multiple pushes occur rapidly.
    • Fix: Add concurrency to link-check or define it once at the workflow level.

Minor

  1. Artifact Retention
    • Line: 32
    • Note: retention-days: 1 is aggressive but acceptable for documentation artifacts.

Summary
Fix the invalid actions/checkout@v6 reference immediately (Item 1). Consider tightening permissions (Item 2) and fixing the job dependency graph (Items 4-5) before merging.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

  1. .github/workflows/pr-main_mdbook.yml: deploy only depends on build, so a broken-link failure does not gate publishing on main. As written, the docs can be deployed as soon as the artifact is uploaded, even if link-check fails afterward. deploy should depend on link-check as well, e.g. needs: [build, link-check].

  2. .github/workflows/pr-main_mdbook.yml: permissions: contents: write is set at workflow scope, which gives the build and link-check jobs a write-capable GITHUB_TOKEN even though they only need read access. For a security-sensitive repo, that is broader than necessary. Move contents: write to the deploy job and keep the rest read-only.

  3. .github/workflows/pr-main_mdbook.yml and Makefile: the workflow is path-filtered on docs/**, book.toml, and the workflow file itself, but the actual build runs make docs. A PR that changes the docs recipe in Makefile can break publication without this workflow running at all. Add Makefile to the path filters, or invoke mdbook build directly in the workflow so the trigger matches the real dependency.

No consensus-layer code is touched in this PR, so I do not have findings on fork choice, state transition, attestation, XMSS, or SSZ logic here.

I did not run the workflow locally; mdbook and actionlint are not installed in this environment.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 30, 2026

Greptile Summary

This PR adds a GitHub Actions workflow to build the mdBook documentation site and publish it to GitHub Pages on pushes to main, alongside minor improvements to Makefile (--locked flag) and book.toml (optional = true, warning-policy = \"error\"). The workflow is well-structured: it scopes contents: write to the deploy job only, uses per-job concurrency groups, and separates internal link checking (via mdbook-linkcheck2 inside the build) from external link checking (via lychee).

Confidence Score: 5/5

Safe to merge — only one P2 style finding; all core logic is correct and well-guarded

No P0 or P1 issues found. The single finding is a P2 suggestion to pin dtolnay/rust-toolchain@master to a stable reference. The artifact version pairing (upload@v7 / download@v8) is the intended combination per GitHub's changelog. Permissions, concurrency, and deploy guards are all correctly scoped.

No files require special attention

Important Files Changed

Filename Overview
.github/workflows/pr-main_mdbook.yml New workflow to build mdBook docs and publish to GitHub Pages; well-structured with per-job permissions and concurrency, but uses a floating @master reference for dtolnay/rust-toolchain
Makefile Adds --locked to both cargo install calls in docs-deps for reproducible installs; clean improvement
book.toml Adds optional = true to skip linkcheck2 when not installed, and warning-policy = "error" to fail CI on broken internal links; correct and well-commented

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    trigger["Trigger: push/PR on docs/**, book.toml\nor workflow_dispatch"]

    trigger --> build

    subgraph build["Build job"]
        checkout1["checkout@v6"]
        setupMdbook["Setup mdBook 0.5.2"]
        setupRust["Setup Rust stable"]
        installLC2["Install mdbook-linkcheck2 0.12.0"]
        makeDocs["make docs → book/html/"]
        uploadCheck{{"ref == main?"}}
        upload["upload-artifact@v7\ndocs artifact"]

        checkout1 --> setupMdbook --> setupRust --> installLC2 --> makeDocs --> uploadCheck
        uploadCheck -- yes --> upload
        uploadCheck -- no --> skipUpload["skip upload"]
    end

    build --> linkcheck
    build --> deployCheck

    subgraph linkcheck["Link Check job"]
        checkout2["checkout@v6"]
        lychee["lychee-action@v2\nexternal URL check on docs/"]
        checkout2 --> lychee
    end

    deployCheck{{"ref == main?"}}
    deployCheck -- yes --> deploy
    deployCheck -- no --> nodeploy["skip deploy"]

    subgraph deploy["Deploy job (contents:write)"]
        download["download-artifact@v8\ndocs artifact"]
        ghpages["actions-gh-pages@v4\npublish to gh-pages branch"]
        download --> ghpages
    end
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/workflows/pr-main_mdbook.yml:41
**Unpinned `@master` reference for rust-toolchain action**

`dtolnay/rust-toolchain@master` is a floating, mutable reference — any commit pushed to `master` silently changes the action's behavior in CI. All other actions in this workflow use stable version tags (`@v2`, `@v4`, etc.). Pinning to a SHA or a stable tag improves reproducibility and reduces supply-chain risk.

```suggestion
        uses: dtolnay/rust-toolchain@stable
```

Reviews (2): Last reviewed commit: "ci(docs): exclude eprint.iacr.org from l..." | Re-trigger Greptile

Comment thread .github/workflows/pr-main_mdbook.yml
Comment thread .github/workflows/pr-main_mdbook.yml
Comment thread .github/workflows/pr-main_mdbook.yml
Comment thread .github/workflows/pr-main_mdbook.yml Outdated
@MegaRedHand MegaRedHand marked this pull request as draft April 30, 2026 15:57
The linkcheck2 preprocessor is already configured in book.toml and
matches what `make docs-deps` installs locally. Drop the lychee job and
let `mdbook build` fail on broken internal links by setting
`warning-policy = "error"`. CI installs mdbook-linkcheck2 so the
optional renderer actually runs.

Verified locally: `mdbook build` exits 101 when SUMMARY.md is missing
an entry referenced from another page.
`cargo install` without --locked regenerates the lockfile from semver
constraints, so a transitive dep can update overnight and break the
install. Use the lockfile published with each tool for reproducible
installs locally and in CI.
mdbook-linkcheck2 only validates internal links (follow-web-links is
off so contributors don't get blocked by transient external failures
or rate limits). Lychee covers the external side: bitrot in linked
papers, blog posts, and other clients' READMEs.

Lychee runs alongside deploy on main; deploy doesn't depend on it,
so a flaky external server can't block a release.
Addresses bot-review feedback on PR #331:

- Default workflow permissions to `contents: read`, scope `contents:
  write` to the deploy job only. Limits blast radius of any compromise
  in the build/link-check toolchain.
- Add `cancel-in-progress: true` to the build concurrency group so a
  new push drops the stale build. Keep deploy as `cancel-in-progress:
  false` and split the group key so a fresh build does not cancel an
  in-flight deploy mid-push.
- Add `Makefile` to path filters so changes to `make docs` retrigger
  the workflow.
- Add a header comment linking the ethrex workflow used as a model.

Skipped feedback (intentional):
- `actions/checkout@v6` is current (v6.0.2) and matches `ci.yml`.
- `deploy needs: build` (not link-check) is by design: a flaky
  external URL should not gate a docs deploy. Internal links are
  enforced inside the build via `warning-policy = "error"`.
The pinned `@v4` tags were three (upload) and four (download) major
versions behind. No breaking changes affect our usage:

- We download by `name:`, not `artifact-ids:`, so the v5
  download-artifact path consistency change is irrelevant.
- The v6/v7 Node.js 24 bumps require runner ≥ 2.327.1, which
  GitHub-hosted runners already satisfy.
- The v7 upload-artifact direct-upload feature is opt-in
  (`archive: false`); default behavior is unchanged.
- The v8 download-artifact hash-mismatch default of `error` is a
  security improvement, transparent on successful runs.
@MegaRedHand MegaRedHand marked this pull request as ready for review April 30, 2026 19:14
@github-actions
Copy link
Copy Markdown

🤖 Kimi Code Review

Critical: Invalid GitHub Actions versions

The workflow references action versions that do not exist. These must be corrected or the workflow will fail:

  1. .github/workflows/pr-main_mdbook.yml:29: actions/checkout@v6 → Use v4 (latest stable).
  2. .github/workflows/pr-main_mdbook.yml:56: actions/upload-artifact@v7 → Use v4.
  3. .github/workflows/pr-main_mdbook.yml:97: actions/download-artifact@v8 → Use v4.

Security/Reproducibility

  1. .github/workflows/pr-main_mdbook.yml:38: Pin dtolnay/rust-toolchain@master to a specific version (e.g., dtolnay/rust-toolchain@stable or a commit SHA). Using @master introduces supply chain risk as the action's behavior can change without code review.

Code Quality Observations

  1. Makefile (lines 53-54): Adding --locked to cargo install commands is correct. This ensures reproducible builds by respecting the upstream Cargo.lock files, preventing unexpected breakage from semantic versioning violations in dependencies.

  2. .github/workflows/pr-main_mdbook.yml (permissions): The permission structure follows least-privilege correctly—contents: read globally with contents: write elevated only in the deploy job.

  3. book.toml:21-26: The configuration for mdbook-linkcheck2 is sound. Setting optional = true prevents friction for local contributors while warning-policy = "error" ensures CI enforces link integrity.

Consensus-layer note: This PR contains only documentation infrastructure changes (CI, Makefile, mdbook config). No consensus logic (fork choice, state transition, XMSS, SSZ) was modified and therefore requires no security review for those components.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

The IACR preprint server returns 403 to lychee's default User-Agent
(anti-bot protection); links work fine in a browser. Treating these
as broken adds noise without catching real bitrot.
@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Here is the review of PR #331:


PR #331ci(docs): publish mdbook to GitHub Pages

Overview: Adds a GitHub Actions workflow to build, link-check, and deploy the mdBook documentation site to GitHub Pages. Also adds --locked to Makefile cargo install commands and sets warning-policy = "error" in book.toml so broken internal links fail CI.


Bug / Correctness

Artifact action version mismatch (high priority)

upload-artifact@v7 (build job, line 57) and download-artifact@v8 (deploy job, line 81) use different major versions. These two actions must be compatible in how they store/retrieve artifacts. Using a newer download version against an older upload version risks a silent failure or format mismatch at deploy time. They should be pinned to the same major version.

link-check does not gate deploy

Both link-check and deploy declare needs: build and run in parallel. A failed external link check on a push to main will not block the deployment — the workflow will still publish a build that lychee flagged as broken. If the intent is to enforce external link health before publishing, deploy should add link-check to its needs list. If the intent is to treat external link failures as informational (transient network failures are common), then fail: true on lychee is misleading. Pick one model and be consistent.


Security / Supply Chain

dtolnay/rust-toolchain@master (line 42)

Using @master is an unpinned floating reference. If the action changes behavior, CI will silently pick it up. Consider pinning to a specific commit SHA. This is common practice in the Rust ecosystem but is still worth flagging for a security-conscious project.

Third-party actions (peaceiris/actions-mdbook@v2, peaceiris/actions-gh-pages@v4, lycheeverse/lychee-action@v2) are not SHA-pinned. Pinning to a commit SHA is the GitHub-recommended hardening for contents: write workflows.


Design Observations

link-check runs on main pushes too, but results are non-blocking

The job checks docs/ source Markdown, not the built HTML. This is fine for catching raw URL rot, but if external link failures don't block the deploy (see above), running it on every main push adds CI cost with no enforcement. Consider restricting it to PRs only (if: github.event_name == 'pull_request') unless you're also adding it to deploy's needs.

build job uploads the artifact only on refs/heads/main (line 55)

The deploy job also has if: ${{ github.ref == 'refs/heads/main' }}, so this pairing is correct. However, link-check (needs: build) will try to run on PRs without a pre-built artifact upload — that's fine since it doesn't use the artifact, but it's worth confirming the intent is to run link checks on PRs against source files only (not built HTML).


What Looks Good

  • Workflow-level permissions: contents: read with per-job escalation only for the deploy job — correct least-privilege approach.
  • cancel-in-progress: true on build, false on deploy — exactly right; you never want to interrupt an in-flight deploy.
  • retention-days: 1 on the artifact is appropriate for a transient deploy artifact.
  • workflow_dispatch trigger is a useful escape hatch.
  • --locked added to Makefile's cargo install calls is a solid reproducibility improvement.
  • warning-policy = "error" with optional = true in book.toml is a clean separation: local builds are lenient, CI is strict.
  • No checkout in the deploy job (not needed, correct).
  • The book/html artifact path is correct given book.toml declares both [output.html] and [output.linkcheck2] (mdBook writes HTML to book/html/).

Summary

Two items need resolution before merge:

  1. Align upload-artifact and download-artifact to the same major version to avoid a potential format incompatibility at deploy time.
  2. Decide whether link-check failures should gate deploy, and either add link-check to deploy's needs, or drop fail: true from lychee (and document the decision).

The @master / unpinned third-party action pins are lower-priority but worth addressing given contents: write is in scope.


Automated review by Claude (Anthropic) · sonnet · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

  1. .github/workflows/pr-main_mdbook.ymldeploy only depends on build, so pushes to main can publish gh-pages before external link validation finishes. If lychee fails, the workflow ends failed but the broken docs may already be live. deploy should depend on link-check too. Since link-check only scans docs/, .github/workflows/pr-main_mdbook.yml can likely drop needs: build and run in parallel.

  2. .github/workflows/pr-main_mdbook.yml — The new workflow introduces mutable action refs (dtolnay/rust-toolchain@master, plus tag-based refs on the other uses: lines). In a pipeline that ends with contents: write deployment, that is a supply-chain risk: upstream retags or force-pushes can change what runs here without a repo change. Pin actions to commit SHAs, especially the deploy path.

  3. .github/workflows/pr-main_mdbook.yml — The paths filters omit Makefile, even though this workflow runs make docs and this PR changes docs-related targets there. A future Makefile-only change can break docs CI or change docs build behavior without triggering this workflow. Add Makefile to both push.paths and pull_request.paths.

Aside from that, the Makefile and book.toml changes themselves look good: adding --locked improves reproducibility, and warning-policy = "error" makes internal link failures visible in CI. No consensus-layer code paths are touched in this PR.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

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.

Publish mdbook to GitHub Pages

1 participant