fix(browse): refresh tree after workflow moves, guard double-submit, fix modal listener leaks

Workflow data-consistency cleanup across the transmittal modules.

Stale-tree / re-trigger hazard: Stage, Unstage, and Accept reported success
with "reload to see the move" and never refreshed, leaving the moved item at
its old location in the tree — inviting the user to re-fire the action on a
folder the server had already moved. They now refresh the current listing on
success. This also revealed that events.refreshListing was never exported,
so upload.js's comment-upload refresh (which guards on it) was silently a
no-op — exporting it fixes that path too.

Non-atomic stage: "New folder" does mkdir then a separate move; if the move
failed after the mkdir succeeded the user got a generic "move failed" with an
unexplained empty folder left behind. invokeStage now tracks whether it
created the folder and says so, and refreshes so the orphan is visible.

Double-submit: Accept / Plan Review / Stage / Unstage take a module-level
busy guard so a second menu click while a POST is in flight is ignored.

Modal listener leaks (verified): the Escape keydown handler in accept,
plan-review, and create-transmittal was only removed on the Escape path —
cancel / overlay-click / submit all leaked a live document listener bound to
a detached modal. Bound once and removed in close() (matching history.js).

history.js restore: split the PUT from the post-restore refetch so a refetch
error can no longer surface a misleading "Restore failed" after the restore
has already persisted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
ZDDC 2026-06-03 14:59:23 -05:00
parent cfb2fab401
commit 41d4e59899
6 changed files with 182 additions and 106 deletions

View file

@ -189,7 +189,15 @@
}); });
}); });
// Bind the Escape handler once and remove it in close() — every
// dismissal path (cancel, overlay-click, submit, Escape) routes
// through close(), so the document listener can't outlive the
// modal.
function onKeydown(e) {
if (e.key === 'Escape') { close(); reject(new Error('cancelled')); }
}
function close() { function close() {
document.removeEventListener('keydown', onKeydown);
if (overlay.parentNode) overlay.parentNode.removeChild(overlay); if (overlay.parentNode) overlay.parentNode.removeChild(overlay);
} }
box.querySelector('#acc-cancel').addEventListener('click', function () { box.querySelector('#acc-cancel').addEventListener('click', function () {
@ -198,12 +206,7 @@
overlay.addEventListener('click', function (e) { overlay.addEventListener('click', function (e) {
if (e.target === overlay) { close(); reject(new Error('cancelled')); } if (e.target === overlay) { close(); reject(new Error('cancelled')); }
}); });
document.addEventListener('keydown', function escHandler(e) { document.addEventListener('keydown', onKeydown);
if (e.key === 'Escape') {
document.removeEventListener('keydown', escHandler);
close(); reject(new Error('cancelled'));
}
});
box.querySelector('#acc-submit').addEventListener('click', function () { box.querySelector('#acc-submit').addEventListener('click', function () {
var values = { var values = {
@ -243,7 +246,10 @@
return lines.join('\n'); return lines.join('\n');
} }
var busy = false;
async function invoke(node) { async function invoke(node) {
if (busy) return;
var tree = window.app.modules.tree; var tree = window.app.modules.tree;
if (!tree) return; if (!tree) return;
var url = tree.pathFor(node); var url = tree.pathFor(node);
@ -275,34 +281,43 @@
return; return;
} }
status('Accept Transmittal — submitting…'); busy = true;
var resp;
try { try {
resp = await fetch(url, { status('Accept Transmittal — submitting…');
method: 'POST', var resp;
headers: { try {
'X-ZDDC-Op': 'accept-transmittal', resp = await fetch(url, {
'Content-Type': 'application/yaml' method: 'POST',
}, headers: {
body: buildBody(values), 'X-ZDDC-Op': 'accept-transmittal',
credentials: 'same-origin' 'Content-Type': 'application/yaml'
}); },
} catch (e) { body: buildBody(values),
status('Accept failed: ' + (e && e.message ? e.message : e), 'error'); credentials: 'same-origin'
return; });
} catch (e) {
status('Accept failed: ' + (e && e.message ? e.message : e), 'error');
return;
}
if (!resp.ok) {
var text = '';
try { text = await resp.text(); } catch (_e) { /* ignore */ }
status('Accept failed (' + resp.status + '): ' + text, 'error');
return;
}
var data; try { data = await resp.json(); } catch (_e) { data = null; }
var msg = 'Accepted ' + (data && data.moved_files ? data.moved_files : '?') + ' file(s) into '
+ (data && data.received_path ? data.received_path : 'received/');
if (data && data.merged) msg += ' (merged with existing tracking)';
if (data && data.plan_review) msg += ' · Plan Review scaffolded';
status(msg, 'success');
// Refresh the incoming/ listing so the now-moved folder drops out
// of the tree — the stale entry was the main re-trigger hazard.
var ev = window.app.modules.events;
if (ev && typeof ev.refreshListing === 'function') ev.refreshListing();
} finally {
busy = false;
} }
if (!resp.ok) {
var text = '';
try { text = await resp.text(); } catch (_e) { /* ignore */ }
status('Accept failed (' + resp.status + '): ' + text, 'error');
return;
}
var data; try { data = await resp.json(); } catch (_e) { data = null; }
var msg = 'Accepted ' + (data && data.moved_files ? data.moved_files : '?') + ' file(s) into '
+ (data && data.received_path ? data.received_path : 'received/');
if (data && data.merged) msg += ' (merged with existing tracking)';
if (data && data.plan_review) msg += ' · Plan Review scaffolded';
status(msg + ' — reload to see the move.', 'success');
} }
window.app.modules.acceptTransmittal = { window.app.modules.acceptTransmittal = {

View file

@ -78,19 +78,22 @@
input.addEventListener('input', revalidate); input.addEventListener('input', revalidate);
revalidate(); revalidate();
function close() { if (overlay.parentNode) overlay.parentNode.removeChild(overlay); } // Escape handler bound once, removed in close() so it can't
// outlive a modal dismissed via cancel / overlay-click / submit.
function onKeydown(e) {
if (e.key === 'Escape') { close(); reject(new Error('cancelled')); }
}
function close() {
document.removeEventListener('keydown', onKeydown);
if (overlay.parentNode) overlay.parentNode.removeChild(overlay);
}
box.querySelector('#ct-cancel').addEventListener('click', function () { box.querySelector('#ct-cancel').addEventListener('click', function () {
close(); reject(new Error('cancelled')); close(); reject(new Error('cancelled'));
}); });
overlay.addEventListener('click', function (e) { overlay.addEventListener('click', function (e) {
if (e.target === overlay) { close(); reject(new Error('cancelled')); } if (e.target === overlay) { close(); reject(new Error('cancelled')); }
}); });
document.addEventListener('keydown', function escHandler(e) { document.addEventListener('keydown', onKeydown);
if (e.key === 'Escape') {
document.removeEventListener('keydown', escHandler);
close(); reject(new Error('cancelled'));
}
});
submit.addEventListener('click', function () { submit.addEventListener('click', function () {
var v = input.value.trim(); var v = input.value.trim();
var parsed = window.zddc.parseFolder(v); var parsed = window.zddc.parseFolder(v);

View file

@ -1447,6 +1447,11 @@
statusInfo: statusInfo, statusInfo: statusInfo,
statusClear: statusClear, statusClear: statusClear,
showBrowseRoot: showBrowseRoot, showBrowseRoot: showBrowseRoot,
applyResolvedViewMode: applyResolvedViewMode applyResolvedViewMode: applyResolvedViewMode,
// Re-fetch + re-render the current listing (restoring expansion +
// selection). Workflow modules call this after a move/accept so the
// tree reflects the change without a manual reload. upload.js already
// depends on it being present.
refreshListing: refreshListing
}; };
})(); })();

View file

@ -346,6 +346,10 @@
if (!confirm('Restore the version from ' + fmtTime(ent.ts) + '?\nThis is saved as a new version — nothing is lost.')) { if (!confirm('Restore the version from ' + fmtTime(ent.ts) + '?\nThis is saved as a new version — nothing is lost.')) {
return; return;
} }
// The restore itself (the PUT) is the operation that can "fail".
// Keep it in its own try so a later error while refreshing the UI
// can't surface a misleading "Restore failed" after the restore has
// already been persisted.
try { try {
var text = await fetchVersion(node, ent.id); var text = await fetchVersion(node, ent.id);
var resp = await fetch(node.url, { var resp = await fetch(node.url, {
@ -355,18 +359,22 @@
body: text body: text
}); });
if (!resp.ok) throw new Error('HTTP ' + resp.status); if (!resp.ok) throw new Error('HTTP ' + resp.status);
toast('Restored version from ' + fmtTime(ent.ts), 'success'); } catch (e) {
// Reflect the new head: refetch the list. toast('Restore failed: ' + (e.message || e), 'error');
return;
}
toast('Restored version from ' + fmtTime(ent.ts), 'success');
// Best-effort UI refresh — the restore already succeeded, so a
// failure here is logged but never reported as a restore failure.
try {
var entries = await fetchList(node); var entries = await fetchList(node);
renderList(modal, node, entries); renderList(modal, node, entries);
// If the file is open in the preview pane, reload it. // If the file is open in the preview pane, reload it.
var preview = window.app && window.app.modules && window.app.modules.preview; var preview = window.app && window.app.modules && window.app.modules.preview;
if (preview && typeof preview.showFilePreview === 'function') { if (preview && typeof preview.showFilePreview === 'function') {
try { preview.showFilePreview(node); } catch (_e) { /* best effort */ } preview.showFilePreview(node);
} }
} catch (e) { } catch (_e) { /* refresh is best-effort; restore is done */ }
toast('Restore failed: ' + (e.message || e), 'error');
}
} }
// ── Entry point ───────────────────────────────────────────────────── // ── Entry point ─────────────────────────────────────────────────────

View file

@ -145,7 +145,14 @@
}); });
}); });
// Escape handler bound once, removed in close() — every
// dismissal path routes through close() so the document
// listener never outlives the modal.
function onKeydown(e) {
if (e.key === 'Escape') { close(); reject(new Error('cancelled')); }
}
function close() { function close() {
document.removeEventListener('keydown', onKeydown);
if (overlay.parentNode) overlay.parentNode.removeChild(overlay); if (overlay.parentNode) overlay.parentNode.removeChild(overlay);
} }
@ -159,13 +166,7 @@
reject(new Error('cancelled')); reject(new Error('cancelled'));
} }
}); });
document.addEventListener('keydown', function escHandler(e) { document.addEventListener('keydown', onKeydown);
if (e.key === 'Escape') {
document.removeEventListener('keydown', escHandler);
close();
reject(new Error('cancelled'));
}
});
box.querySelector('#pr-submit').addEventListener('click', function () { box.querySelector('#pr-submit').addEventListener('click', function () {
var values = { var values = {
@ -211,8 +212,11 @@
&& parts[3].toLowerCase() === 'received'; && parts[3].toLowerCase() === 'received';
} }
var busy = false;
// Run the Plan Review flow: open the modal, POST the result. // Run the Plan Review flow: open the modal, POST the result.
async function invoke(node) { async function invoke(node) {
if (busy) return;
var tree = window.app.modules.tree; var tree = window.app.modules.tree;
if (!tree) return; if (!tree) return;
var url = tree.pathFor(node); var url = tree.pathFor(node);
@ -227,43 +231,48 @@
return; // cancelled return; // cancelled
} }
statusInfo('Plan Review — submitting…'); busy = true;
var body = buildBody(values);
var resp;
try { try {
resp = await fetch(url, { statusInfo('Plan Review — submitting…');
method: 'POST', var body = buildBody(values);
headers: { var resp;
'X-ZDDC-Op': 'plan-review', try {
'Content-Type': 'application/yaml' resp = await fetch(url, {
}, method: 'POST',
body: body, headers: {
credentials: 'same-origin' 'X-ZDDC-Op': 'plan-review',
}); 'Content-Type': 'application/yaml'
} catch (e) { },
statusError('Plan Review failed: ' + (e && e.message ? e.message : e)); body: body,
return; credentials: 'same-origin'
} });
if (!resp.ok) { } catch (e) {
var text = ''; statusError('Plan Review failed: ' + (e && e.message ? e.message : e));
try { text = await resp.text(); } catch (_e) { /* ignore */ } return;
statusError('Plan Review failed (' + resp.status + '): ' + text); }
return; if (!resp.ok) {
} var text = '';
var data; try { text = await resp.text(); } catch (_e) { /* ignore */ }
try { data = await resp.json(); } catch (_e) { data = null; } statusError('Plan Review failed (' + resp.status + '): ' + text);
if (data && data.reviewing && data.staging) { return;
var rPart = data.reviewing.created ? 'created' : 'updated'; }
var sPart = data.staging.created ? 'created' : 'updated'; var data;
var seal = (data.received && data.received.created) try { data = await resp.json(); } catch (_e) { data = null; }
? ' Canonical record sealed.' if (data && data.reviewing && data.staging) {
: (data.received && !data.received.zddc_written) var rPart = data.reviewing.created ? 'created' : 'updated';
? ' Canonical dates left untouched (already sealed).' var sPart = data.staging.created ? 'created' : 'updated';
: ''; var seal = (data.received && data.received.created)
statusInfo('Plan Review: reviewing ' + rPart + ', staging ' + sPart + '.' + seal + ? ' Canonical record sealed.'
' Reload the relevant folder to see the new entries.'); : (data.received && !data.received.zddc_written)
} else { ? ' Canonical dates left untouched (already sealed).'
statusInfo('Plan Review complete.'); : '';
statusInfo('Plan Review: reviewing ' + rPart + ', staging ' + sPart + '.' + seal +
' Reload the relevant folder to see the new entries.');
} else {
statusInfo('Plan Review complete.');
}
} finally {
busy = false;
} }
} }

View file

@ -25,6 +25,15 @@
var t = window.zddc && window.zddc.toast; var t = window.zddc && window.zddc.toast;
if (t) t(msg, level || 'info'); if (t) t(msg, level || 'info');
} }
// Re-fetch the current listing so the moved file appears/disappears
// without a manual reload. Best-effort: absent on older builds.
function refreshListing() {
var ev = window.app.modules.events;
if (ev && typeof ev.refreshListing === 'function') ev.refreshListing();
}
// Guard against a second invocation while a move is mid-flight (e.g. a
// double menu click). The picker modal also blocks re-entry while open.
var busy = false;
function escapeHtml(s) { function escapeHtml(s) {
return String(s).replace(/[&<>"']/g, function (c) { return String(s).replace(/[&<>"']/g, function (c) {
return ({ '&':'&amp;','<':'&lt;','>':'&gt;','"':'&quot;',"'":'&#39;' })[c]; return ({ '&':'&amp;','<':'&lt;','>':'&gt;','"':'&quot;',"'":'&#39;' })[c];
@ -267,6 +276,7 @@
// ── Action drivers ───────────────────────────────────────────────── // ── Action drivers ─────────────────────────────────────────────────
async function invokeStage(node) { async function invokeStage(node) {
if (busy) return;
var tree = window.app.modules.tree; var tree = window.app.modules.tree;
if (!tree) return; if (!tree) return;
var srcUrl = tree.pathFor(node); var srcUrl = tree.pathFor(node);
@ -289,26 +299,46 @@
choice = await openStagePicker({ fileCount: 1, folders: folders }); choice = await openStagePicker({ fileCount: 1, folders: folders });
} catch (_e) { return; } } catch (_e) { return; }
if (choice.create) { busy = true;
try {
// Stage is a non-atomic mkdir-then-move (no single composite op).
// Track whether the folder was freshly created so that, if the
// move then fails, we can tell the user the folder exists but the
// file didn't make it — otherwise an empty folder appears with a
// generic "move failed" and no explanation.
var createdFolder = false;
if (choice.create) {
try {
await mkdir(stagingBase + encodeURIComponent(choice.folderName) + '/');
createdFolder = true;
} catch (e) {
status((e && e.message) || 'mkdir failed', 'error');
return;
}
}
var dstUrl = stagingBase + encodeURIComponent(choice.folderName) + '/' + encodeURIComponent(node.name);
try { try {
await mkdir(stagingBase + encodeURIComponent(choice.folderName) + '/'); await moveFile(srcUrl, dstUrl);
} catch (e) { } catch (e) {
status((e && e.message) || 'mkdir failed', 'error'); var msg = (e && e.message) || 'move failed';
if (createdFolder) {
msg += ' — the new folder "' + choice.folderName
+ '" was created but ' + node.name + ' was not moved into it.';
}
status(msg, 'error');
refreshListing(); // surface the (possibly empty) new folder
return; return;
} }
status('Staged ' + node.name + ' → ' + info.party + '/staging/' + choice.folderName + '/', 'success');
refreshListing();
} finally {
busy = false;
} }
var dstUrl = stagingBase + encodeURIComponent(choice.folderName) + '/' + encodeURIComponent(node.name);
try {
await moveFile(srcUrl, dstUrl);
} catch (e) {
status((e && e.message) || 'move failed', 'error');
return;
}
status('Staged ' + node.name + ' → ' + info.party + '/staging/' + choice.folderName + '/ — reload to see the move.', 'success');
} }
async function invokeUnstage(node) { async function invokeUnstage(node) {
if (busy) return;
var tree = window.app.modules.tree; var tree = window.app.modules.tree;
if (!tree) return; if (!tree) return;
var srcUrl = tree.pathFor(node); var srcUrl = tree.pathFor(node);
@ -326,13 +356,19 @@
var target = choice.target; var target = choice.target;
if (!target.endsWith('/')) target += '/'; if (!target.endsWith('/')) target += '/';
var dstUrl = target + encodeURIComponent(node.name); var dstUrl = target + encodeURIComponent(node.name);
busy = true;
try { try {
await moveFile(srcUrl, dstUrl); try {
} catch (e) { await moveFile(srcUrl, dstUrl);
status((e && e.message) || 'move failed', 'error'); } catch (e) {
return; status((e && e.message) || 'move failed', 'error');
return;
}
status('Unstaged ' + node.name + ' → ' + target, 'success');
refreshListing();
} finally {
busy = false;
} }
status('Unstaged ' + node.name + ' → ' + target + ' — reload to see the move.', 'success');
} }
window.app.modules.stage = { window.app.modules.stage = {