PR_26180_OWNER_018-move-src-browser-to-www#297
Conversation
There was a problem hiding this comment.
💡 Codex Review
After this file moved from src/shared/toolbox to www/src/shared/toolbox, this unchanged import now resolves to www/www/toolbox/tool-registry-api-client.js, which does not exist; node --test dev/tests/tools/AssetUsageIntegration.test.mjs fails with ERR_MODULE_NOT_FOUND. The same ../../../www/toolbox/... pattern appears in the moved toolbox shell/host modules, so any consumer of these shared toolbox modules fails before executing unless the relative path is adjusted to the new location.
HTML-JavaScript-Gaming/www/src/engine/replay/ReplayModel.js
Lines 7 to 9 in b424f29
This moved module now resolves the contract import under www/src/shared/contracts/replayContracts.js, but the commit leaves the contracts directory only under root src/shared/contracts, so importing the replay runtime fails immediately; node --test dev/tests/replay/ReplaySystem.test.mjs reports ERR_MODULE_NOT_FOUND for this path. The same missing-contract problem also affects the moved shared state guards/normalization imports, so the contracts need to be moved with these modules or the imports need to keep resolving to the remaining contract location.
With this module now under www/src, the default schema URL points at www/src/shared/schemas/tools/object-vector-studio-v2.schema.json, but the commit leaves schemas under root src/shared/schemas; service.schemaPath now resolves to the missing www/src/shared/schemas/... path. Any Object Vector runtime path that calls loadSchema() with the default schemaUrl will fetch a 404/missing file until the schemas are moved with browser runtime code or this URL is retargeted to the remaining schema location.
Moving this metadata module out of root src leaves existing Node consumers such as www/toolbox/toolRegistry.js importing ../../src/shared/toolbox/tool-metadata-inventory.js, which now no longer exists; node --test dev/tests/tools/RequiredToolsBaseline.test.mjs and related registry tests fail with ERR_MODULE_NOT_FOUND. Browser URL resolution may still collapse to /src/..., but direct Node imports from the tracked www modules need a filesystem-valid path like the other updated imports.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "www/toolbox/toolRegistry.js", | ||
| "www/toolbox/tools-page-accordions.js", | ||
| "src/engine/paletteList.js", | ||
| "www/src/engine/paletteList.js", |
There was a problem hiding this comment.
Keep the canonical guardrail scanning www/src engine files
Changing the approved exceptions to www/src/... is incomplete because the guardrail still only treats src/engine/ as a relevant engine path in its matching logic, so the moved root-level engine JS/CSS is no longer audited and the updated regression test now fails (legacy.length is 1 instead of 2, and the violation fixture count drops from 7 to 4). When engine files are under www/src, the guardrail needs to match that prefix too rather than only adding it to the exception set.
Useful? React with 👍 / 👎.
| }); | ||
| expect(sources.controls).toContain("../../src/engine/input/NormalizedInputRegistry.js"); | ||
| expect(sources.controls).not.toContain("../../src/engine/input/InputService.js"); | ||
| expect(sources.controls).toContain("../../www/src/engine/input/NormalizedInputRegistry.js"); |
There was a problem hiding this comment.
Assert the served import paths, not nonexistent www paths
This Playwright test fetches the actual browser-served files, and those files still contain relative imports like ../../../../../src/engine/input/NormalizedInputRegistry.js and ../../src/engine/input/InputService.js so that they resolve to the public /src/... compatibility route. Updating the assertions to look for ../../www/src/... and ../www/src/... makes the test fail even when the page source is correct, because those strings are not present in the served modules.
Useful? React with 👍 / 👎.
PR_26180_OWNER_018-move-src-browser-to-www Report
Executive Summary
Moved PR017-audited browser/www-owned legacy
src/files intowww/src/so public browser imports such as/src/...continue to resolve under thewwwweb root. This PR does not move API/server-owned or dev-ownedsrcfiles and does not change product behavior.Scope Confirmation
PR_26180_OWNER_018-move-src-browser-to-wwwPR_26180_OWNER_017-src-dissection-and-demo-cleanupf4be737bbfcccd0f04f5298963ac7573362df7caImplementation Notes
src/advanced,src/apibrowser clients,src/dev-runtime/adminbrowser viewer files,src/engine, browser/shared helpers, toolbox browser modules, and browser tool helpers intowww/src/according to the PR017 destination audit.www/src/filesystem paths./src/engine/core/Engine.js,/src/api/admin-owner-navigation.js, and/src/dev-runtime/admin/notes.htmlthrough the existing static web root compatibility layer.Files/Folders Moved
src/advanced/**->www/src/advanced/**src/api/**browser API clients ->www/src/api/**src/dev-runtime/admin/**browser viewer files ->www/src/dev-runtime/admin/**src/engine/**browser/runtime engine modules ->www/src/engine/**src/shared/**helper/toolbox modules ->www/src/shared/**src/tools/**browser tool helpers ->www/src/tools/**Remaining Source Buckets
src/shared/contracts/**,src/shared/schemas/**, andsrc/shared/projectDataStore/**remain for PR019 API/server migration.src/dev-runtime/admin/.gitkeepand any remaining dev/reference ownership remain for later scoped PRs.src/is not retired in this PR.