You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
add engine-level Prism bootstrap script that sets window.Prism.manual = true before loading Prism core/autoloader
harden Prism init script with guarded retries so highlighting recovers when plugin/lang path init timing is delayed
apply the same hardening for both site-page Prism injection and API docs generation
add regression tests for local hashed Prism assets and CDN Prism assets in website builds
extend PowerShell API docs tests to assert bootstrap ordering and retry script output
Why
We hit recurring Prism inconsistencies across generated websites (especially with local/hashed asset names). This change makes the engine deterministic so themes/sites do not need per-site JavaScript workarounds.
Validation
dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "WebSiteBuilderPrismAssetInjectionTests|WebApiDocsGeneratorPowerShellTests"
Passed (12/12)
dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.
PR Review: Harden Prism bootstrap order and retry highlighting
Overall this is a well-motivated change that fixes a real timing issue. The approach — injecting window.Prism.manual=true before loading Prism core, then retrying highlighting — is correct and aligned with how Prism recommends manual mode be used.
A few findings worth discussing:
Bug: attempts incremented before early-exit guards (both files)
In both BuildPrismInitScript and BuildApiPrismInitScript, the counter is incremented unconditionally at the top of run(), but the function can return early before the setTimeout retry is scheduled:
varrun=function(){attempts++;// consumed herevarroot=...;varp=window.Prism;if(!p||!hasCode(root)){return;}// exits without scheduling retry
...
if(!hasTokens(root)&&attempts<maxAttempts){setTimeout(run,delayMs);}};
If window.Prism is undefined on the first call (e.g., the script tag failed to load or is async), all 8 attempts are exhausted immediately with no retry scheduled — because the retry setTimeout is only reachable after the early-exit guard. In the current setup scripts are synchronous so this guard should never trigger, but it's a latent correctness issue if loading strategy changes.
Suggestion: Move attempts++ inside the branch that actually schedules a retry, or restructure so early exits (e.g., Prism not loaded yet) schedule a retry rather than silently giving up.
Bug: Missing StringComparison.Ordinal in WebSiteBuilder.MarkdownAndTemplates.cs
BuildPrismInitScript in the site-builder file:
// WebSiteBuilder.MarkdownAndTemplates.cs line 163varsafePath=(languagesPath??string.Empty).Replace("'","\\'");
The equivalent in WebApiDocsGenerator.Html.cs (line 736) correctly passes StringComparison.Ordinal:
On non-invariant cultures this inconsistency could theoretically produce different results. Small fix, easy to miss.
Design concern: duplicated init script logic across two files
BuildPrismManualScript / BuildApiPrismManualScript are byte-for-byte identical. BuildPrismInitScript / BuildApiPrismInitScript are nearly identical — the only meaningful difference is the root element selector (.api-content fallback vs. document directly).
Any future changes to the retry logic, maxAttempts, delayMs, or escape handling need to be applied in two places. This PR already demonstrates the risk: the StringComparison.Ordinal discrepancy above is a direct result of that duplication.
Consider extracting shared logic to a PrismScriptBuilder helper class in PowerForge.Web that both call into, with a parameter for the root selector.
Potential XSS: languagesPath escaping is incomplete
This is a pre-existing issue (not introduced by this PR), but the retry logic makes it more visible:
Only single quotes are escaped. A path containing </script> would prematurely close the script tag:
targetPath='</script><script>alert(1)//'
If languagesPath is ever sourced from user-controllable input (e.g., front matter or query parameters), this is an XSS vector. Worth tracking even if the current call sites use configuration-only values.
Test observation: ordering assertions rely on first-occurrence index
In WebSiteBuilderPrismAssetInjectionTests and the updated WebApiDocsGeneratorPowerShellTests, ordering is validated using IndexOf on the full HTML string. This is fine for the current template structure, but will produce false negatives if the same string appears earlier in the document for an unrelated reason (e.g., in a page title or meta tag). Worth noting for future maintenance.
Minor: autoloader.languages_path is set on every retry
This assignment runs on every retry call. It is a benign no-op after the first successful set, but moving it outside the run loop (into the outer IIFE, executed once on init) would be marginally cleaner and avoids repeated property writes.
Tests look solid overall
Good coverage of both local-hashed and CDN asset paths.
Negative test updated correctly to assert the bootstrap script is absent when prism_mode=off.
Ordering assertions (bootstrap -> core -> autoloader) are the right way to validate this constraint.
CreateTempRoot / Cleanup pattern is consistent with other test classes in the suite.
Summary: The core change is correct and the tests verify the right things. The two actionable items before merging are:
Add StringComparison.Ordinal to the Replace call in BuildPrismInitScript (site-builder version) to match the API-docs version.
Consider whether the attempts++-before-early-exit pattern could silently swallow retries in edge cases.
The duplication and XSS issues are worth tracking but don't need to block this PR.
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
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.
Summary
window.Prism.manual = truebefore loading Prism core/autoloaderWhy
We hit recurring Prism inconsistencies across generated websites (especially with local/hashed asset names). This change makes the engine deterministic so themes/sites do not need per-site JavaScript workarounds.
Validation
dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "WebSiteBuilderPrismAssetInjectionTests|WebApiDocsGeneratorPowerShellTests"dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c ReleaseWebSiteSocialCardsTests.Build_DoesNotAutoGenerateCards_ForDocsPages_UnlessOverridden