diff --git a/.github/workflows/pr-preview.yml b/.github/workflows/pr-preview.yml new file mode 100644 index 0000000..9025199 --- /dev/null +++ b/.github/workflows/pr-preview.yml @@ -0,0 +1,100 @@ +# Builds every PR and uploads the result to Cloudflare as a Worker *version* — +# a preview that leaves production untouched — then comments the preview URL on +# the PR. Complements Workers Builds, which deploys `main` to production but +# reports nothing back to GitHub. +# +# Requires two repository secrets: +# CLOUDFLARE_API_TOKEN — API token with Workers Scripts: Edit on the account +# CLOUDFLARE_ACCOUNT_ID — the Cloudflare account id +# Until they are set (and on fork/Dependabot PRs, which cannot read secrets), +# the build still runs as a check and the preview upload is skipped with a note. +name: PR preview + +on: + pull_request: + branches: [main] + +concurrency: + group: pr-preview-${{ github.event.pull_request.number }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: write + +jobs: + preview: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version-file: .node-version + cache: npm + + - run: npm ci + + - run: npx astro build + + - name: Check for Cloudflare credentials + id: creds + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + run: | + if [ -n "$CLOUDFLARE_API_TOKEN" ]; then + echo "available=true" >> "$GITHUB_OUTPUT" + else + echo "available=false" >> "$GITHUB_OUTPUT" + echo "CLOUDFLARE_API_TOKEN is not available (missing secret, fork PR, or Dependabot) — preview upload skipped." >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Upload preview version to Cloudflare + if: steps.creds.outputs.available == 'true' + id: upload + uses: cloudflare/wrangler-action@v3 + with: + apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} + accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + # wrangler is not a project dependency - it is invoked through npx - so the action + # cannot find one and falls back to its own default, 3.90.0. That version predates + # assets-only Workers and fails with "Missing entry-point", because wrangler.jsonc + # declares `assets` and no `main`. Pin the version the project actually uses. + wranglerVersion: "4.127.0" + command: versions upload + + - name: Comment the preview URL on the PR + if: steps.creds.outputs.available == 'true' + uses: actions/github-script@v7 + env: + WRANGLER_OUTPUT: ${{ steps.upload.outputs.command-output }} + with: + script: | + const match = (process.env.WRANGLER_OUTPUT || '').match(/https:\/\/\S+\.workers\.dev/); + const marker = ''; + const sha = context.payload.pull_request.head.sha.slice(0, 7); + const body = match + ? `${marker}\n### 🔍 Preview\n\n**${match[0]}**\n\nBuilt from \`${sha}\`. Uploaded as a Worker version — production is unaffected until merge.` + : `${marker}\n### 🔍 Preview\n\nThe preview version uploaded, but no preview URL appeared in the wrangler output — check that the worker's Preview URLs setting is enabled. Built from \`${sha}\`.`; + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + per_page: 100, + }); + const existing = comments.find((c) => c.body && c.body.includes(marker)); + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body, + }); + } diff --git a/.gitignore b/.gitignore index 06353e8..eda4f7e 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ dist/ .wrangler/ screenshots/ .playwright-mcp/ +.DS_Store diff --git a/CLAUDE.md b/CLAUDE.md index da45e46..4ebbfda 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,8 +9,8 @@ Migrating OpenDialog's product docs off GitBook onto Astro Starlight, deployed t ## Hard rules 1. **`documentation` branch is read-only.** GitBook syncs to it bidirectionally. Writing to it corrupts the live site. Work on a feature branch. -2. **Never hand-edit files in `src/content/docs/`.** They are generated. Found a bug? Fix `scripts/convert.mjs` and re-run. A manual edit is silently destroyed on the next run and creates a bug that reappears at cutover. -3. **Scripts must be idempotent.** `convert.mjs` and `assets.mjs` run repeatedly, right up to cutover day, against fresh GitBook syncs. Same input must give byte-identical output. +2. **`src/content/docs/` is hand-authored and is the source of truth.** Author pages in Starlight dialect: `.md`/`.mdx`, site-absolute internal links (`/core-concepts/…`), and `~/assets/…` image paths so `astro:assets` optimises them. The GitBook conversion was a one-time process and it is complete — never regenerate this directory to change content. +3. **Never run `npm run convert` or `scripts/convert.mjs`.** `convert.mjs` deletes `src/content/docs/` wholesale and rewrites it from the frozen `source/` snapshot, discarding every page authored since the migration. The pipeline is retired; it and its tests are kept as the record of how the corpus was produced. `assets.mjs` and `routes.mjs` remain safe to run on their own, and must stay idempotent — same input, byte-identical output. 4. **`source/` is pristine and git-ignored.** All scripts read from `source/`, write to `src/`. Never mutate `source/` in place. 5. **URLs do not change.** Not the ugly ones either. Every path in the live `sitemap.xml` must resolve. This is the acceptance test. 6. **Do not edit documentation prose.** Not to fix typos, not to improve clarity. Log it in `MIGRATION-NOTES.md`. @@ -19,11 +19,11 @@ Migrating OpenDialog's product docs off GitBook onto Astro Starlight, deployed t ## Gotchas that will bite - **`{ attr | filter }` in prose breaks MDX builds.** OpenDialog's docs are full of this template syntax. In `.mdx` the braces parse as JSX expressions. Safe inside code fences, fatal outside them. Prefer `.md` — Starlight asides (`:::note`) work there. Only promote to `.mdx` when a Starlight component is genuinely required. -- **Images must live in `src/assets/`, never `public/`.** Files in `public/` bypass `astro:assets` entirely and ship unoptimised. With 541 MB of source images this is the whole performance story. +- **Images must live in `src/assets/`, never `public/`.** Files in `public/` bypass `astro:assets` entirely and ship unoptimised. With 541 MB of source images this is the whole performance story. Exception: video and downloadable files cannot pass through `astro:assets`, so the one MP4 lives in `public/media/` and the one CSV in `public/files/`. - **Raw `` in markdown is not optimised.** The source has 430 `
` blocks. They must become markdown `![]()` syntax or the optimisation never happens. Highest-value transformation in the project. - **1,317 asset filenames contain spaces or parentheses**, and GitBook wraps those paths in angle brackets: `![](<../.gitbook/assets/image (149).png>)`. Handle that form or you will silently drop images. -- **50 assets have no file extension.** Sniff magic bytes and rename, or they get served with the wrong MIME type. -- **One 28 MB GIF exceeds Cloudflare's 25 MiB per-file limit** and will fail deployment. Re-encode to MP4. +- **50 assets have no file extension**, and all 50 are orphans — referenced from nowhere in `source/` — so none is ever copied and no MIME type is ever served. +- **The GIF that ships, `Knowledge Base Demo.gif`, is 22.56 MiB** — under Cloudflare's 25 MiB per-file limit — and is re-encoded to a 1.12 MiB MP4 for weight, not to clear the cap. A separate 28 MB GIF that does exceed the cap is an orphan, referenced by nothing, and is never copied. - **Do not set `run_worker_first`** in `wrangler.jsonc`. Static asset requests are free and unlimited; Worker invocations are metered at 100k/day on the free plan. - **Never make DNS changes.** Pat repoints `docs.opendialog.ai` manually at cutover. Cloudflare API access is available for Workers and deployments only. - **Pat owns the GitBook analytics export.** Do not attempt it. @@ -55,16 +55,16 @@ URL set. Measured against the live sitemap: 204. Route parity is judged against ```bash npm run dev # local dev server npm run build # astro build -> dist/ -npm run convert # source/ -> route-map.json -> src/content/docs/ -> sidebar -node scripts/assets.mjs # rename, re-encode, rewrite refs +npm run convert # source/ -> route-map.json -> asset-map.json -> src/content/docs/ -> sidebar +node scripts/assets.mjs # copy, encode, write asset-map.json node scripts/routes.mjs # built routes vs live sitemap.xml, writes route-map.json npx wrangler deploy # deploy to Cloudflare ``` -`npm run convert` runs `routes.mjs`, `convert.mjs` and `sidebar.mjs` in that order. `convert.mjs` -reads `route-map.json` rather than `source/` directly, so running it on its own against a sync -that adds a page silently leaves the new page unconverted — always use `npm run convert`, never -`node scripts/convert.mjs` alone. +`npm run convert` runs `routes.mjs`, `assets.mjs`, `convert.mjs` and `sidebar.mjs` in that order. +`convert.mjs` reads `route-map.json` and `asset-map.json` rather than `source/` directly, so +running it on its own against a sync that adds a page or an image silently leaves the addition +unconverted — always use `npm run convert`, never `node scripts/convert.mjs` alone. ## Definition of done for any change diff --git a/MIGRATION-NOTES.md b/MIGRATION-NOTES.md index b08d227..05fa8e3 100644 --- a/MIGRATION-NOTES.md +++ b/MIGRATION-NOTES.md @@ -437,6 +437,37 @@ image. --- +## 2026-07-29 — Workers Builds branch control: no per-branch exclusions exist + +Correcting earlier guidance in this file and in PR #13, which said to exclude +`documentation`, `3.9` and `release/*` from build triggers. **That capability does not +exist.** Workers Builds' branch control (Overview -> Worker -> Settings -> Build -> Branch +control) offers only two things: a production-branch dropdown, and a single all-or-nothing +checkbox, "Builds for non-production branches". + +Non-production builds are currently **enabled**. Evidence: two pushes to +`fix/ci-lockfile-npm-version` each produced a `versions upload` with no deployment, matching +the documented non-production deploy command. + +| Commit pushed | Version uploaded | +|---|---| +| `af08648` 12:11:56Z | 12:12:28Z | +| `a2772eb` 12:15:40Z | 12:16:20Z | + +So the `documentation` problem is live. Every GitBook sync to that branch triggers a build +that fails at `npm ci`, because the branch has no `package.json`. There is no clean +mitigation: the failure happens at the install step before any command we control runs, and +a `package.json` cannot be added to `documentation` — it is GitBook's, read-only, and writing +to it corrupts the live site. + +**Recommendation: disable non-production branch builds.** The reason is alarm fatigue rather +than noise as such — if `documentation` fails several times a day, a real failure on `main` +is lost in the stream. Preview URLs would be convenient for the Phase 4 look-and-feel review, +but are not required: `scripts/screenshots.mjs` points at a local dev server just as well as +at a preview URL. Re-enable for that stretch if it earns its keep. + +--- + ## 2026-07-30 — Phase 2 gate: conversion script complete `scripts/convert.mjs` and `scripts/sidebar.mjs` are done. 204 pages convert, `astro build` @@ -1348,6 +1379,664 @@ re-run twice leaves `git status --short` empty except for the intentional source themselves. Exactly two files under `src/content/docs/` changed as a result of the critical fix; zero changed as a result of the `protectCode` or `convertContentRefs` fixes. +--- + +## 2026-07-30 — Phase 3 gate: assets complete + +Every image on the built site resolves. `scripts/assets.mjs` copies, slugifies and re-encodes +the 500 referenced GitBook assets into `src/assets/` and `public/`; `convert.mjs` emits their +real paths from `asset-map.json`. Before this phase every image 404'd. + +**Every figure below was re-measured at this commit**, from a fresh `npm run convert` + +`npx astro build`, not transcribed from the design spec, the plan, or the per-task ledger. Where +a figure could only be sourced from the (git-ignored, soon-deleted) task ledger — the pre-encode +byte count and the historical ffmpeg-flag proof — that is stated explicitly rather than presented +as newly measured. + +### Gate evidence + +| Check | Result | +|---|---| +| `npm run convert` | `routes.mjs`, `assets.mjs`, `convert.mjs`, `sidebar.mjs` all `ok`/green | +| `npx astro build` | succeeds, 205 pages (204 + `404.html`), 496 optimised image variants | +| `npm test` | 187/187 pass | +| On-disk reference resolution (corrected check, see Override 1 below) | 523 checked, **0 broken** | +| Built-site placeholder scan (`src="/.gitbook/assets/…"` in `dist`) | **0** occurrences | +| `_astro`-referencing HTML files in `dist` | 205 of 205 | +| `src/assets` size (byte sum, the gate) | 33,591,322 B = 32.0 MiB against 60,000,000 | +| `src/assets` size (`du -sh`, allocated blocks) | 33M (`du -sm`: 34) | +| `public` size (`du -sh`, allocated blocks) | 2.0M (`du -sm`: 3) | +| `public` size (byte sum, all three tracked files) | 1,195,546 B = 1.14 MiB | +| Largest file, any destination | 1,179,445 B = 1.12 MiB (the MP4), under the 25 MiB cap | +| `find src/assets public -type f -size +25000k` | empty | +| `git status` after the whole pipeline re-run twice | clean | + +**`du` and the byte sum disagree, and the gate is defined on the byte sum.** `du` reports +allocated disk blocks; with ~500 small files the block overhead is real. Both numbers stay far +under the 60 MB gate, so this is a reading-the-number caution, not a risk: use the byte sum +(`33,591,322 B`) as the authoritative figure, `du -sh`'s `33M` as a sanity check only. + +**`public`'s byte sum includes `favicon.png`, which this phase does not manage.** `du -sh public` +reads `2.0M`; the true byte sum of everything `find public -type f` walks is `1,195,546 B`: +`public/favicon.png` (15,074 B, tracked since `c31854d`, Phase 1's brand asset) + +`public/files/deliveryknowledgebase.csv` (1,027 B) + +`public/media/knowledge-base-demo.mp4` (1,179,445 B). This phase's own contribution — the CSV +and the MP4 — is `1,180,472 B`, matching the "Files in `public/media`: 1" / "Files in +`public/files`: 1" figures below. + +### Override 1 — the brief's Step 2 check was blind to the corpus's one `public/files/` reference + +The brief's on-disk proof matched only the double-quoted attribute form, +`/"\/(media|files)\/([^"]+)"/g`. The corpus's single `public/files/` reference is emitted as a +**markdown link** — `[DeliveryKnowledgeBase.csv](/files/deliveryknowledgebase.csv)` — which has +no quotes, so that check could not see the one reference it most needed to prove. Extended here +to also scan the markdown-link delimiter shape (`](…)`, with optional angle brackets) for both +`/media/` and `/files/`, matching what `convert.mjs`'s own on-disk scan already does. Re-run: + +``` +references checked: 523 +broken: 0 +``` + +523 matches `convert.mjs`'s own `asset references 523` counter exactly. Confirmed the CSV line +is inside that count by direct grep — the reference resolves to +`public/files/deliveryknowledgebase.csv`, which exists. + +### The final invariant output of `assets.mjs`, from a real run + +``` +ok references 509 +ok coverHrefs 13 +ok copySet 500 +ok mapEntries 500 +ok slugCollisions 0 +ok src/assets size 32.0 MiB +ok largest file 1.1 MiB + encoded 0 + reused from cache 500 + +wrote 500 assets and asset-map.json +``` + +`encoded 0` / `reused from cache 500` because every asset was already committed +byte-identical from Tasks 4–7. **Verified reproducibility independently at this gate**, not +merely asserted: `asset-map.json` was deleted and `assets.mjs` re-run cold. Cold run took +**75.8 s wall / 92.7 s user** (the ledger's Task 6 run recorded ~89 s; the difference is +machine-load variance in a `sharp`/`ffmpeg`-bound job, not a regression — user time is the +closer comparison since both are CPU-bound). Output was **byte-identical**: `git status` +after the cold re-encode was clean, and a second run afterwards again reported +`reused from cache 500` / `encoded 0` in 0.3 s. + +### `convert.mjs`'s own invariants, from the same run + +``` +ok files 204 +ok mdx 46 +ok asides 258 +ok contentRefCards 51 +ok cardTableCards 41 +ok embeds 36 +ok cardGrids 9 +ok images 528 +ok droppedCovers 13 +ok survivingBlocks 0 +ok survivingEntities 0 +ok survivingImgTags 0 +ok survivingBraces 0 + ordered list items 149 + asset references 523 +``` + +### Every figure in the phase's measured-figures table, re-verified here + +| | Table said | Re-measured | | +|---|---|---|---| +| Asset references in `source/` | 509, 0 missing | **509**, `assets.mjs` did not throw its missing-asset guard | confirmed | +| Cover href occurrences | 13 | **13** (`ok coverHrefs 13`) | confirmed | +| Copy set / `asset-map.json` entries | 500 | **500** / **500** | confirmed | +| Dropped card covers, excluded | 9 | **9** — all absent from `asset-map.json`, verified by name | confirmed | +| Slug collisions | 0 | **0** (`ok slugCollisions 0`) | confirmed | +| Files in `src/assets` | 499 (498 + logo) | **499**; `opendialog-logo.png` present, referenced at `astro.config.mjs:16` | confirmed | +| Files in `public/media` | 1 (the MP4) | **1** | confirmed | +| Files in `public/files` | 1 (the CSV) | **1** | confirmed | +| Copy set by source extension | 487 png, 7 jpg, 2 webp, 3 gif, 1 csv | **487 / 7 / 2 / 3 / 1**, read from `asset-map.json` keys | confirmed | +| Stills | 496 | **496** = 487+7+2 | confirmed | +| Map entries by kind | image 498, file 1, video 1 | **image 498, file 1, video 1** | confirmed | +| `src/assets` after encoding | 33,591,322 B = 32.0 MiB | **33,591,322 B** | confirmed | +| Largest file, all destinations | 1.12 MiB (the MP4) | **1,179,445 B = 1.12 MiB** | confirmed | +| The video | 23,654,022 B → 1,179,445 B | **23,654,022 B → 1,179,445 B**, `ISO Media, MP4 Base Media v1` | confirmed | +| Cold encode / cached run | ~89 s / 0.4 s, reused 500 / encoded 0 | **75.8 s wall (92.7 s user)** / **0.3 s**, reused 500 / encoded 0 | timing varies, mechanism confirmed | +| Markdown images emitted | 528, +1 `