|
| 1 | +# PR_26172_CHARLIE_039-assets-api-worker-exception-resolution |
| 2 | + |
| 3 | +## Summary |
| 4 | + |
| 5 | +Status: PASS with worker exception retained. |
| 6 | + |
| 7 | +Resolved Assets retained exceptions using PR_037 and PR_038 findings: |
| 8 | + |
| 9 | +- Moved `toolbox/assets/assets-api-client.js` to `assets/js/shared/assets-api-client.js`. |
| 10 | +- Kept `toolbox/assets/assets-upload-worker.js` as a temporary exception with a removal plan. |
| 11 | + |
| 12 | +## Files Changed |
| 13 | + |
| 14 | +- `assets/js/shared/assets-api-client.js` |
| 15 | +- `assets/toolbox/assets/js/index.js` |
| 16 | +- `assets/toolbox/objects/js/index.js` |
| 17 | +- `scripts/validate-canonical-repository-structure.mjs` |
| 18 | +- `scripts/validate-browser-env-agnostic.mjs` |
| 19 | +- `tests/dev-runtime/ProductDataProviderContractHardening.test.mjs` |
| 20 | +- `docs_build/dev/reports/PR_26172_CHARLIE_039-assets-api-worker-exception-resolution.md` |
| 21 | +- `docs_build/dev/reports/codex_review.diff` |
| 22 | +- `docs_build/dev/reports/codex_changed_files.txt` |
| 23 | + |
| 24 | +## API Client Resolution |
| 25 | + |
| 26 | +Decision: migrated to shared. |
| 27 | + |
| 28 | +Reason: |
| 29 | + |
| 30 | +- `assets-api-client.js` is used by both Assets and Objects. |
| 31 | +- `assets/js/shared/` is the canonical shared JavaScript location. |
| 32 | +- The canonical structure guardrail already accepts files under `assets/js/shared/`. |
| 33 | + |
| 34 | +Updated imports: |
| 35 | + |
| 36 | +- `assets/toolbox/assets/js/index.js` |
| 37 | +- `assets/toolbox/objects/js/index.js` |
| 38 | + |
| 39 | +Updated validation references: |
| 40 | + |
| 41 | +- `scripts/validate-canonical-repository-structure.mjs` |
| 42 | +- `scripts/validate-browser-env-agnostic.mjs` |
| 43 | +- `tests/dev-runtime/ProductDataProviderContractHardening.test.mjs` |
| 44 | + |
| 45 | +## Worker Resolution |
| 46 | + |
| 47 | +Decision: keep temporary exception. |
| 48 | + |
| 49 | +Reason: |
| 50 | + |
| 51 | +- PR_038 confirmed the upload HTTP 500 is caused by Local API persistence/provider behavior, not by the worker path. |
| 52 | +- The worker loads successfully from `toolbox/assets/assets-upload-worker.js`. |
| 53 | +- Current canonical governance does not define an approved tool-local worker path. |
| 54 | +- Moving the worker to `assets/js/shared/` would be misleading because it is not shared. |
| 55 | + |
| 56 | +Removal plan: |
| 57 | + |
| 58 | +1. Add or approve a canonical worker path rule. |
| 59 | +2. Move the worker to that approved path. |
| 60 | +3. Update `new Worker(...)` URL construction in the Assets entrypoint. |
| 61 | +4. Run upload validation after the persistence/API blocker is resolved. |
| 62 | +5. Remove `toolbox/assets/assets-upload-worker.js` from guardrail exceptions. |
| 63 | + |
| 64 | +## Validation Lane Report |
| 65 | + |
| 66 | +- `node --check assets/js/shared/assets-api-client.js` |
| 67 | + - Result: PASS |
| 68 | +- `node --check assets/toolbox/assets/js/index.js` |
| 69 | + - Result: PASS |
| 70 | +- `node --check assets/toolbox/objects/js/index.js` |
| 71 | + - Result: PASS |
| 72 | +- `git diff --check` |
| 73 | + - Result: PASS |
| 74 | +- `npm run validate:canonical-structure` |
| 75 | + - Result: PASS |
| 76 | + - Blocking violations: 0 |
| 77 | + - Approved legacy exceptions: 480 |
| 78 | +- `node --test tests/regression/CanonicalRepositoryStructureGuardrail.test.mjs` |
| 79 | + - Result: PASS |
| 80 | +- Active stale reference check for `toolbox/assets/assets-api-client.js` |
| 81 | + - Result: PASS |
| 82 | + - No active non-report references remain. |
| 83 | +- Direct browser page-load probe: |
| 84 | + - Assets: PASS, heading rendered and shared client returned HTTP 200. |
| 85 | + - Objects: PASS, heading rendered and shared client returned HTTP 200. |
| 86 | +- `node --test tests/dev-runtime/ProductDataProviderContractHardening.test.mjs` |
| 87 | + - Result: FAIL |
| 88 | + - Existing unrelated failures: |
| 89 | + - Public config request order expectation. |
| 90 | + - Admin/tool votes import expectation. |
| 91 | + - The Assets client path list was updated in this test, but the remaining failures are outside this PR scope. |
| 92 | + |
| 93 | +## Branch Validation |
| 94 | + |
| 95 | +- Current branch: `PR_26172_CHARLIE_repository-compliance-stack` |
| 96 | +- Expected branch: `PR_26172_CHARLIE_repository-compliance-stack` |
| 97 | +- Local/origin sync before PR: `0 0` |
| 98 | +- Branch validation: PASS |
| 99 | + |
| 100 | +## Requirement Checklist |
| 101 | + |
| 102 | +- Use PR_037 findings: PASS |
| 103 | +- Use PR_038 findings: PASS |
| 104 | +- Resolve Assets API client placement: PASS |
| 105 | +- Resolve Assets worker placement: PASS, retained as temporary exception. |
| 106 | +- Migrate if safe: PASS, API client migrated. |
| 107 | +- Keep documented temporary exception if unsafe: PASS, worker retained. |
| 108 | +- Preserve upload behavior: PASS, no worker behavior changed. |
| 109 | +- No feature changes: PASS |
| 110 | +- Produce ZIP artifact: PASS after artifact creation. |
| 111 | + |
| 112 | +## Manual Validation Notes |
| 113 | + |
| 114 | +This PR removes one active Assets retained exception. The upload worker remains the only Assets-specific retained JS exception because the codebase needs an approved worker placement rule before a safe move. |
0 commit comments