[codex] Fix test env webmcp and site chrome#189
Conversation
|
Warning Review limit reached
More reviews will be available in 27 minutes and 29 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR extends canonical URL support to Changeswebmcp.html Canonical and Header-Hiding Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the test-environment publishing script so the top-level webmcp.html behaves like the other standalone test pages (no site chrome), while ensuring nested /site/*.html pages retain the normal site navbar and still receive the test-environment SEO/indicator directives. Adds integration coverage for the new header-hiding helper.
Changes:
- Extracts
applyTopHeaderHideToHtmland uses it to hide the top header onwebmcp.html(in addition to other top-level test pages). - Stops stripping headers from
/site/app.html,/site/generator.html, and/site/combinatorial.html; instead applies SEO directives (including testenv indicator + canonical URLs). - Adds Jest integration tests validating header-hide injection and non-duplication behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/create-testenv.mjs | Adds an HTML-level helper for header hiding, applies it to webmcp.html, and adjusts /site/* handling to preserve navbar while still injecting SEO/indicator directives. |
| tests/integration/create-testenv-seo.test.js | Adds integration tests covering applyTopHeaderHideToHtml injection and idempotency. |
Greptile SummaryThis PR extends the test-environment publish script to include
Confidence Score: 5/5Safe to merge — the changes are additive and symmetric, giving webmcp.html the same build-script treatment already applied to the three other root pages. All four root pages are now handled consistently in both the header-hiding pass (outputDir) and the SEO-directives pass (fullSiteDir). The idempotency concern from the previous review thread is resolved by delegating to upsertHeadStyle. New tests cover the header-hiding wrapper, the webmcp.html canonical rewrite, and the no-duplication guarantee. No logic regressions identified. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Web Build] --> B[copyWebBuild to outputDir]
B --> C[hideTopHeaderInBuiltPage app.html / generator.html / combinatorial.html / webmcp.html]
C --> D[Storybook build]
D --> E[Docusaurus build to fullSiteDir]
E --> F[copyWebBuild to fullSiteDir]
F --> G[applySeoDirectivesToFile fullSiteDir pages]
G --> H[rm tempWebDir]
H --> I[for ROOT_PAGE_CANONICALS applySeoDirectivesToFile outputDir]
I --> J[applySeoDirectivesToDirectory storybookDir]
Reviews (3): Last reviewed commit: "apply site seo directives to webmcp" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/create-testenv.mjs`:
- Around line 598-606: The build copies apps/web/webmcp.html into fullSiteDir
but the script only calls applySeoDirectivesToFile for app.html, generator.html,
and combinatorial.html so webmcp.html lacks SEO directives; update the
fullSiteDir processing to also call
applySeoDirectivesToFile(path.join(fullSiteDir, 'webmcp.html'), { canonicalUrl:
ROOT_PAGE_CANONICALS['webmcp.html'] }) or alternatively iterate
ROOT_PAGE_CANONICALS for the fullSiteDir so that applySeoDirectivesToFile is
invoked for 'webmcp.html' (references: applySeoDirectivesToFile, fullSiteDir,
ROOT_PAGE_CANONICALS, 'webmcp.html').
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 18c39974-c045-4eb1-a639-3004749f238a
📒 Files selected for processing (2)
scripts/create-testenv.mjstests/integration/create-testenv-seo.test.js
Summary
webmcp.htmlpage so it behaves like the standalone app and generator test pages/site/app.htmland/site/generator.htmlnavbar intact while still injecting the test environment indicator stylesWhy
The test environment publish flow was stripping headers from nested
/site/*pages that should retain the normal site navbar, while the standalonewebmcp.htmlpage was not getting the same no-navigation treatment as the other top-level test pages.Validation
pnpm run verify:localpassedSummary by CodeRabbit
Chores
Tests