Skip to content

Commit 57646ab

Browse files
author
DavidQ
committed
Fix next failing tool from audit to enforce strict Workspace V2 contract - PR_11_326
1 parent 26ff7f1 commit 57646ab

6 files changed

Lines changed: 94 additions & 11 deletions

File tree

docs/dev/codex_commands.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,13 @@ PR_11_325
251251
npx @openai/codex run --model gpt-5.3-codex --reasoning medium "Implement PR_11_325: Fix the highest-impact failing Workspace V2 tool interaction from tool_completion_audit.md."
252252
```
253253

254+
---
255+
PR_11_326
256+
257+
```bash
258+
npx @openai/codex run --model gpt-5.3-codex --reasoning medium "Implement PR_11_326: Fix the next failing tool from tool_completion_audit.md."
259+
```
260+
254261
---
255262
PR_11_321
256263

docs/dev/commit_comment.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Fix Workspace V2 palette-manager launch/session contract handling and clear the top audit failure - PR 11.325
1+
Scope Asset Manager V2 persistence to explicit add/remove actions and remove passive render-time mutation writes - PR 11.326
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# PR_11_326 Report
2+
3+
## Scope
4+
- Single-tool fix only:
5+
- `tools/asset-manager-v2/index.js`
6+
- No schema changes.
7+
- No other tool changes.
8+
9+
## Targeted Audit Failure Addressed
10+
- Prior audit failure for `asset-manager-v2`:
11+
- Workspace integration/no payload mutation failed because persistence happened during normal render flow.
12+
- Fix applied:
13+
- passive load/render now does not persist
14+
- persistence occurs only on explicit add/remove actions
15+
16+
## Files Changed
17+
- `tools/asset-manager-v2/index.js`
18+
- `docs/pr/PR_11_326_ASSET_MANAGER_MUTATION_SCOPE_FIX/PLAN_PR.md`
19+
- `docs/pr/PR_11_326_ASSET_MANAGER_MUTATION_SCOPE_FIX/BUILD_PR.md`
20+
- `docs/dev/reports/PR_11_326_report.md`
21+
- `docs/dev/codex_commands.md`
22+
- `docs/dev/commit_comment.txt`
23+
24+
## Validation Run
25+
- `node --check tools/asset-manager-v2/index.js` -> PASS
26+
- `node tests/runtime/V2AssetManagerWorkspacePersistence.test.mjs` -> PASS
27+
- `npm run test:workspace-v2` -> PASS (`1 passed`, `failed=0`)
28+
29+
## Notes
30+
- This PR intentionally keeps launch behavior unchanged (sample/workspace paths remain intact).
31+
- No fallback/default data paths were added.
32+
- No schema or cross-tool contract changes were introduced.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# BUILD_PR_11_326
2+
3+
## Implementation
4+
- Updated `tools/asset-manager-v2/index.js` only.
5+
- Scoped session persistence to explicit mutation paths:
6+
- `addAssetEntry` -> `loadContract(nextSessionContext, true)`
7+
- `removeAssetEntryById` -> `loadContract(nextSessionContext, true)`
8+
- Updated passive load path to avoid implicit persistence:
9+
- `readSession` -> `loadContract(parsedSession, false)`
10+
- Updated contract/render pipeline:
11+
- `loadContract(sessionContext, persistToWorkspace = false)`
12+
- `renderCatalog(assetCatalog, sessionContext, persistToWorkspace = false)`
13+
- persistence readout now reflects passive load vs explicit mutation persistence.
14+
15+
## Validation
16+
- `node --check tools/asset-manager-v2/index.js`
17+
- `node tests/runtime/V2AssetManagerWorkspacePersistence.test.mjs`
18+
- `npm run test:workspace-v2`
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# PLAN_PR_11_326
2+
3+
## Purpose
4+
Fix the next failing tool from the audit (`asset-manager-v2`) by removing unintended passive payload mutation while preserving launch and explicit add/remove behavior.
5+
6+
## Scope
7+
- `tools/asset-manager-v2/index.js`
8+
- `docs/pr/PR_11_326_ASSET_MANAGER_MUTATION_SCOPE_FIX/PLAN_PR.md`
9+
- `docs/pr/PR_11_326_ASSET_MANAGER_MUTATION_SCOPE_FIX/BUILD_PR.md`
10+
- `docs/dev/reports/PR_11_326_report.md`
11+
- `docs/dev/codex_commands.md`
12+
- `docs/dev/commit_comment.txt`
13+
14+
## Steps
15+
1. Keep valid/invalid JSON contract handling unchanged.
16+
2. Ensure workspace persistence is not performed during passive render/load.
17+
3. Keep persistence tied to explicit asset modification actions.
18+
4. Validate:
19+
- syntax check for changed tool file
20+
- target tool persistence behavior check
21+
- Workspace V2 Playwright gate

tools/asset-manager-v2/index.js

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ class AssetBrowserV2 {
211211
}
212212
const nextSessionContext = this.cloneSessionValue(this.currentSessionContext);
213213
nextSessionContext.payloadJson.assetCatalog.entries.push(newEntry.entry);
214-
this.loadContract(nextSessionContext);
214+
this.loadContract(nextSessionContext, true);
215215
this.clearAddAssetForm();
216216
this.setActionStatus(`Asset '${newEntry.entry.id}' added.`);
217217
}
@@ -245,7 +245,7 @@ class AssetBrowserV2 {
245245
this.setActionStatus(`Remove blocked. Asset id '${normalizedId}' was not found.`);
246246
return;
247247
}
248-
this.loadContract(nextSessionContext);
248+
this.loadContract(nextSessionContext, true);
249249
this.setActionStatus(`Asset '${normalizedId}' removed.`);
250250
}
251251

@@ -279,7 +279,8 @@ class AssetBrowserV2 {
279279
return;
280280
}
281281
this.loadContract(
282-
JSON.parse(serializedSession)
282+
JSON.parse(serializedSession),
283+
false
283284
);
284285
} catch (error) {
285286
const runtimeMessage = `Unable to read Asset Manager V2 session context: ${error instanceof Error ? error.message : "unknown error"}`;
@@ -288,7 +289,7 @@ class AssetBrowserV2 {
288289
}
289290
}
290291

291-
loadContract(sessionContext) {
292+
loadContract(sessionContext, persistToWorkspace = false) {
292293
console.log("[ASSET_BROWSER_V2_CONTRACT_LOADED]");
293294
if (!sessionContext || typeof sessionContext !== "object" || Array.isArray(sessionContext)) {
294295
this.renderError("Session context is invalid. Expected an object containing payloadJson.assetCatalog.");
@@ -315,7 +316,7 @@ class AssetBrowserV2 {
315316
this.renderError("Asset Manager V2 session data is invalid. Remove payloadJson.importName/importDestination. Load assets from payloadJson.assetCatalog only.");
316317
return;
317318
}
318-
this.renderCatalog(versionCheck.payload.payloadJson.assetCatalog, versionCheck.payload);
319+
this.renderCatalog(versionCheck.payload.payloadJson.assetCatalog, versionCheck.payload, persistToWorkspace);
319320
}
320321

321322
persistValidSessionForWorkspace(sessionContext, assetCatalog) {
@@ -355,7 +356,7 @@ class AssetBrowserV2 {
355356
return { ok: true, message: "" };
356357
}
357358

358-
renderCatalog(assetCatalog, sessionContext) {
359+
renderCatalog(assetCatalog, sessionContext, persistToWorkspace = false) {
359360
if (typeof assetCatalog.name !== "string" || !assetCatalog.name.trim()) {
360361
this.renderError("Asset Manager V2 session data is invalid. Expected assetCatalog.name.");
361362
return;
@@ -386,10 +387,14 @@ class AssetBrowserV2 {
386387

387388
document.getElementById("assetBrowserV2SessionReadout").textContent = `Session: loaded\nContext: ${this.urlState.hostContextId}\nTool: ${typeof sessionContext.toolId === "string" && sessionContext.toolId.trim() ? sessionContext.toolId.trim() : "not provided"}${this.optionalUrlStateSummary() ? `\nURL State: ${this.optionalUrlStateSummary()}` : ""}`;
388389
document.getElementById("assetBrowserV2ContractReadout").textContent = "payloadJson loaded\npayloadJson.assetCatalog valid\nentries[] valid";
389-
const persistence = this.persistValidSessionForWorkspace(sessionContext, assetCatalog);
390-
document.getElementById("assetBrowserV2WorkspaceReadout").textContent = persistence.ok
391-
? "Workspace session context was read and persisted for Workspace V2 export."
392-
: `Workspace session context was read but could not be persisted: ${persistence.message}`;
390+
if (persistToWorkspace) {
391+
const persistence = this.persistValidSessionForWorkspace(sessionContext, assetCatalog);
392+
document.getElementById("assetBrowserV2WorkspaceReadout").textContent = persistence.ok
393+
? "Workspace session context was read and persisted for Workspace V2 export."
394+
: `Workspace session context was read but could not be persisted: ${persistence.message}`;
395+
} else {
396+
document.getElementById("assetBrowserV2WorkspaceReadout").textContent = "Workspace session context loaded. Changes persist only after Add or Remove actions.";
397+
}
393398
document.getElementById("assetBrowserV2Title").textContent = assetCatalog.name.trim();
394399
document.getElementById("assetBrowserV2Count").textContent = `${assetCatalog.entries.length} asset${assetCatalog.entries.length === 1 ? "" : "s"}`;
395400
document.getElementById("assetBrowserV2State").textContent = assetCatalog.entries.length === 0

0 commit comments

Comments
 (0)