From 4e6b9753bbc2c7e001ca12bd861641bcaebe85a1 Mon Sep 17 00:00:00 2001 From: Evan Gress <106449014+evangress@users.noreply.github.com> Date: Mon, 17 Aug 2026 15:44:50 -0400 Subject: [PATCH 1/4] feat(update): give an installed Toril a way forward MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1.0.0 shipped with no update path, so every copy is stranded on the version it was installed with — whatever gets built next cannot reach anyone who already has it. That is what makes this branch precede vault search rather than follow it (ROADMAP §7, trust before reach). Three official Tauri plugins, no new invoke commands: updater, process, window-state. They ship their own JS API, wrapped in ipc.ts so nothing else in the app reaches past that seam. Toril notifies and never installs on its own. The plugin can download-and-replace silently; we deliberately never call it that way, because this is an editor holding unsaved buffers. The restart is the one path in the feature that could destroy one, so it is the one path that refuses: it will not relaunch over a dirty tab, and says the update applies next launch instead — the install is already on disk, so waiting is free. The policy is split from the network call and the toast so it can be gated without a release server. What tests/update.test.ts pins is the asymmetry that makes an updater tolerable: a startup check is rate-limited, skippable and silent, while a direct question via Help → Check for Updates always gets an answer. Two rules there are not obvious. A lastCheckedAt in the future fails *open* — a corrected clock would otherwise disable update checks forever with no symptom anyone could notice. And skipping a version means "stop telling me", not "never let me have it", so a later version still surfaces and a manual check still shows the skipped one. The notice is a fixed-position toast rather than another row in the layout, which is the opposite of conflictbar.ts on purpose: a conflict is about the document in front of you and must be impossible to miss; an update is ambient news and is never worth reflowing a sentence for. It also keeps this branch out of body's grid, which declares rows and no columns (§12b rule 2). Two unrelated signatures, kept apart in docs/RELEASE-SIGNING.md. Minisign signs the update artifacts and is required — the bundler refuses to emit an unsigned update, so release.yml checks for the key up front and fails in five seconds with an explanation rather than twenty minutes into a Rust build. That preflight is a hard stop rather than a "skip the updater and carry on": silently cutting another release with no update path is exactly how v1.0.0 stranded itself. Authenticode is optional, needs an Azure account that takes days to provision, and lives in an overlay config applied in CI only when the secrets exist — a fork, or a local pnpm tauri build, must not need an Azure subscription to produce a working installer. Still keyless: generating the minisign keypair, pasting the public half into plugins.updater.pubkey and storing the private half is the owner's step, not something to do on their behalf. Until then a check degrades to a reported error rather than a crash. Everything downstream of the network is unverifiable here and is listed as §D of docs/ON-DEVICE-VERIFICATION.md rather than assumed fine — including the toast's measured layout (§12b), which could not be swept in the browser harness this session. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release.yml | 65 ++++- .gitignore | 8 + CHANGELOG.md | 24 ++ CLAUDE.md | 58 ++++- ROADMAP.md | 51 ++-- dev-harness.html | 18 ++ docs/ON-DEVICE-VERIFICATION.md | 41 +++- docs/RELEASE-SIGNING.md | 93 +++++++ package.json | 3 + pnpm-lock.yaml | 30 +++ src-tauri/Cargo.lock | 368 +++++++++++++++++++++++++++- src-tauri/Cargo.toml | 19 ++ src-tauri/capabilities/default.json | 4 +- src-tauri/src/lib.rs | 19 +- src-tauri/src/menu.rs | 9 + src-tauri/src/settings.rs | 13 + src-tauri/tauri.conf.json | 10 + src-tauri/tauri.signing.conf.json | 8 + src/ipc.ts | 58 +++++ src/main.ts | 142 +++++++++++ src/styles/chrome.css | 67 +++++ src/ui/updatenotice.ts | 144 +++++++++++ src/update.ts | 125 ++++++++++ tests/update.test.ts | 140 +++++++++++ 24 files changed, 1487 insertions(+), 30 deletions(-) create mode 100644 docs/RELEASE-SIGNING.md create mode 100644 src-tauri/tauri.signing.conf.json create mode 100644 src/ui/updatenotice.ts create mode 100644 src/update.ts create mode 100644 tests/update.test.ts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1260efa..a22efc2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -25,9 +25,46 @@ jobs: args: "" runs-on: ${{ matrix.platform }} + + # Job level, not step level, so a step's `if:` can read it. A step cannot + # reliably test an env var it declares itself. + env: + AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} + AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} + AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} + steps: - uses: actions/checkout@v4 + # Fail in five seconds with an explanation rather than twenty minutes into + # a Rust build. `createUpdaterArtifacts` is on in tauri.conf.json, and the + # bundler refuses to produce an unsigned update — so a missing key is a + # configuration error, not a build error, and should read like one. + # + # This is deliberately a hard stop rather than a "skip the updater and + # carry on": silently cutting another release with no update path is + # exactly how v1.0.0 stranded every copy of itself. + - name: Check the updater signing key is configured + shell: bash + env: + KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }} + run: | + if [ -z "$KEY" ]; then + echo "::error::TAURI_SIGNING_PRIVATE_KEY is not set. Update artifacts cannot be signed." + echo "::error::Generate a keypair with 'pnpm tauri signer generate', add the private key" + echo "::error::as this repository secret, and paste the public key into" + echo "::error::src-tauri/tauri.conf.json under plugins.updater.pubkey." + echo "::error::See docs/RELEASE-SIGNING.md." + exit 1 + fi + if ! grep -q '"pubkey": *"[^"]' src-tauri/tauri.conf.json; then + echo "::error::plugins.updater.pubkey is empty in src-tauri/tauri.conf.json." + echo "::error::Installed builds verify updates against it, so shipping it empty" + echo "::error::produces a release that can never update itself." + echo "::error::See docs/RELEASE-SIGNING.md." + exit 1 + fi + - name: Install Linux webview dependencies if: matrix.platform == 'ubuntu-22.04' run: | @@ -58,9 +95,29 @@ jobs: - name: Install frontend dependencies run: pnpm install --frozen-lockfile + # Windows Authenticode via Azure Trusted Signing, and *only* when the + # account exists. The signCommand lives in an overlay config applied here + # rather than in tauri.conf.json, so a fork — or a local `pnpm tauri build` + # — still produces a working unsigned installer instead of failing on a + # missing tool. Without this, every contributor would need an Azure + # subscription to build the app at all. + - name: Set up Windows code signing + if: matrix.platform == 'windows-latest' && env.AZURE_CLIENT_ID != '' + shell: bash + run: | + cargo install trusted-signing-cli --locked + echo "SIGNING_CONFIG=--config src-tauri/tauri.signing.conf.json" >> "$GITHUB_ENV" + - uses: tauri-apps/tauri-action@v0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Signs the update artifacts (minisign). Not the same thing as + # Authenticode above: this proves an update came from us, that one + # stops SmartScreen warning about the installer. + TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }} + TAURI_SIGNING_PRIVATE_KEY_PASSWORD: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY_PASSWORD }} + # The AZURE_* trio that trusted-signing-cli reads comes from the job's + # env above, so it is already in scope here. with: tagName: ${{ github.ref_name }} releaseName: "Toril ${{ github.ref_name }}" @@ -81,4 +138,10 @@ jobs: # pinned to `true` through the alpha/beta series, which would have published # v1.0.0 itself as a prerelease labelled "early alpha". prerelease: ${{ contains(github.ref_name, '-') }} - args: ${{ matrix.args }} + # Publishes `latest.json` beside the installers — the static manifest + # the in-app updater fetches. Without it the plugin has nothing to read + # and every check reports "up to date" forever. + includeUpdaterJson: true + # `SIGNING_CONFIG` is set only on Windows and only when the Azure + # secrets exist; it expands to nothing otherwise. + args: ${{ matrix.args }} ${{ env.SIGNING_CONFIG }} diff --git a/.gitignore b/.gitignore index 4a89707..8d13031 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,14 @@ dist-ssr /toril-harness-*.png /sweep-*.png +# Signing keys — a backstop, not the intended home. The updater's private key +# belongs outside the repository entirely (docs/RELEASE-SIGNING.md); losing it +# strands every installed copy, and committing it lets anyone ship an "update" +# that Toril would trust and install. +*.key +*.key.pub +/.tauri/ + # Editor / OS .DS_Store .idea/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a2e229..0910d52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,30 @@ GitHub Release notes plus the commits that shipped in it. ## [Unreleased] +### Added +- **Toril can update itself.** `v1.0.0` had no update path at all, so every copy was + stranded on the version it was installed with — the only way forward was to notice a + new release and download the installer by hand. Toril now checks for a newer build + once a day at launch, and whenever you ask via **Help → Check for Updates…**. + + **It tells you; it never installs behind your back.** You choose when to download, and + a restart is refused while anything is unsaved — the update is already on disk by then + and applies the next time you start, so waiting costs nothing. Automatic checks can be + turned off in **View → Check for Updates on Launch**. + + The check is a plain request for a static file. Nothing about you, your vault or your + session is sent with it, and there is no telemetry in Toril. + +- **The window remembers where it was.** Size, position and maximized state come back + the way you left them. + +### Notes +- Updates are cryptographically signed, and an installed Toril refuses one that does not + verify. Setting that up is a one-time step for whoever cuts releases — see + `docs/RELEASE-SIGNING.md`. +- Windows installers are still unsigned, so SmartScreen still warns on first run. The + wiring for Azure Trusted Signing is in place but inert until an account exists. + ## [v1.0.0] — 2026-08-17 **Toril leaves beta.** Same promise as always: your notes are plain `.md` (and diff --git a/CLAUDE.md b/CLAUDE.md index 156a1ac..86caf2f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -122,8 +122,20 @@ Implementation notes for whoever resumes (learned the hard way): Milkdown `$mark **no-op** runner (an empty-type `withMark` produces an mdast node remark cannot stringify — and Export HTML/RTF always serialize via `docToMarkdown`). -**Next:** finish Phase 4 — remaining is shippable-quality work: optional code-signing (removes the -SmartScreen warning) and on-device verification. A backlog of further QoL features is in §13. The +**Self-update, window state and code-signing wiring — `feat/release-readiness` +(ROADMAP Movement I.5).** `v1.0.0` shipped with no update path, so every installed copy +was stranded on the version it was installed with; that is now closed. Details and the +two-signatures distinction are in §5 (Self-update) and `docs/RELEASE-SIGNING.md`. +**One manual step remains before it does anything:** generate the minisign keypair, paste +the public half into `plugins.updater.pubkey`, and add the private half as a repository +secret. Until then `plugins.updater.pubkey` is empty, so a check degrades to a reported +error rather than a crash, and `release.yml` refuses to cut a tag with a one-line +explanation instead of failing deep in a Rust build. + +**Next:** finish Phase 4 — remaining is the QoL half of `feat/release-readiness` (editor +zoom, recent-files MRU, open-links-in-browser, drag-drop open, first-run empty state), +Azure Trusted Signing once an account exists, and on-device verification. A backlog of +further QoL features is in §13. The **forward plan beyond Phase 4** — turning the editor into a notes *system* (search, links, version history, sync coexistence, the AI wedge) branch-by-branch, with per-stage publicity guidance — lives in **`ROADMAP.md`**. @@ -332,7 +344,7 @@ frontend never touches the filesystem directly; it asks via `invoke()`. | `export_rtf` | `content, defaultName` | `path?` | renders (comrak via `mdrtf`) **and** writes, all in Rust; inert output, no sanitize (§7) | | `export_pdf` | `content, theme` | `path` | *(deferred — §7)* | | `save_clipboard_image` | `bytes, docPath` | `relative_path` | writes pasted image to `./assets/` (`imgasset`), returns MD-relative path (§6) | -| `load_settings` / `save_settings` | — / `Settings` | `Settings` / `()` | JSON in app config dir; includes `theme`, `sidebar_visible`/`sidebar_width`, `rail_visible`/`rail_width`/`rail_tab`, and `properties_expanded` (the front-matter strip's collapse state; `null` ⇒ expanded). The legacy `outline_visible`/`history_visible` pair is **read once to migrate** into the rail fields and then never written again — that one-directionality is what stops a stale flag from overriding the migrated state | +| `load_settings` / `save_settings` | — / `Settings` | `Settings` / `()` | JSON in app config dir; includes `theme`, `sidebar_visible`/`sidebar_width`, `rail_visible`/`rail_width`/`rail_tab`, `properties_expanded` (the front-matter strip's collapse state; `null` ⇒ expanded), and the update trio `update_check`/`update_last_checked`/`update_skipped_version` (§Self-update). The legacy `outline_visible`/`history_visible` pair is **read once to migrate** into the rail fields and then never written again — that one-directionality is what stops a stale flag from overriding the migrated state | | `save_recovery` | `entries` | `()` | **atomic** write of `recovery.json` in the app config dir — crash-recovery journal (§3) | | `load_recovery` | — | `RecoveryEntry[]` | empty on missing/corrupt (never bricks startup) | | `clear_recovery` | — | `()` | delete `recovery.json` — the clean-shutdown sentinel | @@ -403,6 +415,33 @@ frontend never touches the filesystem directly; it asks via `invoke()`. > rename that calls it is Movement II.12. Restore snapshots the current state first, so it is > undoable. Frontend: `src/ui/history.ts` panel + `src/ui/linediff.ts`. > +> **Self-update (`feat/release-readiness`, ROADMAP Movement I.5).** Three official Tauri +> plugins — `updater`, `process`, `window-state` — so there are no new `invoke` commands; +> they ship their own JS API, wrapped in `ipc.ts` (`checkForUpdate`, `relaunchApp`) so +> nothing else in the app reaches past that seam. **Toril notifies and never installs on +> its own.** The plugin can download-and-replace silently; we deliberately never call it +> that way, because this is an editor holding unsaved buffers and §3 outranks saving +> someone two clicks. The policy — check at most daily at launch, silent when there is +> nothing to say, loud for every outcome when the user asks via Help → Check for +> Updates — lives in `src/update.ts`, pure and gated by `tests/update.test.ts`; the +> network call and the toast are separate so the rules need no release server to test. +> **The restart is the one path that could destroy a buffer, so it is the one path that +> refuses**: `restartForUpdate` will not relaunch over a dirty tab, and says the update +> applies next launch instead (the install is already on disk, so waiting is free). +> No telemetry — the check is a plain GET for a static manifest. +> +> **Two unrelated signatures, both documented in `docs/RELEASE-SIGNING.md`.** Minisign +> signs the *update artifacts* and is **required**: `createUpdaterArtifacts` is on, the +> bundler refuses to emit an unsigned update, and `release.yml` fails fast with an +> explanation rather than deep in a Rust build. Losing that private key strands every +> installed copy permanently — there is no rotation, because installed builds verify +> against the public key they shipped with. Authenticode (Azure Trusted Signing) is +> what stops SmartScreen warning, is **optional**, and needs an account that takes days +> to provision — so its `signCommand` lives in `tauri.signing.conf.json`, an overlay +> applied in CI only when the `AZURE_*` secrets exist. That split is deliberate: a fork +> or a local `pnpm tauri build` must not need an Azure subscription to produce a working +> installer. +> > **Events (Rust → frontend):** `workspace:change` (file watcher), `menu` (native menu item id > `menu_*` → mapped to the same handlers as toolbar buttons), and `open-file` (a *second* launch's > file path, forwarded by the single-instance plugin while Toril is already running). Subscribe via @@ -543,6 +582,12 @@ Phases 0–3 are complete and Phase 4 (polish) is in progress; the shipped detai briefly narrowed. - **Action double-fire:** `tests/actions.test.ts` — `menu.rs` carries real accelerators, so one Ctrl+S arrives twice (menu *and* webview keydown). One dispatcher collapses the pair. +- **Update policy:** `tests/update.test.ts` — the startup/manual asymmetry (a background + check is rate-limited, skippable and silent; a direct question always gets an answer), + the interval boundary itself rather than either side of it, and that a `lastCheckedAt` + in the **future** fails *open* — a corrected clock must not disable update checks + forever with no symptom. What it cannot cover is anything downstream of the network: + see §D of `docs/ON-DEVICE-VERIFICATION.md`. - Plus `vaultscan`, `imgasset`, `theme`, `statusbar`, `search`, `security`, `tabs` suites. > **The browser harness.** `dev-harness.html` is `app.html` plus a fake Tauri IPC bridge @@ -576,8 +621,11 @@ test harness — it needs a live Milkdown editor and Tauri IPC.** `crates/mergem `src/paths.ts`, and the tab bookkeeping in `src/ui/tabs.ts` are gated in isolation; the glue that calls them in the right order, at the right time, is verified on-device only. -**Remaining for Phase 4:** optional code-signing (removes the SmartScreen warning — see the -code-signing memory) and on-device verification of GUI/Rust flows that can't be tested here. +**Remaining for Phase 4:** the QoL half of `feat/release-readiness`; Azure Trusted Signing +(removes the SmartScreen warning — the wiring is in place and inert, `docs/RELEASE-SIGNING.md`); +and on-device verification of GUI/Rust flows that can't be tested here, now including the +update flow (§D of `docs/ON-DEVICE-VERIFICATION.md` — nothing headless can prove a signed +artifact downloads, verifies, and replaces a running binary). Shortcut-reference panel deferred (the menu lists shortcuts). --- diff --git a/ROADMAP.md b/ROADMAP.md index 0843589..18e973e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -49,12 +49,22 @@ no AI. That gap is this roadmap. > **Status (2026-08-17).** Shipped through **`v1.0.0`** (see `CHANGELOG.md`). > **Movement I, branches 1–4 are complete** (autosave + crash-recovery journal; > safe-delete-to-trash; local version history; sync coexistence — 3-way merge, conflict -> banner, parked conflict copies). Branch 5 is unstarted. From Movement II, **branch 11 -> (outline panel) and branch 10 (front-matter properties)** have landed — 10 out of -> order, because front matter was being *corrupted* rather than merely unsupported, +> banner, parked conflict copies). **Branch 5's update half has landed** on +> `feat/release-readiness`; its QoL half and Azure signing remain. From Movement II, +> **branch 11 (outline panel) and branch 10 (front-matter properties)** have landed — 10 +> out of order, because front matter was being *corrupted* rather than merely unsupported, > which made it a §3 fix rather than a convenience. -> **▶ Pick up at Movement II, branch 6 — `feat/vault-search`.** It is the largest -> remaining gap, and branch 7 (command palette) depends on it. Vet `tantivy` per §2 at +> +> **This document previously pointed at branch 6 with branch 5 unstarted, and that was +> overtaken by `v1.0.0` shipping.** The ladder in §2 ties release-readiness to +> `v0.2.0-alpha`; the tag ran past it, which turned "no auto-updater" from a scheduling +> detail into a live problem — every 1.0.0 install had no way to receive anything built +> after it, so the next feature could not have reached anyone. §7's *trust before reach* +> is what settles the order. Read the pointer below as the ordering, not the ladder. +> +> **▶ Pick up at branch 5's QoL half, then Movement II, branch 6 — +> `feat/vault-search`.** Search is the largest remaining functional gap, and branch 7 +> (command palette) depends on it. Vet `tantivy` per §2 at > adoption — it would be the project's largest new dependency. Branch 4's spec lived > **on its own branch**, not on `main`: > `docs/superpowers/specs/2026-07-24-sync-coexistence-design.md`; branch 10's is on @@ -199,18 +209,29 @@ nice in a synced folder. This movement is also the prerequisite for the AI wedge loss) + a `tests/` watcher-reaction suite. - *§3:* a conflict must **never** silently overwrite either side. -- [ ] **5. `feat/release-readiness`** — auto-update + signing + first-run, so the floor is - *shippable to strangers*. - - *Scope:* wire **`tauri-plugin-updater`** (official) and **`tauri-plugin-window-state`** - (vet versions per §2); editor zoom (`Ctrl +/-/0`); recent-files MRU; open-links-in- - browser; drag-drop `.md` to open; a real **first-run / empty-state** (welcome note + - "open a folder"). Adopt **Azure Trusted Signing** for Windows (the old `TODO.md` - item: `bundle.windows.signCommand` → `trusted-signing-cli`; `AZURE_*` CI secrets; - soften the SmartScreen note in `README.md` + `docs/index.html`). +- [~] **5. `feat/release-readiness`** — auto-update + signing + first-run, so the floor is + *shippable to strangers*. **Split in two; the update half has landed.** + - [x] *Update half (2026-08-17).* **`tauri-plugin-updater`** + **`-process`** + + **`tauri-plugin-window-state`**, all official and pinned (2.10.1 / 2.3.1 / 2.4.1). + Notify-only by policy — Toril offers, the user decides, and a restart is refused + over a dirty buffer (§3). Rules in `src/update.ts`, gated by `tests/update.test.ts`; + toast in `src/ui/updatenotice.ts`; minisign signing + the fail-fast preflight in + `release.yml`; Authenticode wiring inert until an Azure account exists. See + `docs/RELEASE-SIGNING.md` and CLAUDE.md §5 (Self-update). + - **Blocked on one manual step:** generate the minisign keypair, paste the public + half into `plugins.updater.pubkey`, add the private half as a repo secret. Until + then the updater is present but keyless. + - [ ] *QoL half (not started).* Editor zoom (`Ctrl +/-/0`); recent-files MRU; + open-links-in-browser; drag-drop `.md` to open; a real **first-run / empty-state** + (welcome note + "open a folder"). + - [ ] *Azure Trusted Signing.* Wiring done (`tauri.signing.conf.json` overlay, applied + in CI only when `AZURE_*` secrets exist, so a fork still builds). Needs an account and + an identity validation that takes business days — then update the placeholder account + names and soften the SmartScreen note in `README.md` + `docs/index.html`. - *Touches:* `tauri.conf.json`, `.github/workflows/release.yml`, `settings.rs`, `menu.rs`, `main.ts`. - - *Gate:* settings round-trip for the new prefs; manual on-device verify of update + - first-run (no webview here). + - *Gate:* `tests/update.test.ts` for the policy; §D of `docs/ON-DEVICE-VERIFICATION.md` + for everything downstream of the network, which no headless gate can reach. - [ ] **⬢ RELEASE `v0.2.0-alpha`** — *"Safe to live in."* First build you can hand to someone without an asterisk on their data. diff --git a/dev-harness.html b/dev-harness.html index be86d5e..a87a64e 100644 --- a/dev-harness.html +++ b/dev-harness.html @@ -107,6 +107,24 @@ "plugin:dialog|open": () => null, "plugin:dialog|ask": () => true, "plugin:dialog|message": () => null, + // The updater, faked so the notice can actually be driven here (§8). + // `?update` in the URL makes the check report an available version, so + // the toast's layout and target sizes can be swept without waiting for + // a real release; without it the harness answers "up to date", which is + // what a normal harness session should see. + "plugin:updater|check": () => + new URLSearchParams(location.search).has("update") + ? { + available: true, + version: "9.9.9", + currentVersion: "0.0.0-harness", + body: "A pretend release, for driving the update notice. " + + "https://example.com/an-unbroken-url-that-must-not-widen-the-toast", + rid: 1, + } + : { available: false }, + "plugin:updater|download_and_install": () => null, + "plugin:process|restart": () => null, load_settings: () => SETTINGS, save_settings: () => null, load_recovery: () => [], diff --git a/docs/ON-DEVICE-VERIFICATION.md b/docs/ON-DEVICE-VERIFICATION.md index 157ee4c..8bad2a4 100644 --- a/docs/ON-DEVICE-VERIFICATION.md +++ b/docs/ON-DEVICE-VERIFICATION.md @@ -157,6 +157,41 @@ rendered layout is unverified in either engine. The logic underneath it is gated and RTF; the properties must not appear in the output. Export no longer relies on comrak stripping them, so this is a genuinely new path. +## D. Release readiness (`feat/release-readiness`, 2026-08-17) + +The update flow is the one feature here whose payoff — a stranded `v1.0.0` install +finding its way forward — cannot be demonstrated by any gate. `tests/update.test.ts` +pins *when* Toril checks and *whether* it interrupts you; nothing headless can pin +that a signed artifact downloads, verifies and replaces a running binary. + +- [ ] **D1 — A real update installs.** The only end-to-end check that matters, and it + needs two releases. Install `v1.0.0`'s NSIS build, publish a later tag, then launch + the old copy: the notice should appear, Install should download, and Restart should + come back on the new version **with the session intact**. Repeat for the MSI — + per-machine install plus a per-user updater is the combination most likely to fail. +- [ ] **D2 — The restart guard actually refuses.** Make a tab dirty, take an update, + click Restart now. Toril must refuse and say the update applies next launch — this + is the one path in the feature that could destroy a buffer (§3), and it is guarded + in `main.ts`, which has no harness. +- [ ] **D3 — Signature verification fails closed.** Tamper with a published artifact (or + sign it with a different key) and confirm the install is **refused**, not attempted. + A verification path that silently accepts is worse than no updater at all. +- [ ] **D4 — Toast layout, measured not eyeballed** (§12b). Drive `dev-harness.html?update` + at 1400/1100/900/700px and assert with `getBoundingClientRect()` that the notice + never overlaps `#statusbar`, stays inside the viewport, and that the long unbroken + URL in the harness fixture does not widen it. **Not done on this branch** — the + browser harness could not be driven in the authoring session, so this is genuinely + unverified rather than assumed-fine. +- [ ] **D5 — Both engines.** The notice is `position: fixed` over a flex/grid shell; + confirm in WebKitGTK as well as WebView2 that it is not clipped by a pane's + `overflow: hidden` (it is parented to `body` specifically to avoid that). +- [ ] **D6 — Window state.** Move and resize the window, quit, relaunch: size, position + and maximized state should return. Then relaunch on a machine with **fewer or + smaller monitors** and confirm the window is not restored off-screen. +- [ ] **D7 — The check is quiet when it should be.** With no network, launch: nothing + appears. Ask via Help → Check for Updates: it says it could not check. That + asymmetry is the whole design and is easy to regress. + ## B. Standing items (pre-existing, not from this branch) - [ ] **B1 — HTML as a first-class format.** Open a real AI-authored `.html` artifact, @@ -192,8 +227,10 @@ rendered layout is unverified in either engine. The logic underneath it is gated - [ ] **B8 — Installer behavior.** NSIS installs per-user into `%LOCALAPPDATA%\Toril` with **no UAC prompt**, and the WebView2 bootstrapper runs on a clean Win10 box. README states both as user-facing guarantees (CLAUDE.md §9). -- [ ] **B9 — SmartScreen.** Unsigned installers warn on first run. Expected, not a bug — - until Azure Trusted Signing lands (ROADMAP Movement I, `feat/release-readiness`). +- [ ] **B9 — SmartScreen.** Unsigned installers warn on first run. Expected, not a bug. + The Authenticode wiring now exists and is **inert until an Azure Trusted Signing + account does** (`docs/RELEASE-SIGNING.md`) — so this stays open, and the README + wording stays as it is, until a signed installer has been run on a clean box. - [~] **B11 — Crash recovery.** *(Partly verified 2026-08-17, incidentally.)* Force-killing the process with three open buffers and relaunching restored all three, with "3 documents recovered" in the status bar — the journal survives a real `SIGKILL` diff --git a/docs/RELEASE-SIGNING.md b/docs/RELEASE-SIGNING.md new file mode 100644 index 0000000..ad4c9ee --- /dev/null +++ b/docs/RELEASE-SIGNING.md @@ -0,0 +1,93 @@ +# Release Signing & Self-Update + +Two different signatures, for two different problems. They are easy to confuse +because both are called "signing", so this file keeps them apart. + +| | **Update signing** (minisign) | **Code signing** (Authenticode) | +|---|---|---| +| Answers | "Did this update really come from Toril?" | "Is this installer from a known publisher?" | +| Needed for | The in-app updater to install anything | Windows SmartScreen to stop warning | +| Costs | Nothing | An Azure subscription (monthly) + identity validation | +| Blocks a release? | **Yes** — `release.yml` fails fast without it | No — builds unsigned, as today | + +--- + +## 1. Update signing — required + +`bundle.createUpdaterArtifacts` is on, so every release build produces signed +update artifacts plus a `latest.json` manifest. The bundler refuses to emit an +*unsigned* update, which is why the release workflow checks for the key up front +and stops with an explanation rather than failing deep in a Rust build. + +**One-time setup:** + +```bash +pnpm tauri signer generate -w ~/.tauri/toril-updater.key +``` + +It prints a public key and writes the private key to that path. Then: + +1. Paste the **public** key into `src-tauri/tauri.conf.json` → + `plugins.updater.pubkey`. It ships inside the app and is what an installed + copy uses to verify a downloaded update, so it must be committed. +2. Add the **private** key file's *contents* as the repository secret + `TAURI_SIGNING_PRIVATE_KEY`. +3. If you set a password, add it as `TAURI_SIGNING_PRIVATE_KEY_PASSWORD`. + +**Keep the private key.** It is not recoverable, and it is not rotatable in the +usual sense: every copy of Toril already installed verifies against the public +key it shipped with. Lose the private key and those copies can never be updated +again — their owners would have to download a new installer by hand, which is +the exact situation this branch exists to end. Back it up somewhere you would +also keep a password manager export. + +> **The private key must never be committed.** `.gitignore` covers the +> conventional `*.key` / `.tauri/` paths as a backstop, but the intended home is +> outside the repository entirely. + +--- + +## 2. Windows code signing — optional, and needs an account first + +Without it, SmartScreen warns on first run. That is expected, not a bug, and +`README.md` says so. + +Turning it on is **not** a code change you can finish in one sitting — it needs +an Azure Trusted Signing account and an identity validation that takes business +days. The wiring is already in place and inert until the secrets exist: + +- `src-tauri/tauri.signing.conf.json` — the `signCommand` overlay. It lives in + its own file, applied only in CI, so a fork or a local `pnpm tauri build` + still produces a working unsigned installer instead of failing on a missing + tool. Nobody needs an Azure subscription to build Toril. +- `.github/workflows/release.yml` — installs `trusted-signing-cli` and applies + the overlay **only** when `AZURE_CLIENT_ID` is present. + +**To enable it:** + +1. Create an Azure Trusted Signing account and a certificate profile, and + complete identity validation. +2. Update the endpoint / account / profile names in + `src-tauri/tauri.signing.conf.json` — the committed values are placeholders + (`toril-signing`, `toril`) and will not match your account. +3. Add repository secrets `AZURE_CLIENT_ID`, `AZURE_CLIENT_SECRET`, + `AZURE_TENANT_ID` for a service principal with the *Trusted Signing Certificate + Profile Signer* role. +4. Soften the SmartScreen wording in `README.md`, `docs/index.html`, and the + `releaseBody` in `release.yml`. Verify on a real Windows machine before you + do — B9 in `docs/ON-DEVICE-VERIFICATION.md`. + +--- + +## 3. What the app does with an update + +Policy lives in `src/update.ts` and is gated by `tests/update.test.ts`. + +- Checks at most once a day at launch, and whenever you ask via + **Help → Check for Updates…**. +- The check is a plain GET for a static manifest. No telemetry: nothing about + the user, the vault, or the session goes with it. +- **Toril never installs on its own.** It offers; you choose. A restart is + refused while any tab is unsaved — the install is already on disk and applies + next launch, so waiting costs nothing (§3). +- Automatic checks can be turned off in **View → Check for Updates on Launch**. diff --git a/package.json b/package.json index bbec2c1..278ef87 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,9 @@ "@milkdown/theme-nord": "7.21.1", "@tauri-apps/api": "2.11.0", "@tauri-apps/plugin-dialog": "2.7.1", + "@tauri-apps/plugin-process": "2.3.1", + "@tauri-apps/plugin-updater": "2.10.1", + "@tauri-apps/plugin-window-state": "2.4.1", "dompurify": "3.4.12", "smol-toml": "1.8.0", "yaml": "2.9.0" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a7e4724..db299be 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -31,6 +31,15 @@ importers: '@tauri-apps/plugin-dialog': specifier: 2.7.1 version: 2.7.1 + '@tauri-apps/plugin-process': + specifier: 2.3.1 + version: 2.3.1 + '@tauri-apps/plugin-updater': + specifier: 2.10.1 + version: 2.10.1 + '@tauri-apps/plugin-window-state': + specifier: 2.4.1 + version: 2.4.1 dompurify: specifier: 3.4.12 version: 3.4.12 @@ -635,6 +644,15 @@ packages: '@tauri-apps/plugin-dialog@2.7.1': resolution: {integrity: sha512-OK1UBXYt+ojcmxMktzzuyonYIFta8CmAASpX+CA+DTGK24KlHjhYI6x2iOJ/TjZF4N7/ACK1oFmEOjIY9IhzOQ==} + '@tauri-apps/plugin-process@2.3.1': + resolution: {integrity: sha512-nCa4fGVaDL/B9ai03VyPOjfAHRHSBz5v6F/ObsB73r/dA3MHHhZtldaDMIc0V/pnUw9ehzr2iEG+XkSEyC0JJA==} + + '@tauri-apps/plugin-updater@2.10.1': + resolution: {integrity: sha512-NFYMg+tWOZPJdzE/PpFj2qfqwAWwNS3kXrb1tm1gnBJ9mYzZ4WDRrwy8udzWoAnfGCHLuePNLY1WVCNHnh3eRA==} + + '@tauri-apps/plugin-window-state@2.4.1': + resolution: {integrity: sha512-OuvdrzyY8Q5Dbzpj+GcrnV1iCeoZbcFdzMjanZMMcAEUNy/6PH5pxZPXpaZLOR7whlzXiuzx0L9EKZbH7zpdRw==} + '@types/chai@5.2.3': resolution: {integrity: sha512-Mw558oeA9fFbv65/y4mHtXDs9bPnFMZAL/jxdPFUpOHHIXX91mcgEHbS5Lahr+pwZFR8A7GQleRWeI6cGFC2UA==} @@ -1981,6 +1999,18 @@ snapshots: dependencies: '@tauri-apps/api': 2.11.0 + '@tauri-apps/plugin-process@2.3.1': + dependencies: + '@tauri-apps/api': 2.11.0 + + '@tauri-apps/plugin-updater@2.10.1': + dependencies: + '@tauri-apps/api': 2.11.0 + + '@tauri-apps/plugin-window-state@2.4.1': + dependencies: + '@tauri-apps/api': 2.11.0 + '@types/chai@5.2.3': dependencies: '@types/deep-eql': 4.0.2 diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 2cc252c..a20dfc0 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -119,6 +119,15 @@ dependencies = [ "security-framework", ] +[[package]] +name = "arbitrary" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3d036a3c4ab069c7b410a2ce876bd74808d2d0888a82667669f8e783a898bf1" +dependencies = [ + "derive_arbitrary", +] + [[package]] name = "async-broadcast" version = "0.7.2" @@ -885,6 +894,17 @@ dependencies = [ "serde_core", ] +[[package]] +name = "derive_arbitrary" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e567bd82dcff979e4b03460c307b3cdc9e96fde3d73bed1496d2bc75d9dd62a" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "derive_more" version = "2.1.1" @@ -1196,6 +1216,16 @@ dependencies = [ "rustc_version", ] +[[package]] +name = "filetime" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c287a33c7f0a620c38e641e7f60827713987b3c0f26e8ddc9462cc69cf75759" +dependencies = [ + "cfg-if", + "libc", +] + [[package]] name = "find-msvc-tools" version = "0.1.9" @@ -1798,6 +1828,21 @@ dependencies = [ "want", ] +[[package]] +name = "hyper-rustls" +version = "0.27.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33ca68d021ef39cf6463ab54c1d0f5daf03377b70561305bb89a8f83aab66e0f" +dependencies = [ + "http", + "hyper", + "hyper-util", + "rustls", + "tokio", + "tokio-rustls", + "tower-service", +] + [[package]] name = "hyper-util" version = "0.1.20" @@ -2102,6 +2147,36 @@ dependencies = [ "windows-sys 0.45.0", ] +[[package]] +name = "jni" +version = "0.22.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5efd9a482cf3a427f00d6b35f14332adc7902ce91efb778580e180ff90fa3498" +dependencies = [ + "cfg-if", + "combine", + "jni-macros", + "jni-sys 0.4.1", + "log", + "simd_cesu8", + "thiserror 2.0.18", + "walkdir", + "windows-link 0.2.1", +] + +[[package]] +name = "jni-macros" +version = "0.22.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a00109accc170f0bdb141fed3e393c565b6f5e072365c3bd58f5b062591560a3" +dependencies = [ + "proc-macro2", + "quote", + "rustc_version", + "simd_cesu8", + "syn 2.0.117", +] + [[package]] name = "jni-sys" version = "0.3.1" @@ -2374,6 +2449,12 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" +[[package]] +name = "minisign-verify" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22f9645cb765ea72b8111f36c522475d2daa0d22c957a9826437e97534bc4e9e" + [[package]] name = "miniz_oxide" version = "0.8.9" @@ -2713,6 +2794,18 @@ dependencies = [ "objc2-core-foundation", ] +[[package]] +name = "objc2-osa-kit" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f112d1746737b0da274ef79a23aac283376f335f4095a083a267a082f21db0c0" +dependencies = [ + "bitflags 2.11.1", + "objc2", + "objc2-app-kit", + "objc2-foundation", +] + [[package]] name = "objc2-quartz-core" version = "0.3.2" @@ -2804,6 +2897,12 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "openssl-probe" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" + [[package]] name = "option-ext" version = "0.2.0" @@ -2820,6 +2919,20 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "osakit" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "732c71caeaa72c065bb69d7ea08717bd3f4863a4f451402fc9513e29dbd5261b" +dependencies = [ + "objc2", + "objc2-foundation", + "objc2-osa-kit", + "serde", + "serde_json", + "thiserror 2.0.18", +] + [[package]] name = "pango" version = "0.18.3" @@ -3221,15 +3334,20 @@ dependencies = [ "http-body", "http-body-util", "hyper", + "hyper-rustls", "hyper-util", "js-sys", "log", "percent-encoding", "pin-project-lite", + "rustls", + "rustls-pki-types", + "rustls-platform-verifier", "serde", "serde_json", "sync_wrapper", "tokio", + "tokio-rustls", "tokio-util", "tower", "tower-http", @@ -3265,6 +3383,20 @@ dependencies = [ "windows-sys 0.60.2", ] +[[package]] +name = "ring" +version = "0.17.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" +dependencies = [ + "cc", + "cfg-if", + "getrandom 0.2.17", + "libc", + "untrusted", + "windows-sys 0.52.0", +] + [[package]] name = "rustc-hash" version = "2.1.2" @@ -3293,6 +3425,79 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustls" +version = "0.23.43" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0283386ce02abc0151e1761d08802dfe86c173b0b494af5cbc086574e453da06" +dependencies = [ + "once_cell", + "ring", + "rustls-pki-types", + "rustls-webpki", + "subtle", + "zeroize", +] + +[[package]] +name = "rustls-native-certs" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dab5152771c58876a2146916e53e35057e1a4dfa2b9df0f0305b07f611fdea4d" +dependencies = [ + "openssl-probe", + "rustls-pki-types", + "schannel", + "security-framework", +] + +[[package]] +name = "rustls-pki-types" +version = "1.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f4925028c7eb5d1fcdaf196971378ed9d2c1c4efc7dc5d011256f76c99c0a96" +dependencies = [ + "zeroize", +] + +[[package]] +name = "rustls-platform-verifier" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26d1e2536ce4f35f4846aa13bff16bd0ff40157cdb14cc056c7b14ba41233ba0" +dependencies = [ + "core-foundation", + "core-foundation-sys", + "jni 0.22.4", + "log", + "once_cell", + "rustls", + "rustls-native-certs", + "rustls-platform-verifier-android", + "rustls-webpki", + "security-framework", + "security-framework-sys", + "webpki-root-certs", + "windows-sys 0.61.2", +] + +[[package]] +name = "rustls-platform-verifier-android" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f87165f0995f63a9fbeea62b64d10b4d9d8e78ec6d7d51fb2125fda7bb36788f" + +[[package]] +name = "rustls-webpki" +version = "0.103.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0527518605e68109d875e248ea259b6758801cf165e4b2c2733ae3b51f12535a" +dependencies = [ + "ring", + "rustls-pki-types", + "untrusted", +] + [[package]] name = "rustversion" version = "1.0.22" @@ -3308,6 +3513,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "schannel" +version = "0.1.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91c1b7e4904c873ef0710c1f407dde2e6287de2bebc1bbbf7d430bb7cbffd939" +dependencies = [ + "windows-sys 0.61.2", +] + [[package]] name = "schemars" version = "0.8.22" @@ -3633,6 +3847,22 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "703d5c7ef118737c72f1af64ad2f6f8c5e1921f818cdcb97b8fe6fc69bf66214" +[[package]] +name = "simd_cesu8" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11031e251abf8611c80f460e19dbdeb54a66db918e49c65a7065b46ac7aec520" +dependencies = [ + "rustc_version", + "simdutf8", +] + +[[package]] +name = "simdutf8" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3a9fe34e3e7a50316060351f37187a3f546bce95496156754b601a5fa71b76e" + [[package]] name = "similar" version = "2.7.0" @@ -3873,7 +4103,7 @@ dependencies = [ "gdkwayland-sys", "gdkx11-sys", "gtk", - "jni", + "jni 0.21.1", "libc", "log", "ndk", @@ -3906,6 +4136,17 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "tar" +version = "0.4.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6221d9a6003c78398e3b239969f352578258df48c8eb051caadae0015bc840" +dependencies = [ + "filetime", + "libc", + "xattr", +] + [[package]] name = "target-lexicon" version = "0.12.16" @@ -3929,7 +4170,7 @@ dependencies = [ "gtk", "heck 0.5.0", "http", - "jni", + "jni 0.21.1", "libc", "log", "mime", @@ -4083,6 +4324,16 @@ dependencies = [ "url", ] +[[package]] +name = "tauri-plugin-process" +version = "2.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d55511a7bf6cd70c8767b02c97bf8134fa434daf3926cfc1be0a0f94132d165a" +dependencies = [ + "tauri", + "tauri-plugin", +] + [[package]] name = "tauri-plugin-single-instance" version = "2.4.2" @@ -4098,6 +4349,54 @@ dependencies = [ "zbus", ] +[[package]] +name = "tauri-plugin-updater" +version = "2.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "806d9dac662c2e4594ff03c647a552f2c9bd544e7d0f683ec58f872f952ce4af" +dependencies = [ + "base64 0.22.1", + "dirs", + "flate2", + "futures-util", + "http", + "infer", + "log", + "minisign-verify", + "osakit", + "percent-encoding", + "reqwest", + "rustls", + "semver", + "serde", + "serde_json", + "tar", + "tauri", + "tauri-plugin", + "tempfile", + "thiserror 2.0.18", + "time", + "tokio", + "url", + "windows-sys 0.60.2", + "zip", +] + +[[package]] +name = "tauri-plugin-window-state" +version = "2.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73736611e14142408d15353e21e3cca2f12a3cfb523ad0ce85999b6d2ef1a704" +dependencies = [ + "bitflags 2.11.1", + "log", + "serde", + "serde_json", + "tauri", + "tauri-plugin", + "thiserror 2.0.18", +] + [[package]] name = "tauri-runtime" version = "2.11.2" @@ -4108,7 +4407,7 @@ dependencies = [ "dpi", "gtk", "http", - "jni", + "jni 0.21.1", "objc2", "objc2-ui-kit", "objc2-web-kit", @@ -4131,7 +4430,7 @@ checksum = "b83849ee63ecb27a8e8d0fe51915ca215076914aca43f96db1179f0f415f6cd9" dependencies = [ "gtk", "http", - "jni", + "jni 0.21.1", "log", "objc2", "objc2-app-kit", @@ -4341,6 +4640,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "tokio-rustls" +version = "0.26.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" +dependencies = [ + "rustls", + "tokio", +] + [[package]] name = "tokio-util" version = "0.7.18" @@ -4491,7 +4800,10 @@ dependencies = [ "tauri", "tauri-build", "tauri-plugin-dialog", + "tauri-plugin-process", "tauri-plugin-single-instance", + "tauri-plugin-updater", + "tauri-plugin-window-state", "trashbin", "vaultscan", ] @@ -4706,6 +5018,12 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "untrusted" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" + [[package]] name = "url" version = "2.5.8" @@ -5011,6 +5329,15 @@ dependencies = [ "system-deps", ] +[[package]] +name = "webpki-root-certs" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b96554aa2acc8ccdb7e1c9a58a7a68dd5d13bccc69cd124cb09406db612a1c9b" +dependencies = [ + "rustls-pki-types", +] + [[package]] name = "webview2-com" version = "0.38.2" @@ -5223,6 +5550,15 @@ dependencies = [ "windows-targets 0.42.2", ] +[[package]] +name = "windows-sys" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" +dependencies = [ + "windows-targets 0.52.6", +] + [[package]] name = "windows-sys" version = "0.59.0" @@ -5606,7 +5942,7 @@ dependencies = [ "gtk", "http", "javascriptcore-rs", - "jni", + "jni 0.21.1", "libc", "ndk", "objc2", @@ -5653,6 +5989,16 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "xattr" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32e45ad4206f6d2479085147f02bc2ef834ac85886624a23575ae137c8aa8156" +dependencies = [ + "libc", + "rustix", +] + [[package]] name = "xdg" version = "3.0.0" @@ -5823,6 +6169,18 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "zip" +version = "4.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "caa8cd6af31c3b31c6631b8f483848b91589021b28fffe50adada48d4f4d2ed1" +dependencies = [ + "arbitrary", + "crc32fast", + "indexmap 2.14.0", + "memchr", +] + [[package]] name = "zmij" version = "1.0.21" diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 94a1ae8..58a842e 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -45,6 +45,10 @@ tauri-plugin-dialog = "2" # spawning a duplicate process (file-association open, CLAUDE.md §5). Official # Tauri plugin (§2 healthy-dep rule). tauri-plugin-single-instance = "2" +# Remember the window's size, position and maximized state across launches. +# Official Tauri plugin (§2). Purely presentational — it persists to its own +# file in the app config dir and touches nothing in the vault. +tauri-plugin-window-state = "2" serde = { version = "1", features = ["derive"] } serde_json = "1" notify = "8" @@ -58,6 +62,21 @@ snapshots = { path = "crates/snapshots" } mergemd = { path = "crates/mergemd" } keystore = { path = "crates/keystore" } +# Self-update, desktop only (ROADMAP Movement I.5). `v1.0.0` shipped with no +# update path, so every installed copy is stranded on the version it was +# installed with — this is what gives it a way forward. Official Tauri plugins +# (§2). Scoped to desktop because the mobile targets (Movement V.28) install +# through their app stores and have no business linking an updater. +# +# **Notify-only, by policy** (`src/update.ts`): the plugin can download and +# install silently, and we deliberately never call it that way. Swapping the +# binary under an editor holding unsaved buffers is not a §3 trade we make. +# `tauri-plugin-process` is here for the one restart the user explicitly asks +# for after an install they explicitly accepted. +[target.'cfg(not(any(target_os = "android", target_os = "ios")))'.dependencies] +tauri-plugin-updater = "2" +tauri-plugin-process = "2" + # Security patch for glib (Linux/gtk3 transitive dep via Tauri). # GHSA-wrw7-89jp-8q8g: VariantStrIter::impl_get passed a NULL out-pointer as # `&p` instead of `&mut p` — unsound, and dropped under optimization, causing diff --git a/src-tauri/capabilities/default.json b/src-tauri/capabilities/default.json index dd51a8f..3c24d22 100644 --- a/src-tauri/capabilities/default.json +++ b/src-tauri/capabilities/default.json @@ -6,6 +6,8 @@ "permissions": [ "core:default", "core:window:allow-destroy", - "dialog:default" + "dialog:default", + "updater:default", + "process:allow-restart" ] } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 58b42ce..5827135 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -34,7 +34,21 @@ fn take_launch_path(state: tauri::State<'_, LaunchPath>) -> Option { pub fn run() { let launch_path = LaunchPath(Mutex::new(launch_path_from_args(std::env::args()))); - tauri::Builder::default() + #[allow(unused_mut)] + let mut builder = tauri::Builder::default(); + + // Self-update, desktop only — the mobile targets install through their app + // stores (ROADMAP Movement V.28). Registering the plugin only exposes the + // *ability* to check and install; whether either happens is decided in + // `src/update.ts`, which never installs without the user saying so. + #[cfg(desktop)] + { + builder = builder + .plugin(tauri_plugin_updater::Builder::new().build()) + .plugin(tauri_plugin_process::init()); + } + + builder // Single-instance: a second launch (e.g. double-clicking another file // while Toril is open) forwards its argv here instead of starting a new // process — open the file in the existing window and focus it. @@ -47,6 +61,9 @@ pub fn run() { } })) .plugin(tauri_plugin_dialog::init()) + // Remember where the window was and how big it was. Presentational + // only — it writes its own file in the app config dir, never the vault. + .plugin(tauri_plugin_window_state::Builder::default().build()) .menu(menu::build) .on_menu_event(menu::on_event) .manage(launch_path) diff --git a/src-tauri/src/menu.rs b/src-tauri/src/menu.rs index 9053466..b79e281 100644 --- a/src-tauri/src/menu.rs +++ b/src-tauri/src/menu.rs @@ -108,6 +108,9 @@ pub fn build(app: &AppHandle) -> tauri::Result> { MenuItemBuilder::with_id("menu_toggle_history", "Version &History").build(app)?; let toggle_autosave = MenuItemBuilder::with_id("menu_toggle_autosave", "&Autosave").build(app)?; + let toggle_update_check = + MenuItemBuilder::with_id("menu_toggle_update_check", "Check for &Updates on Launch") + .build(app)?; let view = SubmenuBuilder::new(app, "&View") .item(&find) @@ -117,9 +120,15 @@ pub fn build(app: &AppHandle) -> tauri::Result> { .item(&toggle_history) .separator() .item(&toggle_autosave) + .item(&toggle_update_check) .build()?; + // "Check for Updates…" sits in Help rather than File because it is about the + // application, not the document — and it is where every desktop app the + // target user has used puts it. let help = SubmenuBuilder::new(app, "&Help") + .text("menu_check_updates", "Check for &Updates…") + .separator() .text("menu_about", "&About Toril") .build()?; diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index 8a6901f..f1a3913 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -59,6 +59,19 @@ pub struct Settings { pub autosave: Option, /// Autosave/journal debounce in ms. `None` ⇒ 2000 (frontend default). pub autosave_debounce_ms: Option, + /// Whether Toril checks for a newer build on launch. `None` ⇒ on. + pub update_check: Option, + /// Epoch **milliseconds** of the last completed check; `None` ⇒ never. + /// + /// `i64`, not `u64`: this comes from the webview's `Date.now()`, and a + /// machine whose clock is set before 1970 would otherwise fail to + /// deserialize and take the whole settings file down to defaults with it — + /// losing the open tabs and the workspace over a wrong clock. The policy in + /// `src/update.ts` already treats a nonsensical timestamp as "due". + pub update_last_checked: Option, + /// A version the user dismissed. Startup will not raise it again; an + /// explicit Help → Check for Updates still will. + pub update_skipped_version: Option, } fn settings_path(app: &AppHandle) -> Result { diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index bcc9b23..4e355bc 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -23,8 +23,18 @@ "csp": null } }, + "plugins": { + "updater": { + "endpoints": [ + "https://github.com/kovirlabs/toril/releases/latest/download/latest.json" + ], + "windows": { "installMode": "passive" }, + "pubkey": "" + } + }, "bundle": { "active": true, + "createUpdaterArtifacts": true, "targets": ["nsis", "msi", "app", "dmg", "deb", "rpm", "appimage"], "windows": { "webviewInstallMode": { "type": "downloadBootstrapper" }, diff --git a/src-tauri/tauri.signing.conf.json b/src-tauri/tauri.signing.conf.json new file mode 100644 index 0000000..7a89013 --- /dev/null +++ b/src-tauri/tauri.signing.conf.json @@ -0,0 +1,8 @@ +{ + "$schema": "https://schema.tauri.app/config/2", + "bundle": { + "windows": { + "signCommand": "trusted-signing-cli -e https://eus.codesigning.azure.net -a toril-signing -c toril -d Toril %1" + } + } +} diff --git a/src/ipc.ts b/src/ipc.ts index afffd80..cab0c56 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -153,6 +153,58 @@ export async function showAbout(): Promise { }); } +// ---- Self-update (ROADMAP Movement I.5) ------------------------------------ +// +// The updater and process plugins are the one pair of backend calls that do not +// go through `invoke()` by hand — they ship their own typed JS API. They are +// still declared here so `ipc.ts` stays the single inventory of what the +// frontend can ask the backend to do (§10), and so the rest of the app never +// imports the plugin directly and cannot reach past this seam. + +/** An available update, reduced to what the policy and the banner need. */ +export interface AvailableUpdate { + version: string; + notes?: string; + /** + * Download and install, then resolve. Deliberately a callback on the object + * rather than a free function: an install can only ever apply to an update + * that was actually found and shown, so there is no way to spell "install" + * without having first checked. + */ + install(): Promise; +} + +/** + * Ask whether a newer build exists. Resolves to `null` when up to date. + * + * This performs a plain GET for a static manifest and sends nothing about the + * user, the vault or the session — there is no telemetry in Toril and this is + * not a back door for it. *Whether* to call this is decided by `update.ts`; this + * function only does what it is told. + */ +export async function checkForUpdate(): Promise { + const { check } = await import("@tauri-apps/plugin-updater"); + const found = await check(); + if (!found) return null; + return { + version: found.version, + notes: found.body ?? undefined, + install: () => found.downloadAndInstall(), + }; +} + +/** + * Restart into the freshly installed build. + * + * Only ever called after the user accepted an install *and* Toril confirmed + * there is nothing unsaved — see `main.ts`. Relaunching over a dirty buffer + * would be a §3 data-loss path dressed as a convenience. + */ +export async function relaunchApp(): Promise { + const { relaunch } = await import("@tauri-apps/plugin-process"); + await relaunch(); +} + /** Persisted session + preferences (mirrors Rust `settings::Settings`, §5). */ export interface Settings { version: number; @@ -185,6 +237,12 @@ export interface Settings { autosave: boolean | null; /** Autosave/journal debounce in ms. `null` ⇒ 2000 (default). */ autosave_debounce_ms: number | null; + /** Whether Toril checks for updates on launch. `null` ⇒ on (default). */ + update_check: boolean | null; + /** Epoch ms of the last completed check. `null` ⇒ never checked. */ + update_last_checked: number | null; + /** A version the user dismissed; startup will not raise it again. */ + update_skipped_version: string | null; } /** Load persisted settings; resolves to defaults if none exist or the file is corrupt. */ diff --git a/src/main.ts b/src/main.ts index 0369143..432385a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,6 +17,7 @@ import { type Settings, type UnlistenFn, type WorkspaceChange, + checkForUpdate, clearRecovery, exportHtml, exportRtf, @@ -35,6 +36,7 @@ import { pickFileToOpen, pickFolder, readSnapshot, + relaunchApp, restoreSnapshot, saveClipboardImage, saveFile, @@ -57,7 +59,15 @@ import { selectRemovedOnDisk, selectSavable, } from "./sync"; +import { + decideCheck, + decideErrorPresentation, + decidePresentation, + type UpdateState, + type UpdateTrigger, +} from "./update"; import { ConflictBar } from "./ui/conflictbar"; +import { UpdateNotice } from "./ui/updatenotice"; import { PropertiesStrip } from "./ui/properties"; import { History } from "./ui/history"; import { Outline } from "./ui/outline"; @@ -106,6 +116,18 @@ let properties: PropertiesStrip | null = null; let propertiesExpanded = true; let autosaveDebounceMs = 2000; let theme: ThemeController | null = null; +let updateNotice: UpdateNotice | null = null; +/** + * Update-check state, persisted across launches. Defaults to checking: an + * editor that cannot tell you it is out of date is how `v1.0.0` stranded every + * copy of itself. The check sends nothing about you (see `ipc.checkForUpdate`), + * and it can be turned off in the View menu. + */ +const updateState: UpdateState = { + enabled: true, + lastCheckedAt: null, + skippedVersion: null, +}; let workspaceRoot: string | null = null; let panes: PaneState = defaultPaneState(); @@ -708,6 +730,107 @@ function toggleAutosave(): void { if (autosaveEnabled) autosave?.notifyChange(); } +// ---- Self-update (ROADMAP Movement I.5) ------------------------------------ +// +// The rules live in `update.ts` and are gated there; this is only the wiring +// that connects them to the network, the toast and persisted settings. Nothing +// here decides anything on its own — that separation is what makes the policy +// testable without a release server. + +function toggleUpdateCheck(): void { + updateState.enabled = !updateState.enabled; + setStatus(`Update checks ${updateState.enabled ? "on" : "off"}`); + scheduleSessionSave(); +} + +/** + * Run an update check, if the policy allows one, and present whatever comes + * back. + * + * Always resolves — a startup check must never be able to reject into an + * unhandled rejection during bootstrap, and a failure to reach the network is + * not something to trouble a writer with. + */ +async function checkUpdates(trigger: UpdateTrigger): Promise { + const decision = decideCheck(trigger, updateState, Date.now()); + if (decision.kind === "skip") return; + + let presentation; + try { + const found = await checkForUpdate(); + // Recorded only on a *completed* check, so an offline launch doesn't burn + // the day's allowance and leave the user a version behind for another 24h. + updateState.lastCheckedAt = Date.now(); + scheduleSessionSave(); + presentation = decidePresentation(trigger, found, updateState); + if (presentation.kind === "offer" && found) offerUpdate(found); + } catch (e) { + presentation = decideErrorPresentation(trigger, String(e)); + } + + if (presentation.kind === "up-to-date") { + updateNotice?.info("Toril is up to date.", () => updateNotice?.hide()); + } else if (presentation.kind === "error") { + updateNotice?.info(`Could not check for updates: ${presentation.message}`, () => + updateNotice?.hide(), + ); + } +} + +/** Show the offer, and own the install → restart sequence if the user takes it. */ +function offerUpdate(found: { version: string; notes?: string; install(): Promise }): void { + updateNotice?.show({ + version: found.version, + notes: found.notes, + onInstall: () => { + updateNotice?.busy(`Downloading Toril ${found.version}…`); + found + .install() + .then(() => { + updateNotice?.installed( + () => void restartForUpdate(), + () => updateNotice?.hide(), + ); + }) + .catch((e: unknown) => { + updateNotice?.failed(String(e), () => updateNotice?.hide()); + }); + }, + onSkip: () => { + updateState.skippedVersion = found.version; + scheduleSessionSave(); + updateNotice?.hide(); + }, + onDismiss: () => updateNotice?.hide(), + }); +} + +/** + * Restart into the new build — but not over unsaved work. + * + * This is the one place in the update flow that can destroy a buffer, so it is + * the one place that checks. A relaunch is a process exit: an unsaved tab would + * be gone, and "I clicked the update button" is not consent to lose a + * paragraph (§3). The install is already on disk either way, so refusing here + * costs the user nothing but a save. + */ +async function restartForUpdate(): Promise { + const dirty = tabs.list().filter((t) => t.dirty); + if (dirty.length > 0) { + updateNotice?.info( + `Save your work first — ${dirty.length} unsaved document${dirty.length === 1 ? "" : "s"}. The update is installed and will apply next time you start Toril.`, + () => updateNotice?.hide(), + ); + return; + } + // A clean exit: drop the recovery journal so the next launch doesn't offer to + // restore buffers that were already saved (see `installCloseGuard`). + await clearRecovery().catch(() => {}); + await relaunchApp().catch((e: unknown) => { + updateNotice?.failed(String(e), () => updateNotice?.hide()); + }); +} + // ---- Export ---------------------------------------------------------------- /** @@ -1143,6 +1266,9 @@ function scheduleSessionSave(): void { properties_expanded: propertiesExpanded, autosave: autosaveEnabled, autosave_debounce_ms: autosaveDebounceMs, + update_check: updateState.enabled, + update_last_checked: updateState.lastCheckedAt, + update_skipped_version: updateState.skippedVersion, }; void saveSettings(settings).catch(() => {}); // best-effort }, 400); @@ -1182,6 +1308,10 @@ async function restoreSession(): Promise { if (settings.autosave_debounce_ms !== null) autosaveDebounceMs = settings.autosave_debounce_ms; autosave?.setConfig({ enabled: autosaveEnabled, debounceMs: autosaveDebounceMs }); + if (settings.update_check !== null) updateState.enabled = settings.update_check; + updateState.lastCheckedAt = settings.update_last_checked; + updateState.skippedVersion = settings.update_skipped_version; + if (settings.last_folder) { try { await loadWorkspace(settings.last_folder); @@ -1281,6 +1411,8 @@ const ACTIONS: Record void> = { menu_toggle_outline: () => selectRail("outline"), menu_toggle_history: () => selectRail("history"), menu_toggle_autosave: () => toggleAutosave(), + menu_toggle_update_check: () => toggleUpdateCheck(), + menu_check_updates: () => void checkUpdates("manual"), menu_export_html: () => void doExportHtml(), menu_export_rtf: () => void doExportRtf(), menu_find: () => searchBar?.open(), @@ -1389,6 +1521,10 @@ window.addEventListener("DOMContentLoaded", async () => { const conflictEl = document.querySelector("#conflictbar"); if (conflictEl) conflictBar = new ConflictBar(conflictEl); + // On `body`, not in #main: the update toast is out of flow (see + // `ui/updatenotice.ts`), so it must not inherit a pane's overflow clipping. + updateNotice = new UpdateNotice(document.body); + // Above the writing surface, below the banner: front matter reads as the top // of the document, and the strip never touches the doc, the tab, or disk — it // hands back a complete block and this file decides what that means. @@ -1471,4 +1607,10 @@ window.addEventListener("DOMContentLoaded", async () => { // While Toril is already running, a second double-click is forwarded here by // the single-instance plugin rather than starting a new process (§5). void onOpenFile((path) => void openPath(path)); + + // Last, and only after the session is on screen: the check is a network round + // trip, and nothing about it should delay a document appearing. `checkUpdates` + // never rejects, and at startup it stays silent unless there is genuinely + // something to offer. + void checkUpdates("startup"); }); diff --git a/src/styles/chrome.css b/src/styles/chrome.css index c3d6687..c80668f 100644 --- a/src/styles/chrome.css +++ b/src/styles/chrome.css @@ -747,6 +747,73 @@ body { background: rgb(255 255 255 / 12%); } +/* ---- Update notice -------------------------------------------------------- */ +/* Out of flow on purpose (see `ui/updatenotice.ts`): an update is ambient news, + so it must not reflow the writing surface the way the conflict banner + deliberately does. Anchored bottom-right, clear of the status bar. */ +.update-notice { + position: fixed; + right: var(--sp-4); + /* Above the status bar rather than over it — the status bar is short and + fixed, so a constant clearance is honest here without measuring it. */ + bottom: calc(var(--sp-6) + var(--sp-2)); + z-index: 20; + display: flex; + flex-direction: column; + gap: var(--sp-2); + /* Bounded so long release notes scroll inside the toast instead of growing it + across the editor (§12b rule 4). `max-width` is in ch so it tracks the font + rather than assuming a viewport. */ + max-width: min(46ch, calc(100vw - var(--sp-6))); + max-height: 40vh; + overflow-y: auto; + padding: var(--sp-3); + border: 1px solid var(--border-strong); + border-radius: var(--r-lg); + background: var(--chrome-raised); + color: var(--fg); + box-shadow: var(--shadow-panel); + font-size: var(--text-md); +} + +.update-notice[hidden] { + display: none; +} + +.update-notice-text { + margin: 0; + min-width: 0; + /* One long unbroken token in release notes must not set the toast's + min-content width (§12b rule 4). */ + overflow-wrap: anywhere; +} + +.update-notice-notes { + color: var(--muted); + font-size: var(--text-sm); +} + +.update-notice-actions { + display: flex; + flex-wrap: wrap; + gap: var(--sp-2); +} + +.update-notice-btn { + min-height: var(--target-sm); + padding: 0 var(--sp-3); + border: 1px solid var(--border-strong); + border-radius: var(--r-sm); + background: transparent; + color: inherit; + cursor: pointer; + font: inherit; +} + +.update-notice-btn:hover { + background: var(--hover-bg); +} + /* ---- Properties strip ----------------------------------------------------- */ /* A flex row in the #main column, above the writing surface (§12b rule 1). It holds its own scroll rather than growing without bound, so a note with thirty diff --git a/src/ui/updatenotice.ts b/src/ui/updatenotice.ts new file mode 100644 index 0000000..dffc12f --- /dev/null +++ b/src/ui/updatenotice.ts @@ -0,0 +1,144 @@ +// The update notice (ROADMAP Movement I.5). +// +// Deliberately *not* shaped like `conflictbar.ts`, and the difference is the +// point. A conflict is about the document in front of you and must be +// impossible to miss, so it takes a row in the layout and pushes the editor +// down. An update is ambient news about the application: it is worth telling you +// and never worth interrupting a sentence for. So this is an out-of-flow toast +// — it reflows nothing, it can be dismissed, and a writer who ignores it loses +// nothing. +// +// That choice also keeps this branch out of `body`'s grid, which currently +// declares rows and no columns (§12b rule 2). Adding a third row there to +// announce a point release is not a trade worth making. +// +// **Nothing here installs anything on its own.** Every transition below is +// driven by a click; see `src/update.ts` for why. + +export interface UpdateNoticeOptions { + version: string; + /** Release notes from the manifest, when it carries them. */ + notes?: string; + /** Download and install. The notice goes busy until the caller says otherwise. */ + onInstall(): void; + /** Don't raise this version again automatically. */ + onSkip(): void; + /** Close for now; a later launch may raise it again. */ + onDismiss(): void; +} + +export class UpdateNotice { + private readonly el: HTMLDivElement; + /** + * The live region, created once and never replaced — same reasoning as + * `ConflictBar.text`: assistive tech announces *mutations* to a region that is + * already exposed, so a span built fresh inside a hidden container announces + * nothing. + */ + private readonly text: HTMLParagraphElement; + private readonly actions: HTMLDivElement; + + constructor(host: HTMLElement) { + this.el = document.createElement("div"); + this.el.className = "update-notice"; + this.el.hidden = true; + // `status`, not `alert`: an available update is not urgent, and `alert` + // interrupts a screen-reader user mid-word to say so. + this.el.setAttribute("role", "status"); + this.el.setAttribute("aria-label", "Software update"); + + this.text = document.createElement("p"); + this.text.className = "update-notice-text"; + this.text.setAttribute("aria-live", "polite"); + this.el.appendChild(this.text); + + this.actions = document.createElement("div"); + this.actions.className = "update-notice-actions"; + this.el.appendChild(this.actions); + + host.appendChild(this.el); + } + + private button(label: string, title: string, onClick: () => void): HTMLButtonElement { + const b = document.createElement("button"); + b.type = "button"; + b.className = "update-notice-btn"; + b.textContent = label; + b.title = title; + b.addEventListener("click", onClick); + this.actions.appendChild(b); + return b; + } + + private reset(): void { + this.actions.replaceChildren(); + this.el.hidden = false; // exposed before the text changes — see `text` + } + + /** Offer an available update. */ + show(opts: UpdateNoticeOptions): void { + this.reset(); + // The version is the whole message; notes are supporting detail and are + // truncated rather than allowed to grow the toast over the editor. + this.text.textContent = `Toril ${opts.version} is available.`; + if (opts.notes) { + const notes = document.createElement("span"); + notes.className = "update-notice-notes"; + notes.textContent = opts.notes.slice(0, 240); + this.text.appendChild(document.createElement("br")); + this.text.appendChild(notes); + } + + this.button("Install", "Download and install this update", opts.onInstall); + this.button("Skip", "Don't mention this version again", opts.onSkip); + this.button("Later", "Close — Toril may mention it again next launch", opts.onDismiss); + } + + /** + * Installing. No buttons: the only actions available were Install (now + * running) and dismissals that would strand a half-written binary behind a + * closed toast. + */ + busy(message: string): void { + this.reset(); + this.text.textContent = message; + } + + /** Installed — offer the restart, which is the user's call, not ours. */ + installed(onRestart: () => void, onLater: () => void): void { + this.reset(); + this.text.textContent = "Update installed. Restart to finish."; + this.button("Restart now", "Close Toril and reopen on the new version", onRestart); + this.button("Later", "Finish the next time you start Toril", onLater); + } + + /** Something went wrong. Says so, and gets out of the way. */ + failed(message: string, onDismiss: () => void): void { + this.reset(); + this.text.textContent = `Update failed: ${message}`; + this.button("Close", "Dismiss", onDismiss); + } + + /** + * A plain statement with nothing to act on — "you are up to date", or why a + * check failed. + * + * Reusing the toast rather than raising a native `message()` dialog keeps one + * mechanism for everything the updater has to say, and keeps a modal off the + * screen: even for an answer the user asked for, a dialog steals focus from + * the editor and has to be clicked away before typing resumes. + */ + info(message: string, onDismiss: () => void): void { + this.reset(); + this.text.textContent = message; + this.button("Close", "Dismiss", onDismiss); + } + + hide(): void { + this.el.hidden = true; + this.actions.replaceChildren(); + // Emptied, not removed: the next show() has to be a mutation on a region + // that already exists. + this.text.textContent = ""; + } +} diff --git a/src/update.ts b/src/update.ts new file mode 100644 index 0000000..2d66ba2 --- /dev/null +++ b/src/update.ts @@ -0,0 +1,125 @@ +// Update-check policy (ROADMAP Movement I.5). +// +// `v1.0.0` shipped with no update path at all, so every installed copy is +// stranded on the version it was installed with. This module is the decision +// layer that fixes that — *when* Toril asks whether a newer build exists, and +// *whether* the answer is worth interrupting the user for. +// +// It is deliberately pure: no DOM, no `invoke`, no clock of its own. The network +// call lives in `ipc.ts` and the banner in `ui/updatebar.ts`, so the rules below +// are gated headlessly (§8) rather than discovered by waiting a day for a timer +// to elapse on a device. +// +// **Toril notifies; it never installs behind you.** There is no silent +// download-and-replace: this is an editor that holds unsaved buffers, and §3 +// says the user's writing outranks convenience. Swapping the binary under a live +// document to save someone two clicks is not a trade we make. The plugin *can* +// auto-install; we don't call it that way. +// +// No telemetry either — the check is a plain GET for a static manifest. Nothing +// about the user, the vault, or the session goes with it. + +/** How long a startup check stays satisfied. Once a day is plenty for an editor. */ +export const CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000; + +/** + * What triggered the check, which is the single input that changes every rule + * below. + * + * `"startup"` is Toril's own idea and must stay quiet: rate-limited, skippable, + * silent when there is nothing to say. `"manual"` is the user asking a direct + * question via Help → Check for Updates, and a direct question always gets a + * direct answer — it ignores the interval, ignores a previous skip, and reports + * "you are up to date" and failures out loud. Collapsing the two is how update + * checkers end up either nagging or appearing broken. + */ +export type UpdateTrigger = "startup" | "manual"; + +/** The persisted half of the policy's input. */ +export interface UpdateState { + /** Whether automatic startup checks are on. */ + enabled: boolean; + /** Epoch ms of the last completed check, or null if never. */ + lastCheckedAt: number | null; + /** A version the user dismissed, which startup will not raise again. */ + skippedVersion: string | null; +} + +export type CheckDecision = + | { kind: "check" } + | { kind: "skip"; reason: "disabled" | "too-soon" }; + +/** + * Decide whether to ask the network at all. + * + * Note the clock handling: a `lastCheckedAt` in the *future* counts as due. A + * timezone change or a corrected system clock can otherwise park that timestamp + * years ahead and silently disable update checks forever — the failure mode is + * invisible, so it fails open instead. + */ +export function decideCheck( + trigger: UpdateTrigger, + state: UpdateState, + now: number, + intervalMs: number = CHECK_INTERVAL_MS, +): CheckDecision { + if (trigger === "manual") return { kind: "check" }; + if (!state.enabled) return { kind: "skip", reason: "disabled" }; + if (state.lastCheckedAt === null) return { kind: "check" }; + const elapsed = now - state.lastCheckedAt; + if (elapsed < 0) return { kind: "check" }; // clock moved back — see above + return elapsed >= intervalMs ? { kind: "check" } : { kind: "skip", reason: "too-soon" }; +} + +/** What the network said. `null` from the plugin means "no update available". */ +export interface UpdateFound { + version: string; + /** Release notes, when the manifest carries them. */ + notes?: string; +} + +export type Presentation = + | { kind: "offer"; version: string; notes?: string } + | { kind: "up-to-date" } + | { kind: "error"; message: string } + | { kind: "silent" }; + +/** + * Decide what the user sees once the check returns. + * + * The asymmetry is the whole point. A startup check that finds nothing, or + * fails, says nothing — a failed background request is Toril's problem, not the + * writer's, and an editor that pops "could not reach the update server" over a + * paragraph is worse than one that quietly tries again tomorrow. A manual check + * reports every outcome, because silence in answer to a direct question reads as + * a broken button. + */ +export function decidePresentation( + trigger: UpdateTrigger, + result: UpdateFound | null, + state: UpdateState, +): Presentation { + if (result === null) { + return trigger === "manual" ? { kind: "up-to-date" } : { kind: "silent" }; + } + // A version the user already dismissed stays dismissed for automatic checks + // only: skipping is "stop telling me", not "never let me have it". + if (trigger === "startup" && result.version === state.skippedVersion) { + return { kind: "silent" }; + } + return { kind: "offer", version: result.version, notes: result.notes }; +} + +/** + * Presentation for a check that threw. + * + * Split from {@link decidePresentation} rather than folded in as a third result + * shape, because the caller reaches it from a `catch` and there is no + * {@link UpdateFound} to pass. Same asymmetry: loud when asked, silent when not. + */ +export function decideErrorPresentation( + trigger: UpdateTrigger, + message: string, +): Presentation { + return trigger === "manual" ? { kind: "error", message } : { kind: "silent" }; +} diff --git a/tests/update.test.ts b/tests/update.test.ts new file mode 100644 index 0000000..5df30f8 --- /dev/null +++ b/tests/update.test.ts @@ -0,0 +1,140 @@ +// GATE for the update-check policy (ROADMAP Movement I.5, `feat/release-readiness`). +// +// `v1.0.0` shipped with no update path, so this is the branch that gives an +// installed copy a way forward. The rules that matter are not "does an HTTP call +// succeed" — that needs a release server — but *when Toril asks* and *when it is +// willing to interrupt someone who is writing*. Both are pure, so both are +// pinned here rather than discovered on a device a day later. +// +// The through-line: a startup check is Toril's own idea and must stay quiet; a +// manual check is the user asking a question and always gets an answer. +import { describe, expect, it } from "vitest"; +import { + CHECK_INTERVAL_MS, + decideCheck, + decideErrorPresentation, + decidePresentation, + type UpdateState, +} from "../src/update"; + +const NOW = 1_700_000_000_000; + +function state(over: Partial = {}): UpdateState { + return { enabled: true, lastCheckedAt: null, skippedVersion: null, ...over }; +} + +describe("decideCheck", () => { + it("checks on first run, when there is no previous check to rate-limit against", () => { + expect(decideCheck("startup", state(), NOW)).toEqual({ kind: "check" }); + }); + + it("does not check at startup when automatic checks are off", () => { + expect(decideCheck("startup", state({ enabled: false }), NOW)).toEqual({ + kind: "skip", + reason: "disabled", + }); + }); + + it("rate-limits startup to once per interval", () => { + const recent = state({ lastCheckedAt: NOW - 1000 }); + expect(decideCheck("startup", recent, NOW)).toEqual({ kind: "skip", reason: "too-soon" }); + }); + + it("checks again once the interval has elapsed", () => { + const stale = state({ lastCheckedAt: NOW - CHECK_INTERVAL_MS }); + expect(decideCheck("startup", stale, NOW)).toEqual({ kind: "check" }); + }); + + // The boundary itself, not just either side of it: an off-by-one here means + // either a double check or a day's silence, and neither is visible in use. + it("treats exactly one interval as due, and a millisecond less as not", () => { + expect(decideCheck("startup", state({ lastCheckedAt: NOW - CHECK_INTERVAL_MS }), NOW)).toEqual({ + kind: "check", + }); + expect( + decideCheck("startup", state({ lastCheckedAt: NOW - CHECK_INTERVAL_MS + 1 }), NOW), + ).toEqual({ kind: "skip", reason: "too-soon" }); + }); + + // A corrected system clock or a timezone jump can park the stored timestamp in + // the future. Failing closed there disables update checks permanently, with no + // symptom the user could ever notice. + it("fails open when the clock has moved backwards", () => { + const future = state({ lastCheckedAt: NOW + 30 * CHECK_INTERVAL_MS }); + expect(decideCheck("startup", future, NOW)).toEqual({ kind: "check" }); + }); + + it("always checks when the user asks directly, whatever the state says", () => { + const hostile = state({ + enabled: false, + lastCheckedAt: NOW, + skippedVersion: "9.9.9", + }); + expect(decideCheck("manual", hostile, NOW)).toEqual({ kind: "check" }); + }); +}); + +describe("decidePresentation", () => { + it("offers an update found at startup", () => { + expect(decidePresentation("startup", { version: "1.1.0" }, state())).toEqual({ + kind: "offer", + version: "1.1.0", + }); + }); + + it("carries release notes through when the manifest has them", () => { + const found = { version: "1.1.0", notes: "Fixes a thing" }; + expect(decidePresentation("manual", found, state())).toEqual({ + kind: "offer", + version: "1.1.0", + notes: "Fixes a thing", + }); + }); + + it("says nothing at startup when there is no update", () => { + expect(decidePresentation("startup", null, state())).toEqual({ kind: "silent" }); + }); + + it("says so out loud when the user asked and there is no update", () => { + expect(decidePresentation("manual", null, state())).toEqual({ kind: "up-to-date" }); + }); + + it("does not raise a version the user dismissed", () => { + const dismissed = state({ skippedVersion: "1.1.0" }); + expect(decidePresentation("startup", { version: "1.1.0" }, dismissed)).toEqual({ + kind: "silent", + }); + }); + + // Skipping one version must not opt out of the next one — otherwise a single + // "not now" quietly turns automatic updates off for good. + it("still raises a newer version after an earlier one was dismissed", () => { + const dismissed = state({ skippedVersion: "1.1.0" }); + expect(decidePresentation("startup", { version: "1.2.0" }, dismissed)).toEqual({ + kind: "offer", + version: "1.2.0", + }); + }); + + // "Stop telling me" is not "never let me have it". + it("shows a dismissed version again when the user asks directly", () => { + const dismissed = state({ skippedVersion: "1.1.0" }); + expect(decidePresentation("manual", { version: "1.1.0" }, dismissed)).toEqual({ + kind: "offer", + version: "1.1.0", + }); + }); +}); + +describe("decideErrorPresentation", () => { + it("swallows a background failure", () => { + expect(decideErrorPresentation("startup", "offline")).toEqual({ kind: "silent" }); + }); + + it("reports a failure the user is waiting on", () => { + expect(decideErrorPresentation("manual", "offline")).toEqual({ + kind: "error", + message: "offline", + }); + }); +}); From 8f601cbc9452bbcb73cf052fb952fc06d14f14ae Mon Sep 17 00:00:00 2001 From: Evan Gress <106449014+evangress@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:02:27 -0400 Subject: [PATCH 2/4] feat(qol): zoom, recent files, link opening, drops, and a real first run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second half of release-readiness. Five small features, but three of them are decisions about untrusted input rather than conveniences, so each has a gate. Editor zoom scales the writing surface and deliberately not the chrome — the OS already scales the whole UI, and a bigger tab bar is not what a tired writer wants. That means editor.css heading sizes and the measure move from rem to em, so they follow the surface rather than the root. The ladder is fixed rather than "multiply by 1.1": free scaling accumulates float drift, so five steps in and five out would not land back on 100%, and "reset" would quietly stop meaning "the size I had". A persisted 0 snaps to the default rather than clamping to the nearest end, because it would otherwise render the editor unreadable *and* unfixable — every step from zero is still zero. Opening links is a §3.3 boundary, not a convenience filter, and src/links.ts is where that shows. sanitize.ts already stops a hostile href executing in the webview; this stops it executing outside, which is strictly worse because the shell is not sandboxed. So it is an allowlist of three schemes parsed through URL, not a blocklist matched against raw text — the OS folds case and strips control characters before it acts, so jAvAsCrIpT: and java\tscript: are the same link to it and a different string to us. A blocklist would also have to have heard of ms-msdt: and search-ms: in advance; an allowlist refuses them without knowing they exist. Drag-and-drop gets its own allowlist for the same reason: it is the one open path with no file-type filter in front of it, so anything on the desktop can land on the window. formatForPath is not a substitute — it answers "how do I parse this", and answering "markdown" is right for an unknown text file and wrong for a dropped binary. Recent files rebuild the native menu wholesale, since muda submenus are built rather than mutated. Items carry an index, never a path: a path is arbitrary user data, menu ids are matched as strings on the frontend, and encoding one into the other makes the mapping depend on data neither side controls. The first-run note and the empty state used to be the same two-line stub, which meant an existing user got the tour every time they closed their last tab. They are separate now, and firstRun comes from the settings file having never been written rather than from "nothing was restored". The welcome note is a round-trip fixture, because it claims in its own text that Toril does not rewrite your files — if saving it produced a diff, the first thing a new user does would contradict the paragraph they just read. That gate earned itself immediately: it caught unpadded table pipes, and `**Ctrl+\**`, where the backslash escapes its own closing marker and mangles the bold. Neither was visible by reading. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 18 +++ CLAUDE.md | 52 +++++-- ROADMAP.md | 21 ++- dev-harness.html | 3 + docs/ON-DEVICE-VERIFICATION.md | 22 +++ package.json | 1 + pnpm-lock.yaml | 10 ++ src-tauri/Cargo.lock | 53 +++++++ src-tauri/Cargo.toml | 4 + src-tauri/capabilities/default.json | 3 +- src-tauri/src/lib.rs | 2 + src-tauri/src/menu.rs | 102 ++++++++++++- src-tauri/src/settings.rs | 9 ++ src/ipc.ts | 54 +++++++ src/links.ts | 65 ++++++++ src/main.ts | 227 +++++++++++++++++++++++++++- src/paths.ts | 30 ++++ src/recent.ts | 70 +++++++++ src/styles/chrome.css | 41 +++++ src/styles/editor.css | 19 ++- src/ui/sidebar.ts | 39 ++++- src/welcome.ts | 68 +++++++++ src/zoom.ts | 64 ++++++++ tests/links.test.ts | 91 +++++++++++ tests/paths.test.ts | 45 +++++- tests/recent.test.ts | 94 ++++++++++++ tests/roundtrip.test.ts | 11 ++ tests/zoom.test.ts | 94 ++++++++++++ 28 files changed, 1274 insertions(+), 38 deletions(-) create mode 100644 src/links.ts create mode 100644 src/recent.ts create mode 100644 src/welcome.ts create mode 100644 src/zoom.ts create mode 100644 tests/links.test.ts create mode 100644 tests/recent.test.ts create mode 100644 tests/zoom.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0910d52..4406952 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,24 @@ GitHub Release notes plus the commits that shipped in it. - **The window remembers where it was.** Size, position and maximized state come back the way you left them. +- **Zoom the writing surface** with `Ctrl` and `+` / `-` / `0`. It scales the text and + the measure, not the tab bar — your display scaling already handles the whole UI, and + a bigger tab bar is not what anyone wants at 11pm. + +- **File → Open Recent** lists the last ten notes you opened. An entry that no longer + resolves removes itself rather than failing twice. + +- **Ctrl-click a link** to open it in your browser. Only web and email links are handed + to the system — a note can come from anywhere, and the rest of what a URL can name is + not something an editor should hand to your operating system on a click. + +- **Drop notes on the window to open them.** `.md`, `.markdown`, `.html` and `.htm`; + anything else in the same drop is skipped and counted. + +- **A real welcome note on first run**, and a blank page on every later launch with + nothing to restore — the two used to be the same two-line stub. With no folder open, + the files pane now offers to open one instead of only saying that none is. + ### Notes - Updates are cryptographically signed, and an installed Toril refuses one that does not verify. Setting that up is a one-time step for whoever cuts releases — see diff --git a/CLAUDE.md b/CLAUDE.md index 86caf2f..ca6c3a0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -132,10 +132,12 @@ secret. Until then `plugins.updater.pubkey` is empty, so a check degrades to a r error rather than a crash, and `release.yml` refuses to cut a tag with a one-line explanation instead of failing deep in a Rust build. -**Next:** finish Phase 4 — remaining is the QoL half of `feat/release-readiness` (editor -zoom, recent-files MRU, open-links-in-browser, drag-drop open, first-run empty state), -Azure Trusted Signing once an account exists, and on-device verification. A backlog of -further QoL features is in §13. The +The **QoL half of the same branch** has also landed: editor zoom, open-links-in-browser, +drag-and-drop open, a recent-files list in File → Open Recent, and a real first-run +welcome note distinct from the empty state. See §5 (Quality-of-life batch). + +**Next:** finish Phase 4 — remaining is Azure Trusted Signing once an account exists, and +on-device verification. A backlog of further QoL features is in §13. The **forward plan beyond Phase 4** — turning the editor into a notes *system* (search, links, version history, sync coexistence, the AI wedge) branch-by-branch, with per-stage publicity guidance — lives in **`ROADMAP.md`**. @@ -356,6 +358,7 @@ frontend never touches the filesystem directly; it asks via `invoke()`. | `restore_snapshot` | `path, hash` | `()` | snapshots current on-disk content **first**, then atomically writes the chosen version — restore is undoable (§3) | | `merge_external` | `path, base, mine` | `{ outcome, content?, theirs? }` | Reads the file and 3-way merges via `mergemd`. **Never writes.** `outcome` is one of `unchanged` / `theirsOnly` / `merged` / `conflict` / `missing` — `missing` is a deleted file (`io::ErrorKind::NotFound`), distinct from an unreadable one, so a gone file can be recreated by an explicit save rather than blocked forever. `content` is set only for `merged`; `theirs` (the bytes now on disk) is set for every outcome **except `unchanged` and `missing`** — nothing to park in either case — so the caller can set its new merge base and park the losing side without a second read that would race the writer (ROADMAP I.4) | | `write_conflict_copy` | `path, content` | `conflict_path` | Parks the losing side as `note (conflict 2026-07-25 14-32-05).md` beside the original — **atomic** via `fsatomic`, and `-2`/`-3`… suffixed rather than overwritten on a timestamp collision (§3) | +| `set_recent_files` | `paths` | `()` | Rebuild the native menu so File → Open Recent lists `paths`. The **whole** menu is replaced — muda submenus are built, not mutated — so this is called only when the list changes. Items carry an **index** (`menu_recent_3`), never a path: a path is arbitrary user data and menu ids are matched as strings, so the frontend resolves the index against its own list (`src/recent.ts`) | | `take_launch_path` | — | `path?` | file the app was launched with (double-click / "Open with"); returns it **once**, then `null` (§file-open) | | `set_api_key` | `provider, key` | `()` | Validate and store an API key in the **OS keychain** (`keystore`) — Credential Manager / Keychain / Secret Service. Replaces any existing key for that provider (ROADMAP IV.20) | | `clear_api_key` | `provider` | `()` | Remove a stored key. **Idempotent** — clearing an absent key succeeds, so pressing Clear twice is not an error | @@ -442,6 +445,24 @@ frontend never touches the filesystem directly; it asks via `invoke()`. > or a local `pnpm tauri build` must not need an Azure subscription to produce a working > installer. > +> **Quality-of-life batch (`feat/release-readiness`, second half).** Editor zoom +> (`src/zoom.ts` — a **fixed ladder**, not `× 1.1`, so five steps in and five out land +> exactly back on 100% and a persisted value can't drift; a stored `0` snaps to the +> default rather than being clamped, because it would otherwise render the editor +> unreadable *and* unfixable). Zoom scales the writing surface only, never the chrome — +> so `editor.css` heading sizes and the measure are `em`, not `rem`. **Open links in the +> browser** on Ctrl/Cmd-click, gated by `src/links.ts`: a **three-scheme allowlist** +> (http/https/mailto) parsed via `URL`, not string-matched, because the OS normalizes +> case and control characters before acting. This is a §3.3 boundary, not a convenience +> filter — `sanitize.ts` stops a hostile link executing *in* the webview, this stops it +> executing *outside* it, which is strictly worse since the shell is not sandboxed. +> **Drag-and-drop open** via Tauri's native drag-drop (HTML5 drops carry no real paths), +> filtered by `paths.selectOpenable` — an allowlist, because a drop is the one open path +> with no file-type filter in front of it. **Recent files** (`src/recent.ts` + +> `set_recent_files`). **First-run vs. empty state**: `src/welcome.ts` holds both, and +> `firstRun` comes from `settings.version === 0` (never written) rather than "nothing +> was restored", which is also true when an existing user closes their last tab. +> > **Events (Rust → frontend):** `workspace:change` (file watcher), `menu` (native menu item id > `menu_*` → mapped to the same handlers as toolbar buttons), and `open-file` (a *second* launch's > file path, forwarded by the single-instance plugin while Toril is already running). Subscribe via @@ -582,6 +603,15 @@ Phases 0–3 are complete and Phase 4 (polish) is in progress; the shipped detai briefly narrowed. - **Action double-fire:** `tests/actions.test.ts` — `menu.rs` carries real accelerators, so one Ctrl+S arrives twice (menu *and* webview keydown). One dispatcher collapses the pair. +- **Zoom / links / recents:** `tests/zoom.test.ts` (the ladder round-trips exactly, and a + stored `0` cannot make the editor unusable), `tests/links.test.ts` (the §3.3 allowlist — + `jAvAsCrIpT:`, `java\tscript:`, `file://`, `ms-msdt:`; the cases a blocklist gets wrong), + `tests/recent.test.ts` (dedupe-and-move, no in-place mutation, junk in `session.json` + degrading to empty rather than throwing during bootstrap), and the `paths` suite's + drop allowlist. Plus the shipped **welcome note is itself a round-trip fixture** in + `tests/roundtrip.test.ts`: it claims in its own text that Toril doesn't rewrite your + files, so it has to survive a save. That gate immediately caught two real defects in + the copy (unpadded table pipes, and `**Ctrl+\**` escaping its own closing marker). - **Update policy:** `tests/update.test.ts` — the startup/manual asymmetry (a background check is rate-limited, skippable and silent; a direct question always gets an answer), the interval boundary itself rather than either side of it, and that a `lastCheckedAt` @@ -621,7 +651,7 @@ test harness — it needs a live Milkdown editor and Tauri IPC.** `crates/mergem `src/paths.ts`, and the tab bookkeeping in `src/ui/tabs.ts` are gated in isolation; the glue that calls them in the right order, at the right time, is verified on-device only. -**Remaining for Phase 4:** the QoL half of `feat/release-readiness`; Azure Trusted Signing +**Remaining for Phase 4:** Azure Trusted Signing (removes the SmartScreen warning — the wiring is in place and inert, `docs/RELEASE-SIGNING.md`); and on-device verification of GUI/Rust flows that can't be tested here, now including the update flow (§D of `docs/ON-DEVICE-VERIFICATION.md` — nothing headless can prove a signed @@ -782,16 +812,16 @@ Keep the project's rules: testable logic in `crates/*` or pure TS helpers, all d (§5/§10), one canonical serializer (§3.2), no unhealthy deps (§2). **Easy (pure frontend, fully testable here):** -- **Editor zoom** — `Ctrl +`/`-`/`0` adjusts an editor font-size CSS variable; persist in `Settings`. +- ~~**Editor zoom**~~ — *shipped* (`feat/release-readiness`). - **Spellcheck** — ensure the ProseMirror editable carries `spellcheck="true"`. Verify on-device. - **Tab niceties** — middle-click to close, "Close others / Close all". **Easy–medium (small Rust / Tauri, needs on-device verify):** -- **Auto-save** — debounced save of dirty *saved* files; reuse atomic `saveFile`; toggle in `Settings`. -- **Remember window size/position** — add the maintained `tauri-plugin-window-state` (vet per §2). -- **Recent files / recent folders** — extend the persisted session with an MRU list; surface in File menu. -- **Open links in browser** (`Ctrl/Cmd`-click) — route through Tauri's shell-open. -- **Drag-and-drop a `.md` onto the window to open it** — Tauri drag-drop event → existing `openPath`. +- ~~**Auto-save**~~ — *shipped* (Movement I.1). +- ~~**Remember window size/position**~~, ~~**Recent files**~~, ~~**Open links in browser**~~, + ~~**Drag-and-drop to open**~~ — all *shipped* (`feat/release-readiness`). **Recent + folders** was not done: reopening a folder is a heavier action than reopening a file + and the sidebar already remembers the last one. **Medium (more UI / a new command, but high value):** - **Global workspace search ("find in files")** — a Rust command scanning the vault (sibling to diff --git a/ROADMAP.md b/ROADMAP.md index 18e973e..f5bddb3 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -49,8 +49,9 @@ no AI. That gap is this roadmap. > **Status (2026-08-17).** Shipped through **`v1.0.0`** (see `CHANGELOG.md`). > **Movement I, branches 1–4 are complete** (autosave + crash-recovery journal; > safe-delete-to-trash; local version history; sync coexistence — 3-way merge, conflict -> banner, parked conflict copies). **Branch 5's update half has landed** on -> `feat/release-readiness`; its QoL half and Azure signing remain. From Movement II, +> banner, parked conflict copies). **Branch 5 has landed** on `feat/release-readiness` — +> self-update, window state and the QoL batch — leaving only Azure Trusted Signing, which +> is blocked on provisioning an account rather than on code. From Movement II, > **branch 11 (outline panel) and branch 10 (front-matter properties)** have landed — 10 > out of order, because front matter was being *corrupted* rather than merely unsupported, > which made it a §3 fix rather than a convenience. @@ -62,8 +63,9 @@ no AI. That gap is this roadmap. > after it, so the next feature could not have reached anyone. §7's *trust before reach* > is what settles the order. Read the pointer below as the ordering, not the ladder. > -> **▶ Pick up at branch 5's QoL half, then Movement II, branch 6 — -> `feat/vault-search`.** Search is the largest remaining functional gap, and branch 7 +> **▶ Pick up at Movement II, branch 6 — `feat/vault-search`.** (Branch 5 is done bar +> Azure signing, which is blocked on an account, not on code.) +> Search is the largest remaining functional gap, and branch 7 > (command palette) depends on it. Vet `tantivy` per §2 at > adoption — it would be the project's largest new dependency. Branch 4's spec lived > **on its own branch**, not on `main`: @@ -221,9 +223,14 @@ nice in a synced folder. This movement is also the prerequisite for the AI wedge - **Blocked on one manual step:** generate the minisign keypair, paste the public half into `plugins.updater.pubkey`, add the private half as a repo secret. Until then the updater is present but keyless. - - [ ] *QoL half (not started).* Editor zoom (`Ctrl +/-/0`); recent-files MRU; - open-links-in-browser; drag-drop `.md` to open; a real **first-run / empty-state** - (welcome note + "open a folder"). + - [x] *QoL half (2026-08-17).* Editor zoom (`Ctrl +/-/0`, a fixed ladder so it cannot + drift — `src/zoom.ts`); recent-files MRU in File → Open Recent (`src/recent.ts` + + `set_recent_files`, which rebuilds the native menu); open-links-in-browser on + Ctrl-click behind a three-scheme allowlist (`src/links.ts` — a §3.3 boundary, since + the OS shell is not sandboxed the way the webview is); drag-drop to open, filtered + by `paths.selectOpenable`; and a real **first-run welcome note** distinct from the + empty state (`src/welcome.ts`), which is itself a round-trip fixture — it claims + Toril doesn't rewrite your files, so it has to survive its own first save. - [ ] *Azure Trusted Signing.* Wiring done (`tauri.signing.conf.json` overlay, applied in CI only when `AZURE_*` secrets exist, so a fork still builds). Needs an account and an identity validation that takes business days — then update the placeholder account diff --git a/dev-harness.html b/dev-harness.html index a87a64e..672ba81 100644 --- a/dev-harness.html +++ b/dev-harness.html @@ -125,6 +125,9 @@ : { available: false }, "plugin:updater|download_and_install": () => null, "plugin:process|restart": () => null, + "plugin:opener|open_url": () => null, + // Rebuilds the native menu, which the harness does not have. + set_recent_files: () => null, load_settings: () => SETTINGS, save_settings: () => null, load_recovery: () => [], diff --git a/docs/ON-DEVICE-VERIFICATION.md b/docs/ON-DEVICE-VERIFICATION.md index 8bad2a4..8bae349 100644 --- a/docs/ON-DEVICE-VERIFICATION.md +++ b/docs/ON-DEVICE-VERIFICATION.md @@ -188,6 +188,28 @@ that a signed artifact downloads, verifies and replaces a running binary. - [ ] **D6 — Window state.** Move and resize the window, quit, relaunch: size, position and maximized state should return. Then relaunch on a machine with **fewer or smaller monitors** and confirm the window is not restored off-screen. +- [ ] **D8 — Zoom, and the two shortcuts that usually don't work.** `Ctrl+-` and `Ctrl+0` + are unambiguous; `Ctrl+Plus` is not — check `Ctrl+Shift+=`, `Ctrl+=` and the numeric + keypad's `+` all zoom in, in WebView2 *and* through the native accelerator. Confirm + the chrome does **not** scale, and that the level survives a restart. +- [ ] **D9 — Open Recent.** Open several notes, confirm the File submenu lists them + newest-first by file name and that reopening one moves it rather than duplicating + it. Then delete a listed file on disk and pick it: it must report the failure and + remove itself. The menu is rebuilt wholesale from Rust — watch for flicker or a + lost menu on Linux. +- [ ] **D10 — Link opening, including the refusals.** Ctrl-click an `https://` link: it + opens in the default browser, and Toril does not navigate. Then author a note + containing `file:///C:/Windows/System32/cmd.exe` and `javascript:alert(1)` and + Ctrl-click both — **nothing must launch**, and the status bar should say it was + refused. `tests/links.test.ts` gates the rule; only a device proves the rule is + the thing actually consulted. +- [ ] **D11 — Drag and drop.** Drop a `.md`, a folder, and a mixed selection including a + `.png`. Notes open, the rest is skipped and counted. Native drag-drop is a + per-platform path with no headless coverage at all. +- [ ] **D12 — First run really is first.** With no `session.json`, launch: the welcome + note appears. Save it somewhere and diff — it must be **byte-identical**, which is + the claim its own second paragraph makes. Then quit and relaunch: a returning user + gets a blank page, not the tour again. - [ ] **D7 — The check is quiet when it should be.** With no network, launch: nothing appears. Ask via Help → Check for Updates: it says it could not check. That asymmetry is the whole design and is easy to regress. diff --git a/package.json b/package.json index 278ef87..52e7be3 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ "@milkdown/theme-nord": "7.21.1", "@tauri-apps/api": "2.11.0", "@tauri-apps/plugin-dialog": "2.7.1", + "@tauri-apps/plugin-opener": "2.5.4", "@tauri-apps/plugin-process": "2.3.1", "@tauri-apps/plugin-updater": "2.10.1", "@tauri-apps/plugin-window-state": "2.4.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index db299be..43f3831 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -31,6 +31,9 @@ importers: '@tauri-apps/plugin-dialog': specifier: 2.7.1 version: 2.7.1 + '@tauri-apps/plugin-opener': + specifier: 2.5.4 + version: 2.5.4 '@tauri-apps/plugin-process': specifier: 2.3.1 version: 2.3.1 @@ -644,6 +647,9 @@ packages: '@tauri-apps/plugin-dialog@2.7.1': resolution: {integrity: sha512-OK1UBXYt+ojcmxMktzzuyonYIFta8CmAASpX+CA+DTGK24KlHjhYI6x2iOJ/TjZF4N7/ACK1oFmEOjIY9IhzOQ==} + '@tauri-apps/plugin-opener@2.5.4': + resolution: {integrity: sha512-1HnPkb+AmgO29HBazm4uPLKB+r7zzcTBW1d0fyYp1uP+jwtpoiNDGKMMzz58SFp49nOIrxdE3aUJtT57lfO9CQ==} + '@tauri-apps/plugin-process@2.3.1': resolution: {integrity: sha512-nCa4fGVaDL/B9ai03VyPOjfAHRHSBz5v6F/ObsB73r/dA3MHHhZtldaDMIc0V/pnUw9ehzr2iEG+XkSEyC0JJA==} @@ -1999,6 +2005,10 @@ snapshots: dependencies: '@tauri-apps/api': 2.11.0 + '@tauri-apps/plugin-opener@2.5.4': + dependencies: + '@tauri-apps/api': 2.11.0 + '@tauri-apps/plugin-process@2.3.1': dependencies: '@tauri-apps/api': 2.11.0 diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index a20dfc0..083b48c 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -2090,6 +2090,25 @@ version = "2.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d98f6fed1fde3f8c21bc40a1abb88dd75e67924f9cffc3ef95607bad8017f8e2" +[[package]] +name = "is-docker" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "928bae27f42bc99b60d9ac7334e3a21d10ad8f1835a4e12ec3ec0464765ed1b3" +dependencies = [ + "once_cell", +] + +[[package]] +name = "is-wsl" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "173609498df190136aa7dea1a91db051746d339e18476eed5ca40521f02d7aa5" +dependencies = [ + "is-docker", + "once_cell", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -2897,6 +2916,17 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "open" +version = "5.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f9cfef937e9c486488c7e3d949ae31c0f1d06bdacd75b99c086cb35356e30408" +dependencies = [ + "dunce", + "is-wsl", + "libc", +] + [[package]] name = "openssl-probe" version = "0.2.1" @@ -4324,6 +4354,28 @@ dependencies = [ "url", ] +[[package]] +name = "tauri-plugin-opener" +version = "2.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "17e1bea14edce6b793a04e2417e3fd924b9bc4faae83cdee7d714156cceeed29" +dependencies = [ + "dunce", + "glob", + "objc2-app-kit", + "objc2-foundation", + "open", + "schemars 0.8.22", + "serde", + "serde_json", + "tauri", + "tauri-plugin", + "thiserror 2.0.18", + "url", + "windows", + "zbus", +] + [[package]] name = "tauri-plugin-process" version = "2.3.1" @@ -4800,6 +4852,7 @@ dependencies = [ "tauri", "tauri-build", "tauri-plugin-dialog", + "tauri-plugin-opener", "tauri-plugin-process", "tauri-plugin-single-instance", "tauri-plugin-updater", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 58a842e..73e4263 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -49,6 +49,10 @@ tauri-plugin-single-instance = "2" # Official Tauri plugin (§2). Purely presentational — it persists to its own # file in the app config dir and touches nothing in the vault. tauri-plugin-window-state = "2" +# Hand a link to the OS browser. Official Tauri plugin (§2). **Which** links are +# handed over is decided in `src/links.ts`, not here — an opened `.md` is +# untrusted (§3.3), so only http/https/mailto ever reach this. +tauri-plugin-opener = "2" serde = { version = "1", features = ["derive"] } serde_json = "1" notify = "8" diff --git a/src-tauri/capabilities/default.json b/src-tauri/capabilities/default.json index 3c24d22..ac05935 100644 --- a/src-tauri/capabilities/default.json +++ b/src-tauri/capabilities/default.json @@ -8,6 +8,7 @@ "core:window:allow-destroy", "dialog:default", "updater:default", - "process:allow-restart" + "process:allow-restart", + "opener:allow-open-url" ] } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 5827135..12851c1 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -64,6 +64,7 @@ pub fn run() { // Remember where the window was and how big it was. Presentational // only — it writes its own file in the app config dir, never the vault. .plugin(tauri_plugin_window_state::Builder::default().build()) + .plugin(tauri_plugin_opener::init()) .menu(menu::build) .on_menu_event(menu::on_event) .manage(launch_path) @@ -97,6 +98,7 @@ pub fn run() { commands::secrets::clear_api_key, commands::secrets::has_api_key, commands::secrets::list_api_keys, + menu::set_recent_files, take_launch_path, ]) .run(tauri::generate_context!()) diff --git a/src-tauri/src/menu.rs b/src-tauri/src/menu.rs index b79e281..888b573 100644 --- a/src-tauri/src/menu.rs +++ b/src-tauri/src/menu.rs @@ -36,8 +36,42 @@ use tauri::menu::{Menu, MenuBuilder, MenuEvent, MenuItemBuilder, SubmenuBuilder}; use tauri::{AppHandle, Emitter, Runtime}; -/// Build the application menu (File / Edit / View / Help). +/// Build the application menu with an empty recent-files list. +/// +/// The startup entry point: nothing has been opened yet, and the frontend +/// replaces the menu via [`set_recent_files`] once it has restored the list. pub fn build(app: &AppHandle) -> tauri::Result> { + build_with_recent(app, &[]) +} + +/// Replace the application menu so File → Open Recent lists `paths`. +/// +/// A whole-menu rebuild, because muda submenus are built rather than mutated — +/// there is no "replace these items" on a live menu. It is cheap (a dozen items) +/// and only happens when a file is opened or closed. +/// +/// Items are identified by **index** (`menu_recent_3`), not by path. A path is +/// arbitrary user data — it can contain any character the filesystem allows — +/// and menu ids are matched as strings on the frontend, so encoding one into an +/// id makes the mapping depend on data neither side controls. The frontend holds +/// the authoritative list and resolves the index against it. +#[tauri::command] +pub fn set_recent_files(app: AppHandle, paths: Vec) -> Result<(), String> { + let menu = build_with_recent(&app, &paths).map_err(|e| e.to_string())?; + app.set_menu(menu).map_err(|e| e.to_string())?; + Ok(()) +} + +/// The display name for a recent entry: the file name, not the whole path. +/// +/// A menu is a narrow column and an absolute path is mostly directories the +/// user already knows. The full path goes nowhere here — the frontend's own +/// surfaces show it where there is room. +fn recent_label(path: &str) -> &str { + path.rsplit(['/', '\\']).next().unwrap_or(path) +} + +fn build_with_recent(app: &AppHandle, recent: &[String]) -> tauri::Result> { // `CmdOrCtrl` maps to Ctrl on Windows/Linux and Cmd on macOS, so one string // is correct everywhere. let new = MenuItemBuilder::with_id("menu_new", "&New") @@ -63,9 +97,34 @@ pub fn build(app: &AppHandle) -> tauri::Result> { .build(app)?; let export_rtf = MenuItemBuilder::with_id("menu_export_rtf", "Export &RTF…").build(app)?; + // Open Recent. Built even when empty, carrying a single disabled "No recent + // files" row: a submenu that vanishes on a fresh install teaches the user it + // is not there, and a disabled row that explains itself is a smaller + // surprise than a menu whose shape changes. + let mut recent_menu = SubmenuBuilder::new(app, "Open &Recent"); + if recent.is_empty() { + recent_menu = recent_menu.item( + &MenuItemBuilder::with_id("menu_recent_none", "No recent files") + .enabled(false) + .build(app)?, + ); + } else { + for (i, path) in recent.iter().enumerate() { + recent_menu = recent_menu.item( + &MenuItemBuilder::with_id(format!("menu_recent_{i}"), recent_label(path)) + .build(app)?, + ); + } + recent_menu = recent_menu.separator().item( + &MenuItemBuilder::with_id("menu_recent_clear", "&Clear Recent Files").build(app)?, + ); + } + let recent_submenu = recent_menu.build()?; + let file = SubmenuBuilder::new(app, "&File") .item(&new) .item(&open) + .item(&recent_submenu) .item(&open_folder) .separator() .item(&save) @@ -106,6 +165,18 @@ pub fn build(app: &AppHandle) -> tauri::Result> { .build(app)?; let toggle_history = MenuItemBuilder::with_id("menu_toggle_history", "Version &History").build(app)?; + // Zoom scales the writing surface, not the chrome — the OS already scales + // the whole UI, and a bigger tab bar is not what a tired writer wants. + let zoom_in = MenuItemBuilder::with_id("menu_zoom_in", "Zoom &In") + .accelerator("CmdOrCtrl+Plus") + .build(app)?; + let zoom_out = MenuItemBuilder::with_id("menu_zoom_out", "Zoom O&ut") + .accelerator("CmdOrCtrl+-") + .build(app)?; + let zoom_reset = MenuItemBuilder::with_id("menu_zoom_reset", "&Reset Zoom") + .accelerator("CmdOrCtrl+0") + .build(app)?; + let toggle_autosave = MenuItemBuilder::with_id("menu_toggle_autosave", "&Autosave").build(app)?; let toggle_update_check = @@ -119,6 +190,10 @@ pub fn build(app: &AppHandle) -> tauri::Result> { .item(&toggle_outline) .item(&toggle_history) .separator() + .item(&zoom_in) + .item(&zoom_out) + .item(&zoom_reset) + .separator() .item(&toggle_autosave) .item(&toggle_update_check) .build()?; @@ -145,3 +220,28 @@ pub fn on_event(app: &AppHandle, event: MenuEvent) { let _ = app.emit("menu", id); } } + +#[cfg(test)] +mod tests { + use super::recent_label; + + #[test] + fn shows_the_file_name_not_the_path() { + assert_eq!(recent_label(r"C:\Users\me\vault\todo.md"), "todo.md"); + assert_eq!(recent_label("/home/me/vault/todo.md"), "todo.md"); + } + + #[test] + fn handles_a_bare_name_and_mixed_separators() { + assert_eq!(recent_label("todo.md"), "todo.md"); + assert_eq!(recent_label("C:/vault\\sub/todo.md"), "todo.md"); + } + + /// A path that ends in a separator has no file name; returning the empty + /// string is honest, and better than panicking on data from disk. + #[test] + fn does_not_panic_on_a_trailing_separator() { + assert_eq!(recent_label("/vault/"), ""); + assert_eq!(recent_label(""), ""); + } +} diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index f1a3913..300e599 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -72,6 +72,15 @@ pub struct Settings { /// A version the user dismissed. Startup will not raise it again; an /// explicit Help → Check for Updates still will. pub update_skipped_version: Option, + /// Writing-surface zoom multiplier (chrome is unaffected). `None` ⇒ 1. + /// Deliberately not validated here — `src/zoom.ts` snaps whatever comes + /// back onto its ladder, so the rule lives in one place rather than two + /// that could disagree. + pub editor_zoom: Option, + /// Recently opened files, newest first. Paths only, never contents — the + /// same rule the rest of this file follows (§3.2). Order and length are the + /// frontend's business (`src/recent.ts`); this only stores what it is given. + pub recent_files: Vec, } fn settings_path(app: &AppHandle) -> Result { diff --git a/src/ipc.ts b/src/ipc.ts index cab0c56..76d5175 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -205,6 +205,50 @@ export async function relaunchApp(): Promise { await relaunch(); } +/** + * Rebuild the native menu so File → Open Recent lists `paths`. + * + * The whole menu is replaced (muda submenus are built, not mutated), so this is + * called only when the list actually changes. Items are identified by index — + * `menu_recent_3` — and the frontend resolves that against its own list, so an + * arbitrary path never has to survive a round trip through a menu id. + */ +export function setRecentFiles(paths: string[]): Promise { + return invoke("set_recent_files", { paths }); +} + +/** + * Files dropped onto the window. + * + * The webview's own HTML5 drop events don't carry real paths (a `File` object + * has no filesystem location), so this comes from Tauri's native drag-drop + * instead — which is also the only version that can hand the paths to the Rust + * side, where all disk access lives (§10). Callers filter with + * `paths.selectOpenable` before doing anything with them. + */ +export async function onFilesDropped( + handler: (paths: string[]) => void, +): Promise { + const { getCurrentWebview } = await import("@tauri-apps/api/webview"); + return getCurrentWebview().onDragDropEvent((event) => { + if (event.payload.type === "drop") handler(event.payload.paths); + }); +} + +/** + * Hand a URL to the OS default browser. + * + * **Callers must have checked it first.** `src/links.ts` owns that decision and + * allows only http/https/mailto; this wrapper deliberately does no validation + * of its own, so there is exactly one place the rule lives and no second, + * quietly-weaker copy of it. An opened `.md` is untrusted (§3.3), and the shell + * acts on far more than web addresses. + */ +export async function openExternal(url: string): Promise { + const { openUrl } = await import("@tauri-apps/plugin-opener"); + await openUrl(url); +} + /** Persisted session + preferences (mirrors Rust `settings::Settings`, §5). */ export interface Settings { version: number; @@ -243,6 +287,16 @@ export interface Settings { update_last_checked: number | null; /** A version the user dismissed; startup will not raise it again. */ update_skipped_version: string | null; + /** Writing-surface zoom multiplier. `null` ⇒ 1. Normalized on load. */ + editor_zoom: number | null; + /** + * Recently opened files, newest first. Paths only, never contents (§3.2). + * Typed as `unknown` deliberately: this is the one settings field with + * unbounded shape, and `recent.normalizeRecent` is what turns it into a list. + * Declaring it `string[]` here would be a claim about a file on disk that + * nothing checks. + */ + recent_files: unknown; } /** Load persisted settings; resolves to defaults if none exist or the file is corrupt. */ diff --git a/src/links.ts b/src/links.ts new file mode 100644 index 0000000..188f3de --- /dev/null +++ b/src/links.ts @@ -0,0 +1,65 @@ +// Which links Toril is willing to hand to the operating system. +// +// This is a **security boundary**, not a convenience filter, and it is the +// reason opening links is a module with a gate rather than a one-line handler. +// §3.3 says an opened `.md` file is untrusted: its content can come from a +// shared vault, a sync folder, a downloaded note, or an AI assistant's output. +// Handing a href from that document to the OS shell is handing an attacker a +// primitive — the shell will happily act on far more than a web address. +// +// `sanitize.ts` already stops such a link from *executing in the webview*. +// This stops it from executing *outside* the webview, which is the strictly +// larger risk: the webview is sandboxed, the shell is not. +// +// So the rule is an **allowlist of three schemes**, not a blocklist. A +// blocklist has to anticipate every dangerous scheme the host registers — +// `file:`, `javascript:`, `vbscript:`, `data:`, `smb:`, `ms-msdt:`, `search-ms:`, +// and whatever an installed application added last week. An allowlist only has +// to know the three we actually mean, and everything invented afterwards is +// refused by default. + +/** The only schemes Toril will open externally. */ +const ALLOWED = new Set(["http:", "https:", "mailto:"]); + +/** + * Whether `href` may be handed to the OS. + * + * Parsing rather than string-matching is deliberate: `URL` performs the same + * normalization the OS will (case folding, whitespace and control-character + * stripping, percent-decoding of the scheme), so a check written against the + * raw text can be made to disagree with what actually gets opened — + * `jAvAsCrIpT:`, `java\tscript:` and a leading newline are the classic three. + * Anything `URL` cannot parse is refused, which also disposes of relative and + * malformed hrefs. + */ +export function isExternallyOpenable(href: string): boolean { + let url: URL; + try { + url = new URL(href); + } catch { + return false; + } + return ALLOWED.has(url.protocol); +} + +/** + * Resolve the click target to a link href, or null if the click was not on one. + * + * Walks ancestors because a click usually lands on a text node or an inline + * element *inside* the anchor — `text` is ordinary + * markdown, and matching only `target.tagName === "A"` misses it. + * + * Separated from the DOM handler so both halves are testable: the traversal + * runs against a jsdom fragment and the scheme rule runs against strings. + */ +export function linkHrefFrom(target: EventTarget | null): string | null { + let node = target instanceof Element ? target : null; + while (node) { + if (node.tagName === "A") { + const href = node.getAttribute("href"); + return href && href.length > 0 ? href : null; + } + node = node.parentElement; + } + return null; +} diff --git a/src/main.ts b/src/main.ts index 432385a..1bff961 100644 --- a/src/main.ts +++ b/src/main.ts @@ -29,9 +29,11 @@ import { type MergeReport, mergeExternal, onMenuAction, + onFilesDropped, onOpenFile, onWorkspaceChange, openFile, + openExternal, openFolder, pickFileToOpen, pickFolder, @@ -43,6 +45,7 @@ import { saveFileAs, saveRecovery, saveSettings, + setRecentFiles, showAbout, takeLaunchPath, watchFolder, @@ -50,7 +53,8 @@ import { } from "./ipc"; import { ActionDispatcher } from "./actions"; import { AutosaveScheduler, type RecoveryEntry, selectDirtySaved, snapshotDirty } from "./autosave"; -import { isAtOrUnder } from "./paths"; +import { isExternallyOpenable, linkHrefFrom } from "./links"; +import { isAtOrUnder, selectOpenable } from "./paths"; import { blocksWrite, decideAction, @@ -66,6 +70,8 @@ import { type UpdateState, type UpdateTrigger, } from "./update"; +import { ZOOM_DEFAULT, formatZoom, normalizeZoom, zoomIn, zoomOut } from "./zoom"; +import { RECENT_LIMIT, forgetRecent, normalizeRecent, pushRecent } from "./recent"; import { ConflictBar } from "./ui/conflictbar"; import { UpdateNotice } from "./ui/updatenotice"; import { PropertiesStrip } from "./ui/properties"; @@ -95,10 +101,7 @@ import { type TabState, type DocFormat, TabManager } from "./ui/tabs"; import { ThemeController, isTheme } from "./ui/theme"; import { FormattingToolbar } from "./ui/toolbar"; -const WELCOME = `# Welcome to Toril - -Open a folder to browse your notes, or start typing here. -`; +import { EMPTY, FIRST_RUN } from "./welcome"; let editor: Editor; let tabs: TabManager; @@ -128,6 +131,22 @@ const updateState: UpdateState = { lastCheckedAt: null, skippedVersion: null, }; +/** Writing-surface zoom multiplier; chrome is deliberately unaffected (`zoom.ts`). */ +let editorZoom = ZOOM_DEFAULT; +/** Recently opened files, newest first. Paths only, never contents (§3.2). */ +let recentFiles: string[] = []; +/** + * Whether this is the very first launch, which decides between the welcome + * document and a blank one. + * + * Derived from the settings file having never been written (`version` is 0 for + * `Settings::default()`, and `save_settings` always stamps the current version) + * — not from "nothing was restored", which is also true whenever an existing + * user closes their last tab. Defaults to `false` so a settings load that + * *fails* shows a returning user a blank page rather than the tour: the tour is + * the more annoying of the two mistakes. + */ +let firstRun = false; let workspaceRoot: string | null = null; let panes: PaneState = defaultPaneState(); @@ -302,10 +321,17 @@ async function openPath(path: string): Promise { if (existing) { tabs.setActive(existing.id); updateTitle(); + // Focusing an already-open tab still counts as reaching for the file, so it + // moves to the front of the recent list. + rememberRecent(existing.path ?? path); return; } const file = await openFile(path); openDocument(file.path, basename(file.path), file.content, formatForPath(file.path)); + // Recorded from the path the backend resolved, and only once the read + // succeeded — a file that failed to open must not enter a list whose whole + // purpose is offering it again. + rememberRecent(file.path); setStatus(`Opened ${basename(file.path)}`); } @@ -730,6 +756,120 @@ function toggleAutosave(): void { if (autosaveEnabled) autosave?.notifyChange(); } +// ---- Recent files ---------------------------------------------------------- + +/** + * Push the native menu the list it should show. + * + * Best-effort: a menu that failed to rebuild is a cosmetic problem, and letting + * it reject would turn opening a file into a visible error over nothing. + */ +function syncRecentMenu(): void { + void setRecentFiles(recentFiles).catch(() => {}); +} + +/** Record a successfully opened file. Called only after the read succeeded. */ +function rememberRecent(path: string): void { + const next = pushRecent(recentFiles, path, RECENT_LIMIT); + // Reopening the file already at the front changes nothing — skip the menu + // rebuild and the settings write rather than doing both on every tab switch. + if (next.length === recentFiles.length && next[0] === recentFiles[0]) { + const same = next.every((p, i) => p === recentFiles[i]); + if (same) return; + } + recentFiles = next; + syncRecentMenu(); + scheduleSessionSave(); +} + +/** Drop an entry whose file turned out to be gone, so it can't fail twice. */ +function dropRecent(path: string): void { + const next = forgetRecent(recentFiles, path); + if (next.length === recentFiles.length) return; + recentFiles = next; + syncRecentMenu(); + scheduleSessionSave(); +} + +function clearRecentFiles(): void { + if (recentFiles.length === 0) return; + recentFiles = []; + syncRecentMenu(); + scheduleSessionSave(); + setStatus("Recent files cleared"); +} + +/** + * Open the nth recent file. + * + * The index comes from the menu id; the list it indexes is this one. A stale + * index — a menu rebuilt between render and click — resolves to nothing rather + * than to the wrong file, which is why the bounds check is here and not an + * assertion. + */ +async function openRecent(index: number): Promise { + const path = recentFiles[index]; + if (path === undefined) return; + try { + await openPath(path); + } catch { + // Gone or unreadable. Forget it rather than leaving an entry whose only + // possible outcome is this same failure. + dropRecent(path); + setStatus(`${basename(path)} could not be opened — removed from recent files`); + } +} + +// ---- Opening links --------------------------------------------------------- + +/** + * Ctrl/Cmd-click a link to open it in the default browser. + * + * Modifier-required, matching every other editor: in a WYSIWYG surface a plain + * click has to remain "put the caret here", or a link becomes a place you + * cannot edit. + * + * Whether a href may leave the app is decided by `links.ts`, which allows only + * http/https/mailto — an opened `.md` is untrusted (§3.3) and the OS shell acts + * on much more than web addresses. A refused link is reported rather than + * silently ignored, so "nothing happened" is never the whole story. + */ +function installLinkOpener(root: HTMLElement): void { + root.addEventListener("click", (e) => { + if (!(e.ctrlKey || e.metaKey)) return; + const href = linkHrefFrom(e.target); + if (!href) return; + e.preventDefault(); + if (!isExternallyOpenable(href)) { + setStatus(`Refused to open a non-web link: ${href.slice(0, 60)}`); + return; + } + void openExternal(href).catch((err: unknown) => setStatus(`Could not open link: ${String(err)}`)); + }); +} + +// ---- Editor zoom ----------------------------------------------------------- + +/** + * Apply the current zoom to the writing surface. + * + * One CSS variable on the root, consumed by `.editor .milkdown` — everything + * inside it is sized in `em`, so headings, code and the measure all follow from + * this single number. Nothing about the document changes: zoom is presentation + * only, so §3.2 is untouched. + */ +function applyZoom(): void { + document.documentElement.style.setProperty("--editor-zoom", String(editorZoom)); +} + +function setZoom(next: number): void { + if (next === editorZoom) return; + editorZoom = next; + applyZoom(); + setStatus(`Zoom ${formatZoom(editorZoom)}`); + scheduleSessionSave(); +} + // ---- Self-update (ROADMAP Movement I.5) ------------------------------------ // // The rules live in `update.ts` and are gated there; this is only the wiring @@ -1269,6 +1409,8 @@ function scheduleSessionSave(): void { update_check: updateState.enabled, update_last_checked: updateState.lastCheckedAt, update_skipped_version: updateState.skippedVersion, + editor_zoom: editorZoom, + recent_files: recentFiles, }; void saveSettings(settings).catch(() => {}); // best-effort }, 400); @@ -1288,6 +1430,11 @@ async function restoreSession(): Promise { return; } + // `version` is 0 only for a settings file that has never been written, since + // every save stamps the current version — so this is "Toril has never run + // here", not "nothing to restore". + firstRun = settings.version === 0; + // Theme first, so the restored UI paints in the right palette. if (theme && isTheme(settings.theme)) { theme.applyInitial(settings.theme); @@ -1312,6 +1459,18 @@ async function restoreSession(): Promise { updateState.lastCheckedAt = settings.update_last_checked; updateState.skippedVersion = settings.update_skipped_version; + // Normalized rather than trusted: session.json is a file a user can edit, and + // a stored zoom of 0 would render the editor unreadable with no way to zoom + // back out (`zoom.ts`). + editorZoom = normalizeZoom(settings.editor_zoom); + applyZoom(); + + // Untrusted shape, not just untrusted values: a hand-edited or crash-truncated + // session.json must not throw here, where an exception costs the user the + // whole restored session. + recentFiles = normalizeRecent(settings.recent_files); + syncRecentMenu(); + if (settings.last_folder) { try { await loadWorkspace(settings.last_folder); @@ -1413,6 +1572,10 @@ const ACTIONS: Record void> = { menu_toggle_autosave: () => toggleAutosave(), menu_toggle_update_check: () => toggleUpdateCheck(), menu_check_updates: () => void checkUpdates("manual"), + menu_zoom_in: () => setZoom(zoomIn(editorZoom)), + menu_zoom_out: () => setZoom(zoomOut(editorZoom)), + menu_zoom_reset: () => setZoom(ZOOM_DEFAULT), + menu_recent_clear: () => clearRecentFiles(), menu_export_html: () => void doExportHtml(), menu_export_rtf: () => void doExportRtf(), menu_find: () => searchBar?.open(), @@ -1429,6 +1592,15 @@ const dispatcher = new ActionDispatcher(); * `actions.ts` for why both doors are kept rather than one being disabled. */ function runAction(id: string): void { + // Recent-file items are generated, so they cannot live in the static ACTIONS + // table — their ids carry an index into `recentFiles`. Matched before the + // table lookup, and still routed through the dispatcher so a menu item with + // an accelerator could be added later without reintroducing the double-fire. + const recent = /^menu_recent_(\d+)$/.exec(id); + if (recent) { + dispatcher.dispatch(id, () => void openRecent(Number(recent[1]))); + return; + } const action = ACTIONS[id]; if (action) dispatcher.dispatch(id, action); } @@ -1458,6 +1630,18 @@ function shortcutAction(e: KeyboardEvent): string | null { return "menu_find"; case "e": return "menu_export_html"; + // Zoom, spelled every way a keyboard actually produces it. `Ctrl` and `+` + // means `Ctrl+Shift+=` on a US layout, `Ctrl+=` when the user skips shift, + // and `Ctrl+Add` on the numeric keypad — all three are the same intent, and + // binding only one of them is why zoom shortcuts so often "don't work". + case "+": + case "=": + return "menu_zoom_in"; + case "-": + case "_": + return "menu_zoom_out"; + case "0": + return "menu_zoom_reset"; default: return null; } @@ -1474,7 +1658,12 @@ window.addEventListener("DOMContentLoaded", async () => { const formatBar = document.querySelector("#format-toolbar"); if (!editorRoot || !tabbar || !sidebarEl || !formatBar) return; - sidebar = new Sidebar(sidebarEl, { onOpenFile: (p) => void openPath(p) }); + sidebar = new Sidebar(sidebarEl, { + onOpenFile: (p) => void openPath(p), + // Routed through the same action as the menu item, so the empty state's + // button cannot drift from File → Open Folder. + onOpenFolder: () => runAction("menu_open_folder"), + }); sidebar.setRoot(null, []); tabs = new TabManager(tabbar, { onDeactivate, onActivate, onCloseRequest }); @@ -1577,6 +1766,7 @@ window.addEventListener("DOMContentLoaded", async () => { document.querySelector("#btn-rail-history")?.addEventListener("click", () => selectRail("history")); installShortcuts(); installResizers(); + installLinkOpener(editorRoot); void onMenuAction(runAction); // native menu → the same named actions as the keyboard // Guard against losing unsaved work on close, and clear the recovery journal // on every clean close so a leftover journal always means "we crashed" (§3). @@ -1601,13 +1791,36 @@ window.addEventListener("DOMContentLoaded", async () => { } if (!tabs.active()) { - openDocument(null, "Untitled", WELCOME); + openDocument( + null, + firstRun ? "Welcome" : "Untitled", + firstRun ? FIRST_RUN : EMPTY, + ); } // While Toril is already running, a second double-click is forwarded here by // the single-instance plugin rather than starting a new process (§5). void onOpenFile((path) => void openPath(path)); + // Dropping notes on the window opens them. A drop is the one open path with + // no file-type filter in front of it, so `selectOpenable` is an allowlist — + // and a drop of five files where two are notes reports as much rather than + // appearing to have half-worked. + void onFilesDropped((paths) => { + const openable = selectOpenable(paths); + if (openable.length === 0) { + setStatus( + paths.length > 0 ? "Nothing there Toril can open (.md, .markdown, .html)" : "", + ); + return; + } + void (async () => { + for (const path of openable) await openPath(path); + const skipped = paths.length - openable.length; + if (skipped > 0) setStatus(`Opened ${openable.length}; skipped ${skipped}`); + })(); + }); + // Last, and only after the session is on screen: the check is a network round // trip, and nothing about it should delay a document appearing. `checkUpdates` // never rejects, and at startup it stays silent unless there is genuinely diff --git a/src/paths.ts b/src/paths.ts index 1e70a4a..d3de30c 100644 --- a/src/paths.ts +++ b/src/paths.ts @@ -27,3 +27,33 @@ export function isAtOrUnder(child: string, parent: string): boolean { if (c === p) return true; return c.startsWith(p.endsWith("/") ? p : `${p}/`); } + +/** + * The extensions Toril will open — the same set `tauri.conf.json` registers as + * file associations, kept in step with it. + * + * An **allowlist**, because this gates drag-and-drop: a drop is the one open + * path where the user never picked from a filter, so anything on the desktop + * can land on the window. `formatForPath` in `main.ts` is not a substitute — it + * answers "how do I parse this?" and answers "markdown" for everything it does + * not recognise, which is the right default once a file is known to be text and + * exactly the wrong one for deciding whether to open a dropped `.exe` at all. + */ +const OPENABLE = /\.(md|markdown|html?)$/i; + +/** Whether a dropped or forwarded path is one Toril should open. */ +export function isOpenablePath(path: string): boolean { + return OPENABLE.test(path); +} + +/** + * Narrow a drop to the files Toril can open, preserving order. + * + * Returns everything openable rather than just the first: dropping a selection + * of notes should open the selection. Non-matching entries are dropped + * silently here — the caller reports the count, because "3 of 5 opened" is the + * useful message and this function has no business composing it. + */ +export function selectOpenable(paths: readonly string[]): string[] { + return paths.filter(isOpenablePath); +} diff --git a/src/recent.ts b/src/recent.ts new file mode 100644 index 0000000..35f3ac2 --- /dev/null +++ b/src/recent.ts @@ -0,0 +1,70 @@ +// The recent-files list (ROADMAP Movement I.5, was §13 backlog). +// +// Pure list arithmetic, kept out of the controller because every interesting +// property here is about *ordering and identity*, and both are easy to get +// subtly wrong in a way no one notices until the list is full of duplicates. +// +// **Paths only, never contents** — the same rule `session.json` already +// follows (§3.2). The list is a set of pointers to files on disk; it never +// becomes a second copy of anything. + +/** How many entries the list keeps. Long enough to be useful, short enough to scan. */ +export const RECENT_LIMIT = 10; + +/** + * Put `path` at the front, remove any earlier occurrence, and cap the length. + * + * Returns a **new array**; the caller's list is untouched. That matters because + * the same list is read while rendering a menu, and mutating it in place turns + * "open a file" into a visual glitch somewhere unrelated. + * + * Reopening a file already in the list must *move* it rather than add a second + * entry, which is the whole reason this dedupes before it prepends. + */ +export function pushRecent( + list: readonly string[], + path: string, + limit: number = RECENT_LIMIT, +): string[] { + if (path === "") return [...list]; + return [path, ...list.filter((p) => p !== path)].slice(0, limit); +} + +/** + * Drop a path from the list — for a file that turned out to be gone. + * + * A recent entry that no longer resolves is worse than no entry: it offers the + * user an action that can only fail. The controller calls this when an open + * attempt reports the file missing. + */ +export function forgetRecent(list: readonly string[], path: string): string[] { + return list.filter((p) => p !== path); +} + +/** + * Coerce whatever was persisted into a usable list. + * + * `session.json` is a file a user can edit and a file that can be truncated by + * a crash, so this treats its contents as untrusted shape rather than assuming + * an array of strings: a non-array, a null entry, or a number all have to + * degrade to "no recents" instead of throwing during bootstrap, where an + * exception would cost the user their whole restored session. + * + * Deduping on load as well as on push is not redundant — an older Toril, or a + * hand-edited file, can supply a list this module never produced. + */ +export function normalizeRecent( + value: unknown, + limit: number = RECENT_LIMIT, +): string[] { + if (!Array.isArray(value)) return []; + const seen = new Set(); + const out: string[] = []; + for (const entry of value) { + if (typeof entry !== "string" || entry === "" || seen.has(entry)) continue; + seen.add(entry); + out.push(entry); + if (out.length >= limit) break; + } + return out; +} diff --git a/src/styles/chrome.css b/src/styles/chrome.css index c80668f..4afe0b1 100644 --- a/src/styles/chrome.css +++ b/src/styles/chrome.css @@ -164,6 +164,47 @@ body { padding: var(--sp-2); } +/* The empty state carries its own fix, so it is a small stack rather than one + italic line. `min-width: 0` because it lives in a pane that collapses to + zero (§12b rule 3). */ +.sidebar-empty { + display: flex; + flex-direction: column; + align-items: flex-start; + gap: var(--sp-2); + min-width: 0; +} + +.sidebar-empty-text { + margin: 0; +} + +.sidebar-empty-btn { + min-height: var(--target); + padding: 0 var(--sp-3); + border: 1px solid var(--border-strong); + border-radius: var(--r-sm); + background: var(--chrome-raised); + color: var(--fg); + cursor: pointer; + font: inherit; + font-style: normal; +} + +.sidebar-empty-btn:hover { + background: var(--hover-bg); +} + +.sidebar-empty-note { + margin: 0; + font-size: var(--text-sm); + font-style: normal; + color: var(--dim); + /* A long folder-path or URL in this copy must not set the pane's min-content + width (§12b rule 4). */ + overflow-wrap: anywhere; +} + .sidebar-root { font-weight: 600; padding: var(--sp-1) var(--sp-1) var(--sp-2); diff --git a/src/styles/editor.css b/src/styles/editor.css index 912777f..18fbe9b 100644 --- a/src/styles/editor.css +++ b/src/styles/editor.css @@ -22,8 +22,13 @@ padding: var(--sp-6) 0 var(--sp-6); } +/* Zoom scales this element's font-size, and everything inside is sized in `em` + so it follows. The measure is `em` too, deliberately: holding it at a fixed + pixel width while the type grows would run a zoomed-in line to eighteen + words, when the whole point of a measure is characters-per-line. */ .editor .milkdown { - max-width: 45rem; /* ~720px measure */ + font-size: calc(var(--text-base) * var(--editor-zoom, 1)); + max-width: 45em; /* ~720px measure at 100% */ margin: 0 auto; padding: 0 var(--sp-5); font-family: var(--font-prose); @@ -49,13 +54,13 @@ margin: var(--sp-5) 0 var(--sp-2); } -.editor .milkdown h1 { font-size: 1.9rem; letter-spacing: -0.02em; } -.editor .milkdown h2 { font-size: 1.45rem; letter-spacing: -0.01em; } -.editor .milkdown h3 { font-size: 1.2rem; } -.editor .milkdown h4 { font-size: 1.05rem; } -.editor .milkdown h5 { font-size: 0.95rem; color: var(--muted); } +.editor .milkdown h1 { font-size: 1.9em; letter-spacing: -0.02em; } +.editor .milkdown h2 { font-size: 1.45em; letter-spacing: -0.01em; } +.editor .milkdown h3 { font-size: 1.2em; } +.editor .milkdown h4 { font-size: 1.05em; } +.editor .milkdown h5 { font-size: 0.95em; color: var(--muted); } .editor .milkdown h6 { - font-size: 0.85rem; + font-size: 0.85em; color: var(--muted); text-transform: uppercase; letter-spacing: 0.06em; diff --git a/src/ui/sidebar.ts b/src/ui/sidebar.ts index 6a31bef..32a98ba 100644 --- a/src/ui/sidebar.ts +++ b/src/ui/sidebar.ts @@ -5,6 +5,14 @@ import type { FileNode } from "../ipc"; export interface SidebarCallbacks { onOpenFile(path: string): void; + /** + * Open a folder from the empty state. + * + * Optional so the sidebar stays constructible without it, but when it is + * absent the empty state falls back to text — a dead button that looks live + * is worse than a sentence. + */ + onOpenFolder?(): void; } export class Sidebar { @@ -18,10 +26,35 @@ export class Sidebar { this.container.replaceChildren(); if (rootName === null) { + // An empty state that only *names* the emptiness leaves the user to find + // the menu; the fix for "no folder open" belongs next to the message. + const wrap = document.createElement("div"); + wrap.className = "sidebar-empty"; + const hint = document.createElement("p"); - hint.className = "sidebar-empty"; - hint.textContent = "No folder open"; - this.container.append(hint); + hint.className = "sidebar-empty-text"; + hint.textContent = "No folder open."; + wrap.append(hint); + + if (this.cb.onOpenFolder) { + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "sidebar-empty-btn"; + btn.textContent = "Open Folder…"; + btn.addEventListener("click", () => this.cb.onOpenFolder?.()); + wrap.append(btn); + + const note = document.createElement("p"); + note.className = "sidebar-empty-note"; + // Worth saying once, here: the folder is the user's, not Toril's. It is + // the single most load-bearing promise in §1 and the empty state is the + // moment someone is deciding whether to point it at a real vault. + note.textContent = + "Pick any folder of notes — including an Obsidian vault. Files stay plain Markdown where they are."; + wrap.append(note); + } + + this.container.append(wrap); return; } diff --git a/src/welcome.ts b/src/welcome.ts new file mode 100644 index 0000000..4cd45d5 --- /dev/null +++ b/src/welcome.ts @@ -0,0 +1,68 @@ +// The two documents Toril opens when there is nothing else to show +// (ROADMAP Movement I.5). +// +// Their own module rather than constants in `main.ts` for one concrete reason: +// FIRST_RUN claims, in its own text, that Toril does not rewrite your files — +// so it is itself a round-trip fixture, and `tests/roundtrip.test.ts` imports +// it from here to check that saving it is a no-op diff. A welcome note that +// reformatted itself on first save would refute its own second paragraph. + +// Two different documents for two different situations, which the old single +// stub conflated. +// +// FIRST_RUN is shown once, to someone who has never opened Toril. It is written +// in the canonical form the serializer emits, so saving it is a no-op diff — +// the welcome note demonstrating the round-trip guarantee rather than +// contradicting it on its first save. +// +// Two details here are load-bearing and were found by the gate, not by reading: +// the table pipes are **padded** to the column width (remark's canonical form — +// `| --- |` gets rewritten), and the pane shortcuts are code spans rather than +// bold because `**Ctrl+\**` ends a strong span with a backslash, which escapes +// the closing marker and mangles the text. Neither is a style preference; edit +// this copy and rerun `tests/roundtrip.test.ts`. +// +// EMPTY is for every later launch with nothing to restore. Someone who has used +// Toril before does not need the tour again; they need a blank page. +export const FIRST_RUN = `# Welcome to Toril + +This is a real document — edit it, or start over with **Ctrl+N**. + +## Your notes stay yours + +Toril reads and writes plain \`.md\` files in ordinary folders. There is no +database, no proprietary container, and nothing to export later. Point it at an +Obsidian vault and both apps can use it. + +- **Ctrl+Shift+O** — open a folder of notes +- **Ctrl+O** — open a single file +- **Ctrl+S** — save · **Ctrl+F** — find and replace +- \`Ctrl+\\\` — files pane · \`Ctrl+Shift+\\\` — outline +- **Ctrl** and **+** / **-** / **0** — bigger, smaller, reset + +## Formatting happens as you type + +Type \`## \` for a heading, \`- \` for a bullet, \`> \` for a quote — the line +becomes the thing. There is no preview pane because there is nothing to preview. + +| It does | tables too | +| ------- | ---------- | +| and | task lists | + +- [ ] like this one +- [x] which you can tick + +## If something goes wrong + +Every save is atomic, so an interrupted write cannot corrupt a note. Every save +also records a version you can go back to, and if a file changes underneath you +— a sync client, another editor — Toril tells you rather than picking a winner. + +--- + +Ctrl-click a link to open it in your browser: +`; + +export const EMPTY = `# Untitled + +`; diff --git a/src/zoom.ts b/src/zoom.ts new file mode 100644 index 0000000..ba3ed07 --- /dev/null +++ b/src/zoom.ts @@ -0,0 +1,64 @@ +// Editor zoom (ROADMAP Movement I.5, was §13 backlog). +// +// Scales the *writing surface* only — not the chrome. Zooming the whole app is +// what the OS display scaling already does; what a writer actually wants at +// 11pm is bigger prose without a bigger tab bar eating the window. That is also +// why this lives here rather than reaching for the webview's own zoom. +// +// Pure, and expressed as a multiplier rather than a font size, so the caller +// decides what it multiplies. The measure scales with it (`45em`, not `45rem`, +// in editor.css), which keeps characters-per-line roughly constant instead of +// letting a zoomed-in line run to eighteen words. + +/** Multipliers, ascending. A fixed ladder, not a free-form number. */ +export const ZOOM_STEPS = [0.8, 0.9, 1, 1.1, 1.25, 1.5, 1.75, 2] as const; + +/** The multiplier `Ctrl+0` returns to, and the one a fresh install starts at. */ +export const ZOOM_DEFAULT = 1; + +/** + * A discrete ladder rather than "multiply by 1.1 each time" on purpose. Free + * scaling accumulates float drift, so a user who zooms in five times and out + * five times does not land back where they started — and "reset" then quietly + * means something different from "the size I had". A fixed ladder is also + * closed under round-tripping through JSON, which matters because it persists. + */ +function indexOfNearest(zoom: number): number { + let best = 0; + for (let i = 1; i < ZOOM_STEPS.length; i++) { + if (Math.abs(ZOOM_STEPS[i] - zoom) < Math.abs(ZOOM_STEPS[best] - zoom)) best = i; + } + return best; +} + +/** The next step up, or the current one if already at the top. */ +export function zoomIn(current: number): number { + const i = indexOfNearest(current); + return ZOOM_STEPS[Math.min(i + 1, ZOOM_STEPS.length - 1)]; +} + +/** The next step down, or the current one if already at the bottom. */ +export function zoomOut(current: number): number { + const i = indexOfNearest(current); + return ZOOM_STEPS[Math.max(i - 1, 0)]; +} + +/** + * Coerce a persisted value onto the ladder. + * + * Settings are a JSON file a user can edit, and a corrupt or hand-written value + * must not be able to render the editor unreadable — a stored `0` would collapse + * the prose to nothing with no way to zoom back out, since every step from zero + * is still zero. Anything unusable snaps to the default rather than being + * clamped to the nearest end: a nonsense value carries no intent to honour. + */ +export function normalizeZoom(value: number | null | undefined): number { + if (value === null || value === undefined) return ZOOM_DEFAULT; + if (!Number.isFinite(value) || value <= 0) return ZOOM_DEFAULT; + return ZOOM_STEPS[indexOfNearest(value)]; +} + +/** How the zoom reads in the status line — "100%", "125%". */ +export function formatZoom(zoom: number): string { + return `${Math.round(zoom * 100)}%`; +} diff --git a/tests/links.test.ts b/tests/links.test.ts new file mode 100644 index 0000000..ab4e3ed --- /dev/null +++ b/tests/links.test.ts @@ -0,0 +1,91 @@ +// GATE for external link opening (ROADMAP Movement I.5). +// +// This is a §3.3 security gate, not a feature test. An opened `.md` is +// untrusted — it can arrive from a shared vault, a sync folder, or an AI +// assistant — and Ctrl-clicking a link in one hands its href to the operating +// system's shell. `sanitize.ts` stops such a link executing *in* the webview; +// this stops it executing *outside* it, which is the larger risk, because the +// webview is sandboxed and the shell is not. +// +// The cases below are the ones a blocklist gets wrong. They are why the rule is +// an allowlist. +import { describe, expect, it } from "vitest"; +import { isExternallyOpenable, linkHrefFrom } from "../src/links"; + +describe("isExternallyOpenable", () => { + it("allows the three schemes a markdown link actually means", () => { + expect(isExternallyOpenable("https://example.com/notes")).toBe(true); + expect(isExternallyOpenable("http://example.com")).toBe(true); + expect(isExternallyOpenable("mailto:someone@example.com")).toBe(true); + }); + + it("refuses script schemes", () => { + expect(isExternallyOpenable("javascript:alert(1)")).toBe(false); + expect(isExternallyOpenable("vbscript:msgbox(1)")).toBe(false); + expect(isExternallyOpenable("data:text/html,")).toBe(false); + }); + + // The whole reason to parse rather than string-match: the OS normalizes case, + // interior whitespace and control characters before acting, so a raw-text + // check can be made to disagree with what actually opens. + it("refuses script schemes however they are spelled", () => { + expect(isExternallyOpenable("jAvAsCrIpT:alert(1)")).toBe(false); + expect(isExternallyOpenable(" javascript:alert(1)")).toBe(false); + expect(isExternallyOpenable("java\tscript:alert(1)")).toBe(false); + expect(isExternallyOpenable("java\nscript:alert(1)")).toBe(false); + expect(isExternallyOpenable("\u0000javascript:alert(1)")).toBe(false); + }); + + // Handing these to the shell is arbitrary local execution, not navigation. + it("refuses anything that reaches the local machine", () => { + expect(isExternallyOpenable("file:///C:/Windows/System32/cmd.exe")).toBe(false); + expect(isExternallyOpenable("file:///etc/passwd")).toBe(false); + expect(isExternallyOpenable("smb://attacker/share")).toBe(false); + // Windows protocol handlers used in real attacks. An allowlist refuses + // these without ever having heard of them, which is the point. + expect(isExternallyOpenable("ms-msdt:/id PCWDiagnostic")).toBe(false); + expect(isExternallyOpenable("search-ms:query=x&crumb=location:\\\\attacker")).toBe(false); + }); + + it("refuses what it cannot parse, rather than guessing", () => { + expect(isExternallyOpenable("")).toBe(false); + expect(isExternallyOpenable("not a url")).toBe(false); + expect(isExternallyOpenable("./relative/note.md")).toBe(false); + expect(isExternallyOpenable("//example.com")).toBe(false); + }); +}); + +describe("linkHrefFrom", () => { + function fragment(html: string): Element { + const host = document.createElement("div"); + host.innerHTML = html; + return host; + } + + it("finds the href when the anchor itself is clicked", () => { + const a = fragment('x').querySelector("a"); + expect(linkHrefFrom(a)).toBe("https://example.com"); + }); + + // `text` is ordinary markdown, and the click lands on + // the inner element — matching only `tagName === "A"` misses every styled link. + it("finds the href from an element inside the anchor", () => { + const inner = fragment('x').querySelector( + "strong", + ); + expect(linkHrefFrom(inner)).toBe("https://example.com"); + }); + + it("returns null off a link, so a plain click is never a navigation", () => { + const p = fragment("

just text

").querySelector("p"); + expect(linkHrefFrom(p)).toBe(null); + expect(linkHrefFrom(null)).toBe(null); + }); + + it("treats an anchor with no usable href as not a link", () => { + const a = fragment("anchor").querySelector("a"); + expect(linkHrefFrom(a)).toBe(null); + const empty = fragment('anchor').querySelector("a"); + expect(linkHrefFrom(empty)).toBe(null); + }); +}); diff --git a/tests/paths.test.ts b/tests/paths.test.ts index 7e1cb31..3baa0fe 100644 --- a/tests/paths.test.ts +++ b/tests/paths.test.ts @@ -3,7 +3,7 @@ // the directory path, so every open tab underneath it has to be matched by // containment rather than equality. import { describe, expect, it } from "vitest"; -import { isAtOrUnder } from "../src/paths"; +import { isAtOrUnder, isOpenablePath, selectOpenable } from "../src/paths"; describe("isAtOrUnder", () => { it("matches the path itself", () => { @@ -45,3 +45,46 @@ describe("isAtOrUnder", () => { expect(isAtOrUnder("/vault/x.md", "")).toBe(false); }); }); + +// Drag-and-drop is the one open path where the user never picked from a file +// filter, so anything on the desktop can land on the window. That makes this an +// allowlist rather than a convenience filter (ROADMAP Movement I.5). +describe("isOpenablePath", () => { + it("accepts the formats Toril edits", () => { + for (const p of ["a.md", "a.markdown", "a.html", "a.htm"]) { + expect(isOpenablePath(`/vault/${p}`)).toBe(true); + } + }); + + it("accepts them however they are cased", () => { + expect(isOpenablePath("/vault/NOTE.MD")).toBe(true); + expect(isOpenablePath("/vault/Page.HtMl")).toBe(true); + }); + + // `formatForPath` answers "markdown" for anything it does not recognise, + // which is right once a file is known to be text and wrong for deciding + // whether to open a dropped binary at all. + it("refuses everything else, rather than defaulting to markdown", () => { + for (const p of ["a.exe", "a.png", "a.txt", "a.pdf", "a.md.exe", "noextension"]) { + expect(isOpenablePath(`/vault/${p}`)).toBe(false); + } + }); + + it("is not fooled by the extension appearing mid-path", () => { + expect(isOpenablePath("/vault/notes.md/thing.exe")).toBe(false); + }); +}); + +describe("selectOpenable", () => { + it("keeps the openable files in the order they were dropped", () => { + expect(selectOpenable(["/a.md", "/b.png", "/c.html", "/d.exe"])).toEqual([ + "/a.md", + "/c.html", + ]); + }); + + it("returns nothing when a drop has nothing Toril can open", () => { + expect(selectOpenable(["/a.png", "/b.zip"])).toEqual([]); + expect(selectOpenable([])).toEqual([]); + }); +}); diff --git a/tests/recent.test.ts b/tests/recent.test.ts new file mode 100644 index 0000000..872f5b7 --- /dev/null +++ b/tests/recent.test.ts @@ -0,0 +1,94 @@ +// GATE for the recent-files list (ROADMAP Movement I.5). +// +// Small surface, but every property here has a failure mode that is invisible +// until the list is already wrong: a missing dedupe fills it with one file, a +// mutating push corrupts a menu being rendered elsewhere, and a load that +// throws on a hand-edited session.json costs the user their restored session. +import { describe, expect, it } from "vitest"; +import { RECENT_LIMIT, forgetRecent, normalizeRecent, pushRecent } from "../src/recent"; + +describe("pushRecent", () => { + it("puts the newest file first", () => { + expect(pushRecent(["/b.md"], "/a.md")).toEqual(["/a.md", "/b.md"]); + }); + + // Reopening a file must move it, not add a second copy — otherwise the list + // fills with whatever you are working on today. + it("moves an existing entry instead of duplicating it", () => { + expect(pushRecent(["/a.md", "/b.md", "/c.md"], "/c.md")).toEqual([ + "/c.md", + "/a.md", + "/b.md", + ]); + }); + + it("caps the list", () => { + let list: string[] = []; + for (let i = 0; i < RECENT_LIMIT + 5; i++) list = pushRecent(list, `/n${i}.md`); + expect(list).toHaveLength(RECENT_LIMIT); + expect(list[0]).toBe(`/n${RECENT_LIMIT + 4}.md`); + }); + + it("drops the oldest entry when it caps, not the newest", () => { + const full = Array.from({ length: RECENT_LIMIT }, (_, i) => `/n${i}.md`); + const next = pushRecent(full, "/new.md"); + expect(next[0]).toBe("/new.md"); + expect(next).not.toContain(`/n${RECENT_LIMIT - 1}.md`); + }); + + // The same array is read while rendering the File menu; mutating in place + // turns "open a file" into a glitch somewhere unrelated. + it("does not mutate the list it was given", () => { + const original = ["/a.md"]; + const copy = [...original]; + pushRecent(original, "/b.md"); + expect(original).toEqual(copy); + }); + + it("ignores an empty path rather than storing one", () => { + expect(pushRecent(["/a.md"], "")).toEqual(["/a.md"]); + }); +}); + +describe("forgetRecent", () => { + // An entry that no longer resolves is worse than no entry: it offers an + // action that can only fail. + it("removes a path that turned out to be gone", () => { + expect(forgetRecent(["/a.md", "/b.md"], "/a.md")).toEqual(["/b.md"]); + }); + + it("is a no-op for a path that was never there", () => { + expect(forgetRecent(["/a.md"], "/z.md")).toEqual(["/a.md"]); + }); +}); + +describe("normalizeRecent", () => { + it("keeps a well-formed list", () => { + expect(normalizeRecent(["/a.md", "/b.md"])).toEqual(["/a.md", "/b.md"]); + }); + + // Bootstrap reads this. Throwing here costs the user the whole restored + // session over a malformed field. + it("degrades to empty rather than throwing on junk", () => { + expect(normalizeRecent(null)).toEqual([]); + expect(normalizeRecent(undefined)).toEqual([]); + expect(normalizeRecent("not a list")).toEqual([]); + expect(normalizeRecent(42)).toEqual([]); + expect(normalizeRecent({ 0: "/a.md" })).toEqual([]); + }); + + it("skips entries that are not usable paths", () => { + expect(normalizeRecent(["/a.md", null, 7, "", "/b.md"])).toEqual(["/a.md", "/b.md"]); + }); + + // An older Toril, or a hand-edited file, can supply a list this module never + // produced — so loading dedupes too. + it("dedupes a list it did not produce, keeping the first occurrence", () => { + expect(normalizeRecent(["/a.md", "/b.md", "/a.md"])).toEqual(["/a.md", "/b.md"]); + }); + + it("caps an over-long stored list", () => { + const long = Array.from({ length: RECENT_LIMIT + 5 }, (_, i) => `/n${i}.md`); + expect(normalizeRecent(long)).toHaveLength(RECENT_LIMIT); + }); +}); diff --git a/tests/roundtrip.test.ts b/tests/roundtrip.test.ts index 654c87d..7120a29 100644 --- a/tests/roundtrip.test.ts +++ b/tests/roundtrip.test.ts @@ -26,6 +26,7 @@ import { emoji } from "@milkdown/plugin-emoji"; import { useCanonical } from "../src/editor/canonical"; import { docToMarkdown } from "../src/editor/serializer"; import { joinFrontMatter, splitFrontMatter } from "../src/editor/frontmatter"; +import { FIRST_RUN } from "../src/welcome"; /** Parse `md` into a real editor doc, then serialize it back to markdown. */ async function roundtrip(md: string): Promise { @@ -172,6 +173,16 @@ const frontMatterNormalized: Record = { }; describe("round-trip fidelity (Phase 1 gate)", () => { + // The welcome note is shipped copy that *claims* Toril does not rewrite your + // files. If it does not itself survive a save, the first thing a new user + // does contradicts the second paragraph they just read. + it("does not rewrite the first-run welcome note", async () => { + const once = await roundtrip(FIRST_RUN); + expect(once).toBe(FIRST_RUN); + const twice = await roundtrip(once); + expect(twice).toBe(once); + }); + for (const [name, md] of Object.entries(fixtures)) { it(`is canonical & stable: ${name}`, async () => { const once = await roundtrip(md); diff --git a/tests/zoom.test.ts b/tests/zoom.test.ts new file mode 100644 index 0000000..0f7decc --- /dev/null +++ b/tests/zoom.test.ts @@ -0,0 +1,94 @@ +// GATE for editor zoom (ROADMAP Movement I.5). +// +// The reason this is a module with a test rather than three lines in main.ts is +// the round-trip property below: zoom persists, so a value that drifts or a +// value that comes back corrupt has consequences past the current session. The +// worst case is not cosmetic — a stored zoom of 0 renders the editor unreadable +// *and* unfixable, because every step from zero is still zero. +import { describe, expect, it } from "vitest"; +import { + ZOOM_DEFAULT, + ZOOM_STEPS, + formatZoom, + normalizeZoom, + zoomIn, + zoomOut, +} from "../src/zoom"; + +describe("stepping", () => { + it("moves one step at a time", () => { + expect(zoomIn(1)).toBe(1.1); + expect(zoomOut(1)).toBe(0.9); + }); + + it("stops at the ends instead of running off them", () => { + const max = ZOOM_STEPS[ZOOM_STEPS.length - 1]; + const min = ZOOM_STEPS[0]; + expect(zoomIn(max)).toBe(max); + expect(zoomOut(min)).toBe(min); + }); + + // The property that makes a fixed ladder worth having over `* 1.1`: without + // it, five in and five out lands somewhere near 1 but not on it, and "reset" + // silently stops meaning "the size I had". + it("returns exactly where it started after equal steps in and out", () => { + let z = ZOOM_DEFAULT; + for (let i = 0; i < 5; i++) z = zoomIn(z); + for (let i = 0; i < 5; i++) z = zoomOut(z); + expect(z).toBe(ZOOM_DEFAULT); + }); + + it("walks the whole ladder without skipping or repeating", () => { + const seen: number[] = [ZOOM_STEPS[0]]; + let z: number = ZOOM_STEPS[0]; + for (let i = 0; i < ZOOM_STEPS.length; i++) { + const next = zoomIn(z); + if (next === z) break; + seen.push(next); + z = next; + } + expect(seen).toEqual([...ZOOM_STEPS]); + }); +}); + +describe("normalizeZoom", () => { + it("keeps a value already on the ladder", () => { + for (const step of ZOOM_STEPS) expect(normalizeZoom(step)).toBe(step); + }); + + it("defaults when there is nothing stored", () => { + expect(normalizeZoom(null)).toBe(ZOOM_DEFAULT); + expect(normalizeZoom(undefined)).toBe(ZOOM_DEFAULT); + }); + + // The unreadable-and-unfixable case. A clamp to the nearest end would give + // 0.8 here, which is a guess at intent; a nonsense value carries none. + it("refuses a zoom that would make the editor unusable", () => { + expect(normalizeZoom(0)).toBe(ZOOM_DEFAULT); + expect(normalizeZoom(-2)).toBe(ZOOM_DEFAULT); + expect(normalizeZoom(Number.NaN)).toBe(ZOOM_DEFAULT); + expect(normalizeZoom(Number.POSITIVE_INFINITY)).toBe(ZOOM_DEFAULT); + }); + + it("snaps an off-ladder value to the nearest step", () => { + expect(normalizeZoom(1.12)).toBe(1.1); + expect(normalizeZoom(1.4)).toBe(1.5); + // Beyond the top of the ladder is a real intent — pin it to the maximum. + expect(normalizeZoom(99)).toBe(ZOOM_STEPS[ZOOM_STEPS.length - 1]); + }); + + it("survives a JSON round trip, which is how it is actually stored", () => { + for (const step of ZOOM_STEPS) { + const back = JSON.parse(JSON.stringify({ z: step })).z as number; + expect(normalizeZoom(back)).toBe(step); + } + }); +}); + +describe("formatZoom", () => { + it("reads as a percentage", () => { + expect(formatZoom(1)).toBe("100%"); + expect(formatZoom(1.25)).toBe("125%"); + expect(formatZoom(0.8)).toBe("80%"); + }); +}); From 719287454b218928709dae10efd75cc803c53435 Mon Sep 17 00:00:00 2001 From: Evan Gress <106449014+evangress@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:23:52 -0400 Subject: [PATCH 3/4] feat(sidebar): new, rename and delete, without leaving the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Toril could open and save a note but not organize one: renaming meant going to the file manager and coming back. This adds new note, new folder, rename and delete to the files pane, and in doing so gives two already-built, already-tested pieces of Rust their first caller — trashbin shipped in v1.0.0 with no UI at all, and snapshots::rekey was written for exactly this rename. The rules go in crates/fileops rather than the command layer, because they are not obvious and they are mostly Windows: reserved device names that appear to work and then behave like hardware, names ending in a dot or space that Windows silently strips (so the file that appears is not the one we returned a path for), and a case-only rename that a case-insensitive filesystem reports as a collision with the file itself. Every operation refuses to clobber and requires its target inside the open folder, mirroring trashbin's boundary for the delete direction. Containment is checked with canonicalize and never used to build a result. On Windows that returns a \\?\C:\... path, which does I/O fine and matches nothing else in the app — not the sidebar tree, not an open tab, not a note's history key — so it would quietly split one note into two identities. Pinned by a test. Delete offers Undo instead of asking first: the file moves into .trash/, so the act is reversible and a confirmation would be friction in front of it. The one thing trash cannot bring back is an unsaved buffer, and that is the one case that stops and asks. The section 3 surface here is rename. The watcher reports it as delete then create, which is the shape removedOnDisk exists to catch — left alone, an open tab decides its file vanished and offers to recreate it, resurrecting the old note beside its new name. doRenameEntry re-points every affected tab (including tabs under a renamed folder), bumps the removal epoch for each old path so an in-flight reconcile cannot apply a stale "missing" verdict, and clears removedOnDisk — all in one synchronous block. base is deliberately untouched: a rename changes no bytes, so the merge base is still exactly right. The context menu is DOM, not native: a native popup cannot be driven by the headless gates, and native dialogs are the one thing documented to hang the app on the Linux dev box. vaultscan now keeps an empty directory. It pruned any folder without markdown in it, which meant New Folder created something that immediately disappeared and could not be put a note into. Asset-only folders stay pruned. Gates: cargo test -p fileops (28), tests/sidebar.test.ts, tests/contextmenu.test.ts. Two real defects were caught writing them — a window blur listener registered with capture, which sees every element's blur and so closed the menu the instant it focused its own first item; and the canonicalized-path leak above. Not verified on a device: the browser harness could not be driven in this session (the Chrome extension was not connected). Checklist in docs/ON-DEVICE-VERIFICATION.md section E. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 8 +- CHANGELOG.md | 22 + CLAUDE.md | 82 ++- ROADMAP.md | 36 +- dev-harness.html | 143 ++++- docs/ON-DEVICE-VERIFICATION.md | 43 +- src-tauri/Cargo.lock | 5 + src-tauri/Cargo.toml | 2 + src-tauri/crates/fileops/Cargo.toml | 7 + src-tauri/crates/fileops/src/lib.rs | 764 ++++++++++++++++++++++++++ src-tauri/crates/vaultscan/src/lib.rs | 44 +- src-tauri/src/commands/entries.rs | 114 ++++ src-tauri/src/commands/mod.rs | 1 + src-tauri/src/lib.rs | 4 + src/ipc.ts | 89 +++ src/main.ts | 207 ++++++- src/styles/chrome.css | 132 +++++ src/ui/contextmenu.ts | 198 +++++++ src/ui/sidebar.ts | 533 ++++++++++++++++-- tests/contextmenu.test.ts | 188 +++++++ tests/sidebar.test.ts | 315 +++++++++++ 21 files changed, 2868 insertions(+), 69 deletions(-) create mode 100644 src-tauri/crates/fileops/Cargo.toml create mode 100644 src-tauri/crates/fileops/src/lib.rs create mode 100644 src-tauri/src/commands/entries.rs create mode 100644 src/ui/contextmenu.ts create mode 100644 tests/contextmenu.test.ts create mode 100644 tests/sidebar.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 63240ef..c12d53b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,7 +85,7 @@ jobs: workspaces: ./src-tauri -> target # Named packages, NOT --workspace: that would pull in the Tauri app crate - # and with it WebKitGTK on Linux / WebView2 on Windows. These nine are + # and with it WebKitGTK on Linux / WebView2 on Windows. These ten are # split out (CLAUDE.md §4) precisely so their gates run without a webview. # # keystore's tests all run against its in-memory double. The real @@ -94,7 +94,10 @@ jobs: # # Windows matters here too: fsatomic is the §3.1 atomic-save gate and the # one crate doing real filesystem syscalls, where replace-over-existing - # differs between rename(2) and MoveFileEx. + # differs between rename(2) and MoveFileEx. fileops is the second such + # crate and needs Windows even more specifically: the names it rejects + # (reserved devices, trailing dots) and the case-only rename it must allow + # are *Windows* behaviours, and the Linux leg alone cannot see either. - name: Test logic crates run: > cargo test @@ -107,6 +110,7 @@ jobs: -p snapshots -p mergemd -p keystore + -p fileops # `--all` covers workspace *members*, and the vendored glib is excluded # from the workspace (§2) — so unlike clippy, this never touches upstream diff --git a/CHANGELOG.md b/CHANGELOG.md index 4406952..88a7ab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,28 @@ GitHub Release notes plus the commits that shipped in it. ## [Unreleased] ### Added +- **Manage your notes from the files pane.** New note, new folder, rename and delete, + from a right-click on any file or folder — or the `+` button beside the folder name. + Until now Toril could open and save notes but not *organize* them: renaming a note + meant leaving for the file manager and coming back. + + **Deleting moves the note to a `.trash` folder inside your workspace, and offers you + Undo.** Nothing is unlinked, so a delete is recoverable long after the message has + gone — and because it is recoverable, Toril does not interrupt you to confirm it. The + one thing trash cannot bring back is a buffer you never saved, so that is the one case + that stops and asks. + + **A rename takes the note's version history with it**, and moves any open tab — + including every tab inside a renamed folder — to the new path. Renaming does not + rewrite a single byte of the file. + + Names are checked before anything touches disk, against the rules Windows actually + enforces: no `< > : " / \ | ? *`, nothing ending in a space or a dot, and none of the + reserved device names (`CON`, `NUL`, `COM1`…) that appear to work and then behave like + hardware. Renaming `notes.md` to `Notes.md` — a change of case only — works. Nothing + can be created or renamed outside the folder you opened, and no operation will ever + overwrite a file that is already there. + - **Toril can update itself.** `v1.0.0` had no update path at all, so every copy was stranded on the version it was installed with — the only way forward was to notice a new release and download the installer by hand. Toril now checks for a newer build diff --git a/CLAUDE.md b/CLAUDE.md index ca6c3a0..8dcf893 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -136,6 +136,16 @@ The **QoL half of the same branch** has also landed: editor zoom, open-links-in- drag-and-drop open, a recent-files list in File → Open Recent, and a real first-run welcome note distinct from the empty state. See §5 (Quality-of-life batch). +**Sidebar file operations — `feat/sidebar-file-ops` (ROADMAP Movement II.12).** New note, +new folder, rename and delete, from a context menu in the files pane. This closes two +pieces of already-built, already-tested Rust that had **no caller at all**: `trashbin` +(shipped in v1.0.0 with no UI) and `snapshots::rekey` (written for exactly this rename). +New crate `crates/fileops` holds the rules; `commands/entries.rs` wraps it; the UX is +`src/ui/sidebar.ts` on a new `src/ui/contextmenu.ts`. Details and the rename-ordering +argument are in §5 (Sidebar file operations). Still open from the branch: drag-to-move in +the tree, multi-select, and a trash browser (`list_trash` has a command and a wrapper but +no UI, so Undo reaches only the most recent delete). + **Next:** finish Phase 4 — remaining is Azure Trusted Signing once an account exists, and on-device verification. A backlog of further QoL features is in §13. The **forward plan beyond Phase 4** — turning the editor into a notes *system* (search, links, version @@ -151,8 +161,8 @@ pnpm tauri build # production .exe + installer (Windows; see §9) pnpm test # vitest — round-trip + toolbar + theme + export + tabs + security (jsdom) pnpm typecheck # tsc --noEmit (TS strict) pnpm build # tsc + vite build (frontend only) -# logic crates — the same nine CI runs (plain `cargo test` also builds the app crate) -cd src-tauri && cargo test -p fsatomic -p vaultscan -p mdhtml -p mdrtf -p imgasset -p trashbin -p snapshots -p mergemd -p keystore +# logic crates — the same ten CI runs (plain `cargo test` also builds the app crate) +cd src-tauri && cargo test -p fsatomic -p vaultscan -p mdhtml -p mdrtf -p imgasset -p trashbin -p snapshots -p mergemd -p keystore -p fileops cd src-tauri && cargo fmt --all && cargo clippy # clean before commit (§10) ``` @@ -277,7 +287,8 @@ toril/ │ │ ├── html-serializer.ts # the ONE HTML <-> doc converter (§3.2/§3.3) │ │ └── html-constructs.ts # richer HTML-only schema: callout/details/dl/mark/sub/sup (§6) │ ├── ui/ -│ │ ├── sidebar.ts # file tree +│ │ ├── sidebar.ts # file tree + file operations UX (§5, ROADMAP II.12) +│ │ ├── contextmenu.ts # the one in-app context menu (DOM, not native — §8) │ │ ├── panes.ts # PURE pane state: visibility, widths, rail tab (§13) │ │ ├── rail.ts # the single tabbed right rail (outline | history) │ │ ├── resizer.ts # drag-to-resize: pure geometry + thin DOM binding @@ -310,13 +321,15 @@ toril/ │ ├── trashbin/ # soft-delete to workspace .trash/ + restore (§3) │ ├── snapshots/ # content-addressed local version history (§3, ROADMAP I.3) │ ├── mergemd/ # line-based 3-way merge + conflict filenames (§3, ROADMAP I.4) - │ └── keystore/ # OS-keychain API key storage (§3, ROADMAP IV.20) + │ ├── keystore/ # OS-keychain API key storage (§3, ROADMAP IV.20) + │ └── fileops/ # create/rename: name rules, containment, no clobber (ROADMAP II.12) └── src/ ├── main.rs # bin entry → lib::run() ├── lib.rs # Tauri builder + menu + command registration ├── menu.rs # native app menu → `menu` events (§8) ├── commands/ │ ├── files.rs # open / save (ATOMIC) / save_as + │ ├── entries.rs # create note/folder, rename (+ history rekey) — ROADMAP II.12 │ ├── workspace.rs # open folder, list tree, watch (notify crate) │ ├── export.rs # markdown_to_html + export_html; export_rtf (all-Rust) │ ├── images.rs # save_clipboard_image (imgasset) @@ -350,6 +363,10 @@ frontend never touches the filesystem directly; it asks via `invoke()`. | `save_recovery` | `entries` | `()` | **atomic** write of `recovery.json` in the app config dir — crash-recovery journal (§3) | | `load_recovery` | — | `RecoveryEntry[]` | empty on missing/corrupt (never bricks startup) | | `clear_recovery` | — | `()` | delete `recovery.json` — the clean-shutdown sentinel | +| `create_note` | `vault_root, dir, name` | `path` | Create an **empty** note in `dir` (`crates/fileops`). `name` may omit the extension (`.md` added). `create_new`, so the existence check and the creation are one operation — two racing creates cannot both win | +| `create_folder` | `vault_root, parent, name` | `path` | `create_dir`, not `create_dir_all`: an existing folder is an error, not a silent success | +| `suggest_note_name` | `vault_root, dir, stem` | `name` | `Untitled.md`, else `Untitled 2.md`… A **suggestion** for the inline field only; `create_note` still refuses a collision. Takes `vault_root` although it only reads: it answers "does this file exist?" for any path handed to it, and the webview is untrusted (§3.3) | +| `rename_entry` | `vault_root, path, new_name` | `path` | Rename within the same parent, refusing to clobber a *different* entry (a **case-only** rename is allowed — resolved via `canonicalize`, since on a case-insensitive filesystem the destination "exists" as the source itself). Carries version history through `snapshots::rekey` — for a folder, every note in the subtree — **best-effort and additive**: the rename already happened, and history failing to follow must not report it as a failure | | `move_to_trash` | `vault_root, path` | `TrashEntry` | soft-delete into workspace `.trash/` via `trashbin` — **atomic** move (§3) | | `list_trash` | `vault_root` | `TrashEntry[]` | newest first; empty when no `.trash/` | | `restore_from_trash` | `vault_root, id` | `path` | restore to original path; errors **without clobbering** an existing file | @@ -402,9 +419,34 @@ frontend never touches the filesystem directly; it asks via `invoke()`. > `manifest.json`) rather than `rm`; restore reads the manifest and atomically renames > it back, refusing to clobber a file that reappeared at the path. `.trash/` starts with > `.`, so `vaultscan` already hides it from the sidebar (§1) and Obsidian hides it too. -> Backed by `crates/trashbin`; commands are not yet called by any UI (the sidebar file-ops -> branch wires them). +> Backed by `crates/trashbin`, and wired to the sidebar as of Movement II.12. +> +> **Sidebar file operations (Movement II.12).** `crates/fileops` holds the rules — +> name validation, vault containment, refusing to clobber — and `commands/entries.rs` +> is a thin wrapper plus the history rekey. The UX is `src/ui/sidebar.ts` (context menu, +> inline name field) on `src/ui/contextmenu.ts`. **The menu is a DOM menu, not a native +> one**, and deliberately: a native popup cannot be driven by the headless gates, and +> native dialogs are the one thing documented to hang the app on the Linux dev box. The +> single native dialog left is the unsaved-changes confirm on delete (`confirmDiscard`), +> which is the close guard's prompt and the same judgement call. +> +> **Delete offers Undo instead of asking first.** The move is into `.trash/`, so it is +> reversible by construction; a confirmation would be friction in front of a reversible +> act. `restore_from_trash` — dead code until this branch — is what the Undo calls. What +> trash cannot restore is an **unsaved buffer**, so that case does stop and ask. > +> **A rename is the §3 surface of this feature, and the ordering in `doRenameEntry` is +> the fix.** The watcher reports a rename as delete-then-create, which is exactly the +> shape `removedOnDisk` exists to catch — left alone, an open tab would decide its file +> had vanished and offer to recreate it, resurrecting the old note beside its new name. +> So, in one synchronous block after the rename resolves: re-point every affected tab +> (including tabs *under* a renamed folder), bump `removalEpoch` for each old path (a +> `reconcile` may be in flight against it and would apply a `missing` verdict to a tab +> that has moved), and clear `removedOnDisk` in case the delete event already landed. +> `base` is deliberately **not** reset — a rename changes no bytes, so the merge base is +> still exactly right and re-reading would only invite a race. +> + > **Version history.** Every save records a content-addressed snapshot (`crates/snapshots`): > per-note dir under `/history//` — a `manifest.json` + gzip, > sha256-addressed blobs. It lives **outside the vault** (like recovery/session, §1) so it @@ -582,6 +624,18 @@ Phases 0–3 are complete and Phase 4 (polish) is in progress; the shipped detai never drops a line. The wire protocol's fifth outcome, `missing`, is produced one layer up in `src-tauri/src/commands/sync.rs` (an `io::ErrorKind::NotFound` on the read, before `mergemd` is even called) — that file has **no tests of its own**, so `missing` is exercised on-device only. +- **File operations:** `cargo test -p fileops` — the name rules (the Windows-forbidden character + set, control characters, trailing dot/space, reserved device names *with* an extension, and the + near-misses that must still be **accepted**: `console.md`, `com10.md`), containment against a + traversal dressed up as a subdirectory, every create/rename refusing to clobber while a + **case-only** rename still succeeds, and that a returned path keeps the *caller's* spelling — + the regression that pins it: containment is checked with `canonicalize`, which on Windows + returns `\\?\C:\…`, and building the result from that hands back a path matching neither the + sidebar tree, nor an open tab, nor the note's history key. Plus `tests/sidebar.test.ts` (the menu + offers only what is wired, a rejected name keeps the field open with the message, blur cancels + rather than commits, and a background refresh cannot delete what is being typed) and + `tests/contextmenu.test.ts` (one menu at a time; dismissal actually removes its document + listeners; the menu closes *before* its handler runs, since handlers take focus). - **External-change policy:** `tests/sync.test.ts` — `decideAction`'s outcome→action mapping is total and fails closed, HTML never auto-merges, and `selectSavable` excludes a diverged tab from every bulk write path (§5). @@ -638,9 +692,11 @@ Phases 0–3 are complete and Phase 4 (polish) is in progress; the shipped detai **CI runs these automatically** on every pull request and on pushes to `main` (`.github/workflows/ci.yml`): `pnpm typecheck` + `pnpm test` + `pnpm build`, and `cargo test` over the -nine logic crates — each on **Ubuntu and Windows**, plus `cargo fmt --all --check` on Ubuntu. The -Windows leg is not ceremony: `pnpm install --frozen-lockfile` is what applies the Milkdown patch, and -`fsatomic` is the §3.1 gate whose replace-over-existing semantics differ from POSIX there. +ten logic crates — each on **Ubuntu and Windows**, plus `cargo fmt --all --check` on Ubuntu. The +Windows leg is not ceremony: `pnpm install --frozen-lockfile` is what applies the Milkdown patch, +`fsatomic` is the §3.1 gate whose replace-over-existing semantics differ from POSIX there, and +`fileops` encodes *Windows* naming rules (reserved devices, trailing dots, case-only renames) that +the Linux leg alone cannot see. **What CI cannot cover — still yours to run:** interactive GUI flows (`pnpm tauri dev` — dialogs, menus, the reload prompt), macOS, the Tauri app crate, and `cargo clippy` (§10; excluded from CI @@ -826,6 +882,8 @@ Keep the project's rules: testable logic in `crates/*` or pure TS helpers, all d **Medium (more UI / a new command, but high value):** - **Global workspace search ("find in files")** — a Rust command scanning the vault (sibling to `vaultscan`, keeping scan logic unit-testable) + a results panel. Distinct from in-document Find. -- **Sidebar file operations** — new / rename / delete / new-folder via context menu, backed by new - **atomic** Rust commands; mind the watcher interplay. -- **Document outline / TOC panel** — list headings from the doc, click to scroll. +- ~~**Sidebar file operations**~~ — *shipped* (ROADMAP Movement II.12). Still open from that + branch: **drag-to-move** within the tree, **multi-select**, and a **trash browser** + (`list_trash` has a command and an `ipc.ts` wrapper but no UI — Undo is currently the only + way back, and it only reaches the most recent delete). +- ~~**Document outline / TOC panel**~~ — *shipped* (Movement II.11). diff --git a/ROADMAP.md b/ROADMAP.md index f5bddb3..d749eda 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -63,11 +63,18 @@ no AI. That gap is this roadmap. > after it, so the next feature could not have reached anyone. §7's *trust before reach* > is what settles the order. Read the pointer below as the ordering, not the ladder. > +> **Branch 12 (sidebar file operations) has also landed**, taken ahead of 6 for the same +> kind of reason 10 was: it wired two crates — `trashbin` and `snapshots::rekey` — that +> had shipped with **no caller at all**, and until it landed a note could be opened and +> saved but not renamed or deleted without leaving the app. +> > **▶ Pick up at Movement II, branch 6 — `feat/vault-search`.** (Branch 5 is done bar > Azure signing, which is blocked on an account, not on code.) > Search is the largest remaining functional gap, and branch 7 > (command palette) depends on it. Vet `tantivy` per §2 at -> adoption — it would be the project's largest new dependency. Branch 4's spec lived +> adoption — it would be the project's largest new dependency, and worth weighing +> against a hand-rolled inverted index the way the snapshot store was weighed +> against `gix` (§8). Branch 4's spec lived > **on its own branch**, not on `main`: > `docs/superpowers/specs/2026-07-24-sync-coexistence-design.md`; branch 10's is on > `main` at `docs/superpowers/specs/2026-08-17-frontmatter-properties-design.md`. @@ -303,11 +310,27 @@ editor something you keep using rather than something you tried. frontend (was §13 backlog). - *Touches:* `src/ui/outline.ts`. *Gate:* `tests/outline.test.ts`. -- [ ] **12. `feat/sidebar-file-ops`** — new / rename / delete / new-folder via context menu, - backed by **atomic** Rust commands; delete routes through `trashbin`. - - *Touches:* `commands/files.rs` (new atomic ops + contract rows), `src/ui/sidebar.ts`; - mind the watcher interplay. - - *Gate:* extend `fsatomic` / a new ops suite. +- [x] **12. `feat/sidebar-file-ops`** — *(shipped)* new / rename / delete / new-folder via a + context menu; delete routes through `trashbin` and offers **Undo** rather than a + confirmation, because a move into `.trash/` is reversible by construction. Wires two + crates that had no caller: `trashbin` and `snapshots::rekey`. + - *New crate:* `crates/fileops` — name validation (the Windows rules, applied on every + platform, since a vault is a folder that gets synced), vault containment, and + create/rename that refuse to clobber. Deliberately returns paths in the **caller's** + spelling: `canonicalize` is used to check containment and never to build a result, + because on Windows it yields `\\?\C:\…`, which matches neither the sidebar tree nor an + open tab nor a note's history key. + - *Built:* `commands/entries.rs` (thin wrapper + the history rekey, best-effort so it can + never fail a rename that already happened), `src/ui/contextmenu.ts`, file-ops UX in + `src/ui/sidebar.ts`. + - *§3:* the rename ordering in `main.ts`'s `doRenameEntry` — the watcher reports a rename + as delete-then-create, so tabs are re-pointed, `removalEpoch` bumped and `removedOnDisk` + cleared in one synchronous block, or an open tab would offer to recreate the old note + beside its new name. + - *Gate:* `cargo test -p fileops` + `tests/sidebar.test.ts` + `tests/contextmenu.test.ts`. + - *Open:* drag-to-move in the tree, multi-select, and a trash browser (`list_trash` is + wired to no UI, so Undo reaches only the most recent delete). On-device verification in + `docs/ON-DEVICE-VERIFICATION.md` §E. - [ ] **⬢ RELEASE `v0.5.0-beta.1`** — *"A real notes system."* First **beta**. --- @@ -499,6 +522,7 @@ All follow the `crates/*` pattern: webview-free, unit-tested, healthy pure-Rust | `trashbin` | Soft-delete + restore (atomic) | — | `feat/safe-delete-trash` | | `snapshots` | Content-addressed local version history | hand-rolled (`sha2` + `flate2`); see §8 | `feat/local-version-history` | | `mergemd` | 3-way markdown merge + conflict files | `similar` | `feat/sync-coexistence` | +| `fileops` | Create/rename: name rules, containment, no clobber | — | `feat/sidebar-file-ops` | | `vaultsearch` | Incremental full-text vault index | `tantivy` | `feat/vault-search` | | `linkgraph` | `[[link]]`/`#tag` parse + backlink index | hand-rolled / `pulldown-cmark` | `feat/wikilinks-backlinks` | | `highlight` | Code → highlighted spans | `syntect` (`fancy-regex`) | `feat/code-highlighting` | diff --git a/dev-harness.html b/dev-harness.html index 672ba81..2799bf1 100644 --- a/dev-harness.html +++ b/dev-harness.html @@ -96,6 +96,80 @@ { hash: "c9d0e1f2", saved_at: Date.now() - 86_400_000 * 3, bytes: 201 }, ]; + // ---- in-memory vault mutations (ROADMAP II.12) -------------------------- + // + // Enough of `fileops` / `trashbin` to drive the sidebar's file operations + // here: create, rename, delete-to-trash and restore against TREE + FILES. + // Deliberately *not* a reimplementation of the rules — the real ones are + // unit-tested in Rust. What this stub owes the harness is the small set of + // refusals the UI has to render (a taken name, a reserved device name), so + // the error path can be seen without a backend. + const TRASH = []; + let trashSeq = 0; + + /** The children array a path belongs in, or null if the folder is unknown. */ + function listFor(dir) { + if (dir === "/vault") return TREE; + let found = null; + const walk = (nodes) => { + for (const n of nodes) { + if (n.is_dir && n.path === dir) found = n.children; + else if (n.is_dir) walk(n.children); + } + }; + walk(TREE); + return found; + } + + function findNode(path) { + let hit = null; + const walk = (nodes, parent) => { + for (const n of nodes) { + if (n.path === path) hit = { node: n, siblings: nodes }; + else if (n.is_dir) walk(n.children, n); + } + }; + walk(TREE, null); + return hit; + } + + // vaultscan's order: directories first, then case-insensitive by name. + function insert(list, node) { + list.push(node); + list.sort((a, b) => + a.is_dir === b.is_dir + ? a.name.toLowerCase().localeCompare(b.name.toLowerCase()) + : a.is_dir ? -1 : 1, + ); + } + + const RESERVED = /^(con|prn|aux|nul|com[0-9]|lpt[0-9])(\.|$)/i; + function checkName(name) { + if (!name.trim()) throw new Error("Enter a name."); + if (/[<>:"/\\|?*]/.test(name)) throw new Error(`A name cannot contain ${name.match(/[<>:"/\\|?*]/)[0]}`); + if (/[. ]$/.test(name)) throw new Error("A name cannot end with a space or a dot."); + if (RESERVED.test(name)) throw new Error(`"${name.split(".")[0]}" is a reserved name on Windows.`); + } + + function withExtension(name) { + return /\.(md|markdown|html?)$/i.test(name) ? name : `${name}.md`; + } + + /** + * Move a subtree's paths (and any FILES entries) from one root to another. + * `from` stays the *original* root throughout, so a child's path — still + * un-rewritten when it is visited — slices correctly against it. + */ + function repath(node, from, to) { + const oldPath = node.path; + node.path = to + oldPath.slice(from.length); + if (!node.is_dir && oldPath in FILES) { + FILES[node.path] = FILES[oldPath]; + delete FILES[oldPath]; + } + for (const child of node.children) repath(child, from, to); + } + // ---- fake Tauri bridge -------------------------------------------------- const callbacks = new Map(); let nextCallbackId = 1; @@ -146,7 +220,74 @@ list_history: () => HISTORY, read_snapshot: ({ path }) => FILES[path] ?? "", restore_snapshot: () => null, - list_trash: () => [], + create_note: ({ dir, name }) => { + const fileName = withExtension(name); + checkName(fileName); + const list = listFor(dir); + if (!list) throw new Error("That location is outside the open folder."); + const path = `${dir}/${fileName}`; + if (findNode(path)) throw new Error(`"${fileName}" already exists`); + insert(list, { name: fileName, path, is_dir: false, children: [] }); + FILES[path] = ""; + return path; + }, + create_folder: ({ parent, name }) => { + checkName(name); + const list = listFor(parent); + if (!list) throw new Error("That location is outside the open folder."); + const path = `${parent}/${name}`; + if (findNode(path)) throw new Error(`"${name}" already exists`); + insert(list, { name, path, is_dir: true, children: [] }); + return path; + }, + suggest_note_name: ({ dir, stem }) => { // vaultRoot ignored: one vault here + + for (let n = 1; n < 100; n++) { + const candidate = n === 1 ? `${stem}.md` : `${stem} ${n}.md`; + if (!findNode(`${dir}/${candidate}`)) return candidate; + } + return `${stem}.md`; + }, + rename_entry: ({ path, newName }) => { + checkName(newName); + const hit = findNode(path); + if (!hit) throw new Error("no such entry"); + const parent = path.slice(0, path.lastIndexOf("/")); + const target = `${parent}/${newName}`; + if (target !== path && findNode(target)) throw new Error(`"${newName}" already exists`); + repath(hit.node, path, target); + hit.node.name = newName; + insert(hit.siblings, hit.siblings.splice(hit.siblings.indexOf(hit.node), 1)[0]); + return target; + }, + move_to_trash: ({ path }) => { + const hit = findNode(path); + if (!hit) throw new Error("no such entry"); + hit.siblings.splice(hit.siblings.indexOf(hit.node), 1); + const entry = { + id: `harness-${trashSeq++}`, + original_path: path, + name: hit.node.name, + deleted_at: Date.now(), + }; + TRASH.push({ entry, node: hit.node }); + return entry; + }, + restore_from_trash: ({ id }) => { + const idx = TRASH.findIndex((t) => t.entry.id === id); + if (idx === -1) throw new Error("no such trash entry"); + const { entry, node } = TRASH[idx]; + const parent = entry.original_path.slice(0, entry.original_path.lastIndexOf("/")); + const list = listFor(parent); + if (!list) throw new Error("the original folder is gone"); + // The real `restore` refuses rather than clobbers; so does this, since + // that refusal is a message the UI has to render. + if (findNode(entry.original_path)) throw new Error("something is already there"); + insert(list, node); + TRASH.splice(idx, 1); + return entry.original_path; + }, + list_trash: () => TRASH.map((t) => t.entry), list_api_keys: () => [ { provider: "anthropic", configured: false }, { provider: "openai", configured: false }, diff --git a/docs/ON-DEVICE-VERIFICATION.md b/docs/ON-DEVICE-VERIFICATION.md index 8bae349..faf452c 100644 --- a/docs/ON-DEVICE-VERIFICATION.md +++ b/docs/ON-DEVICE-VERIFICATION.md @@ -1,7 +1,7 @@ # On-Device Verification CI covers the headless gates: `pnpm typecheck` / `test` / `build` and `cargo test` over -the nine logic crates, on Ubuntu and Windows (CLAUDE.md §8). A green PR means those +the ten logic crates, on Ubuntu and Windows (CLAUDE.md §8). A green PR means those passed — **not** that the app was driven. This file is the standing list of what a green PR cannot tell you: interactive flows @@ -214,6 +214,47 @@ that a signed artifact downloads, verifies and replaces a running binary. appears. Ask via Help → Check for Updates: it says it could not check. That asymmetry is the whole design and is easy to regress. +## E. Sidebar file operations (`feat/sidebar-file-ops`, 2026-08-17) + +The rules are gated hard (`cargo test -p fileops`, `tests/sidebar.test.ts`, +`tests/contextmenu.test.ts`), but three things sit outside every headless gate: the +*Windows* filesystem the name rules describe, the watcher's reaction to an operation +Toril itself performed, and whether the version history actually followed the file. + +- [ ] **E1 — Rename does not resurrect the old note.** The §3 case, and the reason + `doRenameEntry` orders its work the way it does. Open a note, rename it from the + sidebar, then wait for the watcher: the tab must follow to the new name, must + **not** show "removed on disk", and autosave must not recreate the old path. Then + repeat with the note *dirty*, and with a note open inside a folder you rename. +- [ ] **E2 — Version history followed the rename.** Save a note two or three times, + rename it, then open the history panel: the earlier versions must still be listed. + `snapshots::rekey` had **no caller before this branch**, so this is its first + real exercise. Repeat for a note inside a renamed folder (the subtree path). +- [ ] **E3 — Delete and Undo.** Delete a note with a tab open: the tab closes, the file + appears under `.trash/`, and the status bar offers Undo. Click it — the note comes + back at its original path. Then delete a note, create a *different* file at that + path, and Undo: it must refuse rather than clobber, and say the note is still in + `.trash`. Finally delete a **dirty** tab's file and confirm the native prompt + appears (the one native dialog this feature uses — see B7 for the Linux hazard). +- [ ] **E4 — Windows name rules against the real filesystem.** The crate's tests assert + what we *reject*; only Windows proves the rejections are the right ones. Try + `CON.md`, `note.` , `note ` (trailing space), `a:b.md` and a 300-character name — + each refused with a readable message beside the field. Then rename `notes.md` to + `Notes.md`: this **must succeed** on NTFS, where the destination "already exists" + as the source itself, and the sidebar must show the new casing. +- [ ] **E5 — Paths keep their spelling.** After creating a note from the sidebar, confirm + the tab, the sidebar highlight and the window title all agree, and that saving it + produces history under the same key. A `\\?\`-prefixed path would work for I/O and + silently split the note into two identities; the crate pins this, the app proves it. +- [ ] **E6 — The context menu in both engines.** It is `position: fixed` at `--z-dialog`. + Confirm in WebKitGTK as well as WebView2 that it is not clipped by a pane's + `overflow: hidden`, that right-clicking near the right edge or the bottom keeps it + on screen, and that Shift+F10 / the menu key opens it anchored to the row rather + than the window corner. +- [ ] **E7 — The refresh race.** Start typing a new note's name, then touch an unrelated + file in the vault from outside (or let a sync client do it). The field must keep + what you typed; the new tree must appear as soon as you confirm or cancel. + ## B. Standing items (pre-existing, not from this branch) - [ ] **B1 — HTML as a first-class format.** Open a real AI-authored `.html` artifact, diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 083b48c..1d99d84 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1216,6 +1216,10 @@ dependencies = [ "rustc_version", ] +[[package]] +name = "fileops" +version = "0.1.0" + [[package]] name = "filetime" version = "0.2.29" @@ -4839,6 +4843,7 @@ checksum = "756daf9b1013ebe47a8776667b466417e2d4c5679d441c26230efd9ef78692db" name = "toril-app" version = "1.0.0" dependencies = [ + "fileops", "fsatomic", "imgasset", "keystore", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 73e4263..62be048 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -12,6 +12,7 @@ members = [ "crates/snapshots", "crates/mergemd", "crates/keystore", + "crates/fileops", ] # Vendored, security-patched glib (see [patch.crates-io] below). Excluded so it # is never a workspace member — we don't fmt/clippy/test upstream third-party code. @@ -65,6 +66,7 @@ trashbin = { path = "crates/trashbin" } snapshots = { path = "crates/snapshots" } mergemd = { path = "crates/mergemd" } keystore = { path = "crates/keystore" } +fileops = { path = "crates/fileops" } # Self-update, desktop only (ROADMAP Movement I.5). `v1.0.0` shipped with no # update path, so every installed copy is stranded on the version it was diff --git a/src-tauri/crates/fileops/Cargo.toml b/src-tauri/crates/fileops/Cargo.toml new file mode 100644 index 0000000..3c8ff8d --- /dev/null +++ b/src-tauri/crates/fileops/Cargo.toml @@ -0,0 +1,7 @@ +[package] +name = "fileops" +version = "0.1.0" +edition = "2024" +description = "Create / rename primitives for a Toril workspace (CLAUDE.md §3, ROADMAP II.12). Name validation, vault containment and clobber-refusing moves; no Tauri dep so the rules are unit-testable anywhere." + +[dependencies] diff --git a/src-tauri/crates/fileops/src/lib.rs b/src-tauri/crates/fileops/src/lib.rs new file mode 100644 index 0000000..5ac8091 --- /dev/null +++ b/src-tauri/crates/fileops/src/lib.rs @@ -0,0 +1,764 @@ +//! Create / rename primitives for a Toril workspace (CLAUDE.md §3, ROADMAP +//! Movement II.12 `feat/sidebar-file-ops`). +//! +//! Deleting is not here — that is `trashbin`, which soft-deletes into the +//! workspace `.trash/` and can restore. What is here is everything that brings a +//! path into existence or changes which path a note lives at: +//! +//! - [`validate_name`] — the rules a single path component must satisfy. +//! - [`create_note`] / [`create_folder`] — refuse to clobber, never overwrite. +//! - [`rename`] — same parent, new name; refuses to clobber. +//! - [`available_note_name`] — pick an unused default (`Untitled 2.md`). +//! - [`descendant_files`] — the subtree a folder rename moved, so version +//! history can follow it. +//! +//! **Two rules carry the safety, and both are enforced here rather than in the +//! caller.** Every operation requires its target to resolve *inside* the vault +//! root — the webview is untrusted (§3.3) and a crafted `invoke` must not be able +//! to write outside the folder the user opened. And every operation that creates +//! a path refuses an existing one instead of replacing it: there is no flag to +//! force it, because "rename over the note that was already there" is exactly the +//! silent loss §3 forbids. Mirrors `trashbin::move_to_trash`, which draws the +//! same containment boundary for the delete direction. +//! +//! No Tauri dependency: pure `std`, fully unit-tested. + +use std::ffi::OsStr; +use std::fs; +use std::io; +use std::path::{Path, PathBuf}; + +/// Extensions Toril treats as a note, kept in step with `src/paths.ts`'s +/// `OPENABLE` and `tauri.conf.json`'s file associations. +const NOTE_EXTENSIONS: [&str; 4] = ["md", "markdown", "html", "htm"]; + +/// The extension a new note gets when the user types a bare name. +const DEFAULT_EXTENSION: &str = "md"; + +/// Longest single path component we will create. 255 is the limit on NTFS, ext4, +/// APFS and HFS+ alike; measured in bytes because that is what the strictest of +/// them counts. +const MAX_COMPONENT_BYTES: usize = 255; + +/// Characters no path component may contain. +/// +/// This is the **Windows** set, applied on every platform on purpose. Toril's +/// primary target is Windows (§1) but a vault is a plain folder that may be +/// synced from a Mac or a Linux box, so a name that is legal to create here and +/// impossible to open there is a portability bug we would be authoring into the +/// user's own files. `/` and `\` are in the set for a second reason as well: +/// they would turn one component into a path. +const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '\\', '|', '?', '*']; + +/// Device names Windows reserves, which cannot be used as a file name *even with +/// an extension* — `NUL.md` is still the null device. Creating one appears to +/// succeed and then behaves like a device, so it is rejected before we try. +const RESERVED_STEMS: [&str; 4] = ["con", "prn", "aux", "nul"]; + +/// Reserved device-name prefixes that take a single trailing digit (`COM1`, +/// `LPT9`). `0` is included: it is reserved on current Windows and costs nothing. +const RESERVED_NUMBERED: [&str; 2] = ["com", "lpt"]; + +fn invalid(msg: &str) -> io::Error { + io::Error::new(io::ErrorKind::InvalidInput, msg.to_string()) +} + +fn exists(path: &Path) -> io::Error { + io::Error::new( + io::ErrorKind::AlreadyExists, + format!( + "\"{}\" already exists", + path.file_name().unwrap_or_default().to_string_lossy() + ), + ) +} + +/// Whether `stem` is a Windows reserved device name (case-insensitive). +fn is_reserved_stem(stem: &str) -> bool { + let lower = stem.to_ascii_lowercase(); + if RESERVED_STEMS.contains(&lower.as_str()) { + return true; + } + RESERVED_NUMBERED.iter().any(|prefix| { + lower + .strip_prefix(prefix) + .is_some_and(|rest| rest.len() == 1 && rest.as_bytes()[0].is_ascii_digit()) + }) +} + +/// Check that `name` is a usable single path component. +/// +/// Rejects, with a message written for the user rather than the log: emptiness, +/// path separators and the other characters Windows forbids, control characters, +/// `.` / `..`, a trailing space or dot (Windows *silently strips* both, so the +/// file that appears is not the one that was asked for and the caller's returned +/// path would be a lie), Windows reserved device names, and anything over +/// [`MAX_COMPONENT_BYTES`]. +pub fn validate_name(name: &str) -> io::Result<()> { + if name.is_empty() { + return Err(invalid("Enter a name.")); + } + if name.trim().is_empty() { + return Err(invalid("A name cannot be only spaces.")); + } + if name.len() > MAX_COMPONENT_BYTES { + return Err(invalid("That name is too long.")); + } + if name == "." || name == ".." { + return Err(invalid("\".\" and \"..\" are not names.")); + } + if let Some(c) = name.chars().find(|c| FORBIDDEN_CHARS.contains(c)) { + return Err(invalid(&format!("A name cannot contain {c}"))); + } + if name.chars().any(|c| c.is_control()) { + return Err(invalid("A name cannot contain control characters.")); + } + if name.ends_with(' ') || name.ends_with('.') { + return Err(invalid("A name cannot end with a space or a dot.")); + } + // The stem before the *first* dot is what Windows matches a device against, + // so `nul.md` and `nul.tar.gz` are both the null device. + let stem = name.split('.').next().unwrap_or(name); + if is_reserved_stem(stem) { + return Err(invalid(&format!( + "\"{stem}\" is a reserved name on Windows." + ))); + } + Ok(()) +} + +/// Require `inside` to resolve to a location at or under `root`. Both must exist. +/// +/// `canonicalize` resolves `..` and symlinks, which is what makes this a +/// containment check rather than a string comparison, and on Windows it applies +/// the same verbatim prefix to both sides so they compare. +/// +/// **It returns nothing on purpose.** The obvious shape — hand back the resolved +/// path and build on that — would be wrong on Windows, where `canonicalize` +/// returns the extended-length `\\?\C:\…` form. Every path in this app is a +/// string that has to match: the sidebar tree's paths, the tab a path opens, +/// the key a note's version history is stored under. A `\\?\` path works for +/// I/O and matches none of them, so it would silently split one note into two +/// identities. Callers therefore validate here and keep building from the +/// caller's own path spelling. +fn require_inside(root: &Path, inside: &Path) -> io::Result<()> { + let canon_root = fs::canonicalize(root)?; + let canon = fs::canonicalize(inside)?; + if !canon.starts_with(&canon_root) { + return Err(invalid("That location is outside the open folder.")); + } + Ok(()) +} + +/// The name a new note gets on disk: `name` if it already carries a note +/// extension (case-insensitive), otherwise `name.md`. +/// +/// The condition is "*a note* extension", not "any extension", so a name that +/// merely contains a dot still becomes a note: `Meeting 2026.08.17` → +/// `Meeting 2026.08.17.md`, where an any-extension rule would read `.17` as +/// deliberate and leave a file the sidebar cannot show (`vaultscan` lists +/// markdown only). The cost is that `notes.txt` becomes `notes.txt.md` — the +/// right trade, because New Note means a note, and a wrong-looking name is +/// visible and one rename away, while an invisible file is neither. +pub fn with_note_extension(name: &str) -> String { + match Path::new(name).extension().and_then(OsStr::to_str) { + Some(ext) if NOTE_EXTENSIONS.contains(&ext.to_ascii_lowercase().as_str()) => { + name.to_string() + } + _ => format!("{name}.{DEFAULT_EXTENSION}"), + } +} + +/// Create an empty note called `name` in `dir`, and return its path. +/// +/// `create_new` makes the existence check and the creation one atomic operation, +/// so two racing creates cannot both believe they won and one silently truncate +/// the other's note. The file is created **empty**: a new note's first bytes are +/// whatever the user types, and writing a template here would be content they did +/// not ask for (§3). +pub fn create_note(vault_root: &Path, dir: &Path, name: &str) -> io::Result { + let file_name = with_note_extension(name); + validate_name(&file_name)?; + require_inside(vault_root, dir)?; + let path = dir.join(&file_name); + match fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + { + Ok(_) => Ok(path), + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => Err(exists(&path)), + Err(e) => Err(e), + } +} + +/// Create a folder called `name` inside `parent`, and return its path. +/// +/// `create_dir` (not `create_dir_all`) so an existing folder is an error rather +/// than a silent success — the caller asked to make something new, and reporting +/// success for a folder that was already there would let the UI claim it created +/// a note's home that in fact belongs to something else. +pub fn create_folder(vault_root: &Path, parent: &Path, name: &str) -> io::Result { + validate_name(name)?; + require_inside(vault_root, parent)?; + let path = parent.join(name); + match fs::create_dir(&path) { + Ok(()) => Ok(path), + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => Err(exists(&path)), + Err(e) => Err(e), + } +} + +/// Rename `from` to `new_name` within the same parent, and return the new path. +/// +/// A no-op rename (the same name, byte for byte) succeeds and returns the +/// existing path — otherwise confirming an unchanged inline edit would report a +/// clobber error for a file against itself. +/// +/// **`fs::rename` overwrites an existing destination on both Unix and Windows**, +/// so the existence check below is the only thing standing between a rename and +/// a destroyed note. It is a check-then-act, and therefore racy against another +/// process creating that exact path in the window between — the race is accepted +/// (`std` exposes no atomic rename-if-absent that is portable), and it is the +/// same shape `trashbin::restore` already carries. What makes it survivable is +/// that the loser was on disk and is therefore in version history (§3): a +/// clobber is recoverable, undetected-but-recoverable being the standing edge of +/// the guarantee (CLAUDE.md §5). +/// +/// A file's extension is **not** forced or preserved: renaming `note.md` to +/// `note.html` is allowed, because the bytes are untouched and it is the user's +/// file. The caller must not reinterpret an open document's format on the +/// strength of a rename — re-serializing content that did not change is the +/// unrequested rewrite this project exists to avoid. +pub fn rename(vault_root: &Path, from: &Path, new_name: &str) -> io::Result { + validate_name(new_name)?; + require_inside(vault_root, from)?; + let parent = from + .parent() + .ok_or_else(|| invalid("That item has no parent folder."))?; + let to = parent.join(new_name); + + if to == from { + return Ok(to); + } + // Case-only renames (`notes.md` → `Notes.md`) are a real thing users want, and + // on a case-insensitive filesystem they arrive here as "destination exists" — + // against the source file itself. Comparing *names* would not tell that apart + // (they differ, by exactly the case being changed), so resolve the + // destination and ask whether it is the same file: `canonicalize` returns the + // real on-disk name, which equals the already-canonicalized `from` when the + // two are one file. A destination that cannot be resolved is treated as a + // genuine occupant — failing closed on an ambiguous read is the safe + // direction when the alternative is a clobbering rename. + if to.exists() { + let same_file = match (fs::canonicalize(&to), fs::canonicalize(from)) { + (Ok(a), Ok(b)) => a == b, + _ => false, + }; + if !same_file { + return Err(exists(&to)); + } + } + fs::rename(from, &to)?; + Ok(to) +} + +/// An unused `stem`-based note name in `dir` — `Untitled.md`, else `Untitled 2.md`, +/// `Untitled 3.md`, … +/// +/// Used for the default the inline rename field is pre-filled with, so pressing +/// New Note twice in a row proposes a name that will actually work rather than +/// one that fails on confirm. It is a *suggestion*: `create_note` still refuses a +/// collision, because the user can edit the field and something can appear on +/// disk between the suggestion and the create. +pub fn available_note_name(vault_root: &Path, dir: &Path, stem: &str) -> String { + let first = format!("{stem}.{DEFAULT_EXTENSION}"); + // Containment even though this only reads: it answers "does this file + // exist?" for any path handed to it, and the webview is untrusted (§3.3). + // On refusal it returns the plain first candidate — the caller is only + // pre-filling a field, and `create_note` enforces the same boundary for + // real. + if require_inside(vault_root, dir).is_err() { + return first; + } + if !dir.join(&first).exists() { + return first; + } + // Bounded so a pathological directory cannot spin forever; at that point the + // suggestion is wrong but harmless, and `create_note` reports the collision. + for n in 2..1000 { + let candidate = format!("{stem} {n}.{DEFAULT_EXTENSION}"); + if !dir.join(&candidate).exists() { + return candidate; + } + } + first +} + +/// Every regular file under `dir`, recursively, skipping hidden entries. +/// +/// Exists so a **folder** rename can carry version history: the snapshot store is +/// keyed by a note's absolute path, so moving a folder re-keys every note beneath +/// it, and the caller needs the before-list to pair with the after-list. Hidden +/// entries are skipped for the same reason `vaultscan` skips them — `.trash/` and +/// `.obsidian/` are not the user's notes. +/// +/// Unreadable subdirectories are skipped rather than failing the walk: this feeds +/// a best-effort, additive history migration (§3), and one permission-denied +/// folder must not turn a successful rename into a reported failure. +pub fn descendant_files(dir: &Path) -> Vec { + let mut out = Vec::new(); + collect_files(dir, &mut out); + out.sort(); + out +} + +fn collect_files(dir: &Path, out: &mut Vec) { + let Ok(entries) = fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + if name.to_string_lossy().starts_with('.') { + continue; + } + let path = entry.path(); + match entry.file_type() { + Ok(t) if t.is_dir() => collect_files(&path, out), + Ok(t) if t.is_file() => out.push(path), + _ => {} + } + } +} + +/// Re-root `path` from under `old_root` to under `new_root`. +/// +/// The other half of the folder-rename history migration: given the file list +/// taken before the move, this says where each one landed. Returns `None` if +/// `path` was not under `old_root`, so a caller cannot accidentally map an +/// unrelated path into the new tree. +pub fn reroot(path: &Path, old_root: &Path, new_root: &Path) -> Option { + let rest = path.strip_prefix(old_root).ok()?; + Some(new_root.join(rest)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::{AtomicU64, Ordering}; + use std::time::{SystemTime, UNIX_EPOCH}; + + struct TempDir(PathBuf); + impl TempDir { + fn new(tag: &str) -> TempDir { + static N: AtomicU64 = AtomicU64::new(0); + let n = N.fetch_add(1, Ordering::Relaxed); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let dir = std::env::temp_dir().join(format!("fileops-{tag}-{nanos}-{n}")); + fs::create_dir_all(&dir).unwrap(); + TempDir(dir) + } + fn path(&self) -> &Path { + &self.0 + } + } + impl Drop for TempDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + fn write_file(path: &Path, contents: &str) { + if let Some(p) = path.parent() { + fs::create_dir_all(p).unwrap(); + } + fs::write(path, contents).unwrap(); + } + + // ---- validate_name ---------------------------------------------------- + + #[test] + fn accepts_ordinary_names() { + for name in [ + "note.md", + "Meeting Notes.md", + "2026-08-17.md", + "café ☕.md", + "a.very.dotted.name.md", + "-leading-dash.md", + ".hidden.md", // legal; vaultscan just won't list it + ] { + assert!(validate_name(name).is_ok(), "should accept {name}"); + } + } + + #[test] + fn rejects_empty_and_whitespace() { + assert!(validate_name("").is_err()); + assert!(validate_name(" ").is_err()); + } + + #[test] + fn rejects_path_separators_so_a_name_cannot_become_a_path() { + assert!(validate_name("sub/note.md").is_err()); + assert!(validate_name("sub\\note.md").is_err()); + assert!(validate_name("../escape.md").is_err()); + } + + #[test] + fn rejects_the_windows_forbidden_set_on_every_platform() { + for name in [ + "ab.md", "a:b.md", "a\"b.md", "a|b.md", "a?b.md", "a*b.md", + ] { + assert!(validate_name(name).is_err(), "should reject {name}"); + } + } + + #[test] + fn rejects_control_characters() { + assert!(validate_name("note\u{0}.md").is_err()); + assert!(validate_name("note\n.md").is_err()); + assert!(validate_name("note\t.md").is_err()); + } + + #[test] + fn rejects_dot_names_and_trailing_dot_or_space() { + assert!(validate_name(".").is_err()); + assert!(validate_name("..").is_err()); + // Windows strips these silently, so the created file would not be the + // one we returned a path for. + assert!(validate_name("note.").is_err()); + assert!(validate_name("note ").is_err()); + assert!(validate_name("...").is_err()); + } + + #[test] + fn rejects_windows_reserved_device_names_with_or_without_extension() { + for name in [ + "CON", + "con", + "NUL.md", + "nul.tar.gz", + "AUX.md", + "prn.markdown", + "COM1.md", + "lpt9.md", + "COM0.md", + ] { + assert!(validate_name(name).is_err(), "should reject {name}"); + } + // Not reserved: a longer word that merely starts with one, or a + // multi-digit port that Windows does not reserve. + for name in ["console.md", "connection.md", "com10.md", "auxiliary.md"] { + assert!(validate_name(name).is_ok(), "should accept {name}"); + } + } + + #[test] + fn rejects_an_overlong_component() { + let long = format!("{}.md", "a".repeat(300)); + assert!(validate_name(&long).is_err()); + let ok = format!("{}.md", "a".repeat(200)); + assert!(validate_name(&ok).is_ok()); + } + + // ---- extension handling ------------------------------------------------ + + #[test] + fn appends_md_only_when_the_name_lacks_a_note_extension() { + assert_eq!(with_note_extension("note"), "note.md"); + assert_eq!(with_note_extension("note.md"), "note.md"); + assert_eq!(with_note_extension("note.MD"), "note.MD"); + assert_eq!(with_note_extension("note.markdown"), "note.markdown"); + assert_eq!(with_note_extension("page.html"), "page.html"); + // Not a *note* extension, so it still gets one — New Note makes a note. + assert_eq!(with_note_extension("notes.txt"), "notes.txt.md"); + // The case that rule exists for: a dotted name is not an extension. + assert_eq!( + with_note_extension("Meeting 2026.08.17"), + "Meeting 2026.08.17.md" + ); + assert_eq!(with_note_extension("v1.2.3"), "v1.2.3.md"); + } + + // ---- create_note ------------------------------------------------------- + + #[test] + fn creates_an_empty_note_and_returns_its_path() { + let t = TempDir::new("create"); + let path = create_note(t.path(), t.path(), "First note").unwrap(); + + assert_eq!(path.file_name().unwrap(), "First note.md"); + assert_eq!(fs::read_to_string(&path).unwrap(), ""); + } + + /// Every returned path must be spelled the way the caller spelled it. + /// + /// The regression this pins: containment is checked with `canonicalize`, + /// which on Windows returns the extended-length `\\?\C:\…` form. Building + /// the result from that would hand back a path that does real I/O fine and + /// matches nothing else in the app — not the sidebar tree, not an open + /// tab's path, not the note's version-history key — silently splitting one + /// note into two identities. + #[test] + fn returned_paths_keep_the_callers_spelling() { + let t = TempDir::new("spelling"); + let dir = t.path(); + + let created = create_note(dir, dir, "note").unwrap(); + assert_eq!(created, dir.join("note.md")); + + let folder = create_folder(dir, dir, "sub").unwrap(); + assert_eq!(folder, dir.join("sub")); + + let renamed = rename(dir, &created, "other.md").unwrap(); + assert_eq!(renamed, dir.join("other.md")); + + for path in [&created, &folder, &renamed] { + assert!( + !path.to_string_lossy().contains(r"\\?\"), + "{path:?} leaked a canonicalized prefix" + ); + } + } + + #[test] + fn create_note_refuses_to_clobber_an_existing_note() { + let t = TempDir::new("clobber"); + write_file(&t.path().join("note.md"), "precious"); + + let err = create_note(t.path(), t.path(), "note.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + assert_eq!( + fs::read_to_string(t.path().join("note.md")).unwrap(), + "precious", + "the existing note must be untouched" + ); + } + + #[test] + fn create_note_refuses_a_directory_outside_the_vault() { + let t = TempDir::new("outside"); + let vault = t.path().join("vault"); + let elsewhere = t.path().join("elsewhere"); + fs::create_dir_all(&vault).unwrap(); + fs::create_dir_all(&elsewhere).unwrap(); + + let err = create_note(&vault, &elsewhere, "escaped.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert!( + !elsewhere.join("escaped.md").exists(), + "nothing may be created outside the vault" + ); + } + + #[test] + fn create_note_refuses_a_traversal_dressed_up_as_a_subdirectory() { + let t = TempDir::new("traverse"); + let vault = t.path().join("vault"); + fs::create_dir_all(&vault).unwrap(); + fs::create_dir_all(t.path().join("elsewhere")).unwrap(); + + let sneaky = vault.join("..").join("elsewhere"); + let err = create_note(&vault, &sneaky, "escaped.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + } + + // ---- create_folder ----------------------------------------------------- + + #[test] + fn creates_a_folder_and_refuses_an_existing_one() { + let t = TempDir::new("folder"); + let path = create_folder(t.path(), t.path(), "Projects").unwrap(); + assert!(path.is_dir()); + + let err = create_folder(t.path(), t.path(), "Projects").unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + } + + #[test] + fn create_folder_does_not_append_an_extension() { + let t = TempDir::new("folder-ext"); + let path = create_folder(t.path(), t.path(), "notes").unwrap(); + assert_eq!(path.file_name().unwrap(), "notes"); + } + + // ---- rename ------------------------------------------------------------ + + #[test] + fn renames_within_the_same_parent() { + let t = TempDir::new("rename"); + let sub = t.path().join("sub"); + fs::create_dir_all(&sub).unwrap(); + let from = sub.join("old.md"); + write_file(&from, "body"); + + let to = rename(t.path(), &from, "new.md").unwrap(); + + assert_eq!(to.parent().unwrap().file_name().unwrap(), "sub"); + assert_eq!(to.file_name().unwrap(), "new.md"); + assert_eq!(fs::read_to_string(&to).unwrap(), "body"); + assert!(!from.exists()); + } + + #[test] + fn rename_refuses_to_clobber_a_different_file() { + let t = TempDir::new("rename-clobber"); + let from = t.path().join("a.md"); + let occupied = t.path().join("b.md"); + write_file(&from, "mine"); + write_file(&occupied, "theirs"); + + let err = rename(t.path(), &from, "b.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + assert_eq!(fs::read_to_string(&occupied).unwrap(), "theirs"); + assert_eq!( + fs::read_to_string(&from).unwrap(), + "mine", + "a refused rename must leave the source in place" + ); + } + + #[test] + fn renaming_to_the_same_name_is_a_no_op_not_a_collision() { + let t = TempDir::new("rename-same"); + let from = t.path().join("note.md"); + write_file(&from, "body"); + + let to = rename(t.path(), &from, "note.md").unwrap(); + + assert_eq!(fs::read_to_string(&to).unwrap(), "body"); + } + + #[test] + fn case_only_rename_is_allowed() { + // On a case-insensitive filesystem the destination "exists" — it is the + // same file. Must not be reported as a collision. + let t = TempDir::new("rename-case"); + let from = t.path().join("notes.md"); + write_file(&from, "body"); + + let to = rename(t.path(), &from, "Notes.md").unwrap(); + + assert_eq!(to.file_name().unwrap(), "Notes.md"); + assert_eq!(fs::read_to_string(&to).unwrap(), "body"); + } + + #[test] + fn rename_refuses_a_name_that_is_a_path() { + let t = TempDir::new("rename-path"); + let from = t.path().join("note.md"); + write_file(&from, "body"); + + assert!(rename(t.path(), &from, "../escaped.md").is_err()); + assert!(rename(t.path(), &from, "sub/escaped.md").is_err()); + assert!(from.exists()); + } + + #[test] + fn rename_refuses_a_source_outside_the_vault() { + let t = TempDir::new("rename-outside"); + let vault = t.path().join("vault"); + fs::create_dir_all(&vault).unwrap(); + let outside = t.path().join("outside.md"); + write_file(&outside, "not yours"); + + let err = rename(&vault, &outside, "taken.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert!(outside.exists()); + } + + #[test] + fn renames_a_folder_with_its_contents() { + let t = TempDir::new("rename-dir"); + let dir = t.path().join("old"); + write_file(&dir.join("a.md"), "a"); + write_file(&dir.join("deep").join("b.md"), "b"); + + let to = rename(t.path(), &dir, "new").unwrap(); + + assert_eq!(fs::read_to_string(to.join("a.md")).unwrap(), "a"); + assert_eq!(fs::read_to_string(to.join("deep/b.md")).unwrap(), "b"); + assert!(!dir.exists()); + } + + // ---- available_note_name ----------------------------------------------- + + #[test] + fn suggests_the_first_unused_name() { + let t = TempDir::new("suggest"); + let vault = t.path(); + assert_eq!(available_note_name(vault, vault, "Untitled"), "Untitled.md"); + + write_file(&vault.join("Untitled.md"), ""); + assert_eq!( + available_note_name(vault, vault, "Untitled"), + "Untitled 2.md" + ); + + write_file(&vault.join("Untitled 2.md"), ""); + assert_eq!( + available_note_name(vault, vault, "Untitled"), + "Untitled 3.md" + ); + } + + #[test] + fn suggestion_does_not_probe_outside_the_vault() { + let t = TempDir::new("suggest-outside"); + let vault = t.path().join("vault"); + let elsewhere = t.path().join("elsewhere"); + fs::create_dir_all(&vault).unwrap(); + fs::create_dir_all(&elsewhere).unwrap(); + write_file(&elsewhere.join("Untitled.md"), ""); + + // Would be `Untitled 2.md` if the existence check had run out there. + assert_eq!( + available_note_name(&vault, &elsewhere, "Untitled"), + "Untitled.md" + ); + } + + // ---- subtree walk / reroot --------------------------------------------- + + #[test] + fn lists_descendant_files_and_skips_hidden() { + let t = TempDir::new("walk"); + write_file(&t.path().join("a.md"), "a"); + write_file(&t.path().join("deep/b.md"), "b"); + write_file(&t.path().join(".trash/gone/c.md"), "c"); + write_file(&t.path().join(".hidden.md"), "h"); + + let files = descendant_files(t.path()); + + assert_eq!(files.len(), 2, "got {files:?}"); + assert!(files.iter().any(|p| p.ends_with("a.md"))); + assert!(files.iter().any(|p| p.ends_with("b.md"))); + } + + #[test] + fn descendant_files_of_a_missing_directory_is_empty_not_an_error() { + let t = TempDir::new("walk-missing"); + assert!(descendant_files(&t.path().join("nope")).is_empty()); + } + + #[test] + fn reroot_maps_a_subtree_path_and_refuses_an_unrelated_one() { + let old = Path::new("/vault/old"); + let new = Path::new("/vault/new"); + + assert_eq!( + reroot(Path::new("/vault/old/deep/b.md"), old, new), + Some(PathBuf::from("/vault/new/deep/b.md")) + ); + assert_eq!(reroot(Path::new("/vault/other/b.md"), old, new), None); + } +} diff --git a/src-tauri/crates/vaultscan/src/lib.rs b/src-tauri/crates/vaultscan/src/lib.rs index c6230e6..e5e386e 100644 --- a/src-tauri/crates/vaultscan/src/lib.rs +++ b/src-tauri/crates/vaultscan/src/lib.rs @@ -6,8 +6,11 @@ //! - Hidden entries (name starting with `.`) are skipped — this drops `.git`, //! `.obsidian`, etc. //! - Only `.md` / `.markdown` files are listed. -//! - A directory is included only if its subtree contains at least one markdown -//! file, so empty / asset-only folders don't clutter the tree. +//! - A directory is included if its subtree contains at least one markdown file +//! **or** it is empty, so asset-only folders don't clutter the tree but a +//! folder the user just made is still there. That second half is not a +//! refinement — without it, New Folder (ROADMAP II.12) creates a directory +//! that immediately disappears and cannot be put a note into. //! - Entries are sorted directories-first, then case-insensitive by name. //! //! No Tauri dependency: the walk is pure `std` + `serde` and fully unit-tested. @@ -46,7 +49,7 @@ fn scan_dir(dir: &Path) -> io::Result> { if file_type.is_dir() { let children = scan_dir(&path)?; - if !children.is_empty() { + if !children.is_empty() || is_visibly_empty(&path) { nodes.push(FileNode { name, path: path.to_string_lossy().into_owned(), @@ -72,6 +75,23 @@ fn scan_dir(dir: &Path) -> io::Result> { Ok(nodes) } +/// Whether `dir` holds nothing the user would see — no entries at all, or only +/// hidden ones. +/// +/// Distinguishes "you just made this" from "this is an assets folder". A folder +/// with a PNG in it stays pruned; a folder with nothing in it is shown, because +/// the alternative is New Folder appearing to do nothing. An unreadable +/// directory answers `false`: unknown is not empty, and guessing the other way +/// would put a folder in the tree that nothing can be done with. +fn is_visibly_empty(dir: &Path) -> bool { + match fs::read_dir(dir) { + Ok(entries) => !entries + .flatten() + .any(|e| !e.file_name().to_string_lossy().starts_with('.')), + Err(_) => false, + } +} + fn is_markdown(name: &str) -> bool { let lower = name.to_lowercase(); lower.ends_with(".md") || lower.ends_with(".markdown") @@ -139,6 +159,24 @@ mod tests { assert!(file.path.ends_with("b-note.md")); } + /// A folder the user just made has nothing in it yet. Pruning it would make + /// New Folder (ROADMAP II.12) look like it had failed, and leave nowhere to + /// put the first note. + #[test] + fn keeps_an_empty_directory_but_still_prunes_an_asset_only_one() { + let t = TempDir::new(); + let root = &t.0; + touch(&root.join("note.md")); + fs::create_dir_all(root.join("Brand new")).unwrap(); + touch(&root.join("assets/pic.png")); + // Only hidden entries: nothing the user can see, so it counts as empty. + touch(&root.join("quiet/.keep")); + + let names: Vec = scan(root).unwrap().into_iter().map(|n| n.name).collect(); + + assert_eq!(names, ["Brand new", "quiet", "note.md"]); + } + #[test] fn empty_folder_yields_empty_tree() { let t = TempDir::new(); diff --git a/src-tauri/src/commands/entries.rs b/src-tauri/src/commands/entries.rs new file mode 100644 index 0000000..1664e55 --- /dev/null +++ b/src-tauri/src/commands/entries.rs @@ -0,0 +1,114 @@ +//! Workspace entry commands (CLAUDE.md §5, ROADMAP Movement II.12): create a +//! note, create a folder, rename either. +//! +//! Thin wrappers over the `fileops` crate (aliased `ops`, the `commands/snapshots.rs` +//! precedent) — the rules that make these safe live there and are unit-tested +//! without Tauri: name validation, vault containment, and a refusal to clobber. +//! Deleting is not here; it is `commands/trash.rs`, which soft-deletes and can +//! restore. +//! +//! The one piece of real logic in this file is **carrying version history across +//! a rename**. The snapshot store is keyed by a note's absolute path, so a rename +//! would otherwise orphan every version of that note behind a path nothing points +//! at any more. `snapshots::rekey` exists for exactly this and has had no caller +//! until now. It is applied **best-effort and additively** (§3), matching how +//! snapshots are taken on save: the rename already succeeded on disk, and history +//! failing to follow must not turn it into a reported failure — the user would be +//! left with a completed rename and an error message, unable to tell what state +//! anything is in. + +use std::path::{Path, PathBuf}; + +use fileops as ops; +use tauri::AppHandle; + +fn as_string(path: PathBuf) -> String { + path.to_string_lossy().into_owned() +} + +/// Create an empty note called `name` inside `dir`, and return its path. +/// +/// `name` may omit the extension (`.md` is added). Refuses to overwrite an +/// existing file and refuses a `dir` outside `vault_root`. +#[tauri::command] +pub fn create_note(vault_root: String, dir: String, name: String) -> Result { + ops::create_note(Path::new(&vault_root), Path::new(&dir), &name) + .map(as_string) + .map_err(|e| e.to_string()) +} + +/// Create a folder called `name` inside `parent`, and return its path. +#[tauri::command] +pub fn create_folder(vault_root: String, parent: String, name: String) -> Result { + ops::create_folder(Path::new(&vault_root), Path::new(&parent), &name) + .map(as_string) + .map_err(|e| e.to_string()) +} + +/// A name the caller can safely propose for a new note in `dir` (`Untitled.md`, +/// `Untitled 2.md`, …). +/// +/// A suggestion only — `create_note` still refuses a collision, because the user +/// edits the field before confirming and a sync daemon can land a file in +/// between. Its job is to stop New Note twice in a row from proposing a name +/// that is already taken. +#[tauri::command] +pub fn suggest_note_name(vault_root: String, dir: String, stem: String) -> Result { + Ok(ops::available_note_name( + Path::new(&vault_root), + Path::new(&dir), + &stem, + )) +} + +/// Rename the file or folder at `path` to `new_name`, within the same parent. +/// +/// Returns the new path. Version history follows the note (or, for a folder, +/// every note beneath it) — see the module docs for why that is best-effort. +#[tauri::command] +pub fn rename_entry( + app: AppHandle, + vault_root: String, + path: String, + new_name: String, +) -> Result { + let from = Path::new(&path); + let was_dir = from.is_dir(); + // Taken *before* the move: after it, the old tree no longer exists to walk. + let old_files = if was_dir { + ops::descendant_files(from) + } else { + Vec::new() + }; + + let to = ops::rename(Path::new(&vault_root), from, &new_name).map_err(|e| e.to_string())?; + + // The rename is done. Everything below is additive (§3) and cannot fail it. + // + // `fileops` returns paths in the caller's own spelling, so the pre-move file + // list (built by walking `from`) and the new root (`to`) share a prefix and + // `reroot` pairs them directly — no canonicalization to reconcile. + if was_dir { + for old_file in old_files { + let Some(new_file) = ops::reroot(&old_file, from, &to) else { + continue; + }; + carry_history(&app, &as_string(old_file), &as_string(new_file)); + } + } else { + carry_history(&app, &path, &as_string(to.clone())); + } + + Ok(as_string(to)) +} + +/// Move one note's version history from `old_path` to `new_path`. Logged and +/// swallowed on failure — see the module docs. +fn carry_history(app: &AppHandle, old_path: &str, new_path: &str) { + let Ok(root) = super::snapshots::history_root(app) else { + return; + }; + if let Err(e) = snapshots::rekey(&root, old_path, new_path) { + eprintln!("version-history: rekey failed for {old_path} → {new_path}: {e}"); + } +} diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index 2d31541..a636061 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -1,3 +1,4 @@ +pub mod entries; pub mod export; pub mod files; pub mod images; diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 12851c1..d3addb9 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -84,6 +84,10 @@ pub fn run() { commands::recovery::save_recovery, commands::recovery::load_recovery, commands::recovery::clear_recovery, + commands::entries::create_note, + commands::entries::create_folder, + commands::entries::suggest_note_name, + commands::entries::rename_entry, commands::trash::move_to_trash, commands::trash::list_trash, commands::trash::restore_from_trash, diff --git a/src/ipc.ts b/src/ipc.ts index 76d5175..dcb0fc3 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -68,6 +68,82 @@ export function openFolder(path: string): Promise { return invoke("open_folder", { path }); } +/** + * Create an empty note called `name` inside `dir` (§5, ROADMAP II.12). + * + * `name` may omit the extension — Rust adds `.md`. Rejects, rather than + * overwrites, an existing file, and rejects a `dir` outside `vaultRoot`; both + * rules live in `crates/fileops` so there is one place they are enforced and + * one place they are tested. Resolves to the new note's path. + */ +export function createNote(vaultRoot: string, dir: string, name: string): Promise { + return invoke("create_note", { vaultRoot, dir, name }); +} + +/** Create a folder called `name` inside `parent`. Resolves to its path. */ +export function createFolder(vaultRoot: string, parent: string, name: string): Promise { + return invoke("create_folder", { vaultRoot, parent, name }); +} + +/** + * A note name not currently taken in `dir` — `Untitled.md`, `Untitled 2.md`, … + * + * Only a suggestion for the inline field's initial value; {@link createNote} + * still refuses a collision. + */ +export function suggestNoteName( + vaultRoot: string, + dir: string, + stem: string, +): Promise { + return invoke("suggest_note_name", { vaultRoot, dir, stem }); +} + +/** + * Rename the file or folder at `path` to `newName`, in the same parent. + * + * Resolves to the new path. Version history follows the note (or every note + * under a renamed folder) via `snapshots::rekey`, best-effort — see + * `commands/entries.rs`. Refuses to overwrite a different existing entry. + */ +export function renameEntry( + vaultRoot: string, + path: string, + newName: string, +): Promise { + return invoke("rename_entry", { vaultRoot, path, newName }); +} + +/** A soft-deleted item in the workspace trash (mirrors Rust `trashbin::TrashEntry`). */ +export interface TrashEntry { + id: string; + original_path: string; + name: string; + deleted_at: number; +} + +/** + * Soft-delete `path` into `/.trash/` (§3). Resolves to the entry, + * whose `id` is what {@link restoreFromTrash} takes — which is what makes the + * delete undoable rather than merely recoverable-if-you-know-where-to-look. + */ +export function moveToTrash(vaultRoot: string, path: string): Promise { + return invoke("move_to_trash", { vaultRoot, path }); +} + +/** The workspace trash, newest first. */ +export function listTrash(vaultRoot: string): Promise { + return invoke("list_trash", { vaultRoot }); +} + +/** + * Restore trash entry `id` to its original path. Resolves to that path; rejects + * (without clobbering) if a file has since reappeared there. + */ +export function restoreFromTrash(vaultRoot: string, id: string): Promise { + return invoke("restore_from_trash", { vaultRoot, id }); +} + /** Start watching `path` for external changes; events arrive via {@link onWorkspaceChange}. */ export function watchFolder(path: string): Promise { return invoke("watch_folder", { path }); @@ -138,6 +214,19 @@ export async function installCloseGuard( }); } +/** + * Ask the user to confirm an action that would discard unsaved work. + * + * Native, like the close guard's prompt and for the same reason: this is the + * question that must not be missable, and an in-page banner can be scrolled + * past or ignored. It is deliberately the *only* native dialog the file + * operations use — every other decision they need is answered inline in the + * sidebar, where it can be driven by the headless gates. + */ +export function confirmDiscard(question: string): Promise { + return ask(question, { title: "Toril", kind: "warning" }); +} + /** Show the native "About Toril" dialog (Help menu). */ export async function showAbout(): Promise { let version = ""; diff --git a/src/main.ts b/src/main.ts index 1bff961..d36a363 100644 --- a/src/main.ts +++ b/src/main.ts @@ -19,10 +19,14 @@ import { type WorkspaceChange, checkForUpdate, clearRecovery, + confirmDiscard, + createFolder, + createNote, exportHtml, exportRtf, installCloseGuard, listHistory, + moveToTrash, loadRecovery, loadSettings, markdownToHtml, @@ -39,6 +43,8 @@ import { pickFolder, readSnapshot, relaunchApp, + renameEntry, + restoreFromTrash, restoreSnapshot, saveClipboardImage, saveFile, @@ -47,7 +53,9 @@ import { saveSettings, setRecentFiles, showAbout, + suggestNoteName, takeLaunchPath, + type TrashEntry, watchFolder, writeConflictCopy, } from "./ipc"; @@ -178,6 +186,31 @@ function setStatus(msg: string): void { if (el) el.textContent = msg; } +/** + * A status message with one inline action — currently Undo, after a delete. + * + * It lives in the status bar rather than a toast because it must not steal + * focus or cover anything: the user just deleted a file and may be about to + * click another. The next `setStatus` clears it, which is the dismissal — and + * missing the button costs nothing, since the file is sitting in `.trash/` + * either way. This is an affordance for the common case, not the safety net; + * the safety net is that the delete was never destructive. + */ +function setStatusWithAction(msg: string, label: string, onAction: () => void): void { + const el = document.querySelector("#status"); + if (!el) return; + el.textContent = `${msg} `; + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "status-action"; + btn.textContent = label; + btn.addEventListener("click", () => { + setStatus(""); // one action, one use — a second click would restore twice + onAction(); + }); + el.append(btn); +} + function updateTitle(): void { const tab = tabs.active(); const shown = tab ? `${tab.name}${tab.dirty ? " *" : ""}` : "Toril"; @@ -357,7 +390,7 @@ async function doOpenFolder(): Promise { async function loadWorkspace(path: string): Promise { const tree = await openFolder(path); workspaceRoot = path; - sidebar.setRoot(basename(path), tree); + sidebar.setRoot(basename(path), tree, path); sidebar.setActivePath(tabs.active()?.path ?? null); setStatus(`Opened folder ${basename(path)}`); @@ -571,6 +604,167 @@ function doNew(): void { setStatus("New document"); } +// ---- Workspace file operations (ROADMAP Movement II.12) --------------------- +// +// The rules live in Rust (`crates/fileops` validates and refuses to clobber; +// `trashbin` soft-deletes) and the UX lives in `ui/sidebar.ts`. What is here is +// the part neither of them can own: keeping the *open tabs* consistent with +// what just happened on disk. +// +// That is the §3 surface of this feature. A rename moves the file out from under +// every tab pointing into it, and the watcher reports it as a delete followed by +// a create — the exact shape `removedOnDisk` exists to catch. Left alone, the +// tab would decide its file had vanished and offer to recreate it, resurrecting +// the old note beside its new name. + +/** + * Create a note and open it. + * + * Opening is the point: creating a file the user then has to find in the tree + * is a worse New Note than the Untitled buffer they already have. Rejections + * propagate — the sidebar keeps the name field open and shows the message. + */ +async function doCreateNote(dir: string, name: string): Promise { + if (!workspaceRoot) throw new Error("No folder is open."); + const path = await createNote(workspaceRoot, dir, name); + scheduleSidebarRefresh(); + // The file is empty and was just written by us, so there is nothing to read + // back: open the buffer directly rather than paying a round trip to learn it + // is empty. `base` is "" for the same reason, which is exactly right — that + // is what is on disk. + openDocument(path, basename(path), "", formatForPath(path)); + rememberRecent(path); + scheduleSessionSave(); + setStatus(`Created ${basename(path)}`); +} + +async function doCreateFolder(parent: string, name: string): Promise { + if (!workspaceRoot) throw new Error("No folder is open."); + await createFolder(workspaceRoot, parent, name); + scheduleSidebarRefresh(); + setStatus(`Created ${name}`); +} + +/** + * Rename a note or folder, and move every affected tab with it. + * + * Three things have to happen together, and the order matters: + * + * 1. **Re-point the tabs**, including every tab *underneath* a renamed folder. + * 2. **Bump the removal epoch** for each old path. A `reconcile` may already be + * in flight against the old path; it captured that path before its await and + * would apply its verdict — "missing", most likely — to a tab that has since + * moved. This is the same guard the watcher's remove branch uses. + * 3. **Clear `removedOnDisk`**, because the watcher's delete event for the old + * path may have landed while the rename was in flight. Steps 1–3 run in one + * synchronous block after the await, so no event can interleave between them. + * + * `base` is deliberately *not* reset: a rename does not change a single byte, so + * the merge base is still exactly right and re-reading would only invite a race. + */ +async function doRenameEntry(path: string, newName: string): Promise { + if (!workspaceRoot) throw new Error("No folder is open."); + const newPath = await renameEntry(workspaceRoot, path, newName); + + for (const tab of tabs.list()) { + const old = tab.path; + if (!old) continue; + // Exact match is the file case; the prefix branch is the folder case. The + // prefix is a raw slice because both strings come from the same scan of the + // same tree, so they share a spelling — `isAtOrUnder` is the predicate, + // `startsWith` is what makes the arithmetic safe. + let moved: string | null = null; + if (old === path) moved = newPath; + else if (isAtOrUnder(old, path) && old.startsWith(path)) moved = newPath + old.slice(path.length); + if (moved === null) continue; + + // A queued reconcile for the old path would read a file that no longer + // exists; cancel it as well as invalidating one already running. + const queued = reconcileTimers.get(old); + if (queued) { + clearTimeout(queued); + reconcileTimers.delete(old); + } + removalEpoch.set(old, (removalEpoch.get(old) ?? 0) + 1); + + tabs.setPath(tab.id, moved, basename(moved)); + tabs.setRemovedOnDisk(tab.id, false); + dropRecent(old); + rememberRecent(moved); + } + + scheduleSidebarRefresh(); + updateTitle(); + renderConflictBar(); + scheduleSessionSave(); // the session's open-file paths just changed + setStatus(`Renamed to ${basename(newPath)}`); +} + +/** + * Soft-delete a note or folder, and offer to undo it. + * + * The delete is reversible by construction — `trashbin` moves the file into the + * workspace `.trash/` rather than unlinking it — so this asks nothing in the + * ordinary case, and offers Undo instead. The one thing trash cannot bring back + * is an *unsaved buffer*, so that is the one case that stops and asks. + */ +async function doDeleteEntry(path: string, isDir: boolean): Promise { + if (!workspaceRoot) return; + const affected = tabs + .list() + .filter((t) => t.path !== null && (isDir ? isAtOrUnder(t.path, path) : t.path === path)); + + const unsaved = affected.filter((t) => t.dirty); + if (unsaved.length > 0) { + const names = unsaved.map((t) => t.name).join(", "); + const verb = unsaved.length === 1 ? "has" : "have"; + const ok = await confirmDiscard( + `${names} ${verb} unsaved changes that trash cannot bring back. Delete anyway?`, + ); + if (!ok) return; + } + + let entry: TrashEntry; + try { + entry = await moveToTrash(workspaceRoot, path); + } catch (e) { + setStatus(`Delete failed: ${String(e)}`); + return; + } + + // Closed only after the move succeeded — a failed delete must not also cost + // the user their open tab. + for (const tab of affected) { + if (tab.path) { + removalEpoch.set(tab.path, (removalEpoch.get(tab.path) ?? 0) + 1); + // Offering a deleted note in Open Recent is offering a failure; restoring + // it from trash re-opens it and puts it back at the front. + dropRecent(tab.path); + } + tabs.close(tab.id); + } + if (!isDir) dropRecent(path); // deleted without ever being opened + scheduleSidebarRefresh(); + updateTitle(); + renderConflictBar(); + scheduleSessionSave(); + setStatusWithAction(`Moved ${entry.name} to trash`, "Undo", () => void undoDelete(entry)); +} + +async function undoDelete(entry: TrashEntry): Promise { + if (!workspaceRoot) return; + try { + const path = await restoreFromTrash(workspaceRoot, entry.id); + scheduleSidebarRefresh(); + setStatus(`Restored ${basename(path)}`); + } catch (e) { + // `restore` refuses rather than clobbers when something has reappeared at + // the path, so the file is still safe in `.trash/` — say so, because the + // difference between "not restored" and "lost" is the whole point. + setStatus(`Could not restore ${entry.name} — it is still in .trash (${String(e)})`); + } +} + // ---- Pane layout (sidebar + tabbed rail) ----------------------------------- // // All pane decisions live in `panes.ts` as value transforms; this half only @@ -1051,7 +1245,7 @@ function scheduleSidebarRefresh(): void { if (!workspaceRoot) return; openFolder(workspaceRoot) .then((tree) => { - sidebar.setRoot(basename(workspaceRoot!), tree); + sidebar.setRoot(basename(workspaceRoot!), tree, workspaceRoot); sidebar.setActivePath(tabs.active()?.path ?? null); }) .catch(() => {}); @@ -1663,6 +1857,15 @@ window.addEventListener("DOMContentLoaded", async () => { // Routed through the same action as the menu item, so the empty state's // button cannot drift from File → Open Folder. onOpenFolder: () => runAction("menu_open_folder"), + // These three deliberately do **not** catch: the sidebar keeps its name + // field open on a rejection and shows the message beside it, which is where + // a bad name belongs. Swallowing the error here would leave the field + // looking like it had worked. + onCreateNote: (dir, name) => doCreateNote(dir, name), + onCreateFolder: (parent, name) => doCreateFolder(parent, name), + onRename: (path, newName) => doRenameEntry(path, newName), + onDelete: (path, isDir) => void doDeleteEntry(path, isDir), + suggestName: (dir) => suggestNoteName(workspaceRoot ?? dir, dir, "Untitled"), }); sidebar.setRoot(null, []); tabs = new TabManager(tabbar, { onDeactivate, onActivate, onCloseRequest }); diff --git a/src/styles/chrome.css b/src/styles/chrome.css index 4afe0b1..422b577 100644 --- a/src/styles/chrome.css +++ b/src/styles/chrome.css @@ -206,6 +206,9 @@ body { } .sidebar-root { + display: flex; + align-items: center; + gap: var(--sp-1); font-weight: 600; padding: var(--sp-1) var(--sp-1) var(--sp-2); text-transform: uppercase; @@ -214,6 +217,118 @@ body { color: var(--dim); } +/* The folder name yields to the buttons rather than pushing them out of the + pane — `min-width: 0` is what lets it ellipsize (§12b rule 3). */ +.sidebar-root-name { + flex: 1; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.sidebar-root-btn { + flex: none; + display: flex; + align-items: center; + justify-content: center; + min-width: var(--target-sm); + min-height: var(--target-sm); + padding: 0; + border: 0; + border-radius: var(--r-sm); + background: none; + color: var(--muted); + cursor: pointer; + font: inherit; + font-size: var(--text-md); + line-height: 1; +} + +.sidebar-root-btn:hover { + background: var(--hover-bg); + color: var(--fg); +} + +/* ---- Inline name field (new note / new folder / rename) ------------------- */ +.sidebar-edit { + display: flex; + flex-direction: column; + gap: var(--sp-1); + padding: var(--sp-1) 0; + min-width: 0; +} + +.sidebar-edit-input { + min-height: var(--target-sm); + width: 100%; + min-width: 0; + padding: 0 var(--sp-1); + border: 1px solid var(--accent); + border-radius: var(--r-sm); + background: var(--chrome-raised); + color: var(--fg); + font: inherit; +} + +/* The message the backend refused the name with. `anywhere` because it can + quote a name the user typed, which may be one long unbroken string (§12b + rule 4). */ +.sidebar-edit-error { + font-size: var(--text-xs); + color: var(--danger); + overflow-wrap: anywhere; +} + +/* ---- Context menu -------------------------------------------------------- */ +.context-menu { + position: fixed; + z-index: var(--z-dialog); + min-width: 10rem; + padding: var(--sp-1); + border: 1px solid var(--border-strong); + border-radius: var(--r-md); + background: var(--chrome-raised); + box-shadow: var(--shadow-panel); + font-size: var(--text-sm); +} + +.context-menu-item { + display: flex; + align-items: center; + width: 100%; + min-height: var(--target); + padding: 0 var(--sp-2); + border: 0; + border-radius: var(--r-sm); + background: none; + color: var(--fg); + cursor: pointer; + font: inherit; + text-align: left; + white-space: nowrap; +} + +.context-menu-item:hover:not(:disabled), +.context-menu-item:focus-visible { + background: var(--hover-bg); +} + +.context-menu-item[data-danger="true"] { + color: var(--danger); +} + +.context-menu-item:disabled { + color: var(--dim); + cursor: default; +} + +.context-menu-sep { + height: 1px; + margin: var(--sp-1) 0; + background: var(--border); +} + #sidebar .tree { list-style: none; margin: 0; @@ -1057,6 +1172,23 @@ body { border-top: 1px solid var(--border); } +/* The inline action a status message can carry (Undo, after a delete). Styled + as a link rather than a button: it sits inside a sentence, and the status bar + is 1.75rem tall — a bordered control would not fit without growing it. */ +.status-action { + padding: 0; + border: 0; + background: none; + color: var(--accent); + cursor: pointer; + font: inherit; + text-decoration: underline; +} + +.status-action:hover { + color: var(--fg); +} + .status-right { display: flex; align-items: center; diff --git a/src/ui/contextmenu.ts b/src/ui/contextmenu.ts new file mode 100644 index 0000000..f3c1cd9 --- /dev/null +++ b/src/ui/contextmenu.ts @@ -0,0 +1,198 @@ +// A small in-app context menu (ROADMAP Movement II.12, `feat/sidebar-file-ops`). +// +// **Why not a native menu.** `menu.rs` can build one, and the app menu already +// does — but a native popup cannot be driven by the headless gates, and native +// dialogs are the one thing documented to hang the app on the Linux dev box +// (`docs/ON-DEVICE-VERIFICATION.md`). A DOM menu is testable in jsdom, renders +// identically in both webview engines, and keeps the file operations reachable +// on the box where they are actually developed. +// +// Exactly one menu exists at a time: opening a second closes the first. That is +// enforced here rather than by callers, because a stray menu left behind would +// keep a document-level dismiss listener alive and swallow the next click. + +/** A selectable row. `danger` styles a destructive action (delete). */ +export interface MenuItem { + label: string; + onSelect(): void; + danger?: boolean; + /** Rendered but unselectable, with the reason as its tooltip. */ + disabled?: boolean; + title?: string; +} + +/** A rule between groups. Never focusable — it is decoration, not a row. */ +export type MenuEntry = MenuItem | "separator"; + +export interface ContextMenuOptions { + /** Viewport coordinates, normally from the triggering pointer event. */ + x: number; + y: number; + entries: MenuEntry[]; + /** Where to mount. Defaults to `document.body`. */ + host?: HTMLElement; +} + +let openMenu: HTMLElement | null = null; +let dismiss: (() => void) | null = null; +/** + * Where focus was before the menu opened. + * + * Restored on close, and it matters more here than in most menus: the sidebar + * row that opened this is the thing a keyboard user was navigating, and + * dropping focus to `` would send them back to the top of the document + * every time they pressed Escape. + */ +let returnFocusTo: HTMLElement | null = null; + +/** Close any open context menu. Safe to call when none is open. */ +export function closeContextMenu(): void { + dismiss?.(); +} + +function isItem(entry: MenuEntry): entry is MenuItem { + return entry !== "separator"; +} + +/** + * Open a context menu at `(x, y)`. + * + * Selecting an item closes the menu **before** running its handler: handlers + * open inline editors and move focus, and a menu still on screen would fight + * them for it. + */ +export function openContextMenu(opts: ContextMenuOptions): void { + closeContextMenu(); + + const host = opts.host ?? document.body; + const active = document.activeElement; + returnFocusTo = active instanceof HTMLElement ? active : null; + + const menu = document.createElement("div"); + menu.className = "context-menu"; + menu.setAttribute("role", "menu"); + menu.style.left = `${Math.max(0, opts.x)}px`; + menu.style.top = `${Math.max(0, opts.y)}px`; + + const buttons: HTMLButtonElement[] = []; + for (const entry of opts.entries) { + if (!isItem(entry)) { + const rule = document.createElement("div"); + rule.className = "context-menu-sep"; + // Decorative: exposing it as a menu row would make screen readers count + // separators among the choices. + rule.setAttribute("role", "none"); + menu.append(rule); + continue; + } + + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "context-menu-item"; + btn.setAttribute("role", "menuitem"); + btn.textContent = entry.label; + if (entry.danger) btn.dataset.danger = "true"; + if (entry.title) btn.title = entry.title; + if (entry.disabled) { + btn.disabled = true; + } else { + btn.addEventListener("click", () => { + close(); + entry.onSelect(); + }); + buttons.push(btn); + } + menu.append(btn); + } + + function close(restoreFocus = true): void { + document.removeEventListener("pointerdown", onPointerDown, true); + document.removeEventListener("keydown", onKeyDown, true); + window.removeEventListener("resize", onDismissEvent); + window.removeEventListener("blur", onDismissEvent); + menu.remove(); + if (openMenu === menu) { + openMenu = null; + dismiss = null; + } + // Only when the menu still owned focus. A pointer dismiss has already moved + // focus somewhere the user chose, and yanking it back would undo that. + if (restoreFocus && returnFocusTo?.isConnected) returnFocusTo.focus(); + returnFocusTo = null; + } + + function onPointerDown(e: PointerEvent): void { + if (e.target instanceof Node && menu.contains(e.target)) return; + close(false); + } + + function onDismissEvent(): void { + close(false); + } + + function onKeyDown(e: KeyboardEvent): void { + if (e.key === "Escape") { + e.preventDefault(); + e.stopPropagation(); + close(); + return; + } + // Tab out of a menu means "I'm done here" everywhere else; closing without + // swallowing the key lets focus move on naturally. + if (e.key === "Tab") { + close(false); + return; + } + if (buttons.length === 0) return; + + const current = buttons.indexOf(document.activeElement as HTMLButtonElement); + let next: number | null = null; + if (e.key === "ArrowDown") next = current < 0 ? 0 : (current + 1) % buttons.length; + else if (e.key === "ArrowUp") + next = current < 0 ? buttons.length - 1 : (current - 1 + buttons.length) % buttons.length; + else if (e.key === "Home") next = 0; + else if (e.key === "End") next = buttons.length - 1; + + if (next !== null) { + e.preventDefault(); + e.stopPropagation(); + buttons[next]?.focus(); + } + } + + host.append(menu); + openMenu = menu; + dismiss = () => close(); + + // Keep the menu on screen. Measured after mounting because the width is not + // known until the labels are laid out; `getBoundingClientRect` reports zeroes + // under jsdom, and the clamp is written to be a no-op in that case rather + // than to produce a negative offset. + const rect = menu.getBoundingClientRect(); + if (rect.width > 0 && opts.x + rect.width > window.innerWidth) { + menu.style.left = `${Math.max(0, window.innerWidth - rect.width)}px`; + } + if (rect.height > 0 && opts.y + rect.height > window.innerHeight) { + menu.style.top = `${Math.max(0, window.innerHeight - rect.height)}px`; + } + + // Capture phase on the document pair: the sidebar, the editor and the global + // keymap all listen at the document level, and an open menu must take the key + // or the click before they act on it. + document.addEventListener("pointerdown", onPointerDown, true); + document.addEventListener("keydown", onKeyDown, true); + // **Not** capture on the window pair, and `blur` is why. `blur` does not + // bubble, but a capturing window listener still sees every element's blur on + // the way down — so the menu would dismiss itself the instant it focused its + // own first item, taking whatever the user was about to click with it. Without + // capture this is what it reads as: the *window* losing focus. + window.addEventListener("resize", onDismissEvent); + window.addEventListener("blur", onDismissEvent); + + buttons[0]?.focus(); +} + +/** Whether a context menu is currently open (for tests and guards). */ +export function isContextMenuOpen(): boolean { + return openMenu !== null; +} diff --git a/src/ui/sidebar.ts b/src/ui/sidebar.ts index 32a98ba..79364c0 100644 --- a/src/ui/sidebar.ts +++ b/src/ui/sidebar.ts @@ -1,8 +1,25 @@ // Workspace file tree (CLAUDE.md §4). Renders the FileNode[] from `open_folder` // as a collapsible tree; clicking a markdown file asks the controller to open // it. Folders use native
for zero-JS collapse. +// +// File operations (ROADMAP Movement II.12) live here as *UX only*: the context +// menu, the inline name field, and where an error message lands. Every rule that +// decides whether an operation is allowed — name validation, vault containment, +// refusing to clobber — is in `crates/fileops`, and every disk write is a Rust +// command (§10). Nothing in this file may be the only thing standing between a +// bad name and the filesystem. +import { openContextMenu, type MenuEntry } from "./contextmenu"; import type { FileNode } from "../ipc"; +/** + * Callbacks the controller supplies. The file-operation ones are **optional**, + * and the menu offers only what is wired — the same rule the empty state already + * follows: a dead control that looks live is worse than an absent one. + * + * The operation callbacks may reject. The sidebar catches, shows the message + * beside the still-open name field, and lets the user correct it — so a rejected + * name costs a keystroke rather than the whole interaction. + */ export interface SidebarCallbacks { onOpenFile(path: string): void; /** @@ -13,68 +30,182 @@ export interface SidebarCallbacks { * is worse than a sentence. */ onOpenFolder?(): void; + /** Create an empty note called `name` in `dir`. */ + onCreateNote?(dir: string, name: string): Promise; + /** Create a folder called `name` in `parent`. */ + onCreateFolder?(parent: string, name: string): Promise; + /** Rename the entry at `path` to `newName`, in the same parent. */ + onRename?(path: string, newName: string): Promise; + /** Delete (soft, to trash) the entry at `path`. */ + onDelete?(path: string, isDir: boolean): void; + /** A note name not already taken in `dir`, to pre-fill the field with. */ + suggestName?(dir: string): Promise; +} + +/** What the open inline field is for. */ +type EditKind = "note" | "folder" | "rename"; + +interface EditState { + kind: EditKind; + /** Container the new entry goes in (create), or the entry's parent (rename). */ + dir: string; + /** The entry being renamed. Absent for creates. */ + target?: { path: string; name: string }; + /** Current field text, so a re-open after a rejected name keeps it. */ + value: string; + error: string | null; } export class Sidebar { + private rootName: string | null = null; + private rootPath: string | null = null; + private tree: FileNode[] = []; + private edit: EditState | null = null; + /** + * A tree that arrived while a name was being typed. + * + * The controller refreshes on every watcher event, and a sync daemon touching + * an unrelated file mid-rename would otherwise re-render the field out from + * under the user, losing what they had typed. Deferring is safe because the + * delay is bounded by the edit: confirming or cancelling flushes immediately, + * and the operation itself triggers a fresh refresh straight after. + */ + private pendingTree: { rootName: string | null; rootPath: string | null; tree: FileNode[] } | null = + null; + private activePath: string | null = null; + constructor( private readonly container: HTMLElement, private readonly cb: SidebarCallbacks, ) {} - /** Render a workspace, or an empty-state hint when no folder is open. */ - setRoot(rootName: string | null, tree: FileNode[]): void { - this.container.replaceChildren(); + /** + * Render a workspace, or an empty-state hint when no folder is open. + * + * `rootPath` is what the file operations need — a new note has to be created + * *somewhere*, and the root heading is a legitimate target. + */ + setRoot(rootName: string | null, tree: FileNode[], rootPath: string | null = null): void { + if (this.edit) { + this.pendingTree = { rootName, rootPath, tree }; + return; + } + this.rootName = rootName; + this.rootPath = rootPath; + this.tree = tree; + this.render(); + } - if (rootName === null) { - // An empty state that only *names* the emptiness leaves the user to find - // the menu; the fix for "no folder open" belongs next to the message. - const wrap = document.createElement("div"); - wrap.className = "sidebar-empty"; - - const hint = document.createElement("p"); - hint.className = "sidebar-empty-text"; - hint.textContent = "No folder open."; - wrap.append(hint); - - if (this.cb.onOpenFolder) { - const btn = document.createElement("button"); - btn.type = "button"; - btn.className = "sidebar-empty-btn"; - btn.textContent = "Open Folder…"; - btn.addEventListener("click", () => this.cb.onOpenFolder?.()); - wrap.append(btn); - - const note = document.createElement("p"); - note.className = "sidebar-empty-note"; - // Worth saying once, here: the folder is the user's, not Toril's. It is - // the single most load-bearing promise in §1 and the empty state is the - // moment someone is deciding whether to point it at a real vault. - note.textContent = - "Pick any folder of notes — including an Obsidian vault. Files stay plain Markdown where they are."; - wrap.append(note); - } + /** Highlight the currently active file by path (no-op if not in the tree). */ + setActivePath(path: string | null): void { + this.activePath = path; + this.applyActive(); + } + + /** Whether an inline name field is currently open (renders are deferred while it is). */ + isEditing(): boolean { + return this.edit !== null; + } + + private applyActive(): void { + for (const el of this.container.querySelectorAll(".file-entry")) { + el.dataset.active = String(el.dataset.path === this.activePath); + } + } - this.container.append(wrap); + private render(): void { + this.container.replaceChildren(); + + if (this.rootName === null) { + this.container.append(this.renderEmptyState()); return; } + this.container.append(this.renderHeading(this.rootName)); + this.container.append(this.renderNodes(this.tree, this.rootPath)); + this.mountEditor(); + this.applyActive(); + } + + private renderEmptyState(): HTMLElement { + // An empty state that only *names* the emptiness leaves the user to find + // the menu; the fix for "no folder open" belongs next to the message. + const wrap = document.createElement("div"); + wrap.className = "sidebar-empty"; + + const hint = document.createElement("p"); + hint.className = "sidebar-empty-text"; + hint.textContent = "No folder open."; + wrap.append(hint); + + if (this.cb.onOpenFolder) { + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "sidebar-empty-btn"; + btn.textContent = "Open Folder…"; + btn.addEventListener("click", () => this.cb.onOpenFolder?.()); + wrap.append(btn); + + const note = document.createElement("p"); + note.className = "sidebar-empty-note"; + // Worth saying once, here: the folder is the user's, not Toril's. It is + // the single most load-bearing promise in §1 and the empty state is the + // moment someone is deciding whether to point it at a real vault. + note.textContent = + "Pick any folder of notes — including an Obsidian vault. Files stay plain Markdown where they are."; + wrap.append(note); + } + return wrap; + } + + private renderHeading(rootName: string): HTMLElement { const heading = document.createElement("div"); heading.className = "sidebar-root"; - heading.textContent = rootName; - this.container.append(heading); - this.container.append(this.renderNodes(tree)); - } - /** Highlight the currently active file by path (no-op if not in the tree). */ - setActivePath(path: string | null): void { - for (const el of this.container.querySelectorAll(".file-entry")) { - el.dataset.active = String(el.dataset.path === path); + const label = document.createElement("span"); + label.className = "sidebar-root-name"; + label.textContent = rootName; + heading.append(label); + + const root = this.rootPath; + if (root !== null) { + // Buttons as well as the context menu: right-click is invisible, and a + // brand-new note is the one operation someone needs on their first minute + // in a folder. + if (this.cb.onCreateNote) { + heading.append( + this.iconButton("New note", "+", () => this.beginEdit({ kind: "note", dir: root })), + ); + } + if (this.cb.onCreateFolder) { + heading.append( + this.iconButton("New folder", "🗀", () => this.beginEdit({ kind: "folder", dir: root })), + ); + } + heading.addEventListener("contextmenu", (e) => this.openMenuForFolder(e, root, null)); } + return heading; + } + + private iconButton(label: string, glyph: string, onClick: () => void): HTMLButtonElement { + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "sidebar-root-btn"; + btn.title = label; + btn.setAttribute("aria-label", label); + // The glyph is decorative — the accessible name comes from aria-label, and + // announcing "plus sign" after it would be noise. + btn.textContent = glyph; + btn.addEventListener("click", onClick); + return btn; } - private renderNodes(nodes: FileNode[]): HTMLElement { + private renderNodes(nodes: FileNode[], dir: string | null): HTMLElement { const ul = document.createElement("ul"); ul.className = "tree"; + // Marks which folder this list belongs to, so an inline "new note" field can + // be mounted into the right one after a render. + if (dir !== null) ul.dataset.dir = dir; for (const node of nodes) { ul.append(node.is_dir ? this.renderDir(node) : this.renderFile(node)); } @@ -87,7 +218,9 @@ export class Sidebar { details.open = true; const summary = document.createElement("summary"); summary.textContent = node.name; - details.append(summary, this.renderNodes(node.children)); + summary.dataset.path = node.path; + summary.addEventListener("contextmenu", (e) => this.openMenuForFolder(e, node.path, node)); + details.append(summary, this.renderNodes(node.children, node.path)); li.append(details); return li; } @@ -99,7 +232,323 @@ export class Sidebar { entry.dataset.path = node.path; entry.textContent = node.name; entry.addEventListener("click", () => this.cb.onOpenFile(node.path)); + entry.addEventListener("contextmenu", (e) => this.openMenuForFile(e, node)); li.append(entry); return li; } + + // ---- context menus ------------------------------------------------------- + + /** + * Menu coordinates. A `contextmenu` event raised from the keyboard (Shift+F10, + * the menu key) carries no pointer position and reports 0,0 — anchoring to the + * row's own box puts the menu where the user is looking instead of the corner + * of the window. + */ + private menuPosition(e: MouseEvent): { x: number; y: number } { + if (e.clientX !== 0 || e.clientY !== 0) return { x: e.clientX, y: e.clientY }; + const target = e.currentTarget; + if (target instanceof HTMLElement) { + const rect = target.getBoundingClientRect(); + return { x: rect.left, y: rect.bottom }; + } + return { x: 0, y: 0 }; + } + + private openMenuForFile(e: MouseEvent, node: FileNode): void { + e.preventDefault(); + e.stopPropagation(); + const dir = parentDir(node.path); + const entries: MenuEntry[] = [ + { label: "Open", onSelect: () => this.cb.onOpenFile(node.path) }, + ]; + if (this.cb.onRename) { + entries.push({ + label: "Rename…", + onSelect: () => + this.beginEdit({ kind: "rename", dir, target: { path: node.path, name: node.name } }), + }); + } + if (this.cb.onDelete) { + entries.push("separator", { + label: "Delete", + danger: true, + title: "Move to the workspace .trash folder — this can be undone", + onSelect: () => this.cb.onDelete?.(node.path, false), + }); + } + openContextMenu({ ...this.menuPosition(e), entries }); + } + + /** `node` is null for the workspace root, which can be added to but not renamed. */ + private openMenuForFolder(e: MouseEvent, path: string, node: FileNode | null): void { + e.preventDefault(); + e.stopPropagation(); + const entries: MenuEntry[] = []; + if (this.cb.onCreateNote) { + entries.push({ + label: "New Note…", + onSelect: () => this.beginEdit({ kind: "note", dir: path }), + }); + } + if (this.cb.onCreateFolder) { + entries.push({ + label: "New Folder…", + onSelect: () => this.beginEdit({ kind: "folder", dir: path }), + }); + } + if (node) { + if (this.cb.onRename) { + entries.push("separator", { + label: "Rename…", + onSelect: () => + this.beginEdit({ + kind: "rename", + dir: parentDir(node.path), + target: { path: node.path, name: node.name }, + }), + }); + } + if (this.cb.onDelete) { + if (!this.cb.onRename) entries.push("separator"); + entries.push({ + label: "Delete", + danger: true, + title: "Move the folder and everything in it to .trash — this can be undone", + onSelect: () => this.cb.onDelete?.(node.path, true), + }); + } + } + if (entries.length === 0) return; + openContextMenu({ ...this.menuPosition(e), entries }); + } + + // ---- the inline name field ---------------------------------------------- + + private beginEdit(edit: Omit & { value?: string }): void { + this.edit = { + ...edit, + value: edit.value ?? edit.target?.name ?? "", + error: null, + }; + this.render(); + + // A create field starts empty and is filled in asynchronously, so the user + // can begin typing immediately and a slow suggestion never overwrites what + // they typed — checked against the field's *current* value, not the state's. + if (edit.kind === "note" && this.cb.suggestName && !edit.value) { + const dir = edit.dir; + void this.cb.suggestName(dir).then((name) => { + const input = this.fieldInput(); + if (!this.edit || this.edit.kind !== "note" || this.edit.dir !== dir) return; + if (!input || input.value !== "") return; + input.value = name; + this.edit.value = name; + selectStem(input); + }); + } + } + + private endEdit(): void { + this.edit = null; + const pending = this.pendingTree; + this.pendingTree = null; + if (pending) { + this.rootName = pending.rootName; + this.rootPath = pending.rootPath; + this.tree = pending.tree; + } + this.render(); + } + + private fieldInput(): HTMLInputElement | null { + return this.container.querySelector(".sidebar-edit-input"); + } + + /** + * Put the field into the rendered tree. + * + * Called after every render so the field survives one — a create is mounted + * into its folder's list, a rename replaces the row it renames. If the anchor + * is missing (the folder disappeared from under us) the edit is abandoned + * rather than mounted somewhere arbitrary. + */ + private mountEditor(): void { + const edit = this.edit; + if (!edit) return; + + const field = this.buildField(edit); + + if (edit.kind === "rename" && edit.target) { + const row = + this.container.querySelector(`.file-entry[data-path="${cssEscape(edit.target.path)}"]`) ?? + this.container.querySelector(`summary[data-path="${cssEscape(edit.target.path)}"]`); + if (!row) { + this.edit = null; + return; + } + row.replaceWith(field); + } else { + const list = this.container.querySelector( + `ul[data-dir="${cssEscape(edit.dir)}"]`, + ); + if (!list) { + this.edit = null; + return; + } + const li = document.createElement("li"); + li.append(field); + list.prepend(li); + // A collapsed folder would hide the field the user is typing into. + const details = list.closest("details"); + if (details) details.open = true; + } + + const input = field.querySelector("input"); + input?.focus(); + if (input) selectStem(input); + } + + private buildField(edit: EditState): HTMLElement { + const wrap = document.createElement("div"); + wrap.className = "sidebar-edit"; + wrap.dataset.kind = edit.kind; + + const input = document.createElement("input"); + input.type = "text"; + input.className = "sidebar-edit-input"; + input.value = edit.value; + input.spellcheck = false; + input.setAttribute( + "aria-label", + edit.kind === "rename" ? `Rename ${edit.target?.name ?? ""}` : `Name of the new ${edit.kind}`, + ); + input.addEventListener("input", () => { + if (this.edit) this.edit.value = input.value; + }); + input.addEventListener("keydown", (e) => { + if (e.key === "Enter") { + e.preventDefault(); + void this.commit(input.value); + } else if (e.key === "Escape") { + e.preventDefault(); + // Escape must not also reach the global keymap (which closes panes). + e.stopPropagation(); + this.endEdit(); + } + }); + // Blur **cancels** rather than commits. Committing on blur is how a + // half-typed name becomes a real rename because someone clicked away; the + // cost of cancelling is retyping, which is recoverable, and this direction + // never touches the filesystem on an action the user did not confirm. + // + // `isConnected` distinguishes the user leaving the field from the field + // being replaced by a re-render — showing an error rebuilds it, and treating + // that as a cancel would wipe the message the moment it appeared. + input.addEventListener("blur", () => { + if (!input.isConnected) return; + if (this.edit && !this.busy) this.endEdit(); + }); + wrap.append(input); + + if (edit.error) { + const err = document.createElement("div"); + err.className = "sidebar-edit-error"; + err.setAttribute("role", "alert"); + err.textContent = edit.error; + wrap.append(err); + } + return wrap; + } + + /** + * True while an operation is in flight. + * + * Guards two things: the blur handler (the field loses focus while the awaited + * command runs, and that must not read as a cancel), and double submission + * from a second Enter before the first resolves — which for a create would be + * a second create attempt against a path the first one is making. + */ + private busy = false; + + private async commit(value: string): Promise { + const edit = this.edit; + if (!edit || this.busy) return; + + const name = value.trim(); + if (name === "") { + this.showError("Enter a name."); + return; + } + // A rename to the same name is a no-op the backend also accepts; short- + // circuiting here keeps it from showing up as a save in version history. + if (edit.kind === "rename" && name === edit.target?.name) { + this.endEdit(); + return; + } + + this.busy = true; + try { + if (edit.kind === "note") await this.cb.onCreateNote?.(edit.dir, name); + else if (edit.kind === "folder") await this.cb.onCreateFolder?.(edit.dir, name); + else if (edit.target) await this.cb.onRename?.(edit.target.path, name); + this.busy = false; + this.endEdit(); + } catch (e) { + this.busy = false; + this.showError(messageOf(e)); + } + } + + /** Re-render the field with an error beside it, keeping what was typed. */ + private showError(error: string): void { + if (!this.edit) return; + this.edit.error = error; + this.render(); + } +} + +/** The containing directory of `path`, using whichever separator it carries. */ +function parentDir(path: string): string { + const cut = Math.max(path.lastIndexOf("/"), path.lastIndexOf("\\")); + return cut > 0 ? path.slice(0, cut) : path; +} + +/** + * Select the name but not the extension, so typing replaces `note` in `note.md`. + * + * The convention every file manager uses, and it matters more here: the field is + * pre-filled with a full file name, and a plain select-all means renaming + * usually starts by retyping `.md`. + */ +function selectStem(input: HTMLInputElement): void { + const dot = input.value.lastIndexOf("."); + // `setSelectionRange` is not implemented for every input type in every engine; + // a failure here costs a nicety, never the rename. + try { + if (dot > 0) input.setSelectionRange(0, dot); + else input.select(); + } catch { + /* selection is cosmetic */ + } +} + +/** + * Quote a path for use inside an attribute selector. + * + * Paths are arbitrary user data and Windows paths are full of backslashes, which + * a selector reads as escapes. `CSS.escape` is the correct tool and exists in + * both webview engines; the fallback keeps this working under a bare jsdom that + * has not implemented it. + */ +function cssEscape(value: string): string { + if (typeof CSS !== "undefined" && typeof CSS.escape === "function") return CSS.escape(value); + return value.replace(/["\\]/g, "\\$&"); +} + +/** The user-facing text of a rejection, which from Tauri is a plain string. */ +function messageOf(e: unknown): string { + if (typeof e === "string") return e; + if (e instanceof Error) return e.message; + return String(e); } diff --git a/tests/contextmenu.test.ts b/tests/contextmenu.test.ts new file mode 100644 index 0000000..8e9f376 --- /dev/null +++ b/tests/contextmenu.test.ts @@ -0,0 +1,188 @@ +// Context-menu gate (CLAUDE.md §8, ROADMAP Movement II.12). +// +// The menu is the entry point to every destructive file operation, so what is +// pinned here is the behaviour that decides whether the *right* action runs: +// only one menu at a time, dismissal actually removing its document listeners, +// and the menu closing before its handler runs (the handlers open an inline +// field and take focus). +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + closeContextMenu, + isContextMenuOpen, + openContextMenu, + type MenuEntry, +} from "../src/ui/contextmenu"; + +afterEach(() => { + closeContextMenu(); + document.body.replaceChildren(); +}); + +function items(): HTMLButtonElement[] { + return Array.from(document.querySelectorAll(".context-menu-item")); +} + +function press(key: string): void { + document.dispatchEvent(new KeyboardEvent("keydown", { key, bubbles: true })); +} + +function open(entries: MenuEntry[]): void { + openContextMenu({ x: 10, y: 10, entries }); +} + +describe("openContextMenu", () => { + it("renders one row per item and skips separators as choices", () => { + open([ + { label: "Open", onSelect: () => {} }, + "separator", + { label: "Delete", onSelect: () => {}, danger: true }, + ]); + + expect(items().map((b) => b.textContent)).toEqual(["Open", "Delete"]); + expect(document.querySelectorAll(".context-menu-sep")).toHaveLength(1); + expect(items()[1].dataset.danger).toBe("true"); + }); + + it("opening a second menu replaces the first", () => { + open([{ label: "A", onSelect: () => {} }]); + open([{ label: "B", onSelect: () => {} }]); + + expect(document.querySelectorAll(".context-menu")).toHaveLength(1); + expect(items().map((b) => b.textContent)).toEqual(["B"]); + }); + + it("closes before running the handler, so the handler can take focus", () => { + let openWhenHandlerRan: boolean | null = null; + open([{ label: "Rename…", onSelect: () => (openWhenHandlerRan = isContextMenuOpen()) }]); + + items()[0].click(); + + expect(openWhenHandlerRan).toBe(false); + expect(isContextMenuOpen()).toBe(false); + }); + + it("Escape closes without selecting anything", () => { + const onSelect = vi.fn(); + open([{ label: "Delete", onSelect }]); + + press("Escape"); + + expect(isContextMenuOpen()).toBe(false); + expect(onSelect).not.toHaveBeenCalled(); + }); + + it("a pointer press outside closes it; one inside does not", () => { + open([{ label: "Open", onSelect: () => {} }]); + + items()[0].dispatchEvent(new MouseEvent("pointerdown", { bubbles: true })); + expect(isContextMenuOpen()).toBe(true); + + document.body.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true })); + expect(isContextMenuOpen()).toBe(false); + }); + + /** + * The failure this pins is silent: a menu that closes but leaves its + * document-level listeners attached goes on swallowing the next Escape or + * click, and nothing about the UI shows why. + */ + it("stops handling keys once closed", () => { + const onSelect = vi.fn(); + open([{ label: "Delete", onSelect }]); + closeContextMenu(); + + press("ArrowDown"); + press("Enter"); + + expect(onSelect).not.toHaveBeenCalled(); + expect(document.querySelector(".context-menu")).toBeNull(); + }); + + it("arrow keys move focus and wrap in both directions", () => { + open([ + { label: "One", onSelect: () => {} }, + { label: "Two", onSelect: () => {} }, + { label: "Three", onSelect: () => {} }, + ]); + const [one, two, three] = items(); + + expect(document.activeElement).toBe(one); + press("ArrowDown"); + expect(document.activeElement).toBe(two); + press("ArrowUp"); + expect(document.activeElement).toBe(one); + press("ArrowUp"); // wraps to the end + expect(document.activeElement).toBe(three); + press("ArrowDown"); // wraps to the start + expect(document.activeElement).toBe(one); + press("End"); + expect(document.activeElement).toBe(three); + press("Home"); + expect(document.activeElement).toBe(one); + }); + + it("skips a disabled item in keyboard traversal and cannot select it", () => { + const onSelect = vi.fn(); + open([ + { label: "Open", onSelect: () => {} }, + { label: "Rename…", onSelect, disabled: true }, + { label: "Delete", onSelect: () => {} }, + ]); + + press("ArrowDown"); + + expect((document.activeElement as HTMLElement).textContent).toBe("Delete"); + items()[1].click(); + expect(onSelect).not.toHaveBeenCalled(); + }); + + it("restores focus to whatever opened it", () => { + const trigger = document.createElement("button"); + document.body.append(trigger); + trigger.focus(); + + open([{ label: "Open", onSelect: () => {} }]); + expect(document.activeElement).not.toBe(trigger); + + press("Escape"); + expect(document.activeElement).toBe(trigger); + }); + + it("does not yank focus back when dismissed by a click elsewhere", () => { + const trigger = document.createElement("button"); + const other = document.createElement("input"); + document.body.append(trigger, other); + trigger.focus(); + + open([{ label: "Open", onSelect: () => {} }]); + other.focus(); + other.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true })); + + expect(document.activeElement).toBe(other); + }); + + /** + * Regression: the dismiss-on-window-blur listener was registered with + * `capture: true`, and a capturing window listener sees every *element's* + * blur too — so opening a menu while anything was focused closed it again + * the instant it focused its own first item. + */ + it("survives opening while another element has focus", () => { + const trigger = document.createElement("button"); + document.body.append(trigger); + trigger.focus(); + + open([{ label: "Open", onSelect: () => {} }]); + + expect(isContextMenuOpen()).toBe(true); + expect(items()).toHaveLength(1); + }); + + it("keeps a negative anchor on screen", () => { + openContextMenu({ x: -50, y: -20, entries: [{ label: "Open", onSelect: () => {} }] }); + const menu = document.querySelector(".context-menu"); + + expect(menu?.style.left).toBe("0px"); + expect(menu?.style.top).toBe("0px"); + }); +}); diff --git a/tests/sidebar.test.ts b/tests/sidebar.test.ts new file mode 100644 index 0000000..0f2a897 --- /dev/null +++ b/tests/sidebar.test.ts @@ -0,0 +1,315 @@ +// Sidebar file-operations gate (CLAUDE.md §8, ROADMAP Movement II.12). +// +// What the sidebar owns is the *interaction*, so that is what is pinned here: +// which operations the menu offers, that a rejected name keeps the field open +// with the message rather than silently doing nothing, that blur cancels rather +// than commits, and that a background refresh cannot delete what the user is +// typing. The rules the operations are judged against live in `crates/fileops` +// and are tested there; the disk itself is never touched from this file. +import { afterEach, describe, expect, it, vi } from "vitest"; +import { closeContextMenu } from "../src/ui/contextmenu"; +import { Sidebar, type SidebarCallbacks } from "../src/ui/sidebar"; +import type { FileNode } from "../src/ipc"; + +const ROOT = "/vault"; + +function file(name: string, dir = ROOT): FileNode { + return { name, path: `${dir}/${name}`, is_dir: false, children: [] }; +} + +function folder(name: string, children: FileNode[], dir = ROOT): FileNode { + return { name, path: `${dir}/${name}`, is_dir: true, children }; +} + +function mount(cb: Partial = {}, tree: FileNode[] = [file("note.md")]) { + const container = document.createElement("div"); + document.body.append(container); + const sidebar = new Sidebar(container, { onOpenFile: () => {}, ...cb }); + sidebar.setRoot("vault", tree, ROOT); + return { sidebar, container }; +} + +function rightClick(el: Element): void { + el.dispatchEvent(new MouseEvent("contextmenu", { bubbles: true, clientX: 5, clientY: 5 })); +} + +function menuLabels(): string[] { + return Array.from(document.querySelectorAll(".context-menu-item")).map( + (b) => b.textContent ?? "", + ); +} + +function clickMenu(label: string): void { + const item = Array.from( + document.querySelectorAll(".context-menu-item"), + ).find((b) => b.textContent === label); + if (!item) throw new Error(`no menu item "${label}" (have: ${menuLabels().join(", ")})`); + item.click(); +} + +function field(container: HTMLElement): HTMLInputElement { + const input = container.querySelector(".sidebar-edit-input"); + if (!input) throw new Error("no name field is open"); + return input; +} + +function typeAndConfirm(input: HTMLInputElement, value: string): void { + input.value = value; + input.dispatchEvent(new Event("input", { bubbles: true })); + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true })); +} + +/** Let the awaited callback inside `commit` settle. */ +const settle = (): Promise => new Promise((r) => setTimeout(r, 0)); + +afterEach(() => { + closeContextMenu(); + document.body.replaceChildren(); +}); + +describe("the tree", () => { + it("renders files and folders, and opens a file on click", () => { + const onOpenFile = vi.fn(); + const { container } = mount({ onOpenFile }, [folder("sub", [file("deep.md", "/vault/sub")])]); + + const entry = container.querySelector(".file-entry"); + expect(entry?.textContent).toBe("deep.md"); + entry?.click(); + expect(onOpenFile).toHaveBeenCalledWith("/vault/sub/deep.md"); + }); +}); + +describe("the context menu", () => { + /** + * The empty state already follows this rule for Open Folder: a control that + * looks live and does nothing is worse than an absent one. It matters more + * here, because the absent handler would be Delete. + */ + it("offers only the operations that are wired", () => { + const { container } = mount({}); + rightClick(container.querySelector(".file-entry")!); + expect(menuLabels()).toEqual(["Open"]); + + closeContextMenu(); + document.body.replaceChildren(); + + const wired = mount({ onRename: vi.fn(), onDelete: vi.fn() }); + rightClick(wired.container.querySelector(".file-entry")!); + expect(menuLabels()).toEqual(["Open", "Rename…", "Delete"]); + }); + + it("offers creation on a folder, and on the workspace root", () => { + const { container } = mount( + { onCreateNote: vi.fn(), onCreateFolder: vi.fn(), onRename: vi.fn(), onDelete: vi.fn() }, + [folder("sub", [file("deep.md", "/vault/sub")])], + ); + + rightClick(container.querySelector("summary")!); + expect(menuLabels()).toEqual(["New Note…", "New Folder…", "Rename…", "Delete"]); + + closeContextMenu(); + rightClick(container.querySelector(".sidebar-root")!); + // The root is the folder the user opened: it can be added to, but Toril has + // no business renaming or trashing it from inside the tree. + expect(menuLabels()).toEqual(["New Note…", "New Folder…"]); + }); + + it("marks Delete as destructive", () => { + const { container } = mount({ onDelete: vi.fn() }); + rightClick(container.querySelector(".file-entry")!); + + const del = Array.from( + document.querySelectorAll(".context-menu-item"), + ).find((b) => b.textContent === "Delete"); + expect(del?.dataset.danger).toBe("true"); + }); + + it("passes is_dir so the caller can find every tab under a folder", () => { + const onDelete = vi.fn(); + const { container } = mount({ onDelete }, [folder("sub", [file("deep.md", "/vault/sub")])]); + + rightClick(container.querySelector("summary")!); + clickMenu("Delete"); + + expect(onDelete).toHaveBeenCalledWith("/vault/sub", true); + }); +}); + +describe("creating", () => { + it("creates a note in the folder that was right-clicked", async () => { + const onCreateNote = vi.fn().mockResolvedValue(undefined); + const { container } = mount({ onCreateNote }, [ + folder("sub", [file("deep.md", "/vault/sub")]), + ]); + + rightClick(container.querySelector("summary")!); + clickMenu("New Note…"); + typeAndConfirm(field(container), "Ideas"); + await settle(); + + expect(onCreateNote).toHaveBeenCalledWith("/vault/sub", "Ideas"); + expect(container.querySelector(".sidebar-edit-input")).toBeNull(); + }); + + it("creates at the root from the header button", async () => { + const onCreateNote = vi.fn().mockResolvedValue(undefined); + const { container } = mount({ onCreateNote }); + + container.querySelector(".sidebar-root-btn")?.click(); + typeAndConfirm(field(container), "Ideas"); + await settle(); + + expect(onCreateNote).toHaveBeenCalledWith(ROOT, "Ideas"); + }); + + it("pre-fills a suggested name without overwriting what is already typed", async () => { + let release!: (name: string) => void; + const suggestName = vi.fn(() => new Promise((r) => (release = r))); + const { container } = mount({ onCreateNote: vi.fn(), suggestName }); + + container.querySelector(".sidebar-root-btn")?.click(); + const input = field(container); + // The user does not wait for the round trip. + input.value = "Ideas"; + input.dispatchEvent(new Event("input", { bubbles: true })); + release("Untitled 2.md"); + await settle(); + + expect(input.value).toBe("Ideas"); + }); + + it("fills in the suggestion when the field is still untouched", async () => { + const suggestName = vi.fn().mockResolvedValue("Untitled 2.md"); + const { container } = mount({ onCreateNote: vi.fn(), suggestName }); + + container.querySelector(".sidebar-root-btn")?.click(); + await settle(); + + expect(field(container).value).toBe("Untitled 2.md"); + }); + + it("refuses an empty name without calling the backend", async () => { + const onCreateNote = vi.fn(); + const { container } = mount({ onCreateNote }); + + container.querySelector(".sidebar-root-btn")?.click(); + typeAndConfirm(field(container), " "); + await settle(); + + expect(onCreateNote).not.toHaveBeenCalled(); + expect(container.querySelector(".sidebar-edit-error")?.textContent).toBe("Enter a name."); + }); +}); + +describe("renaming", () => { + it("sends the new name and closes the field", async () => { + const onRename = vi.fn().mockResolvedValue(undefined); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + expect(field(container).value).toBe("note.md"); + + typeAndConfirm(field(container), "renamed.md"); + await settle(); + + expect(onRename).toHaveBeenCalledWith("/vault/note.md", "renamed.md"); + }); + + it("treats an unchanged name as a cancel, not a rename", async () => { + const onRename = vi.fn(); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + typeAndConfirm(field(container), "note.md"); + await settle(); + + // A no-op rename that reached Rust would still be a write in the eyes of the + // watcher, and would show up as a change to a file nothing changed. + expect(onRename).not.toHaveBeenCalled(); + expect(container.querySelector(".sidebar-edit-input")).toBeNull(); + }); + + it("keeps the field open with the message when the backend refuses", async () => { + const onRename = vi.fn().mockRejectedValue('"CON" is a reserved name on Windows.'); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + typeAndConfirm(field(container), "CON.md"); + await settle(); + + expect(container.querySelector(".sidebar-edit-error")?.textContent).toBe( + '"CON" is a reserved name on Windows.', + ); + // Still editable, and still holding what was typed — a rejected name should + // cost a keystroke, not the whole interaction. + expect(field(container).value).toBe("CON.md"); + }); + + it("Escape cancels without renaming", async () => { + const onRename = vi.fn(); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + const input = field(container); + input.value = "half-typed"; + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + await settle(); + + expect(onRename).not.toHaveBeenCalled(); + expect(container.querySelector(".file-entry")?.textContent).toBe("note.md"); + }); + + /** + * Blur must **cancel**. Committing on blur is how a half-typed name becomes a + * real rename because the user clicked somewhere else — and unlike a cancel, + * that one touches the filesystem on an action nobody confirmed. + */ + it("blur cancels rather than committing", async () => { + const onRename = vi.fn(); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + const input = field(container); + input.value = "half-typed"; + input.dispatchEvent(new Event("blur", { bubbles: false })); + await settle(); + + expect(onRename).not.toHaveBeenCalled(); + expect(container.querySelector(".sidebar-edit-input")).toBeNull(); + }); +}); + +describe("refreshes while a name is being typed", () => { + /** + * The controller refreshes the tree on every watcher event, so a sync daemon + * touching an unrelated file mid-rename would otherwise re-render the field + * out from under the user and lose what they had typed. + */ + it("defers the new tree until the edit ends, then applies it", async () => { + const onRename = vi.fn().mockResolvedValue(undefined); + const { sidebar, container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + const input = field(container); + input.value = "half-typed"; + input.dispatchEvent(new Event("input", { bubbles: true })); + + sidebar.setRoot("vault", [file("note.md"), file("arrived.md")], ROOT); + + expect(sidebar.isEditing()).toBe(true); + expect(field(container).value).toBe("half-typed"); + expect(container.textContent).not.toContain("arrived.md"); + + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + await settle(); + + expect(sidebar.isEditing()).toBe(false); + expect(container.textContent).toContain("arrived.md"); + }); +}); From 073dbbd493b827f604d30448d984d30e385b5426 Mon Sep 17 00:00:00 2001 From: Evan Gress <106449014+evangress@users.noreply.github.com> Date: Tue, 18 Aug 2026 06:49:42 -0400 Subject: [PATCH 4/4] docs(roadmap): correct three stale entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch 12b (the chrome rework) shipped on main on 2026-08-12 as PR #36 — panes.ts, rail.ts, resizer.ts and the tokens/chrome/editor stylesheet split are all there — but its checkbox was never ticked, and the status block listed it under "landed outside the movement ladder" when it is a numbered branch in the ladder. Two records of the same thing, both wrong in different directions. The v0.2.0-alpha release point under branch 5 was overtaken by the v1.0.0 tag. The prose above it already says so; the unticked checkbox did not, which left the document implying a version still to be cut. Marked as overtaken and kept for what it meant, rather than deleted — the release point is the record of when the data-safety floor became handable to a stranger, and that is worth keeping even though the number is gone. Branch 12 now names its PR. It was ticked as shipped while nothing was on main and no branch was even pushed, which is the failure mode this document exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) --- ROADMAP.md | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index d749eda..9ef1c6e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -46,7 +46,7 @@ It is a fine **editor**. It is not yet a **notes system** — no global search, quick switcher, no links, no tags, no version history, no sync conflict handling, no AI. That gap is this roadmap. -> **Status (2026-08-17).** Shipped through **`v1.0.0`** (see `CHANGELOG.md`). +> **Status (2026-08-18).** Shipped through **`v1.0.0`** (see `CHANGELOG.md`). > **Movement I, branches 1–4 are complete** (autosave + crash-recovery journal; > safe-delete-to-trash; local version history; sync coexistence — 3-way merge, conflict > banner, parked conflict copies). **Branch 5 has landed** on `feat/release-readiness` — @@ -63,12 +63,14 @@ no AI. That gap is this roadmap. > after it, so the next feature could not have reached anyone. §7's *trust before reach* > is what settles the order. Read the pointer below as the ordering, not the ladder. > -> **Branch 12 (sidebar file operations) has also landed**, taken ahead of 6 for the same -> kind of reason 10 was: it wired two crates — `trashbin` and `snapshots::rekey` — that -> had shipped with **no caller at all**, and until it landed a note could be opened and -> saved but not renamed or deleted without leaving the app. +> **Branch 12 (sidebar file operations) has also landed** — PR #41, stacked on branch 5's +> PR #40 — taken ahead of 6 for the same kind of reason 10 was: it wired two crates — +> `trashbin` and `snapshots::rekey` — that had shipped with **no caller at all**, and until +> it landed a note could be opened and saved but not renamed or deleted without leaving the +> app. **Branch 12b (chrome rework) is ticked too**; it shipped on `main` 2026-08-12 and this +> document had simply never recorded it. > -> **▶ Pick up at Movement II, branch 6 — `feat/vault-search`.** (Branch 5 is done bar +> **▶ In progress: Movement II, branch 6 — `feat/vault-search`.** (Branch 5 is done bar > Azure signing, which is blocked on an account, not on code.) > Search is the largest remaining functional gap, and branch 7 > (command palette) depends on it. Vet `tantivy` per §2 at @@ -82,9 +84,10 @@ no AI. That gap is this roadmap. > *Landed since, outside the movement ladder:* a serializer-normalization precursor > (canonical markdown now matches Obsidian — `-` bullets, `---` rules, tight lists > preserved), which exists to keep branch 4's 3-way merge from drowning in -> reformatting noise; the chrome/layout rework (CLAUDE.md §12b); a fix for programmatic -> loads marking every document dirty; CI running the headless gates on Ubuntu and -> Windows for every pull request; and the GitHub community-standards docs. +> reformatting noise; a fix for programmatic loads marking every document dirty; CI running +> the headless gates on Ubuntu and Windows for every pull request; and the GitHub +> community-standards docs. (The chrome/layout rework — CLAUDE.md §12b — used to be listed +> here as outside the ladder. It is not: it is **branch 12b**, and it is ticked below.) > > *Version note, and read it before leaning on the number:* `v1.0.0` says the editor and > its data-safety floor are ready to depend on — **not** that this roadmap is finished. @@ -246,8 +249,12 @@ nice in a synced folder. This movement is also the prerequisite for the AI wedge `menu.rs`, `main.ts`. - *Gate:* `tests/update.test.ts` for the policy; §D of `docs/ON-DEVICE-VERIFICATION.md` for everything downstream of the network, which no headless gate can reach. - - [ ] **⬢ RELEASE `v0.2.0-alpha`** — *"Safe to live in."* First build you can hand to - someone without an asterisk on their data. + - [~] **⬢ RELEASE `v0.2.0-alpha`** — *"Safe to live in."* **Overtaken by the `v1.0.0` + tag**, which shipped the data-safety floor under a number this ladder never planned + for. Kept as the record of what the release point *meant* — the first build you can + hand to someone without an asterisk on their data — not as a version still to be cut. + The floor itself is complete; only Azure Trusted Signing is outstanding, and it is + blocked on provisioning, not on code. --- @@ -343,7 +350,7 @@ between an editor that works and one that is pleasant to spend hours in. **Branches** -- [ ] **12b. `feat/chrome-ux`** — the UI shell around the editor: a **single tabbed right +- [x] **12b. `feat/chrome-ux`** — *(shipped 2026-08-12)* the UI shell around the editor: a **single tabbed right rail** (outline and history become alternatives, not two rails), pane collapse/expand that actually animates, drag-to-resize with persisted widths, pointer-adaptive touch targets, a real design-token layer, and **real menu accelerators**. Taken out of