Skip to content

Commit defb10f

Browse files
author
DavidQ
committed
Bind restored workspace saves to game manifest source and validate writes - PR_26130_002-save-source-binding-validation
1 parent 5421677 commit defb10f

6 files changed

Lines changed: 359 additions & 42 deletions

File tree

docs/dev/codex_commands.md

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,34 @@
1-
# Codex Commands - PR_26130_001-workspace-header-save-validation
1+
# Codex Commands - PR_26130_002-save-source-binding-validation
22

33
```text
44
codex
55
66
Changes:
7-
Create PR_26130_001-workspace-header-save-validation.
7+
Create PR_26130_002-save-source-binding-validation.
88
Read docs/dev/PROJECT_INSTRUCTIONS.md first.
9-
Remove Workspace header nav buttons that are no longer needed.
10-
Move Save, Close, and Cancel into the Workspace header nav.
11-
Fix Save so it writes the active game/toolState file and validates the file after write.
12-
After Save, log saved path, file size, item/count details, and validation result.
13-
Investigate whether repoPath is used anywhere. If used, document exact usage in the PR report. If unused, document that finding only.
14-
Keep scope limited to Workspace Manager V2 / Preview Generator V2 lifecycle and save validation.
9+
Fix Workspace Manager V2 save source binding.
10+
When restoring an active game from session context, rebind it to the discovered game.manifest.json source for that game.
11+
Save must write to the actual game.manifest.json file, not only persisted browser/session context.
12+
After write, re-read the file and validate that:
13+
- modified timestamp changed or file content changed
14+
- saved JSON validates
15+
- root game.workspace toolState exists
16+
- expected dirty/clean state was persisted
17+
If save cannot bind to a real game.manifest.json source, log the exact missing field/source and recovery action.
18+
Document whether repoPath is used and where.
19+
Keep scope limited to Workspace Manager V2 save/source binding.
1520
No unrelated files.
1621
No start_of_day changes.
1722
1823
Validation:
1924
Run npm run test:workspace-v2.
20-
Add/update Playwright tests for header Save/Close/Cancel placement, save write verification, dirty-state button behavior, and post-save log details.
25+
Add/update Playwright tests for restored session save binding, actual file write validation, and failure logging when file source is missing.
2126
Do not run full samples smoke test; document skipped reason.
2227
2328
Required reports:
2429
Create docs/dev/reports/codex_review.diff.
2530
Create docs/dev/reports/codex_changed_files.txt.
26-
Create docs/dev/reports/PR_26130_001-workspace-header-save-validation.md.
31+
Create docs/dev/reports/PR_26130_002-save-source-binding-validation.md.
2732
Update docs/dev/codex_commands.md.
2833
Update docs/dev/commit_comment.txt.
2934
Produce required repo-structured ZIP under tmp/.
@@ -34,32 +39,45 @@ Produce required repo-structured ZIP under tmp/.
3439
```powershell
3540
Get-Content -Path "docs/dev/PROJECT_INSTRUCTIONS.md"
3641
Get-Content -Path ".codex/skills/repo-build/SKILL.md"
37-
git status --short
38-
rg -n "repoPath" tools/workspace-manager-v2 tools/preview-generator-v2 tools/asset-manager-v2 tools/schemas games tests/playwright/tools/WorkspaceManagerV2.spec.mjs
39-
rg -n "exportWorkspaceManifest|importWorkspaceManifest|onExportManifest|onImportManifest|setExportEnabled|exportManifestButton|importManifest|activeGame(Save|Close|Cancel)Button|workspace-manager-v2__active-game-controls" tools/workspace-manager-v2 tests/playwright/tools/WorkspaceManagerV2.spec.mjs
40-
npm run test:workspace-v2
42+
git status --short --untracked-files=all
43+
rg -n "restoreWorkspaceFromSession|contextForSave|writeActiveGameToolStateFile|gameManifestPath|manifestPath|repoPath|manifestWrites|saveWorkspaceSession|dirtyPaletteToolState|Save" tools/workspace-manager-v2/js tests/playwright/tools/WorkspaceManagerV2.spec.mjs tools/preview-generator-v2/PreviewGeneratorV2App.js tools/asset-manager-v2/js/services/WorkspaceBridge.js tools/schemas games/Asteroids/game.manifest.json games/GravityWell/game.manifest.json games/Pong/game.manifest.json games/_template/workspace-manager-v2-UAT.manifest.json
44+
node --check tools/workspace-manager-v2/js/WorkspaceManagerV2App.js
45+
node --check tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js
46+
node --check tests/playwright/tools/WorkspaceManagerV2.spec.mjs
47+
git diff --check
4148
npm run test:workspace-v2
4249
npm run test:workspace-v2
4350
```
4451

4552
## Validation
4653

47-
`npm run test:workspace-v2` was attempted once with a 120 second command timeout and was cut off before Playwright returned a result.
54+
`npm run test:workspace-v2` passed: 21 passed.
55+
56+
After hardening missing repo-handle failures to use the source-binding recovery log, `npm run test:workspace-v2` was rerun and passed: 21 passed.
57+
58+
Syntax checks passed for:
59+
60+
- `tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
61+
- `tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
62+
- `tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
63+
64+
`git diff --check` passed.
65+
66+
Full samples smoke test skipped because this PR is limited to Workspace Manager V2 save/source binding and does not modify shared sample loading, sample manifests, or broad runtime sample behavior.
67+
68+
## Playwright Impact
69+
70+
Playwright impacted: Yes.
4871

49-
`npm run test:workspace-v2` was rerun with a longer timeout and passed: 19 passed.
72+
Workspace Manager V2 Playwright now validates restored-session Save rebinding to the discovered `game.manifest.json` source, actual file write/read-back validation, dirty payload persistence with clean toolState marking after Save, and exact recovery logging when a real file source cannot be bound.
5073

51-
After removing the now-unreachable import/export app methods, `npm run test:workspace-v2` was run again and passed: 19 passed.
74+
Expected pass behavior: Save writes the active toolState to the real `game.manifest.json`, re-reads the file, validates `game.workspace`, logs write/source validation, and marks dirty toolState keys clean.
5275

53-
Full samples smoke test skipped because this PR is limited to Workspace Manager V2 / Preview Generator V2 lifecycle controls and save validation, and does not modify sample manifests broadly, shared sample loading, or runtime sample smoke behavior.
76+
Expected fail behavior: the test fails if Save only updates browser/session context, loses the restored game manifest source, accepts an unchanged file, omits `game.workspace`, fails to mark toolState clean, or silently falls back when the file source is missing.
5477

55-
## Playwright Coverage
78+
## Coverage
5679

57-
Updated `tests/playwright/tools/WorkspaceManagerV2.spec.mjs` covers:
80+
Playwright V8 coverage report generated by `npm run test:workspace-v2`:
5881

59-
- Header Save, Close, and Cancel placement with Import/Export header actions removed.
60-
- Opened-game disabling for repo destination selection and the game dropdown.
61-
- Dirty-state lifecycle behavior: Save enabled and Close disabled while dirty; Save disabled and Close enabled after save.
62-
- Save write verification against the active `game.manifest.json` toolState file.
63-
- Post-save logs for saved path, file size, toolState item/count details, and validation result.
64-
- Close clearing clean toolState state.
65-
- Cancel warning before dirty toolState data is discarded.
82+
- `(88%) tools/workspace-manager-v2/js/WorkspaceManagerV2App.js - executed lines 532/532; executed functions 36/41`
83+
- `(93%) tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js - executed lines 1347/1347; executed functions 129/138`

docs/dev/commit_comment.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Move Workspace Manager V2 lifecycle controls into the header and validate active toolState saves - PR_26130_001-workspace-header-save-validation
1+
Bind restored Workspace Manager V2 saves to real game manifest sources - PR_26130_002-save-source-binding-validation
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# PR_26130_002-save-source-binding-validation
2+
3+
## Summary
4+
5+
- Added Workspace Manager V2 save source rebinding so restored session context can bind back to the real discovered `game.manifest.json` source before Save writes.
6+
- Save now writes through the active game manifest file handle, then re-reads the file and verifies content/timestamp change, valid JSON/schema, `root.game.workspace`, and exact saved workspace toolState.
7+
- Save logs source binding, write validation, schema/toolState validation, and dirty/clean validation.
8+
- Missing source binding now fails visibly with the exact active source, missing context field/source, and recovery action.
9+
10+
## Validation
11+
12+
- `node --check tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
13+
- `node --check tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
14+
- `node --check tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
15+
- `git diff --check`
16+
- `npm run test:workspace-v2` passed: 21 passed.
17+
- Final rerun after source-binding recovery log hardening passed: 21 passed.
18+
19+
Full samples smoke test skipped because this PR is limited to Workspace Manager V2 save/source binding and does not modify shared sample loading, sample manifests, or broad runtime sample behavior.
20+
21+
## Playwright Coverage
22+
23+
Playwright impacted: Yes.
24+
25+
Coverage added/updated in `tests/playwright/tools/WorkspaceManagerV2.spec.mjs` validates:
26+
27+
- Restored session Save rebinds from `sessionStorage:*` active game source to `/games/Asteroids/game.manifest.json`.
28+
- Save writes the actual mock repo `game.manifest.json` file and re-read validation logs file content changed.
29+
- Re-read JSON contains `root.game.workspace` and the dirty palette payload that triggered Save.
30+
- Save marks dirty toolState keys clean after file persistence.
31+
- Missing file source logs the exact source binding failure and recovery action without writing browser-only fallback data.
32+
33+
Expected pass behavior: Save writes the real active game manifest file, re-reads and validates it, logs source/write/schema/dirty-clean validation, and leaves dirty toolState clean.
34+
35+
Expected fail behavior: tests fail if Save writes only session context, keeps a `sessionStorage:*` game source, omits `game.workspace`, does not change file content/timestamp, or silently falls back when the repo/file source is missing.
36+
37+
## Playwright V8 Coverage
38+
39+
- `(88%) tools/workspace-manager-v2/js/WorkspaceManagerV2App.js - executed lines 532/532; executed functions 36/41`
40+
- `(93%) tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js - executed lines 1347/1347; executed functions 129/138`
41+
42+
## repoPath Usage
43+
44+
`repoPath` is used.
45+
46+
- `tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js:849`: `contextResultFromManifest` copies `workspaceManifest.repoPath || ""` onto active game metadata as `game.repoPath`.
47+
- `tools/preview-generator-v2/PreviewGeneratorV2App.js:1449`: workspace launch hydration reads `manifest.repoPath` with `normalizeAbsoluteRepoPath(manifest.repoPath)` when present.
48+
- `tools/asset-manager-v2/js/services/WorkspaceBridge.js:87`: workspace context extra-field filtering allows `repoPath` as a known root workspace field.
49+
- `tools/schemas/workspace.manifest.schema.json:49` and `tools/schemas/game.manifest.schema.json:136`: schemas define the `repoPath` field.
50+
- Current manifests persist `repoPath` in `games/Asteroids/game.manifest.json:27`, `games/GravityWell/game.manifest.json:27`, `games/Pong/game.manifest.json:27`, and `games/_template/workspace-manager-v2-UAT.manifest.json:12`.
51+
52+
## Manual Test
53+
54+
1. Open Workspace Manager V2.
55+
2. Pick the repo folder and select Asteroids.
56+
3. Launch Palette Manager V2, add a swatch, and return to Workspace Manager V2.
57+
4. Click Save.
58+
5. Expected: status log shows source binding to the Asteroids `game.manifest.json`, file content/timestamp validation, schema/toolState validation, and dirty/clean validation.
59+
6. Recovery path check: if Save has no active real repo/file source, expected status log identifies the missing source and instructs the user to cancel active toolState, pick the repo folder again, reopen the game, and retry Save.
60+
61+
## Changed Files
62+
63+
- `tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
64+
- `tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
65+
- `tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
66+
- `docs/dev/codex_commands.md`
67+
- `docs/dev/commit_comment.txt`
68+
- `docs/dev/reports/PR_26130_002-save-source-binding-validation.md`
69+
- `docs/dev/reports/codex_review.diff`
70+
- `docs/dev/reports/codex_changed_files.txt`

0 commit comments

Comments
 (0)