Skip to content

Commit 74954cd

Browse files
author
DavidQ
committed
Guard restored workspace state until live repo file handle is rebound - PR_26130_003-session-restore-file-handle-guard
1 parent defb10f commit 74954cd

7 files changed

Lines changed: 325 additions & 49 deletions

File tree

docs/dev/codex_commands.md

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,33 @@
1-
# Codex Commands - PR_26130_002-save-source-binding-validation
1+
# Codex Commands - PR_26130_003-session-restore-file-handle-guard
22

33
```text
44
codex
55
66
Changes:
7-
Create PR_26130_002-save-source-binding-validation.
7+
Create PR_26130_003-session-restore-file-handle-guard.
88
Read docs/dev/PROJECT_INSTRUCTIONS.md first.
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.
9+
Fix Workspace Manager V2 restore behavior when no active repo folder handle exists.
10+
Do not restore/open an active game from sessionStorage as save-capable unless a live repo folder handle is present.
11+
If sessionStorage has active game context but no live repo folder handle:
12+
- show restored game details as read-only context only, or clear active game state
13+
- keep Save disabled
14+
- require repo folder selection before opening a save-capable game
15+
- log exact reason and required action
16+
After repo folder is picked, rebind active game to the real game.manifest.json source.
17+
Ensure Save writes to the actual file and validates by re-reading after write.
18+
Keep scope limited to Workspace Manager V2 restore/save binding.
2019
No unrelated files.
2120
No start_of_day changes.
2221
2322
Validation:
2423
Run npm run test:workspace-v2.
25-
Add/update Playwright tests for restored session save binding, actual file write validation, and failure logging when file source is missing.
24+
Add/update Playwright tests for sessionStorage restore without file handle, Save disabled until repo folder is selected, repo rebind after folder pick, and successful post-save validation.
2625
Do not run full samples smoke test; document skipped reason.
2726
2827
Required reports:
2928
Create docs/dev/reports/codex_review.diff.
3029
Create docs/dev/reports/codex_changed_files.txt.
31-
Create docs/dev/reports/PR_26130_002-save-source-binding-validation.md.
30+
Create docs/dev/reports/PR_26130_003-session-restore-file-handle-guard.md.
3231
Update docs/dev/codex_commands.md.
3332
Update docs/dev/commit_comment.txt.
3433
Produce required repo-structured ZIP under tmp/.
@@ -41,43 +40,51 @@ Get-Content -Path "docs/dev/PROJECT_INSTRUCTIONS.md"
4140
Get-Content -Path ".codex/skills/repo-build/SKILL.md"
4241
git status --short --untracked-files=all
4342
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
43+
rg --files "tools/workspace-manager-v2/js" | rg "Menu|menu|Repo|Selector|ToolTiles"
44+
rg -n "expectWorkspaceReturnRehydrated|returnToWorkspaceButton|saveWorkspaceButton|closeWorkspaceButton|cancelWorkspaceButton|pickRepoBtn|activeGameSelect" tests/playwright/tools/WorkspaceManagerV2.spec.mjs
45+
rg -n "bindGameManifestSourceForSave|writeActiveGameToolStateFile|restorePersistedContextById|restorePersistedContext" tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js
46+
npm run test:workspace-v2
4447
node --check tools/workspace-manager-v2/js/WorkspaceManagerV2App.js
48+
node --check tools/workspace-manager-v2/js/controls/ToolTilesControl.js
4549
node --check tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js
4650
node --check tests/playwright/tools/WorkspaceManagerV2.spec.mjs
4751
git diff --check
48-
npm run test:workspace-v2
49-
npm run test:workspace-v2
52+
git status --short --untracked-files=all
53+
git diff --stat
54+
git diff -- tools/workspace-manager-v2/js/WorkspaceManagerV2App.js tools/workspace-manager-v2/js/controls/ToolTilesControl.js tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js tests/playwright/tools/WorkspaceManagerV2.spec.mjs
5055
```
5156

5257
## Validation
5358

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.
59+
`npm run test:workspace-v2` passed: 22 passed.
5760

5861
Syntax checks passed for:
5962

6063
- `tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
64+
- `tools/workspace-manager-v2/js/controls/ToolTilesControl.js`
6165
- `tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
6266
- `tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
6367

6468
`git diff --check` passed.
6569

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.
70+
Full samples smoke test skipped because this PR is limited to Workspace Manager V2 restore/save binding and does not modify shared sample loading, sample manifests, or broad sample runtime behavior.
6771

6872
## Playwright Impact
6973

7074
Playwright impacted: Yes.
7175

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.
76+
Workspace Manager V2 Playwright now validates sessionStorage restore without a live repo folder handle, Save disabled until a repo folder is selected, active game dropdown locked while restored read-only context is shown, repo rebind after folder pick, successful post-save write/read-back validation, and missing-handle Save recovery logging.
7377

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.
78+
Expected pass behavior: restored sessionStorage toolState is read-only until repo selection, Save stays disabled without a live repo handle, repo selection rebinds the active game to `/games/<game>/game.manifest.json`, and Save writes/re-reads the actual manifest file before marking dirty toolState clean.
7579

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.
80+
Expected fail behavior: tests fail if restored sessionStorage context becomes save-capable without a live handle, Save writes only browser/session context, repo rebinding is skipped, tool launch is allowed before rebind, or post-save file validation/dirty-clean logging is missing.
7781

7882
## Coverage
7983

8084
Playwright V8 coverage report generated by `npm run test:workspace-v2`:
8185

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`
86+
- `(88%) tools/workspace-manager-v2/js/WorkspaceManagerV2App.js - executed lines 598/598; executed functions 36/41`
87+
- `(93%) tools/workspace-manager-v2/js/controls/ToolTilesControl.js - changed JS file with browser V8 coverage`
88+
- `(93%) tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js - changed JS file with browser V8 coverage`
89+
90+
`tests/playwright/tools/WorkspaceManagerV2.spec.mjs` is a changed test file and is not collected as browser runtime coverage.

docs/dev/commit_comment.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Bind restored Workspace Manager V2 saves to real game manifest sources - PR_26130_002-save-source-binding-validation
1+
Guard restored Workspace Manager V2 saves behind live repo handles - PR_26130_003-session-restore-file-handle-guard
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
# PR_26130_003-session-restore-file-handle-guard
2+
3+
## Summary
4+
5+
- Workspace Manager V2 now marks restored sessionStorage toolState context as read-only when no live repo folder handle exists.
6+
- Save stays disabled and tool launch is blocked until the user picks a repo folder.
7+
- Picking the repo folder while restored context is active rebinds the active game to the real discovered `game.manifest.json` source.
8+
- Save continues to write the actual game manifest file and validate by re-reading the file after write.
9+
- Missing live-handle Save attempts log the active source, context game fields, and required recovery action.
10+
11+
## Validation
12+
13+
- `node --check tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
14+
- `node --check tools/workspace-manager-v2/js/controls/ToolTilesControl.js`
15+
- `node --check tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
16+
- `node --check tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
17+
- `git diff --check`
18+
- `npm run test:workspace-v2` passed: 22 passed.
19+
20+
Full samples smoke test skipped because this PR is limited to Workspace Manager V2 restore/save binding and does not modify shared sample loading, sample manifests, or broad sample runtime behavior.
21+
22+
## Playwright Coverage
23+
24+
Playwright impacted: Yes.
25+
26+
Coverage added/updated in `tests/playwright/tools/WorkspaceManagerV2.spec.mjs` validates:
27+
28+
- Restoring active game/toolState context from sessionStorage without a live repo folder handle.
29+
- Save remains disabled until repo folder selection supplies a live handle.
30+
- Repo destination selection remains enabled and active game selection remains locked for restored read-only context.
31+
- Tool tiles remain blocked before rebind and show the required repo folder state.
32+
- Repo folder selection rebinds the restored active game to `/games/Asteroids/game.manifest.json`.
33+
- Save after rebind writes the actual mock repo `game.manifest.json`, re-reads it, validates `root.game.workspace`, and marks dirty toolState clean.
34+
- Missing-handle Save attempts log the exact missing handle/source and required action without writing browser-only fallback data.
35+
36+
Expected pass behavior: restored sessionStorage toolState is read-only until repo selection, Save stays disabled without a live handle, repo selection rebinds the active game to the real manifest source, and Save writes/re-reads the manifest file before clearing dirty state.
37+
38+
Expected fail behavior: tests fail if restored sessionStorage context becomes save-capable without a live repo handle, Save writes only persisted browser/session context, repo rebinding is skipped, tool launch is allowed before rebind, or post-save file validation/dirty-clean logging is missing.
39+
40+
## Playwright V8 Coverage
41+
42+
- `(88%) tools/workspace-manager-v2/js/WorkspaceManagerV2App.js - executed lines 598/598; executed functions 36/41`
43+
- `(93%) tools/workspace-manager-v2/js/controls/ToolTilesControl.js - changed JS file with browser V8 coverage`
44+
- `(93%) tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js - changed JS file with browser V8 coverage`
45+
46+
`tests/playwright/tools/WorkspaceManagerV2.spec.mjs` is a changed test file and is not collected as browser runtime coverage.
47+
48+
## Manual Test
49+
50+
1. Open Workspace Manager V2.
51+
2. Pick the repo folder and select Asteroids.
52+
3. Launch Palette Manager V2, add a swatch, and return to Workspace Manager V2.
53+
4. Expected: Workspace Manager V2 restores the Asteroids toolState as read-only, Save is disabled, Pick Repo Folder is enabled, and the status log explains that a live repo folder handle is required.
54+
5. Pick the repo folder again.
55+
6. Expected: the status log reports the restored toolState was rebound to `/games/Asteroids/game.manifest.json`; Save is enabled because the toolState is dirty.
56+
7. Click Save.
57+
8. Expected: status log shows source binding, saved path, file content/timestamp validation, schema/toolState validation, and dirty/clean validation.
58+
59+
## Changed Files
60+
61+
- `tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
62+
- `tools/workspace-manager-v2/js/controls/ToolTilesControl.js`
63+
- `tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
64+
- `tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
65+
- `docs/dev/codex_commands.md`
66+
- `docs/dev/commit_comment.txt`
67+
- `docs/dev/reports/PR_26130_003-session-restore-file-handle-guard.md`
68+
- `docs/dev/reports/codex_review.diff`
69+
- `docs/dev/reports/codex_changed_files.txt`

0 commit comments

Comments
 (0)