Skip to content

TEMPORARY — W0 red-path proof (do not merge) - #4

Closed
The-Running-Dev wants to merge 7 commits into
mainfrom
codex/w0-red-path-proof
Closed

TEMPORARY — W0 red-path proof (do not merge)#4
The-Running-Dev wants to merge 7 commits into
mainfrom
codex/w0-red-path-proof

Conversation

@The-Running-Dev

Copy link
Copy Markdown
Owner

Temporary verification PR for W0's Definition of Done, which requires evidence that a
deliberate failure turns each required check red. Three isolated, deliberate failures:

  • a failing test → engine
  • a broken relative link in agent.mdDocumentation links and terminology
  • a broken site-absolute route in docs/docs/engine/TODO.mdVerify Documentation Build

Each was checked locally to confirm it trips exactly one check before pushing. Run URLs
will be recorded in the W0 plan once these register here, then this PR is closed without
merging and the branch deleted.

Do not merge.

The-Running-Dev and others added 6 commits July 27, 2026 22:04
Both surfaced while verifying the plan's findings against the published image
and the live Pages configuration. Neither changes scope, so they are folded
into the phases they affect and recorded as A1/A2 rather than raised as
findings needing approval.

A1 — the generated homepage changes the sidebar (Phase 5)
ConvertTo-DocumentationHomepage.ps1 writes front matter, not just body text:
a title and sidebar_position: 1. The sidebar is autogenerated from docs/docs/,
which until now held only engine/, so a new top-level entry appears above that
category. Confirmed this does not disturb ordering inside engine/ — those
positions apply within the category and encode the stated reading order, in
which 04-core precedes 03-story-graph-kind. Added the front matter, both
consequences, and two verification items so it is confirmed rather than
discovered.

A2 — the bare root will 404 after deployment (Phase 6)
routeBasePath stays 'docs' and Phase 5 deliberately does not change it, so the
site publishes at /docs/ while the bare hostname keeps returning 404. Today
both 404 because nothing is deployed; after the first deploy they diverge.
Fine for a docs subdomain, but it stops being hypothetical once the domain is
shareable, so it is now recorded as an expected end state with the two exits
noted (routeBasePath '/' or a root page under docs/src/pages/), both outside
W0. Added an acceptance item so the 404 is reported as expected rather than as
a deployment failure.

Verified while reviewing, and unchanged by this commit: F1 reproduced — the
dispatcher -WhatIf prints one line and enumerates nothing, while the direct
pwsh -File form printed exactly the six creates and five skips the plan
predicts, writing nothing. F2 confirmed at 15 occurrences across 10 unique
targets. F3 confirmed — docs-deploy.yml triggers only on push to main and
workflow_dispatch, so requiring it on pull requests would leave every one
pending. External state confirmed via the Pages API: build_type workflow,
cname game-engine.subzerodev.com, domain verified, https_enforced false, no
branch protection, both URLs 404 pre-deploy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three of Codex's findings are correct, each reproduced before being
accepted, and each corrects a defect in plans/02 rather than in its own plan.
Applied now rather than deferred: they are factual corrections with one right
answer, and a round-trip would only delay execution.

F1 - the documented dry run never reached the installer
Ran both forms. The Invoke-SetupDocs dispatcher with -WhatIf prints a single
line -- "Performing the operation 'Invoke discovered PowerShell Script'" --
and enumerates nothing, so it is not a preview at all. Replaced with the
direct pwsh -File /PSModule/Scripts/setup-docs.ps1 form, which printed exactly
six creates and five skips and wrote nothing to the worktree. Both the
expected file lists are now in the plan, and the real install still uses the
public entry point, since only -WhatIf degrades.

F1b - the expected skip list was four files, not five
docs/.dockerignore exists here and is skipped too. Corrected in both the
decision table and the dry-run step.

F2 - the README link count was wrong
15 occurrences across 10 unique targets, not 14. The old figure came from
counting distinct link texts: [MVP] and [MVP.md] point at the same file and
were counted twice while others were missed. Replaced the bare count with the
full relative -> absolute target map, and the instruction is now "replace by
target, not by count", which is the failure mode that produced the error.

F3 - requiring the deploy check would have hung every pull request
Verified against the installed template: Build and Deploy Documentation lives
in docs-deploy.yml, which triggers only on push to main and workflow_dispatch.
It can never report on a pull request, so requiring it would leave every PR
pending indefinitely. Required checks are now the three that actually run --
CI / engine, Documentation links and terminology, Verify Documentation Build
-- with deploy called out as post-merge verification. Fixed in the reference
section, the install step, the acceptance list, and TODO's W0 done-criteria so
plan and unit cannot drift.

plans/04 now records the disposition: findings approved with the evidence for
each, its Phase 1 marked done and retained as the record, and the peer-review
order narrowed to what actually remains (Phases 2-6).

Verified: 0 unresolved cross-document section references; every internal
markdown link resolves; docs production build passes with onBrokenLinks set to
throw; git diff --check clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes the file-level work Phase 4 (already installed) left outstanding:
author ci.yml, align Node, and integrate the published documentation. Every
step verified locally before commit, not assumed from the plan.

Phase 5 first, ahead of the plan's stated order
Converting the README had to happen before regenerating the homepage, or the
generator bakes the broken links into docs/docs/index.md again. Confirmed
this was necessary: before the fix, the installed gate failed with exactly
the 15 errors F2 predicted, and the Docusaurus production build failed on the
same links via MDX compilation. This is the ordering defect flagged during
the prior review, worth recording as resolved by doing Phase 5's README work
before regenerating from Phase 4's install.

- README.md: all 15 relative-link occurrences (10 unique targets) replaced
  with absolute URLs per the approved target map in plans/02 -- spec links to
  https://game-engine.subzerodev.com/docs/engine/..., docs.ps1 and
  src/engine/ to the code host. Zero relative links remain.
- docs/docusaurus.config.ts: url set to the real published origin,
  https://game-engine.subzerodev.com; baseUrl was already '/'.
- docs/docs/index.md: regenerated via
  build/ConvertTo-DocumentationHomepage.ps1 -ReadmePath ./README.md
  -Title 'Game Engine' -SiteUrl 'https://game-engine.subzerodev.com/',
  matching the parameters used at install time. Confirmed the generator
  rewrites only links matching SiteUrl to root-relative paths; the two
  GitHub links were left absolute, as intended.
- Verified: pwsh build/Test-Documentation.ps1 now passes across 16 Markdown
  files (was 15 errors); the production Invoke-DocsBuild succeeds with no
  broken links.

Phase 3
- .github/workflows/ci.yml authored to the approved pattern: one `engine`
  job, checkout@v7, setup-node@v7 on Node 24 with the subdirectory lockfile
  cache, the concurrency group keyed on workflow + head repository + branch
  name, separate typecheck/lint/test steps, contents: read only, no docs job.
  Quoted "on": to match the installed workflows' convention and avoid YAML
  1.1's boolean coercion of the bare key. Parsed with Ruby's YAML loader in
  the absence of a local Actions linter.
- src/engine/package.json: added engines.node ">=24"; @types/node ^22.0.0 ->
  ^24.0.0.
- Regenerated the lockfile with a plain npm install; reviewed the diff --
  limited to @types/node, its transitive undici-types bump, and the new
  engines block. No unrelated dependency moved.
- Verified from a wiped node_modules: npm ci, typecheck, lint, and all 15
  tests green.

A --user bug found while reproducing the CI docs build locally
plans/02's documented "reproduce the CI build" command passed --user
"$(id -u):$(id -g)" to Invoke-DocsBuild, copied from the Invoke-SetupDocs
example next to it. Running it failed: Invoke-DocsBuild overlays docs/ onto
/template inside the image before building, and /template is root-owned
(755) in the published image, so a non-root user cannot write into it --
Copy-Item: Access to the path '/template/Dockerfile' is denied. Confirmed by
running the identical command without --user, which then succeeded, and by
checking the installed docs-ci.yml: its container: job carries no user:
override, so CI itself runs this as root and was never affected -- this was
a local-reproduction-only defect. Corrected both occurrences in
plans/02-w0-ci-workflow.md and the one in plans/04, with the reasoning
recorded so the flag is not reintroduced by analogy with Invoke-SetupDocs,
which needs it for the opposite reason (it writes only into the mounted
/work).

Not done here, deliberately: nothing pushed, no PR opened, no branch
protection or required status checks configured, no push to main to trigger
docs-deploy.yml. Those are shared-state / repo-settings actions and W0's own
Definition of Done requires the red-path proof (a deliberate failure per
check, with run URLs recorded) before it can be marked complete -- none of
that can be produced from local-only work.

Verified end to end: engine npm ci / typecheck / lint / test green (15
tests); documentation gate green across 16 files; production docs build
green with zero broken links; ci.yml parses as valid YAML; git diff --check
clean; only the Phase 1 allowed file surface changed, confirmed against
951efeb (the install commit).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- src/engine/src/core/__redpath.test.ts: a failing test, for the engine check.
- agent.md: a broken relative link, for the docs gate (Documentation links
  and terminology) -- outside the Docusaurus content tree, so it does not
  also trip the production build.
- docs/docs/engine/TODO.md: a broken site-absolute route (/docs/...), for the
  production build (Verify Documentation Build) -- the gate's own source
  skips absolute/site-rooted link targets entirely, so this does not also
  trip the gate.

Verified locally before push: the gate reports exactly one error (agent.md
only); the production build fails at Docusaurus's own executeBrokenLinksCheck;
the engine test suite fails exactly the one new test, the other 15 pass.

This commit is temporary and will be reverted before this branch is deleted.
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add engine CI + docs gate/build workflows and W0 red-path proof failures

✨ Enhancement ⚙️ Configuration changes 📝 Documentation 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Add GitHub Actions engine CI plus docs gate/build and Pages deploy workflows.
• Introduce documentation validation for links/anchors/terminology and generated-homepage drift.
• Deliberately break one test and two doc links to prove required checks go red (temporary).
Diagram

graph TD
  A(["PR / Push"]) --> B(["CI / engine (workflow)"]) --> C["Node 24 + npm scripts"] --> D["typecheck/lint/test"]
  A --> E(["Docs CI (workflow)"]) --> F["Test-Documentation.ps1 (gate)"] --> G["DocumentationRules.psd1"]
  E --> H["Docs build (container)"] --> I["upload-pages-artifact"]
  J(["main push"]) --> K(["Docs Deploy (workflow)"]) --> L["docs-build.ps1 (container)"] --> M["deploy-pages"]
  N["README.md"] --> O["ConvertTo-DocumentationHomepage.ps1"] --> P["docs/docs/index.md"]
  subgraph Legend
    direction LR
    _evt(["Event"]) ~~~ _wf(["Workflow"]) ~~~ _scr["Script/File"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use workflow inputs to induce failures (no committed broken files)
  • ➕ Avoids committing intentional breakages (failing test, broken links) to the branch
  • ➕ Keeps the diff purely “real changes” while still demonstrating red-path behavior
  • ➕ Reduces risk of accidentally merging deliberate failures
  • ➖ Requires designing/maintaining failure toggles in workflows/scripts
  • ➖ Less representative of real-world failures (e.g., link rot) than actual broken content
2. Demonstrate red-path with draft PRs / separate proof branches per check
  • ➕ Isolates each deliberate failure so reviewers can validate one red check at a time
  • ➕ Reduces noise in a single PR run timeline
  • ➖ More administrative overhead (multiple PRs/branches)
  • ➖ Harder to keep proof artifacts organized and referenced
3. Use an off-the-shelf markdown link checker action
  • ➕ Less custom code to maintain than a bespoke PowerShell gate
  • ➕ Potentially faster to iterate on common link-check behavior
  • ➖ Harder to precisely match repo needs (masking rules, anchor slugging parity, generated-file drift checks)
  • ➖ Adds external dependencies and may require network access, which this gate intentionally avoids

Recommendation: For long-term maintainability, the PR’s approach (a local, deterministic docs gate + Docusaurus broken-link enforcement + drift checking for generated homepage) is strong and appropriately avoids network dependence. For the temporary W0 red-path proof specifically, consider replacing committed intentional failures with workflow-controlled failure toggles to eliminate the chance of accidental merge; otherwise keep the current approach but ensure branch protections/merge rules prevent merging while failures exist.

Files changed (16) +1875 / -59

Enhancement (2) +745 / -0
ConvertTo-DocumentationHomepage.ps1Add README→Docusaurus homepage generator with URL rewriting +113/-0

Add README→Docusaurus homepage generator with URL rewriting

• Adds a PowerShell script that produces docs/docs/index.md content from README.md, prepending front matter and rewriting the configured SiteUrl origin to root-relative links. Includes YAML-safe single-quoted scalar serialization and LF normalization.

build/ConvertTo-DocumentationHomepage.ps1

Test-Documentation.ps1Add documentation gate: link/anchor checks, terminology casing, drift detection +632/-0

Add documentation gate: link/anchor checks, terminology casing, drift detection

• Introduces a comprehensive PowerShell validator that scans Markdown for missing relative link targets and invalid heading anchors, enforces terminology casing over prose (with code/URLs masked), and checks generated files for drift against their generator. Emits findings and fails on any non-warning errors (or warnings when TreatWarningsAsErrors is set).

build/Test-Documentation.ps1

Tests (1) +7 / -0
__redpath.test.tsAdd intentional failing test for CI red-path proof +7/-0

Add intentional failing test for CI red-path proof

• Adds a Vitest test that always fails to prove the engine CI check turns red. Intended to be removed/reverted before merging.

src/engine/src/core/__redpath.test.ts

Documentation (6) +853 / -47
README.mdConvert README relative doc links to published URLs and code-host links +13/-13

Convert README relative doc links to published URLs and code-host links

• Replaces docs/docs/* relative links with site URLs under https://game-engine.subzerodev.com/docs/engine/…. Updates repository-path links (src/engine, docs.ps1) to GitHub blob/tree URLs to keep them valid outside the docs site.

README.md

agent.mdInsert deliberate broken relative link for docs gate red-path proof +3/-0

Insert deliberate broken relative link for docs gate red-path proof

• Adds a comment and a relative Markdown link to a nonexistent file to intentionally fail the documentation gate check. Intended to be reverted before merging.

agent.md

TODO.mdClarify W0 required checks and add deliberate broken site-absolute route +9/-4

Clarify W0 required checks and add deliberate broken site-absolute route

• Updates W0 Definition of Done language to distinguish PR-required checks from main-only deploy, avoiding a permanently pending required check. Appends an intentional broken /docs/… route link to force the Docusaurus build to fail for red-path proof.

docs/docs/engine/TODO.md

index.mdAdd generated docs homepage (from README) with site-relative routes +78/-0

Add generated docs homepage (from README) with site-relative routes

• Adds the Docusaurus homepage with front matter and content mirroring README, but using site-relative /docs/engine/... links where appropriate. Intended to be regenerated by the homepage generator and kept in sync via drift checking.

docs/docs/index.md

02-w0-ci-workflow.mdRefine W0 plan details: required checks, README link mapping, and local repro notes +87/-30

Refine W0 plan details: required checks, README link mapping, and local repro notes

• Updates planning documentation with corrected required-check guidance (do not require main-only deploy), a complete README relative-link mapping (15 occurrences / 10 targets), and clarified container invocation guidance (not using --user for docs build). Adds additional verification and sequencing notes discovered during review.

plans/02-w0-ci-workflow.md

04-w0-phase-1-implementation.mdAdd W0 Phase 1 draft implementation plan with findings and verification checklists +663/-0

Add W0 Phase 1 draft implementation plan with findings and verification checklists

• Introduces a detailed, phased implementation plan documenting approved findings (F1–F3), consequences, and guardrails for W0 execution. Captures external configuration assumptions, installer evidence, and step-by-step verification criteria.

plans/04-w0-phase-1-implementation.md

Other (7) +270 / -12
DocumentationRules.psd1Add documentation validation rules and generated-homepage definition +69/-0

Add documentation validation rules and generated-homepage definition

• Introduces terminology casing rules, excluded paths/files, and a GeneratedFiles block defining docs/docs/index.md as generated from README.md. Supplies generator arguments (title and site URL) used both for generation and drift checking.

.config/DocumentationRules.psd1

ci.ymlAdd engine CI workflow (Node 24, typecheck/lint/test) +44/-0

Add engine CI workflow (Node 24, typecheck/lint/test)

• Creates a single-job GitHub Actions workflow that runs in src/engine, installs dependencies via npm ci, and runs typecheck, lint, and tests. Uses concurrency cancellation to avoid redundant runs per branch.

.github/workflows/ci.yml

docs-ci.ymlAdd docs CI workflow: Markdown gate plus containerized docs build verification +75/-0

Add docs CI workflow: Markdown gate plus containerized docs build verification

• Adds a documentation gate job that runs build/Test-Documentation.ps1 and a separate job that builds the site using the published docs-template container. Uploads a Pages artifact to validate the deploy artifact path without deploying.

.github/workflows/docs-ci.yml

docs-deploy.ymlAdd docs deploy workflow to GitHub Pages (main only) +63/-0

Add docs deploy workflow to GitHub Pages (main only)

• Creates a Pages deployment workflow that builds docs in the docs-template container, uploads the Pages artifact, and deploys via deploy-pages. Keeps Pages write/id-token permissions scoped only to this workflow.

.github/workflows/docs-deploy.yml

docusaurus.config.tsSet Docusaurus site URL to the real custom domain +4/-3

Set Docusaurus site URL to the real custom domain

• Replaces the placeholder docs URL with https://game-engine.subzerodev.com and updates comments to reflect the published configuration. Keeps onBrokenLinks: 'throw' behavior for strict broken-link enforcement.

docs/docusaurus.config.ts

package-lock.jsonUpdate lockfile for Node 24 engine floor and @types/node v24 +11/-8

Update lockfile for Node 24 engine floor and @types/node v24

• Bumps @types/node from 22 to 24 and updates undici-types accordingly. Records engines.node >=24 in the package metadata represented in the lockfile.

src/engine/package-lock.json

package.jsonDeclare Node 24 as minimum supported version and bump @types/node +4/-1

Declare Node 24 as minimum supported version and bump @types/node

• Adds engines.node >=24 to establish the supported baseline and align with CI. Updates @types/node to ^24.0.0 to match the new Node floor.

src/engine/package.json

@The-Running-Dev

Copy link
Copy Markdown
Owner Author

Red-path evidence captured (failing test, broken relative link, broken absolute route — each independently confirmed red on the intended check), reverted, and confirmed green again. Recorded in plans/02-w0-ci-workflow.md and TODO.md. Closing without merge; branch will be deleted.

@The-Running-Dev
The-Running-Dev deleted the codex/w0-red-path-proof branch July 27, 2026 20:19
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 9 rules

Grey Divider


Action required

1. Committed failing test 🐞 Bug ≡ Correctness
Description
A new Vitest test asserts true is false, so the CI / engine workflow’s npm test step will
deterministically fail on every run.
Code

src/engine/src/core/__redpath.test.ts[R3-6]

+describe("W0 red-path proof — deliberate, reverted before merge", () => {
+  test("intentionally fails to prove CI goes red", () => {
+    expect(true).toBe(false);
+  });
Evidence
The test file contains expect(true).toBe(false) and the CI workflow explicitly runs npm test, so
CI will execute this test and fail.

src/engine/src/core/__redpath.test.ts[1-7]
.github/workflows/ci.yml[34-44]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`src/engine/src/core/__redpath.test.ts` contains an unconditional failing assertion, which will fail the engine CI job.

### Issue Context
The CI workflow runs `npm test` in `src/engine`.

### Fix Focus Areas
- src/engine/src/core/__redpath.test.ts[1-7]
- .github/workflows/ci.yml[34-44]

### Suggested fix
Delete the red-path test file or change the assertion to a meaningful passing test before merge.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Broken relative doc link 🐞 Bug ≡ Correctness
Description
agent.md and docs/docs/engine/TODO.md introduce links to non-existent targets (a relative Markdown
file path and a site-absolute Docusaurus route), which will be detected as broken during CI and
cause the documentation validation/build jobs to fail.
Code

agent.md[R91-92]

+<!-- W0 red-path proof: deliberate broken relative link, reverted before merge -->
+See [this does not exist](docs/does-not-exist.md) for details.
Evidence
The cited changes add link targets that do not resolve to real content: the relative path in
agent.md is checked by the documentation gate, which resolves relative targets and uses Test-Path to
verify the referenced file exists, emitting a MarkdownLink error when it does not, while the
site-absolute route added in docs/docs/engine/TODO.md is subject to Docusaurus’ broken link/markdown
link handling configured to throw, so an unresolved internal route will fail the Docusaurus build
step in the docs CI workflow.

agent.md[91-92]
build/Test-Documentation.ps1[344-445]
.github/workflows/docs-ci.yml[34-44]
docs/docs/engine/TODO.md[323-325]
docs/docusaurus.config.ts[19-25]
.github/workflows/docs-ci.yml[46-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Documentation now contains links that point to targets that do not exist (a relative Markdown file link and an internal Docusaurus route), which causes CI documentation validation/build steps to fail due to broken link detection.

## Issue Context
The docs CI workflow runs `./build/Test-Documentation.ps1` (which enforces a `MarkdownLink` rule by resolving relative targets and verifying they exist) and also runs a Docusaurus build in the “Verify Documentation Build” job; Docusaurus is configured to `throw` on broken links/markdown links.

## Fix Focus Areas
- agent.md[91-92]
- docs/docs/engine/TODO.md[324-325]
- build/Test-Documentation.ps1[344-445]
- docs/docusaurus.config.ts[19-25]
- .github/workflows/docs-ci.yml[34-44]
- .github/workflows/docs-ci.yml[46-66]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Link checker escapes repo 🐞 Bug ⛨ Security
Description
Test-DocumentationLink resolves relative Markdown targets with GetFullPath(Join-Path ...) and then
checks Test-Path without ensuring the resolved path stays under the repository root, so links like
'../../etc/passwd' can incorrectly pass and also act as a filesystem existence oracle on the runner.
Code

build/Test-Documentation.ps1[R418-429]

+            $decoded = [uri]::UnescapeDataString($filePart)
+            $resolved = [IO.Path]::GetFullPath((Join-Path $directory $decoded))
+
+            if (-not (Test-Path -LiteralPath $resolved)) {
+                New-DocumentationFinding `
+                    -RelativePath $relativePath `
+                    -Line $lineNumber `
+                    -Column $target.Column `
+                    -Severity 'Error' `
+                    -Rule 'MarkdownLink' `
+                    -Message "Link target '$filePart' does not exist."
+                continue
Evidence
The link checker computes an absolute resolved path from the current file’s directory and the link
target, then uses Test-Path directly; there is no validation that the resolved path is within
$Root.

build/Test-Documentation.ps1[344-444]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Test-DocumentationLink` resolves relative link targets and calls `Test-Path` on the resolved absolute path without verifying it remains inside the repository root. This allows path traversal out of the repo, making the link-check gate both bypassable and an unintended filesystem probe.

### Issue Context
The function already receives `$Root`, but uses it only for reporting relative paths, not for containment.

### Fix Focus Areas
- build/Test-Documentation.ps1[344-444]

### Suggested fix
1. Normalize the root: `$rootFull = [IO.Path]::GetFullPath($Root)`.
2. After computing `$resolved`, reject any target that is not under `$rootFull` (directory-boundary safe):
  - Ensure `$resolved` starts with `$rootFull` + directory separator (or equals `$rootFull` only when appropriate).
  - Use an ordinal ignore-case comparison on Windows if needed.
3. When outside root, emit a `MarkdownLink` error like "Link target resolves outside repository root." and continue.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Unvalidated SiteUrl format 🐞 Bug ⚙ Maintainability
Description
ConvertTo-DocumentationHomepage.ps1 relies on callers providing SiteUrl with a trailing slash, but
does not validate or normalize it; a missing trailing slash can produce malformed root-relative
links like //docs/... in the generated homepage.
Code

build/ConvertTo-DocumentationHomepage.ps1[R106-111]

+$body = if ([string]::IsNullOrWhiteSpace($SiteUrl)) {
+    $readme
+}
+else {
+    $readme.Replace($SiteUrl, '/')
+}
Evidence
The script explicitly states the trailing-slash requirement, but the implementation performs a raw
Replace($SiteUrl, '/') without enforcing that requirement.

build/ConvertTo-DocumentationHomepage.ps1[29-33]
build/ConvertTo-DocumentationHomepage.ps1[106-111]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The generator documents that `SiteUrl` must include a trailing slash, but does not enforce that precondition before running `Replace`. If a future caller passes `https://example.com` instead of `https://example.com/`, the replacement can yield unintended `//...` paths.

### Issue Context
Current repo configuration passes a trailing slash, so this is robustness hardening for future use.

### Fix Focus Areas
- build/ConvertTo-DocumentationHomepage.ps1[29-33]
- build/ConvertTo-DocumentationHomepage.ps1[106-111]

### Suggested fix
Add a guard before replacement:
- If `SiteUrl` is non-empty and does not end with `/`, either:
 - append `/` (normalization), or
 - throw with a clear message (validation).
Then proceed with the replacement using the normalized value.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +418 to +429
$decoded = [uri]::UnescapeDataString($filePart)
$resolved = [IO.Path]::GetFullPath((Join-Path $directory $decoded))

if (-not (Test-Path -LiteralPath $resolved)) {
New-DocumentationFinding `
-RelativePath $relativePath `
-Line $lineNumber `
-Column $target.Column `
-Severity 'Error' `
-Rule 'MarkdownLink' `
-Message "Link target '$filePart' does not exist."
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Link checker escapes repo 🐞 Bug ⛨ Security

Test-DocumentationLink resolves relative Markdown targets with GetFullPath(Join-Path ...) and then
checks Test-Path without ensuring the resolved path stays under the repository root, so links like
'../../etc/passwd' can incorrectly pass and also act as a filesystem existence oracle on the runner.
Agent Prompt
### Issue description
`Test-DocumentationLink` resolves relative link targets and calls `Test-Path` on the resolved absolute path without verifying it remains inside the repository root. This allows path traversal out of the repo, making the link-check gate both bypassable and an unintended filesystem probe.

### Issue Context
The function already receives `$Root`, but uses it only for reporting relative paths, not for containment.

### Fix Focus Areas
- build/Test-Documentation.ps1[344-444]

### Suggested fix
1. Normalize the root: `$rootFull = [IO.Path]::GetFullPath($Root)`.
2. After computing `$resolved`, reject any target that is not under `$rootFull` (directory-boundary safe):
   - Ensure `$resolved` starts with `$rootFull` + directory separator (or equals `$rootFull` only when appropriate).
   - Use an ordinal ignore-case comparison on Windows if needed.
3. When outside root, emit a `MarkdownLink` error like "Link target resolves outside repository root." and continue.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +106 to +111
$body = if ([string]::IsNullOrWhiteSpace($SiteUrl)) {
$readme
}
else {
$readme.Replace($SiteUrl, '/')
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Unvalidated siteurl format 🐞 Bug ⚙ Maintainability

ConvertTo-DocumentationHomepage.ps1 relies on callers providing SiteUrl with a trailing slash, but
does not validate or normalize it; a missing trailing slash can produce malformed root-relative
links like //docs/... in the generated homepage.
Agent Prompt
### Issue description
The generator documents that `SiteUrl` must include a trailing slash, but does not enforce that precondition before running `Replace`. If a future caller passes `https://example.com` instead of `https://example.com/`, the replacement can yield unintended `//...` paths.

### Issue Context
Current repo configuration passes a trailing slash, so this is robustness hardening for future use.

### Fix Focus Areas
- build/ConvertTo-DocumentationHomepage.ps1[29-33]
- build/ConvertTo-DocumentationHomepage.ps1[106-111]

### Suggested fix
Add a guard before replacement:
- If `SiteUrl` is non-empty and does not end with `/`, either:
  - append `/` (normalization), or
  - throw with a clear message (validation).
Then proceed with the replacement using the normalized value.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/engine/src/core/__redpath.test.ts Outdated
Comment on lines +3 to +6
describe("W0 red-path proof — deliberate, reverted before merge", () => {
test("intentionally fails to prove CI goes red", () => {
expect(true).toBe(false);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

3. Committed failing test 🐞 Bug ≡ Correctness

A new Vitest test asserts true is false, so the CI / engine workflow’s npm test step will
deterministically fail on every run.
Agent Prompt
### Issue description
`src/engine/src/core/__redpath.test.ts` contains an unconditional failing assertion, which will fail the engine CI job.

### Issue Context
The CI workflow runs `npm test` in `src/engine`.

### Fix Focus Areas
- src/engine/src/core/__redpath.test.ts[1-7]
- .github/workflows/ci.yml[34-44]

### Suggested fix
Delete the red-path test file or change the assertion to a meaningful passing test before merge.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread agent.md Outdated
Comment on lines +91 to +92
<!-- W0 red-path proof: deliberate broken relative link, reverted before merge -->
See [this does not exist](docs/does-not-exist.md) for details.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

4. Broken relative doc link 🐞 Bug ≡ Correctness

agent.md and docs/docs/engine/TODO.md introduce links to non-existent targets (a relative Markdown
file path and a site-absolute Docusaurus route), which will be detected as broken during CI and
cause the documentation validation/build jobs to fail.
Agent Prompt
## Issue description
Documentation now contains links that point to targets that do not exist (a relative Markdown file link and an internal Docusaurus route), which causes CI documentation validation/build steps to fail due to broken link detection.

## Issue Context
The docs CI workflow runs `./build/Test-Documentation.ps1` (which enforces a `MarkdownLink` rule by resolving relative targets and verifying they exist) and also runs a Docusaurus build in the “Verify Documentation Build” job; Docusaurus is configured to `throw` on broken links/markdown links.

## Fix Focus Areas
- agent.md[91-92]
- docs/docs/engine/TODO.md[324-325]
- build/Test-Documentation.ps1[344-445]
- docs/docusaurus.config.ts[19-25]
- .github/workflows/docs-ci.yml[34-44]
- .github/workflows/docs-ci.yml[46-66]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

The-Running-Dev added a commit that referenced this pull request Jul 27, 2026
…findings

Completes everything in W0's Definition of Done that doesn't require merging.

Required status checks
Added a required_status_checks rule to the repository's "Main" ruleset
(id 19779713) -- this repo uses rulesets, not classic branch protection,
which is why the old protection endpoint 404s. Contexts: engine,
Documentation links and terminology, Verify Documentation Build, each pinned
to the GitHub Actions app (integration_id 15368); Build and Deploy
Documentation deliberately excluded, since it only triggers on push to main
and would leave every PR pending forever if required. Confirmed via the
check-runs API before configuring that the required-status-check context is
the job name alone ("engine"), not the workflow-qualified "CI / engine" label
GitHub's UI shows and this plan used as shorthand in several places --
corrected the one spot presenting that shorthand as literal config
(plans/02, "Make the three pull-request checks required").

Red-path proof
Ran on a temporary branch (codex/w0-red-path-proof, PR #4, closed without
merging and deleted after capture): three deliberate, isolated failures --
a failing test for `engine`; a broken relative link in agent.md (outside the
Docusaurus content tree, so it doesn't also trip the build) for the gate; a
broken site-absolute route in docs/docs/engine/TODO.md (the gate's own source
skips http(s) and leading-/ targets outright, so this doesn't also trip the
gate) for the production build. Verified locally before pushing that each
trips exactly one check, then confirmed all three remotely at the intended
cause, not incidentally: AssertionError on the deliberate test, the exact
MarkdownLink error on agent.md, Docusaurus's own executeBrokenLinksCheck on
the injected route. A fourth, incidental result confirmed the concurrency
group works as designed -- an earlier run on the same branch shows cancelled.
Reverted in one commit, pushed, all three back to green. Full evidence with
run URLs recorded in plans/04.

Documentation
- plans/04: Phase 6 checklist filled in with what's done vs what's blocked on
  merging (first deploy, HTTPS enforcement); acceptance checklist updated
  with concrete verification (sidebar front matter inspected directly, not
  assumed); a new section states plainly that merging is paused for explicit
  confirmation since it publishes a live site to a real custom domain.
- TODO.md W0: recorded the same evidence inline, plus the two known-deliberate-
  end-state facts (bare root 404s while /docs/ serves; generated homepage adds
  a top-level sidebar entry, engine/ ordering unaffected) so they're visible
  in the living spec, not only the execution-record plan.
- TODO.md Known Open Items: added the three docs-template findings from PR #3
  review as a tracked, unfiled follow-up -- explicitly deferred until after
  this PR merges, since raising them means a separate PR against a different
  repository. Includes the -BaseImage/-Overwrite investigation for the first
  finding and links back to the full reasoning on each review thread.

Verified: documentation gate passes across 16 files; engine suite green
(15 tests); git diff --check clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The-Running-Dev added a commit that referenced this pull request Jul 27, 2026
* Plan W0 phase 1 implementation

* Add two peer-review consequences to the W0 Phase 1 plan

Both surfaced while verifying the plan's findings against the published image
and the live Pages configuration. Neither changes scope, so they are folded
into the phases they affect and recorded as A1/A2 rather than raised as
findings needing approval.

A1 — the generated homepage changes the sidebar (Phase 5)
ConvertTo-DocumentationHomepage.ps1 writes front matter, not just body text:
a title and sidebar_position: 1. The sidebar is autogenerated from docs/docs/,
which until now held only engine/, so a new top-level entry appears above that
category. Confirmed this does not disturb ordering inside engine/ — those
positions apply within the category and encode the stated reading order, in
which 04-core precedes 03-story-graph-kind. Added the front matter, both
consequences, and two verification items so it is confirmed rather than
discovered.

A2 — the bare root will 404 after deployment (Phase 6)
routeBasePath stays 'docs' and Phase 5 deliberately does not change it, so the
site publishes at /docs/ while the bare hostname keeps returning 404. Today
both 404 because nothing is deployed; after the first deploy they diverge.
Fine for a docs subdomain, but it stops being hypothetical once the domain is
shareable, so it is now recorded as an expected end state with the two exits
noted (routeBasePath '/' or a root page under docs/src/pages/), both outside
W0. Added an acceptance item so the 404 is reported as expected rather than as
a deployment failure.

Verified while reviewing, and unchanged by this commit: F1 reproduced — the
dispatcher -WhatIf prints one line and enumerates nothing, while the direct
pwsh -File form printed exactly the six creates and five skips the plan
predicts, writing nothing. F2 confirmed at 15 occurrences across 10 unique
targets. F3 confirmed — docs-deploy.yml triggers only on push to main and
workflow_dispatch, so requiring it on pull requests would leave every one
pending. External state confirmed via the Pages API: build_type workflow,
cname game-engine.subzerodev.com, domain verified, https_enforced false, no
branch protection, both URLs 404 pre-deploy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Apply F1-F3 to the authoritative W0 documents

All three of Codex's findings are correct, each reproduced before being
accepted, and each corrects a defect in plans/02 rather than in its own plan.
Applied now rather than deferred: they are factual corrections with one right
answer, and a round-trip would only delay execution.

F1 - the documented dry run never reached the installer
Ran both forms. The Invoke-SetupDocs dispatcher with -WhatIf prints a single
line -- "Performing the operation 'Invoke discovered PowerShell Script'" --
and enumerates nothing, so it is not a preview at all. Replaced with the
direct pwsh -File /PSModule/Scripts/setup-docs.ps1 form, which printed exactly
six creates and five skips and wrote nothing to the worktree. Both the
expected file lists are now in the plan, and the real install still uses the
public entry point, since only -WhatIf degrades.

F1b - the expected skip list was four files, not five
docs/.dockerignore exists here and is skipped too. Corrected in both the
decision table and the dry-run step.

F2 - the README link count was wrong
15 occurrences across 10 unique targets, not 14. The old figure came from
counting distinct link texts: [MVP] and [MVP.md] point at the same file and
were counted twice while others were missed. Replaced the bare count with the
full relative -> absolute target map, and the instruction is now "replace by
target, not by count", which is the failure mode that produced the error.

F3 - requiring the deploy check would have hung every pull request
Verified against the installed template: Build and Deploy Documentation lives
in docs-deploy.yml, which triggers only on push to main and workflow_dispatch.
It can never report on a pull request, so requiring it would leave every PR
pending indefinitely. Required checks are now the three that actually run --
CI / engine, Documentation links and terminology, Verify Documentation Build
-- with deploy called out as post-merge verification. Fixed in the reference
section, the install step, the acceptance list, and TODO's W0 done-criteria so
plan and unit cannot drift.

plans/04 now records the disposition: findings approved with the evidence for
each, its Phase 1 marked done and retained as the record, and the peer-review
order narrowed to what actually remains (Phases 2-6).

Verified: 0 unresolved cross-document section references; every internal
markdown link resolves; docs production build passes with onBrokenLinks set to
throw; git diff --check clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Install documentation system

* Implement W0 Phases 3 and 5; fix a --user bug found while verifying

Completes the file-level work Phase 4 (already installed) left outstanding:
author ci.yml, align Node, and integrate the published documentation. Every
step verified locally before commit, not assumed from the plan.

Phase 5 first, ahead of the plan's stated order
Converting the README had to happen before regenerating the homepage, or the
generator bakes the broken links into docs/docs/index.md again. Confirmed
this was necessary: before the fix, the installed gate failed with exactly
the 15 errors F2 predicted, and the Docusaurus production build failed on the
same links via MDX compilation. This is the ordering defect flagged during
the prior review, worth recording as resolved by doing Phase 5's README work
before regenerating from Phase 4's install.

- README.md: all 15 relative-link occurrences (10 unique targets) replaced
  with absolute URLs per the approved target map in plans/02 -- spec links to
  https://game-engine.subzerodev.com/docs/engine/..., docs.ps1 and
  src/engine/ to the code host. Zero relative links remain.
- docs/docusaurus.config.ts: url set to the real published origin,
  https://game-engine.subzerodev.com; baseUrl was already '/'.
- docs/docs/index.md: regenerated via
  build/ConvertTo-DocumentationHomepage.ps1 -ReadmePath ./README.md
  -Title 'Game Engine' -SiteUrl 'https://game-engine.subzerodev.com/',
  matching the parameters used at install time. Confirmed the generator
  rewrites only links matching SiteUrl to root-relative paths; the two
  GitHub links were left absolute, as intended.
- Verified: pwsh build/Test-Documentation.ps1 now passes across 16 Markdown
  files (was 15 errors); the production Invoke-DocsBuild succeeds with no
  broken links.

Phase 3
- .github/workflows/ci.yml authored to the approved pattern: one `engine`
  job, checkout@v7, setup-node@v7 on Node 24 with the subdirectory lockfile
  cache, the concurrency group keyed on workflow + head repository + branch
  name, separate typecheck/lint/test steps, contents: read only, no docs job.
  Quoted "on": to match the installed workflows' convention and avoid YAML
  1.1's boolean coercion of the bare key. Parsed with Ruby's YAML loader in
  the absence of a local Actions linter.
- src/engine/package.json: added engines.node ">=24"; @types/node ^22.0.0 ->
  ^24.0.0.
- Regenerated the lockfile with a plain npm install; reviewed the diff --
  limited to @types/node, its transitive undici-types bump, and the new
  engines block. No unrelated dependency moved.
- Verified from a wiped node_modules: npm ci, typecheck, lint, and all 15
  tests green.

A --user bug found while reproducing the CI docs build locally
plans/02's documented "reproduce the CI build" command passed --user
"$(id -u):$(id -g)" to Invoke-DocsBuild, copied from the Invoke-SetupDocs
example next to it. Running it failed: Invoke-DocsBuild overlays docs/ onto
/template inside the image before building, and /template is root-owned
(755) in the published image, so a non-root user cannot write into it --
Copy-Item: Access to the path '/template/Dockerfile' is denied. Confirmed by
running the identical command without --user, which then succeeded, and by
checking the installed docs-ci.yml: its container: job carries no user:
override, so CI itself runs this as root and was never affected -- this was
a local-reproduction-only defect. Corrected both occurrences in
plans/02-w0-ci-workflow.md and the one in plans/04, with the reasoning
recorded so the flag is not reintroduced by analogy with Invoke-SetupDocs,
which needs it for the opposite reason (it writes only into the mounted
/work).

Not done here, deliberately: nothing pushed, no PR opened, no branch
protection or required status checks configured, no push to main to trigger
docs-deploy.yml. Those are shared-state / repo-settings actions and W0's own
Definition of Done requires the red-path proof (a deliberate failure per
check, with run URLs recorded) before it can be marked complete -- none of
that can be produced from local-only work.

Verified end to end: engine npm ci / typecheck / lint / test green (15
tests); documentation gate green across 16 files; production docs build
green with zero broken links; ci.yml parses as valid YAML; git diff --check
clean; only the Phase 1 allowed file surface changed, confirmed against
951efeb (the install commit).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Configure required checks, capture red-path proof, document deferred findings

Completes everything in W0's Definition of Done that doesn't require merging.

Required status checks
Added a required_status_checks rule to the repository's "Main" ruleset
(id 19779713) -- this repo uses rulesets, not classic branch protection,
which is why the old protection endpoint 404s. Contexts: engine,
Documentation links and terminology, Verify Documentation Build, each pinned
to the GitHub Actions app (integration_id 15368); Build and Deploy
Documentation deliberately excluded, since it only triggers on push to main
and would leave every PR pending forever if required. Confirmed via the
check-runs API before configuring that the required-status-check context is
the job name alone ("engine"), not the workflow-qualified "CI / engine" label
GitHub's UI shows and this plan used as shorthand in several places --
corrected the one spot presenting that shorthand as literal config
(plans/02, "Make the three pull-request checks required").

Red-path proof
Ran on a temporary branch (codex/w0-red-path-proof, PR #4, closed without
merging and deleted after capture): three deliberate, isolated failures --
a failing test for `engine`; a broken relative link in agent.md (outside the
Docusaurus content tree, so it doesn't also trip the build) for the gate; a
broken site-absolute route in docs/docs/engine/TODO.md (the gate's own source
skips http(s) and leading-/ targets outright, so this doesn't also trip the
gate) for the production build. Verified locally before pushing that each
trips exactly one check, then confirmed all three remotely at the intended
cause, not incidentally: AssertionError on the deliberate test, the exact
MarkdownLink error on agent.md, Docusaurus's own executeBrokenLinksCheck on
the injected route. A fourth, incidental result confirmed the concurrency
group works as designed -- an earlier run on the same branch shows cancelled.
Reverted in one commit, pushed, all three back to green. Full evidence with
run URLs recorded in plans/04.

Documentation
- plans/04: Phase 6 checklist filled in with what's done vs what's blocked on
  merging (first deploy, HTTPS enforcement); acceptance checklist updated
  with concrete verification (sidebar front matter inspected directly, not
  assumed); a new section states plainly that merging is paused for explicit
  confirmation since it publishes a live site to a real custom domain.
- TODO.md W0: recorded the same evidence inline, plus the two known-deliberate-
  end-state facts (bare root 404s while /docs/ serves; generated homepage adds
  a top-level sidebar entry, engine/ ordering unaffected) so they're visible
  in the living spec, not only the execution-record plan.
- TODO.md Known Open Items: added the three docs-template findings from PR #3
  review as a tracked, unfiled follow-up -- explicitly deferred until after
  this PR merges, since raising them means a separate PR against a different
  repository. Includes the -BaseImage/-Overwrite investigation for the first
  finding and links back to the full reasoning on each review thread.

Verified: documentation gate passes across 16 files; engine suite green
(15 tests); git diff --check clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix three defects found in final review

All three originate in this repository's own authored files, not in the
installed template. Two were latent traps that would have surfaced later.

1. Concurrency design blocked the merge
PR #3 sat at mergeStateStatus BLOCKED with all three checks green. ci.yml
triggered on every push AND pull_request, deduplicated by a shared concurrency
group with cancel-in-progress; both fire on a PR branch, so one run cancelled
the other. A cancelled run still publishes its check-run, so the commit carried
two `engine` results -- one success, one cancelled -- and the
required_status_checks rule blocks on the cancelled one.

Settled the diagnosis by comparing against the installed docs-ci.yml, which
uses `push: branches: [main]` and no concurrency group at all: it prevents the
duplicate structurally rather than cancelling after the fact. Matched that.
Kept a concurrency group keyed on github.ref, which is now only reachable for
pushes to main, where superseding consecutive runs is still correct.

Note this was a race, not a deterministic failure: on commit 1e1dd86 the push
run won and both results were green, so it would have blocked merges
intermittently and looked inexplicable.

2. .gitignore silently swallowed installer source files
Line 6 was a bare `build/` under "Build output", but the documentation system
installs its *source* scripts to ./build (Test-Documentation.ps1,
ConvertTo-DocumentationHomepage.ps1). The two already-tracked files kept
working, which is what hid it; verified by probe that a NEW file under build/
is invisible to `git add -A`. A future Invoke-SetupDocs adding a script there
would produce a file that exists locally, passes locally, and is missing in CI
with nothing explaining why.

Confirmed the pattern protected nothing before removing it: TypeScript emits to
dist/ (src/engine/tsconfig.json outDir) and the only build/ in the repository
is the installed scripts. Replaced with a comment recording why it must not
return.

3. artifacts/ was not ignored
Invoke-DocsBuild -OutputPath artifacts/docs -- the command in our own
documented verification steps -- writes the full rendered site into the
repository. It was deleted by hand twice during this implementation; anyone
following the steps then running `git add -A` would commit the whole built
site. Added artifacts/.

Each fix verified by probe rather than inspection: a new build/ file now
appears in git status; a file under artifacts/ is reported ignored by
git check-ignore; both tracked build/ scripts remain tracked; ci.yml parses
and still declares exactly one job.

plans/02 corrected at the source of the bad guidance -- it specified the
unrestricted trigger plus shared group -- with the failure mode recorded so the
design is not reintroduced. plans/04 records all three under the final-review
findings.

Verified: documentation gate passes across 16 files; engine suite green (15
tests); ci.yml parses; git diff --check clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant