Skip to content

Commit 907a431

Browse files
author
DavidQ
committed
Preserve workspace tool enablement after returning from tools - PR_26130_004-workspace-return-tool-enable-regression
1 parent 74954cd commit 907a431

12 files changed

Lines changed: 442 additions & 57 deletions

File tree

docs/dev/codex_commands.md

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,31 @@
1-
# Codex Commands - PR_26130_003-session-restore-file-handle-guard
1+
# Codex Commands - PR_26130_004-workspace-return-tool-enable-regression
22

33
```text
44
codex
55
66
Changes:
7-
Create PR_26130_003-session-restore-file-handle-guard.
7+
Create PR_26130_004-workspace-return-tool-enable-regression.
88
Read docs/dev/PROJECT_INSTRUCTIONS.md first.
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.
9+
Fix Workspace Manager V2 regression where returning from any tool leaves all workspace tools disabled/gray.
10+
On return from a tool, preserve the selected repo, selected game, live repo folder handle, active game binding, and enabled tool list.
11+
Do not treat normal return navigation as Cancel, Close, or lost file handle.
12+
Only disable tools when no valid opened game/toolState is active or when repo binding is actually missing.
13+
Add explicit log lines for return-from-tool state restoration: repo selected, game selected, source binding status, enabled tool count.
14+
Keep Save disabled only when no live writable file handle exists.
15+
Keep scope limited to Workspace Manager V2 return state and tool enablement regression.
1916
No unrelated files.
2017
No start_of_day changes.
2118
2219
Validation:
2320
Run npm run test:workspace-v2.
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.
21+
Add/update Playwright test for opening a game, launching a tool, returning to workspace, and verifying tools remain enabled.
22+
Add/update Playwright test that Save state remains correct after return.
2523
Do not run full samples smoke test; document skipped reason.
2624
2725
Required reports:
2826
Create docs/dev/reports/codex_review.diff.
2927
Create docs/dev/reports/codex_changed_files.txt.
30-
Create docs/dev/reports/PR_26130_003-session-restore-file-handle-guard.md.
28+
Create docs/dev/reports/PR_26130_004-workspace-return-tool-enable-regression.md.
3129
Update docs/dev/codex_commands.md.
3230
Update docs/dev/commit_comment.txt.
3331
Produce required repo-structured ZIP under tmp/.
@@ -39,52 +37,63 @@ Produce required repo-structured ZIP under tmp/.
3937
Get-Content -Path "docs/dev/PROJECT_INSTRUCTIONS.md"
4038
Get-Content -Path ".codex/skills/repo-build/SKILL.md"
4139
git status --short --untracked-files=all
42-
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
40+
rg -n "returnToWorkspaceButton|expectWorkspaceRestoreRequiresRepoRebind|rebindRestoredWorkspace|expectWorkspaceReturnRehydrated|activeToolStateRequiresRepoHandle|workspace.repo.reference|FileSystemDirectoryHandle|handle" tests/playwright/tools/WorkspaceManagerV2.spec.mjs tools/workspace-manager-v2/js tools/preview-generator-v2 tools/asset-manager-v2/js -g "*.js" -g "*.mjs"
4741
node --check tools/workspace-manager-v2/js/WorkspaceManagerV2App.js
48-
node --check tools/workspace-manager-v2/js/controls/ToolTilesControl.js
4942
node --check tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js
43+
node --check tools/asset-manager-v2/js/AssetManagerV2App.js
44+
node --check tools/asset-manager-v2/js/services/WorkspaceBridge.js
45+
node --check tools/palette-manager-v2/main.js
46+
node --check tools/preview-generator-v2/PreviewGeneratorV2ShellControl.js
47+
node --check tools/session-inspector-v2/js/SessionInspectorV2App.js
5048
node --check tests/playwright/tools/WorkspaceManagerV2.spec.mjs
49+
npx playwright test tests/playwright/tools/WorkspaceManagerV2.spec.mjs --project=playwright --workers=1 --reporter=list -g "uses header lifecycle controls|keeps Preview Generator V2 repo writer|loads Gravity Well"
50+
npm run test:workspace-v2
5151
git diff --check
5252
git status --short --untracked-files=all
5353
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
5554
```
5655

5756
## Validation
5857

5958
`npm run test:workspace-v2` passed: 22 passed.
6059

60+
Focused rerun for the return-regression slices passed: 3 passed.
61+
6162
Syntax checks passed for:
6263

6364
- `tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
64-
- `tools/workspace-manager-v2/js/controls/ToolTilesControl.js`
6565
- `tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
66+
- `tools/asset-manager-v2/js/AssetManagerV2App.js`
67+
- `tools/asset-manager-v2/js/services/WorkspaceBridge.js`
68+
- `tools/palette-manager-v2/main.js`
69+
- `tools/preview-generator-v2/PreviewGeneratorV2ShellControl.js`
70+
- `tools/session-inspector-v2/js/SessionInspectorV2App.js`
6671
- `tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
6772

6873
`git diff --check` passed.
6974

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.
75+
Full samples smoke test skipped because this PR is limited to Workspace Manager V2 return state/tool enablement and does not modify shared sample loading, sample manifests, or broad sample runtime behavior.
7176

7277
## Playwright Impact
7378

7479
Playwright impacted: Yes.
7580

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.
81+
Workspace Manager V2 Playwright now validates opening a game, launching workspace tools, returning to Workspace Manager V2, keeping workspace tools enabled, preserving repo/game/source binding, and preserving clean/dirty Save and Close button state after return.
7782

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.
83+
Expected pass behavior: normal Return to Workspace restores the active repo, game, source binding, enabled tool count, and refreshed dirty toolState data without requiring Pick Repo Folder.
7984

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.
85+
Expected fail behavior: tests fail if return navigation drops the live repo handle, grays out tools, enables Save without a live binding, disables Save while dirty after a valid return, or omits the return-state log lines.
8186

8287
## Coverage
8388

8489
Playwright V8 coverage report generated by `npm run test:workspace-v2`:
8590

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`
91+
- `(63%) tools/asset-manager-v2/js/AssetManagerV2App.js - executed lines 643/643; executed functions 36/57`
92+
- `(76%) tools/preview-generator-v2/PreviewGeneratorV2ShellControl.js - executed lines 166/166; executed functions 13/17`
93+
- `(83%) tools/palette-manager-v2/main.js - executed lines 227/227; executed functions 15/18`
94+
- `(87%) tools/workspace-manager-v2/js/WorkspaceManagerV2App.js - executed lines 710/710; executed functions 39/45`
95+
- `(90%) tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js - executed lines 1458/1458; executed functions 136/151`
96+
- `(93%) tools/asset-manager-v2/js/services/WorkspaceBridge.js - executed lines 305/305; executed functions 25/27`
97+
- `(93%) tools/session-inspector-v2/js/SessionInspectorV2App.js - executed lines 309/309; executed functions 41/44`
8998

9099
`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-
Guard restored Workspace Manager V2 saves behind live repo handles - PR_26130_003-session-restore-file-handle-guard
1+
Preserve Workspace Manager V2 tool enablement after tool return - PR_26130_004-workspace-return-tool-enable-regression
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# PR_26130_004-workspace-return-tool-enable-regression
2+
3+
## Summary
4+
5+
- Fixed normal Return to Workspace navigation so it preserves Workspace Manager V2 active repo/game/toolState state instead of reopening as a missing-handle restore.
6+
- Workspace Manager V2 now stamps the active hostContextId into history before tool launch and records an explicit return marker.
7+
- Workspace-launched tools use the return marker to prefer browser history return, preserving the live page state when available.
8+
- If the browser reloads Workspace Manager on return, the active repo handle is restored from the explicit repo handle cache only for marked return navigation.
9+
- Direct sessionStorage restores without a return marker still remain read-only until the user picks the repo folder.
10+
- Return-state logging now reports repo selected, game selected, source binding status, and enabled tool count.
11+
12+
## Validation
13+
14+
- `node --check tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
15+
- `node --check tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
16+
- `node --check tools/asset-manager-v2/js/AssetManagerV2App.js`
17+
- `node --check tools/asset-manager-v2/js/services/WorkspaceBridge.js`
18+
- `node --check tools/palette-manager-v2/main.js`
19+
- `node --check tools/preview-generator-v2/PreviewGeneratorV2ShellControl.js`
20+
- `node --check tools/session-inspector-v2/js/SessionInspectorV2App.js`
21+
- `node --check tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
22+
- Focused return-regression rerun passed: 3 passed.
23+
- `git diff --check`
24+
- `npm run test:workspace-v2` passed: 22 passed.
25+
26+
Full samples smoke test skipped because this PR is limited to Workspace Manager V2 return state/tool enablement and does not modify shared sample loading, sample manifests, or broad sample runtime behavior.
27+
28+
## Playwright Coverage
29+
30+
Playwright impacted: Yes.
31+
32+
Coverage added/updated in `tests/playwright/tools/WorkspaceManagerV2.spec.mjs` validates:
33+
34+
- Opening a game, launching Session Inspector V2, and returning keeps Workspace Manager V2 tools enabled.
35+
- Returning after clean toolState keeps Save disabled and Close enabled.
36+
- Returning after dirty Palette Manager V2 or Asset Manager V2 changes keeps Save enabled and Close disabled.
37+
- Returning after Preview Generator V2 preserves the active repo/game/source binding and enabled tool list.
38+
- Gravity Well and Pong return paths keep their selected game bindings and tools enabled.
39+
- Direct sessionStorage restore without a return marker still requires repo folder selection before Save/tool launch.
40+
41+
Expected pass behavior: normal Return to Workspace restores the active repo, game, source binding, enabled tool count, and refreshed dirty toolState data without requiring Pick Repo Folder.
42+
43+
Expected fail behavior: tests fail if return navigation drops the live repo handle, grays out tools, enables Save without a live binding, disables Save while dirty after a valid return, or omits return-state log lines.
44+
45+
## Playwright V8 Coverage
46+
47+
- `(63%) tools/asset-manager-v2/js/AssetManagerV2App.js - executed lines 643/643; executed functions 36/57`
48+
- `(76%) tools/preview-generator-v2/PreviewGeneratorV2ShellControl.js - executed lines 166/166; executed functions 13/17`
49+
- `(83%) tools/palette-manager-v2/main.js - executed lines 227/227; executed functions 15/18`
50+
- `(87%) tools/workspace-manager-v2/js/WorkspaceManagerV2App.js - executed lines 710/710; executed functions 39/45`
51+
- `(90%) tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js - executed lines 1458/1458; executed functions 136/151`
52+
- `(93%) tools/asset-manager-v2/js/services/WorkspaceBridge.js - executed lines 305/305; executed functions 25/27`
53+
- `(93%) tools/session-inspector-v2/js/SessionInspectorV2App.js - executed lines 309/309; executed functions 41/44`
54+
55+
`tests/playwright/tools/WorkspaceManagerV2.spec.mjs` is a changed test file and is not collected as browser runtime coverage.
56+
57+
## Manual Test
58+
59+
1. Open Workspace Manager V2.
60+
2. Pick the repo folder and select Asteroids.
61+
3. Launch Session Inspector V2 and click Return to Workspace.
62+
4. Expected: Asteroids remains selected, Pick Repo Folder remains disabled, workspace tools remain enabled, Save remains disabled, Close remains enabled, and the status log includes return-state repo/game/source/enabled-count lines.
63+
5. Launch Palette Manager V2, add a swatch, and click Return to Workspace.
64+
6. Expected: workspace tools remain enabled, Save is enabled, Close is disabled, and the dirty Palette Manager V2 toolState is reflected in the tiles.
65+
7. Refresh Workspace Manager V2 directly with a hostContextId but without selecting a repo folder.
66+
8. Expected: Save remains disabled and Workspace Manager V2 requires Pick Repo Folder before Save/tool launch.
67+
68+
## Changed Files
69+
70+
- `tools/workspace-manager-v2/js/WorkspaceManagerV2App.js`
71+
- `tools/workspace-manager-v2/js/services/WorkspaceManagerV2ContextService.js`
72+
- `tools/asset-manager-v2/js/AssetManagerV2App.js`
73+
- `tools/asset-manager-v2/js/services/WorkspaceBridge.js`
74+
- `tools/palette-manager-v2/main.js`
75+
- `tools/preview-generator-v2/PreviewGeneratorV2ShellControl.js`
76+
- `tools/session-inspector-v2/js/SessionInspectorV2App.js`
77+
- `tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
78+
- `docs/dev/codex_commands.md`
79+
- `docs/dev/commit_comment.txt`
80+
- `docs/dev/reports/PR_26130_004-workspace-return-tool-enable-regression.md`
81+
- `docs/dev/reports/codex_review.diff`
82+
- `docs/dev/reports/codex_changed_files.txt`

playwright.config.cjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ module.exports = {
2121
use: {
2222
headless: false,
2323
launchOptions: {
24-
slowMo: 500
24+
slowMo: 100
2525
},
2626
trace: "on"
2727
}

0 commit comments

Comments
 (0)