Skip to content

Commit 82364f7

Browse files
committed
PR_26172_CHARLIE_042-canonical-tool-worker-placement
1 parent b6ecabc commit 82364f7

8 files changed

Lines changed: 262 additions & 92 deletions
File renamed without changes.

assets/toolbox/assets/js/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ const UPLOAD_ACCEPT_BY_ASSET_TYPE = Object.freeze({
7171
const DEFAULT_UPLOAD_CHUNK_SIZE_BYTES = 64 * 1024;
7272
const MAX_UPLOAD_CHUNK_SIZE_BYTES = 1024 * 1024;
7373
const SERVER_RECEIVE_PROGRESS_INTERVAL_MS = 1000;
74-
const UPLOAD_WORKER_URL = new URL("../../../../toolbox/assets/assets-upload-worker.js", import.meta.url);
74+
const UPLOAD_WORKER_URL = new URL("./assets-upload-worker.js", import.meta.url);
7575

7676
function isBrowserValidationHost() {
7777
return ["", "localhost", "127.0.0.1", "::1"].includes(window.location.hostname);
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# PR_26172_CHARLIE_042-canonical-tool-worker-placement
2+
3+
## Summary
4+
5+
PASS: The Assets upload worker was moved from the remaining legacy toolbox sidecar path into the canonical tool-local worker path:
6+
7+
- From: `toolbox/assets/assets-upload-worker.js`
8+
- To: `assets/toolbox/assets/js/assets-upload-worker.js`
9+
10+
The Assets tool entrypoint now constructs the module worker with `new URL("./assets-upload-worker.js", import.meta.url)`.
11+
12+
## Canonical Worker Rule Applied
13+
14+
PASS: Tool-local web workers may use:
15+
16+
`assets/toolbox/{tool-name}/js/{worker-name}.js`
17+
18+
For guardrail enforcement, the implemented validator allows `assets/toolbox/{tool-name}/js/*-worker.js` as a canonical tool-local worker sidecar while keeping general helper files outside `index.js` disallowed.
19+
20+
## Files Reviewed
21+
22+
- `docs_build/dev/ProjectInstructions/`
23+
- `project-instructions/addendums/canonical-repository-structure.md`
24+
- `project-instructions/addendums/legacy-migration-policy.md`
25+
- `assets/toolbox/assets/js/index.js`
26+
- `toolbox/assets/assets-upload-worker.js`
27+
- `scripts/validate-canonical-repository-structure.mjs`
28+
- `tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs`
29+
- `tests/playwright/tools/AssetToolMockRepository.spec.mjs`
30+
31+
## Files Changed
32+
33+
- `assets/toolbox/assets/js/assets-upload-worker.js`
34+
- `assets/toolbox/assets/js/index.js`
35+
- `scripts/validate-canonical-repository-structure.mjs`
36+
- `tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs`
37+
- `tests/playwright/tools/AssetToolMockRepository.spec.mjs`
38+
- `toolbox/assets/assets-upload-worker.js` removed by move
39+
40+
## Validation Commands
41+
42+
- PASS: `node --check assets/toolbox/assets/js/index.js`
43+
- PASS: `node --check assets/toolbox/assets/js/assets-upload-worker.js`
44+
- PASS: `git diff --check`
45+
- PASS: `npm run validate:canonical-structure`
46+
- PASS: `node --test tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs`
47+
- PASS: Targeted in-browser worker load probe:
48+
- loaded `/toolbox/assets/index.html`
49+
- fetched `/assets/toolbox/assets/js/assets-upload-worker.js`
50+
- constructed `new Worker("/assets/toolbox/assets/js/assets-upload-worker.js", { type: "module" })`
51+
- processed a 3-byte file to worker completion
52+
53+
## Diagnostic Validation Note
54+
55+
- FAIL, documented blocker: `npx playwright test tests/playwright/tools/AssetToolMockRepository.spec.mjs -g "Assets worker keeps UI responsive"` still reaches the worker and then reports `Batch upload complete: 0 written, 1 failed, 0 skipped, 0 warnings.`
56+
- This matches the previously documented Assets upload persistence/API blocker and is not evidence that the worker path failed. The focused worker-load probe confirms the moved worker loads and completes file processing.
57+
58+
## Branch Validation
59+
60+
- PASS: Current branch was `PR_26172_CHARLIE_repository-compliance-stack`.
61+
- PASS: Worktree was clean before PR_042 edits.
62+
- PASS: Local/origin sync was `0 0` before PR_042 edits.
63+
- PASS: Scope stayed within Team Charlie repository compliance and canonical structure work.
64+
65+
## Requirement Checklist
66+
67+
- PASS: Reviewed ProjectInstructions.
68+
- PASS: Reviewed canonical repository structure governance.
69+
- PASS: Defined safe canonical tool-local worker path.
70+
- PASS: Applied path only to Assets upload worker.
71+
- PASS: Updated Worker URL construction.
72+
- PASS: Preserved worker behavior.
73+
- PASS: No feature changes.
74+
- PASS: Updated canonical guardrail and targeted guardrail test.
75+
- PASS: Confirmed old active worker path is no longer referenced outside historical reports.
76+
- PASS: Created ZIP artifact under `tmp/`.
77+
78+
## Manual Validation Notes
79+
80+
- The upload worker is still an Assets-only file processing worker.
81+
- The worker move does not change upload payload shape, file chunking, progress messages, or completion/error message schema.
82+
- Browser environment validation was not run for PR_042 because browser environment validation rules were not changed.
83+
84+
## Recommendation
85+
86+
Continue to PR_043 final Charlie compliance re-audit. The only retained upload blocker is the pre-existing Assets persistence/API write failure documented in earlier Charlie reports.
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
A docs_build/dev/reports/PR_26172_CHARLIE_041-final-retained-exceptions-reaudit.md
1+
R100 toolbox/assets/assets-upload-worker.js assets/toolbox/assets/js/assets-upload-worker.js
2+
M assets/toolbox/assets/js/index.js
3+
A docs_build/dev/reports/PR_26172_CHARLIE_042-canonical-tool-worker-placement.md
4+
M scripts/validate-canonical-repository-structure.mjs
5+
M tests/playwright/tools/AssetToolMockRepository.spec.mjs
6+
M tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs
Lines changed: 162 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,106 +1,185 @@
1-
diff --git a/docs_build/dev/reports/PR_26172_CHARLIE_041-final-retained-exceptions-reaudit.md b/docs_build/dev/reports/PR_26172_CHARLIE_041-final-retained-exceptions-reaudit.md
1+
diff --git a/toolbox/assets/assets-upload-worker.js b/assets/toolbox/assets/js/assets-upload-worker.js
2+
similarity index 100%
3+
rename from toolbox/assets/assets-upload-worker.js
4+
rename to assets/toolbox/assets/js/assets-upload-worker.js
5+
diff --git a/assets/toolbox/assets/js/index.js b/assets/toolbox/assets/js/index.js
6+
index e5df5a3a7..d09e88a81 100644
7+
--- a/assets/toolbox/assets/js/index.js
8+
+++ b/assets/toolbox/assets/js/index.js
9+
@@ -71,7 +71,7 @@ const UPLOAD_ACCEPT_BY_ASSET_TYPE = Object.freeze({
10+
const DEFAULT_UPLOAD_CHUNK_SIZE_BYTES = 64 * 1024;
11+
const MAX_UPLOAD_CHUNK_SIZE_BYTES = 1024 * 1024;
12+
const SERVER_RECEIVE_PROGRESS_INTERVAL_MS = 1000;
13+
-const UPLOAD_WORKER_URL = new URL("../../../../toolbox/assets/assets-upload-worker.js", import.meta.url);
14+
+const UPLOAD_WORKER_URL = new URL("./assets-upload-worker.js", import.meta.url);
15+
16+
function isBrowserValidationHost() {
17+
return ["", "localhost", "127.0.0.1", "::1"].includes(window.location.hostname);
18+
diff --git a/docs_build/dev/reports/PR_26172_CHARLIE_042-canonical-tool-worker-placement.md b/docs_build/dev/reports/PR_26172_CHARLIE_042-canonical-tool-worker-placement.md
219
new file mode 100644
3-
index 000000000..0fa6d26cf
20+
index 000000000..86c635601
421
--- /dev/null
5-
+++ b/docs_build/dev/reports/PR_26172_CHARLIE_041-final-retained-exceptions-reaudit.md
6-
@@ -0,0 +1,100 @@
7-
+# PR_26172_CHARLIE_041-final-retained-exceptions-reaudit
22+
+++ b/docs_build/dev/reports/PR_26172_CHARLIE_042-canonical-tool-worker-placement.md
23+
@@ -0,0 +1,86 @@
24+
+# PR_26172_CHARLIE_042-canonical-tool-worker-placement
825
+
926
+## Summary
1027
+
11-
+Status: PASS with one retained owner-review exception.
28+
+PASS: The Assets upload worker was moved from the remaining legacy toolbox sidecar path into the canonical tool-local worker path:
1229
+
13-
+Final retained-exceptions status after PR_037 through PR_040:
30+
+- From: `toolbox/assets/assets-upload-worker.js`
31+
+- To: `assets/toolbox/assets/js/assets-upload-worker.js`
1432
+
15-
+- `toolbox/controls/controls-api-client.js`: resolved, moved to `assets/js/shared/controls-api-client.js`.
16-
+- `toolbox/assets/assets-api-client.js`: resolved, moved to `assets/js/shared/assets-api-client.js`.
17-
+- `toolbox/game-journey/game-journey-api-client.js`: resolved, moved to `assets/js/shared/game-journey-api-client.js`.
18-
+- `toolbox/assets/assets-upload-worker.js`: retained temporary exception, owner review required for canonical worker placement.
33+
+The Assets tool entrypoint now constructs the module worker with `new URL("./assets-upload-worker.js", import.meta.url)`.
1934
+
20-
+## Remaining Exception
35+
+## Canonical Worker Rule Applied
2136
+
22-
+| Path | Status | Blocker | Owner review item |
23-
+| --- | --- | --- | --- |
24-
+| `toolbox/assets/assets-upload-worker.js` | Temporary exception | Canonical governance does not define tool-local worker module placement. | Approve a worker path rule before moving. |
37+
+PASS: Tool-local web workers may use:
2538
+
26-
+## Migration Status
39+
+`assets/toolbox/{tool-name}/js/{worker-name}.js`
2740
+
28-
+| Original exception | Final status |
29-
+| --- | --- |
30-
+| `toolbox/controls/controls-api-client.js` | Migrated to shared |
31-
+| `toolbox/assets/assets-api-client.js` | Migrated to shared |
32-
+| `toolbox/assets/assets-upload-worker.js` | Retained exception |
33-
+| `toolbox/game-journey/game-journey-api-client.js` | Migrated to shared |
41+
+For guardrail enforcement, the implemented validator allows `assets/toolbox/{tool-name}/js/*-worker.js` as a canonical tool-local worker sidecar while keeping general helper files outside `index.js` disallowed.
3442
+
35-
+## Active Reference Audit
36-
+
37-
+Remaining active references from the original exception list:
43+
+## Files Reviewed
3844
+
45+
+- `docs_build/dev/ProjectInstructions/`
46+
+- `project-instructions/addendums/canonical-repository-structure.md`
47+
+- `project-instructions/addendums/legacy-migration-policy.md`
48+
+- `assets/toolbox/assets/js/index.js`
3949
+- `toolbox/assets/assets-upload-worker.js`
40-
+ - `assets/toolbox/assets/js/index.js`
41-
+ - `scripts/validate-canonical-repository-structure.mjs`
42-
+ - `tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs`
43-
+ - `tests/playwright/tools/AssetToolMockRepository.spec.mjs`
44-
+
45-
+Resolved shared client references:
46-
+
47-
+- `assets/js/shared/assets-api-client.js`
48-
+- `assets/js/shared/controls-api-client.js`
49-
+- `assets/js/shared/game-journey-api-client.js`
50-
+
51-
+## Blockers
52-
+
53-
+1. Assets upload worker placement:
54-
+ - The canonical structure currently defines `assets/toolbox/{tool-name}/js/index.js` and `assets/js/shared/`.
55-
+ - The worker is tool-specific, not shared.
56-
+ - Moving it safely requires an approved worker path rule.
57-
+
58-
+2. Assets upload HTTP 500:
59-
+ - PR_038 traced the failure to Local API persistence/provider behavior.
60-
+ - This is separate from worker placement and should be fixed in a persistence/API validation PR.
61-
+
62-
+3. Browser-env gate:
63-
+ - `npm run validate:browser-env-agnostic` still fails on existing Local API product service contract findings and Messages implementation wording.
64-
+ - These findings are outside the retained-exceptions migration scope.
65-
+
66-
+## Guardrail Status
67-
+
68-
+- `npm run validate:canonical-structure`
69-
+ - Result: PASS
70-
+ - Blocking violations: 0
71-
+ - Approved legacy exceptions: 478
72-
+- `node --test tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs`
73-
+ - Result: PASS
74-
+- `npm run validate:browser-env-agnostic`
75-
+ - Result: FAIL
76-
+ - Existing findings:
77-
+ - Product service contract findings in `src/dev-runtime/server/local-api-router.mjs`.
78-
+ - Messages user-facing implementation wording findings.
50+
+- `scripts/validate-canonical-repository-structure.mjs`
51+
+- `tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs`
52+
+- `tests/playwright/tools/AssetToolMockRepository.spec.mjs`
53+
+
54+
+## Files Changed
55+
+
56+
+- `assets/toolbox/assets/js/assets-upload-worker.js`
57+
+- `assets/toolbox/assets/js/index.js`
58+
+- `scripts/validate-canonical-repository-structure.mjs`
59+
+- `tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs`
60+
+- `tests/playwright/tools/AssetToolMockRepository.spec.mjs`
61+
+- `toolbox/assets/assets-upload-worker.js` removed by move
62+
+
63+
+## Validation Commands
64+
+
65+
+- PASS: `node --check assets/toolbox/assets/js/index.js`
66+
+- PASS: `node --check assets/toolbox/assets/js/assets-upload-worker.js`
67+
+- PASS: `git diff --check`
68+
+- PASS: `npm run validate:canonical-structure`
69+
+- PASS: `node --test tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs`
70+
+- PASS: Targeted in-browser worker load probe:
71+
+ - loaded `/toolbox/assets/index.html`
72+
+ - fetched `/assets/toolbox/assets/js/assets-upload-worker.js`
73+
+ - constructed `new Worker("/assets/toolbox/assets/js/assets-upload-worker.js", { type: "module" })`
74+
+ - processed a 3-byte file to worker completion
75+
+
76+
+## Diagnostic Validation Note
77+
+
78+
+- FAIL, documented blocker: `npx playwright test tests/playwright/tools/AssetToolMockRepository.spec.mjs -g "Assets worker keeps UI responsive"` still reaches the worker and then reports `Batch upload complete: 0 written, 1 failed, 0 skipped, 0 warnings.`
79+
+- This matches the previously documented Assets upload persistence/API blocker and is not evidence that the worker path failed. The focused worker-load probe confirms the moved worker loads and completes file processing.
7980
+
8081
+## Branch Validation
8182
+
82-
+- Current branch: `PR_26172_CHARLIE_repository-compliance-stack`
83-
+- Expected branch: `PR_26172_CHARLIE_repository-compliance-stack`
84-
+- Local/origin sync before PR: `0 0`
85-
+- Branch validation: PASS
83+
+- PASS: Current branch was `PR_26172_CHARLIE_repository-compliance-stack`.
84+
+- PASS: Worktree was clean before PR_042 edits.
85+
+- PASS: Local/origin sync was `0 0` before PR_042 edits.
86+
+- PASS: Scope stayed within Team Charlie repository compliance and canonical structure work.
8687
+
8788
+## Requirement Checklist
8889
+
89-
+- Final audit remaining exceptions: PASS
90-
+- Report migration status: PASS
91-
+- Report blockers: PASS
92-
+- Report owner review items: PASS
93-
+- Report guardrail status: PASS
94-
+- Do not merge: PASS
95-
+- Produce ZIP artifact: PASS after artifact creation.
90+
+- PASS: Reviewed ProjectInstructions.
91+
+- PASS: Reviewed canonical repository structure governance.
92+
+- PASS: Defined safe canonical tool-local worker path.
93+
+- PASS: Applied path only to Assets upload worker.
94+
+- PASS: Updated Worker URL construction.
95+
+- PASS: Preserved worker behavior.
96+
+- PASS: No feature changes.
97+
+- PASS: Updated canonical guardrail and targeted guardrail test.
98+
+- PASS: Confirmed old active worker path is no longer referenced outside historical reports.
99+
+- PASS: Created ZIP artifact under `tmp/`.
96100
+
97101
+## Manual Validation Notes
98102
+
99-
+The retained-exceptions workstream reduced the original target exception list from four active legacy files to one tool-specific worker exception. The remaining worker path should stay documented until the owner approves canonical worker placement.
100-
+
101-
+## Recommended Next PRs
102-
+
103-
+1. OWNER/Charlie worker placement governance for tool-local module workers.
104-
+2. Assets upload persistence/API validation recovery for `addAssetRecord` HTTP 500.
105-
+3. Assets worker migration after worker placement governance and upload persistence validation are complete.
106-
+4. Browser-env gate cleanup for Local API product service contract and Messages wording findings.
103+
+- The upload worker is still an Assets-only file processing worker.
104+
+- The worker move does not change upload payload shape, file chunking, progress messages, or completion/error message schema.
105+
+- Browser environment validation was not run for PR_042 because browser environment validation rules were not changed.
106+
+
107+
+## Recommendation
108+
+
109+
+Continue to PR_043 final Charlie compliance re-audit. The only retained upload blocker is the pre-existing Assets persistence/API write failure documented in earlier Charlie reports.
110+
diff --git a/scripts/validate-canonical-repository-structure.mjs b/scripts/validate-canonical-repository-structure.mjs
111+
index 5d963a1ec..e9296d04c 100644
112+
--- a/scripts/validate-canonical-repository-structure.mjs
113+
+++ b/scripts/validate-canonical-repository-structure.mjs
114+
@@ -6,7 +6,6 @@ import { fileURLToPath } from "node:url";
115+
const repoRoot = path.resolve(fileURLToPath(new URL("..", import.meta.url)));
116+
117+
export const APPROVED_LEGACY_JS_PATHS = Object.freeze(new Set([
118+
- "toolbox/assets/assets-upload-worker.js",
119+
"toolbox/game-hub/game-hub-api-client.js",
120+
"toolbox/game-hub/game-hub.js",
121+
"toolbox/messages/message-tts-service-registry.js",
122+
@@ -104,6 +103,7 @@ function record(severity, area, file, message, expected) {
123+
124+
function isCanonicalAssetJs(filePath) {
125+
return /^assets\/toolbox\/[^/]+\/js\/index\.js$/.test(filePath) ||
126+
+ /^assets\/toolbox\/[^/]+\/js\/[^/]+-worker\.js$/.test(filePath) ||
127+
filePath.startsWith("assets/js/shared/") ||
128+
filePath.startsWith("assets/theme-v2/js/");
129+
}
130+
@@ -123,8 +123,8 @@ function auditJavaScript(filePath) {
131+
"FAIL",
132+
"JS",
133+
filePath,
134+
- "JavaScript under assets must use assets/toolbox/{tool}/js/index.js, assets/js/shared/, or assets/theme-v2/js/.",
135+
- "assets/toolbox/{tool}/js/index.js or assets/js/shared/",
136+
+ "JavaScript under assets must use assets/toolbox/{tool}/js/index.js, assets/toolbox/{tool}/js/{worker-name}.js for tool-local workers, assets/js/shared/, or assets/theme-v2/js/.",
137+
+ "assets/toolbox/{tool}/js/index.js, assets/toolbox/{tool}/js/{worker-name}.js, or assets/js/shared/",
138+
);
139+
}
140+
if (filePath.startsWith("toolbox/")) {
141+
@@ -134,7 +134,7 @@ function auditJavaScript(filePath) {
142+
"JS",
143+
filePath,
144+
"Approved legacy toolbox JavaScript sidecar awaiting canonical migration.",
145+
- "assets/toolbox/{tool}/js/index.js or assets/js/shared/",
146+
+ "assets/toolbox/{tool}/js/index.js, assets/toolbox/{tool}/js/{worker-name}.js, or assets/js/shared/",
147+
);
148+
}
149+
return record(
150+
diff --git a/tests/playwright/tools/AssetToolMockRepository.spec.mjs b/tests/playwright/tools/AssetToolMockRepository.spec.mjs
151+
index 3cc244b26..6a3f6e56f 100644
152+
--- a/tests/playwright/tools/AssetToolMockRepository.spec.mjs
153+
+++ b/tests/playwright/tools/AssetToolMockRepository.spec.mjs
154+
@@ -817,7 +817,7 @@ test("Assets worker keeps UI responsive while server-received upload progress dr
155+
const workerPromise = page.waitForEvent("worker");
156+
await editRow.getByLabel("Upload File").setInputFiles(uploadFile("worker-progress.png", "image/png", Buffer.alloc(48, 7)));
157+
const worker = await workerPromise;
158+
- expect(worker.url()).toContain("toolbox/assets/assets-upload-worker.js");
159+
+ expect(worker.url()).toContain("assets/toolbox/assets/js/assets-upload-worker.js");
160+
161+
await expect(inlineProgress).toBeVisible();
162+
await expect(progressBar).toHaveJSProperty("value", 0);
163+
diff --git a/tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs b/tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs
164+
index 02029aed5..0d5f9fa27 100644
165+
--- a/tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs
166+
+++ b/tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs
167+
@@ -13,8 +13,8 @@ test("canonical repository structure guardrail accepts canonical paths and appro
168+
"assets/js/shared/dom.js",
169+
"assets/theme-v2/js/admin-system-health.js",
170+
"assets/theme-v2/css/theme.css",
171+
+ "assets/toolbox/assets/js/assets-upload-worker.js",
172+
"src/engine/rendering/Renderer.js",
173+
- "toolbox/assets/assets-upload-worker.js",
174+
"src/engine/ui/baseLayout.css",
175+
"tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs",
176+
"tests/runtime/V2SessionValidation.test.mjs",
177+
@@ -22,7 +22,7 @@ test("canonical repository structure guardrail accepts canonical paths and appro
178+
179+
assert.equal(result.status, "PASS");
180+
assert.equal(result.findings.length, 0);
181+
- assert.equal(result.legacy.length, 3);
182+
+ assert.equal(result.legacy.length, 2);
183+
});
184+
185+
test("canonical repository structure guardrail fails unapproved violation fixture paths", () => {

0 commit comments

Comments
 (0)