Skip to content

Support demos build with npm - #4

Merged
zjn0505 merged 2 commits into
masterfrom
demo/build-npm
Aug 31, 2026
Merged

zjn0505 merged 2 commits into
masterfrom
demo/build-npm

Conversation

@zjn0505

@zjn0505 zjn0505 commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

This pull request updates the process and documentation for publishing OpenAPI demos to GitHub Pages, focusing on improved safety, clearer rules for demo publishing, and stricter CI/CD controls. The changes clarify how demo folders are selected and built, introduce safety checks in the assemble script, and update the workflow to ensure only trusted code is built and deployed.

Demo publishing and build process improvements:

  • The criteria for publishing a demo have been clarified: a folder is published if it has no vercel.json and either (a) a root index.html with no package.json (entire folder copied), or (b) a package.json and a lockfile, in which case CI builds and publishes the dist/ directory. Builds without a lockfile now fail fast. [1] [2] [3] [4]
  • The assemble-pages.sh script now uses canonical path normalization for the output directory and refuses to operate on unsafe paths, improving safety.
  • The script now skips folders that do not meet the new publishing criteria and emits clear error messages for missing build artifacts or lockfiles. [1] [2]

Workflow and CI/CD controls:

  • The GitHub Actions workflow is split into assemble (builds site artifacts) and deploy (publishes to Pages), with strict conditions: only same-repo PRs and the master branch are built or deployed, and fork PRs are excluded from running arbitrary builds.
  • Artifacts are uploaded with hidden files included, ensuring .nojekyll is preserved for Pages.

Documentation and rules:

  • New documentation (.cursor/rules/demos-pages.mdc) specifies token safety, demo folder naming, assemble script rules, and workflow trust boundaries, aligning the README and script.
  • The README.md is updated to match the new publishing and build rules, with clear instructions for different demo types and new guidance for PR contributors. [1] [2]

These changes collectively ensure a safer, more predictable, and well-documented process for demo publishing and CI/CD.

@zjn0505
zjn0505 requested a lite review from Copilot August 31, 2026 04:55
@vercel

vercel Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
temi-openapi-docs Ready Ready Preview Aug 31, 2026 5:49am

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new npm-build path should handle/communicate missing lockfiles more clearly (or adjust install strategy) and the documentation should be unambiguous about the entrypoint/publishing expectations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds support for building and publishing demo apps to GitHub Pages via an npm build step, enabling demos that use tooling like Vite/TypeScript to be deployed as static dist/ output.

Changes:

  • Documented how Pages publishing works for demos with package.json and npm run build.
  • Updated the Pages assembly script to run npm ci && npm run build and publish dist/ when a demo contains package.json.
  • Updated the Pages workflow to install Node.js before assembling the site.
File summaries
File Description
demos/README.md Documents publishing rules for static vs npm-built demos and expected build output (dist/).
demos/assemble-pages.sh Adds npm build support for demos with package.json and publishes dist/ output.
.github/workflows/pages-demos.yml Ensures Node is available in CI so demo builds can run.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread demos/README.md Outdated
Comment thread demos/assemble-pages.sh
Comment thread .github/workflows/pages-demos.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The workflow now runs npm builds on pull_request, which should be gated to avoid executing arbitrary scripts for forked PRs, and the new npm ci path should clearly enforce/document lockfile requirements.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

demos/README.md:26

  • This text implies any demo with package.json will build via npm ci, but npm ci fails without a lockfile. Consider documenting the lockfile requirement explicitly so contributors don’t discover it only via CI failures.
Official static host: [https://robotemi.github.io/openapi/](https://robotemi.github.io/openapi/) (index of every client-only demo). A folder is published when it has `index.html` and no `vercel.json`. If the folder has `package.json`, Pages runs `npm ci && npm run build` and publishes `dist/`.

demos/assemble-pages.sh:74

  • npm ci requires a lockfile (package-lock.json or npm-shrinkwrap.json). As written, any demo that adds package.json without a lockfile will fail with a less clear npm error; it’s better to fail fast with a targeted message (and optionally disable audit/fund network calls for CI speed/noise).
        if [ -f "${dir}package.json" ]; then
            echo "assemble-pages: building $name"
            (cd "$dir" && npm ci && npm run build)
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .github/workflows/pages-demos.yml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The workflow likely lacks required pages permissions for the configure-pages step in the assemble job, which may break master deployments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .github/workflows/pages-demos.yml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new documentation/rules text contains a few mismatches/overstatements relative to the actual assembly behavior and should be corrected before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

demos/README.md:42

  • The contributing checklist for Vite/TypeScript demos requires package-lock.json, but the actual assemble script also accepts npm-shrinkwrap.json. The README should mention shrinkwrap here too so the guidance matches the implementation.
4. Client-only static demos (no build): add `index.html` at the folder root, omit `package.json` and `vercel.json`, and use a URL-safe folder name (`A–Z a–z 0–9 . _ -`). Merge to `master` and the Pages workflow copies the folder to `https://robotemi.github.io/openapi/<name>/`.
5. Client-only demos that use Vite/TypeScript: commit `package.json` **and** `package-lock.json`, with `npm run build` writing `dist/index.html`. Set Vite `base` to `./`. Omit `vercel.json`. Do not commit `node_modules/` or `dist/`. Pages runs `npm ci && npm run build` and publishes `dist/`.
6. Serverless demos: include `vercel.json`. Deploy as a separate Vercel project.
  • Files reviewed: 4/4 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread demos/README.md Outdated
Comment thread .cursor/rules/demos-pages.mdc Outdated
Comment thread demos/assemble-pages.sh Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The updated workflow’s job-level permissions omit actions: read, which can break artifact upload/download and prevent deployment.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

.github/workflows/pages-demos.yml:53

  • actions/download-artifact@v4 requires actions: read when permissions are locked down per-job; currently the deploy job permissions omit it, which can prevent downloading the site artifact and break deployment.
    runs-on: ubuntu-latest
    permissions:
      contents: read
      pages: write
      id-token: write
    steps:
      - uses: actions/download-artifact@v4
        with:
          name: site
          path: _site
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread demos/assemble-pages.sh
Comment thread .github/workflows/pages-demos.yml
@zjn0505
zjn0505 merged commit a1baf23 into master Aug 31, 2026
4 checks passed
@zjn0505
zjn0505 deleted the demo/build-npm branch August 31, 2026 05:51

This branch was successfully deployed

1 active deployment
Preview — 22a57596 Deployed Aug 31, 2026 by vercel[bot]
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.

2 participants