|
| 1 | +# PLAN_PR: PR_26175_CHARLIE_003-r2-storage-standardization |
| 2 | + |
| 3 | +Team: Charlie |
| 4 | + |
| 5 | +Purpose: Plan a scoped R2 storage configuration standardization PR around `GAMEFOUNDRY_STORAGE_PROJECTS_PREFIX` and the approved project storage prefixes. |
| 6 | + |
| 7 | +## Branch and Context Validation |
| 8 | + |
| 9 | +| Check | Expected | Actual | Status | |
| 10 | +|---|---|---|---| |
| 11 | +| Active branch | `PR_26172_CHARLIE_repository-compliance-stack` | `PR_26172_CHARLIE_repository-compliance-stack` | PASS | |
| 12 | +| Worktree before PLAN changes | clean | clean | PASS | |
| 13 | +| Local/origin sync before PLAN changes | `0 0` | `0 0` | PASS | |
| 14 | +| Active team | Team Charlie | Team Charlie | PASS | |
| 15 | +| Ownership scope | Infrastructure / Storage / System Health | R2 storage configuration standardization | PASS | |
| 16 | + |
| 17 | +## Scope |
| 18 | + |
| 19 | +Plan only for R2 storage configuration standardization. |
| 20 | + |
| 21 | +Approved standard: |
| 22 | + |
| 23 | +- Configuration variable: `GAMEFOUNDRY_STORAGE_PROJECTS_PREFIX` |
| 24 | +- Allowed project storage prefixes: |
| 25 | + - `/dev/projects/` |
| 26 | + - `/ist/projects/` |
| 27 | + - `/uat/projects/` |
| 28 | + - `/prod/projects/` |
| 29 | + |
| 30 | +Allowed APPLY/build scope: |
| 31 | + |
| 32 | +- Standardize project asset storage config validation around the approved prefix set. |
| 33 | +- Keep R2 as the project asset storage provider direction. |
| 34 | +- Keep object keys server-owned and under the configured prefix. |
| 35 | +- Keep prefix normalization for slash and backslash cleanup, then validate against the approved prefix set. |
| 36 | +- Keep safe status surfaces in Admin Infrastructure and Admin System Health aligned with the same allowed prefix set. |
| 37 | +- Preserve existing R2 list/read/write/delete connectivity behavior. |
| 38 | +- Preserve existing Postgres, Runtime Environment, Limits, Diagnostics Plan, and Diagnostics Log behavior. |
| 39 | +- Preserve secret masking and never return access key or secret key values. |
| 40 | + |
| 41 | +Explicitly out of scope: |
| 42 | + |
| 43 | +- Telemetry implementation. |
| 44 | +- Configurable runtime ports. |
| 45 | +- New storage provider abstraction. |
| 46 | +- Live R2 credential changes. |
| 47 | +- New persistence tables or database schema. |
| 48 | +- Backup prefix changes beyond checking for conflicts with project storage prefix rules. |
| 49 | +- Editing local ignored `.env` files. |
| 50 | +- Samples smoke. |
| 51 | + |
| 52 | +## Current Implementation Inventory |
| 53 | + |
| 54 | +Exact target files reviewed: |
| 55 | + |
| 56 | +- `src/dev-runtime/storage/storage-config.mjs` |
| 57 | +- `src/dev-runtime/storage/r2-project-asset-storage.mjs` |
| 58 | +- `src/dev-runtime/server/local-api-router.mjs` |
| 59 | +- `scripts/validate-storage-config.mjs` |
| 60 | +- `admin/infrastructure.html` |
| 61 | +- `assets/theme-v2/js/admin-infrastructure.js` |
| 62 | +- `admin/system-health.html` |
| 63 | +- `assets/theme-v2/js/admin-system-health.js` |
| 64 | +- `tests/playwright/tools/AdminPlatformToolsWireframes.spec.mjs` |
| 65 | + |
| 66 | +Current findings: |
| 67 | + |
| 68 | +- `storage-config.mjs` normalizes `GAMEFOUNDRY_STORAGE_PROJECTS_PREFIX` but currently accepts any non-empty normalized prefix. |
| 69 | +- `local-api-router.mjs` already has `STORAGE_PROJECTS_PREFIX_LANES` with the four approved prefixes and reports invalid paths in Infrastructure/System Health status. |
| 70 | +- `admin/infrastructure.html` lists `/dev/projects/`, `/ist/projects/`, `/uat/projects/`, and `/prd/projects/`. |
| 71 | +- `assets/theme-v2/js/admin-infrastructure.js` mirrors those four paths client-side for fallback/error rows. |
| 72 | +- `AdminPlatformToolsWireframes.spec.mjs` has focused coverage for missing, invalid, DEV, and IST path status and includes the full four-prefix list. |
| 73 | +- `scripts/validate-storage-config.mjs` validates configured R2 connectivity but depends on `loadStorageConfig()` for prefix acceptance. |
| 74 | +- `.env.example` includes `GAMEFOUNDRY_STORAGE_PROJECTS_PREFIX=` but does not currently enforce a value. |
| 75 | +- Tracked `.env.dev`, `.env.ist`, `.env.uat`, and `.env.prd` files were not present in this workspace when searched. Local ignored environment files must not be edited by this PR. |
| 76 | + |
| 77 | +## Proposed APPLY/Build Steps |
| 78 | + |
| 79 | +1. Add an exported approved project prefix list in `src/dev-runtime/storage/storage-config.mjs`, for example `STORAGE_PROJECTS_ALLOWED_PREFIXES`. |
| 80 | +2. Update `loadStorageConfig()` so `GAMEFOUNDRY_STORAGE_PROJECTS_PREFIX` is normalized first, then accepted only when it matches one of: |
| 81 | + - `/dev/projects/` |
| 82 | + - `/ist/projects/` |
| 83 | + - `/uat/projects/` |
| 84 | + - `/prod/projects/` |
| 85 | +3. Return a safe validation error when the prefix is missing or outside the approved list. |
| 86 | +4. Update `src/dev-runtime/server/local-api-router.mjs` to reuse the storage-config approved prefix list where practical, or keep its lane metadata explicitly synchronized if importing would create an unwanted dependency. |
| 87 | +5. Update `scripts/validate-storage-config.mjs` output so invalid prefixes report the approved values without printing secrets. |
| 88 | +6. Update `.env.example` comments near `GAMEFOUNDRY_STORAGE_PROJECTS_PREFIX` to document the approved values. Leave the value blank if templates should not default to a lane. |
| 89 | +7. Add or update targeted Node tests for `normalizeStorageProjectsPrefix()` and `loadStorageConfig()`: |
| 90 | + - accepts slash variants that normalize to approved prefixes; |
| 91 | + - rejects `/qa/projects/`; |
| 92 | + - rejects generic `/projects/`; |
| 93 | + - rejects empty values; |
| 94 | + - never exposes access key or secret key values in safe config. |
| 95 | +8. Update targeted Admin/System Health validation only if the payload shape changes. |
| 96 | +9. Produce required reports and repo-structured ZIP. |
| 97 | + |
| 98 | +## Status and Severity Rules |
| 99 | + |
| 100 | +Use existing status model: |
| 101 | + |
| 102 | +- `PASS`: configured prefix matches exactly one approved lane. |
| 103 | +- `WARN`: missing or incomplete storage configuration. |
| 104 | +- `FAIL`: malformed or unsafe storage configuration if the existing caller expects failure semantics. |
| 105 | +- `PENDING`: intentionally unwired future work only. |
| 106 | + |
| 107 | +For this PR, invalid project prefixes should remain visible and actionable. The recommended server-facing status is `WARN` unless an existing route/test already treats invalid prefix as `ERROR` and preserving that behavior is safer. Browser-facing status must not expose secret values. |
| 108 | + |
| 109 | +## Validation Plan |
| 110 | + |
| 111 | +Required targeted validation for APPLY/build: |
| 112 | + |
| 113 | +- `git diff --check` |
| 114 | +- `node --test tests/dev-runtime/StorageConfig.test.mjs` or equivalent new targeted storage-config test |
| 115 | +- `node --test tests/dev-runtime/AdminHealthOperations.test.mjs` |
| 116 | +- `npx playwright test tests/playwright/tools/AdminPlatformToolsWireframes.spec.mjs --grep "Infrastructure storage path status"` |
| 117 | +- `npx playwright test tests/playwright/tools/AdminHealthOperationsPage.spec.mjs` |
| 118 | +- `node scripts/validate-storage-config.mjs` only when `.env` is present and configured; otherwise document SKIP from script output. |
| 119 | + |
| 120 | +Required static checks: |
| 121 | + |
| 122 | +- Verify `GAMEFOUNDRY_STORAGE_PROJECTS_PREFIX` appears in config/docs/status surfaces. |
| 123 | +- Verify approved prefixes appear: |
| 124 | + - `/dev/projects/` |
| 125 | + - `/ist/projects/` |
| 126 | + - `/uat/projects/` |
| 127 | + - `/prod/projects/` |
| 128 | +- Verify no raw R2 credentials are printed in reports, tests, or status payloads. |
| 129 | + |
| 130 | +Skipped lanes: |
| 131 | + |
| 132 | +- Full samples smoke: skipped because scope is R2 storage config standardization only. |
| 133 | +- Broad Playwright: skipped unless targeted Admin/System Health validation fails. |
| 134 | +- Telemetry validation: skipped because telemetry is explicitly out of scope. |
| 135 | +- Runtime-port validation: skipped because configurable runtime ports are explicitly out of scope. |
| 136 | + |
| 137 | +## Required Reports for APPLY/Build |
| 138 | + |
| 139 | +- `docs_build/dev/reports/codex_review.diff` |
| 140 | +- `docs_build/dev/reports/codex_changed_files.txt` |
| 141 | +- `docs_build/dev/reports/PR_26175_CHARLIE_003-r2-storage-standardization.md` |
| 142 | +- `docs_build/dev/reports/PR_26175_CHARLIE_003-r2-storage-standardization-manual-validation-notes.md` |
| 143 | +- `docs_build/dev/reports/PR_26175_CHARLIE_003-r2-storage-standardization-instruction-compliance-checklist.md` |
| 144 | +- repo-structured delta ZIP under `tmp/` |
| 145 | + |
| 146 | +## PLAN_PR Result |
| 147 | + |
| 148 | +PLAN status: PASS. |
| 149 | + |
| 150 | +Recommendation: Proceed to BUILD/APPLY on the active Charlie stack branch if owner approves the scoped server-side prefix validation and targeted test additions. |
0 commit comments