diff --git a/docs/technical_plugin.md b/docs/technical_plugin.md index 0bfc49d..12f6770 100644 --- a/docs/technical_plugin.md +++ b/docs/technical_plugin.md @@ -14,6 +14,7 @@ of Geode testable without a running app. - [What The Plugin Owns](#what-the-plugin-owns) - [The First Sync Dialog](#the-first-sync-dialog) - [The Mass Change Dialog](#the-mass-change-dialog) +- [The Status Bar](#the-status-bar) - [Toasts](#toasts) - [Guards](#guards) - [Writing Files Safely](#writing-files-safely) @@ -136,12 +137,57 @@ the dialog opens again, and says so: a second identical looking prompt with no e a bug, and someone who has already clicked through one is exactly the person who will click through the next without reading it. +### The Status Bar + +The status bar is the only thing Geode has on screen all the time, so it has to answer two questions +without being clicked: is it doing something right now, and when did it last work. A spinner with no +counts reads as hung after the first minute, and an idle cloud cannot tell "synced ten seconds ago" +from "has not synced since Tuesday". Automatic sync is what made both of those required rather than +cosmetic: the click used to be the feedback, and there is no click any more. + +An icon, a label beside it, and a tooltip. Every state is one row of a table in `status/status.ts`, +and every move between them is a pure transition in the same module, so the plugin holds the value +and draws it and decides none of it. + +| State | Says | The tooltip adds | +| -------------------- | ---------------- | ----------------------------------------------- | +| Nothing synced yet | `Not synced yet` | That clicking syncs | +| Resting | `Synced 2m ago` | The same age, and that clicking syncs | +| A pass with no plan | `Checking...` | That it is looking for changes | +| A pass applying one | `Syncing 12/340` | The same count, spelled out | +| A pass failed | `Sync failed` | The reason, and when the vault was last current | +| Automatic sync off | `Sync paused` | That clicking still syncs once | + +The count is the plan, reported before the first action runs and once per action after it, whether +that action worked or not. Reporting it up front matters because the first action of a large pull +can outlast anyone's patience on its own, and counting failures matters because a number that stops +moving on the first bad file reads as exactly the hang it is there to disprove. + +`Checking...` is not a placeholder. A first sync spends most of its time reading the manifest and +hashing the vault, before there is any plan to count, and that is the stretch the old spinner was +worst at. Saying "0/0" there would be a number pretending to be progress. + +Ages are coarse on purpose, minutes then hours then days: nothing finer would be read, and nothing +coarser answers the question. The bar redraws on the scheduler's tick rather than only when +something happens, since "2m ago" is a claim that goes stale on its own. + +The time itself lives in vault scoped localStorage, alongside the pause flag and the device ID and +for the same reason: when this laptop last synced is a fact about this laptop, not about the vault. +Repointing at a bucket this vault has never synced forgets it, because the old time is then about +somewhere else, and a confident wrong answer is worse than no answer. Only a state file that was +actually read can say that, though: a read that failed forgets nothing, since "I could not tell" +is not the same answer as "never synced here", and one transient failure should not erase a history +that is still true. A pass that applied nothing still counts as synced, since it proves the vault +and the bucket agree, which is the whole of what the question asks. + +The mobile app has no status bar at all, which is half of why toasts exist. + ### Toasts -The status bar is a cloud icon and a tooltip, which is enough to answer "what is it doing" and -nothing like enough to say something you have to act on. Toasts are the other half, and every one -geode raises comes from a single table in `notify/notify.ts`, so the wording, the duration, and the -silences are all pinned by one test rather than scattered across the plugin class. +The status bar says what is happening and when it last did, which is nothing like enough to say +something you have to act on. Toasts are the other half, and every one geode raises comes from a +single table in `notify/notify.ts`, so the wording, the duration, and the silences are all pinned by +one test rather than scattered across the plugin class. | Occasion | Says | Stays | | --------------------------------- | -------------------------------------------------------- | ----- | diff --git a/docs/technical_sync.md b/docs/technical_sync.md index bcbe741..1a9e4b0 100644 --- a/docs/technical_sync.md +++ b/docs/technical_sync.md @@ -74,8 +74,9 @@ laptop should never quietly pause your desktop. That is why the pause flag lives vault scoped localStorage rather than in `data.json`, the same reasoning the device ID uses (see [Device](technical_device.md)). -While paused, the status bar shows a dimmed cloud with a line through it. Clicking it still syncs -once, and the **Sync** command still works. Pause stops the timer, never the escape hatch. +While paused, the status bar shows a dimmed cloud with a line through it, reading `Sync paused`. +Clicking it still syncs once, and the **Sync** command still works. Pause stops the timer, never the +escape hatch. ### The Scheduler @@ -146,6 +147,10 @@ The caller owns persistence. The previous snapshot is passed in and the new one than read or written internally, so a pass stays pure over its inputs and tests can drive it with their own store. +Step 7 reports its progress to whoever asked for the pass, once before the first action and once per +action after it, so a long pass can be counted down rather than spun at (see +[Plugin](technical_plugin.md#the-status-bar)). + A pass that planned nothing and found a manifest already describing exactly that skips step 8. Writing it anyway is not merely a wasted request: every manifest upload is a compare and swap, so a device with nothing to say is a device that can lose a race it had no reason to enter and report diff --git a/src/main.ts b/src/main.ts index 96ba8f4..b79f799 100644 --- a/src/main.ts +++ b/src/main.ts @@ -32,6 +32,18 @@ import { prefixError, } from "./settings/settings"; import { GeodeSettingTab } from "./settings/tab"; +import { + DEFAULT_STATUS, + type Kind, + LAST_SYNCED_KEY, + lastSyncedFrom, + noteKind, + noteProgress, + noteSynced, + noteUnsynced, + type Status, + view, +} from "./status/status"; import { obsidianTransport } from "./storage/obsidian"; import { createS3Client, probeConditionalWrites } from "./storage/storage"; import type { MassChange } from "./sync/guard"; @@ -71,8 +83,9 @@ type AppWithSetting = App & { // since neither answer contains the other: "stop" is not a message, and a message is not a policy. type PassOutcome = { pass: Pass; result: PassResult }; -// SyncStatus is the state the status bar item reflects. -type SyncStatus = "idle" | "syncing" | "error" | "paused"; +// StateRead is what reading state.json found: this bucket has synced before, it has not, or the +// file could not be read at all, which is a different answer from "it has not". +type StateRead = "synced" | "unreadable" | "unsynced"; // deviceLabel returns the human recognisable half of this device's ID, asking "phone or tablet" // before "which desktop OS" because an iPad reports itself as macOS on some builds. @@ -96,20 +109,6 @@ function deviceLabel(): string { return "device"; } -// iconFor returns the status bar icon for status. -function iconFor(status: SyncStatus): string { - if (status === "syncing") { - return "refresh-cw"; - } - if (status === "error") { - return "cloud-alert"; - } - if (status === "paused") { - return "cloud-off"; - } - return "cloud"; -} - // passResultFor maps how a pass failed onto what the scheduler should do about it, and is the whole // of what those two modules need from each other. function passResultFor(fault: SyncFault): PassResult { @@ -133,20 +132,6 @@ function reportFor(pass: Pass): SyncReport { return { ok: false, message: pass.message }; } -// tooltipFor returns the status bar hover text for status. detail is folded into the error case. -function tooltipFor(status: SyncStatus, detail: string): string { - if (status === "syncing") { - return "Geode: syncing..."; - } - if (status === "error") { - return `Geode: ${detail}`; - } - if (status === "paused") { - return "Geode: automatic sync paused; click to sync once"; - } - return "Geode: click to sync"; -} - // GeodePlugin is the Obsidian plugin entry point that owns settings load and save; see // docs/technical_plugin.md for the layering rule every adapter here follows. export default class GeodePlugin extends Plugin { @@ -158,7 +143,15 @@ export default class GeodePlugin extends Plugin { logger!: Logger; private logBus!: LogBus; private logSink!: LogSink; + // status is everything the status bar reflects, held as one value so nothing can set the icon and + // the text to two different stories. + private status: Status = DEFAULT_STATUS; private statusBarEl!: HTMLElement; + // The icon and the label are separate children because setIcon replaces the whole content of the + // element it is given, and because redrawing the icon restarts the spin (see renderStatus). + private statusIconEl!: HTMLElement; + private statusIconName = ""; + private statusTextEl!: HTMLElement; // schedule decides when an automatic pass is due, and holds the in flight flag too so "a pass // is running" has one home rather than two fields that can disagree. private schedule: State = DEFAULT_STATE; @@ -251,8 +244,15 @@ export default class GeodePlugin extends Plugin { this.statusBarEl = this.addStatusBarItem(); this.statusBarEl.addClass("geode-status-bar", "mod-clickable"); + this.statusIconEl = this.statusBarEl.createSpan({ cls: "geode-status-icon" }); + this.statusTextEl = this.statusBarEl.createSpan({ cls: "geode-status-text" }); this.statusBarEl.addEventListener("click", () => void this.syncNow("manual")); - this.setSyncStatus(this.restingStatus(), ""); + this.status = { + ...DEFAULT_STATUS, + kind: this.restingKind(), + lastSyncedAt: lastSyncedFrom(this.app.loadLocalStorage(LAST_SYNCED_KEY)), + }; + this.renderStatus(); this.addSettingTab(new GeodeSettingTab(this.app, this)); this.logger.info(`loaded (provider=${this.settings.provider}, device=${this.deviceId})`); @@ -381,10 +381,10 @@ export default class GeodePlugin extends Plugin { ); } - // restingStatus returns the status bar state to fall back to once nothing is happening, which is + // restingKind returns the status bar state to fall back to once nothing is happening, which is // "paused" rather than "idle" on a device where automatic sync is switched off. Silence means // everything is fine only if a device that has stopped syncing says so. - private restingStatus(): SyncStatus { + private restingKind(): Kind { if (this.paused) { return "paused"; } @@ -396,7 +396,7 @@ export default class GeodePlugin extends Plugin { private setPaused(paused: boolean): void { this.paused = paused; this.app.saveLocalStorage(PAUSE_KEY, paused); - this.setSyncStatus(this.restingStatus(), ""); + this.setStatus(this.restingKind()); if (paused) { this.logger.info("automatic sync paused on this device"); this.toaster.show(toastFor({ kind: "paused" })); @@ -406,18 +406,52 @@ export default class GeodePlugin extends Plugin { this.toaster.show(toastFor({ kind: "resumed" })); } - // setSyncStatus updates the status bar icon and tooltip to reflect status. - private setSyncStatus(status: SyncStatus, detail: string): void { + // applyStatus puts the status a transition returned on screen, the one place the held value and + // the bar move together. + private applyStatus(next: Status): void { + this.status = next; + this.renderStatus(); + } + + // forgetSynced drops the last synced time, on screen and on this device. + private forgetSynced(): void { + this.applyStatus(noteUnsynced(this.status)); + this.app.saveLocalStorage(LAST_SYNCED_KEY, null); + } + + // rememberSynced records a pass completing per device, so a restart never reads as a vault that + // has synced nothing. + private rememberSynced(at: number): void { + this.applyStatus(noteSynced(this.status, at)); + this.app.saveLocalStorage(LAST_SYNCED_KEY, at); + } + + // renderStatus draws the current status into the bar. The icon is redrawn only when it changes, + // since setIcon rebuilds the SVG and would restart the spin every time the count ticks over. + private renderStatus(): void { + const rendered = view(this.status, Date.now()); this.statusBarEl.removeClass("is-idle", "is-syncing", "is-error", "is-paused"); - this.statusBarEl.addClass(`is-${status}`); - setIcon(this.statusBarEl, iconFor(status)); - setTooltip(this.statusBarEl, tooltipFor(status, detail)); + this.statusBarEl.addClass(`is-${this.status.kind}`); + if (rendered.icon !== this.statusIconName) { + setIcon(this.statusIconEl, rendered.icon); + this.statusIconName = rendered.icon; + } + this.statusTextEl.setText(rendered.label); + setTooltip(this.statusBarEl, rendered.tooltip); + } + + // setStatus moves the status bar to kind, carrying detail for the error case. + private setStatus(kind: Kind, detail = ""): void { + this.applyStatus(noteKind(this.status, kind, detail)); } // tick asks the scheduler, every TICK_MS, whether a pass is due. Both questions it asks are // answered in schedule.ts; all this contributes is reading the settings that armed needs, since // knowing what a setting looks like is the one thing the scheduler is kept away from. private tick(): void { + // Redrawn on every tick, armed or not: "synced 2m ago" is a claim that goes stale on its own, + // and a bar that only updates when something happens is exactly wrong while nothing does. + this.renderStatus(); const configured = hasConnectionConfig(this.settings) && prefixError(this.settings.prefix) === ""; if (!armed({ configured, paused: this.paused, syncedBefore: this.syncedBefore })) { @@ -466,7 +500,7 @@ export default class GeodePlugin extends Plugin { this.schedule = noteResumed(this.schedule); } this.schedule = notePassStarted(this.schedule); - this.setSyncStatus("syncing", ""); + this.setStatus("syncing"); let outcome: PassOutcome = { pass: { ...DEFAULT_PASS, message: "unexpected error" }, result: "retry", @@ -479,7 +513,7 @@ export default class GeodePlugin extends Plugin { message = err.message; } this.logger.error(`sync: ${message}`); - this.setSyncStatus("error", message); + this.setStatus("error", message); outcome = { pass: { ...DEFAULT_PASS, message }, result: "retry" }; } finally { this.schedule = notePassFinished(this.schedule, outcome.result, Date.now()); @@ -497,7 +531,7 @@ export default class GeodePlugin extends Plugin { // refuse reports a pass that never started. The status bar carries it, and so does a toast, since // a refusal nobody sees is indistinguishable from a sync that silently never runs. private refuse(message: string): SyncReport { - this.setSyncStatus("error", message); + this.setStatus("error", message); this.toaster.show(toastFor({ kind: "pass", pass: { ...DEFAULT_PASS, message } })); return { ok: false, message }; @@ -514,7 +548,7 @@ export default class GeodePlugin extends Plugin { if (secretAccessKey === null || secretAccessKey === "") { const message = "secret access key not found; open settings to reconfigure"; this.logger.error(`sync: secret access key not found for ID "${this.settings.secretId}"`); - this.setSyncStatus("error", message); + this.setStatus("error", message); return { pass: { ...DEFAULT_PASS, message }, result: "stop" }; } @@ -524,7 +558,7 @@ export default class GeodePlugin extends Plugin { const probe = await probeConditionalWrites(storage); if (!probe.ok) { this.logger.error(`sync: conditional write check failed: ${probe.message}`); - this.setSyncStatus("error", probe.message); + this.setStatus("error", probe.message); return { pass: { ...DEFAULT_PASS, message: probe.message }, result: "stop" }; } this.conditionalWritesVerified = true; @@ -553,10 +587,11 @@ export default class GeodePlugin extends Plugin { () => crypto.randomUUID(), this.deviceId, confirmed, + (done, total) => this.applyStatus(noteProgress(this.status, done, total)), ); if (!outcome.ok && outcome.fault === "blocked") { this.logger.warn(`sync: ${outcome.message}`); - this.setSyncStatus("error", outcome.message); + this.setStatus("error", outcome.message); new GeodeMassChangeModal(this.app, outcome.change, outcome.restated, (confirmed) => { void this.syncNow("manual", confirmed); }).open(); @@ -576,7 +611,7 @@ export default class GeodePlugin extends Plugin { this.logger.error(`sync: ${failure.path}: ${failure.message}`); } this.logger.error(`sync: ${outcome.message}`); - this.setSyncStatus("error", outcome.message); + this.setStatus("error", outcome.message); return { pass: { ...DEFAULT_PASS, message: outcome.message }, @@ -586,12 +621,15 @@ export default class GeodePlugin extends Plugin { await stateStore.write(outcome.snapshot); this.syncedBefore = true; + // A pass that applied nothing still counts: it proves the vault matches the bucket, which is + // the whole of what "last synced" is asked to answer. + this.rememberSynced(Date.now()); // An idle pass logs nothing, since a line each would flush the capped file inside two days, but // a pass someone asked for always reports. if (outcome.changeCount > 0 || trigger === "manual") { this.logger.info(`sync: complete (${trigger}, ${outcome.changeCount} change(s) applied)`); } - this.setSyncStatus(this.restingStatus(), ""); + this.setStatus(this.restingKind()); return { pass: { @@ -622,20 +660,30 @@ export default class GeodePlugin extends Plugin { this.settings = normalizeSettings(await this.loadData()); } - // loadSyncedBefore records whether this bucket has completed a pass already. The state store - // refuses a file written against different settings, so repointing reads back as never synced. - private async loadSyncedBefore(): Promise { + // loadSyncedBefore records whether this bucket has completed a pass already, reporting what the + // read found rather than only what it decided, since a file it could not read is evidence of + // nothing. The store refuses one written against other settings, so repointing reads as unsynced. + private async loadSyncedBefore(): Promise { const dir = this.manifest.dir; if (dir === undefined) { - return; + return "unreadable"; } const store = createObsidianStore(this.app.vault.adapter, `${dir}/state.json`, this.settings); try { const snapshot = await store.read(); this.syncedBefore = snapshot.vaultId !== undefined; + if (this.syncedBefore) { + return "synced"; + } + + return "unsynced"; } catch (err) { + // Automatic sync still stands down, since an unreadable state file is no basis for a pass, + // but nothing is forgotten over it. this.syncedBefore = false; this.logger.error(`could not read sync state: ${err}`); + + return "unreadable"; } } @@ -654,7 +702,11 @@ export default class GeodePlugin extends Plugin { // Saving settings is the one action most likely to have fixed whatever a halt was about, and // it may also have repointed the vault at a bucket it has never synced. this.schedule = noteResumed(this.schedule); - await this.loadSyncedBefore(); + // Only a state file that was read can say this vault now points somewhere it has never synced. + // A read that failed is not that answer, and forgetting on it would erase a time still true. + if ((await this.loadSyncedBefore()) === "unsynced") { + this.forgetSynced(); + } this.logger.info("settings saved"); this.toaster.show(toastFor({ kind: "settingsSaved" })); // Saving a connection is the moment someone has said what they want and nothing has happened diff --git a/src/status/status.test.ts b/src/status/status.test.ts new file mode 100644 index 0000000..68f4e5c --- /dev/null +++ b/src/status/status.test.ts @@ -0,0 +1,179 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { + agoLabel, + DEFAULT_STATUS, + lastSyncedFrom, + noteKind, + noteProgress, + noteSynced, + noteUnsynced, + type Status, + type View, + view, +} from "./status.ts"; + +// NOW is the clock every case below is read against, a round number so the ages in each row are +// arithmetic anyone can check by eye. +const NOW = 1_000_000_000; + +// status returns a resting status with everything at its zero value except what a case moves, so +// each row proves which rule answered rather than which field happened to be set. +function status(over: Partial = {}): Status { + return { ...DEFAULT_STATUS, ...over }; +} + +test("view: every state says the same thing in the bar and the tooltip", () => { + const cases: { name: string; status: Status; want: View }[] = [ + { + name: "a vault that has never synced says so rather than showing a time it does not have", + status: status(), + want: { + icon: "cloud", + label: "Not synced yet", + tooltip: "Geode: not synced yet; click to sync", + }, + }, + { + name: "a pass that landed seconds ago reads as now, not as a rounded down zero", + status: status({ lastSyncedAt: NOW - 30_000 }), + want: { + icon: "cloud", + label: "Synced just now", + tooltip: "Geode: last synced just now; click to sync", + }, + }, + { + name: "an idle vault carries the age of its last pass, which is the whole point of it", + status: status({ lastSyncedAt: NOW - 2 * 60_000 }), + want: { + icon: "cloud", + label: "Synced 2m ago", + tooltip: "Geode: last synced 2m ago; click to sync", + }, + }, + { + name: "a pass with no plan yet says it is looking, since a first sync spends minutes here", + status: status({ kind: "syncing" }), + want: { + icon: "refresh-cw", + label: "Checking...", + tooltip: "Geode: checking for changes", + }, + }, + { + name: "a pass with a plan counts it down, which is what a spinner cannot say", + status: status({ kind: "syncing", progress: { done: 12, total: 340 } }), + want: { + icon: "refresh-cw", + label: "Syncing 12/340", + tooltip: "Geode: syncing, 12 of 340 changes applied", + }, + }, + { + name: "a plan of nothing never claims to be 0/0, it is still looking", + status: status({ kind: "syncing", progress: { done: 0, total: 0 } }), + want: { + icon: "refresh-cw", + label: "Checking...", + tooltip: "Geode: checking for changes", + }, + }, + { + name: "a failure leads with the failure and still says when the vault was last current", + status: status({ kind: "error", detail: "2 file(s) failed to sync", lastSyncedAt: NOW }), + want: { + icon: "cloud-alert", + label: "Sync failed", + tooltip: "Geode: 2 file(s) failed to sync, last synced just now", + }, + }, + { + name: "a failure before the first pass has nothing to add, so it adds nothing", + status: status({ kind: "error", detail: "storage isn't configured yet" }), + want: { + icon: "cloud-alert", + label: "Sync failed", + tooltip: "Geode: storage isn't configured yet", + }, + }, + { + name: "a failure nothing anticipated still reads as a sentence", + status: status({ kind: "error" }), + want: { + icon: "cloud-alert", + label: "Sync failed", + tooltip: "Geode: sync failed", + }, + }, + { + name: "a paused device says so, and says the click still works", + status: status({ kind: "paused", lastSyncedAt: NOW - 3 * 3_600_000 }), + want: { + icon: "cloud-off", + label: "Sync paused", + tooltip: "Geode: automatic sync paused, last synced 3h ago; click to sync once", + }, + }, + ]; + + for (const testCase of cases) { + assert.deepEqual(view(testCase.status, NOW), testCase.want, testCase.name); + } +}); + +test("agoLabel: each unit holds up to the boundary of the next one", () => { + const cases: { name: string; elapsed: number; want: string }[] = [ + { name: "the same instant", elapsed: 0, want: "just now" }, + { name: "one second short of a minute", elapsed: 59_999, want: "just now" }, + { name: "exactly a minute", elapsed: 60_000, want: "1m ago" }, + { name: "part minutes round down rather than up", elapsed: 119_000, want: "1m ago" }, + { name: "one second short of an hour", elapsed: 3_599_000, want: "59m ago" }, + { name: "exactly an hour", elapsed: 3_600_000, want: "1h ago" }, + { name: "one second short of a day", elapsed: 86_399_000, want: "23h ago" }, + { name: "exactly a day", elapsed: 86_400_000, want: "1d ago" }, + { name: "a long weekend away", elapsed: 4 * 86_400_000, want: "4d ago" }, + ]; + + for (const testCase of cases) { + assert.equal(agoLabel(NOW - testCase.elapsed, NOW), testCase.want, testCase.name); + } +}); + +test("agoLabel: a clock that moved backwards reads as now rather than as a negative age", () => { + assert.equal(agoLabel(NOW + 60_000, NOW), "just now"); +}); + +test("noteKind: moving to a new state drops the count belonging to the pass that reported it", () => { + const syncing = noteProgress(status({ kind: "syncing" }), 12, 340); + + assert.deepEqual(noteKind(syncing, "idle", ""), status()); + assert.deepEqual(noteKind(syncing, "error", "2 file(s) failed to sync"), { + ...status(), + detail: "2 file(s) failed to sync", + kind: "error", + }); +}); + +test("noteSynced and noteUnsynced move the time and nothing else", () => { + const failed = status({ detail: "storage is unwell", kind: "error" }); + const synced = noteSynced(failed, NOW); + + assert.deepEqual(synced, { ...failed, lastSyncedAt: NOW }); + assert.deepEqual(noteUnsynced(synced), failed); +}); + +test("lastSyncedFrom: only a positive number is a time, everything else is never", () => { + const cases: { name: string; stored: unknown; want: number }[] = [ + { name: "a stored time", stored: NOW, want: NOW }, + { name: "nothing stored yet", stored: null, want: 0 }, + { name: "a key that was cleared", stored: undefined, want: 0 }, + { name: "a zero written by an older build", stored: 0, want: 0 }, + { name: "a clock that wrote a negative", stored: -1, want: 0 }, + { name: "something that is not a number at all", stored: "yesterday", want: 0 }, + ]; + + for (const testCase of cases) { + assert.equal(lastSyncedFrom(testCase.stored), testCase.want, testCase.name); + } +}); diff --git a/src/status/status.ts b/src/status/status.ts new file mode 100644 index 0000000..5040bf4 --- /dev/null +++ b/src/status/status.ts @@ -0,0 +1,180 @@ +// Deciding what the status bar says, kept apart from putting it on screen: every state below is one +// row of a table, so the wording is pinned by a test rather than assembled inside the plugin class. +// The case for each row is in docs/technical_plugin.md. + +// DEFAULT_STATUS is the complete zero value: resting, nothing wrong, never synced, nothing in +// flight. +export const DEFAULT_STATUS: Status = { + detail: "", + kind: "idle", + lastSyncedAt: 0, + progress: null, +}; + +// LAST_SYNCED_KEY is where the time of the last completed pass is kept: vault scoped localStorage, +// not data.json, since when this device last synced is a fact about this device rather than +// something every device should inherit. +export const LAST_SYNCED_KEY = "geode-last-synced-at"; + +// DAY_MS, HOUR_MS, and MINUTE_MS are the three thresholds a relative time steps through; a fourth +// unit would mean weeks, and "synced 2w ago" is a sentence nobody should ever read from a sync tool. +const DAY_MS = 86_400_000; +const HOUR_MS = 3_600_000; +const MINUTE_MS = 60_000; + +// Kind is what the status bar is currently reflecting: the two resting states, the one in flight, +// and the one that needs answering. +export type Kind = "error" | "idle" | "paused" | "syncing"; + +// Progress is how far through its plan a running pass has got, counted in actions. +export type Progress = { done: number; total: number }; + +// Status is everything the status bar reflects, held as data so the plugin never has to remember +// which of several fields it last wrote. +export type Status = { + // detail is why the last pass failed, empty when nothing did. + detail: string; + kind: Kind; + // lastSyncedAt is when a pass last completed, zero if none ever has on this device. + lastSyncedAt: number; + // progress is null until a pass knows what it is about to do, which is most of a long first sync. + progress: Progress | null; +}; + +// View is one rendered status bar item: the icon, the text beside it, and the hover text. +export type View = { icon: string; label: string; tooltip: string }; + +// agoLabel returns how long ago then was in the coarsest unit that still answers the question, and +// treats a future timestamp as now, since a clock that moved backwards is not worth a case. +export function agoLabel(then: number, now: number): string { + const elapsed = now - then; + if (elapsed < MINUTE_MS) { + return "just now"; + } + if (elapsed < HOUR_MS) { + return `${Math.floor(elapsed / MINUTE_MS)}m ago`; + } + if (elapsed < DAY_MS) { + return `${Math.floor(elapsed / HOUR_MS)}h ago`; + } + + return `${Math.floor(elapsed / DAY_MS)}d ago`; +} + +// lastSyncedFrom returns the time held in a stored value, reading anything that is not a positive +// number as never, so an absent or damaged one says "not synced yet" rather than lying about a date. +export function lastSyncedFrom(stored: unknown): number { + if (typeof stored === "number" && stored > 0) { + return stored; + } + + return 0; +} + +// noteKind moves the status bar to kind, dropping any count with it: a count belongs to the pass +// that reported it, and one left behind would describe a pass that has already ended. +export function noteKind(status: Status, kind: Kind, detail: string): Status { + return { ...status, detail, kind, progress: null }; +} + +// noteProgress records how far through its plan the running pass has got. +export function noteProgress(status: Status, done: number, total: number): Status { + return { ...status, progress: { done, total } }; +} + +// noteSynced records a pass completing, which is the one event the resting label is about. +export function noteSynced(status: Status, at: number): Status { + return { ...status, lastSyncedAt: at }; +} + +// noteUnsynced forgets when this device last synced, for a vault repointed at a bucket it has never +// synced: the old time is then about somewhere else, and a confident wrong answer is the worst one. +export function noteUnsynced(status: Status): Status { + return { ...status, lastSyncedAt: 0 }; +} + +// view returns what the status bar shows for status at now. +export function view(status: Status, now: number): View { + return { + icon: iconFor(status.kind), + label: labelFor(status, now), + tooltip: tooltipFor(status, now), + }; +} + +// iconFor returns the status bar icon for kind. +function iconFor(kind: Kind): string { + if (kind === "syncing") { + return "refresh-cw"; + } + if (kind === "error") { + return "cloud-alert"; + } + if (kind === "paused") { + return "cloud-off"; + } + + return "cloud"; +} + +// labelFor returns the text beside the icon, which is the whole of what someone reads without +// hovering, and on a phone the whole of what they can read at all. +function labelFor(status: Status, now: number): string { + if (status.kind === "syncing") { + if (status.progress === null || status.progress.total === 0) { + return "Checking..."; + } + + return `Syncing ${status.progress.done}/${status.progress.total}`; + } + if (status.kind === "error") { + return "Sync failed"; + } + if (status.kind === "paused") { + return "Sync paused"; + } + if (status.lastSyncedAt === 0) { + return "Not synced yet"; + } + + return `Synced ${agoLabel(status.lastSyncedAt, now)}`; +} + +// tooltipFor returns the hover text, which says the same thing as the label plus the part that does +// not fit: the failure, or how a click behaves in this state. +function tooltipFor(status: Status, now: number): string { + if (status.kind === "syncing") { + if (status.progress === null || status.progress.total === 0) { + return "Geode: checking for changes"; + } + const { done, total } = status.progress; + + return `Geode: syncing, ${done} of ${total} changes applied`; + } + if (status.kind === "error") { + // A pass can fail in a way nothing anticipated, and a tooltip reading "Geode: " helps nobody. + if (status.detail === "") { + return `Geode: sync failed${sinceClause(status, now)}`; + } + + return `Geode: ${status.detail}${sinceClause(status, now)}`; + } + if (status.kind === "paused") { + return `Geode: automatic sync paused${sinceClause(status, now)}; click to sync once`; + } + if (status.lastSyncedAt === 0) { + return "Geode: not synced yet; click to sync"; + } + + return `Geode: last synced ${agoLabel(status.lastSyncedAt, now)}; click to sync`; +} + +// sinceClause returns the trailing ", last synced ..." a state carries when the news it leads with +// is something other than the time, and nothing at all before the first pass has ever landed. +function sinceClause(status: Status, now: number): string { + if (status.lastSyncedAt === 0) { + return ""; + } + + return `, last synced ${agoLabel(status.lastSyncedAt, now)}`; +} diff --git a/src/sync/execute.test.ts b/src/sync/execute.test.ts index c6fcb68..631d4bf 100644 --- a/src/sync/execute.test.ts +++ b/src/sync/execute.test.ts @@ -1509,3 +1509,46 @@ test("executeSyncPlan: conflict with hash mismatch on remote restore leaves the assert.deepEqual(completed, []); assert.deepEqual(pushedFiles, []); }); + +test("executeSyncPlan: progress is reported once per action, whether it worked or not", async () => { + // A count that stalls on the first failed file reads as the hang it is there to disprove, so + // every action reports, and the total is the plan rather than what succeeded. + const reader = fakeReader({ "a.md": "one", "b.md": "two" }); + const { writer } = fakeLocalWriter(); + const { storage } = fakeStorage(); + const unwell = blobKeyFor(await hashOf("two")); + const put = storage.putObject; + storage.putObject = async (key, body, condition) => { + if (key === unwell) { + return { ok: false, status: "server", message: "storage is unwell" }; + } + + return put(key, body, condition); + }; + const reported: number[][] = []; + + const { completed, failed } = await executeSyncPlan( + [ + { kind: "push", path: "a.md" }, + { kind: "push", path: "b.md" }, + { kind: "pushDelete", path: "c.md" }, + ], + empty, + reader, + writer, + storage, + 1, + empty, + null, + "", + (done, total) => reported.push([done, total]), + ); + + assert.equal(completed.length, 2); + assert.equal(failed.length, 1); + assert.deepEqual(reported, [ + [1, 3], + [2, 3], + [3, 3], + ]); +}); diff --git a/src/sync/execute.ts b/src/sync/execute.ts index b8cf2f2..90d261a 100644 --- a/src/sync/execute.ts +++ b/src/sync/execute.ts @@ -21,6 +21,10 @@ const HASH_MISMATCH_MESSAGE = "fetched bytes do not match manifest hash; sync ag const MANIFEST_DRIFT_MESSAGE = "changed remotely mid sync; sync again to reconcile"; const MANIFEST_MISSING_HASH_MESSAGE = "manifest missing expected hash for this path"; +// NO_PROGRESS is the default for a caller with nothing watching, so the loop reports unconditionally +// rather than asking whether anyone is listening. +const NO_PROGRESS: Progress = () => undefined; + // ExecuteResult reports what executeSyncPlan carried out: completed and failed actions, per file // failures, and pushedFiles, the FileState of every blob a bucket write actually landed, whether // or not the action it belonged to ultimately failed. @@ -39,6 +43,10 @@ export type LocalWriter = { renameFile: (path: string, newPath: string) => Promise; }; +// Progress is called once per action, completed or failed, so a caller can say how far along a +// long pass is while it runs rather than showing a spinner for minutes. +export type Progress = (done: number, total: number) => void; + // StagedWrite is pulled content already written to a staging file beside its destination, waiting // to either claim that path on commit or be thrown away by discard. export type StagedWrite = { @@ -79,6 +87,7 @@ export async function executeSyncPlan( remote: Snapshot = { files: [] }, manifestEtag: string | null = null, deviceId = "", + onProgress: Progress = NO_PROGRESS, ): Promise { const completed: SyncAction[] = []; const failed: SyncAction[] = []; @@ -104,12 +113,15 @@ export async function executeSyncPlan( } if (actionResult.failures.length === 0) { completed.push(action); - continue; - } - failed.push(action); - for (const failure of actionResult.failures) { - failures.push(failure); + } else { + failed.push(action); + for (const failure of actionResult.failures) { + failures.push(failure); + } } + // Counted as attempted rather than succeeded: a pass that failed halfway still moved, and a + // progress count that stalls on the first failure reads as the hang it is reporting on. + onProgress(completed.length + failed.length, actions.length); } return { completed, failed, failures, pushedFiles }; diff --git a/src/sync/sync.test.ts b/src/sync/sync.test.ts index 517a369..0669be3 100644 --- a/src/sync/sync.test.ts +++ b/src/sync/sync.test.ts @@ -1426,3 +1426,31 @@ test("revertFailedPaths: a path the ancestor never knew is dropped, so it re-pla assert.deepEqual(result, empty); }); + +test("syncOnce: the plan's size is reported before the first action, and its end when it lands", async () => { + // The first action of a big pull can outlast anyone's patience on its own, so the count has to + // exist before it starts rather than only once it finishes. + const reader = fakeReader({ "a.md": "alpha", "b.md": "beta" }); + const { writer } = fakeLocalWriter(); + const { storage } = fakeStorage(); + const reported: number[][] = []; + + const outcome = await syncOnce( + empty, + reader, + writer, + storage, + 1, + () => "minted-id", + "", + null, + (done, total) => reported.push([done, total]), + ); + + assert.ok(outcome.ok); + assert.deepEqual(reported, [ + [0, 2], + [1, 2], + [2, 2], + ]); +}); diff --git a/src/sync/sync.ts b/src/sync/sync.ts index 4231b7c..ef37913 100644 --- a/src/sync/sync.ts +++ b/src/sync/sync.ts @@ -9,7 +9,7 @@ import { type Snapshot, takeSnapshot, } from "../vault/vault.ts"; -import { executeSyncPlan, type LocalWriter, type SyncFailure } from "./execute.ts"; +import { executeSyncPlan, type LocalWriter, type Progress, type SyncFailure } from "./execute.ts"; import { type MassChange, massChangeApproved, massChangeFor, massChangeHalts } from "./guard.ts"; import { BLOB_PREFIX, @@ -233,6 +233,7 @@ export async function syncOnce( newVaultId: () => string = () => crypto.randomUUID(), deviceId = "", confirmed: MassChange | null = null, + onProgress: Progress = () => undefined, ): Promise { const [remote, sentinelResult] = await Promise.all([ readRemoteManifest(storage), @@ -335,6 +336,9 @@ export async function syncOnce( }; } + // Reported before the first action rather than after it: the plan's size is the answer to "is + // this hung", and the first action of a big pull can take longer than the patience it is spending. + onProgress(0, actions.length); const executed = await executeSyncPlan( actions, local, @@ -345,6 +349,7 @@ export async function syncOnce( remoteView, manifestEtag, deviceId, + onProgress, ); // The manifest is derived from what the plan just did to the bucket, never a fresh disk snapshot, diff --git a/styles.css b/styles.css index 37186a5..e171221 100644 --- a/styles.css +++ b/styles.css @@ -160,6 +160,18 @@ white-space: pre-wrap; } +.geode-status-bar { + display: inline-flex; + align-items: center; + gap: 0.4em; +} + +/* The icon box is sized to the icon so the label beside it does not shift as the icon changes. */ +.geode-status-icon { + display: inline-flex; + align-items: center; +} + .geode-status-bar.is-error { color: var(--text-error); }