diff --git a/.agents/skills/ponytail/SKILL.md b/.agents/skills/ponytail/SKILL.md new file mode 100644 index 0000000..02c0712 --- /dev/null +++ b/.agents/skills/ponytail/SKILL.md @@ -0,0 +1,120 @@ +--- +name: ponytail +description: > + Forces the laziest solution that actually works, simplest, shortest, most + minimal. Channels a senior dev who has seen everything: question whether the + task needs to exist at all (YAGNI), reach for the standard library before + custom code, native platform features before dependencies, one line before + fifty. Supports intensity levels: lite, full (default), ultra. Use on ANY + coding task: writing, adding, refactoring, fixing, reviewing, or designing + code, and choosing libraries or dependencies. Also use whenever the user + says "ponytail", "be lazy", "lazy mode", "simplest solution", "minimal + solution", "yagni", "do less", or "shortest path", or complains about + over-engineering, bloat, boilerplate, or unnecessary dependencies. Do NOT + use for non-coding requests (general knowledge, prose, translation, + summaries, recipes). +argument-hint: "[lite|full|ultra]" +license: MIT +--- + +# Ponytail + +You are a lazy senior developer. Lazy means efficient, not careless. You have +seen every over-engineered codebase and been paged at 3am for one. The best +code is the code never written. + +## Persistence + +ACTIVE EVERY RESPONSE. No drift back to over-building. Still active if +unsure. Off only: "stop ponytail" / "normal mode". Default: **full**. +Switch: `/ponytail lite|full|ultra`. + +## The ladder + +Stop at the first rung that holds: + +1. **Does this need to exist at all?** Speculative need = skip it, say so in one line. (YAGNI) +2. **Already in this codebase?** A helper, util, type, or pattern that already lives here → reuse it. Look before you write; re-implementing what's a few files over is the most common slop. +3. **Stdlib does it?** Use it. +4. **Native platform feature covers it?** `` over a picker lib, CSS over JS, DB constraint over app code. +5. **Already-installed dependency solves it?** Use it. Never add a new one for what a few lines can do. +6. **Can it be one line?** One line. +7. **Only then:** the minimum code that works. + +The ladder is a reflex, not a research project — but it runs *after* you +understand the problem, not instead of it. Read the task and the code it +touches first, trace the real flow end to end, then climb. Two rungs work → +take the higher one and move on. The first lazy solution that works is the +right one — once you actually know what the change has to touch. + +**Bug fix = root cause, not symptom.** A report names a symptom. Before you +edit, grep every caller of the function you're about to touch. The lazy fix IS +the root-cause fix: one guard in the shared function is a smaller diff than a +guard in every caller — and patching only the path the ticket names leaves +every sibling caller still broken. Fix it once, where all callers route through. + +## Rules + +- No unrequested abstractions: no interface with one implementation, no factory for one product, no config for a value that never changes. +- No boilerplate, no scaffolding "for later", later can scaffold for itself. +- Deletion over addition. Boring over clever, clever is what someone decodes at 3am. +- Fewest files possible. Shortest working diff wins — but only once you understand the problem. The smallest change in the wrong place isn't lazy, it's a second bug. +- Complex request? Ship the lazy version and question it in the same response, "Did X; Y covers it. Need full X? Say so." Never stall on an answer you can default. +- Two stdlib options, same size? Take the one that's correct on edge cases. Lazy means writing less code, not picking the flimsier algorithm. +- Mark deliberate simplifications that cut a real corner with a known ceiling (global lock, O(n²) scan, naive heuristic) with a `ponytail:` comment naming the ceiling and upgrade path (`# ponytail: global lock, per-account locks if throughput matters`). + +## Output + +Code first. Then at most three short lines: what was skipped, when to add it. +No essays, no feature tours, no design notes. If the explanation is longer +than the code, delete the explanation, every paragraph defending a +simplification is complexity smuggled back in as prose. Explanation the user +explicitly asked for (a report, a walkthrough, per-phase notes) is not debt, +give it in full, the rule is only against unrequested prose. + +Pattern: `[code] → skipped: [X], add when [Y].` + +## Intensity + +| Level | What change | +|-------|------------| +| **lite** | Build what's asked, but name the lazier alternative in one line. User picks. | +| **full** | The ladder enforced. Stdlib and native first. Shortest diff, shortest explanation. Default. | +| **ultra** | YAGNI extremist. Deletion before addition. Ship the one-liner and challenge the rest of the requirement in the same breath. | + +Example: "Add a cache for these API responses." +- lite: "Done, cache added. FYI: `functools.lru_cache` covers this in one line if you'd rather not own a cache class." +- full: "`@lru_cache(maxsize=1000)` on the fetch function. Skipped custom cache class, add when lru_cache measurably falls short." +- ultra: "No cache until a profiler says so. When it does: `@lru_cache`. A hand-rolled TTL cache class is a bug farm with a hit rate." + +## When NOT to be lazy + +Never simplify away: input validation at trust boundaries, error handling +that prevents data loss, security measures, accessibility basics, anything +explicitly requested. User insists on the full version → build it, no +re-arguing. + +Never lazy about understanding the problem. The ladder shortens the +solution, never the reading. Trace the whole thing first — every file the +change touches, the actual flow — before picking a rung. Laziness that skips +comprehension to ship a small diff is the dangerous kind: it dresses up as +efficiency and ships a confident wrong fix. Read fully, then be lazy. + +Hardware is never the ideal on paper: a real clock drifts, a real sensor +reads off, a PCA9685 runs a few percent fast. Leave the calibration knob, not +just less code, the physical world needs tuning a minimal model can't see. + +Lazy code without its check is unfinished. Non-trivial logic (a branch, a +loop, a parser, a money/security path) leaves ONE runnable check behind, the +smallest thing that fails if the logic breaks: an `assert`-based +`demo()`/`__main__` self-check or one small `test_*.py`. No frameworks, no +fixtures, no per-function suites unless asked. Trivial one-liners need no +test, YAGNI applies to tests too. + +## Boundaries + +Ponytail governs what you build, not how you talk (pair with Caveman for +terse prose). "stop ponytail" / "normal mode": revert. Level persists until +changed or session end. + +The shortest path to done is the right path. diff --git a/.gitignore b/.gitignore index 6b7caf6..a95bf4c 100644 --- a/.gitignore +++ b/.gitignore @@ -2,7 +2,7 @@ main.js /dist/ -# Dev vault (created by `make create-dev-vault`) +# Dev vault (created by `npm run create-dev-vault`) /dev-vault/ /data.json /state.json diff --git a/AGENTS.md b/AGENTS.md index db0a412..94154be 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,7 +58,7 @@ via relevent simple diagrams. Less is always more, simple is always better, boring is best, avoid the magic! Whilst still meeting requirements, being secure, and delivering value to our users. -## Code Style +## Code Style & Implementation The `typescript-as-go` skill (`.agents/skills/typescript-as-go/SKILL.md`) is the source of truth for how TypeScript is written here, comments included. Abide by every rule in it, no exceptions, and @@ -72,3 +72,7 @@ Additional rules for the project: class; the plugin class is the one default export Obsidian requires 3. Framework code stays thin glue; logic lives in pure modules that never import `obsidian` 4. `erasableSyntaxOnly` in tsconfig enforces strippable syntax + +Also adhere to the `ponytail` skill (`.agents/skills/ponytail/SKILL.md`) when writing code and +planning/making changes, to ensure that this is a codebase without bloat. The rules in this skill +come second to the ones above and in the `typescript-as-go` skill. diff --git a/docs/project_roadmap.md b/docs/project_roadmap.md index 28e7802..4588a1a 100644 --- a/docs/project_roadmap.md +++ b/docs/project_roadmap.md @@ -6,7 +6,7 @@ Planned upcoming releases for **Geode**: | ------- | ------- | ---------------------------------------------------------------- | | `0.1.0` | Bedrock | Two desktop devices can sync a vault, using a provided S3 bucket | | `0.2.0` | | iOS & Android support | -| `0.3.0` | | Encyption | +| `0.3.0` | | Encryption | | `0.4.0` | | MCP | | `0.5.0` | | API | | `0.6.0` | | CLI | diff --git a/skills-lock.json b/skills-lock.json index 9758614..1b14fd1 100644 --- a/skills-lock.json +++ b/skills-lock.json @@ -1,6 +1,12 @@ { "version": 1, "skills": { + "ponytail": { + "source": "dietrichgebert/ponytail", + "sourceType": "github", + "skillPath": "skills/ponytail/SKILL.md", + "computedHash": "210fa0831f649ce9f8a305f7b7c8cf2a0dd09ac4dd53067109941b96e7811d4e" + }, "typescript-as-go": { "source": "revett/typescript-as-go", "sourceType": "github", diff --git a/src/device/device.test.ts b/src/device/device.test.ts index ddd02b6..1e41542 100644 --- a/src/device/device.test.ts +++ b/src/device/device.test.ts @@ -4,7 +4,7 @@ import { conflictCopyPath } from "../sync/plan.ts"; import { isSafePath } from "../vault/vault.ts"; import { DEVICE_ID_KEY, deviceIdFrom, deviceSuffixFrom } from "./device.ts"; -test("deviceSuffixFrom: five bytes encode to eight base32 characters (#103)", () => { +test("deviceSuffixFrom: five bytes encode to eight base32 characters", () => { // 40 bits split into eight 5-bit groups holding 0 through 7 in order, so the expected output // reads straight off the front of the alphabet. const suffix = deviceSuffixFrom(new Uint8Array([0x00, 0x44, 0x32, 0x14, 0xc7])); @@ -12,7 +12,7 @@ test("deviceSuffixFrom: five bytes encode to eight base32 characters (#103)", () assert.equal(suffix, "01234567"); }); -test("deviceSuffixFrom: the alphabet is lowercase and skips the ambiguous letters (#103)", () => { +test("deviceSuffixFrom: the alphabet is lowercase and skips the ambiguous letters", () => { // One case throughout is what stops two device IDs colliding by case alone, and i, l, o and u // are absent so a suffix read off a filename cannot be transcribed back wrong. const every = deviceSuffixFrom(new Uint8Array([255, 255, 255, 255, 255])); @@ -44,7 +44,7 @@ test("deviceIdFrom: an empty half degrades to the other rather than leaving a st assert.equal(deviceIdFrom("mac", ""), "mac"); }); -test("deviceIdFrom: every generated ID is safe in a conflict copy path (#103)", () => { +test("deviceIdFrom: every generated ID is safe in a conflict copy path", () => { // The ID lands in a filename written to disk, so it has to clear the same rules a pulled // manifest entry does, and must never introduce uppercase that could let two devices collide by // case alone. diff --git a/src/log/adapter.ts b/src/log/adapter.ts index 4e1d035..184a575 100644 --- a/src/log/adapter.ts +++ b/src/log/adapter.ts @@ -23,6 +23,7 @@ export function createLogSink( if (dir === undefined) { return createMemorySink(maxLines); } + return createObsidianLogSink(adapter, `${dir}/geode.log`, maxLines); } @@ -74,6 +75,7 @@ export function createObsidianLogSink( entries.push(entry); } } + return entries; }, clear: async () => { diff --git a/src/log/log.ts b/src/log/log.ts index d5d9a45..8974504 100644 --- a/src/log/log.ts +++ b/src/log/log.ts @@ -145,9 +145,26 @@ export function parseLogLine(line: string): LogEntry | undefined { if (Number.isNaN(time) || !isLogLevel(rawLevel)) { return undefined; } + return { time, level: rawLevel, message: unescapeMessage(rest.join("\t")) }; } +// trimLogLines keeps only the last maxLines lines of a log, dropping the oldest. The result keeps +// the same trailing newline the input had: appending assumes the file already ends in one, and +// dropping it here would glue the next appended line onto the last one still kept. +export function trimLogLines(text: string, maxLines: number): string { + const lines = linesOf(text); + let kept = lines; + if (lines.length > maxLines) { + kept = lines.slice(lines.length - maxLines); + } + if (kept.length === 0) { + return ""; + } + + return `${kept.join("\n")}\n`; +} + // unescapeMessage reverses escapeMessage. Unlike escape, unescape must // scan character by character to avoid matching "\n" inside the stored "\\" sequence. export function unescapeMessage(msg: string): string { @@ -175,21 +192,6 @@ export function unescapeMessage(msg: string): string { return result; } -// trimLogLines keeps only the last maxLines lines of a log, dropping the oldest. The result keeps -// the same trailing newline the input had: appending assumes the file already ends in one, and -// dropping it here would glue the next appended line onto the last one still kept. -export function trimLogLines(text: string, maxLines: number): string { - const lines = linesOf(text); - let kept = lines; - if (lines.length > maxLines) { - kept = lines.slice(lines.length - maxLines); - } - if (kept.length === 0) { - return ""; - } - return `${kept.join("\n")}\n`; -} - // consoleFor returns the console method matching level, so console and persisted output agree on // severity. function consoleFor(level: LogLevel): (message: string) => void { @@ -199,6 +201,7 @@ function consoleFor(level: LogLevel): (message: string) => void { if (level === "error") { return (message) => console.error(message); } + return (message) => console.log(message); } @@ -217,6 +220,7 @@ function linesOf(text: string): string[] { if (parts[parts.length - 1] === "") { return parts.slice(0, -1); } + return parts; } diff --git a/src/main.ts b/src/main.ts index b79f799..800007a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,13 +1,13 @@ import type { App } from "obsidian"; import { Platform, Plugin, setIcon, setTooltip } from "obsidian"; -import { DEVICE_ID_KEY, deviceIdFrom, deviceSuffixFrom } from "./device/device"; -import { createLogSink } from "./log/adapter"; -import { createLogBus, createLogger, type LogBus, type Logger, type LogSink } from "./log/log"; -import { GeodeLogView, LOG_VIEW_TYPE } from "./log/view"; -import { DEFAULT_PASS, type Pass, toastFor } from "./notify/notify"; -import { createToaster, type Toaster } from "./notify/obsidian"; -import { type Actions, GeodeOnboardingModal } from "./onboarding/modal"; -import { type RemoteRead, readRemote, type SyncReport } from "./onboarding/onboarding"; +import { DEVICE_ID_KEY, deviceIdFrom, deviceSuffixFrom } from "./device/device.ts"; +import { createLogSink } from "./log/adapter.ts"; +import { createLogBus, createLogger, type LogBus, type Logger, type LogSink } from "./log/log.ts"; +import { GeodeLogView, LOG_VIEW_TYPE } from "./log/view.ts"; +import { DEFAULT_PASS, type Pass, toastFor } from "./notify/notify.ts"; +import { createToaster, type Toaster } from "./notify/obsidian.ts"; +import { type Actions, GeodeOnboardingModal } from "./onboarding/modal.ts"; +import { type RemoteRead, readRemote, type SyncReport } from "./onboarding/onboarding.ts"; import { armed, DEFAULT_STATE, @@ -23,15 +23,15 @@ import { type State, TICK_MS, type Trigger, -} from "./schedule/schedule"; +} from "./schedule/schedule.ts"; import { DEFAULT_SETTINGS, type GeodeSettings, hasConnectionConfig, normalizeSettings, prefixError, -} from "./settings/settings"; -import { GeodeSettingTab } from "./settings/tab"; +} from "./settings/settings.ts"; +import { GeodeSettingTab } from "./settings/tab.ts"; import { DEFAULT_STATUS, type Kind, @@ -43,18 +43,22 @@ import { noteUnsynced, type Status, view, -} from "./status/status"; -import { obsidianTransport } from "./storage/obsidian"; -import { createS3Client, probeConditionalWrites } from "./storage/storage"; -import type { MassChange } from "./sync/guard"; -import { GeodeMassChangeModal } from "./sync/modal"; -import { type SyncFault, syncOnce } from "./sync/sync"; +} from "./status/status.ts"; +import { obsidianTransport } from "./storage/obsidian.ts"; +import { createS3Client, probeConditionalWrites } from "./storage/storage.ts"; +import type { MassChange } from "./sync/guard.ts"; +import { GeodeMassChangeModal } from "./sync/modal.ts"; +import { type SyncFault, syncOnce } from "./sync/sync.ts"; import { createObsidianLocalWriter, createObsidianReader, createObsidianStore, flushOpenEditors, -} from "./vault/obsidian"; +} from "./vault/obsidian.ts"; + +// DEVICE_SUFFIX_BYTES is how much randomness separates two devices carrying the same platform +// label. Five bytes encode to exactly eight base32 characters with nothing left over. +const DEVICE_SUFFIX_BYTES = 5; // LOG_MIN_LEVEL is fixed rather than user configurable: there's no meaningful "quiet" mode to // offer today, so a verbosity setting would be a toggle with no observable effect. @@ -64,10 +68,6 @@ const LOG_MIN_LEVEL = "debug"; // grow it unbounded. const MAX_LOG_LINES = 500; -// DEVICE_SUFFIX_BYTES is how much randomness separates two devices carrying the same platform -// label. Five bytes encode to exactly eight base32 characters with nothing left over. -const DEVICE_SUFFIX_BYTES = 5; - // AppWithSetting adds Obsidian's internal, undocumented settings-window API (there is no public // equivalent) so the Settings command can jump straight to Geode's tab, and opening the log view // can close the settings modal out from under itself. @@ -206,6 +206,7 @@ export default class GeodePlugin extends Plugin { if (!checking) { this.offerOnboarding(); } + return true; }, }); @@ -221,6 +222,7 @@ export default class GeodePlugin extends Plugin { if (!checking) { this.setPaused(true); } + return true; }, }); @@ -234,6 +236,7 @@ export default class GeodePlugin extends Plugin { if (!checking) { this.setPaused(false); } + return true; }, }); @@ -342,6 +345,7 @@ export default class GeodePlugin extends Plugin { for (const file of files) { paths.push(file.path); } + return paths; }, openLogs: () => void this.openLogView(), diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 167a494..ac911f3 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -14,9 +14,6 @@ export const DEFAULT_SETTINGS: GeodeSettings = { // ConnectionStatus is the current in-memory state of a Test Connection check. export type ConnectionStatus = "unknown" | "checking" | "ok" | "error"; -// Provider identifies a supported S3 compatible storage configuration. -export type Provider = "r2" | "s3" | "custom" | "minio"; - // GeodeSettings is the persisted shape of a Geode plugin's user configuration; see // docs/technical_settings.md for why each field is normalized where it is used rather than saved. export type GeodeSettings = { @@ -35,6 +32,9 @@ export type GeodeSettings = { secretId: string; }; +// Provider identifies a supported S3 compatible storage configuration. +export type Provider = "r2" | "s3" | "custom" | "minio"; + // SaveTarget is the narrow persistence surface needed to save a settings draft. export type SaveTarget = { logger: { @@ -81,30 +81,8 @@ export function draftForDisplay( if (auto) { return { ...savedSettings }; } - return currentDraft; -} - -// normalizeEndpoint ensures the endpoint has an explicit scheme and no trailing slash. -export function normalizeEndpoint(endpoint: string): string { - let normalized = endpoint.trim(); - if (!normalized) { - return ""; - } - - // Require an explicit scheme to prevent generic network errors - const hasScheme = - normalized.toLowerCase().startsWith("http://") || - normalized.toLowerCase().startsWith("https://"); - if (!hasScheme) { - normalized = `https://${normalized}`; - } - - // Strip trailing slashes to prevent double-slash SigV4 canonical path issues - while (normalized.endsWith("/")) { - normalized = normalized.slice(0, -1); - } - return normalized; + return currentDraft; } // endpointFor returns the storage endpoint URL for settings, or "" when none can be derived; an @@ -136,6 +114,7 @@ export function hasConnectionConfig(settings: GeodeSettings): boolean { if (settings.provider === "s3") { return isAwsRegion(regionFor(settings)); } + // MinIO and a custom provider both take their endpoint as typed, with a region for signing. return normalizeEndpoint(settings.endpoint) !== "" && regionFor(settings) !== ""; } @@ -160,6 +139,29 @@ export function isCurrentConnectionResult( return settingsEqual(testedSettings, currentSettings); } +// normalizeEndpoint ensures the endpoint has an explicit scheme and no trailing slash. +export function normalizeEndpoint(endpoint: string): string { + let normalized = endpoint.trim(); + if (normalized === "") { + return ""; + } + + // Require an explicit scheme to prevent generic network errors + const hasScheme = + normalized.toLowerCase().startsWith("http://") || + normalized.toLowerCase().startsWith("https://"); + if (!hasScheme) { + normalized = `https://${normalized}`; + } + + // Strip trailing slashes to prevent double-slash SigV4 canonical path issues + while (normalized.endsWith("/")) { + normalized = normalized.slice(0, -1); + } + + return normalized; +} + // normalizePrefix returns the canonical bucket key prefix: whitespace trimmed and empty segments // dropped, so equivalent slash variants all address the same place. export function normalizePrefix(raw: string): string { @@ -238,6 +240,7 @@ export function providerOr(v: unknown): Provider { if (v === "s3" || v === "custom" || v === "minio") { return v; } + return "r2"; } @@ -302,5 +305,6 @@ function stringOr(v: unknown, fallback: string): string { if (typeof v === "string") { return v; } + return fallback; } diff --git a/src/settings/tab.ts b/src/settings/tab.ts index 8dd9130..3983dbd 100644 --- a/src/settings/tab.ts +++ b/src/settings/tab.ts @@ -7,9 +7,9 @@ import { SecretComponent, Setting, } from "obsidian"; -import type GeodePlugin from "../main"; -import { obsidianTransport } from "../storage/obsidian"; -import { testConnection } from "../storage/storage"; +import type GeodePlugin from "../main.ts"; +import { obsidianTransport } from "../storage/obsidian.ts"; +import { testConnection } from "../storage/storage.ts"; import { type ConnectionStatus, canSave, @@ -22,7 +22,7 @@ import { providerOr, saveDraft, settingsEqual, -} from "./settings"; +} from "./settings.ts"; // DEBUG_LABEL_WIDTH is the column width debug info labels are padded to, so values line up. const DEBUG_LABEL_WIDTH = 12; @@ -46,6 +46,7 @@ function connectionMessageFor(tab: GeodeSettingTab): string { if (tab.connectionStatus === "error") { return tab.connectionMessage; } + return "Not tested yet"; } @@ -58,6 +59,7 @@ function connectionSummary(tab: GeodeSettingTab): string { if (tab.connectionStatus === "unknown") { return ""; } + return tab.connectionStatus; } @@ -115,6 +117,7 @@ function platformLabel(): string { if (Platform.isAndroidApp) { return "Android"; } + return "Unknown"; } @@ -299,6 +302,7 @@ function renderSecretRow(tab: GeodeSettingTab, containerEl: HTMLElement): void { if (button !== null) { button.textContent = "Add secret"; } + return component; }); } diff --git a/src/status/status.ts b/src/status/status.ts index 5040bf4..2f1508d 100644 --- a/src/status/status.ts +++ b/src/status/status.ts @@ -17,7 +17,7 @@ export const DEFAULT_STATUS: Status = { 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. +// unit would mean weeks, and "synced 2w ago" is a sentence nobody should read from a sync tool. const DAY_MS = 86_400_000; const HOUR_MS = 3_600_000; const MINUTE_MS = 60_000; @@ -62,7 +62,8 @@ export function agoLabel(then: number, now: number): string { } // 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. +// 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; @@ -140,6 +141,16 @@ function labelFor(status: Status, now: number): string { return `Synced ${agoLabel(status.lastSyncedAt, now)}`; } +// 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)}`; +} + // 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 { @@ -168,13 +179,3 @@ function tooltipFor(status: Status, now: number): string { 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/storage/errors.ts b/src/storage/errors.ts index 0bd8f26..f618804 100644 --- a/src/storage/errors.ts +++ b/src/storage/errors.ts @@ -17,15 +17,6 @@ export function messageFor(err: unknown): string { ); } -// detailFor extracts the raw message from a caught error, or "" when there is nothing to quote. -function detailFor(err: unknown): string { - if (err instanceof Error) { - return err.message; - } - - return ""; -} - // statusForHttp maps an HTTP code onto a result status, treating anything unrecognised as // retryable. 409 joins 412 as a conflict, since Amazon S3 returns it for a lost conditional write. export function statusForHttp(code: number): ResultStatus { @@ -41,5 +32,15 @@ export function statusForHttp(code: number): ResultStatus { if (code === 412 || code === 409) { return "conflict"; } + return "server"; } + +// detailFor extracts the raw message from a caught error, or "" when there is nothing to quote. +function detailFor(err: unknown): string { + if (err instanceof Error) { + return err.message; + } + + return ""; +} diff --git a/src/storage/storage.test.ts b/src/storage/storage.test.ts index a750134..15b18dc 100644 --- a/src/storage/storage.test.ts +++ b/src/storage/storage.test.ts @@ -47,6 +47,7 @@ function probeStub(over: Partial): StorageClient { deleteObject: async () => ({ ok: true, status: "ok", message: "" }), listObjects: async () => ({ ok: true, status: "ok", message: "", objects: [] }), }; + return { ...base, ...over }; } @@ -354,6 +355,7 @@ function stallingTransport(): { transport: Transport; dispatched: Promise signal(); return new Promise(() => {}); }; + return { transport, dispatched }; } @@ -439,7 +441,7 @@ function listingXml(keys: string[]): string { return `${contents}`; } -test("createS3Client: every key is addressed under the configured prefix (#154)", async () => { +test("createS3Client: every key is addressed under the configured prefix", async () => { const { transport, urls } = recordingTransport(); const client = createS3Client(rootedSettings, "shh", transport); @@ -467,7 +469,7 @@ test("createS3Client: no prefix leaves every key at the bucket root", async () = assert.deepEqual(urls, ["https://s3.example.com/vault/.geode/manifest.json"]); }); -test("listObjects: lists under the prefix and hands keys back relative to it (#154)", async () => { +test("listObjects: lists under the prefix and hands keys back relative to it", async () => { // The round trip that matters: sync reads a blob's hash by slicing BLOB_PREFIX off a listed key, // so a key still carrying the bucket prefix would parse as a hash that matches nothing local and // be reported as content the vault can't explain. @@ -513,7 +515,7 @@ test("listObjects: a key outside the prefix fails the listing, it is never mis-s assert.match(result.message, /outside the configured prefix/); }); -test("createS3Client: an unusable prefix refuses every operation (#154)", async () => { +test("createS3Client: an unusable prefix refuses every operation", async () => { // Settings reach the client straight from data.json, so the settings tab's validation is not on // this path. stubTransport throws, so anything reaching the network fails rather than refuses. const client = createS3Client( @@ -554,7 +556,7 @@ test("createS3Client: an unusable prefix refuses every operation (#154)", async }); }); -test("createS3Client: a leading .. can never address a different bucket (#154)", async () => { +test("createS3Client: a leading .. can never address a different bucket", async () => { // The concrete danger, and why an unusable prefix cannot simply be dropped: signing normalizes // the URL, so "https://host/vault/../evil/x" resolves to bucket "evil". A client that built this // request at all would read and write someone else's bucket while reporting success. diff --git a/src/storage/storage.ts b/src/storage/storage.ts index d6892d5..9f4d9c9 100644 --- a/src/storage/storage.ts +++ b/src/storage/storage.ts @@ -403,6 +403,7 @@ async function s3DeleteObject( message: `Storage rejected the delete (${response.status})`, }; } + return { ok: true, status: "ok", message: "" }; } @@ -478,6 +479,7 @@ async function s3HeadObject( etag: null, }; } + return { ok: true, status: "ok", message: "", etag: response.header("etag") }; } @@ -569,6 +571,7 @@ async function s3PutObject( message: `Storage rejected the write (${response.status})`, }; } + return { ok: true, status: "ok", message: "" }; } diff --git a/src/storage/xml.ts b/src/storage/xml.ts index 279364a..a669280 100644 --- a/src/storage/xml.ts +++ b/src/storage/xml.ts @@ -49,6 +49,7 @@ export function parseListObjectsXml(xml: string): ParsedListPage { if (truncated && token !== "") { nextContinuationToken = token; } + return { ok: true, page: { objects, nextContinuationToken }, @@ -88,6 +89,7 @@ function decodeXmlText(text: string): string { if (named === "apos") { return "'"; } + return match; }, ); @@ -101,6 +103,7 @@ function fieldFrom(block: string, tag: string): string { if (found === null) { return ""; } + return found[1]; } @@ -118,5 +121,6 @@ function looseTagCount(xml: string, tag: string): number { if (found === null) { return 0; } + return found.length; } diff --git a/src/sync/execute.test.ts b/src/sync/execute.test.ts index 631d4bf..5933979 100644 --- a/src/sync/execute.test.ts +++ b/src/sync/execute.test.ts @@ -1041,6 +1041,7 @@ test("executeSyncPlan: a pull stages its payload, then checks cheapest-last, and if (key === MANIFEST_KEY) { ops.push("checkManifest"); } + return innerHead(key); }; const manifestHead = await storage.headObject(MANIFEST_KEY); @@ -1105,6 +1106,7 @@ test("executeSyncPlan: a pull whose local file is edited while the manifest chec if (key === MANIFEST_KEY) { readerFiles["a.md"] = "edited during the manifest check"; } + return innerHead(key); }; const remote = snapshot(file("a.md", hash)); @@ -1155,6 +1157,7 @@ test("executeSyncPlan: a pull refused when the edit landing during the manifest readerFiles["a.md"] = "hello werld"; readerMtimes["a.md"] = 2; } + return innerHead(key); }; const remote = snapshot(file("a.md", hash)); @@ -1201,6 +1204,7 @@ test("executeSyncPlan: a pullDelete refused when the edit landing during the man readerFiles["a.md"] = "hello werld"; readerMtimes["a.md"] = 2; } + return innerHead(key); }; @@ -1242,6 +1246,7 @@ test("executeSyncPlan: a pullDelete whose local file is edited while the manifes if (key === MANIFEST_KEY) { readerFiles["a.md"] = "edited during the manifest check"; } + return innerHead(key); }; @@ -1303,6 +1308,7 @@ test("executeSyncPlan: a conflict fetches, stages and checks the manifest before if (key === MANIFEST_KEY) { ops.push("checkManifest"); } + return innerHead(key); }; const innerPut = storage.putObject; diff --git a/src/sync/execute.ts b/src/sync/execute.ts index 90d261a..559aa60 100644 --- a/src/sync/execute.ts +++ b/src/sync/execute.ts @@ -21,8 +21,8 @@ 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. +// 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 @@ -440,6 +440,7 @@ function localFailureMessage(err: unknown): string { if (err instanceof Error) { return err.message; } + return "local file operation failed"; } diff --git a/src/sync/fake.ts b/src/sync/fake.ts index 4e3a87e..0fe092e 100644 --- a/src/sync/fake.ts +++ b/src/sync/fake.ts @@ -52,6 +52,7 @@ export function fakeLocalWriter(): { writer: LocalWriter; files: Map { @@ -84,6 +86,7 @@ export function fakeReader( if (content === undefined) { throw new Error(`no such file: ${path}`); } + return new TextEncoder().encode(content); }, stat: async (path) => { @@ -144,6 +147,7 @@ export function fakeStorage(objects: Record = {}): { if (stored !== undefined) { etag = stored; } + return { ok: true, status: "ok", @@ -166,6 +170,7 @@ export function fakeStorage(objects: Record = {}): { if (stored !== undefined) { etag = stored; } + return { ok: true, status: "ok", message: "", etag }; }, deleteObject: async (key): Promise => { @@ -181,9 +186,11 @@ export function fakeStorage(objects: Record = {}): { } objects.push({ key, size: content.length, lastModified: "" }); } + return { ok: true, status: "ok", message: "", objects }; }, }; + return { storage, objects: store }; } diff --git a/src/sync/plan.test.ts b/src/sync/plan.test.ts index 2d5572e..abee720 100644 --- a/src/sync/plan.test.ts +++ b/src/sync/plan.test.ts @@ -310,7 +310,7 @@ test("conflictCopyPath: two passes in the same second get different copies", () assert.notEqual(first, second); }); -test("conflictCopyPath: two devices never name the same copy at the same instant (#103)", () => { +test("conflictCopyPath: two devices never name the same copy at the same instant", () => { const mine = conflictCopyPath("a.md", Date.parse("2026-07-14T14:37:22.123Z"), "mac-abc"); const theirs = conflictCopyPath("a.md", Date.parse("2026-07-14T14:37:22.123Z"), "ios-xyz"); diff --git a/src/sync/plan.ts b/src/sync/plan.ts index 842260f..fc5c7b7 100644 --- a/src/sync/plan.ts +++ b/src/sync/plan.ts @@ -70,6 +70,7 @@ export function conflictCopyPath(path: string, now: number, deviceId = ""): stri if (lastDot === -1 || lastDot <= lastSlash + 1) { return `${path}_${marker}`; } + return `${path.slice(0, lastDot)}_${marker}${path.slice(lastDot)}`; } @@ -237,6 +238,7 @@ function changesByPath(changes: Change[]): Map { for (const change of changes) { result.set(change.path, change); } + return result; } diff --git a/src/sync/sync.itest.ts b/src/sync/sync.itest.ts index 2f404f9..7810010 100644 --- a/src/sync/sync.itest.ts +++ b/src/sync/sync.itest.ts @@ -82,7 +82,10 @@ async function readLocal(d: Device, path: string): Promise { // contentOf returns the payload inside an object's envelope, so an assertion compares content // rather than framing. A body that is not a geode object throws, since it can only be a test bug. function contentOf(body: Uint8Array | null): string { - const opened = unwrapObject(body ?? new Uint8Array()); + if (body === null) { + throw new Error("not a geode object: no body"); + } + const opened = unwrapObject(body); if (!opened.ok) { throw new Error(`not a geode object: ${opened.reason}`); } @@ -359,6 +362,7 @@ test("sync: two devices syncing at overlapping times never silently delete a fil interleaved = true; assert.equal((await sync(b)).ok, true); } + return storage.putObject(key, body, condition); }, }; @@ -482,7 +486,7 @@ test("sync: an edit on one device and a delete on another preserves the edit as } }); -test("sync: two devices converge inside a bucket prefix (#154)", async () => { +test("sync: two devices converge inside a bucket prefix", async () => { // The whole spine against a client rooted inside the bucket, which is what proves nothing above // the storage client knows a prefix exists. await resetRemote(); diff --git a/src/sync/sync.test.ts b/src/sync/sync.test.ts index 0669be3..9415ac5 100644 --- a/src/sync/sync.test.ts +++ b/src/sync/sync.test.ts @@ -90,7 +90,7 @@ test("faultFor: trying again is worth something, or it never will be, and a race } }); -test("syncOnce: a rejected access key halts rather than being retried forever (#93)", async () => { +test("syncOnce: a rejected access key halts rather than being retried forever", async () => { // A rejected access key must be reported as permanent, not transient, so it is never retried // forever on a timer. const { storage } = fakeStorage(); @@ -229,7 +229,7 @@ test("readRemoteManifest: a manifest from a format version this build doesn't kn }); }); -test("readRemoteManifest: a manifest entry with a traversal path refuses the pass (#132)", async () => { +test("readRemoteManifest: a manifest entry with a traversal path refuses the pass", async () => { // A remote manifest is untrusted input anyone who can write to the bucket can shape, so a // crafted path must never reach a local file operation. const raw = JSON.stringify({ @@ -247,7 +247,7 @@ test("readRemoteManifest: a manifest entry with a traversal path refuses the pas }); }); -test("readRemoteManifest: two paths differing only by case refuse the pass (#94)", async () => { +test("readRemoteManifest: two paths differing only by case refuse the pass", async () => { // Bucket keys are case sensitive while macOS, Windows, and Android are not by default, so // pulling both would silently let one overwrite the other with no conflict ever raised. const raw = JSON.stringify({ @@ -387,7 +387,7 @@ test("syncOnce: a manifest format this build doesn't know halts the pass before }); }); -test("syncOnce: a genuinely new bucket writes a sentinel too (#183)", async () => { +test("syncOnce: a genuinely new bucket writes a sentinel too", async () => { const reader = fakeReader({ "a.md": "alpha" }); const { writer } = fakeLocalWriter(); const { storage, objects } = fakeStorage(); @@ -405,7 +405,7 @@ test("syncOnce: a genuinely new bucket writes a sentinel too (#183)", async () = assert.equal(JSON.parse(unwrapped(written as string)).vaultId, "minted-id"); }); -test("syncOnce: a pass with nothing to do writes nothing at all (#102)", async () => { +test("syncOnce: a pass with nothing to do writes nothing at all", async () => { // Ancestor, local vault, and remote manifest all agree, so planning finds nothing to do. const ancestor: Snapshot = { files: [file("a.md", "h1")], vaultId: "known-id" }; const remoteManifest = wrapped(encodeSnapshot(snapshot(file("a.md", "h1")))); @@ -437,7 +437,7 @@ test("syncOnce: a pass with nothing to do writes nothing at all (#102)", async ( assert.equal(outcome.snapshot.vaultId, "known-id"); }); -test("syncOnce: a first sync with nothing to do still writes the manifest (#102)", async () => { +test("syncOnce: a first sync with nothing to do still writes the manifest", async () => { // Even with nothing to plan, the manifest still must land: its existence is what ends first sync // state for every later pass. const reader = fakeReader({}); @@ -491,7 +491,7 @@ test("syncOnce: a pass with nothing to do still writes a missing sentinel (#102, assert.equal(outcome.snapshot.vaultId, "minted-id"); }); -test("syncOnce: a device pointed at a different vault's sentinel refuses (#183)", async () => { +test("syncOnce: a device pointed at a different vault's sentinel refuses", async () => { // This device already trusts a different vaultId from a prior sync, and the bucket now belongs to // a genuinely different vault; whether that vault's manifest exists is irrelevant, the mismatch // alone is what must refuse. @@ -525,7 +525,7 @@ test("syncOnce: a device pointed at a different vault's sentinel refuses (#183)" }); }); -test("syncOnce: a never-synced device proceeds without a manifest (#109)", async () => { +test("syncOnce: a never-synced device proceeds without a manifest", async () => { // The sentinel proves this bucket has synced before, but this device has no history of its own to // compare against, so it falls through to the first sync path rather than refuse. const reader = fakeReader({ "a.md": "alpha" }); @@ -776,6 +776,7 @@ test("syncOnce: a manifest overwritten by another device mid sync fails the pass await inner(blobKeyFor(beeHash), new TextEncoder().encode(wrapped("bee"))); await inner(MANIFEST_KEY, new TextEncoder().encode(wrapped(bManifest))); } + return inner(key, body, condition); }; // a.md matches the ancestor's size and mtime so takeSnapshot reuses its hash and sees no local @@ -832,6 +833,7 @@ test("syncOnce: retry adopts an identical orphaned upload with a HEAD, not anoth raceManifest = false; await inner(MANIFEST_KEY, new TextEncoder().encode(wrapped(encodeSnapshot(ancestor)))); } + return inner(key, body, condition); }; const reader = fakeReader({ "a.md": "ours!" }); @@ -895,6 +897,7 @@ test("syncOnce: a file changed mid sync is not recorded in the manifest and is p readerFiles["a.md"] = "edited mid sync"; readerFiles["c.md"] = "created mid sync"; } + return inner(key, body, condition); }; @@ -969,6 +972,7 @@ test("syncOnce: a file edited mid sync is never overwritten by a pull, and the r readerFiles["b.md"] = "edited mid sync"; files.set("b.md", "edited mid sync"); } + return inner(key); }; const now = Date.parse("2026-07-14T10:00:00.000Z"); @@ -1003,7 +1007,7 @@ test("syncOnce: a file edited mid sync is never overwritten by a pull, and the r assert.equal(files.get("b.md"), "b v2"); }); -test("syncOnce: a conflict copy carries the device that made the edit (#103)", async () => { +test("syncOnce: a conflict copy carries the device that made the edit", async () => { // Both sides changed relative to the ancestor, so the local edit is preserved under a conflict // copy. On a three device vault a timestamp alone leaves whose edit it holds to be guessed, so // the device this pass ran on has to be in the name, on disk and in the uploaded manifest. @@ -1116,6 +1120,7 @@ test("syncOnce: a manifest that moves on mid pull is caught before stale content new TextEncoder().encode(wrapped(encodeSnapshot(snapshot(file("a.md", aV3Hash))))), ); } + return inner(key); }; @@ -1154,6 +1159,7 @@ test("syncOnce: a failed push doesn't discard the progress of the rest of the pa if (key === blobKeyFor(worldHash)) { bPushes++; } + return inner(key, body, condition); }; @@ -1263,6 +1269,7 @@ test("syncOnce: the failure message counts files, not operation failures", async if (key === copyBlobKey) { return { ok: false, status: "server", message: "Storage rejected the write (500)" }; } + return inner(key, body, condition); }; const reader = fakeReader({ "a.md": "a local" }); @@ -1310,6 +1317,7 @@ test("syncOnce: a failed pull records progress without the ancestor ever advanci if (key === blobKeyFor(aV1Hash)) { aPushes++; } + return inner(key, body, condition); }; const readerFiles: Record = { "a.md": "a v1" }; @@ -1371,6 +1379,7 @@ test("syncOnce: two first syncs racing for an empty bucket, the loser fails inst raced = true; await inner(MANIFEST_KEY, new TextEncoder().encode(wrapped(otherManifest))); } + return inner(key, body, condition); }; const reader = fakeReader({ "a.md": "alpha" }); diff --git a/src/sync/sync.ts b/src/sync/sync.ts index ef37913..908306c 100644 --- a/src/sync/sync.ts +++ b/src/sync/sync.ts @@ -24,6 +24,10 @@ import { type SyncAction, } from "./plan.ts"; +// SyncFault says how a failed pass should be treated by whatever decides when to try again, since +// a message alone can't answer that; see docs/technical_sync.md for what each value means. +export type SyncFault = "transient" | "raced" | "permanent"; + // SyncOutcome is the result of a single sync pass: on success the new snapshot to persist, how many // actions ran, and how many of them moved a local file aside, on failure a message and any per file // failures, with snapshot carrying progress worth keeping rather than always null. @@ -48,10 +52,6 @@ export type SyncOutcome = snapshot: Snapshot | null; }; -// SyncFault says how a failed pass should be treated by whatever decides when to try again, since -// a message alone can't answer that; see docs/technical_sync.md for what each value means. -export type SyncFault = "transient" | "raced" | "permanent"; - // adoptLiveStats swaps in the live vault's entry for any manifest entry whose hash still matches, // so state.json carries local size and mtime and the next snapshot can stat skip the rehash. // Exported for its tests; syncOnce is the only production caller. @@ -337,7 +337,7 @@ 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. + // this hung", and the first action of a big pull can outlast the patience it is spending. onProgress(0, actions.length); const executed = await executeSyncPlan( actions, diff --git a/src/vault/fs.ts b/src/vault/fs.ts index 9bb6ee7..22fe585 100644 --- a/src/vault/fs.ts +++ b/src/vault/fs.ts @@ -82,18 +82,25 @@ function abs(root: string, path: string): string { // excluding dot prefixed entries (.obsidian, staged .geode-tmp writes), mirroring how Obsidian's // Vault.getFiles() never indexes hidden files. function walk(root: string, dir = ""): string[] { - const here = dir === "" ? root : abs(root, dir); + let here = root; + if (dir !== "") { + here = abs(root, dir); + } const out: string[] = []; for (const entry of readdirSync(here, { withFileTypes: true })) { if (entry.name.startsWith(".")) { continue; } - const rel = dir === "" ? entry.name : `${dir}/${entry.name}`; + let rel = entry.name; + if (dir !== "") { + rel = `${dir}/${entry.name}`; + } if (entry.isDirectory()) { out.push(...walk(root, rel)); continue; } out.push(normalizePath(rel)); } + return out; } diff --git a/src/vault/obsidian.test.ts b/src/vault/obsidian.test.ts index 8437fe8..60eedb1 100644 --- a/src/vault/obsidian.test.ts +++ b/src/vault/obsidian.test.ts @@ -18,6 +18,7 @@ function fakeAdapter(seed: Record = {}): DataAdapter { if (content === undefined) { throw new Error(`no such file: ${path}`); } + return content; }, write: async (path: string, data: string) => { @@ -32,6 +33,7 @@ function fakeAdapter(seed: Record = {}): DataAdapter { files.set(newPath, data); }, }; + return adapter as unknown as DataAdapter; } @@ -390,7 +392,7 @@ test("createObsidianStore: a well shaped snapshot round-trips through write and assert.deepEqual(await store.read(), want); }); -test("createObsidianStore: an interrupted write leaves the previous state.json untouched, never torn (#136)", async () => { +test("createObsidianStore: an interrupted write leaves the previous state.json untouched, never torn", async () => { // The write must be staged and installed via rename, the same atomic pattern pulled vault // content already uses, so a failure between the two steps never leaves a half written file for // the next sync to misread as a corrupt or empty ancestor. @@ -422,7 +424,7 @@ test("createObsidianStore: a fingerprint mismatch reads back as empty", async () assert.deepEqual(await store2.read(), { files: [] }); }); -test("createObsidianStore: repointing at a bucket prefix reads back as empty (#154)", async () => { +test("createObsidianStore: repointing at a bucket prefix reads back as empty", async () => { // A prefix is where the vault lives, so moving it lands on a folder with its own manifest and its // own sentinel. Carrying the old ancestor across would diff this vault against a stranger's. const adapter = fakeAdapter(); @@ -437,7 +439,7 @@ test("createObsidianStore: repointing at a bucket prefix reads back as empty (#1 assert.deepEqual(await store2.read(), { files: [] }); }); -test("fingerprintSettings: a prefix only written differently is the same target (#154)", () => { +test("fingerprintSettings: a prefix only written differently is the same target", () => { // The prefix is stored exactly as typed, so the same folder can be spelled several ways. Treating // those as different targets would throw away a good ancestor and force a full re-hash over a // trailing slash. @@ -501,9 +503,11 @@ function fakeWorkspace(views: unknown[]): Workspace { if (type !== "markdown") { return []; } + return views.map((view) => ({ view })); }, }; + return workspace as unknown as Workspace; } diff --git a/src/vault/obsidian.ts b/src/vault/obsidian.ts index 91d6ce3..a65d006 100644 --- a/src/vault/obsidian.ts +++ b/src/vault/obsidian.ts @@ -73,6 +73,7 @@ export function createObsidianReader(vault: Vault): Reader { mtime: file.stat.mtime, }); } + return files; }, readFile: async (path) => { @@ -92,6 +93,7 @@ export function createObsidianReader(vault: Vault): Reader { if (file === null) { return { present: false, size: 0, mtime: 0 }; } + return { present: true, size: file.stat.size, mtime: file.stat.mtime }; }, }; @@ -166,7 +168,10 @@ async function ensureParentDir(adapter: DataAdapter, path: string): Promise { - const asidePath = hiddenSiblingPath(path, ".geode-old"); - const leftover = await adapter.exists(asidePath); - if (leftover) { - await adapter.remove(asidePath); - } - await adapter.rename(path, asidePath); - try { - await adapter.rename(tempPath, path); - } catch (err) { - await adapter.rename(asidePath, path); - throw err; - } - await adapter.remove(asidePath); -} - // installStaged renames a staged file onto its destination, so a crash leaves that destination // either untouched or fully written. A "create" write refuses an occupied path outright, checked // against the adapter's own stat. @@ -232,3 +215,25 @@ async function installStaged( await replaceViaAside(adapter, tempPath, path); } } + +// replaceViaAside installs a staged file where rename refuses to overwrite, never deleting the +// destination's bytes while a restore is still possible. +async function replaceViaAside( + adapter: DataAdapter, + tempPath: string, + path: string, +): Promise { + const asidePath = hiddenSiblingPath(path, ".geode-old"); + const leftover = await adapter.exists(asidePath); + if (leftover) { + await adapter.remove(asidePath); + } + await adapter.rename(path, asidePath); + try { + await adapter.rename(tempPath, path); + } catch (err) { + await adapter.rename(asidePath, path); + throw err; + } + await adapter.remove(asidePath); +} diff --git a/src/vault/vault.test.ts b/src/vault/vault.test.ts index 846ff88..c764b6d 100644 --- a/src/vault/vault.test.ts +++ b/src/vault/vault.test.ts @@ -28,6 +28,7 @@ function fakeReader(files: Record): for (const [path, file] of Object.entries(files)) { list.push({ path, size: file.content.length, mtime: file.mtime }); } + return list; }, readFile: async (path) => { @@ -36,6 +37,7 @@ function fakeReader(files: Record): if (file === undefined) { throw new Error(`no such file: ${path}`); } + return new TextEncoder().encode(file.content); }, stat: async (path) => { @@ -43,9 +45,11 @@ function fakeReader(files: Record): if (file === undefined) { return { present: false, size: 0, mtime: 0 }; } + return { present: true, size: file.content.length, mtime: file.mtime }; }, }; + return { reader, readCount: () => reads }; } @@ -325,6 +329,7 @@ test("takeSnapshot: concurrency is bounded by the limit", async () => { for (const [path, file] of Object.entries(files)) { list.push({ path, size: file.content.length, mtime: file.mtime }); } + return list; }, readFile: async (path) => { @@ -338,6 +343,7 @@ test("takeSnapshot: concurrency is bounded by the limit", async () => { if (file === undefined) { throw new Error(`no such file: ${path}`); } + return new TextEncoder().encode(file.content); }, stat: async (path) => { @@ -345,6 +351,7 @@ test("takeSnapshot: concurrency is bounded by the limit", async () => { if (file === undefined) { return { present: false, size: 0, mtime: 0 }; } + return { present: true, size: file.content.length, mtime: file.mtime }; }, }; @@ -377,7 +384,7 @@ for (const { name, input, want } of normalizePathCases) { }); } -test("takeSnapshot: an NFD path from the reader is recorded as NFC (#134)", async () => { +test("takeSnapshot: an NFD path from the reader is recorded as NFC", async () => { // macOS decomposes filenames to NFD; the reader hands back whatever the platform holds, but the // snapshot records the composed form so every device agrees on one identity for the file. const { reader } = fakeReader({ "café.md": { content: "hello", mtime: 1 } }); @@ -388,7 +395,7 @@ test("takeSnapshot: an NFD path from the reader is recorded as NFC (#134)", asyn assert.equal(snapshot.files[0].path, "café.md"); }); -test("diffSnapshots: an NFC and NFD pair for one file is not a change (#134)", async () => { +test("diffSnapshots: an NFC and NFD pair for one file is not a change", async () => { // The payoff: the same note snapshotted on Linux and then on macOS must not read as a rename, // which is a delete plus a create, and would push a duplicate out to every other device. const { reader: nfc } = fakeReader({ "café.md": { content: "hello", mtime: 1 } }); @@ -400,7 +407,7 @@ test("diffSnapshots: an NFC and NFD pair for one file is not a change (#134)", a assert.deepEqual(diffSnapshots(previous, current), []); }); -test("decodeSnapshot: an NFD path in a manifest decodes to NFC (#134)", () => { +test("decodeSnapshot: an NFD path in a manifest decodes to NFC", () => { const nfdFile = { path: "café.md", size: 5, mtime: 1, hash: "abc", blob: "abc" }; const raw = JSON.stringify({ version: SNAPSHOT_VERSION, files: [nfdFile] }); @@ -412,7 +419,7 @@ test("decodeSnapshot: an NFD path in a manifest decodes to NFC (#134)", () => { } }); -test("decodeSnapshot: an NFC and NFD entry for one path is refused (#134)", () => { +test("decodeSnapshot: an NFC and NFD entry for one path is refused", () => { // Two entries, one file. Deciding which wins would silently drop an edit, and normalizing them // together would leave two manifest rows fighting over the same path on every later pass. const nfcFile = { path: "café.md", size: 5, mtime: 1, hash: "abc", blob: "abc" }; @@ -436,7 +443,7 @@ test("decodeSnapshot: a duplicate path is refused even when the content matches" assert.deepEqual(decoded, { ok: false, reason: "duplicatePath" }); }); -test("decodeSnapshot: NFC folding happens before the case fold, so both are caught (#134)", () => { +test("decodeSnapshot: NFC folding happens before the case fold, so both are caught", () => { // Normalizing first is what makes the case check mean what it says: on the raw bytes an NFD // "Café.md" and an NFC "café.md" fold to different lowercase strings and both slip through. const upper = { path: "CAFÉ.md", size: 5, mtime: 1, hash: "abc", blob: "abc" }; @@ -462,6 +469,7 @@ test("takeSnapshot: in-flight bytes are bounded by the byte budget", async () => for (const [path, file] of Object.entries(files)) { list.push({ path, size: file.content.length, mtime: file.mtime }); } + return list; }, readFile: async (path) => { @@ -483,6 +491,7 @@ test("takeSnapshot: in-flight bytes are bounded by the byte budget", async () => if (file === undefined) { return { present: false, size: 0, mtime: 0 }; } + return { present: true, size: file.content.length, mtime: file.mtime }; }, }; @@ -562,6 +571,7 @@ test("takeSnapshot: growth since listing is bounded by the fresh size", async () for (const [path, file] of Object.entries(files)) { list.push({ path, size: file.listed, mtime: file.mtime }); } + return list; }, readFile: async (path) => { @@ -583,6 +593,7 @@ test("takeSnapshot: growth since listing is bounded by the fresh size", async () if (file === undefined) { return { present: false, size: 0, mtime: 0 }; } + return { present: true, size: file.actual, mtime: file.mtime }; }, }; diff --git a/src/vault/vault.ts b/src/vault/vault.ts index 5ee76fa..cb85fac 100644 --- a/src/vault/vault.ts +++ b/src/vault/vault.ts @@ -116,6 +116,7 @@ export function byPath(files: FileState[]): Map { for (const file of files) { result.set(file.path, file); } + return result; } @@ -172,15 +173,13 @@ export function decodeSnapshot(raw: string): DecodedSnapshot { files.push({ ...file, path }); } const settingsFingerprint = (parsed as { settingsFingerprint?: unknown }).settingsFingerprint; - const fingerprintStr = typeof settingsFingerprint === "string" ? settingsFingerprint : undefined; const vaultId = (parsed as { vaultId?: unknown }).vaultId; - const vaultIdStr = typeof vaultId === "string" ? vaultId : undefined; const snapshot: Snapshot = { files }; - if (fingerprintStr !== undefined) { - snapshot.settingsFingerprint = fingerprintStr; + if (typeof settingsFingerprint === "string") { + snapshot.settingsFingerprint = settingsFingerprint; } - if (vaultIdStr !== undefined) { - snapshot.vaultId = vaultIdStr; + if (typeof vaultId === "string") { + snapshot.vaultId = vaultId; } return { ok: true, snapshot }; @@ -256,6 +255,7 @@ export async function hashBytes(data: Uint8Array): Promise { for (const byte of new Uint8Array(digest)) { hex += byte.toString(16).padStart(2, "0"); } + return hex; } @@ -419,7 +419,10 @@ function isSafeAddress(value: unknown): boolean { // insensitively and ignoring any extension. function isWindowsReservedName(segment: string): boolean { const dot = segment.indexOf("."); - const base = dot === -1 ? segment : segment.slice(0, dot); + let base = segment; + if (dot !== -1) { + base = segment.slice(0, dot); + } return WINDOWS_RESERVED_NAMES.has(base.toLowerCase()); }