Skip to content

Unify navigation and 404 behavior with pipeline audit guardrails#75

Merged
PrzemyslawKlys merged 5 commits intomasterfrom
fix/nav-404-audit-unification
Feb 5, 2026
Merged

Unify navigation and 404 behavior with pipeline audit guardrails#75
PrzemyslawKlys merged 5 commits intomasterfrom
fix/nav-404-audit-unification

Conversation

@PrzemyslawKlys
Copy link
Member

Summary

  • restore missing Home link in Blazor nav + footer product links
  • move 404 page to markdown content (Website/content/pages/404.md) and remove standalone static 404
  • enable dotnet-publish.clean in pipeline to prevent stale /playground artifacts from polluting output
  • tighten audit nav checks:
    • require Home link (
      avRequiredLinks: /)
    • ignore nav only for known shell files (playground/index.html, playground/404.html) and sitemap
    • keep rendered audit enabled without playwright auto-install

Validation

  • Website/pipeline.json completed successfully
  • audit result: pages 807, broken links 0, missing assets 0, nav mismatches 0, warnings 0

Copilot AI review requested due to automatic review settings February 5, 2026 19:57
@claude
Copy link

claude bot commented Feb 5, 2026

Claude finished @PrzemyslawKlys's task —— View job


PR Review: Unify navigation and 404 behavior with pipeline audit guardrails

Review Tasks:

  • Analyze MainLayout.razor changes (navigation updates)
  • Review 404 page migration from static HTML to markdown
  • Examine build script changes and validation logic
  • Review pipeline.json configuration updates
  • Assess code quality and best practices
  • Check for potential bugs and security issues
  • Evaluate performance implications
  • Review test coverage considerations
  • Address GitHub Pages compatibility concern raised in reviews

🔴 Critical Issue: GitHub Pages 404 Compatibility

Problem: The markdown 404 approach (slug: 404 with TrailingSlash: "always") generates /404/index.html instead of the required /404.html for GitHub Pages.

Impact: Broken 404 handling on production - users will see GitHub's default 404 instead of the branded experience.

Evidence:

  • site.json:7 has "TrailingSlash": "always"
  • Markdown pages in the pages collection get trailing slash treatment
  • GitHub Pages specifically requires /404.html at site root

Recommendation: The build validation in build.ps1:99-103 is excellent and would catch this issue, suggesting the build system may have special handling for 404 pages that generates both /404/index.html AND /404.html. Need to verify the actual build output.


✅ Code Quality Assessment

MainLayout.razor (Lines 180, 192)

Excellent implementation:

  • Consistency: Home link added to both primary nav and footer Product section
  • Proper typing: Uses correct NavLink record with appropriate parameters
  • Clean code: No duplication, follows existing patterns
  • Security: No XSS risks, proper href handling

Build Script Enhancement (Lines 93-103)

Well-designed validation:

  • Fail-fast: Prevents broken deployments with clear error messages
  • Defensive programming: Validates critical site outputs before deployment
  • Good error messaging: "expected '$notFoundPage' for GitHub Pages 404 handling"

Pipeline Configuration

Smart improvements:

"clean": true              // ✅ Prevents stale playground artifacts  
"ignoreNav": "sitemap/**,playground/index.html,playground/404.html"  // ✅ More precise exclusions
"navRequiredLinks": "/"    // ✅ Enforces Home link presence in audits
"renderedEnsureInstalled": false  // ✅ Disables Playwright auto-install

🟡 Minor Concerns

Performance

  • Negligible impact: Adding one navigation link has minimal overhead
  • Good: Clean builds prevent artifact bloat

Security

  • No new attack vectors: Changes are purely navigational/structural
  • Safe: No dynamic content, XSS risks, or credential handling

Test Coverage

  • Gap: No automated tests verify navigation links exist
  • Mitigation: Pipeline audit with navRequiredLinks: "/" provides runtime validation
  • Suggestion: Consider adding unit tests for navigation structure

📋 Detailed Technical Review

MainLayout.razor Analysis

// Line 180 - Primary nav addition
new NavLink("/", "Home"),

// Line 192 - Footer nav addition  
new NavLink("/", "Home"),
  • Type safety: Proper NavLink record usage
  • Consistency: Same pattern as existing links
  • Accessibility: Will inherit proper ARIA attributes from RenderNavLink

404.md Structure

slug: 404                 # ⚠️ Combined with TrailingSlash creates /404/
collection: pages         # ✅ Correct collection
layout: home             # ✅ Consistent layout
meta.breadcrumbs: false  # ✅ Appropriate for error pages

Build Validation Logic

$notFoundPage = Join-Path $SiteRoot '404.html'
if (-not (Test-Path -LiteralPath $notFoundPage -PathType Leaf)) {
    throw "Build validation failed: expected '$notFoundPage' for GitHub Pages 404 handling."
}

Excellent defensive programming - this validation would catch the 404 routing issue.


🎯 Recommendations

  1. Verify 404 build output: Confirm whether the build system creates both /404/index.html and /404.html
  2. Test GitHub Pages deployment: Deploy to staging to verify 404 handling works
  3. Consider navigation tests: Add unit tests to verify required navigation links exist
  4. Document 404 approach: Add comments explaining the markdown-to-404.html generation

✅ Summary

Strong PR overall with excellent code quality, defensive validation, and smart pipeline improvements. The only concern is the potential GitHub Pages 404 compatibility issue flagged by the automated review, though the build validation suggests this may be handled correctly by the build system.

Approval recommendation: Verify the 404.html generation works as expected, then approve.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a601090de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR unifies navigation behavior across the static site and Blazor playground application while improving build pipeline hygiene and audit controls.

Changes:

  • Adds missing "Home" link to Blazor app navigation (header and footer)
  • Migrates 404 page from standalone static HTML to markdown content using the site's templating system
  • Enables clean builds for the playground to prevent stale artifacts
  • Tightens audit navigation checks by requiring Home links and narrowing the ignore list to specific shell files

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
Website/static/404.html Removed standalone static 404 page (196 lines deleted)
Website/content/pages/404.md Added new markdown-based 404 page using the home layout for consistency
Website/pipeline.json Added clean flag to dotnet-publish, tightened ignoreNav to specific files, added navRequiredLinks, disabled Playwright auto-install
CodeGlyphX.Website/Layout/MainLayout.razor Added Home link ("/") to both primary navigation and footer Product section

@codecov
Copy link

codecov bot commented Feb 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.75%. Comparing base (819dc3e) to head (3c30dee).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #75   +/-   ##
=======================================
  Coverage   59.74%   59.75%           
=======================================
  Files         448      448           
  Lines       57814    57814           
  Branches    16563    16563           
=======================================
+ Hits        34543    34545    +2     
+ Misses      17566    17565    -1     
+ Partials     5705     5704    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@PrzemyslawKlys
Copy link
Member Author

Applied the 404-related bot feedback in commit 8db8d70.

  • Kept markdown-driven 404 content (for nav/layout consistency)
  • Added build-time guard in Website/build.ps1 that fails if Website/_site/404.html is missing

This ensures GitHub Pages compatibility while still using unified markdown/layout generation.

@PrzemyslawKlys
Copy link
Member Author

Follow-up cleanup in commit 3c30dee:

  • removed markdown-fix from Website/pipeline.json

Reason: CI website build checks out PSPublishModule@v2-speedgonzales, and that CLI revision does not support the markdown-fix task yet (Unknown task). This keeps the PR green on current CI while preserving the navigation/404/audit fixes.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 5, 2026

@PrzemyslawKlys PrzemyslawKlys merged commit c718ba2 into master Feb 5, 2026
9 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the fix/nav-404-audit-unification branch February 5, 2026 21:53
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