diff --git a/CHANGELOG.md b/CHANGELOG.md index d99b365..b8b2eff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -183,6 +183,28 @@ The format is inspired by Keep a Changelog and SemVer. The skip uses the unresolved load path so a `custom_nodes/` → worktree symlink cannot defeat the check via `Path.resolve()`. +- **Status pill no longer shows "drifted" on every ComfyUI start (#276).** The + #161 boot-time drift guard flagged *any* difference between the tracked + snapshot file and the live state — which is also what ordinary "kept working + after the last snapshot save" looks like, so nearly every session booted into + a false corruption warning and periodic autosaves were misrouted to + `_unsaved_autosave/` instead of the preset's own recovery folder. The check + is now three-way: a mismatch only counts as **drifted** when the live state's + fingerprint matches the persisted last-named-save baseline (it *claims* to be + saved, yet the file disagrees — which pins the out-of-band change to the + *file* side: a cross-machine library sync, a hand-edit, another install + writing it); otherwise it reports the honest **unsaved**. The first-session + baseline seeding now runs *after* the drift check **and only when that check + proved the file and live state agree** — seeding on an unproven boot + baselined whatever the live state happened to be, which both showed "saved" + for state that was never saved to the named file and handed the next boot a + live state that "claims saved" against a file it never matched, re-opening + the same false positive from the other side (reachable whenever a tracked + preset name outlives its fingerprint: upgrades from before the baseline key + existed, or a persist that lost to localStorage quota). Trade-off: + out-of-band changes to the live `/userdata` + side — including corruption — now read "unsaved"; the #162 duplicate-install + vector (the known live-side corruptor) keeps its own dedicated guard. - **Keyframe guidance now defaults to 0.8 (smooth), not 1.0 (robotic).** The timeline editor auto-generated the `guide_strength` string from each clip's per-segment `guideStrength`, filling **1.0** for any clip left unset — and the diff --git a/tests/sidebar/test_boot_drift_detection.py b/tests/sidebar/test_boot_drift_detection.py index f8b3f27..6b596a4 100644 --- a/tests/sidebar/test_boot_drift_detection.py +++ b/tests/sidebar/test_boot_drift_detection.py @@ -1,7 +1,10 @@ -"""Tests for the boot-time tracked-snapshot drift guard (#161). +"""Tests for the boot-time tracked-snapshot drift guard (#161, #276). -Covers the four observable contracts the rest of the codebase depends on: - 1. ``detectBootDrift()`` flags drift on file-vs-live mismatch. +Covers the observable contracts the rest of the codebase depends on: + 1. ``detectBootDrift()`` flags drift on file-vs-live mismatch — but + ONLY when the live state also matches the persisted last-named-save + baseline ("claims saved", #276). A mismatch with unsaved edits (or + with no baseline at all) reports "unsaved", never drift. 2. ``detectBootDrift()`` does NOT flag drift when the named file and live state agree. 3. ``getSnapshotStatus()`` returns ``state: "drifted"`` while the @@ -50,6 +53,7 @@ import { markStateSaved, clearBootDrift, isBootDrifted, + bootCheckProvedAligned, setCurrentPresetName, writePreLoadAutosave, } from "./web/sidebar/snapshot.js"; @@ -155,8 +159,9 @@ def _run(scenario_body: str) -> subprocess.CompletedProcess[str]: def test_detect_drift_flags_mismatch() -> None: """The named snapshot file holds 2 workflows; the live store holds - a different 1-workflow shape. ``detectBootDrift()`` must flag this - as drift and console.warn with the diagnostics.""" + a different 1-workflow shape, and the live state claims to be saved + (baseline matches). ``detectBootDrift()`` must flag this as drift + and console.warn with the diagnostics.""" result = _run( """ // Arrange: named file with workflows A + B. @@ -192,6 +197,9 @@ def test_detect_drift_flags_mismatch() -> None: await loadWorkflowsStore(); setAllPicks([]); setCurrentPresetName("Foo"); + // The live state must claim "saved" for the mismatch to count as + // drift (#276) — baseline it as a prior session's Save would have. + markStateSaved(); // Act const outcome = await detectBootDrift("Foo"); @@ -234,10 +242,16 @@ def test_detect_drift_skips_when_states_match() -> None: await loadWorkflowsStore(); setAllPicks([]); setCurrentPresetName("Bar"); + // Baseline the state so the claims-saved gate (#276) cannot be + // the reason for `drifted: false` — this test must pin the + // fingerprint MATCH branch, not the unsaved-edits early exit. + markStateSaved(); const outcome = await detectBootDrift("Bar"); assert.equal(outcome.drifted, false); + assert.equal(outcome.reason, undefined, + "match branch must not carry an unsaved-edits reason"); assert.equal(isBootDrifted(), false); """ ) @@ -336,6 +350,8 @@ def test_mark_state_saved_clears_drift_flag() -> None: await loadWorkflowsStore(); setAllPicks([]); setCurrentPresetName("Qux"); + // Claims-saved baseline from a prior session's Save (#276). + markStateSaved(); await detectBootDrift("Qux"); assert.equal(isBootDrifted(), true); @@ -368,6 +384,8 @@ def test_clear_boot_drift_public_export_works() -> None: await loadWorkflowsStore(); setAllPicks([]); setCurrentPresetName("Zap"); + // Claims-saved baseline from a prior session's Save (#276). + markStateSaved(); await detectBootDrift("Zap"); assert.equal(isBootDrifted(), true); @@ -402,6 +420,8 @@ def test_pre_load_autosave_redirects_to_unsaved_when_drifted() -> None: await loadWorkflowsStore(); setAllPicks([]); setCurrentPresetName("Drifty"); + // Claims-saved baseline from a prior session's Save (#276). + markStateSaved(); await detectBootDrift("Drifty"); assert.equal(isBootDrifted(), true); @@ -483,16 +503,268 @@ def test_detect_drift_does_not_fire_on_normalize_only_shape_differences() -> Non await loadWorkflowsStore(); setAllPicks([]); setCurrentPresetName("CrossMachine"); + // Baseline the state so the claims-saved gate (#276) cannot be + // the reason for `drifted: false` — this test guards the + // canonicalization contract, so it must reach the fingerprint + // comparison and prove the shapes MATCH. + markStateSaved(); const outcome = await detectBootDrift("CrossMachine"); assert.equal(outcome.drifted, false, "shape differences that normalize to the same canonical form should NOT trigger drift"); + assert.equal(outcome.reason, undefined, + "canonically equal shapes must take the match branch, not the unsaved-edits exit"); assert.equal(isBootDrifted(), false); """ ) assert result.returncode == 0, result.stderr +def test_no_drift_when_live_has_unsaved_edits_since_baseline() -> None: + """#276: the everyday case — the user saved a snapshot, then kept + working. The live state diverges from BOTH the named file and the + persisted last-named-save baseline. That is ordinary work-in-progress + ("unsaved"), not corruption — flagging it as "drifted" made the pill + cry wolf on every boot and misrouted periodic autosaves to + ``_unsaved_autosave/`` instead of the preset's recovery folder.""" + result = _run( + """ + // Arrange: live store + baseline agree at "save time". + SERVER.workflowsStore = { + directories: { + "Renders": { + workflows: { + "A": { graph: { nodes: [{ id: 1 }] }, archived: false, module: false, tags: [] }, + }, + directories: {}, + }, + }, + }; + SERVER.presets.set("Wip.json", { + kind: "koolook-snapshot", + version: 1, + name: "Wip", + exportedAt: "2026-08-10T08:00:00.000Z", + picks: [], + workflows: SERVER.workflowsStore, + }); + await loadWorkflowsStore(); + setAllPicks([]); + setCurrentPresetName("Wip"); + markStateSaved(); // baseline == live == file + + // Act: the user keeps working — live store gains a workflow the + // named file does not have. Baseline is now stale on purpose. + await replaceAllWorkflows({ + directories: { + "Renders": { + workflows: { + "A": { graph: { nodes: [{ id: 1 }] }, archived: false, module: false, tags: [] }, + "B-new": { graph: { nodes: [{ id: 2 }] }, archived: false, module: false, tags: [] }, + }, + directories: {}, + }, + }, + }); + const outcome = await detectBootDrift("Wip"); + + // Assert: no drift — this is the "unsaved" state, not corruption. + assert.equal(outcome.drifted, false, + "post-save edits must not flag boot drift"); + assert.equal(outcome.reason, "unsaved-edits"); + assert.equal(isBootDrifted(), false); + const status = getSnapshotStatus(); + assert.equal(status.state, "unsaved", + `expected honest "unsaved", got "${status.state}"`); + """ + ) + assert result.returncode == 0, result.stderr + + +def test_no_drift_without_saved_baseline() -> None: + """#276: with no persisted last-named-save baseline at all, the live + state cannot *claim* to be saved — so a file-vs-live mismatch is + indistinguishable from ordinary unsaved work and must not flag.""" + result = _run( + """ + SERVER.workflowsStore = { + directories: { + "Renders": { + workflows: { + "C": { graph: { nodes: [{ id: 3 }] }, archived: false, module: false, tags: [] }, + }, + directories: {}, + }, + }, + }; + SERVER.presets.set("NoBase.json", { + kind: "koolook-snapshot", + version: 1, + name: "NoBase", + exportedAt: "2026-08-10T08:00:00.000Z", + picks: ["EasyResize_Koolook"], + workflows: { directories: {} }, + }); + await loadWorkflowsStore(); + setAllPicks([]); + setCurrentPresetName("NoBase"); + // Deliberately NO markStateSaved() — no baseline exists. + + const outcome = await detectBootDrift("NoBase"); + + assert.equal(outcome.drifted, false, + "mismatch without a claims-saved baseline must not flag drift"); + assert.equal(outcome.reason, "unsaved-edits"); + assert.equal(isBootDrifted(), false); + """ + ) + assert result.returncode == 0, result.stderr + + +# ============================================================================= +# 5. Seeding gate — bootCheckProvedAligned (#276 review) +# ============================================================================= + + +def test_boot_check_proved_aligned_only_on_verified_match() -> None: + """``koolook_sidebar.js`` seeds a first-session "saved" baseline only + when the boot check PROVED file ≡ live. Every other outcome — no + preset tracked / unreadable file (``null``), or a mismatch that fell + through to the unsaved-edits exit — must return false. + + Seeding on an unproven boot baselines whatever the live state happens + to be, which hands the NEXT boot a live state that "claims saved" + against a file it never matched: a fresh false "drifted", i.e. #276 + re-opened from the other side.""" + result = _run( + """ + // Proven match — the only true case. + assert.equal(bootCheckProvedAligned({ drifted: false, trackedName: "A" }), true); + + // Mismatch that fell through to the unsaved-edits exit. + assert.equal( + bootCheckProvedAligned({ drifted: false, trackedName: "A", reason: "unsaved-edits" }), + false, "unsaved-edits outcome proves nothing — must not seed"); + + // Real drift. + assert.equal(bootCheckProvedAligned({ drifted: true, trackedName: "A" }), false); + + // No preset tracked, or the named file was unreadable. + assert.equal(bootCheckProvedAligned(null), false, + "null outcome proves nothing — must not seed"); + assert.equal(bootCheckProvedAligned(undefined), false); + """ + ) + assert result.returncode == 0, result.stderr + + +def test_seeding_gate_blocks_baseline_when_file_differs() -> None: + """End-to-end on the real module: a session whose tracked-preset name + outlived its fingerprint baseline (upgrade path, or a persist that + lost to localStorage quota) and whose live state differs from the + named file must NOT be seeded — and a second boot against the same + unchanged state must still report "unsaved", never "drifted". + + This is the regression the seeding gate exists to prevent; without it + the first boot writes a baseline equal to live, and the second boot + reads live == baseline + file != live and flags drift.""" + result = _run( + """ + SERVER.workflowsStore = { + directories: { + "Renders": { + workflows: { + "live-only": { graph: { nodes: [{ id: 9 }] }, archived: false, module: false, tags: [] }, + }, + directories: {}, + }, + }, + }; + SERVER.presets.set("Upgraded.json", { + kind: "koolook-snapshot", + version: 1, + name: "Upgraded", + exportedAt: "2026-08-10T08:00:00.000Z", + picks: [], + workflows: { directories: {} }, + }); + await loadWorkflowsStore(); + setAllPicks([]); + setCurrentPresetName("Upgraded"); + // No markStateSaved() — the tracked name exists but no baseline + // was ever persisted. That is the reachable upgrade/quota state. + + // --- Boot 1 + const first = await detectBootDrift("Upgraded"); + assert.equal(first.drifted, false); + assert.equal(bootCheckProvedAligned(first), false, + "unproven boot must not authorize baseline seeding"); + // The sidebar entry point would call markStateSaved() here only if + // the gate returned true. It didn't, so the state stays unbaselined. + assert.equal(getSnapshotStatus().state, "unsaved"); + + // --- Boot 2 against the same unchanged state. + const second = await detectBootDrift("Upgraded"); + assert.equal(second.drifted, false, + "second boot must not flag drift — nothing changed and nothing was saved"); + assert.equal(isBootDrifted(), false); + assert.equal(getSnapshotStatus().state, "unsaved"); + + // --- Counterfactual: prove the gate is load-bearing, not decorative. + // Replay what the UNGATED seeding did — markStateSaved() on an + // unproven boot — and show the very next boot false-positives. + // If this leg ever stops flagging drift, the gate above has become + // redundant and this whole test can go. + markStateSaved(); + const third = await detectBootDrift("Upgraded"); + assert.equal(third.drifted, true, + "counterfactual must flag drift — otherwise the seeding gate guards nothing"); + assert.equal(getSnapshotStatus().state, "drifted"); + """ + ) + assert result.returncode == 0, result.stderr + + +def test_seeding_gate_allows_baseline_when_file_matches() -> None: + """The other side of the gate: when the boot check proves file ≡ live, + seeding is authorized and the pill reads "saved" from the start — + the original reason the seeding exists.""" + result = _run( + """ + const sharedStore = { + directories: { + "X": { + workflows: { + "Y": { graph: { nodes: [] }, archived: false, module: false, tags: [] }, + }, + directories: {}, + }, + }, + }; + SERVER.workflowsStore = sharedStore; + SERVER.presets.set("Aligned.json", { + kind: "koolook-snapshot", + version: 1, + name: "Aligned", + exportedAt: "2026-08-10T08:00:00.000Z", + picks: [], + workflows: sharedStore, + }); + await loadWorkflowsStore(); + setAllPicks([]); + setCurrentPresetName("Aligned"); + + const outcome = await detectBootDrift("Aligned"); + assert.equal(bootCheckProvedAligned(outcome), true, + "a proven match must authorize seeding"); + // Entry point seeds here. + markStateSaved(); + assert.equal(getSnapshotStatus().state, "saved"); + """ + ) + assert result.returncode == 0, result.stderr + + def test_pre_load_autosave_uses_preset_subdir_when_not_drifted() -> None: """Sanity check the other branch: when drift is NOT set, pre-load autosaves go to ``_autosave/`` per the regular diff --git a/web/koolook_sidebar.js b/web/koolook_sidebar.js index dba9133..6973f1f 100644 --- a/web/koolook_sidebar.js +++ b/web/koolook_sidebar.js @@ -25,6 +25,7 @@ import { markStateSaved, getCurrentPresetName, detectBootDrift, + bootCheckProvedAligned, SNAPSHOT_KIND, SNAPSHOT_VERSION, writePreset, @@ -211,45 +212,63 @@ if (!claimSidebarRegistration(window)) { if (loadResult.fallbackBlob) { wireOfflineFallbackRecovery(loadResult.fallbackBlob); } - // Baseline the saved-state fingerprint at session start IFF the - // tracker says a preset is currently loaded AND we don't already - // have a baseline persisted from a previous session. The "saved - // fingerprint persists across reloads" path is more accurate, but - // first-ever-session needs this seeding to avoid showing "unsaved" - // immediately for users who closed the tab on a clean state. - // Without this, the indicator would flicker "unsaved → saved" the - // first time the user clicks Save (since markStateSaved baselines - // there). With it, "saved" is shown from the start. Trade-off: if - // the user closed mid-edit (state on /userdata diverges from the - // tracked preset's content), we'll show "saved" briefly until they - // mutate again — acceptable for an indicator. - if (getCurrentPresetName() && !localStorage.getItem("koolook.snapshot.savedFingerprint.v1")) { - markStateSaved(); - } - // Boot-time tracked-snapshot drift check (#161). When a preset is - // tracked, compare the named snapshot file on disk against the - // live /userdata + picks state. On mismatch, flag drift — the - // status pill flips to "drifted (reload?)" and periodic auto- - // saves redirect to `_unsaved_autosave/` so the named preset's - // recovery folder is never overwritten with the (potentially - // corrupt) live state. + // Boot-time tracked-snapshot drift check (#161, narrowed by #276). + // When a preset is tracked, compare the named snapshot file on + // disk against the live /userdata + picks state. On mismatch — + // and only when the live state also matches the persisted + // last-named-save baseline, i.e. it *claims* to be saved — flag + // drift: the status pill flips to "drifted (reload?)" and + // periodic auto-saves redirect to `_unsaved_autosave/` so the + // named preset's recovery folder stays untouched until the user + // decides which side to keep. A mismatch without that claim is + // ordinary unsaved work and reports "unsaved". // - // Runs AFTER the localStorage fingerprint baseline above so the - // drift flag wins precedence over the fingerprint match — see - // `getSnapshotStatus()` for the precedence rationale. + // Runs BEFORE the first-session baseline seeding below (#276): + // seeding fabricates a baseline from the live state, which would + // make the live state trivially "claim saved" and re-open the + // false-positive this ordering closes. Ordering alone isn't + // enough though — see the seeding block for why its outcome is + // also gated on what this check proved. // // Failure here is silent (logged, not toasted): a missing file or // unreachable server are distinct failure modes the user already // has surface for elsewhere. Drift specifically means "both // present, both differ". const trackedNameForDrift = getCurrentPresetName(); + let bootDriftOutcome = null; if (trackedNameForDrift) { try { - await detectBootDrift(trackedNameForDrift); + bootDriftOutcome = await detectBootDrift(trackedNameForDrift); } catch (e) { console.warn("[Koolook] boot drift check threw:", e); } } + // Baseline the saved-state fingerprint at session start IFF the + // boot check above PROVED the named file and live state agree AND + // we don't already have a baseline persisted from a previous + // session. The "saved fingerprint persists across reloads" path is + // more accurate, but a first session with no stored baseline needs + // this seeding to avoid showing "unsaved" immediately for users who + // closed the tab on a clean state. Without it the indicator would + // flicker "unsaved → saved" the first time the user clicks Save + // (since markStateSaved baselines there). + // + // The `bootCheckProvedAligned` gate is what makes the seeding safe + // (#276 review). Seeding on an UNPROVEN boot — no tracked file, an + // unreadable one, or a file that demonstrably differs — baselines + // whatever the live state happens to be, which both shows "saved" + // for state that was never saved to the named file and hands the + // next boot's drift check a live state that now "claims saved" + // against a file it never matched: a fresh false "drifted". That + // path is reachable in practice whenever the tracked-preset name + // outlives its fingerprint (upgrade from a build before the + // baseline key existed, or a persist that lost to localStorage + // quota — the same pressure `koolook_draft_guard.js` exists for). + // When the boot check proved nothing, "unsaved" is the honest pill. + if (bootCheckProvedAligned(bootDriftOutcome) + && !localStorage.getItem("koolook.snapshot.savedFingerprint.v1")) { + markStateSaved(); + } app.extensionManager.registerSidebarTab({ id: TAB_ID, title: TAB_TITLE, diff --git a/web/sidebar/snapshot.js b/web/sidebar/snapshot.js index 4a4e78d..f7afecc 100644 --- a/web/sidebar/snapshot.js +++ b/web/sidebar/snapshot.js @@ -193,14 +193,15 @@ function isoToFsToken(iso) { // to `_unsaved_autosave/`. Sanitizes the preset name through the same // filename whitelist so the dirname is server-validation-safe. // -// Boot-time drift override (#161): when the session opened with the -// tracked snapshot's named file diverging from the live state, route +// Boot-time drift override (#161, narrowed by #276): when the session +// opened with the tracked snapshot's named file diverging from the live +// state *while the live state still matched the last named save*, route // auto-saves to `_unsaved_autosave/` regardless of whether a preset is -// tracked. This is the core data-loss-prevention move — without it the -// periodic timer captures the (potentially corrupt) live state INTO the -// named preset's recovery folder, where the Load dialog will later -// present it as a "newer recovery" choice and the user can accept it -// thinking it's a legitimate auto-save of their work. +// tracked. The file changed out of band, so which side the user wants to +// keep is unknown — without this the periodic timer writes the live state +// INTO the named preset's recovery folder, where the Load dialog later +// presents it as a "newer recovery" choice and the user can accept it as +// a legitimate auto-save, quietly discarding whatever the file held. function _autosaveSubdir() { if (_bootDrifted) return AUTOSAVE_UNSAVED_SUBDIR; const name = getCurrentPresetName(); @@ -238,13 +239,16 @@ let _lastPeriodicFingerprint = null; let _lastPeriodicAt = null; // ============================================================================= -// Boot-time drift state (#161). Set by `detectBootDrift()` when the live -// `/userdata` workflow store + picks diverge from the tracked named snapshot -// file on disk at session start. While set: +// Boot-time drift state (#161, narrowed by #276). Set by `detectBootDrift()` +// when the live `/userdata` workflow store + picks diverge from the tracked +// named snapshot file on disk at session start *while the live state claims +// to be saved* (its fingerprint matches the persisted last-named-save +// baseline). A mismatch without that claim is ordinary unsaved work and does +// NOT set the flag. While set: // • `_autosaveSubdir()` routes periodic auto-saves to `_unsaved_autosave/` -// so the named preset's recovery folder is never overwritten with the -// corrupted live state (the data-loss surface this whole guard exists -// to close). +// so the named preset's recovery folder stays untouched while the file +// and live state disagree — the file changed out of band, and which +// side the user wants to keep is unknown until they Reload or Save. // • `getSnapshotStatus()` returns `state: "drifted"` with highest // precedence so the sidebar pill carries the warning. // Cleared by `_clearBootDriftInternal()` whenever `markStateSaved()` runs — @@ -355,28 +359,31 @@ export function markStateAutosaved() { // // Status precedence (highest first): // • "drifted" — boot-time check found the named-snapshot file on disk -// diverged from the live /userdata state; user must -// Reload or re-Save to realign (#161) +// diverged from the live /userdata state WHILE the live +// state's fingerprint matched the last-named-save +// baseline, i.e. it claimed to be saved; user must +// Reload or re-Save to realign (#161, #276) // • "saved" — current fingerprint matches the last named save // • "autosaved" — current matches the latest periodic auto-save (only) // • "unsaved" — there IS a tracked preset but state diverged // • "none" — no preset tracked, no autosave match // -// "drifted" wins over "saved" because the localStorage fingerprint can be -// stale — if a previous session baselined against already-corrupt live -// state, the in-memory fingerprint would falsely satisfy `"saved"`. The -// disk-file comparison run by `detectBootDrift()` at session start is the -// authoritative check; until the user Reloads or Saves, we keep flagging. +// "drifted" wins over "saved" because that is precisely the contradiction +// it flags: the localStorage fingerprint says "saved" but the disk-file +// comparison run by `detectBootDrift()` at session start disagrees — +// something changed one of the two out-of-band. Until the user Reloads or +// Saves, we keep flagging. A divergence WITHOUT the saved claim never sets +// the flag (#276) — it falls through to the honest "unsaved" below. export function getSnapshotStatus() { const name = getCurrentPresetName(); const fp = _computeFingerprint(); if (_bootDrifted) { - // ``lastNamedSaveAt`` is from localStorage and may be a baseline - // captured by a PRIOR session whose live state was already corrupt - // — surfacing it in the drift tooltip ("Last named save: ") - // would mislead users about when the divergence actually happened. - // Replace it with ``driftDetectedAt`` (when this session noticed - // the disk-vs-live disagreement) so the tooltip's timestamp + // Suppress ``lastNamedSaveAt`` in the drifted status: the + // divergence started with an out-of-band change to the FILE at + // some unknowable time after that save, so "Last named save: + // " would misleadingly date a file whose current content + // that save did not produce. ``driftDetectedAt`` (when this + // session noticed the disagreement) is the only timestamp that // anchors on a meaningful event. ``lastAutosaveAt`` stays — it's // session-scoped and the only autosaves landing while drifted go // to ``_unsaved_autosave/`` so the timestamp is still accurate. @@ -401,13 +408,17 @@ export function getSnapshotStatus() { } // ============================================================================= -// Boot-time drift detection (#161). Invoked once from `koolook_sidebar.js`'s -// `setup()` immediately after `loadWorkflowsStore()` completes. Reads the -// tracked named snapshot file from the library, computes its fingerprint +// Boot-time drift detection (#161, narrowed by #276). Invoked once from +// `koolook_sidebar.js`'s `setup()` immediately after `loadWorkflowsStore()` +// completes — and BEFORE any first-session baseline seeding, so a freshly +// fabricated baseline can never satisfy the claims-saved gate below. Reads +// the tracked named snapshot file from the library, computes its fingerprint // using the same shape as the live-state fingerprint (`picks` + `workflows`, // dropping the wrapping `name`/`kind`/`version`/`exportedAt` envelope), and -// compares. On mismatch, sets `_bootDrifted` — see the state block above -// for what gates on the flag. +// compares. On mismatch, sets `_bootDrifted` only when the live state also +// matches the persisted last-named-save baseline (it "claims saved") — see +// the state block above for what gates on the flag and the inline comment +// below for the three-way rationale. // // Skips silently when no preset is tracked (fresh user, nothing to verify // against) or when the named file can't be read (renamed, deleted, server @@ -473,6 +484,44 @@ export async function detectBootDrift(trackedName) { _emitStatusChanged(); return { drifted: false, trackedName }; } + // File and live state disagree — but that alone is NOT corruption + // (#276). The user editing workflows after their last named save + // produces exactly the same mismatch, and that is the ordinary + // "unsaved" state. Drift is only flagged when the live state *claims* + // to be saved — its fingerprint matches the persisted last-named-save + // baseline (the same comparison `getSnapshotStatus` uses for "saved") + // — yet the file on disk tells a different story. Since the baseline + // was captured when file and live agreed, the only way to reach that + // contradiction is the FILE changing out of band: a cross-machine + // sync of the library, a hand-edit, another install writing it. A + // live-side out-of-band change would have broken the baseline match + // instead. Without a baseline the state can't claim anything, so we + // fall through to "unsaved" there too rather than cry wolf. + // + // Trade-off: out-of-band changes to the live /userdata side — + // including corruption — now read "unsaved", whether the state was + // clean or dirty when they landed. The #162 duplicate-install vector + // (the known live-side corruptor) has its own dedicated guard + // (extension_guard.js); this check remains the backstop for + // file-side changes only. + const hasBaseline = Boolean(_lastNamedSaveFingerprint); + const claimsSaved = + hasBaseline && _computeFingerprint() === _lastNamedSaveFingerprint; + if (!claimsSaved) { + // Two distinct ways to land here — say which, or the log claims + // "unsaved edits" for a session that has no save to be edited + // since (fresh profile, cleared storage, a baseline write that + // lost to localStorage quota). + console.log( + `[Koolook] boot drift check: tracked snapshot "${trackedName}" differs ` + + `from live state, but ` + + (hasBaseline + ? "the live state has unsaved edits since the last named save" + : "there is no last-named-save baseline to judge it against") + + ` — reporting "unsaved", not drift.` + ); + return { drifted: false, trackedName, reason: "unsaved-edits" }; + } _bootDrifted = true; _bootDriftDiagnostics = { trackedName, @@ -493,6 +542,26 @@ export async function detectBootDrift(trackedName) { return { drifted: true, trackedName, diagnostics: _bootDriftDiagnostics }; } +// True only when `detectBootDrift()` positively PROVED the tracked named +// file and the live state agree — i.e. it reached the fingerprint +// comparison and they matched. `null` (no preset tracked, file +// unreadable) and the `reason`-carrying early exits both return false: +// those outcomes prove nothing either way. +// +// Exists so `koolook_sidebar.js` can gate its first-session baseline +// seeding on real evidence. Seeding calls `markStateSaved()`, which +// baselines whatever the live state happens to be; doing that after an +// unproven boot both shows a "saved" pill for state that was never saved +// to the named file AND manufactures the claims-saved condition that +// `detectBootDrift` reads next boot — re-opening the #276 false positive +// from the other side. The entry point imports ComfyUI's `app.js` and +// can't be loaded headlessly, so the decision lives here where it is +// unit-testable — same split as `extension_guard.js`'s +// `claimSidebarRegistration`. +export function bootCheckProvedAligned(outcome) { + return Boolean(outcome) && outcome.drifted === false && !outcome.reason; +} + // True iff `detectBootDrift()` flagged a mismatch this session and no // Save/Load has realigned the state since. Read by `_autosaveSubdir()` to // gate the periodic-autosave routing, and by the sidebar pill renderer. diff --git a/web/sidebar/tree.js b/web/sidebar/tree.js index 9e85966..b45bead 100644 --- a/web/sidebar/tree.js +++ b/web/sidebar/tree.js @@ -3301,24 +3301,28 @@ export function renderPanel(container, options = {}) { ? status.lastAutosaveAt : status.lastNamedSaveAt; const location = snapshotLibraryPath || "loading..."; - // Drifted (#161): the named snapshot file on disk diverged from the - // live state when the session started. Tooltip carries the recovery - // instructions so the user understands what the pill colour means - // and how to clear it without hunting through docs. + // Drifted (#161, narrowed by #276): the named snapshot file on disk + // diverged from the live state at session start even though the live + // state still matches the last named save — the file changed out of + // band (another machine's sync, a hand-edit, another install writing + // it). Tooltip carries the recovery instructions so the user + // understands what the pill colour means and how to clear it without + // hunting through docs. if (status.state === "drifted") { // Use ``driftDetectedAt`` (session-noticed-at), NOT - // ``lastNamedSaveAt`` — the latter comes from a localStorage - // baseline that may have been captured by a prior session - // whose live state was already corrupt, and surfacing it - // here as "Last named save" would mislead the user about - // when the divergence actually started. + // ``lastNamedSaveAt`` — the divergence started with an + // out-of-band change to the file at some unknowable time, so + // "when this session noticed" is the only timestamp that + // anchors on a meaningful event; surfacing the named-save time + // here would mislead the user about when the file changed. return ( `Tracked snapshot "${status.name || "?"}" diverges from live state.\n` + - `Periodic auto-saves are being redirected to _unsaved_autosave/\n` + - `to protect the named snapshot's recovery folder.\n` + + `The file changed outside this session (sync, hand-edit, or\n` + + `another install); periodic auto-saves are being redirected to\n` + + `_unsaved_autosave/ until the two are realigned.\n` + `\n` + - `To resolve: Load the tracked snapshot (discards live changes), or\n` + - `Save / Quick Save (overwrites the tracked snapshot with live state).\n` + + `To resolve: Load the tracked snapshot (replaces live state with\n` + + `the file), or Save / Quick Save (overwrites the file with live state).\n` + `\n` + `Drift detected: ${formatLocalTime(status.driftDetectedAt)}\n` + `Location: ${location}`