fix: make the prefix and headless flags mean what they say - #68
Merged
Conversation
Three related bits of build configuration that were decorative. `--path-prefix=` / `MP_PREFIX` were parsed and exported and then read by nothing in the Astro build, so `--path-prefix=docs` produced a dist/ that still said knowledge-base in the Astro base, in every rewritten sub-app URL and in every masthead link — only the closing summary agreed with the flag. Making it real means templating nginx.conf, the /__wf/ rewrite and the gateway's route patterns as well, so the flag is gone and the prefix is one constant the config and both pages import. `vite.config.js` imported ./plugins/marketplace.js, which does not exist, and took index.html at the repo root as its input, which does not exist either. No script invoked it. It was also the only reader of MP_PREFIX, which is what made that flag look wired up. Deleted, and the "Two Build Configs" section of CLAUDE.md with it. Base.astro ORed the per-app headless prop with the global flag, so an app declaring "headless": false in a headless build was still rendered headless — the override only worked in one direction. It now trusts the prop and falls back to the build default only when unset. That default also moves into one helper: Base.astro read `MP_HEADLESS !== 'false'` (unset means headless) while the orchestrator computed the opposite, and nothing caught it because the orchestrator never leaves the variable unset. Unset now means standalone everywhere, documented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqFK6yffibtCBTF8xZ4hXW
An accuracy pass, not a rewrite. What was wrong: - CLAUDE.md listed src/components/Chrome.astro and src/templates/chrome.js as key files. Neither exists; the chrome bar and its theme script were removed when the masthead became the whole navigation. It also credited Base.astro with a "theme script" in a light-only marketplace. - README's summary said standalone mode injects a top chrome with an app switcher. There is no chrome bar and no app switcher in either mode. - CLAUDE.md said the test harness registers ../knowledge-base-docs-example. It registers the vendored tests/fixtures/docs-example.dist.tar.gz — the sibling repo is the optional single-page example, a different entry. - Headless mode was described as injecting the shadow-DOM compat styles. Base.astro emits those unconditionally; the attribute is the only difference between the two modes. - The suite list covered three of eight spec files and neither of the two extra Playwright configs; README's test table omitted the container layer entirely and said "both layers run in CI". - STYLE_GUIDE was described as covering dark mode. - AGENTS.md pointed at scripts/build-vite.js for stageArtifact, which lives in scripts/artifacts.js. - build-vite.js printed "1/3" for the first of four steps, and its header comment omitted the hoist step and named the wrong asset destination. Also drops the archaeology the previous commit left behind: the deleted vite.config.js and the removed MP_PREFIX are simply absent now rather than described as things that used to be wrong. Adds what was missing: the per-app headless override, the CSS url() rewrite in copyAssets, the KB_* test variables, the CRLF checkout caveat, and the container suite's reuseExistingServer trap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqFK6yffibtCBTF8xZ4hXW
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Three pieces of build configuration that looked live and were not: a flag nothing read, a config file nothing could run, and an override that only worked in one direction.
Changes
--path-prefix=/MP_PREFIX— deleted (#46)Both were parsed, exported to the Astro build, and read by nothing.
npm run build -- --path-prefix=docsproduced adist/whose Astrobase, sub-app URL rewriting and masthead links all still saidknowledge-base; only the closing build summary agreed with the flag.Supporting it properly means templating
nginx.conf, the/__wf/rewrite and the fragment gateway's route patterns too — all of which bake the string in — so the honest fix is the one the issue recommends: drop the flag. The prefix is nowPATH_PREFIX/BASE_PATHin the newsrc/utils/config.js, imported byastro.config.mjs,index.astroand[...path].astroinstead of being written out separately in each. Documented as a deployment-wide change rather than a build option.vite.config.js— deleted (#47)It imported
./plugins/marketplace.js(noplugins/directory in the repo) and usedindex.htmlat the repo root as its Rollup input (no such file — the landing page issrc/pages/index.astro). No npm script invoked it, so it failed at module resolution the moment anything tried. It was also the only consumer ofMP_PREFIX, which is precisely why that flag looked wired up.CLAUDE.md's "Two Build Configs" section described it as a live half of the architecture; that section is now "One Build Config".Per-app
"headless": false(#52)Base.astrodidheadless || HEADLESS, so the prop could only ever turn headless on. An app pinned"headless": falsein a headless build came out headless anyway. It is nowheadless ?? isHeadlessBuild()— the prop is authoritative, the build default applies only when it is unset.The default itself was inconsistent:
Base.astroand[...path].astroreadMP_HEADLESS !== 'false'(unset ⇒ headless) whilescripts/build-vite.jscomputed--headless || MP_HEADLESS === 'true'(unset ⇒ standalone). Nothing caught it because the orchestrator always exports an explicit value. All three now callisHeadlessBuild(), unset means standalone, and bothCLAUDE.mdandREADME.mdsay so.Tests
scripts/setup-test-apps.mjspins theexternal-docsentry"headless": false, andtests/build-integrity.spec.jsasserts it builds withoutdata-mp-headlesswhile its neighbours in the same headless build keep it. That entry was chosen because it is the only app no existing test makes a headless assertion about, so the pin does not weaken another test.Docs accuracy pass
While removing the stale
vite.config.jssection, the surrounding documentation turned out to have drifted well beyond it, so all three docs were checked line by line against the tree. Corrected: two key source files that no longer exist (Chrome.astro,templates/chrome.js), a "theme script" in a light-only layout, a top chrome bar with an app switcher that neither mode renders, the wrong fixture named as the test registry source, headless mode credited with injecting styles both modes emit, three of eight spec files listed, a missing container test layer, andstageArtifactattributed to the wrong file. Also fixedbuild-vite.jsprinting "1/3" for the first of four steps. Removed items are now simply absent from the docs rather than described as former mistakes.Verification
npm run build:headless— green, 8 appsnpm test— 82 passednpx playwright test --config=playwright.config.ci.js— 22 passednpm run test:container— 28 passed (fresh image)npm run selftest(actions/publish-single-page-docs) — 20 passedCloses #46
Closes #47
Closes #52