fix: tour -- close the second funnel round 4 found, correct the overclaim
Round 4's review confirmed the commit() guard genuinely closes every project-save path (keyboard shortcuts included), but caught the fix's own comment overreaching: "every mutating path in this module funnels through here" is false. store.js's shortcut/category/transition CRUD functions (saveShortcut, deleteShortcut, addCategory, importShortcuts, clearShortcuts, saveTransition, deleteTransition, importTransitions, clearTransitions) each call their own real, unmocked API.* endpoint directly and never touch commit() at all -- a second, separate funnel the guard doesn't cover. Currently dormant (no Settings > Shortcuts panel is wired up in v5 yet, confirmed by grep -- these functions have no caller anywhere outside store.js/api.js today), same shape as the saveClipToMediaBin landmine rounds 2-3 already closed pre-emptively rather than waiting for it to go live. These write GLOBAL state (the on-disk shortcut/transition library), not sandboxed per-project state like everything commit() guards, so if left open they'd be worse than a stray project PUT once a Shortcuts UI ships. Fixed with the same block-list pattern, and corrected both comments (store.js's commit() guard, tour.js's blocklist) to state their actual scope instead of the overclaim. Verified live: all nine now return the block toast instead of reaching Store's real implementation; full tour sequence still completes with zero thrown errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
61a4817460
commit
cd1d07d96c
+12
-7
@@ -969,16 +969,21 @@
|
||||
}
|
||||
|
||||
async function commit(opts = {}) {
|
||||
// Every mutating path in this module funnels through here before it can
|
||||
// reach a real network write -- scheduleSave()'s own setTimeout calls
|
||||
// this closure-scoped function directly, so patching Store.commit (or
|
||||
// any individual caller) from outside cannot stop it. The tour's own
|
||||
// Every PROJECT-save path in this module funnels through here before it
|
||||
// can reach a real network write -- scheduleSave()'s own setTimeout
|
||||
// calls this closure-scoped function directly, so patching Store.commit
|
||||
// (or any individual caller) from outside cannot stop it. The tour's own
|
||||
// welcome copy promises "nothing here touches your real files," and
|
||||
// enumerating every caller in tour.js's blocklist misses ones reachable
|
||||
// enumerating every caller in tour.js's blocklist missed ones reachable
|
||||
// outside a click (keyboard shortcuts bypass the tour's click-blocking
|
||||
// backdrop entirely, since CSS pointer-events does nothing to keydown
|
||||
// listeners) -- so the guard belongs here, once, covering every path by
|
||||
// construction instead of by an ever-growing enumeration.
|
||||
// listeners) -- so the guard belongs here, once, covering every project-
|
||||
// save path by construction instead of by an ever-growing enumeration.
|
||||
// NOTE: this is not the one true funnel for every mutation tour.js needs
|
||||
// to worry about -- shortcut/transition/category CRUD (saveShortcut,
|
||||
// deleteTransition, etc.) call their own API.* functions directly and
|
||||
// never reach commit() at all; those still need their own entries in
|
||||
// tour.js's patchStore() blocklist.
|
||||
if (window.__FUNPACK_TOUR__) return;
|
||||
if (!state.project) return;
|
||||
if (_commitPromise) { _commitQueued = true; return _commitPromise; }
|
||||
|
||||
+28
-4
@@ -345,6 +345,13 @@
|
||||
"saveSelectedToMediaBin", "commit", "newProject", "loadProject", "deleteProject",
|
||||
"importProject", "downloadProject", "uploadMedia", "deleteMedia", "deleteMediaMany",
|
||||
"interrupt", "resetStudioSession", "syncFromPreview", "saveClipToMediaBin",
|
||||
// Not reachable via any current UI (no Settings > Shortcuts panel is
|
||||
// wired up in v5 yet) -- same "block it now, before a future UI wires
|
||||
// it up" reasoning as saveClipToMediaBin above. These all call real,
|
||||
// unmocked API.* functions that (unlike project saves) don't funnel
|
||||
// through commit(), so the store.js guard doesn't cover them.
|
||||
"saveShortcut", "deleteShortcut", "addCategory", "importShortcuts", "clearShortcuts",
|
||||
"saveTransition", "deleteTransition", "importTransitions", "clearTransitions",
|
||||
];
|
||||
const labels = {
|
||||
generate: "Generate",
|
||||
@@ -364,10 +371,13 @@
|
||||
// Blocking "commit" only stops external callers of Store.commit -- the
|
||||
// property gets reassigned below, but scheduleSave()'s own setTimeout
|
||||
// calls the closure-scoped `commit` function directly, bypassing this
|
||||
// patch. syncFromPreview() is the one UI entry point that reaches
|
||||
// scheduleSave() (and, through it, a real API.saveProject network
|
||||
// call) without going through anything else already on this list, so
|
||||
// it needs its own block rather than relying on "commit" to cover it.
|
||||
// patch. store.js's commit() now has its own window.__FUNPACK_TOUR__
|
||||
// guard as a backstop for every PROJECT-save path (including ones this
|
||||
// list doesn't name, like the keyboard shortcuts that reach
|
||||
// removeSelectedScenes()/splitScene()/etc.) -- but that guard only
|
||||
// covers commit()'s own callers. syncFromPreview() and every entry
|
||||
// below it call a DIFFERENT real, unmocked API.* function that never
|
||||
// reaches commit() at all, so they still need their own block here.
|
||||
syncFromPreview: "Sync scenes from preview",
|
||||
// Not reachable via a real network call today -- API.importClipToMediaBin
|
||||
// is itself a stub ("no render/stitch stage yet") -- but the button
|
||||
@@ -375,6 +385,20 @@
|
||||
// clipSaveableToMediaBin()), so this needs blocking now rather than
|
||||
// whenever that stage lands and the stub silently becomes a real PUT.
|
||||
saveClipToMediaBin: "Save video",
|
||||
// None of these are reachable via any current UI -- no Settings >
|
||||
// Shortcuts panel exists in v5 yet -- but each calls a real, unmocked
|
||||
// API.* function (POST/DELETE /api/shortcuts, /api/transitions) that,
|
||||
// unlike a project save, writes GLOBAL state, not per-project state.
|
||||
// Blocking pre-emptively, same reasoning as saveClipToMediaBin.
|
||||
saveShortcut: "Save shortcut",
|
||||
deleteShortcut: "Delete shortcut",
|
||||
addCategory: "Add category",
|
||||
importShortcuts: "Import shortcuts",
|
||||
clearShortcuts: "Clear shortcuts",
|
||||
saveTransition: "Save transition",
|
||||
deleteTransition: "Delete transition",
|
||||
importTransitions: "Import transitions",
|
||||
clearTransitions: "Clear transitions",
|
||||
};
|
||||
blocked.forEach((name) => {
|
||||
if (typeof Store[name] !== "function") return;
|
||||
|
||||
Reference in New Issue
Block a user