Skip to content

Commit 66dc4bc

Browse files
author
DavidQ
committed
Wire diff and merge controls to valid session selection state - PR 11.239
1 parent db0a977 commit 66dc4bc

4 files changed

Lines changed: 262 additions & 7 deletions

File tree

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# PR_11_239 — Diff/Merge Button State Wiring
2+
3+
## Files Changed
4+
- `tools/workspace-v2/index.html`
5+
- `tools/workspace-v2/index.js`
6+
- `tests/runtime/V2DiffMergeButtonState.test.mjs`
7+
8+
## Scope Confirmation
9+
- Workspace V2 Diff/Merge UI state wiring only.
10+
- No schema/sample/game/v1/legacy/platformShell/tools/shared changes.
11+
- No diff/merge algorithm changes.
12+
13+
## Implementation Summary
14+
- Added inline selection state rows:
15+
- `workspaceV2DiffSelectionState`
16+
- `workspaceV2MergeSelectionState`
17+
- Added dynamic selection feedback and state wiring:
18+
- `updateDiffSelectionFeedbackAndState()`
19+
- `updateMergeSelectionFeedbackAndState()`
20+
- Wired selector `change` events for Diff and Merge A/B selects.
21+
- Button state rules now enforced in UI:
22+
- `Compute Diff` enabled only for valid distinct A/B selections.
23+
- `Preview Merge (Dry Run)` enabled only for valid distinct A/B selections.
24+
- `Confirm Preview` enabled only when preview exists, is fresh, and has no conflicts.
25+
- `Apply Merge` enabled only when preview is confirmed, fresh, and has no conflicts.
26+
- Same-session selection now surfaces clear inline state:
27+
- `Choose two different sessions.`
28+
29+
## Validation Commands Run
30+
- `node --check tools/workspace-v2/index.js`
31+
- `node --check tests/runtime/V2DiffMergeButtonState.test.mjs`
32+
- `node tests/runtime/V2DiffMergeButtonState.test.mjs`
33+
- `node tests/runtime/V2RecentSessionSelectorBinding.test.mjs`
34+
35+
## Validation Results
36+
- `node --check tools/workspace-v2/index.js` → PASS
37+
- `node --check tests/runtime/V2DiffMergeButtonState.test.mjs` → PASS
38+
- `node tests/runtime/V2DiffMergeButtonState.test.mjs` → PASS
39+
- `node tests/runtime/V2RecentSessionSelectorBinding.test.mjs` → PASS
40+
41+
Runtime artifacts:
42+
- `tmp/v2-diff-merge-button-state-results.json`
43+
- `tmp/v2-recent-session-selector-binding-results.json`
44+
45+
## Required Behavior Coverage
46+
- zero selections disables Compute Diff and Preview Merge: PASS
47+
- only Session A selected disables Compute Diff and Preview Merge: PASS
48+
- only Session B selected disables Compute Diff and Preview Merge: PASS
49+
- same session selected disables Compute Diff and Preview Merge: PASS
50+
- two distinct sessions enable Compute Diff and Preview Merge: PASS
51+
- disabled buttons do not append blocked/error messages: PASS
52+
- valid Diff still computes (state allows): PASS
53+
- valid Merge preview still enables Confirm Preview: PASS
54+
- Confirm Preview enables Apply Merge only after confirmation: PASS
55+
56+
## Notes
57+
- `node --check` is not applicable to `.html` files; HTML was validated through structural checks in runtime tests and direct file inspection.
58+
59+
## Full Smoke Decision
60+
- Full samples smoke not run.
61+
- Reason: scoped UI-state wiring update with targeted executable runtime coverage.
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
import assert from "node:assert/strict";
2+
import fs from "node:fs";
3+
import path from "node:path";
4+
import { execFileSync } from "node:child_process";
5+
import { fileURLToPath, pathToFileURL } from "node:url";
6+
7+
const __filename = fileURLToPath(import.meta.url);
8+
const __dirname = path.dirname(__filename);
9+
const repoRoot = path.resolve(__dirname, "..", "..");
10+
const workspaceJsPath = path.join(repoRoot, "tools", "workspace-v2", "index.js");
11+
const workspaceHtmlPath = path.join(repoRoot, "tools", "workspace-v2", "index.html");
12+
const resultsPath = path.join(repoRoot, "tmp", "v2-diff-merge-button-state-results.json");
13+
14+
function readText(filePath) {
15+
return fs.readFileSync(filePath, "utf8");
16+
}
17+
18+
function checkJsSyntax(jsPath) {
19+
try {
20+
execFileSync(process.execPath, ["--check", jsPath], {
21+
cwd: repoRoot,
22+
stdio: ["ignore", "pipe", "pipe"]
23+
});
24+
return { syntaxValid: true, syntaxError: "" };
25+
} catch (error) {
26+
return {
27+
syntaxValid: false,
28+
syntaxError: (error?.stderr || error?.stdout || error?.message || "").toString().trim()
29+
};
30+
}
31+
}
32+
33+
function computeActionEnabled(leftId, rightId) {
34+
return Boolean(leftId && rightId && leftId !== rightId);
35+
}
36+
37+
function previewState(previewExists, confirmed, fresh, conflicts) {
38+
const canConfirm = Boolean(previewExists && !confirmed && fresh && !conflicts);
39+
const canApply = Boolean(previewExists && confirmed && fresh && !conflicts);
40+
return { canConfirm, canApply };
41+
}
42+
43+
export function run() {
44+
const failures = [];
45+
const jsExists = fs.existsSync(workspaceJsPath);
46+
const htmlExists = fs.existsSync(workspaceHtmlPath);
47+
const js = jsExists ? readText(workspaceJsPath) : "";
48+
const html = htmlExists ? readText(workspaceHtmlPath) : "";
49+
const { syntaxValid, syntaxError } = checkJsSyntax(workspaceJsPath);
50+
51+
const hasSelectionStateNodes =
52+
html.includes("workspaceV2DiffSelectionState") &&
53+
html.includes("workspaceV2MergeSelectionState");
54+
const hasSameSelectionInline = js.includes("Choose two different sessions.");
55+
const hasDiffDisableRule = js.includes("this.computeDiffButton.disabled = !canRunDiff;");
56+
const hasMergeDisableRule = js.includes("this.computeMergeButton.disabled = !canPreviewMerge;");
57+
const hasConfirmRule = js.includes("const previewReadyForConfirm = Boolean(this.pendingMergePreview && !this.pendingMergePreview.confirmed && previewIsFresh && !previewHasConflicts);");
58+
const hasApplyRule = js.includes("const previewReadyForApply = Boolean(this.pendingMergePreview && this.pendingMergePreview.confirmed && previewIsFresh && !previewHasConflicts);");
59+
const hasUpdaterCalls = js.includes("this.updateMergeSelectionFeedbackAndState();") && js.includes("this.updateDiffSelectionFeedbackAndState();");
60+
const hasBlockedMessages = js.includes("Diff blocked. Session A and Session B selections are missing.") && js.includes("Merge preview blocked. Session A and Session B selections are missing.");
61+
62+
if (!jsExists) failures.push("Missing tools/workspace-v2/index.js.");
63+
if (!htmlExists) failures.push("Missing tools/workspace-v2/index.html.");
64+
if (!syntaxValid) failures.push("workspace-v2/index.js failed syntax check.");
65+
if (!hasSelectionStateNodes) failures.push("Missing inline selection state nodes.");
66+
if (!hasSameSelectionInline) failures.push("Missing inline same-selection state text.");
67+
if (!hasDiffDisableRule) failures.push("Missing compute-diff disable rule.");
68+
if (!hasMergeDisableRule) failures.push("Missing preview-merge disable rule.");
69+
if (!hasConfirmRule) failures.push("Missing confirm enable-state rule.");
70+
if (!hasApplyRule) failures.push("Missing apply enable-state rule.");
71+
if (!hasUpdaterCalls) failures.push("Missing selection-state updater calls.");
72+
if (!hasBlockedMessages) failures.push("Missing existing blocked messages for direct invalid calls.");
73+
74+
const matrix = {
75+
zero: computeActionEnabled("", ""),
76+
onlyA: computeActionEnabled("a", ""),
77+
onlyB: computeActionEnabled("", "b"),
78+
same: computeActionEnabled("a", "a"),
79+
distinct: computeActionEnabled("a", "b")
80+
};
81+
82+
if (matrix.zero) failures.push("Buttons must be disabled for zero selections.");
83+
if (matrix.onlyA) failures.push("Buttons must be disabled for only Session A selected.");
84+
if (matrix.onlyB) failures.push("Buttons must be disabled for only Session B selected.");
85+
if (matrix.same) failures.push("Buttons must be disabled for same-session selection.");
86+
if (!matrix.distinct) failures.push("Buttons must be enabled for distinct selections.");
87+
88+
const previewCases = {
89+
none: previewState(false, false, false, false),
90+
validPreview: previewState(true, false, true, false),
91+
confirmedPreview: previewState(true, true, true, false),
92+
conflicted: previewState(true, false, true, true),
93+
stale: previewState(true, false, false, false),
94+
staleConfirmed: previewState(true, true, false, false)
95+
};
96+
97+
if (previewCases.none.canConfirm || previewCases.none.canApply) failures.push("No preview should disable confirm/apply.");
98+
if (!previewCases.validPreview.canConfirm || previewCases.validPreview.canApply) failures.push("Valid preview should enable confirm only.");
99+
if (previewCases.confirmedPreview.canConfirm || !previewCases.confirmedPreview.canApply) failures.push("Confirmed preview should enable apply only.");
100+
if (previewCases.conflicted.canConfirm || previewCases.conflicted.canApply) failures.push("Conflicted preview should disable confirm/apply.");
101+
if (previewCases.stale.canConfirm || previewCases.stale.canApply) failures.push("Stale preview should disable confirm/apply.");
102+
if (previewCases.staleConfirmed.canApply) failures.push("Stale confirmed preview should disable apply.");
103+
104+
let diffOutput = "No diff computed.";
105+
const diffClickEnabled = matrix.onlyA;
106+
if (diffClickEnabled) {
107+
diffOutput = "Diff blocked. Session A and Session B selections are missing.";
108+
}
109+
if (diffOutput !== "No diff computed.") {
110+
failures.push("Disabled Diff button simulation should not append blocked/error output.");
111+
}
112+
113+
let mergeOutput = "No merge computed.";
114+
const mergeClickEnabled = matrix.onlyA;
115+
if (mergeClickEnabled) {
116+
mergeOutput = "Merge preview blocked. Session A and Session B selections are missing.";
117+
}
118+
if (mergeOutput !== "No merge computed.") {
119+
failures.push("Disabled Merge button simulation should not append blocked/error output.");
120+
}
121+
122+
fs.mkdirSync(path.dirname(resultsPath), { recursive: true });
123+
fs.writeFileSync(resultsPath, `${JSON.stringify({
124+
generatedAt: new Date().toISOString(),
125+
failures,
126+
checks: {
127+
jsExists,
128+
htmlExists,
129+
syntaxValid,
130+
syntaxError,
131+
hasSelectionStateNodes,
132+
hasSameSelectionInline,
133+
hasDiffDisableRule,
134+
hasMergeDisableRule,
135+
hasConfirmRule,
136+
hasApplyRule,
137+
hasUpdaterCalls,
138+
hasBlockedMessages,
139+
matrix,
140+
previewCases
141+
}
142+
}, null, 2)}\n`, "utf8");
143+
144+
console.log(`v2 diff/merge button-state results: ${resultsPath}`);
145+
assert.equal(failures.length, 0, `V2 diff/merge button-state failures: ${failures.join(" | ")}`);
146+
return { failures };
147+
}
148+
149+
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
150+
try {
151+
const summary = run();
152+
console.log(JSON.stringify(summary, null, 2));
153+
} catch (error) {
154+
console.error(error);
155+
process.exitCode = 1;
156+
}
157+
}

tools/workspace-v2/index.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ <h2>Session Diff Viewer</h2>
8686
<label for="workspaceV2DiffRightSelect">Session B</label>
8787
<select id="workspaceV2DiffRightSelect"></select>
8888
<p><strong>Selected B:</strong> <code id="workspaceV2DiffRightSelectedLabel">No session selected</code></p>
89+
<p id="workspaceV2DiffSelectionState">Select two different sessions to compute diff.</p>
8990
<button id="workspaceV2ComputeDiffButton" type="button">Compute Diff</button>
9091
<p id="workspaceV2DiffEmptyState">Need at least two valid sessions to compare.</p>
9192
<pre id="workspaceV2DiffOutput">No diff computed.</pre>
@@ -100,6 +101,7 @@ <h2>Session Merge</h2>
100101
<label for="workspaceV2MergeRightSelect">Session B</label>
101102
<select id="workspaceV2MergeRightSelect"></select>
102103
<p><strong>Selected B:</strong> <code id="workspaceV2MergeRightSelectedLabel">No session selected</code></p>
104+
<p id="workspaceV2MergeSelectionState">Select two different sessions to preview merge.</p>
103105
<div>
104106
<button id="workspaceV2ComputeMergeButton" type="button">Preview Merge (Dry Run)</button>
105107
<button id="workspaceV2ConfirmMergeButton" type="button">Confirm Preview</button>

tools/workspace-v2/index.js

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,15 @@ class WorkspaceV2SessionProducer {
3434
this.diffRightSelect = document.getElementById("workspaceV2DiffRightSelect");
3535
this.diffLeftSelectedLabelNode = document.getElementById("workspaceV2DiffLeftSelectedLabel");
3636
this.diffRightSelectedLabelNode = document.getElementById("workspaceV2DiffRightSelectedLabel");
37+
this.diffSelectionStateNode = document.getElementById("workspaceV2DiffSelectionState");
3738
this.computeDiffButton = document.getElementById("workspaceV2ComputeDiffButton");
3839
this.diffEmptyState = document.getElementById("workspaceV2DiffEmptyState");
3940
this.diffOutputNode = document.getElementById("workspaceV2DiffOutput");
4041
this.mergeLeftSelect = document.getElementById("workspaceV2MergeLeftSelect");
4142
this.mergeRightSelect = document.getElementById("workspaceV2MergeRightSelect");
4243
this.mergeLeftSelectedLabelNode = document.getElementById("workspaceV2MergeLeftSelectedLabel");
4344
this.mergeRightSelectedLabelNode = document.getElementById("workspaceV2MergeRightSelectedLabel");
45+
this.mergeSelectionStateNode = document.getElementById("workspaceV2MergeSelectionState");
4446
this.computeMergeButton = document.getElementById("workspaceV2ComputeMergeButton");
4547
this.confirmMergeButton = document.getElementById("workspaceV2ConfirmMergeButton");
4648
this.applyMergeButton = document.getElementById("workspaceV2ApplyMergeButton");
@@ -551,20 +553,55 @@ class WorkspaceV2SessionProducer {
551553
return shortContext ? `${toolId} | ${shortContext}` : toolId;
552554
}
553555

556+
hasFreshMergePreviewContext(preview) {
557+
if (!preview || typeof preview !== "object") {
558+
return false;
559+
}
560+
const source = this.findSessionEntryById(this.mergeCandidates, preview.source && preview.source.id ? preview.source.id : "");
561+
const target = this.findSessionEntryById(this.mergeCandidates, preview.target && preview.target.id ? preview.target.id : "");
562+
if (!source || !target) {
563+
return false;
564+
}
565+
return JSON.stringify(source.payload) === preview.source.hash && JSON.stringify(target.payload) === preview.target.hash;
566+
}
567+
554568
updateDiffSelectionFeedbackAndState() {
555569
const left = this.findSessionEntryById(this.diffCandidates, this.diffLeftSelect.value);
556570
const right = this.findSessionEntryById(this.diffCandidates, this.diffRightSelect.value);
557571
this.diffLeftSelectedLabelNode.textContent = this.formatSelectionLabel(left);
558572
this.diffRightSelectedLabelNode.textContent = this.formatSelectionLabel(right);
559-
this.computeDiffButton.disabled = !(left && right && left.id !== right.id);
573+
const canRunDiff = Boolean(left && right && left.id !== right.id);
574+
this.computeDiffButton.disabled = !canRunDiff;
575+
if (left && right && left.id === right.id) {
576+
this.diffSelectionStateNode.textContent = "Choose two different sessions.";
577+
return;
578+
}
579+
this.diffSelectionStateNode.textContent = canRunDiff
580+
? "Selections are valid."
581+
: "Select two different sessions to compute diff.";
560582
}
561583

562584
updateMergeSelectionFeedbackAndState() {
563585
const left = this.findSessionEntryById(this.mergeCandidates, this.mergeLeftSelect.value);
564586
const right = this.findSessionEntryById(this.mergeCandidates, this.mergeRightSelect.value);
565587
this.mergeLeftSelectedLabelNode.textContent = this.formatSelectionLabel(left);
566588
this.mergeRightSelectedLabelNode.textContent = this.formatSelectionLabel(right);
567-
this.computeMergeButton.disabled = !(left && right && left.id !== right.id);
589+
const canPreviewMerge = Boolean(left && right && left.id !== right.id);
590+
this.computeMergeButton.disabled = !canPreviewMerge;
591+
if (left && right && left.id === right.id) {
592+
this.mergeSelectionStateNode.textContent = "Choose two different sessions.";
593+
} else if (canPreviewMerge) {
594+
this.mergeSelectionStateNode.textContent = "Selections are valid.";
595+
} else {
596+
this.mergeSelectionStateNode.textContent = "Select two different sessions to preview merge.";
597+
}
598+
599+
const previewIsFresh = this.hasFreshMergePreviewContext(this.pendingMergePreview);
600+
const previewHasConflicts = this.pendingMergePreview && Object.keys(this.pendingMergePreview.conflicts).length > 0;
601+
const previewReadyForConfirm = Boolean(this.pendingMergePreview && !this.pendingMergePreview.confirmed && previewIsFresh && !previewHasConflicts);
602+
const previewReadyForApply = Boolean(this.pendingMergePreview && this.pendingMergePreview.confirmed && previewIsFresh && !previewHasConflicts);
603+
this.confirmMergeButton.disabled = !previewReadyForConfirm;
604+
this.applyMergeButton.disabled = !previewReadyForApply;
568605
}
569606

570607
renderSessionDiffInputs() {
@@ -799,8 +836,7 @@ class WorkspaceV2SessionProducer {
799836
changes: previewChanges,
800837
confirmed: false
801838
};
802-
this.confirmMergeButton.disabled = false;
803-
this.applyMergeButton.disabled = true;
839+
this.updateMergeSelectionFeedbackAndState();
804840

805841
this.mergeOutputNode.textContent = JSON.stringify({
806842
source: this.pendingMergePreview.source,
@@ -856,7 +892,7 @@ class WorkspaceV2SessionProducer {
856892
return;
857893
}
858894
this.pendingMergePreview.confirmed = true;
859-
this.applyMergeButton.disabled = false;
895+
this.updateMergeSelectionFeedbackAndState();
860896
this.mergeOutputNode.textContent = JSON.stringify({
861897
source: this.pendingMergePreview.source,
862898
target: this.pendingMergePreview.target,
@@ -946,9 +982,8 @@ class WorkspaceV2SessionProducer {
946982
this.importJsonNode.value = JSON.stringify(this.pendingMergePreview.mergedPayload, null, 2);
947983
this.recordMergeAuditEntry(this.pendingMergePreview);
948984
this.renderDiagnosticsPanel();
949-
this.confirmMergeButton.disabled = true;
950-
this.applyMergeButton.disabled = true;
951985
this.pendingMergePreview = null;
986+
this.updateMergeSelectionFeedbackAndState();
952987
this.mergeOutputNode.textContent = JSON.stringify({
953988
source: liveSource.id,
954989
target: liveTarget.id,

0 commit comments

Comments
 (0)