Skip to content

Commit 943a91b

Browse files
committed
PR_26172_CHARLIE_033-game-journey-canonical-js-migration-audit
1 parent 5132cc2 commit 943a91b

3 files changed

Lines changed: 182 additions & 70 deletions

File tree

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# PR_26172_CHARLIE_033-game-journey-canonical-js-migration-audit
2+
3+
## Summary
4+
5+
Status: PASS.
6+
7+
This audit reviewed Game Journey for canonical JS migration readiness without implementation changes.
8+
9+
## Files Reviewed
10+
11+
- `docs_build/dev/ProjectInstructions/README.txt`
12+
- `docs_build/dev/ProjectInstructions/PROJECT_INSTRUCTIONS.md`
13+
- `docs_build/dev/reports/PR_26172_CHARLIE_006A-game-journey-validation-failure-investigation.md`
14+
- `toolbox/game-journey/index.html`
15+
- `toolbox/game-journey/game-journey.js`
16+
- `toolbox/game-journey/game-journey-api-client.js`
17+
- `tests/playwright/tools/GameJourneyTool.spec.mjs`
18+
- `scripts/validate-canonical-repository-structure.mjs`
19+
- `src/dev-runtime/server/local-api-router.mjs`
20+
- `src/api/game-journey-completion-api-client.js`
21+
22+
## Current Active JS Entrypoints
23+
24+
| Current path | Role | Current status | Target path |
25+
| --- | --- | --- | --- |
26+
| `toolbox/game-journey/game-journey.js` | Browser entrypoint | Active legacy JS | `assets/toolbox/game-journey/js/index.js` |
27+
| `toolbox/game-journey/game-journey-api-client.js` | Tool API client and completion metrics re-export | Active legacy JS | Temporary exception unless guardrail/shared-client placement is approved |
28+
29+
## HTML Script References
30+
31+
- `toolbox/game-journey/index.html`
32+
- Current: `toolbox/game-journey/game-journey.js`
33+
- Proposed PR_034 value: `assets/toolbox/game-journey/js/index.js`
34+
35+
## API Client References
36+
37+
- `toolbox/game-journey/game-journey.js` imports:
38+
- `./game-journey-api-client.js`
39+
- `../tool-registry-api-client.js`
40+
- After moving the entrypoint, imports should become:
41+
- `../../../../toolbox/game-journey/game-journey-api-client.js`
42+
- `../../../../toolbox/tool-registry-api-client.js`
43+
44+
## Completion Metrics Risk
45+
46+
PR_26172_CHARLIE_006A determined that `/api/game-journey/completion-metrics` can return HTTP 500 when local legacy SQLite preservation protection is triggered by:
47+
48+
- `tmp/local-api/game-journey-completion-metrics.sqlite`
49+
50+
That behavior is intentional and prevents silent data loss. It is not caused by prior Charlie JS/CSS migrations. PR_034 validation must document this separately if it appears and must not misattribute the known HTTP 500 to the entrypoint move without direct evidence.
51+
52+
## Migration Decision
53+
54+
### `game-journey.js`
55+
56+
Decision: safe to migrate in PR_034 with targeted import and HTML updates.
57+
58+
Reason:
59+
60+
- It is a single browser entrypoint.
61+
- It is referenced directly by the Game Journey page.
62+
- It can keep existing behavior by importing the retained API client and tool registry client through adjusted relative paths.
63+
- No CSS move is required.
64+
65+
### `game-journey-api-client.js`
66+
67+
Decision: do not move in PR_034. Review in PR_035.
68+
69+
Reason:
70+
71+
- The canonical guardrail currently approves the tool entrypoint path `assets/toolbox/{tool}/js/index.js`.
72+
- Adding a second JS file under `assets/toolbox/game-journey/js/` would require either guardrail expansion or a folded/shared-client migration.
73+
- The API client re-exports completion metrics helpers, so it should not be moved in the same PR as the entrypoint unless validation proves it is safe.
74+
75+
## Validation Plan for PR_034
76+
77+
- `node --check assets/toolbox/game-journey/js/index.js`
78+
- `git diff --check`
79+
- `npm run validate:canonical-structure`
80+
- Active stale reference check for `toolbox/game-journey/game-journey.js`
81+
- Targeted Game Journey Playwright validation, with any known completion-metrics legacy SQLite HTTP 500 documented separately.
82+
83+
## Branch Validation
84+
85+
- Current branch: `PR_26172_CHARLIE_repository-compliance-stack`
86+
- Expected branch: `PR_26172_CHARLIE_repository-compliance-stack`
87+
- Local/origin sync before PR: `0 0`
88+
- Branch validation: PASS
89+
90+
## Requirement Checklist
91+
92+
- Review `game-journey.js`: PASS
93+
- Review `game-journey-api-client.js`: PASS
94+
- Review `index.html`: PASS
95+
- Review Game Journey tests: PASS
96+
- Review Local API completion metrics route: PASS
97+
- Review prior PR_006A findings: PASS
98+
- Determine whether `game-journey.js` can safely move: PASS
99+
- Determine whether API client should move, remain, or become shared: PASS
100+
- Define validation that avoids confusing known SQLite preservation behavior with migration failure: PASS
101+
- No implementation changes: PASS
102+
- ZIP artifact exists: PASS after artifact creation.
103+
104+
## Manual Validation Notes
105+
106+
The next safe implementation step is a focused entrypoint move only. The API client should remain an exception until PR_035 because the guardrail does not currently allow a second canonical tool JS file.
107+
108+
## Recommendation
109+
110+
Continue to PR_034 and migrate only `game-journey.js` to `assets/toolbox/game-journey/js/index.js`.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
A docs_build/dev/reports/PR_26172_CHARLIE_032-assets-worker-api-client-migration-or-exception.md
1+
A docs_build/dev/reports/PR_26172_CHARLIE_033-game-journey-canonical-js-migration-audit.md
Lines changed: 71 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,89 +1,90 @@
1-
diff --git a/docs_build/dev/reports/PR_26172_CHARLIE_032-assets-worker-api-client-migration-or-exception.md b/docs_build/dev/reports/PR_26172_CHARLIE_032-assets-worker-api-client-migration-or-exception.md
1+
diff --git a/docs_build/dev/reports/PR_26172_CHARLIE_033-game-journey-canonical-js-migration-audit.md b/docs_build/dev/reports/PR_26172_CHARLIE_033-game-journey-canonical-js-migration-audit.md
22
new file mode 100644
3-
index 000000000..44d4c753c
3+
index 000000000..5ca46e8ab
44
--- /dev/null
5-
+++ b/docs_build/dev/reports/PR_26172_CHARLIE_032-assets-worker-api-client-migration-or-exception.md
6-
@@ -0,0 +1,108 @@
7-
+# PR_26172_CHARLIE_032-assets-worker-api-client-migration-or-exception
5+
+++ b/docs_build/dev/reports/PR_26172_CHARLIE_033-game-journey-canonical-js-migration-audit.md
6+
@@ -0,0 +1,110 @@
7+
+# PR_26172_CHARLIE_033-game-journey-canonical-js-migration-audit
88
+
99
+## Summary
1010
+
11-
+Status: PASS with temporary legacy exceptions retained.
11+
+Status: PASS.
1212
+
13-
+No executable implementation files were changed in this PR. The remaining Assets tool legacy files were reviewed after the PR_031 entrypoint migration:
14-
+
15-
+- `toolbox/assets/assets-api-client.js`
16-
+- `toolbox/assets/assets-upload-worker.js`
17-
+
18-
+Both files remain in approved legacy locations for this stack because moving them would require governance or shared-client decisions outside this PR scope.
13+
+This audit reviewed Game Journey for canonical JS migration readiness without implementation changes.
1914
+
2015
+## Files Reviewed
2116
+
2217
+- `docs_build/dev/ProjectInstructions/README.txt`
2318
+- `docs_build/dev/ProjectInstructions/PROJECT_INSTRUCTIONS.md`
24-
+- `docs_build/dev/reports/PR_26172_CHARLIE_030-assets-tool-canonical-js-migration-audit.md`
25-
+- `docs_build/dev/reports/PR_26172_CHARLIE_031-assets-tool-safe-entrypoint-migration.md`
26-
+- `assets/toolbox/assets/js/index.js`
27-
+- `assets/toolbox/objects/js/index.js`
28-
+- `toolbox/assets/assets-api-client.js`
29-
+- `toolbox/assets/assets-upload-worker.js`
19+
+- `docs_build/dev/reports/PR_26172_CHARLIE_006A-game-journey-validation-failure-investigation.md`
20+
+- `toolbox/game-journey/index.html`
21+
+- `toolbox/game-journey/game-journey.js`
22+
+- `toolbox/game-journey/game-journey-api-client.js`
23+
+- `tests/playwright/tools/GameJourneyTool.spec.mjs`
3024
+- `scripts/validate-canonical-repository-structure.mjs`
31-
+- `scripts/validate-browser-env-agnostic.mjs`
32-
+- `tests/playwright/tools/AssetToolMockRepository.spec.mjs`
25+
+- `src/dev-runtime/server/local-api-router.mjs`
26+
+- `src/api/game-journey-completion-api-client.js`
3327
+
34-
+## Decision
28+
+## Current Active JS Entrypoints
3529
+
36-
+### Assets API Client
30+
+| Current path | Role | Current status | Target path |
31+
+| --- | --- | --- | --- |
32+
+| `toolbox/game-journey/game-journey.js` | Browser entrypoint | Active legacy JS | `assets/toolbox/game-journey/js/index.js` |
33+
+| `toolbox/game-journey/game-journey-api-client.js` | Tool API client and completion metrics re-export | Active legacy JS | Temporary exception unless guardrail/shared-client placement is approved |
3734
+
38-
+Decision: retain as temporary legacy exception.
35+
+## HTML Script References
3936
+
40-
+Reason:
37+
+- `toolbox/game-journey/index.html`
38+
+ - Current: `toolbox/game-journey/game-journey.js`
39+
+ - Proposed PR_034 value: `assets/toolbox/game-journey/js/index.js`
40+
+
41+
+## API Client References
42+
+
43+
+- `toolbox/game-journey/game-journey.js` imports:
44+
+ - `./game-journey-api-client.js`
45+
+ - `../tool-registry-api-client.js`
46+
+- After moving the entrypoint, imports should become:
47+
+ - `../../../../toolbox/game-journey/game-journey-api-client.js`
48+
+ - `../../../../toolbox/tool-registry-api-client.js`
4149
+
42-
+- `toolbox/assets/assets-api-client.js` is not Assets-only.
43-
+- `assets/toolbox/objects/js/index.js` imports `createAssetToolApiRepository` from the same file.
44-
+- Moving the client into `assets/toolbox/assets/js/` would either break Objects or require a shared-client migration that is outside this PR.
50+
+## Completion Metrics Risk
4551
+
46-
+Removal plan:
52+
+PR_26172_CHARLIE_006A determined that `/api/game-journey/completion-metrics` can return HTTP 500 when local legacy SQLite preservation protection is triggered by:
4753
+
48-
+1. Create a shared API-client placement rule for reused toolbox clients.
49-
+2. Move shared reusable code to the approved shared location.
50-
+3. Update Assets and Objects imports together.
51-
+4. Remove the legacy exception after both tools validate.
54+
+- `tmp/local-api/game-journey-completion-metrics.sqlite`
5255
+
53-
+### Assets Upload Worker
56+
+That behavior is intentional and prevents silent data loss. It is not caused by prior Charlie JS/CSS migrations. PR_034 validation must document this separately if it appears and must not misattribute the known HTTP 500 to the entrypoint move without direct evidence.
5457
+
55-
+Decision: retain as temporary legacy exception.
58+
+## Migration Decision
59+
+
60+
+### `game-journey.js`
61+
+
62+
+Decision: safe to migrate in PR_034 with targeted import and HTML updates.
5663
+
5764
+Reason:
5865
+
59-
+- `toolbox/assets/assets-upload-worker.js` is a module worker and is loaded directly by URL.
60-
+- The current canonical guardrail approves tool entrypoints at `assets/toolbox/{tool-name}/js/index.js`.
61-
+- It does not yet define an approved worker filename or worker subpath pattern.
62-
+- PR_031 validation confirmed the retained worker loads from `toolbox/assets/assets-upload-worker.js`.
66+
+- It is a single browser entrypoint.
67+
+- It is referenced directly by the Game Journey page.
68+
+- It can keep existing behavior by importing the retained API client and tool registry client through adjusted relative paths.
69+
+- No CSS move is required.
70+
+
71+
+### `game-journey-api-client.js`
6372
+
64-
+Removal plan:
73+
+Decision: do not move in PR_034. Review in PR_035.
74+
+
75+
+Reason:
6576
+
66-
+1. Add a canonical worker placement rule, or explicitly approve an Assets worker exception.
67-
+2. Move the worker only after the new placement is recognized by the guardrail.
68-
+3. Update `new Worker(...)` URL construction.
69-
+4. Run targeted upload validation before removing the legacy exception.
77+
+- The canonical guardrail currently approves the tool entrypoint path `assets/toolbox/{tool}/js/index.js`.
78+
+- Adding a second JS file under `assets/toolbox/game-journey/js/` would require either guardrail expansion or a folded/shared-client migration.
79+
+- The API client re-exports completion metrics helpers, so it should not be moved in the same PR as the entrypoint unless validation proves it is safe.
7080
+
71-
+## Validation Lane Report
81+
+## Validation Plan for PR_034
7282
+
83+
+- `node --check assets/toolbox/game-journey/js/index.js`
84+
+- `git diff --check`
7385
+- `npm run validate:canonical-structure`
74-
+ - Result: PASS
75-
+ - Blocking violations: 0
76-
+ - Approved legacy exceptions: 482
77-
+- `npm run validate:browser-env-agnostic`
78-
+ - Result: FAIL
79-
+ - Existing findings:
80-
+ - Local API product service contract findings in `src/dev-runtime/server/local-api-router.mjs`.
81-
+ - User-facing implementation wording findings in Messages.
82-
+ - Classification: existing broader browser-env gate findings, not caused by this report-only PR.
83-
+- Active reference check
84-
+ - Result: PASS
85-
+ - `toolbox/assets/assets-api-client.js` is referenced by Assets, Objects, guardrails, and related tests.
86-
+ - `toolbox/assets/assets-upload-worker.js` is referenced by the Assets canonical entrypoint, guardrail, and worker validation test.
86+
+- Active stale reference check for `toolbox/game-journey/game-journey.js`
87+
+- Targeted Game Journey Playwright validation, with any known completion-metrics legacy SQLite HTTP 500 documented separately.
8788
+
8889
+## Branch Validation
8990
+
@@ -94,21 +95,22 @@ index 000000000..44d4c753c
9495
+
9596
+## Requirement Checklist
9697
+
97-
+- Use PR_030 and PR_031 results: PASS
98-
+- Move Assets-only API client only if safe: PASS, retained because it is shared with Objects.
99-
+- Move worker only if safe: PASS, retained because canonical worker placement is undefined.
100-
+- Document approved temporary legacy exception with removal plan: PASS
101-
+- Do not create shared APIs unless clearly reused by multiple tools: PASS
102-
+- No behavior changes: PASS
103-
+- Run targeted Assets validation: PASS by PR_031 page/worker probe; full upload Playwright remains blocked by provider persistence failure.
104-
+- Run browser env validation: PASS with existing failure documented.
105-
+- Run canonical structure guardrail: PASS
106-
+- Confirm ZIP exists: PASS after artifact creation.
98+
+- Review `game-journey.js`: PASS
99+
+- Review `game-journey-api-client.js`: PASS
100+
+- Review `index.html`: PASS
101+
+- Review Game Journey tests: PASS
102+
+- Review Local API completion metrics route: PASS
103+
+- Review prior PR_006A findings: PASS
104+
+- Determine whether `game-journey.js` can safely move: PASS
105+
+- Determine whether API client should move, remain, or become shared: PASS
106+
+- Define validation that avoids confusing known SQLite preservation behavior with migration failure: PASS
107+
+- No implementation changes: PASS
108+
+- ZIP artifact exists: PASS after artifact creation.
107109
+
108110
+## Manual Validation Notes
109111
+
110-
+The safest current state is to keep the API client and worker in their approved legacy paths while the active entrypoint lives in the canonical path. This keeps Objects imports working and avoids inventing an unapproved worker path.
112+
+The next safe implementation step is a focused entrypoint move only. The API client should remain an exception until PR_035 because the guardrail does not currently allow a second canonical tool JS file.
111113
+
112114
+## Recommendation
113115
+
114-
+Continue to Game Journey audit PR_033. Open a later shared-client or worker-placement governance PR before moving the retained Assets exceptions.
116+
+Continue to PR_034 and migrate only `game-journey.js` to `assets/toolbox/game-journey/js/index.js`.

0 commit comments

Comments
 (0)