Skip to content

Commit 3393b92

Browse files
author
DavidQ
committed
Enable confirm preview after valid merge preview and gate apply correctly - PR 11.250
1 parent 4f14e0e commit 3393b92

3 files changed

Lines changed: 218 additions & 1 deletion

File tree

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# PR_11_250 — Enable Confirm Preview After Valid Merge Preview
2+
3+
## Summary
4+
Updated Workspace V2 Session Merge preview/confirm/apply UI state progression so a fresh, conflict-free preview enables `Confirm Preview`, and a confirmed fresh preview enables `Apply Merge`.
5+
6+
## Files Changed
7+
- `tools/workspace-v2/index.js`
8+
- `tests/runtime/V2ConfirmPreviewEnableState.test.mjs`
9+
10+
## Implementation Details
11+
1. Preview state enrichment
12+
- Merge preview state now includes:
13+
- `conflictCount`
14+
- `previewFingerprint`
15+
- existing source/target ids, hashes, and preview result payload.
16+
17+
2. Immediate state recompute
18+
- Button state recompute still runs immediately after:
19+
- preview completion (`computeSelectedSessionMerge`)
20+
- confirmation (`confirmSelectedSessionMergePreview`)
21+
22+
3. Confirm/Apply gating
23+
- Confirm enabled only when:
24+
- preview exists
25+
- preview is fresh
26+
- `conflictCount === 0`
27+
- preview not yet confirmed
28+
- Apply enabled only when:
29+
- preview exists
30+
- preview is fresh
31+
- preview is confirmed
32+
- `conflictCount === 0`
33+
34+
4. Conflict and stale handling
35+
- Conflict preview keeps Confirm/Apply disabled and shows:
36+
- `Preview has conflicts. Resolve conflicts before applying.`
37+
- Stale preview keeps Confirm/Apply disabled and shows:
38+
- `Preview is stale. Run Preview Merge again.`
39+
- Confirm action now guards stale/conflict preview and returns actionable status.
40+
41+
5. Preview output containment
42+
- Existing contained merge preview output remains in-panel (`#workspaceV2MergeOutput` bounded and scrollable), with no page-wide overlay behavior.
43+
44+
## Validation Commands Run
45+
```powershell
46+
node --check tools/workspace-v2/index.js
47+
node --check tests/runtime/V2ConfirmPreviewEnableState.test.mjs
48+
node --check tests/runtime/V2MergePreviewOverlayFix.test.mjs
49+
node tests/runtime/V2ConfirmPreviewEnableState.test.mjs
50+
node tests/runtime/V2MergePreviewOverlayFix.test.mjs
51+
```
52+
53+
## Validation Results
54+
- `node --check tools/workspace-v2/index.js` -> PASS
55+
- `node --check tests/runtime/V2ConfirmPreviewEnableState.test.mjs` -> PASS
56+
- `node --check tests/runtime/V2MergePreviewOverlayFix.test.mjs` -> PASS
57+
- `node tests/runtime/V2ConfirmPreviewEnableState.test.mjs` -> PASS
58+
- output: `tmp/v2-confirm-preview-enable-state-results.json`
59+
- failures: `[]`
60+
- `node tests/runtime/V2MergePreviewOverlayFix.test.mjs` -> PASS
61+
- output: `tmp/v2-merge-preview-overlay-fix-results.json`
62+
- failures: `[]`
63+
64+
## Verified
65+
- valid selections enable Preview Merge
66+
- conflict-free fresh preview enables Confirm Preview
67+
- Apply stays disabled before confirm
68+
- Confirm enables Apply when preview remains fresh/conflict-free
69+
- conflict preview disables Confirm/Apply
70+
- stale preview disables Confirm/Apply
71+
- status text updates across preview -> confirm flow
72+
- preview output remains contained and non-overlay
73+
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
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 htmlPath = path.join(repoRoot, "tools", "workspace-v2", "index.html");
11+
const jsPath = path.join(repoRoot, "tools", "workspace-v2", "index.js");
12+
const resultsPath = path.join(repoRoot, "tmp", "v2-confirm-preview-enable-state-results.json");
13+
14+
function checkSyntax(filePath) {
15+
try {
16+
execFileSync(process.execPath, ["--check", filePath], {
17+
cwd: repoRoot,
18+
stdio: ["ignore", "pipe", "pipe"]
19+
});
20+
return { ok: true, error: "" };
21+
} catch (error) {
22+
return { ok: false, error: (error?.stderr || error?.stdout || error?.message || "").toString().trim() };
23+
}
24+
}
25+
26+
function evaluateState({ canPreviewMerge, previewExists, previewFresh, previewConfirmed, conflictCount }) {
27+
const hasConflicts = previewExists && conflictCount > 0;
28+
const previewReadyForConfirm = previewExists && !previewConfirmed && previewFresh && !hasConflicts;
29+
const previewReadyForApply = previewExists && previewConfirmed && previewFresh && !hasConflicts;
30+
const confirmDisabled = !previewReadyForConfirm;
31+
const applyDisabled = !previewReadyForApply;
32+
let status = "Select two different sessions to enable Preview Merge.";
33+
if (canPreviewMerge) {
34+
if (previewExists && !previewFresh) status = "Preview is stale. Run Preview Merge again.";
35+
else if (hasConflicts) status = "Preview has conflicts. Resolve conflicts before applying.";
36+
else if (previewReadyForApply) status = "Preview confirmed. Ready to apply merge.";
37+
else if (previewReadyForConfirm) status = "Preview ready. Confirm to enable apply.";
38+
else status = "Ready to preview merge.";
39+
}
40+
return { confirmDisabled, applyDisabled, status };
41+
}
42+
43+
export function run() {
44+
const failures = [];
45+
const htmlExists = fs.existsSync(htmlPath);
46+
const jsExists = fs.existsSync(jsPath);
47+
const html = htmlExists ? fs.readFileSync(htmlPath, "utf8") : "";
48+
const js = jsExists ? fs.readFileSync(jsPath, "utf8") : "";
49+
const jsSyntax = checkSyntax(jsPath);
50+
const testSyntax = checkSyntax(path.join(repoRoot, "tests", "runtime", "V2ConfirmPreviewEnableState.test.mjs"));
51+
52+
if (!htmlExists) failures.push("Missing tools/workspace-v2/index.html.");
53+
if (!jsExists) failures.push("Missing tools/workspace-v2/index.js.");
54+
if (!jsSyntax.ok) failures.push("tools/workspace-v2/index.js failed syntax check.");
55+
if (!testSyntax.ok) failures.push("tests/runtime/V2ConfirmPreviewEnableState.test.mjs failed syntax check.");
56+
57+
const requiredTokens = [
58+
"conflictCount: Object.keys(result.conflicts).length",
59+
"previewFingerprint:",
60+
"Preview ready. Confirm to enable apply.",
61+
"Preview confirmed. Ready to apply merge.",
62+
"Preview has conflicts. Resolve conflicts before applying.",
63+
"Preview is stale. Run Preview Merge again."
64+
];
65+
requiredTokens.forEach((token) => {
66+
if (!js.includes(token)) failures.push(`Missing merge preview state token/text: ${token}`);
67+
});
68+
69+
if (!html.includes("id=\"workspaceV2MergeOutput\"") || !html.includes("max-height: 18rem; overflow: auto; position: relative;")) {
70+
failures.push("Merge preview output container is not explicitly contained.");
71+
}
72+
73+
const noSelection = evaluateState({ canPreviewMerge: false, previewExists: false, previewFresh: false, previewConfirmed: false, conflictCount: 0 });
74+
if (noSelection.status !== "Select two different sessions to enable Preview Merge.") failures.push("No-selection status mismatch.");
75+
76+
const readyToPreview = evaluateState({ canPreviewMerge: true, previewExists: false, previewFresh: false, previewConfirmed: false, conflictCount: 0 });
77+
if (readyToPreview.status !== "Ready to preview merge.") failures.push("Ready-to-preview status mismatch.");
78+
79+
const previewReady = evaluateState({ canPreviewMerge: true, previewExists: true, previewFresh: true, previewConfirmed: false, conflictCount: 0 });
80+
if (previewReady.confirmDisabled) failures.push("Conflict-free fresh preview should enable Confirm.");
81+
if (!previewReady.applyDisabled) failures.push("Apply must remain disabled before Confirm.");
82+
if (previewReady.status !== "Preview ready. Confirm to enable apply.") failures.push("Preview-ready status mismatch.");
83+
84+
const previewConfirmed = evaluateState({ canPreviewMerge: true, previewExists: true, previewFresh: true, previewConfirmed: true, conflictCount: 0 });
85+
if (!previewConfirmed.confirmDisabled) failures.push("Confirm should disable after confirmation.");
86+
if (previewConfirmed.applyDisabled) failures.push("Apply should enable after confirmed fresh preview.");
87+
if (previewConfirmed.status !== "Preview confirmed. Ready to apply merge.") failures.push("Preview-confirmed status mismatch.");
88+
89+
const conflictPreview = evaluateState({ canPreviewMerge: true, previewExists: true, previewFresh: true, previewConfirmed: false, conflictCount: 2 });
90+
if (!conflictPreview.confirmDisabled || !conflictPreview.applyDisabled) failures.push("Conflict preview must disable Confirm and Apply.");
91+
if (conflictPreview.status !== "Preview has conflicts. Resolve conflicts before applying.") failures.push("Conflict status mismatch.");
92+
93+
const stalePreview = evaluateState({ canPreviewMerge: true, previewExists: true, previewFresh: false, previewConfirmed: false, conflictCount: 0 });
94+
if (!stalePreview.confirmDisabled || !stalePreview.applyDisabled) failures.push("Stale preview must disable Confirm and Apply.");
95+
if (stalePreview.status !== "Preview is stale. Run Preview Merge again.") failures.push("Stale status mismatch.");
96+
97+
fs.mkdirSync(path.dirname(resultsPath), { recursive: true });
98+
fs.writeFileSync(resultsPath, `${JSON.stringify({
99+
generatedAt: new Date().toISOString(),
100+
failures,
101+
checks: { htmlExists, jsExists, jsSyntax, testSyntax },
102+
states: { noSelection, readyToPreview, previewReady, previewConfirmed, conflictPreview, stalePreview }
103+
}, null, 2)}\n`, "utf8");
104+
105+
console.log(`v2 confirm-preview-enable-state results: ${resultsPath}`);
106+
assert.equal(failures.length, 0, `V2 confirm-preview-enable-state failures: ${failures.join(" | ")}`);
107+
return { failures };
108+
}
109+
110+
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
111+
try {
112+
const summary = run();
113+
console.log(JSON.stringify(summary, null, 2));
114+
} catch (error) {
115+
console.error(error);
116+
process.exitCode = 1;
117+
}
118+
}
119+

tools/workspace-v2/index.js

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -921,11 +921,18 @@ class WorkspaceV2SessionProducer {
921921
const canPreviewMerge = Boolean(left && right && left.id !== right.id);
922922
this.computeMergeButton.disabled = !canPreviewMerge;
923923
const previewIsFresh = this.hasFreshMergePreviewContext(this.pendingMergePreview);
924-
const previewHasConflicts = Boolean(this.pendingMergePreview && Object.keys(this.pendingMergePreview.conflicts).length > 0);
924+
const previewConflictCount = this.pendingMergePreview
925+
? (typeof this.pendingMergePreview.conflictCount === "number"
926+
? this.pendingMergePreview.conflictCount
927+
: Object.keys(this.pendingMergePreview.conflicts || {}).length)
928+
: 0;
929+
const previewHasConflicts = Boolean(this.pendingMergePreview && previewConflictCount > 0);
925930
const previewReadyForConfirm = Boolean(this.pendingMergePreview && !this.pendingMergePreview.confirmed && previewIsFresh && !previewHasConflicts);
926931
const previewReadyForApply = Boolean(this.pendingMergePreview && this.pendingMergePreview.confirmed && previewIsFresh && !previewHasConflicts);
927932
if (!canPreviewMerge) {
928933
this.mergeEnableStateNode.textContent = "Select two different sessions to enable Preview Merge.";
934+
} else if (this.pendingMergePreview && !previewIsFresh) {
935+
this.mergeEnableStateNode.textContent = "Preview is stale. Run Preview Merge again.";
929936
} else if (previewHasConflicts && previewIsFresh) {
930937
this.mergeEnableStateNode.textContent = "Preview has conflicts. Resolve conflicts before applying.";
931938
} else if (previewReadyForApply) {
@@ -1185,6 +1192,8 @@ class WorkspaceV2SessionProducer {
11851192
selectedToolId,
11861193
mergedPayload: versionedPayload,
11871194
conflicts: result.conflicts,
1195+
conflictCount: Object.keys(result.conflicts).length,
1196+
previewFingerprint: `${left.id}|${right.id}|${JSON.stringify(left.payload)}|${JSON.stringify(right.payload)}`,
11881197
changes: previewChanges,
11891198
confirmed: false
11901199
};
@@ -1195,6 +1204,7 @@ class WorkspaceV2SessionProducer {
11951204
target: this.pendingMergePreview.target,
11961205
changes: this.pendingMergePreview.changes,
11971206
conflicts: this.pendingMergePreview.conflicts,
1207+
conflictCount: this.pendingMergePreview.conflictCount,
11981208
confirmed: this.pendingMergePreview.confirmed,
11991209
canApply: Object.keys(this.pendingMergePreview.conflicts).length === 0
12001210
}, null, 2);
@@ -1243,13 +1253,28 @@ class WorkspaceV2SessionProducer {
12431253
this.statusNode.textContent = "No merge preview available. Run Preview Merge (Dry Run) first.";
12441254
return;
12451255
}
1256+
const previewIsFresh = this.hasFreshMergePreviewContext(this.pendingMergePreview);
1257+
if (!previewIsFresh) {
1258+
this.updateMergeSelectionFeedbackAndState();
1259+
this.statusNode.textContent = "Merge preview is stale. Run Preview Merge (Dry Run) again.";
1260+
return;
1261+
}
1262+
const conflictCount = typeof this.pendingMergePreview.conflictCount === "number"
1263+
? this.pendingMergePreview.conflictCount
1264+
: Object.keys(this.pendingMergePreview.conflicts || {}).length;
1265+
if (conflictCount > 0) {
1266+
this.updateMergeSelectionFeedbackAndState();
1267+
this.statusNode.textContent = "Merge preview has conflicts. Resolve conflicts before confirming.";
1268+
return;
1269+
}
12461270
this.pendingMergePreview.confirmed = true;
12471271
this.updateMergeSelectionFeedbackAndState();
12481272
this.mergeOutputNode.textContent = JSON.stringify({
12491273
source: this.pendingMergePreview.source,
12501274
target: this.pendingMergePreview.target,
12511275
changes: this.pendingMergePreview.changes,
12521276
conflicts: this.pendingMergePreview.conflicts,
1277+
conflictCount: this.pendingMergePreview.conflictCount,
12531278
confirmed: true,
12541279
canApply: Object.keys(this.pendingMergePreview.conflicts).length === 0
12551280
}, null, 2);

0 commit comments

Comments
 (0)