Skip to content

feat(web): add PWA install support#576

Open
akrock wants to merge 25 commits into
mainfrom
codex/pwa-install-dx
Open

feat(web): add PWA install support#576
akrock wants to merge 25 commits into
mainfrom
codex/pwa-install-dx

Conversation

@akrock

@akrock akrock commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds first-class PWA install support to ForgeTrust.AppSurface.Web instead of introducing a new package.

  • Adds WebOptions.Pwa with validation for install-critical metadata, required icon sizes, safe app-root-relative paths, display modes, and explicit offline configuration.
  • Maps generated PWA endpoints from AppSurface Web: /manifest.webmanifest, development diagnostics under /_appsurface/pwa, status JSON, and a service worker only when offline fallback support is explicitly enabled.
  • Adds <appsurface:pwa-head /> for MVC/Razor layouts, including manifest, theme color, mobile app metadata, versioned icons, and service-worker metadata when configured.
  • Adds appsurface pwa verify --url <origin> with stable ASPWA2xx diagnostics for manifest/head/icon/origin/offline posture.
  • Adds examples/web-pwa-install as an executable proof app with explicit offline fallback configuration.
  • Updates Web/CLI/package chooser docs and unreleased release notes.

Test Coverage

New coverage added for:

  • PWA option validation, including disabled defaults, missing required fields, unsafe paths, icon requirements, invalid colors, display modes, and offline fallback requirements.
  • Manifest, diagnostics, PathBase, and service-worker endpoint behavior.
  • PWA head TagHelper output and disabled behavior.
  • CLI verifier pass/fail scenarios for secure origins, manifest discovery, required fields, icons, PathBase, diagnostics, and offline service worker posture.

Pre-Landing Review

No issues found. Ship review was rerun after merging origin/main.

Design Review

The feature includes a small MVC sample and diagnostic page. Full /qa browser testing covered the root sample, diagnostics page, and mobile offline fallback.

Eval Results

No prompt-related files changed, so evals were skipped.

Scope Drift

Scope Check: CLEAN

Intent: Make PWA install metadata one-line and diagnosable in the existing AppSurface Web package, with offline support explicit and opt-in.

Delivered: Web PWA options/endpoints/TagHelper, CLI verifier proof, docs, package chooser metadata, and executable example.

Plan Completion

The implementation matches the supplied plan at the package-contract level:

  • WebOptions.Pwa added in ForgeTrust.AppSurface.Web.
  • Manifest, diagnostics, and opt-in service-worker endpoints mapped from Web startup.
  • MVC/Razor head support added through <appsurface:pwa-head />.
  • Offline caching remains opt-in and static/fallback-only by default.
  • CLI proof added through appsurface pwa verify --url <origin>.
  • Docs and example added.
  • Package chooser metadata updated without adding a new package row.

Verification Results

  • dotnet test Cli/ForgeTrust.AppSurface.Cli.Tests/ForgeTrust.AppSurface.Cli.Tests.csproj --no-restore --filter FullyQualifiedName~Pwa passed: 11/11.
  • dotnet test Web/ForgeTrust.AppSurface.Web.Tests/ForgeTrust.AppSurface.Web.Tests.csproj --no-restore --filter FullyQualifiedName~Pwa passed: 25/25.
  • APP_SURFACE_WEB_PWA_PORT=5067 bash examples/web-pwa-install/verify.sh passed.
  • dotnet format --verify-no-changes passed.
  • dotnet run --project tools/ForgeTrust.AppSurface.PackageIndex/ForgeTrust.AppSurface.PackageIndex.csproj --no-restore -- verify passed.
  • git diff --check passed.
  • /qa browser pass found 0 issues and produced .gstack/qa-reports/qa-report-127-0-0-1-5067-2026-06-26.md.

TODOS

No TODO items completed in this PR.

Documentation

  • Updated Web/ForgeTrust.AppSurface.Web/README.md with a 3-minute PWA install path.
  • Added Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md.
  • Added examples/web-pwa-install/README.md.
  • Updated Cli/ForgeTrust.AppSurface.Cli/README.md.
  • Updated packages/package-index.yml, packages/README.md, CHANGELOG.md, and releases/unreleased.md.

Test plan

  • Focused CLI PWA tests pass.
  • Focused Web PWA tests pass.
  • Example app verifier passes against a live local origin.
  • Package chooser/readiness verification passes.
  • Formatting and whitespace checks pass.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 88ef0707-6592-418e-b359-9b7ef1fea5a0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds PWA configuration types, validation, runtime endpoints, Razor head metadata, a CLI verifier, an example app, and supporting tests and documentation across the web and CLI projects.

Changes

PWA install contract

Layer / File(s) Summary
PWA option surface
Web/ForgeTrust.AppSurface.Web/PwaOptions.cs, Web/ForgeTrust.AppSurface.Web/PwaOfflineOptions.cs, Web/ForgeTrust.AppSurface.Web/PwaIcon.cs, Web/ForgeTrust.AppSurface.Web/PwaDisplayMode.cs, Web/ForgeTrust.AppSurface.Web/PwaDiagnosticEndpointExposure.cs, Web/ForgeTrust.AppSurface.Web/WebOptions.cs, Web/ForgeTrust.AppSurface.Web.Tests/PublicEnumContractTests.cs
New PWA option types and enums define manifest metadata, offline settings, diagnostics exposure, and the WebOptions.Pwa configuration entry.
PWA validation
Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs, Web/ForgeTrust.AppSurface.Web.Tests/PwaOptionsTests.cs
The options validator emits diagnostics for required metadata, safe local paths, supported display modes, icon sizes, and offline settings, and the tests cover pass/fail cases and diagnostic codes.
Manifest and diagnostics endpoints
Web/ForgeTrust.AppSurface.Web/PwaEndpointMapper.cs, Web/ForgeTrust.AppSurface.Web/WebStartup.cs, Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs
The endpoint mapper serves the manifest, diagnostics HTML and status JSON, and the starter service worker, while WebStartup enables and registers those endpoints; the end-to-end tests cover mapping, HEAD responses, diagnostics exposure, and PathBase handling.
Razor head metadata
Web/ForgeTrust.AppSurface.Web/TagHelpers/AppSurfacePwaHeadTagHelper.cs, Web/ForgeTrust.AppSurface.Web.Tests/AppSurfacePwaHeadTagHelperTests.cs
The tag helper renders manifest, meta, icon, and offline-service-worker head tags when enabled, and its tests cover disabled output, versioned icon URLs, and unsafe icon fallbacks.
CLI verification command
Cli/ForgeTrust.AppSurface.Cli/PwaCommand.cs, Cli/ForgeTrust.AppSurface.Cli/AppSurfaceCliApp.cs, Cli/ForgeTrust.AppSurface.Cli.Tests/PwaVerifierTests.cs, Cli/ForgeTrust.AppSurface.Cli/README.md
The pwa verify command and verifier fetch app metadata, icons, diagnostics, and offline assets, emit text or JSON reports, and the CLI host registers the verifier HTTP client and logging filter; tests cover report output and verification outcomes.
Example app wiring
ForgeTrust.AppSurface.slnx, examples/web-pwa-install/*.cs, examples/web-pwa-install/Views/*, examples/web-pwa-install/README.md, examples/web-pwa-install/verify.sh, examples/web-pwa-install/WebPwaInstallExample.csproj, examples/web-pwa-install/packages.lock.json, examples/README.md
The example web app wires PWA options, offline fallback, Razor layout/head tags, a local verification flow, and solution/project entries into a runnable sample.
Docs and package metadata
CHANGELOG.md, Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md, Web/ForgeTrust.AppSurface.Web/README.md, packages/README.md, packages/package-index.yml, releases/unreleased.md
Repository docs, release notes, and package metadata describe the PWA install contract, package boundaries, and release surface.

Sequence Diagram(s)

See the hidden artifact above.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90 minutes

Poem

A bunny hopped through tags so bright,
With manifest moons and offline light.
It sniffed each icon, leaped with cheer,
Then whispered, “PWA is here!” 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: adding PWA install support to the web package.
Description check ✅ Passed The description accurately summarizes the PWA support, CLI verifier, example app, and docs added in this changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/pwa-install-dx

Comment @coderabbitai help to get the list of available commands.

Comment thread Cli/ForgeTrust.AppSurface.Cli/PwaCommand.cs Fixed
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Comment thread Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs Fixed
Comment thread Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs Fixed
Comment thread Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs Fixed
Comment thread Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs Fixed
@akrock akrock marked this pull request as ready for review June 26, 2026 09:08
Copilot AI review requested due to automatic review settings June 26, 2026 09:08

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.

Pull request overview

Adds first-class Progressive Web App (PWA) install support to ForgeTrust.AppSurface.Web (options + endpoint mapping + Razor head emission) and introduces a corresponding appsurface pwa verify CLI command plus an executable example and documentation to prove/diagnose install posture.

Changes:

  • Added WebOptions.Pwa configuration surface with startup validation and opt-in offline service worker mapping.
  • Mapped PWA endpoints (manifest, diagnostics HTML/JSON, optional service worker) and added <appsurface:pwa-head /> TagHelper for MVC/Razor layouts.
  • Added CLI verification (appsurface pwa verify --url <origin>) with stable diagnostics plus an example app and docs/release notes.

Reviewed changes

Copilot reviewed 36 out of 38 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Web/ForgeTrust.AppSurface.Web/WebStartup.cs Validates/enables PWA support during startup and maps PWA endpoints.
Web/ForgeTrust.AppSurface.Web/WebOptions.cs Adds WebOptions.Pwa configuration entry point.
Web/ForgeTrust.AppSurface.Web/TagHelpers/AppSurfacePwaHeadTagHelper.cs Implements <appsurface:pwa-head /> to emit manifest/theme/icon/service-worker metadata.
Web/ForgeTrust.AppSurface.Web/README.md Documents the “3-minute” PWA install setup path and key behaviors.
Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs Validates PWA options and provides diagnostic codes + safe-path helpers.
Web/ForgeTrust.AppSurface.Web/PwaOptions.cs Defines the main PWA options model.
Web/ForgeTrust.AppSurface.Web/PwaOfflineOptions.cs Defines opt-in offline fallback/service-worker configuration.
Web/ForgeTrust.AppSurface.Web/PwaIcon.cs Defines manifest/head icon entries.
Web/ForgeTrust.AppSurface.Web/PwaEndpointMapper.cs Maps manifest/diagnostics/service-worker endpoints and renders payloads.
Web/ForgeTrust.AppSurface.Web/PwaDisplayMode.cs Introduces manifest display-mode enum.
Web/ForgeTrust.AppSurface.Web/PwaDiagnosticEndpointExposure.cs Introduces diagnostics exposure enum.
Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md Adds full PWA install/offline/diagnostics/CLI documentation.
Web/ForgeTrust.AppSurface.Web.Tests/PwaOptionsTests.cs Adds unit coverage for PWA option defaults + validation diagnostics.
Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs Adds integration coverage for mapped endpoints and PathBase behavior.
Web/ForgeTrust.AppSurface.Web.Tests/PublicEnumContractTests.cs Adds enum numeric-value stability tests for new public enums.
Web/ForgeTrust.AppSurface.Web.Tests/AppSurfacePwaHeadTagHelperTests.cs Adds TagHelper output/disabled-path tests, including PathBase behavior.
releases/unreleased.md Updates unreleased notes to include PWA feature summary.
packages/README.md Updates package chooser text to mention opt-in PWA support in Web package.
packages/package-index.yml Updates package index metadata to include PWA install support in Web.
ForgeTrust.AppSurface.slnx Adds the new examples/web-pwa-install project to the solution.
examples/web-pwa-install/wwwroot/icons/app-512.svg Adds example PWA icon asset (512).
examples/web-pwa-install/wwwroot/icons/app-192.svg Adds example PWA icon asset (192).
examples/web-pwa-install/WebPwaInstallExample.csproj Adds runnable example project referencing ForgeTrust.AppSurface.Web.
examples/web-pwa-install/Views/Shared/_Layout.cshtml Uses <appsurface:pwa-head /> in the example layout.
examples/web-pwa-install/Views/Home/Index.cshtml Adds example landing page text and verifier command hint.
examples/web-pwa-install/Views/_ViewStart.cshtml Sets example layout.
examples/web-pwa-install/Views/_ViewImports.cshtml Imports TagHelpers including ForgeTrust.AppSurface.Web.
examples/web-pwa-install/verify.sh Adds a script to run the example and execute the CLI verifier.
examples/web-pwa-install/README.md Documents what the example proves and how to run it.
examples/web-pwa-install/Program.cs Configures PWA options + offline fallback endpoint in the example app.
examples/web-pwa-install/packages.lock.json Adds locked restore entries for the new example project.
examples/web-pwa-install/Controllers/HomeController.cs Adds MVC controller to serve the example page.
examples/README.md Adds the new PWA install proof example to the examples list.
Cli/ForgeTrust.AppSurface.Cli/README.md Documents the new appsurface pwa verify command.
Cli/ForgeTrust.AppSurface.Cli/PwaCommand.cs Implements pwa verify command and verifier logic/diagnostics.
Cli/ForgeTrust.AppSurface.Cli/AppSurfaceCliApp.cs Registers HttpClient/verifier DI services and adjusts HttpClient logging.
Cli/ForgeTrust.AppSurface.Cli.Tests/PwaVerifierTests.cs Adds extensive verifier pass/fail coverage and adapter tests.
CHANGELOG.md Adds a changelog entry describing the new PWA install support.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs
Comment thread Web/ForgeTrust.AppSurface.Web/PwaDisplayMode.cs
Comment thread Web/ForgeTrust.AppSurface.Web/PwaDiagnosticEndpointExposure.cs

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/README.md (1)

59-60: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add the required icon MIME type.

PwaOptionsValidator requires PwaOptions.Icons[i].Type, so this sample will fail startup when copied as-is. Add a concrete type, for example image/png, to both icon entries. Based on the upstream validator contract in Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs.

🛠️ Suggested fix
-        options.Pwa.Icons.Add(new PwaIcon { Source = "/icons/app-192.png", Sizes = "192x192" });
-        options.Pwa.Icons.Add(new PwaIcon { Source = "/icons/app-512.png", Sizes = "512x512" });
+        options.Pwa.Icons.Add(new PwaIcon { Source = "/icons/app-192.png", Sizes = "192x192", Type = "image/png" });
+        options.Pwa.Icons.Add(new PwaIcon { Source = "/icons/app-512.png", Sizes = "512x512", Type = "image/png" });
🤖 Prompt for 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.

In `@packages/README.md` around lines 59 - 60, The README sample for the PWA
package is missing the required icon MIME type, which will cause validation to
fail when copied. Update the `ForgeTrust.AppSurface.Web` package example so both
icon entries include a concrete `Type` value, matching the expectations enforced
by `PwaOptionsValidator` and `PwaOptions.Icons`.
🤖 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 `@Cli/ForgeTrust.AppSurface.Cli/AppSurfaceCliApp.cs`:
- Around line 77-79: The PWA verification HttpClient setup in AppSurfaceCliApp
currently allows redirects, which can let PwaVerifier treat a redirected
cross-origin response as valid. Update the AddHttpClient configuration for
PwaVerificationHttpClient to disable automatic redirects, or add a final-URI
validation step in PwaVerifier so responses that leave the app origin/base path
are rejected. Use the existing PwaVerificationHttpClient and PwaVerifier
registration points to apply the fix.

In `@examples/web-pwa-install/Controllers/HomeController.cs`:
- Around line 5-12: Document the public API surface added by HomeController and
its Index action by adding XML documentation to the HomeController class and
Index() method, describing the root route, the fact that it renders the default
view, and the intended sample/expected usage; make sure the docs cover API
shape, behavior, defaults, constraints, and usage so the new controller is fully
documented per the codebase guidelines.

In `@examples/web-pwa-install/Program.cs`:
- Around line 34-53: The new internal module surface in WebPwaInstallModule is
missing XML documentation, so add docs to the class and each member
(IncludeAsApplicationPart, ConfigureServices, RegisterDependentModules,
ConfigureHostBeforeServices, ConfigureHostAfterServices) describing its role and
lifecycle ordering; also document why IncludeAsApplicationPart must remain
enabled for this sample so the analyzer/doc warning is cleared.

In `@examples/web-pwa-install/Views/Home/Index.cshtml`:
- Around line 3-14: The rendered diagnostics links and verify command are
ignoring Request.PathBase, so the page can generate incorrect URLs when the app
is hosted under a sub-path. Update the Home/Index.cshtml logic around
verifierOrigin and the /_appsurface/pwa link to build both values with
Context.Request.PathBase preserved, using the existing verifierOrigin variable
so the displayed appsurface pwa verify --url target matches the deployment root.

In `@Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md`:
- Around line 20-21: The PWA install sample is missing the required icon MIME
type, so copying it will fail validation in PwaOptionsValidator. Update both
PwaIcon entries in the PwaOptions setup to include a concrete Type value, such
as image/png, and keep the change consistent with the validator contract in
PwaOptionsValidator and the sample shown in Pwa-install.md.

In `@Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs`:
- Line 23: The StartUrl validation in PwaOptionsValidator is too strict because
it reuses RequireLocalPath, which rejects valid query strings used by PWA
start_url values. Update the StartUrl-specific validation path to allow a query
component while still rejecting unsafe forms like double slashes, scheme-like
strings, backslashes, fragments, traversal, and control characters; keep the
existing stricter checks for Scope, ManifestPath, and DiagnosticsPath unchanged.

In `@Web/ForgeTrust.AppSurface.Web/README.md`:
- Around line 59-60: The PWA sample is missing the required icon MIME type, so
copying it will fail validation in PwaOptionsValidator. Update the two
PwaOptions.Icons.Add entries in the README example to set PwaIcon.Type for each
icon, using a concrete value like image/png, and keep the existing Source and
Sizes fields unchanged.

---

Outside diff comments:
In `@packages/README.md`:
- Around line 59-60: The README sample for the PWA package is missing the
required icon MIME type, which will cause validation to fail when copied. Update
the `ForgeTrust.AppSurface.Web` package example so both icon entries include a
concrete `Type` value, matching the expectations enforced by
`PwaOptionsValidator` and `PwaOptions.Icons`.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e52ced2a-4c8c-4e7d-800a-50473e108d3c

📥 Commits

Reviewing files that changed from the base of the PR and between 7c0a656 and 0eb1cde.

⛔ Files ignored due to path filters (2)
  • examples/web-pwa-install/wwwroot/icons/app-192.svg is excluded by !**/*.svg
  • examples/web-pwa-install/wwwroot/icons/app-512.svg is excluded by !**/*.svg
📒 Files selected for processing (36)
  • CHANGELOG.md
  • Cli/ForgeTrust.AppSurface.Cli.Tests/PwaVerifierTests.cs
  • Cli/ForgeTrust.AppSurface.Cli/AppSurfaceCliApp.cs
  • Cli/ForgeTrust.AppSurface.Cli/PwaCommand.cs
  • Cli/ForgeTrust.AppSurface.Cli/README.md
  • ForgeTrust.AppSurface.slnx
  • Web/ForgeTrust.AppSurface.Web.Tests/AppSurfacePwaHeadTagHelperTests.cs
  • Web/ForgeTrust.AppSurface.Web.Tests/PublicEnumContractTests.cs
  • Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs
  • Web/ForgeTrust.AppSurface.Web.Tests/PwaOptionsTests.cs
  • Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md
  • Web/ForgeTrust.AppSurface.Web/PwaDiagnosticEndpointExposure.cs
  • Web/ForgeTrust.AppSurface.Web/PwaDisplayMode.cs
  • Web/ForgeTrust.AppSurface.Web/PwaEndpointMapper.cs
  • Web/ForgeTrust.AppSurface.Web/PwaIcon.cs
  • Web/ForgeTrust.AppSurface.Web/PwaOfflineOptions.cs
  • Web/ForgeTrust.AppSurface.Web/PwaOptions.cs
  • Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs
  • Web/ForgeTrust.AppSurface.Web/README.md
  • Web/ForgeTrust.AppSurface.Web/TagHelpers/AppSurfacePwaHeadTagHelper.cs
  • Web/ForgeTrust.AppSurface.Web/WebOptions.cs
  • Web/ForgeTrust.AppSurface.Web/WebStartup.cs
  • examples/README.md
  • examples/web-pwa-install/Controllers/HomeController.cs
  • examples/web-pwa-install/Program.cs
  • examples/web-pwa-install/README.md
  • examples/web-pwa-install/Views/Home/Index.cshtml
  • examples/web-pwa-install/Views/Shared/_Layout.cshtml
  • examples/web-pwa-install/Views/_ViewImports.cshtml
  • examples/web-pwa-install/Views/_ViewStart.cshtml
  • examples/web-pwa-install/WebPwaInstallExample.csproj
  • examples/web-pwa-install/packages.lock.json
  • examples/web-pwa-install/verify.sh
  • packages/README.md
  • packages/package-index.yml
  • releases/unreleased.md

Comment thread Cli/ForgeTrust.AppSurface.Cli/AppSurfaceCliApp.cs Outdated
Comment thread examples/web-pwa-install/Controllers/HomeController.cs
Comment thread examples/web-pwa-install/Program.cs
Comment thread examples/web-pwa-install/Views/Home/Index.cshtml Outdated
Comment thread Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md Outdated
Comment thread Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs Outdated
Comment thread Web/ForgeTrust.AppSurface.Web/README.md 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.

Pull request overview

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

Comment thread Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs
Comment thread Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs
Comment thread Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md Outdated
Comment thread Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md Outdated
@akrock akrock requested a review from Copilot June 27, 2026 07:58
@akrock

akrock commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

Pull request overview

Copilot reviewed 36 out of 38 changed files in this pull request and generated 1 comment.

Comment thread Cli/ForgeTrust.AppSurface.Cli/AppSurfaceCliApp.cs Outdated

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs (1)

245-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse one valid-options builder across the PWA test suites.

ConfigureValidPwa hand-copies part of PwaOptionsTests.CreateValidOptions(), so any new required field or default can make the validator and endpoint suites diverge silently. A single shared test fixture/helper would keep both suites pinned to the same baseline config.

🤖 Prompt for 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.

In `@Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs` around lines 245 -
254, `ConfigureValidPwa` is duplicating the baseline setup from
`PwaOptionsTests.CreateValidOptions`, which can drift from the validator tests.
Refactor the PWA test setup to use a single shared valid-options builder/helper
across both suites, and have `ConfigureValidPwa` populate from that shared
source instead of hand-copying fields so `PwaEndpointTests` and
`PwaOptionsTests` stay aligned.
Cli/ForgeTrust.AppSurface.Cli.Tests/PwaVerifierTests.cs (1)

762-777: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Tighten the fake client’s URL matching.

StringComparer.OrdinalIgnoreCase plus TrimEnd('/') erases path-case and trailing-slash distinctions that this verifier explicitly cares about, so a bad URI resolution can still pass in tests. Use exact URI strings here, or normalize through Uri on both insert and lookup instead of lossy string surgery.

Proposed fix
-        private readonly Dictionary<string, PwaHttpResponse> _responses = new(StringComparer.OrdinalIgnoreCase);
+        private readonly Dictionary<string, PwaHttpResponse> _responses = new(StringComparer.Ordinal);
...
-            _responses[url.TrimEnd('/')] = new PwaHttpResponse(statusCode, contentType, body);
+            _responses[url] = new PwaHttpResponse(statusCode, contentType, body);
...
-            return Task.FromResult(_responses.TryGetValue(uri.ToString().TrimEnd('/'), out var response)
+            return Task.FromResult(_responses.TryGetValue(uri.ToString(), out var response)
                 ? response
                 : new PwaHttpResponse(HttpStatusCode.NotFound, "text/plain", string.Empty));
🤖 Prompt for 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.

In `@Cli/ForgeTrust.AppSurface.Cli.Tests/PwaVerifierTests.cs` around lines 762 -
777, The fake client in PwaVerifierTests currently uses case-insensitive string
keys and TrimEnd('/') in Add and GetAsync, which can hide URI resolution bugs.
Update the _responses lookup in the fake Pwa HTTP client to use exact URI string
matching, or normalize both stored and requested URLs consistently via Uri
rather than lossy string trimming; keep the behavior localized to Add and
GetAsync so the verifier still catches path-case and trailing-slash mismatches.
🤖 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 `@Cli/ForgeTrust.AppSurface.Cli/README.md`:
- Line 79: Update the README summary for the verifier to match the actual
contract: in the CLI docs section, expand the accepted origins in the relevant
description near the verifier behavior to include loopback addresses like
127.0.0.1 and ::1, and document that offline checks also validate
offlineFallbackPath reachability in addition to service-worker posture. Keep the
wording aligned with the existing verifier/ASPWA2xx diagnostics description so
users can tell which URLs are valid and why offline validation fails.

In `@Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs`:
- Around line 207-244: The scope/start-url validation logic is duplicated
between PwaOptionsValidator.RequireStartUrlWithinScope/IsPathWithinScope and the
CLI verifier in PwaCommand, so update this to use one shared implementation
instead of two copies. Extract the common path/scope checks into a shared helper
or utility and have both the runtime validator and appsurface pwa verify call
that same code, keeping the contract consistent when the rule changes.

---

Outside diff comments:
In `@Cli/ForgeTrust.AppSurface.Cli.Tests/PwaVerifierTests.cs`:
- Around line 762-777: The fake client in PwaVerifierTests currently uses
case-insensitive string keys and TrimEnd('/') in Add and GetAsync, which can
hide URI resolution bugs. Update the _responses lookup in the fake Pwa HTTP
client to use exact URI string matching, or normalize both stored and requested
URLs consistently via Uri rather than lossy string trimming; keep the behavior
localized to Add and GetAsync so the verifier still catches path-case and
trailing-slash mismatches.

In `@Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs`:
- Around line 245-254: `ConfigureValidPwa` is duplicating the baseline setup
from `PwaOptionsTests.CreateValidOptions`, which can drift from the validator
tests. Refactor the PWA test setup to use a single shared valid-options
builder/helper across both suites, and have `ConfigureValidPwa` populate from
that shared source instead of hand-copying fields so `PwaEndpointTests` and
`PwaOptionsTests` stay aligned.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e499bbf5-86f9-4371-8150-3b9bcb8c51d5

📥 Commits

Reviewing files that changed from the base of the PR and between 0eb1cde and 63cbc2f.

📒 Files selected for processing (13)
  • Cli/ForgeTrust.AppSurface.Cli.Tests/PwaVerifierTests.cs
  • Cli/ForgeTrust.AppSurface.Cli/PwaCommand.cs
  • Cli/ForgeTrust.AppSurface.Cli/README.md
  • Web/ForgeTrust.AppSurface.Web.Tests/PwaEndpointTests.cs
  • Web/ForgeTrust.AppSurface.Web.Tests/PwaOptionsTests.cs
  • Web/ForgeTrust.AppSurface.Web/Docs/pwa-install.md
  • Web/ForgeTrust.AppSurface.Web/PwaDiagnosticEndpointExposure.cs
  • Web/ForgeTrust.AppSurface.Web/PwaDisplayMode.cs
  • Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs
  • Web/ForgeTrust.AppSurface.Web/README.md
  • examples/web-pwa-install/Controllers/HomeController.cs
  • examples/web-pwa-install/Program.cs
  • examples/web-pwa-install/Views/Home/Index.cshtml

Comment thread Cli/ForgeTrust.AppSurface.Cli/README.md Outdated
Comment thread Web/ForgeTrust.AppSurface.Web/PwaOptionsValidator.cs Outdated
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