diff --git a/.forgejo/workflows/android.yml b/.forgejo/workflows/android.yml index e7e922b..eb64b38 100644 --- a/.forgejo/workflows/android.yml +++ b/.forgejo/workflows/android.yml @@ -19,15 +19,9 @@ name: Android on: push: + # NO `paths:` FILTER — the `decide` job below reads the real file set instead. + # See desktop.yml for why, and 85ead4d for what the duplication cost. branches: [dev, main] - paths: - - "android/**" - # The Rust the .so is built from. A core change reaches the phone exactly - # as it reaches the desktop, so this lane has to rebuild on it. - - "core/**" - - "Cargo.toml" - - "Cargo.lock" - - ".forgejo/workflows/android.yml" workflow_dispatch: concurrency: @@ -42,8 +36,46 @@ env: JAVA_TOOL_OPTIONS: "--enable-native-access=ALL-UNNAMED" jobs: + # Does the APK need rebuilding, or is the channel already serving this source? + # See the equivalent job in desktop.yml — same reasoning, same replacement of a + # hand-kept `paths:` filter with the one file set in `packaging/version.sh`. + # + # The guard runs here so it covers the skip path too (§6.3). + # + # NOTE THE COUPLING WITH ci.yml: when this lane builds, its last step dispatches + # ci.yml so the image bakes in the APK just published. When it SKIPS, no dispatch + # happens — and that is correct, because ci.yml's `gate` stands down only when the + # push touched Android's files, which is the same condition that makes this build. + # The two decisions agree because they read the same fact; they are still two + # readers of it, which is why the gate's grep carries a comment pointing here. + decide: + name: Build, or is the channel already serving this? + runs-on: python-ci + container: + image: git.fabledsword.com/bvandeusen/ci-python:3.14 + outputs: + build: ${{ steps.d.outputs.build }} + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Decide + id: d + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + case "$GITHUB_REF_NAME" in + main) channel=stable ;; + *) channel=dev ;; + esac + sh packaging/guard-forward.sh android "$channel" + echo "build=$(sh packaging/should-build.sh android "$channel")" >> $GITHUB_OUTPUT + build: name: Kotlin + Rust (APK) + needs: [decide] + if: needs.decide.outputs.build == 'true' # runs-on is only a scheduling label (Label Model B). flutter-ci is the # proven-working label that can pull our container images. runs-on: flutter-ci @@ -63,6 +95,10 @@ jobs: steps: - uses: actions/checkout@v4 + with: + # Derives a version, so it needs the whole history — see the note in + # desktop.yml. Depth-1 is silently wrong here, not loudly broken (§6.1). + fetch-depth: 0 - name: Cache Gradle and Cargo uses: actions/cache@v4 @@ -85,13 +121,17 @@ jobs: env: ANDROID_KEYSTORE_BASE64: ${{ secrets.ANDROID_KEYSTORE_BASE64 }} run: | - version="$(sh ../desktop/packaging/build-version.sh)" + # TWO CLOCKS, ON PURPOSE (note 3127 §2). The NAME answers "is this the + # same code?", so it comes from the COMMIT and a dev build and the main + # build of one commit read identically. The CODE answers "may this be + # installed over that?" and must be monotonic BY CONSTRUCTION, because + # Android hard-fails a downgrade with INSTALL_FAILED_VERSION_DOWNGRADE and + # leaves a channel you cannot get out of — so it comes from BUILD time, + # which cannot go backwards. Commit time can. + version="$(sh ../packaging/version.sh display android)" + code="$(sh ../packaging/version.sh key android)" echo "name=$version" >> $GITHUB_OUTPUT - # versionCode must RISE for Android to accept an update, and the run - # number is the same monotonic counter the desktop's version scheme - # already uses — no state carried between runs, and immune to the - # shallow checkout that makes a commit count useless here. - echo "code=$GITHUB_RUN_NUMBER" >> $GITHUB_OUTPUT + echo "code=$code" >> $GITHUB_OUTPUT if [ -n "${ANDROID_KEYSTORE_BASE64:-}" ]; then printf '%s' "$ANDROID_KEYSTORE_BASE64" | base64 -d > /tmp/thoughtsync-release.jks @@ -105,7 +145,7 @@ jobs: echo "profile=debug" >> $GITHUB_OUTPUT echo "keystore=/tmp/thoughtsync-release.jks" >> $GITHUB_OUTPUT echo "apk=android/app/build/outputs/apk/release/app-release.apk" >> $GITHUB_OUTPUT - echo "Signed release build — $version (versionCode $GITHUB_RUN_NUMBER)" + echo "Signed release build — $version (versionCode $code)" else echo "::warning::No ANDROID_KEYSTORE_BASE64 secret. Building an UNSIGNED DEBUG APK: it cannot be installed over a signed build and cannot self-update." echo "variant=Debug" >> $GITHUB_OUTPUT @@ -199,19 +239,29 @@ jobs: JSON cat dist/thoughtsync-android.json - # The rolling dev channel, same fixed-tag release the desktop bundles use. - # CI artifacts are per-run and auth-gated, so they are no use as a fetch - # target; a release asset has a permanent URL. Only ever a SIGNED build — - # publishing an unsigned APK would offer people something they cannot - # install over what they already have. - - name: Publish to the dev channel - if: github.ref == 'refs/heads/dev' && steps.build.outputs.keystore != '' + # The rolling channel for this branch, the same fixed-tag releases the desktop + # bundles use. CI artifacts are per-run and auth-gated, so they are no use as a + # fetch target; a release asset has a permanent URL. Only ever a SIGNED build — + # publishing an unsigned APK would offer people something they cannot install + # over what they already have. + # + # `stable` from main is new in M314 step 3, and it is what lets the server image + # bake in a client that matches its own channel: a :latest image fetches the APK + # from `stable`, a :dev image from `dev`. Before this, main published no APK at + # all and every image — stable included — baked in the dev one. + - name: Publish to the channel for this branch + if: (github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main') && steps.build.outputs.keystore != '' working-directory: . env: GITHUB_TOKEN: ${{ github.token }} - RELEASE_TAG: dev - RELEASE_PRERELEASE: "true" - run: bash desktop/packaging/publish-release.sh + run: | + case "$GITHUB_REF_NAME" in + main) RELEASE_TAG=stable; RELEASE_PRERELEASE=false ;; + *) RELEASE_TAG=dev; RELEASE_PRERELEASE=true ;; + esac + export RELEASE_TAG RELEASE_PRERELEASE + echo "Publishing the APK to the $RELEASE_TAG channel." + bash desktop/packaging/publish-release.sh - name: Upload the APK # Mirrored action, never actions/upload-artifact. @v4+ throws diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 1899bc3..a842cb7 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -1,12 +1,21 @@ # CI runs first; build only proceeds if lint + typecheck pass. # -# Push to dev: typecheck + lint + test + build :dev + : -# Push to main: typecheck + lint + test + build :latest + : -# Tag v* (release): typecheck + lint + test + build :latest + : + : +# Push to dev: typecheck + lint + test + build :dev +# Push to main: typecheck + lint + test + build :latest + : # -# main is the production line, so a merge to main rebuilds and moves :latest to its -# tip (family rule 46) — no version release required. The : image is the -# immutable rollback unit for every build. +# THAT IS THE COMPLETE TAG SET (rule 145). No version-shaped image tag in any lane: +# nothing pins one — verified by looking for a consumer, not for whether one is +# imaginable — and the git release tag is a different object in a different system +# (step 7). The image is addressed by CHANNEL or by COMMIT; the release by date. +# +# A `v*` tag builds nothing at all. The merge to main already published everything, +# so a tag rebuilding that same source would re-push : with different bytes, +# which rule 145 forbids even when they match. +# +# main is the production line, so a merge moves :latest to its tip (family rule 46) +# — no version release required. : is the immutable rollback unit, and it is +# on main ONLY: a sha tag per dev push is a rollback target nobody has ever pulled, +# accumulating forever, for a channel whose entire contract is that it moves. # # Required secret (repo -> Settings -> Secrets -> Actions): # REGISTRY_TOKEN -- Forgejo PAT with write:packages scope @@ -16,27 +25,29 @@ name: CI & Build on: push: + # NO `paths:` FILTER, and unlike the client lanes this one does not skip either — + # the image ALWAYS builds. Two reasons: + # + # * Rule 145 promises that every push to `main` publishes a `:`, so any + # production commit is addressable. A path filter quietly broke that promise + # for a docs-only merge: no trigger, no image, no sha tag for that commit. + # * It is the artifact most exposed to base-image staleness (`python:3.12-slim` + # is a floating tag and this can face the internet), and building every push + # picks those updates up. That is why note 3127 §4's base tension does not + # bite here — the one artifact it would apply to never skips. + # + # Affordable because it is the cheap one: ~15 seconds, against 6 and 9 minutes + # for the clients, which is why THEY skip and this does not. branches: [dev, main] - tags: ["v*"] - paths: - - "src/**" - - "frontend/**" - - "tests/**" - - "pyproject.toml" - - "alembic/**" - - "alembic.ini" - - "Dockerfile" - - ".forgejo/workflows/ci.yml" # Dispatched by the Android lane once it has published a client, so the image # that bakes it in is built AFTER the APK exists rather than racing it. See the # `gate` job below for the other half. workflow_dispatch: -# Cancel older runs on the same branch when a newer push lands. Tag runs get their -# own group implicitly and are never cancelled. +# Cancel older runs on the same branch when a newer push lands. concurrency: group: ci-${{ github.ref }} - cancel-in-progress: ${{ !startsWith(github.ref, 'refs/tags/') }} + cancel-in-progress: true permissions: contents: read @@ -64,7 +75,7 @@ jobs: # than a config so at least it is inspectable in the log. gate: name: Build now, or wait for Android? - if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-python:3.14 @@ -89,17 +100,6 @@ jobs: exit 0 fi - # A tag. The Android lane does not run on tags, so nothing would ever - # call back — standing down here would mean a release tag that never - # produces an image at all. - case "${{ github.ref }}" in - refs/tags/*) - echo "Tag build — the Android lane does not run on tags. Building." - echo "build=true" >> $GITHUB_OUTPUT - exit 0 - ;; - esac - # No parent (first commit, or a force-push that orphaned it) — nothing to # compare, so build rather than stall. if ! git rev-parse --verify -q HEAD^ >/dev/null; then @@ -125,7 +125,12 @@ jobs: echo "Changed in this push:" echo "$changed" | sed 's/^/ /' - if echo "$changed" | grep -qE '^(android/|core/|Cargo\.toml$|Cargo\.lock$|\.forgejo/workflows/android\.yml$)'; then + # MUST match android's file set in packaging/version.sh. `packaging/` was + # missing here after step 4 added it there — so a packaging-only push had + # the Android lane rebuild and dispatch while this gate ALSO let the image + # build, producing two images for one commit and, on main, a second push of + # the same : with different bytes. Rule 145's exact prohibition. + if echo "$changed" | grep -qE '^(android/|core/|packaging/|Cargo\.toml$|Cargo\.lock$|\.forgejo/workflows/android\.yml$)'; then echo "" echo "This push also changes the Android client. Standing down: the" echo "Android lane will publish a new APK and dispatch this workflow," @@ -140,7 +145,7 @@ jobs: typecheck: name: TypeScript typecheck - if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-python:3.14 @@ -157,7 +162,7 @@ jobs: lint: name: Python lint - if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-python:3.14 @@ -170,7 +175,7 @@ jobs: test: name: Python tests - if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-python:3.14 @@ -193,15 +198,14 @@ jobs: # them ever executed by CI — and the schema the migrations build had never been # checked against the models that read it. # - # Runs for visibility and does NOT gate the build, matching the `test` lane and - # FabledScribe's equivalent job. + # Gates the build, along with every other lane — see the `build` job's `needs`. # # Job key stays separator-free ("integration") with no `name:` — rule 80. act_runner # derives the service-container name from the truncated job display name, and the # discovery step below filters `docker ps` by it. Service hostnames are not routable # on this runner (rule 79), so the step resolves the container's bridge IP. integration: - if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-python:3.14 @@ -261,10 +265,16 @@ jobs: build: name: Build & push image - # Build gates on lint + typecheck. The `test` job runs in parallel for - # visibility but does not block dev image builds (DB-backed integration - # testing happens against the dev image manually, not on every push). - needs: [gate, typecheck, lint] + # Every lane gates the build. This once stopped at lint + typecheck, on the + # reasoning that DB-backed testing happened manually against the dev image + # rather than on every push — true until 6f21db8 added the integration lane, + # and false since. + # + # What that gap cost: run 4293 failed `test` and published :dev and : + # anyway, so the deployed server ran a build whose test lane was red. An image + # tag is the rollback substrate (family rule 46); one that can be published + # from a failing run is not a substrate you can roll back TO. + needs: [gate, typecheck, lint, test, integration] if: needs.gate.outputs.build == 'true' runs-on: python-ci container: @@ -274,27 +284,35 @@ jobs: packages: write steps: - uses: actions/checkout@v6 + with: + # Derives a version — see the note in desktop.yml. Depth-1 sees one commit + # and produces a too-low value silently, with the lane green (§6.1). + fetch-depth: 0 - name: Generate image tags and version id: tags # run: steps execute under busybox sh (family rule 81), so use POSIX `case`, # NOT bash `[[ ]]`. run: | - TAGS="${{ env.IMAGE }}:${{ github.sha }}" - BUILD_VERSION="dev" + # The image's version is DERIVED from its own shipped files — including the + # Android client it bakes in, which is why an APK-only change re-versions + # it. One value and no ordering key: nothing compares a server image, so + # §2 says do not invent one just because the other artifacts have one. + # + # This was a short sha on main and the literal "dev" elsewhere, which could + # not answer "how old is this instance?" — the question that actually gets + # asked of a self-hosted app running in several places. + BUILD_VERSION="$(sh packaging/version.sh display server)" case "${{ github.ref }}" in refs/heads/dev) - TAGS="$TAGS,${{ env.IMAGE }}:dev" + TAGS="${{ env.IMAGE }}:dev" ;; refs/heads/main) - # Production line: :latest tracks main's tip (rule 46). No :main tag; - # the : above is the rollback unit. Version label = short sha. - TAGS="$TAGS,${{ env.IMAGE }}:latest" - BUILD_VERSION="$(echo ${{ github.sha }} | cut -c1-7)" + TAGS="${{ env.IMAGE }}:latest,${{ env.IMAGE }}:${{ github.sha }}" ;; - refs/tags/*) - TAGS="$TAGS,${{ env.IMAGE }}:latest,${{ env.IMAGE }}:${{ github.ref_name }}" - BUILD_VERSION="${{ github.ref_name }}" + *) + echo "::error::This lane builds images for dev and main only." + exit 1 ;; esac echo "value=$TAGS" >> $GITHUB_OUTPUT @@ -325,7 +343,18 @@ jobs: GITHUB_TOKEN: ${{ github.token }} run: | mkdir -p client - base="${{ github.server_url }}/${{ github.repository }}/releases/download/dev" + # THE CHANNEL IS A PROPERTY OF THE IMAGE. A :dev image serves the dev + # client; :latest serves the stable one. This read `download/dev` + # unconditionally until M314 step 3, on every branch — so every stable + # server shipped a dev-channel APK to anyone who downloaded the client + # from it. Not a versioning gap; a plain defect, fixed here because this + # is the step that gave `stable` an APK to point at. + case "${{ github.ref_name }}" in + main) channel=stable ;; + *) channel=dev ;; + esac + echo "Baking in the $channel client." + base="${{ github.server_url }}/${{ github.repository }}/releases/download/$channel" ok=1 for f in thoughtsync.apk thoughtsync-android.json; do curl -fsSL -H "Authorization: token $GITHUB_TOKEN" -o "client/$f" "$base/$f" || ok=0 diff --git a/.forgejo/workflows/desktop.yml b/.forgejo/workflows/desktop.yml index ff5cd1f..39748ef 100644 --- a/.forgejo/workflows/desktop.yml +++ b/.forgejo/workflows/desktop.yml @@ -16,33 +16,20 @@ name: Desktop (Tauri) on: push: + # NO `paths:` FILTER. It was a second, independent statement of this artifact's + # file set, hand-kept beside the one in `packaging/version.sh`, and it drifted + # from it within a day (85ead4d). The `decide` job below reads the real set and + # skips in seconds when nothing moved — one definition, one reader (§3). + # + # The cost is that this workflow starts on every push rather than on a matching + # one. That is a ~15s container for a decision, against a lane that cannot + # silently fail to run. branches: [dev, main] - tags: ["v*"] - paths: - - "desktop/**" - # The shared client core (store + sync engine) the desktop wraps. Its own - # crate since the Android client binds the same code, so a change there is a - # change to this app even though nothing under desktop/ moved. - - "core/**" - # The Android uniffi shim. It builds no desktop artifact, but it is a - # workspace member, so this lane's `cargo clippy --all-targets` is what - # compiles and lints it — and until the Android lane exists (M12 step 5), - # it is the ONLY thing that does. - - "android/**" - # The workspace manifest and lockfile, which now live at the repo root. - - "Cargo.toml" - - "Cargo.lock" - # The whole frontend, not just the adapter/bridge seam: it is compiled INTO - # the desktop binary, so any part of it changing means the shipped app is out - # of date. Config and lockfile included — a dependency bump changes the bundle - # as surely as a component does. - - "frontend/**" - - ".forgejo/workflows/desktop.yml" workflow_dispatch: concurrency: group: desktop-${{ github.ref }} - cancel-in-progress: ${{ !startsWith(github.ref, 'refs/tags/') }} + cancel-in-progress: true permissions: # write (not read) so the tag build can publish a Release with the bundles @@ -51,9 +38,47 @@ permissions: contents: write jobs: + # Does anything need building at all? + # + # ONE reader of ONE definition — the file sets in `packaging/version.sh` — replacing + # the `paths:` filters that used to state the same fact a second time. They drifted + # from it within a day: `packaging/` was added to the sets and not to the filters, + # so the commit fixing a derivation bug never ran on the two lanes it fixed + # (85ead4d). Note 3127 §3 warns about exactly that duplication. + # + # THE GUARD RUNS HERE, so it runs on every path INCLUDING the skip one (§6.3). + # Skipping because "the channel already serves this version" is indistinguishable + # from "we derived a stale value that happens to match" unless something checks. + decide: + name: Build, or is the channel already serving this? + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' + runs-on: python-ci + container: + image: git.fabledsword.com/bvandeusen/ci-python:3.14 + outputs: + build: ${{ steps.d.outputs.build }} + steps: + - uses: actions/checkout@v6 + with: + # Derives a version — depth-1 is silently wrong (§6.1). + fetch-depth: 0 + + - name: Decide + id: d + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + case "$GITHUB_REF_NAME" in + main) channel=stable ;; + *) channel=dev ;; + esac + sh packaging/guard-forward.sh desktop "$channel" + echo "build=$(sh packaging/should-build.sh desktop "$channel")" >> $GITHUB_OUTPUT + build: name: Tauri desktop (Linux) - if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + needs: [decide] + if: needs.decide.outputs.build == 'true' runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-tauri:1.97 @@ -64,6 +89,14 @@ jobs: APPIMAGE_EXTRACT_AND_RUN: "1" steps: - uses: actions/checkout@v6 + with: + # DERIVES A VERSION -> needs the whole history. A depth-1 clone sees one + # commit and `git log -- ` produces a too-LOW value, silently, with + # the lane green — note 3127 §6.1, and the direction you cannot recover + # from. `packaging/version.sh` fails loudly on an empty result rather than + # emitting something plausible, which is what turns this into a red lane + # if it is ever dropped. + fetch-depth: 0 # tauri's generate_context! embeds the built frontend at compile time, so the # frontend must exist before any cargo compile (clippy/test/build), not just @@ -123,8 +156,12 @@ jobs: else echo "No TAURI_SIGNING_PRIVATE_KEY — building unsigned, no updater artifacts." fi - version="$(sh ../packaging/build-version.sh)" - echo "Building version $version" + # The ORDERING KEY, not the display version: this string is what Tauri's + # updater parses as semver, and what it stamps into bundle FILENAMES that + # `write-manifest.sh` then selects on. The human-readable version is a + # separate value and arrives with the UI that shows it (#3181). + version="$(sh ../../packaging/version.sh key desktop)" + echo "Building desktop ordering key $version" cargo tauri build \ --config '{"build":{"beforeBuildCommand":""}}' \ --config "{\"version\":\"$version\"}" \ @@ -205,37 +242,40 @@ jobs: # failure, not as a green run with an empty artifact. if-no-files-found: error - # Tag builds only: publish a real, versioned Fabled-Git Release with the - # AppImage + .deb attached — the stable fetch target the install script and - # the in-app updater consume (Actions artifacts above are ephemeral/test). - # Cutting the tag is the operator's action (rule 2); this only publishes a - # Release for a tag that already exists. Dormant on dev/main pushes. - - name: Publish release - if: startsWith(github.ref, 'refs/tags/v') - env: - GITHUB_TOKEN: ${{ github.token }} - run: bash desktop/packaging/publish-release.sh - - # The rolling DEVELOPMENT channel (M10.9): a release whose tag never moves, so - # the updater has a permanent URL to read — Forgejo has no - # /releases/latest/download/ route, so "newest" can't be named in a URL. + # The rolling channel for this branch: `dev` from dev, `stable` from main. Both + # are releases whose tag never moves, so the updater has a permanent URL to + # read — Forgejo has no /releases/latest/download/ route, so "newest" + # cannot be named in a URL. + # + # MAIN PUBLISHING HERE is what makes a `v*` tag optional (note 3127 §0). Until + # M314 step 3 this job built on main and published nothing, so the stable + # channel moved only when somebody cut a tag — that section's diagnostic + # failing outright: main publishing was not sufficient for a user to receive + # the build. # # Gated on the signing key INSIDE the script rather than with an `if:`, because # the secrets context isn't reliably available to step conditions. Publishing # bundles the app would then refuse to verify is worse than publishing nothing: # it looks like a working feed. - - name: Publish to the dev channel - if: github.ref == 'refs/heads/dev' + - name: Publish to the channel for this branch + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' env: GITHUB_TOKEN: ${{ github.token }} TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }} - RELEASE_TAG: dev - RELEASE_PRERELEASE: "true" run: | if [ -z "${TAURI_SIGNING_PRIVATE_KEY:-}" ]; then - echo "No TAURI_SIGNING_PRIVATE_KEY — skipping the dev channel publish." + echo "No TAURI_SIGNING_PRIVATE_KEY — skipping the channel publish." exit 0 fi + # POSIX `case`, not bash `[[ ]]` — these run under busybox sh (rule 81). + # `prerelease` is true for dev so it does not read as a supported build, + # and false for stable, which is the real thing. + case "$GITHUB_REF_NAME" in + main) RELEASE_TAG=stable; RELEASE_PRERELEASE=false ;; + *) RELEASE_TAG=dev; RELEASE_PRERELEASE=true ;; + esac + export RELEASE_TAG RELEASE_PRERELEASE + echo "Publishing to the $RELEASE_TAG channel." bash desktop/packaging/publish-release.sh # Windows installer, CROSS-COMPILED from Linux — there is no Windows build host. @@ -253,12 +293,21 @@ jobs: # built, not that it runs. A real-machine check stays mandatory before trusting it. windows: name: Windows installer (cross-compiled) - if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + needs: [decide] + if: needs.decide.outputs.build == 'true' runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-tauri-win:1.97 steps: - uses: actions/checkout@v6 + with: + # DERIVES A VERSION -> needs the whole history. A depth-1 clone sees one + # commit and `git log -- ` produces a too-LOW value, silently, with + # the lane green — note 3127 §6.1, and the direction you cannot recover + # from. `packaging/version.sh` fails loudly on an empty result rather than + # emitting something plausible, which is what turns this into a red lane + # if it is ever dropped. + fetch-depth: 0 # Same reason as the Linux job: generate_context! embeds the built frontend # at compile time, so it must exist before cargo runs. @@ -292,8 +341,12 @@ jobs: TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }} TAURI_SIGNING_PRIVATE_KEY_PASSWORD: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY_PASSWORD }} run: | - version="$(sh ../packaging/build-version.sh)" - echo "Building version $version" + # The ORDERING KEY, not the display version: this string is what Tauri's + # updater parses as semver, and what it stamps into bundle FILENAMES that + # `write-manifest.sh` then selects on. The human-readable version is a + # separate value and arrives with the UI that shows it (#3181). + version="$(sh ../../packaging/version.sh key desktop)" + echo "Building desktop ordering key $version" updater='{}' if [ -n "${TAURI_SIGNING_PRIVATE_KEY:-}" ]; then updater='{"bundle":{"createUpdaterArtifacts":true}}' @@ -317,35 +370,40 @@ jobs: path: target/x86_64-pc-windows-msvc/release/bundle/nsis/*.exe if-no-files-found: error - # Publishes to the SAME release as the Linux job. Safe to run twice: the - # script reuses an existing release (409) and nullglob means each job uploads - # only the bundles present in its own workspace. - - name: Publish release - if: startsWith(github.ref, 'refs/tags/v') - env: - GITHUB_TOKEN: ${{ github.token }} - run: bash desktop/packaging/publish-release.sh - - # The rolling DEVELOPMENT channel (M10.9): a release whose tag never moves, so - # the updater has a permanent URL to read — Forgejo has no - # /releases/latest/download/ route, so "newest" can't be named in a URL. + # The rolling channel for this branch: `dev` from dev, `stable` from main. Both + # are releases whose tag never moves, so the updater has a permanent URL to + # read — Forgejo has no /releases/latest/download/ route, so "newest" + # cannot be named in a URL. + # + # MAIN PUBLISHING HERE is what makes a `v*` tag optional (note 3127 §0). Until + # M314 step 3 this job built on main and published nothing, so the stable + # channel moved only when somebody cut a tag — that section's diagnostic + # failing outright: main publishing was not sufficient for a user to receive + # the build. # # Gated on the signing key INSIDE the script rather than with an `if:`, because # the secrets context isn't reliably available to step conditions. Publishing # bundles the app would then refuse to verify is worse than publishing nothing: # it looks like a working feed. - - name: Publish to the dev channel - if: github.ref == 'refs/heads/dev' + - name: Publish to the channel for this branch + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' env: GITHUB_TOKEN: ${{ github.token }} TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }} - RELEASE_TAG: dev - RELEASE_PRERELEASE: "true" run: | if [ -z "${TAURI_SIGNING_PRIVATE_KEY:-}" ]; then - echo "No TAURI_SIGNING_PRIVATE_KEY — skipping the dev channel publish." + echo "No TAURI_SIGNING_PRIVATE_KEY — skipping the channel publish." exit 0 fi + # POSIX `case`, not bash `[[ ]]` — these run under busybox sh (rule 81). + # `prerelease` is true for dev so it does not read as a supported build, + # and false for stable, which is the real thing. + case "$GITHUB_REF_NAME" in + main) RELEASE_TAG=stable; RELEASE_PRERELEASE=false ;; + *) RELEASE_TAG=dev; RELEASE_PRERELEASE=true ;; + esac + export RELEASE_TAG RELEASE_PRERELEASE + echo "Publishing to the $RELEASE_TAG channel." bash desktop/packaging/publish-release.sh # The updater manifest, written AFTER both bundle jobs — they run in separate @@ -359,12 +417,20 @@ jobs: manifest: name: Update manifest needs: [build, windows] - if: github.ref == 'refs/heads/dev' || startsWith(github.ref, 'refs/tags/v') + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' runs-on: python-ci container: image: git.fabledsword.com/bvandeusen/ci-tauri:1.97 steps: - uses: actions/checkout@v6 + with: + # DERIVES A VERSION -> needs the whole history. A depth-1 clone sees one + # commit and `git log -- ` produces a too-LOW value, silently, with + # the lane green — note 3127 §6.1, and the direction you cannot recover + # from. `packaging/version.sh` fails loudly on an empty result rather than + # emitting something plausible, which is what turns this into a red lane + # if it is ever dropped. + fetch-depth: 0 - name: Write and publish latest.json env: @@ -376,23 +442,22 @@ jobs: echo "manifest to write. Add the secret to enable in-app updates." exit 0 fi - # The SAME helper the bundles were built with — a second derivation here - # could drift, and a manifest whose version doesn't match the binary it - # points at is an updater that never settles. - version="$(sh desktop/packaging/build-version.sh)" - if [ "${GITHUB_REF_NAME}" = "dev" ]; then - export RELEASE_TAG=dev - export RELEASE_NOTES="Development build from ${GITHUB_SHA}" - # Rolling channel: drop the previous build's bundles once the manifest - # points at this one. Nothing can reach them, and they're ~100 MB a push. - export PRUNE_OLD_ASSETS=true - APP_VERSION="$version" bash desktop/packaging/write-manifest.sh - else - export RELEASE_TAG="${GITHUB_REF_NAME}" - export RELEASE_NOTES="ThoughtSync ${GITHUB_REF_NAME}" - # Twice: once onto the versioned release itself, and once onto the - # permanent `stable` pointer the app actually reads. Same manifest both - # times — its URLs point at the versioned assets either way. - APP_VERSION="$version" bash desktop/packaging/write-manifest.sh - APP_VERSION="$version" MANIFEST_TAG=stable bash desktop/packaging/write-manifest.sh - fi + # The SAME helper AND the same request the bundles were built with — a + # second derivation here could drift, and a manifest whose version doesn't + # match the binary it points at is an updater that never settles. It must + # be `key`: this value is matched against bundle filenames. + version="$(sh packaging/version.sh key desktop)" + # Both channels are rolling: the manifest lands on the same release that + # holds the bundles, and the previous build's bundles are dropped once it + # points at this one. Nothing can reach them, and they are ~100 MB a push. + # + # No tag arm any more. A `v*` tag does not reach this workflow at all — it + # triggers release.yml, which writes a changelog and builds nothing. + case "${GITHUB_REF_NAME}" in + main) export RELEASE_TAG=stable + export RELEASE_NOTES="Stable build from ${GITHUB_SHA}" ;; + *) export RELEASE_TAG=dev + export RELEASE_NOTES="Development build from ${GITHUB_SHA}" ;; + esac + export PRUNE_OLD_ASSETS=true + APP_VERSION="$version" bash desktop/packaging/write-manifest.sh diff --git a/.forgejo/workflows/release.yml b/.forgejo/workflows/release.yml new file mode 100644 index 0000000..a647444 --- /dev/null +++ b/.forgejo/workflows/release.yml @@ -0,0 +1,68 @@ +name: Release + +# A RELEASE BUILDS NOTHING. That is the whole point of this lane (M314 step 7). +# +# The merge to `main` already published everything a user can receive: the server +# image as `:latest` + `:`, the desktop bundles and the APK to the `stable` +# channel, and the updater manifest that advertises them. A tag rebuilding that same +# source would produce identical artifacts under identical names, and would re-push +# `:` with different bytes — which rule 145 forbids even when they match. +# +# So the tag is a BOOKMARK, and this lane gives it the only job it has left: saying +# what was in it. Note 3127 §5 — there are two halves to "what am I running", and +# the version answers only the first: +# +# which build is this? the footer, /api/config, the APK's versionName +# what changed since the one ← this +# I was running last month? +# +# Cutting the tag is the operator's act (rule 2). This only responds to one. +# +# THE TAG IS NOT AN IMAGE TAG and never becomes one. `ci.yml` does not trigger on +# tags at all. The image is addressed by channel or by commit; the release by date. +# Same string as the artifact version (rule 148, `vYYYY.MM.DD.HHMM`), different +# system. + +on: + push: + tags: ["v*"] + +permissions: + contents: write + +jobs: + notes: + name: Write the changelog + runs-on: python-ci + container: + image: git.fabledsword.com/bvandeusen/ci-python:3.14 + steps: + - uses: actions/checkout@v6 + with: + # The whole history AND every tag: the notes are the commit range between + # this tag and the previous `v*` one, and neither end exists in a shallow + # clone. A depth-limited checkout here does not fail — it produces a + # shorter changelog, which is the kind of wrong nobody notices. + fetch-depth: 0 + + - name: Publish the release notes + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + notes="$(sh packaging/release-notes.sh "$GITHUB_REF_NAME")" + echo "$notes" + echo "---" + + # JSON-escaped HERE rather than in publish-release.sh, which cannot assume + # python3 is on PATH in the three images that call it. `json.dumps` then + # strip the surrounding quotes — the script supplies those. + RELEASE_BODY_JSON="$(printf '%s' "$notes" \ + | python3 -c 'import json,sys; print(json.dumps(sys.stdin.read())[1:-1])')" + export RELEASE_BODY_JSON + + # Through publish-release.sh for its create-or-PATCH-on-409 path: a + # release that is only ever POSTed keeps whatever body its first run + # wrote (#2182), so re-tagging or re-running must rewrite it. No bundles + # exist in this workspace, so its asset globs match nothing and it + # uploads none — which is the intended behaviour, not a side effect. + RELEASE_TAG="$GITHUB_REF_NAME" bash desktop/packaging/publish-release.sh diff --git a/README.md b/README.md index b8f4489..60b5bcb 100644 --- a/README.md +++ b/README.md @@ -99,8 +99,10 @@ Then open `http://:5000` and register — **the first account becomes the unset, a signing key is generated and persisted in the database (sessions survive restarts). - Uploaded images live under the `thoughtsync-data` volume at `/var/thoughtsync`. - The app waits for the database and runs migrations (`alembic upgrade head`) automatically on start. -- **Image tags:** `:latest` (stable, built from `main`) · `:dev` (latest `dev` build) · - `:` (immutable, for pinning / rollback). +- **Image tags:** `:latest` (stable, built from `main`) · `:dev` (latest `dev` + build) · `:` on `main` only (immutable, the rollback unit). There are + no version-shaped tags: nothing pins one, and the build reports its own version + at `/api/config` and `/health`. - **Putting it on the public internet:** there are four things to do first — close registration, terminate TLS and forward `X-Forwarded-Proto`, stop publishing the app port, and back up the attachment volume as well as the database. See diff --git a/alembic/versions/0027_checklist_items_into_body.py b/alembic/versions/0027_checklist_items_into_body.py new file mode 100644 index 0000000..bae2368 --- /dev/null +++ b/alembic/versions/0027_checklist_items_into_body.py @@ -0,0 +1,128 @@ +"""fold note_items into the note body and drop the table + +Revision ID: 0027 +Revises: 0026 +Create Date: 2026-08-24 + +M304. A checklist item becomes a `- [ ] milk` line of `notes.body`, and `note_items` +goes. The reason is positional, not cosmetic: a row had a position in a table and no +position in the text, so a separate list could only ever render AFTER the prose. With +the items in the body, a list can sit between two paragraphs — which is the thing that +could not be built before and no amount of restyling would have delivered. + +## This migration rewrites note bodies + +Every note that has items gets its body appended to. The rules below are strict +because rewriting somebody's text deserves it — not, as an earlier draft of this +docstring claimed, because this instance holds imported Google Keep notes. It does +not; note 2916's headline is that nothing here is anyone's work but the operator's +test data. What 2916 actually says about imports is conditional — text arriving from +another app WOULD be real, and any import path has to treat it that way — and the +importer this migration shares a format with is one nobody here has run. + +Careful was still the right call. It cost little, and the same care is what the rule +demands the day someone does import something: + + * Rows are read BEFORE the table is dropped, in this one transaction. + * The existing body is never rewritten, only appended to. + * The layout — a blank line between prose and the list, nothing between consecutive + items — is byte-for-byte what `_note_markdown` has always exported and what + `derive::append_item` produces on every client. All three landing on the same text + is what lets the clients migrate their own SQLite stores independently and still + agree with the server, with no sync required to reconcile them. + +## The fold is inlined on purpose + +`notes/checklist.py` has this same function and this migration deliberately does not +import it. A migration has to keep producing what it produced the day it ran; if the +app's spacing rule ever changes, this file must not change with it. + +## `updated_at` is left alone, and that is load-bearing + +Raw SQL, so SQLAlchemy's `onupdate` never fires. Two reasons, and the second matters +more than the first. Every client folds the same rows the same way, so the new body is +news to nobody. And a client holding an UNPUSHED body edit still has the newer +`updated_at`, so when it pulls the migrated note last-write-wins keeps its edit instead +of the migration silently winning. + +The `notes` row's own `sync_revision` trigger (migration 0015) does fire, so every +migrated note becomes pullable once. That is wanted: it is what makes a client whose +local fold somehow differed converge on the server's text. + +## The downgrade is not a true inverse, and says so + +It recreates an empty `note_items` and leaves the bodies alone. Nothing is lost — +every item is still there as text, which is where this migration put it — but the old +code would show those notes as prose with no checklist. A faithful inverse is not +possible: once the items are lines, nothing distinguishes a line this migration wrote +from one somebody typed, and a downgrade that guessed would eat hand-written task +lists. The real rollback is a database restore. + +Recreating the table is not decoration, though. Migration 0015's downgrade runs +`DROP TRIGGER IF EXISTS trg_note_items_bump_note ON note_items`, and `IF EXISTS` +covers the trigger, not the table — against a missing table that statement errors. So +this is what keeps the migration chain runnable all the way back down. +""" +import re + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects.postgresql import UUID + +revision = "0027" +down_revision = "0026" +branch_labels = None +depends_on = None + +_TASK_RE = re.compile(r"^\s*[-*] +\[[ xX]\](?: +.*)?$") + + +def _append_item(body: str, text: str, checked: bool) -> str: + mark = "x" if checked else " " + text = (text or "").strip() + line = f"- [{mark}] {text}" if text else f"- [{mark}]" + trimmed = (body or "").rstrip("\n") + if not trimmed.strip(): + return line + follows_a_list = bool(_TASK_RE.match(trimmed.split("\n")[-1])) + return f"{trimmed}\n{line}" if follows_a_list else f"{trimmed}\n\n{line}" + + +def upgrade(): + bind = op.get_bind() + rows = bind.execute( + sa.text("SELECT note_id, text, checked FROM note_items ORDER BY note_id, position, created_at") + ).fetchall() + + grouped: dict = {} + for note_id, text, checked in rows: + grouped.setdefault(note_id, []).append((text, bool(checked))) + + for note_id, items in grouped.items(): + body = bind.execute(sa.text("SELECT body FROM notes WHERE id = :id"), {"id": note_id}).scalar() + # An item whose note is already gone has nothing to fold into. The foreign key + # should make this impossible; skipping costs nothing and failing here would + # leave the database half-migrated. + if body is None: + continue + for text, checked in items: + body = _append_item(body, text, checked) + bind.execute(sa.text("UPDATE notes SET body = :body WHERE id = :id"), {"body": body, "id": note_id}) + + op.drop_table("note_items") + + +def downgrade(): + # Column-for-column as migration 0006 created it, index name included: 0015's + # downgrade names both the table and its trigger, so a near-enough copy is not + # good enough. + op.create_table( + "note_items", + sa.Column("id", UUID(as_uuid=True), primary_key=True), + sa.Column("note_id", UUID(as_uuid=True), sa.ForeignKey("notes.id", ondelete="CASCADE"), nullable=False), + sa.Column("text", sa.Text(), nullable=False), + sa.Column("checked", sa.Boolean(), nullable=False, server_default=sa.false()), + sa.Column("position", sa.Integer(), nullable=False, server_default="0"), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False, server_default=sa.func.now()), + ) + op.create_index("ix_note_items_note", "note_items", ["note_id"]) diff --git a/alembic/versions/0028_lift_standalone_tags.py b/alembic/versions/0028_lift_standalone_tags.py new file mode 100644 index 0000000..fc9663c --- /dev/null +++ b/alembic/versions/0028_lift_standalone_tags.py @@ -0,0 +1,168 @@ +"""lift standalone #tags out of note bodies + +Revision ID: 0028 +Revises: 0027 +Create Date: 2026-08-26 + +M311. A `#tag` was being shown twice — once as the text you typed and once as a chip — +and with the chip moved to the top of the card the text is redundant. This removes it, +but only from notes where the tag was standing on its own. + +## This migration rewrites note bodies + +The rule is deliberately narrow, and the same one `notes/tags.py:split_body_tags` +applies from here on: + + * A line containing nothing but tags and whitespace is REMOVED. + * Every other line is left exactly as written. + +So `#todo` on its own line goes, and `remember to call #mom tomorrow` does not. The +looser reading — also stripping a trailing tag off a prose line — was rejected because +the text does not say which kind it is: `buy milk #grocery` is filing, `remember to +call #mom` is the sentence's object, and lifting the second leaves "remember to call". +Rewriting somebody's words to save a duplicate chip is a bad trade, and a migration is +the worst possible place to make it. + +Two guards, both of which cost a note nothing: + + * A line inside a ``` fence is never touched. A `#tag` there is a shell comment in a + snippet somebody pasted, and deleting it would eat a line of their example. + * A note that is NOTHING but tags keeps its text. Lifting would leave a blank card, + which is worse than the duplication this fixes. + +## The label rows have to graduate in the same transaction + +A `via_tag` row means "this label is backed by text still in the body". Once the text +is gone that is false, and leaving it true is not cosmetic: `_lift_and_reconcile_tags` +detaches any `via_tag` row it cannot find a `#tag` for, so the note would lose the tag +on its very next save. The flip to `via_tag = false` is what makes the label the record +instead — and what makes the chip's × appear in both editors, which is now the only way +to remove a tag whose text no longer exists. + +## The transform is inlined, like 0027's + +`split_body_tags` is deliberately NOT imported. A migration has to keep producing what +it produced the day it ran; if the app's rule is ever loosened, this file must not +loosen with it and start eating prose it previously left alone. + +`_display_title` is inlined for the same reason, and is only recomputed for a note whose +body actually moved — a note named after a `#todo` line needs a new name, and reading it +from the app would couple this migration to a rule that has already changed once (M13). + +## `updated_at` is left alone, and that is load-bearing + +Raw SQL, so SQLAlchemy's `onupdate` never fires. A client holding an UNPUSHED body edit +keeps the newer `updated_at`, so when it pulls the migrated note last-write-wins keeps +its edit instead of the migration silently winning. + +The `sync_revision` trigger (migration 0015) does fire, so every rewritten note becomes +pullable once and clients converge on the server's text. That is wanted here: unlike +0027, the clients do NOT yet apply this rule locally, so the server's copy is the only +correct one until they do. + +## The downgrade is not a true inverse, and says so + +It cannot be. Nothing distinguishes a `#todo` line this migration deleted from one that +was never there, and putting one back would be guessing at where in the note it went. + +Nothing is lost, though, which is why that is acceptable: the tag still exists as a +label on the note, and the chip still shows it. What a downgrade cannot restore is the +DUPLICATE — which is the thing this migration set out to remove. Rolling the rows back +to `via_tag = true` would be actively harmful: the text that flag claims to be backed by +is gone, so the next save would detach the label and lose the tag for real. So the +downgrade leaves both alone. The real rollback is a database restore. +""" +import re + +import sqlalchemy as sa +from alembic import op + +revision = "0028" +down_revision = "0027" +branch_labels = None +depends_on = None + +# Frozen copies. See "The transform is inlined" above — these must not follow the app. +_TAG_RE = re.compile(r"(?:^|(?<=\s))#(\w[\w-]*)") +_FENCE_RE = re.compile(r"^\s*(?:```|~~~)") +_TASK_RE = re.compile(r"^(?P\s*)(?P[-*]) +\[(?P[ xX])\](?: +(?P.*))?$") +_DISPLAY_TITLE_CAP = 200 + + +def _is_tag(name: str) -> bool: + """A tag must contain a letter, so #2024 and #_ are not tags — and a line holding + only those is therefore not a tag-only line and is left alone.""" + return any(c.isalpha() for c in name) + + +def _split(body: str) -> tuple[list[str], str]: + """(standalone tag names, body with their lines removed).""" + standalone: list[str] = [] + kept: list[str] = [] + in_fence = False + for line in body.split("\n"): + if _FENCE_RE.match(line): + in_fence = not in_fence + kept.append(line) + continue + matches = [m for m in _TAG_RE.finditer(line) if _is_tag(m.group(1))] + remainder = line + for m in reversed(matches): + remainder = remainder[: m.start()] + remainder[m.end() :] + if in_fence or not matches or remainder.strip(): + kept.append(line) + else: + standalone.extend(m.group(1) for m in matches) + lifted = re.sub(r"\n{3,}", "\n\n", "\n".join(kept)).strip("\n") + if body.strip() and not lifted.strip(): + return [], body # nothing but tags: keep the note readable + # A tag still written in prose somewhere keeps its text, so it stays derived. + still_in_prose = {m.group(1).lower() for m in _TAG_RE.finditer(lifted) if _is_tag(m.group(1))} + return [n for n in standalone if n.lower() not in still_in_prose], lifted + + +def _display_title(body: str) -> str: + for line in body.splitlines(): + stripped = line.strip() + match = _TASK_RE.match(stripped) + text = (match.group("text") or "") if match else stripped + text = text.strip() + if text: + return text[:_DISPLAY_TITLE_CAP] + return "" + + +def upgrade(): + bind = op.get_bind() + rows = bind.execute(sa.text("SELECT id, body FROM notes WHERE body LIKE '%#%'")).fetchall() + + flip = sa.text( + "UPDATE note_labels nl SET via_tag = false " + "FROM labels l " + "WHERE nl.label_id = l.id AND nl.note_id = :nid AND nl.via_tag = true " + "AND lower(l.name) IN :names" + ).bindparams(sa.bindparam("names", expanding=True)) + + for note_id, body in rows: + if not body: + continue + standalone, lifted = _split(body) + if lifted != body: + bind.execute( + sa.text("UPDATE notes SET body = :body, display_title = :title WHERE id = :id"), + {"body": lifted, "title": _display_title(lifted), "id": note_id}, + ) + # Even when the body did not move, a tag can be standalone only in the sense + # that its line was already removed by an earlier pass — so the flip is driven + # by the tag list, not by whether the text changed. + if standalone: + bind.execute(flip, {"nid": note_id, "names": [n.lower() for n in standalone]}) + + +def downgrade(): + """Deliberately empty — see the module docstring. + + Restoring the deleted lines would be guessing, and flipping the rows back to + `via_tag = true` would be worse than doing nothing: the text that flag claims backs + them is gone, so the next save would detach the label and lose the tag for real. + """ diff --git a/alembic/versions/0029_drop_note_color.py b/alembic/versions/0029_drop_note_color.py new file mode 100644 index 0000000..bf0e2e3 --- /dev/null +++ b/alembic/versions/0029_drop_note_color.py @@ -0,0 +1,93 @@ +"""drop notes.color — a card is one neutral surface, colour lives on the tag + +Revision ID: 0029 +Revises: 0028 +Create Date: 2026-08-28 + +M315 step 3. A note's colour was set by a picker and read by three card renderers. +Steps 1 and 2 stopped every one of those reads: the card is one neutral per theme and +the only coloured thing on a board is a tag. This drops the column that nothing has +been reading since, and the picker goes with it. + +`labels.color` is untouched. That is the colour that survived, and the one the whole +milestone was about keeping. + +## What is lost, and why that is the change rather than a cost of it + +Any colour a note was explicitly given. There is nowhere to preserve it TO — the field +it would be preserved in is the one being dropped — and nothing renders it, so a +preserved value would be a column kept warm for a feature that was deliberately +removed. A note that had a colour now takes its identity from its tags, which is what +the operator asked for: "strip color from the cards ... and keep the color for tags +just on the tag." + +The palette itself is not lost. `NOTE_COLORS` moved from `models/note.py` to +`colors.py` in the same change — labels still name a colour, and leaving the vocabulary +defined on the model that lost one would be an invitation to put the column back. + +## The saved-filter sweep is not optional + +`saved_filters.params` is opaque JSON mirroring the `GET /api/notes` facet query, and +a stored view could carry `"color": "teal"`. With the facet gone that key would sit +there forever, and `clean_params` only guards what is written FROM here on. A view that +silently filters on a field the app no longer has is worse than one that visibly lost a +criterion, so the stored rows are swept too. + +Done in Python rather than as `params::jsonb - 'color'`, deliberately. Postgres has no +try-cast: one malformed blob would abort the whole migration, and these rows are +somebody's saved views. `json.loads` in a try/except lets a corrupt row keep whatever it +holds and lets every other row be fixed. + +## Search is not affected + +`notes.search_vector` is a stored generated column over `display_title` and `body` +(rebuilt in 0026). It never named `color`, so unlike the title drop there is nothing +here to tear down and recreate. + +## Downgrade + +Restores the column, empty, at its old default. The values are not recoverable — see +above. It is the schema that comes back, not the data. +""" +import json + +from alembic import op +import sqlalchemy as sa + +revision = "0029" +down_revision = "0028" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.drop_column("notes", "color") + + bind = op.get_bind() + rows = bind.execute( + sa.text("SELECT id, params FROM saved_filters WHERE params LIKE '%color%'") + ).fetchall() + for sf_id, params in rows: + try: + parsed = json.loads(params) + except (ValueError, TypeError): + # A blob that does not parse cannot be edited safely. Leaving it is + # correct: it was already unreadable by the app, and this migration is not + # the place to decide what it should have said. + continue + if not isinstance(parsed, dict) or "color" not in parsed: + continue + parsed.pop("color") + bind.execute( + sa.text("UPDATE saved_filters SET params = :p WHERE id = :id"), + {"p": json.dumps(parsed), "id": sf_id}, + ) + + +def downgrade() -> None: + # Comes back at the default every note would have had anyway. Which notes once + # carried a chosen colour is not recorded anywhere after the upgrade. + op.add_column( + "notes", + sa.Column("color", sa.Text(), nullable=False, server_default="default"), + ) diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index d80d757..5c53b18 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -7,6 +7,9 @@ this permission never exercised. --> + + New note - Take a note… - Discard - Save Empty note - - +%d more item - +%d more items - + + + Note actions + Moved to trash + Undo @@ -38,12 +39,17 @@ Open note Back to notes Add a checklist - Note + Take a note… Add item Remove item Remove label Set a reminder More actions + Saving… + Not saved yet + Edited %1$s + just now + Done Pin Unpin Labels… @@ -61,7 +67,6 @@ Delete - Color Labels Type a label and press enter from #tag @@ -122,6 +127,8 @@ You\'re on the newest build this server has. Check for an update Update + Build %1$s is downloaded and ready. + Later The update didn\'t install Android needs your permission ThoughtSync has to be allowed to install apps before it can update itself. This is a one-time setting. diff --git a/android/app/src/test/java/com/fabledsword/thoughtsync/ui/DerivedTintTest.kt b/android/app/src/test/java/com/fabledsword/thoughtsync/ui/DerivedTintTest.kt new file mode 100644 index 0000000..fbf214e --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/thoughtsync/ui/DerivedTintTest.kt @@ -0,0 +1,137 @@ +package com.fabledsword.thoughtsync.ui + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Test + +/** + * Pins the derived-colour rule against `frontend/src/notes/colors.ts`. + * + * These are not tests of Kotlin — they are the ONE mechanical guard the mirrored pair + * has. The web side is TypeScript with no test runner (its CI lane is `vue-tsc + * --noEmit` and nothing else), so if these values drift, nothing on that surface will + * say so and a tag will simply be a different colour on the phone than in the browser. + * The same names and hashes are written into colors.ts as a comment; changing either + * side means changing both and re-checking here. + * + * SMALLER SINCE M315. Half of what this file used to pin — the generated card fill, and + * the resolution order that chose between a picked colour, a tag's and a generated one + * — went with the code it guarded when the card became one neutral. The four UUID + * hashes stay because they are what the hash ITSELF is pinned by; nothing derives a + * colour from an id any more, only from a tag's name. + */ +class DerivedTintTest { + @Test + fun `hashes match the fixture shared with the web`() { + // Kotlin's Int is signed, so the two hashes above 0x7FFFFFFF are written as + // their negative literal. The unsigned value in the comment is what colors.ts + // records and what an implementation of FNV-1a will actually produce. + assertEquals(-0x41B8712F, tintHash("00000000-0000-0000-0000-000000000000")) // 0xbe478ed1 + assertEquals(0x3D75CC01, tintHash("11111111-1111-1111-1111-111111111111")) + assertEquals(-0x0EF71AD0, tintHash("6ba7b810-9dad-11d1-80b4-00c04fd430c8")) // 0xf108e530 + assertEquals(0x5B651540, tintHash("f47ac10b-58cc-4372-a567-0e02b2c3d479")) + } + + @Test + fun `colours match the fixture shared with the web`() { + assertEquals("purple", derivedTint("00000000-0000-0000-0000-000000000000")) + assertEquals("blue", derivedTint("11111111-1111-1111-1111-111111111111")) + assertEquals("orange", derivedTint("6ba7b810-9dad-11d1-80b4-00c04fd430c8")) + assertEquals("orange", derivedTint("f47ac10b-58cc-4372-a567-0e02b2c3d479")) + } + + /** Half of all 32-bit hashes are negative as Kotlin Ints; a signed remainder would + * index out of the list for those. The bug this catches is a crash, not a wrong + * colour, so it is worth more than one name's worth of coverage. */ + @Test + fun `every derived colour is a real palette key, over many names`() { + for (n in 0 until 2000) { + assertEquals(true, derivedTint("tag-$n") in DERIVED_TINT_KEYS) + } + } + + @Test + fun `the derived palette excludes default`() { + assertEquals(false, "default" in DERIVED_TINT_KEYS) + assertEquals(9, DERIVED_TINT_KEYS.size) + } + + /** The order IS the mapping — reordering silently recolours every tag on one + * surface only. Written out longhand so a reorder fails here loudly. */ + @Test + fun `key order matches colors ts`() { + assertEquals( + listOf("red", "orange", "yellow", "green", "teal", "blue", "purple", "pink", "gray"), + DERIVED_TINT_KEYS, + ) + } + + /** A tag with no colour of its own derives one from its NAME, which is what makes + * every `#todo` chip the same colour rather than nine different ones. */ + @Test + fun `a label with no colour derives one from its name`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + val todo = resolvedLabelColor("todo", "default", known) + assertEquals(derivedTint("todo"), todo) + assertNotEquals("default", todo) + } + + /** Tags dedupe case-insensitively, so `#Todo` and `#todo` are one tag and must not + * be two colours. This is the whole reason the name is lowercased first. */ + @Test + fun `label colour ignores case`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + assertEquals( + resolvedLabelColor("todo", "default", known), + resolvedLabelColor("ToDo", "default", known), + ) + } + + /** `teal` deliberately, NOT the colour "todo" derives to (pink) — asserting the + * derived value here would pass even with the explicit branch deleted. */ + @Test + fun `an explicitly picked label colour still wins`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + assertNotEquals("teal", derivedTint("todo")) + assertEquals("teal", resolvedLabelColor("todo", "teal", known)) + } + + /** An unreadable key is not a choice — it is data from a server newer than this + * client, and the tag should still be drawn as something. */ + @Test + fun `an unknown colour key falls back to the derived colour`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + assertEquals(derivedTint("todo"), resolvedLabelColor("todo", "chartreuse", known)) + } + + /** A label with no name at all has nothing to hash. Neutral, not a random hue. */ + @Test + fun `a nameless label stays default`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + assertEquals("default", resolvedLabelColor("", "", known)) + assertEquals("default", resolvedLabelColor("", "default", known)) + } + + /** + * The spread is real, and collisions are real too. + * + * Nine keys means two tags sharing a colour is not a bug and cannot be designed + * out — in this very sample `home`/`reading` are both gray and `work`/`ideas` are + * both green. Colour is a hint that two chips are distinct, never a claim that two + * of one colour are the same tag; the chip's TEXT is what says which tag it is. + */ + @Test + fun `different tag names spread across the palette`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + val names = listOf("todo", "grocery", "work", "home", "ideas", "reading", "urgent") + val colours = names.map { resolvedLabelColor(it, "default", known) } + assertEquals(true, colours.toSet().size >= 5) + } + + /** The reason the feature exists: two tags on one board should not look identical. */ + @Test + fun `the derived colour spreads across the palette`() { + val seen = (0 until 500).map { derivedTint("spread-$it") }.toSet() + assertEquals(DERIVED_TINT_KEYS.size, seen.size) + } +} diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index 9adfc1a..5d8e766 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -43,8 +43,8 @@ use thoughtsync_core::sync::blobs::BlobStore; use thoughtsync_core::sync::{client, compat, engine, push, state}; use models::{ - patch_from, ClientUpdate, Identity, Label, Note, NoteDraft, NoteEdit, NoteQuery, ProbeResult, - RevokeOutcome, SyncOutcome, SyncStatus, + patch_from, BodyItem, BodyTag, ClientUpdate, Identity, Label, Note, NoteDraft, NoteEdit, + NoteQuery, ProbeResult, RevokeOutcome, SyncOutcome, SyncStatus, }; uniffi::setup_scaffolding!(); @@ -499,6 +499,52 @@ impl ThoughtSync { } } +// ── checklist text, as pure functions ─────────────────────────────────────── +// +// The pair the block editor is built on: one to read a body apart, one to put a line +// back together. Between them, Kotlin can render a checklist as real checkboxes and +// write the markdown back without owning the grammar — which is the point. Three +// implementations of it is the price already being paid (Rust, Python, TypeScript); +// a fourth in Compose would be one more place for a checklist to change shape when +// it syncs. +// +// Free functions rather than methods, because they touch no database. The editor's +// body is LOCAL state — autosaved on an idle debounce, not written per keystroke — +// so editing a checklist there has to rewrite the text the editor is holding, not a +// row the store would hand back a moment later and overwrite the typing with. + +/// One checklist item as the body line that stores it. For an editor that shows a +/// checkbox instead of the markup and has to write the markup back. +#[uniffi::export] +pub fn checklist_render(text: String, checked: bool) -> String { + local::derive::render_item(&text, checked) +} + +/// Every checklist item in a body, with the line each one sits on — so a renderer +/// walking the body line by line knows which lines are boxes and what is in them. +#[uniffi::export] +pub fn checklist_items(body: String) -> Vec { + local::derive::extract_items(&body) + .into_iter() + .map(BodyItem::from) + .collect() +} + +/// Every `#tag` in a body, with the line and the UTF-16 span each one occupies — so +/// a card can colour the tag where it was typed instead of printing it twice. +/// +/// The same argument as `checklist_items` above, and the same answer: the grammar for +/// what a `#tag` is already exists in Rust, Python and TypeScript. Matching it a +/// fourth time in Compose would be a fourth place for a tag to change shape when it +/// syncs — and this one would fail silently, as the wrong characters tinted. +#[uniffi::export] +pub fn body_tags(body: String) -> Vec { + local::derive::extract_tag_spans(&body) + .into_iter() + .map(BodyTag::from) + .collect() +} + /// Helpers, deliberately NOT exported — uniffi only binds what an `#[uniffi::export]` /// block names, so these stay Rust-side. impl ThoughtSync { @@ -571,7 +617,6 @@ mod tests { fn draft(body: &str) -> NoteDraft { NoteDraft { body: body.to_string(), - color: "default".to_string(), items: None, } } @@ -625,7 +670,6 @@ mod tests { let created = app .create_note(NoteDraft { body: String::new(), - color: "default".to_string(), items: Some(vec!["milk".to_string(), "eggs".to_string()]), }) .expect("create should succeed"); @@ -661,7 +705,6 @@ mod tests { let note = app .create_note(NoteDraft { body: "Packing".to_string(), - color: "default".to_string(), items: Some(vec!["socks".to_string()]), }) .expect("create"); @@ -682,8 +725,9 @@ mod tests { assert!(ticked.items[1].checked); assert_eq!( ticked.items[1].text, "charger", - "ticking a box must not disturb its text — the two setters write \ - different columns and neither may clear the other" + "ticking a box must not disturb its text — both setters rewrite the \ + same line of the body now, so one clobbering the other is a live risk \ + rather than a theoretical one" ); let renamed = app diff --git a/android/ffi/src/models.rs b/android/ffi/src/models.rs index 55c5ddc..aacf8f0 100644 --- a/android/ffi/src/models.rs +++ b/android/ffi/src/models.rs @@ -33,7 +33,6 @@ pub struct Note { /// Always present. Derived by the core, never stored. pub display_title: String, pub body: String, - pub color: String, pub position: i64, pub pinned: bool, pub archived: bool, @@ -49,6 +48,63 @@ pub struct Note { pub updated_at: Option, } +/// A checklist item as it sits in a note's body. +/// +/// Mirrors `derive::DerivedItem`. Carries the LINE because every renderer that walks +/// a body line by line needs the text, the state and the position together — the card +/// to draw a box in the right place, the block editor to know where one block ends. +#[derive(Debug, Clone, uniffi::Record)] +pub struct BodyItem { + pub line: u32, + pub text: String, + pub checked: bool, +} + +impl From for BodyItem { + fn from(i: thoughtsync_core::local::derive::DerivedItem) -> Self { + let thoughtsync_core::local::derive::DerivedItem { + text, + checked, + line, + } = i; + BodyItem { + line, + text, + checked, + } + } +} + +/// One `#tag` and where it sits in a note's body. +/// +/// Mirrors `derive::DerivedTag`. The card colours the tag where it was typed rather +/// than repeating it as a chip, so it needs the SPAN — and the offsets are UTF-16 +/// code units precisely because Kotlin's `AnnotatedString` counts that way. +#[derive(Debug, Clone, uniffi::Record)] +pub struct BodyTag { + pub line: u32, + pub start: u32, + pub end: u32, + pub name: String, +} + +impl From for BodyTag { + fn from(t: thoughtsync_core::local::derive::DerivedTag) -> Self { + let thoughtsync_core::local::derive::DerivedTag { + line, + start, + end, + name, + } = t; + BodyTag { + line, + start, + end, + name, + } + } +} + /// An Android build the linked server is offering, already judged to be newer. /// /// A mirror rather than a re-export of `client::ClientRelease`, for the same @@ -130,7 +186,6 @@ impl From for Note { id, display_title, body, - color, position, pinned, archived, @@ -149,7 +204,6 @@ impl From for Note { id, display_title, body, - color, position, pinned, archived, @@ -287,7 +341,6 @@ pub struct NoteQuery { #[derive(Debug, Clone, uniffi::Record)] pub struct NoteFacets { pub q: Option, - pub color: Option, pub label: Option>, pub has_reminder: Option, pub has_attachment: Option, @@ -316,7 +369,6 @@ impl From for core_models::Facets { fn from(value: NoteFacets) -> Self { let NoteFacets { q, - color, label, has_reminder, has_attachment, @@ -325,7 +377,6 @@ impl From for core_models::Facets { } = value; core_models::Facets { q, - color, label, has_reminder, has_attachment, @@ -339,8 +390,6 @@ impl From for core_models::Facets { #[derive(Debug, Clone, uniffi::Record)] pub struct NoteDraft { pub body: String, - /// "default" unless the user picked a colour. - pub color: String, /// Checklist lines. A note can carry both a body and items (M13 step 2), so this /// is not an alternative to `body` — it is an addition to it. pub items: Option>, @@ -348,8 +397,8 @@ pub struct NoteDraft { impl From for core_models::NoteCreateInput { fn from(value: NoteDraft) -> Self { - let NoteDraft { body, color, items } = value; - core_models::NoteCreateInput { body, color, items } + let NoteDraft { body, items } = value; + core_models::NoteCreateInput { body, items } } } @@ -364,7 +413,6 @@ impl From for core_models::NoteCreateInput { #[derive(Debug, Clone, uniffi::Enum)] pub enum NoteEdit { Body { value: String }, - Color { value: String }, Pinned { value: bool }, Archived { value: bool }, RemindAt { value: String }, @@ -384,7 +432,6 @@ impl NoteEdit { use serde_json::Value; match self { NoteEdit::Body { value } => ("body", Value::String(value)), - NoteEdit::Color { value } => ("color", Value::String(value)), NoteEdit::Pinned { value } => ("pinned", Value::Bool(value)), NoteEdit::Archived { value } => ("archived", Value::Bool(value)), NoteEdit::RemindAt { value } => ("remind_at", Value::String(value)), diff --git a/core/src/local/derive.rs b/core/src/local/derive.rs index 2588df3..af96e6a 100644 --- a/core/src/local/derive.rs +++ b/core/src/local/derive.rs @@ -1,19 +1,31 @@ -//! Deriving `#tags` from a note's body — the local mirror of what the server computes -//! on save. Pure string scanning (no regex dependency), kept in lockstep with the -//! frontend's inline rules (see frontend notes/markdown.ts): +//! Deriving structure from a note's body — the local mirror of what the server +//! computes on save. Pure string scanning (no regex dependency), kept in lockstep +//! with the frontend's inline rules (see frontend notes/markdown.ts): //! //! - `#tag`: `#` at a word boundary followed by tag characters (letter first). //! On save these become labels attached with `via_tag = true`. +//! - `- [ ] item`: a checklist item. The body IS the checklist (M304) — there is no +//! table of items beside it, so a list can sit between two paragraphs instead of +//! only after them. +//! +//! The two are the same idea at different strengths. Tags MATERIALISE into label +//! rows, because the board queries by label. Items materialise into nothing, +//! because nothing queries them: their only readers are the card, the editor and +//! `display_title`. So `extract_items` is the whole storage layer for a checklist, +//! and the rewriters below are how one is edited. //! //! Dedupes case-insensitively, preserving first-seen order. //! //! Also derived `[[wiki-links]]` until they were removed (note 2897) — this is a //! capture-and-recall surface, and a linking system is organization. -/// Extract every `#tag` name (without the leading `#`) from `body`. -pub fn extract_tags(body: &str) -> Vec { - let chars: Vec = body.chars().collect(); - let mut out: Vec = Vec::new(); +/// Every `#tag` in ONE line, as `(start, end, name)` in char indices. +/// +/// Char indices rather than byte offsets so the spans can be used to cut the tags +/// back out of the line without ever landing mid-codepoint — see +/// [`lift_standalone_tags`], which is the only reason the spans exist. +fn line_tags(chars: &[char]) -> Vec<(usize, usize, String)> { + let mut out: Vec<(usize, usize, String)> = Vec::new(); let mut i = 0; while i < chars.len() { if chars[i] == '#' { @@ -24,8 +36,7 @@ pub fn extract_tags(body: &str) -> Vec { while j < chars.len() && is_tag_char(chars[j]) { j += 1; } - let tag: String = chars[i + 1..j].iter().collect(); - push_unique(&mut out, &tag); + out.push((i, j, chars[i + 1..j].iter().collect())); i = j; continue; } @@ -35,6 +46,183 @@ pub fn extract_tags(body: &str) -> Vec { out } +/// Extract every `#tag` name (without the leading `#`) from `body`. +/// +/// Line by line, which changes nothing: a line start and a `\n` are both boundaries, +/// so the same tags come out. It means there is ONE scanner rather than two — this and +/// [`lift_standalone_tags`] cannot disagree about what a tag is. +pub fn extract_tags(body: &str) -> Vec { + let mut out: Vec = Vec::new(); + for line in body.split('\n') { + let chars: Vec = line.chars().collect(); + for (_, _, name) in line_tags(&chars) { + push_unique(&mut out, &name); + } + } + out +} + +/// One `#tag` and exactly where it sits, for a renderer drawing the body itself. +/// +/// The card no longer prints a chip for a tag whose text is still in the note — it +/// colours the token where it was typed instead. To do that a renderer needs the +/// SPAN, not just the name, and asking it to find the name again would be a second +/// grammar quietly disagreeing with this one about what `##a` or `#1` is. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DerivedTag { + /// Which body line it sits on, like [`DerivedItem::line`]. + pub line: u32, + /// Offsets into that line, in UTF-16 code units — INCLUDING the leading `#`. + /// + /// UTF-16 rather than chars or bytes because the two languages that consume this + /// both index strings that way: Kotlin's `AnnotatedString` and JavaScript. A char + /// index is right up until somebody puts an emoji before a tag, and then it lands + /// mid-token with no error anywhere. + pub start: u32, + pub end: u32, + pub name: String, +} + +/// Every `#tag` in `body` with its position — the same scan [`extract_tags`] does, +/// keeping the spans instead of throwing them away. +/// +/// Not deduped, unlike `extract_tags`: two mentions of `#todo` are two pieces of text +/// to colour. Fences are not skipped either, and that is deliberate — `extract_tags` +/// does not skip them, so a `#tag` inside a code block IS a label on the note, and a +/// renderer that left it plain would be the only surface disagreeing. +pub fn extract_tag_spans(body: &str) -> Vec { + let mut out = Vec::new(); + for (n, line) in body.split('\n').enumerate() { + let chars: Vec = line.chars().collect(); + let spans = line_tags(&chars); + if spans.is_empty() { + continue; + } + // Prefix sums, built once per tagged line: char index -> UTF-16 offset. + let mut units: Vec = Vec::with_capacity(chars.len() + 1); + let mut total: u32 = 0; + units.push(0); + for c in &chars { + total += c.len_utf16() as u32; + units.push(total); + } + for (start, end, name) in spans { + out.push(DerivedTag { + line: n as u32, + start: units[start], + end: units[end], + name, + }); + } + } + out +} + +/// Whether a line opens or closes a fenced code block. +fn is_fence(line: &str) -> bool { + let trimmed = line.trim_start(); + trimmed.starts_with("```") || trimmed.starts_with("~~~") +} + +/// Runs of three or more newlines become two, and the ends are trimmed. +/// +/// Removing a line must not leave a hole where it was. +fn collapse_blank_runs(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut run = 0; + for c in text.chars() { + if c == '\n' { + run += 1; + if run <= 2 { + out.push(c); + } + } else { + run = 0; + out.push(c); + } + } + out.trim_matches('\n').to_string() +} + +/// Split a body's tags by whether the text around them can be taken away. +/// +/// Returns `(standalone, inline, lifted_body)`. +/// +/// THE RULE: a line containing nothing but tags and whitespace is removed. Anything +/// else is left exactly as written. +/// +/// The MIRROR of `split_body_tags` in the server's `notes/tags.py`, and it has to stay +/// one: a note lifted differently here than there would change under the operator the +/// moment it synced. Same discipline, and the same reason, as `DerivedTint`. +/// +/// The conservative reading of "standalone" is deliberate. A trailing tag is +/// ambiguous and the text does not say which it is — `buy milk #grocery` is filing, +/// `remember to call #mom` is the sentence's object, and lifting the second leaves +/// "remember to call". A tag sharing a line with words keeps its words. +/// +/// `standalone` tags become ORDINARY labels (`via_tag = 0`): nothing is left to derive +/// them from, so the row becomes the record and the chip's × becomes the way to remove +/// one. `inline` tags stay derived exactly as before. That is what `via_tag` means from +/// here on — backed by text still in the body. +pub fn lift_standalone_tags(body: &str) -> (Vec, Vec, String) { + let mut standalone: Vec = Vec::new(); + let mut inline: Vec = Vec::new(); + let mut kept: Vec<&str> = Vec::new(); + let mut in_fence = false; + + for line in body.split('\n') { + if is_fence(line) { + in_fence = !in_fence; + kept.push(line); + continue; + } + let chars: Vec = line.chars().collect(); + let spans = line_tags(&chars); + // Cut the tags out and see whether anything is left. That is what + // "standalone" means, and it is the whole rule. + let mut remainder = String::new(); + let mut pos = 0; + for (start, end, _) in &spans { + remainder.extend(chars[pos..*start].iter()); + pos = *end; + } + remainder.extend(chars[pos..].iter()); + + // A fence's contents are CODE: a `#tag` there is a shell comment in somebody's + // snippet, and deleting the line would eat part of their example. + if in_fence || spans.is_empty() || !remainder.trim().is_empty() { + for (_, _, name) in &spans { + push_unique(&mut inline, name); + } + kept.push(line); + } else { + for (_, _, name) in &spans { + push_unique(&mut standalone, name); + } + } + } + + let lifted = collapse_blank_runs(&kept.join("\n")); + if !body.trim().is_empty() && lifted.trim().is_empty() { + // The note was NOTHING but tags. Lifting would leave a blank card, which is a + // worse outcome than a duplicated chip — so leave it alone. + let mut all = standalone; + for name in &inline { + push_unique(&mut all, name); + } + return (Vec::new(), all, body.to_string()); + } + + // A tag that ALSO appears in prose stays derived: the prose copy still backs it, + // so deleting that copy should still detach the label. + let inline_lower: Vec = inline.iter().map(|n| n.to_lowercase()).collect(); + let standalone = standalone + .into_iter() + .filter(|n| !inline_lower.contains(&n.to_lowercase())) + .collect(); + (standalone, inline, lifted) +} + fn is_tag_char(c: char) -> bool { c.is_alphanumeric() || c == '_' || c == '-' } @@ -45,6 +233,239 @@ fn push_unique(out: &mut Vec, candidate: &str) { } } +// ── checklist items ───────────────────────────────────────────────────────── +// +// The grammar, in one place, because three languages implement it (here, +// `notes/checklist.py`, `notes/markdown.ts`) and a difference between any two of +// them is a checklist that changes shape when it syncs: +// +// optional indent, `-` or `*`, one-or-more spaces, `[ ]`/`[x]`/`[X]`, +// then either end-of-line or one-or-more spaces and the text. +// +// `*` is accepted because markdown.ts already accepts it for a plain bullet, and a +// grammar that takes `* item` but not `* [ ] item` would be a rule with no reason +// anyone could guess. `- [ ]` with nothing after it IS an item with empty text: +// that is exactly what pressing Enter on a list leaves behind, and refusing to +// parse it would make a half-typed list stop being a list. + +/// A checklist item, as found in the body. Its position in the returned vector is +/// its identity — the same thing `position` meant when these were rows, and all the +/// wire ever carried (`push.rs` sent text and checked, never an id). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DerivedItem { + pub text: String, + pub checked: bool, + /// Which body line it sits on. + /// + /// Carried here rather than offered as a second function, because every renderer + /// that walks a body line by line — the Android card, the block editor — needs the + /// text, the state AND the position together, and asking for them separately is + /// how two calls come to disagree about a body that changed between them. + pub line: u32, +} + +/// One parsed task line, holding enough to put it back exactly as it was found. +struct TaskLine<'a> { + indent: &'a str, + /// Preserved rather than normalised to `-`: rewriting someone's `*` bullets + /// because they ticked a box would be an edit they did not ask for. + bullet: char, + checked: bool, + text: &'a str, +} + +fn parse_task_line(line: &str) -> Option> { + let indent_len = line.len() - line.trim_start().len(); + let (indent, rest) = line.split_at(indent_len); + + let bullet = rest.chars().next()?; + if bullet != '-' && bullet != '*' { + return None; + } + // At least one space after the bullet. `-[ ] x` is not a list item in any + // markdown either, so it stays prose here too. + let rest = &rest[bullet.len_utf8()..]; + let gap = rest.len() - rest.trim_start_matches(' ').len(); + if gap == 0 { + return None; + } + let rest = &rest[gap..]; + + let mut chars = rest.chars(); + if chars.next()? != '[' { + return None; + } + let mark = chars.next()?; + if chars.next()? != ']' { + return None; + } + // Decided BEFORE the slice below, which is what guarantees `mark` is one byte + // and `[?]` is exactly three. + let checked = match mark { + ' ' => false, + 'x' | 'X' => true, + _ => return None, + }; + let rest = &rest[3..]; + + let text = if rest.is_empty() { + // "- [ ]" — an empty item, which is what an unfinished list line is. + rest + } else { + let gap = rest.len() - rest.trim_start_matches(' ').len(); + // "- [ ]x" is prose: without the space this is not a marker, it is a + // sentence that happens to start with brackets. + if gap == 0 { + return None; + } + &rest[gap..] + }; + + Some(TaskLine { + indent, + bullet, + checked, + text, + }) +} + +/// One item as the line that stores it, in canonical form. +/// +/// Public because a block editor has to write a line back after someone edits it in a +/// widget that never showed them the marker. Rendering is trivial where PARSING is +/// not, but it still belongs here: this is the file that decides what canonical looks +/// like, and a caller inventing its own `- [x] ` would be a fourth opinion on it. +pub fn render_item(text: &str, checked: bool) -> String { + render_task_line("", '-', checked, text) +} + +fn render_task_line(indent: &str, bullet: char, checked: bool, text: &str) -> String { + // Always lowercase `x`, whatever was parsed: one canonical output is what makes + // a round trip stable, so `- [X]` normalises the first time it is touched and + // never again. + let mark = if checked { 'x' } else { ' ' }; + if text.is_empty() { + format!("{indent}{bullet} [{mark}]") + } else { + format!("{indent}{bullet} [{mark}] {text}") + } +} + +/// The text of a line with its task marker removed, or the line as it was. +/// +/// For naming a note: a list-only note is named by its first item, and calling one +/// "- [ ] milk" would be showing someone the storage instead of the note. +pub fn strip_marker(line: &str) -> &str { + match parse_task_line(line) { + Some(t) => t.text, + None => line, + } +} + +/// Every checklist item in `body`, in the order they appear. +pub fn extract_items(body: &str) -> Vec { + let mut out = Vec::new(); + for (n, line) in body.split('\n').enumerate() { + if let Some(t) = parse_task_line(line) { + out.push(DerivedItem { + text: t.text.to_string(), + checked: t.checked, + line: n as u32, + }); + } + } + out +} + +/// Rewrite the `index`-th task line, or drop it when `f` returns None. +/// +/// A body with fewer task lines than that is returned UNCHANGED rather than +/// panicking: the index comes from a UI that may be a moment behind the store, and +/// a stale tap should do nothing rather than take the app down. +fn map_task_line(body: &str, index: usize, f: F) -> String +where + F: FnOnce(&TaskLine<'_>) -> Option, +{ + let lines: Vec<&str> = body.split('\n').collect(); + let mut target: Option = None; + let mut seen = 0usize; + for (n, line) in lines.iter().enumerate() { + if parse_task_line(line).is_some() { + if seen == index { + target = Some(n); + break; + } + seen += 1; + } + } + let target = match target { + Some(n) => n, + None => return body.to_string(), + }; + let replacement = match parse_task_line(lines[target]) { + Some(parsed) => f(&parsed), + None => return body.to_string(), + }; + + let mut out: Vec = Vec::with_capacity(lines.len()); + for (n, line) in lines.iter().enumerate() { + if n != target { + out.push((*line).to_string()); + } else if let Some(new_line) = &replacement { + out.push(new_line.clone()); + } + // None at the target line drops it, which is `remove_item`. + } + out.join("\n") +} + +/// Tick or untick the `index`-th item. +pub fn set_item_checked(body: &str, index: usize, checked: bool) -> String { + map_task_line(body, index, |t| { + Some(render_task_line(t.indent, t.bullet, checked, t.text)) + }) +} + +/// Replace the text of the `index`-th item, keeping its state and its bullet. +pub fn set_item_text(body: &str, index: usize, text: &str) -> String { + map_task_line(body, index, |t| { + Some(render_task_line(t.indent, t.bullet, t.checked, text.trim())) + }) +} + +/// Delete the `index`-th item, line and all. +pub fn remove_item(body: &str, index: usize) -> String { + map_task_line(body, index, |_| None) +} + +/// Add an item at the end of the body. +/// +/// Spaced exactly as `import_export.py:_note_markdown` writes a list — a blank line +/// between prose and the list, and nothing between consecutive items. That is not +/// cosmetic: the server migration folds existing rows into bodies using the same +/// layout, so an export taken before the migration and one taken after have to +/// agree byte for byte. +/// +/// `checked` is a parameter rather than always false because the two migrations that +/// fold existing rows into bodies have to carry the state those rows were in. A new +/// item from the UI passes false. +pub fn append_item(body: &str, text: &str, checked: bool) -> String { + let line = render_task_line("", '-', checked, text.trim()); + let trimmed = body.trim_end_matches('\n'); + if trimmed.trim().is_empty() { + return line; + } + let follows_a_list = trimmed + .split('\n') + .next_back() + .is_some_and(|l| parse_task_line(l).is_some()); + if follows_a_list { + format!("{trimmed}\n{line}") + } else { + format!("{trimmed}\n\n{line}") + } +} + #[cfg(test)] mod tests { use super::*; @@ -72,4 +493,281 @@ mod tests { fn empty_body() { assert!(extract_tags("").is_empty()); } + + // ── tag spans, for the renderer that draws them in place ───────────────── + + #[test] + fn tag_spans_carry_the_hash_and_the_line() { + let spans = extract_tag_spans("buy milk #grocery\nand call #mom about #mom"); + assert_eq!(spans.len(), 3); + assert_eq!((spans[0].line, spans[0].start, spans[0].end), (0, 9, 17)); + assert_eq!(spans[0].name, "grocery"); + // Not deduped: two mentions are two pieces of text to colour. + assert_eq!(spans[1].line, 1); + assert_eq!(spans[2].name, "mom"); + assert_eq!((spans[2].start, spans[2].end), (20, 24)); + } + + #[test] + fn tag_spans_are_utf16_offsets_not_char_indices() { + // The emoji is ONE char and TWO UTF-16 code units. Kotlin and JS both index + // the second way, so a char index would highlight one character too early. + let spans = extract_tag_spans("🎁 #gift"); + assert_eq!(spans.len(), 1); + assert_eq!((spans[0].start, spans[0].end), (3, 8)); + } + + #[test] + fn tag_spans_agree_with_extract_tags_about_what_a_tag_is() { + let body = "#1 nope a#b no but #Yes ##no"; + let names: Vec = extract_tag_spans(body) + .into_iter() + .map(|t| t.name) + .collect(); + assert_eq!(names, extract_tags(body)); + } + + // ── lifting standalone tags ────────────────────────────────────────────── + // + // The MIRROR of `split_body_tags` in the server's notes/tags.py, case for case. + // A note lifted differently here than there would change under the operator the + // moment it synced, so these are the cases that file agrees to. + + #[test] + fn lifts_a_line_that_is_nothing_but_tags() { + let (standalone, inline, body) = lift_standalone_tags("#todo\nreorganize the homepage"); + assert_eq!(standalone, vec!["todo"]); + assert!(inline.is_empty()); + assert_eq!(body, "reorganize the homepage"); + + let (standalone, _, body) = lift_standalone_tags("needs a tauri app\n#todo"); + assert_eq!(standalone, vec!["todo"]); + assert_eq!(body, "needs a tauri app"); + + let (standalone, _, body) = lift_standalone_tags("#todo #work\nreal text"); + assert_eq!(standalone, vec!["todo", "work"]); + assert_eq!(body, "real text"); + } + + /// The cases that must come back byte-identical. Getting any of these wrong + /// destroys somebody's words, which is why the rule is the conservative one: + /// a trailing tag is ambiguous and the text does not say which kind it is. + #[test] + fn leaves_a_tag_that_shares_its_line_with_words() { + for prose in [ + "remember to call #mom tomorrow", + "buy milk #grocery", + "#2024\nreal", + ] { + let (standalone, _, body) = lift_standalone_tags(prose); + assert!(standalone.is_empty(), "{prose}"); + assert_eq!(body, prose, "{prose}"); + } + } + + #[test] + fn removing_a_line_leaves_no_hole() { + let (_, _, body) = lift_standalone_tags("foo\n\n#todo\n\nbar"); + assert_eq!(body, "foo\n\nbar"); + } + + /// A `#tag` in a fence is a shell comment in somebody's snippet. It still becomes + /// a label — it always has — but the line is never touched. + #[test] + fn never_touches_a_fenced_line() { + let fenced = "code:\n```\n#!/bin/sh\n#deploy\n```\ndone"; + let (standalone, inline, body) = lift_standalone_tags(fenced); + assert!(standalone.is_empty()); + assert_eq!(inline, vec!["deploy"]); + assert_eq!(body, fenced); + } + + /// Lifting would leave a blank card, which is worse than the duplication this + /// removes. So the note keeps its text and its tags stay derived. + #[test] + fn will_not_blank_a_note_that_is_only_tags() { + let (standalone, inline, body) = lift_standalone_tags("#todo"); + assert!(standalone.is_empty()); + assert_eq!(inline, vec!["todo"]); + assert_eq!(body, "#todo"); + } + + /// Appearing on its own line does NOT lift a tag also written in a sentence — the + /// sentence still backs it, so deleting the sentence should still detach it. + #[test] + fn a_tag_still_in_prose_stays_derived() { + let (standalone, inline, body) = lift_standalone_tags("#todo\nremember the #todo list"); + assert!(standalone.is_empty()); + assert_eq!(inline, vec!["todo"]); + assert_eq!(body, "remember the #todo list"); + } + + #[test] + fn lifting_an_empty_body_is_a_no_op() { + let (standalone, inline, body) = lift_standalone_tags(""); + assert!(standalone.is_empty()); + assert!(inline.is_empty()); + assert_eq!(body, ""); + } + + // ── checklist items ───────────────────────────────────────────────────── + + fn item(text: &str, checked: bool, line: u32) -> DerivedItem { + DerivedItem { + text: text.to_string(), + checked, + line, + } + } + + #[test] + fn items_basic() { + let body = "shopping\n\n- [ ] milk\n- [x] eggs"; + assert_eq!( + extract_items(body), + vec![item("milk", false, 2), item("eggs", true, 3)] + ); + } + + #[test] + fn items_may_sit_between_paragraphs() { + // The whole reason the body owns the list: a table of rows could only ever + // render after the prose. + let body = "before\n- [ ] middle\nafter"; + assert_eq!(extract_items(body), vec![item("middle", false, 1)]); + } + + #[test] + fn items_reject_near_misses() { + // Each of these is prose, and each has been someone's bug report somewhere. + for body in [ + "-[ ] no space after the dash", + "- [] empty brackets", + "- [ ]no space after the brackets", + "- [y] not a mark", + "a [ ] mid sentence", + "[ ] no bullet at all", + ] { + assert!(extract_items(body).is_empty(), "should be prose: {body}"); + } + } + + #[test] + fn items_accept_star_bullets_and_indentation() { + // `*` because markdown.ts already takes it for a plain bullet. + let body = "* [ ] star\n - [x] indented"; + assert_eq!( + extract_items(body), + vec![item("star", false, 0), item("indented", true, 1)] + ); + } + + #[test] + fn an_empty_item_is_still_an_item() { + // What pressing Enter on a list leaves behind. + assert_eq!(extract_items("- [ ]"), vec![item("", false, 0)]); + assert_eq!(extract_items("- [ ] "), vec![item("", false, 0)]); + } + + #[test] + fn uppercase_x_parses_and_normalises_on_rewrite() { + assert_eq!(extract_items("- [X] done"), vec![item("done", true, 0)]); + // Touching it once canonicalises it, and never again. + assert_eq!(set_item_checked("- [X] done", 0, true), "- [x] done"); + } + + #[test] + fn checking_preserves_indent_bullet_and_text() { + assert_eq!(set_item_checked(" * [ ] milk", 0, true), " * [x] milk"); + assert_eq!(set_item_checked("- [x] milk", 0, false), "- [ ] milk"); + } + + #[test] + fn checking_addresses_items_not_lines() { + let body = "note\n- [ ] a\nprose\n- [ ] b"; + assert_eq!( + set_item_checked(body, 1, true), + "note\n- [ ] a\nprose\n- [x] b" + ); + } + + #[test] + fn set_text_keeps_state() { + assert_eq!(set_item_text("- [x] old", 0, "new"), "- [x] new"); + } + + #[test] + fn remove_takes_the_whole_line() { + let body = "keep\n- [ ] drop\n- [ ] stay"; + assert_eq!(remove_item(body, 0), "keep\n- [ ] stay"); + } + + #[test] + fn append_spaces_like_the_exporter() { + // Prose then a blank line then the list — byte-for-byte what + // import_export.py:_note_markdown writes, which is what the server + // migration will fold existing rows into. + assert_eq!(append_item("a note", "milk", false), "a note\n\n- [ ] milk"); + // Nothing between consecutive items. + let one = "a note\n\n- [ ] milk"; + assert_eq!( + append_item(one, "eggs", false), + format!("{one}\n- [ ] eggs") + ); + // A list-only note starts at the first line. + assert_eq!(append_item("", "milk", false), "- [ ] milk"); + assert_eq!(append_item("\n\n", "milk", false), "- [ ] milk"); + // Carries state, which is what the two migrations need of it. + assert_eq!(append_item("", "done", true), "- [x] done"); + } + + #[test] + fn strip_marker_names_a_list_only_note() { + assert_eq!(strip_marker("- [x] milk"), "milk"); + assert_eq!(strip_marker("just prose"), "just prose"); + } + + #[test] + fn render_item_is_what_extract_reads_back() { + assert_eq!(render_item("milk", false), "- [ ] milk"); + assert_eq!(render_item("done", true), "- [x] done"); + // An empty item has no trailing space, so a round trip does not grow it. + assert_eq!(render_item("", false), "- [ ]"); + let line = render_item("milk", true); + assert_eq!(extract_items(&line), vec![item("milk", true, 0)]); + } + + #[test] + fn items_carry_the_line_they_sit_on() { + let found = extract_items("a\n- [ ] x\nb\n- [x] y"); + assert_eq!(found.iter().map(|i| i.line).collect::>(), vec![1, 3]); + } + + #[test] + fn a_stale_index_does_nothing() { + // The index comes from a UI that may be a moment behind the store. A tap + // that arrives late should be inert, not fatal. + let body = "- [ ] only"; + assert_eq!(set_item_checked(body, 7, true), body); + assert_eq!(remove_item(body, 7), body); + assert_eq!(set_item_text(body, 7, "x"), body); + } + + #[test] + fn a_plain_body_is_returned_byte_identical() { + let body = "just prose\nwith two lines"; + assert_eq!(set_item_checked(body, 0, true), body); + assert_eq!(set_item_text(body, 0, "x"), body); + assert_eq!(remove_item(body, 0), body); + } + + #[test] + fn round_trip_is_stable() { + let body = "- [ ] a\n- [x] b\n- [ ] c"; + let items = extract_items(body); + // Ticking and unticking returns the original bytes. + let touched = set_item_checked(&set_item_checked(body, 0, true), 0, false); + assert_eq!(touched, body); + assert_eq!(extract_items(&touched), items); + } } diff --git a/core/src/local/models.rs b/core/src/local/models.rs index 3b5b1d0..97234b6 100644 --- a/core/src/local/models.rs +++ b/core/src/local/models.rs @@ -13,7 +13,6 @@ pub struct Note { /// never stored. pub display_title: String, pub body: String, - pub color: String, pub position: i64, pub pinned: bool, pub archived: bool, @@ -121,16 +120,10 @@ pub struct User { pub is_admin: bool, } -fn default_color() -> String { - "default".to_string() -} - #[derive(Deserialize)] pub struct NoteCreateInput { #[serde(default)] pub body: String, - #[serde(default = "default_color")] - pub color: String, #[serde(default)] pub items: Option>, } @@ -154,8 +147,6 @@ pub struct Facets { #[serde(default)] pub q: Option, #[serde(default)] - pub color: Option, - #[serde(default)] pub label: Option>, #[serde(default)] pub has_reminder: Option, diff --git a/core/src/local/schema.rs b/core/src/local/schema.rs index 8b5decc..7cd9867 100644 --- a/core/src/local/schema.rs +++ b/core/src/local/schema.rs @@ -6,14 +6,16 @@ //! //! Migrations are gated on `PRAGMA user_version`; bump it and add a block per change. -use rusqlite::Connection; +use rusqlite::{params, Connection, OptionalExtension}; + +use crate::local::derive; const SCHEMA_V1: &str = r#" CREATE TABLE notes ( id TEXT PRIMARY KEY, title TEXT, body TEXT NOT NULL DEFAULT '', - color TEXT NOT NULL DEFAULT 'default', + color TEXT NOT NULL DEFAULT 'default', -- dropped in v9; kept so DROP COLUMN has something to drop kind TEXT NOT NULL DEFAULT 'text', -- dropped in v6; kept so DROP COLUMN has something to drop position INTEGER NOT NULL DEFAULT 0, pinned INTEGER NOT NULL DEFAULT 0, @@ -54,6 +56,8 @@ CREATE TABLE checklist_items ( position INTEGER NOT NULL DEFAULT 0 ); CREATE INDEX idx_items_note ON checklist_items (note_id); +-- Both dropped in v8; kept here so an existing database has something to migrate +-- FROM, exactly as `kind` above is kept for v6. CREATE TABLE attachments ( id TEXT PRIMARY KEY, @@ -180,7 +184,96 @@ ALTER TABLE notes DROP COLUMN title; ALTER TABLE note_revisions DROP COLUMN title; "#; +// v8 (M304): `checklist_items` is gone. The body IS the checklist — a `- [ ] milk` +// line is the item — so a list can sit between two paragraphs instead of only after +// them, which a side table could never express no matter how it was styled. +// +// Rust rather than a SQL const, for two reasons. The fold has to produce EXACTLY what +// `derive::append_item` produces, and expressing that in SQL would be a second +// implementation of the layout rule. And `group_concat` only gained a guaranteed +// ORDER BY in SQLite 3.44 — a checklist that silently reordered itself during the +// migration would be a poor way to find that out. +// +// `updated_at` and `dirty` are deliberately NOT touched. The server's Alembic +// migration folds the same rows with the same spacing, so both sides land on +// identical bodies and this needs no sync at all; marking every note dirty would +// push a body the server already has, and would do it for every device at once. +fn migrate_v8(conn: &Connection) -> rusqlite::Result<()> { + // Grouped in one pass — the query is ordered by note, so a change of note_id is + // the group boundary. `rowid` breaks ties, because `position` was only ever + // advisory and two rows sharing one is not a reason to reorder someone's list. + let mut grouped: Vec<(String, Vec<(String, bool)>)> = Vec::new(); + { + let mut stmt = conn.prepare( + "SELECT note_id, text, checked FROM checklist_items + ORDER BY note_id ASC, position ASC, rowid ASC", + )?; + let mut rows = stmt.query([])?; + while let Some(row) = rows.next()? { + let note_id: String = row.get(0)?; + let text: String = row.get(1)?; + let checked: bool = row.get(2)?; + match grouped.last_mut() { + Some((id, items)) if *id == note_id => items.push((text, checked)), + _ => grouped.push((note_id, vec![(text, checked)])), + } + } + } + + for (note_id, items) in grouped { + let existing: Option = conn + .query_row("SELECT body FROM notes WHERE id = ?1", [¬e_id], |r| { + r.get(0) + }) + .optional()?; + // An item whose note is already gone has nothing to fold into. The foreign key + // should make this impossible; skipping costs nothing, and failing here would + // leave the only copy of someone's notes half-migrated. + let mut body = match existing { + Some(b) => b, + None => continue, + }; + for (text, checked) in items { + body = derive::append_item(&body, &text, checked); + } + conn.execute( + "UPDATE notes SET body = ?1 WHERE id = ?2", + params![body, note_id], + )?; + } + + conn.execute_batch( + "DROP INDEX IF EXISTS idx_items_note; + DROP TABLE checklist_items;", + )?; + Ok(()) +} + /// Bring the database up to the latest schema. Idempotent. +// v9 (M315): `notes.color` is gone. A card is one neutral surface now and colour lives +// only on a tag, so the column was written by a picker nothing read and read by nothing +// at all. `labels.color` is untouched — that is the colour that survived. +// +// The saved-filter sweep is the second half and not optional. `params` is opaque JSON +// and a stored view could carry `"color": "teal"`; with the facet gone that key would +// sit there forever, and a view that silently filters on a field the app no longer has +// is worse than one that visibly lost a criterion. Guarded on `json_valid` because a +// corrupt blob must keep whatever it holds, not become NULL. +// +// The second guard is a LIKE and not `json_extract(...) IS NOT NULL`, which is the +// obvious way to write it and is a trap: SQLite does not promise to short-circuit AND, +// so `json_extract` can be evaluated against the very rows `json_valid` was there to +// exclude — and on malformed input it does not return NULL, it RAISES, which would +// abort the migration for every other row too. `LIKE` is total over any text. +const SCHEMA_V9: &str = r#" +ALTER TABLE notes DROP COLUMN color; + +UPDATE saved_filters + SET params = json_remove(params, '$.color') + WHERE json_valid(params) + AND params LIKE '%"color"%'; +"#; + pub fn migrate(conn: &Connection) -> rusqlite::Result<()> { conn.execute_batch("PRAGMA foreign_keys = ON;")?; let version: i64 = conn.query_row("PRAGMA user_version", [], |r| r.get(0))?; @@ -212,5 +305,195 @@ pub fn migrate(conn: &Connection) -> rusqlite::Result<()> { conn.execute_batch(SCHEMA_V7)?; conn.execute_batch("PRAGMA user_version = 7;")?; } + if version < 8 { + migrate_v8(conn)?; + conn.execute_batch("PRAGMA user_version = 8;")?; + } + if version < 9 { + conn.execute_batch(SCHEMA_V9)?; + conn.execute_batch("PRAGMA user_version = 9;")?; + } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// A database as it stood before M304 — items still in their own table. + fn v7_db() -> Connection { + let conn = Connection::open_in_memory().expect("open"); + conn.execute_batch("PRAGMA foreign_keys = ON;").expect("fk"); + for batch in [ + SCHEMA_V1, SCHEMA_V2, SCHEMA_V3, SCHEMA_V4, SCHEMA_V5, SCHEMA_V6, SCHEMA_V7, + ] { + conn.execute_batch(batch).expect("batch"); + } + conn.execute_batch("PRAGMA user_version = 7;").expect("v7"); + conn + } + + fn add_note(conn: &Connection, id: &str, body: &str) { + conn.execute( + "INSERT INTO notes (id, body, created_at, updated_at) + VALUES (?1, ?2, '2026-01-01T00:00:00.000Z', '2026-01-01T00:00:00.000Z')", + params![id, body], + ) + .expect("note"); + } + + fn add_item(conn: &Connection, note: &str, text: &str, checked: bool, pos: i64) { + conn.execute( + "INSERT INTO checklist_items (id, note_id, text, checked, position) + VALUES (?1, ?2, ?3, ?4, ?5)", + params![format!("{note}-{pos}"), note, text, checked, pos], + ) + .expect("item"); + } + + fn body_of(conn: &Connection, id: &str) -> String { + conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0)) + .expect("body") + } + + #[test] + fn v8_folds_items_into_the_body() { + let conn = v7_db(); + add_note(&conn, "n1", "shopping"); + add_item(&conn, "n1", "milk", false, 0); + add_item(&conn, "n1", "eggs", true, 1); + + migrate(&conn).expect("migrate"); + + // Prose, blank line, list — the layout _note_markdown already exports, so an + // export taken before this migration and one taken after agree byte for byte. + assert_eq!(body_of(&conn, "n1"), "shopping\n\n- [ ] milk\n- [x] eggs"); + } + + #[test] + fn v8_keeps_a_list_only_note_whole() { + let conn = v7_db(); + add_note(&conn, "n1", ""); + add_item(&conn, "n1", "milk", false, 0); + + migrate(&conn).expect("migrate"); + + assert_eq!(body_of(&conn, "n1"), "- [ ] milk"); + } + + #[test] + fn v8_leaves_timestamps_alone() { + // The whole reason this needs no sync: the server folds the same rows the same + // way, so both sides already agree. Marking notes dirty would push a body the + // server has, from every device at once. + let conn = v7_db(); + add_note(&conn, "n1", "note"); + add_item(&conn, "n1", "milk", false, 0); + + migrate(&conn).expect("migrate"); + + let (updated, dirty): (String, i64) = conn + .query_row( + "SELECT updated_at, dirty FROM notes WHERE id = 'n1'", + [], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .expect("row"); + assert_eq!(updated, "2026-01-01T00:00:00.000Z"); + assert_eq!(dirty, 1); // as inserted, not raised by the migration + } + + #[test] + fn v8_drops_the_table_and_is_idempotent() { + let conn = v7_db(); + add_note(&conn, "n1", "note"); + migrate(&conn).expect("migrate"); + migrate(&conn).expect("again"); + + let exists: i64 = conn + .query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='checklist_items'", + [], + |r| r.get(0), + ) + .expect("count"); + assert_eq!(exists, 0); + } + + #[test] + fn a_fresh_database_reaches_the_latest_version() { + let conn = Connection::open_in_memory().expect("open"); + migrate(&conn).expect("migrate"); + let version: i64 = conn + .query_row("PRAGMA user_version", [], |r| r.get(0)) + .expect("version"); + assert_eq!(version, 9); + } + + /// The column is gone, not merely unread. Asserted by asking SQLite rather than by + /// reading a row: a SELECT that omits `color` would pass either way. + #[test] + fn v9_drops_the_note_colour_column() { + let conn = Connection::open_in_memory().expect("open"); + migrate(&conn).expect("migrate"); + let mut stmt = conn.prepare("PRAGMA table_info(notes)").expect("pragma"); + let columns: Vec = stmt + .query_map([], |r| r.get::<_, String>(1)) + .expect("query") + .collect::>>() + .expect("collect"); + assert!(!columns.iter().any(|c| c == "color")); + // The one that survived. Getting this wrong would take every tag's colour with + // it, which is the whole thing M315 was keeping. + let mut stmt = conn.prepare("PRAGMA table_info(labels)").expect("pragma"); + let label_columns: Vec = stmt + .query_map([], |r| r.get::<_, String>(1)) + .expect("query") + .collect::>>() + .expect("collect"); + assert!(label_columns.iter().any(|c| c == "color")); + } + + /// A stored view that filtered on colour loses that criterion and keeps the rest. + /// The alternative — leaving the key — is a lens that silently narrows on a field + /// the app no longer has and never says why it returned nothing. + #[test] + fn v9_sweeps_colour_out_of_saved_filters() { + let conn = Connection::open_in_memory().expect("open"); + conn.execute_batch("PRAGMA foreign_keys = ON;").expect("fk"); + for batch in [ + SCHEMA_V1, SCHEMA_V2, SCHEMA_V3, SCHEMA_V4, SCHEMA_V5, SCHEMA_V6, SCHEMA_V7, + ] { + conn.execute_batch(batch).expect("schema"); + } + conn.execute_batch("PRAGMA user_version = 8;").expect("v8"); + for (id, params) in [ + ("a", r#"{"color":"teal","q":"milk"}"#), + ("b", r#"{"q":"eggs"}"#), + // Not JSON at all. It must come out UNCHANGED rather than NULL — a blob + // this migration cannot read is not a blob it gets to destroy. + ("c", "not json"), + ] { + conn.execute( + "INSERT INTO saved_filters (id, name, params, created_at) + VALUES (?1, ?1, ?2, '2026-08-28T00:00:00.000Z')", + params![id, params], + ) + .expect("seed"); + } + + migrate(&conn).expect("migrate"); + + let read = |id: &str| -> String { + conn.query_row( + "SELECT params FROM saved_filters WHERE id = ?1", + [id], + |r| r.get(0), + ) + .expect("read") + }; + assert_eq!(read("a"), r#"{"q":"milk"}"#); + assert_eq!(read("b"), r#"{"q":"eggs"}"#); + assert_eq!(read("c"), "not json"); + } +} diff --git a/core/src/local/store.rs b/core/src/local/store.rs index e267184..ccdffe3 100644 --- a/core/src/local/store.rs +++ b/core/src/local/store.rs @@ -9,7 +9,7 @@ use chrono::{DateTime, Duration, SecondsFormat, Utc}; use rusqlite::{params, params_from_iter, Connection, OptionalExtension}; -use serde_json::Value; +use serde_json::{json, Value}; use uuid::Uuid; use crate::local::derive; @@ -24,24 +24,25 @@ fn new_id() -> String { Uuid::new_v4().to_string() } -/// The note's NAME: its first non-blank body line, else its first checklist item. +/// The note's NAME: the first line of its body that says anything. /// /// Mirrors `derive_display_title` in the server's notes/helpers.py — one rule written /// twice, and they have to agree or a synced note is called different things on either /// side of the wire. /// -/// Pure, and given the items rather than fetching them: every caller has already -/// loaded them, so a query here would be a second trip for something already in hand. -fn display_title(body: &str, items: &[ChecklistItem]) -> String { - if let Some(line) = body.lines().map(str::trim).find(|l| !l.is_empty()) { - return line.to_string(); +/// It no longer needs the items, because the items ARE lines of the body now (M304). +/// What it needs instead is to strip the task marker off: a list-only note is still +/// named by its first item, and calling that note "- [ ] milk" would be showing +/// someone the storage rather than the note. An empty item is skipped rather than +/// naming the note "", which is what a half-typed list would otherwise do. +fn display_title(body: &str) -> String { + for line in body.lines() { + let text = derive::strip_marker(line.trim()).trim(); + if !text.is_empty() { + return text.to_string(); + } } - items - .iter() - .map(|i| i.text.trim()) - .find(|t| !t.is_empty()) - .unwrap_or("") - .to_string() + String::new() } fn escape_like(s: &str) -> String { @@ -69,19 +70,24 @@ fn load_labels(conn: &Connection, note_id: &str) -> rusqlite::Result rusqlite::Result> { - let mut stmt = conn.prepare( - "SELECT id, text, checked, position FROM checklist_items WHERE note_id = ?1 ORDER BY position ASC", - )?; - let rows = stmt.query_map([note_id], |r| { - Ok(ChecklistItem { - id: r.get(0)?, - text: r.get(1)?, - checked: r.get(2)?, - position: r.get(3)?, +/// The note's checklist, read out of its body. No query, because there is no table. +/// +/// A `- [ ] milk` line IS the item (M304). The id is the item's ORDINAL rather than a +/// uuid — which is all it ever amounted to anyway, since `push.rs` sent text and +/// checked and never an id, and both sides replaced the whole list on every sync. It +/// is also exactly what the rewriters in `derive` take, so a UI holding an id can act +/// on it directly. +fn items_of(body: &str) -> Vec { + derive::extract_items(body) + .into_iter() + .enumerate() + .map(|(i, item)| ChecklistItem { + id: i.to_string(), + text: item.text, + checked: item.checked, + position: i as i64, }) - })?; - rows.collect() + .collect() } fn load_attachments(conn: &Connection, note_id: &str) -> rusqlite::Result> { @@ -135,7 +141,7 @@ fn load_previews(conn: &Connection, note_id: &str) -> rusqlite::Result rusqlite::Result { let mut note = conn.query_row( - "SELECT id, body, color, position, pinned, archived, trashed, remind_at, recurrence, created_at, updated_at, trashed_at + "SELECT id, body, position, pinned, archived, trashed, remind_at, recurrence, created_at, updated_at, trashed_at FROM notes WHERE id = ?1", [id], |r| { @@ -144,14 +150,13 @@ fn load_note(conn: &Connection, id: &str) -> rusqlite::Result { id: r.get(0)?, display_title: String::new(), // filled below — it may need a query body, - color: r.get(2)?, - position: r.get(3)?, - pinned: r.get(4)?, - archived: r.get(5)?, - trashed: r.get(6)?, - deleted_at: r.get(11)?, - remind_at: r.get(7)?, - recurrence: r.get(8)?, + position: r.get(2)?, + pinned: r.get(3)?, + archived: r.get(4)?, + trashed: r.get(5)?, + deleted_at: r.get(10)?, + remind_at: r.get(6)?, + recurrence: r.get(7)?, labels: Vec::new(), items: Vec::new(), attachments: Vec::new(), @@ -162,11 +167,10 @@ fn load_note(conn: &Connection, id: &str) -> rusqlite::Result { }, )?; note.labels = load_labels(conn, id)?; - note.items = load_items(conn, id)?; + note.items = items_of(¬e.body); note.attachments = load_attachments(conn, id)?; note.previews = load_previews(conn, id)?; - // After the items, because a body-only-empty note is named by its first one. - note.display_title = display_title(¬e.body, ¬e.items); + note.display_title = display_title(¬e.body); Ok(note) } @@ -200,34 +204,89 @@ fn find_or_create_label(conn: &Connection, name: &str) -> rusqlite::Result rusqlite::Result<()> { - let tags = derive::extract_tags(body); - let mut desired: Vec = Vec::with_capacity(tags.len()); - for t in &tags { - desired.push(find_or_create_label(conn, t)?); +/// Attach the note's tag labels, LIFT its standalone tags out of the body, and write +/// the shortened body back. +/// +/// NAMED FOR THE MUTATION. It used to be `sync_tags` and only touched label rows; it +/// now rewrites `notes.body`, and every caller writes the body just before calling — +/// so this overwrites what they wrote, on purpose. +/// +/// `display_title` needs no attention here, unlike on the server: the core derives it +/// on READ (see `display_title` above, called from `load_note`) rather than storing +/// it, so there is no persisted copy to go stale. +/// +/// The two kinds of tag are handled differently, and that difference IS what `via_tag` +/// means from here on — backed by text still in the body: +/// +/// standalone lifted out, attached as an ORDINARY label. Nothing derives it any +/// more, and the way to remove it becomes the chip's ×. +/// inline left in place, attached via_tag = 1, still detached when its text +/// goes. Unchanged from before. +/// +/// Mirrors `_lift_and_reconcile_tags` in the server's `notes/tags.py`. +fn lift_and_sync_tags(conn: &Connection, note_id: &str, body: &str) -> rusqlite::Result<()> { + let (standalone, inline, lifted) = derive::lift_standalone_tags(body); + + let mut standalone_ids: Vec = Vec::with_capacity(standalone.len()); + for name in &standalone { + standalone_ids.push(find_or_create_label(conn, name)?); + } + let mut inline_ids: Vec = Vec::with_capacity(inline.len()); + for name in &inline { + inline_ids.push(find_or_create_label(conn, name)?); } - let current: Vec = { + let current: Vec<(String, bool)> = { let mut stmt = - conn.prepare("SELECT label_id FROM note_labels WHERE note_id = ?1 AND via_tag = 1")?; - let rows = stmt.query_map([note_id], |r| r.get::<_, String>(0))?; - rows.collect::>>()? + conn.prepare("SELECT label_id, via_tag FROM note_labels WHERE note_id = ?1")?; + let rows = stmt.query_map([note_id], |r| { + Ok((r.get::<_, String>(0)?, r.get::<_, bool>(1)?)) + })?; + rows.collect::>>()? }; - for lid in ¤t { - if !desired.contains(lid) { + + for (lid, via_tag) in ¤t { + if !*via_tag { + continue; // manual already: a #tag of the same name changes nothing + } + if standalone_ids.contains(lid) { + // It GRADUATED. The text backing it is about to go, so the row has to + // become the record instead — and BEFORE the delete below, or the same row + // is dropped for no longer being in the body. That is the bug a naive lift + // has, and it silently loses the tag. + conn.execute( + "UPDATE note_labels SET via_tag = 0 WHERE note_id = ?1 AND label_id = ?2", + params![note_id, lid], + )?; + } else if !inline_ids.contains(lid) { conn.execute( "DELETE FROM note_labels WHERE note_id = ?1 AND label_id = ?2 AND via_tag = 1", params![note_id, lid], )?; } } - for lid in &desired { + + // OR IGNORE leaves a label already attached in ANY form alone, which is what keeps + // a manually-added label of the same name manual. + for lid in &standalone_ids { + conn.execute( + "INSERT OR IGNORE INTO note_labels (note_id, label_id, via_tag) VALUES (?1, ?2, 0)", + params![note_id, lid], + )?; + } + for lid in &inline_ids { conn.execute( "INSERT OR IGNORE INTO note_labels (note_id, label_id, via_tag) VALUES (?1, ?2, 1)", params![note_id, lid], )?; } + + if lifted != body { + conn.execute( + "UPDATE notes SET body = ?1 WHERE id = ?2", + params![lifted, note_id], + )?; + } Ok(()) } @@ -267,10 +326,6 @@ pub fn list_notes(conn: &Connection, q: &ListQuery) -> rusqlite::Result rusqlite::Resu [], |r| r.get(0), )?; - conn.execute( - "INSERT INTO notes (id, body, color, position, created_at, updated_at, dirty) - VALUES (?1, ?2, ?3, ?4, ?5, ?5, 1)", - params![id, input.body, input.color, position, ts], - )?; + // Items fold into the body rather than into rows of their own. Callers still hand + // them over separately — the importer has a list, not a blob — but where they end + // up is one place. + let mut body = input.body.clone(); if let Some(items) = &input.items { - for (i, text) in items.iter().enumerate() { - conn.execute( - "INSERT INTO checklist_items (id, note_id, text, position) VALUES (?1, ?2, ?3, ?4)", - params![new_id(), id, text, i as i64], - )?; + for text in items { + body = derive::append_item(&body, text, false); } } - sync_tags(conn, &id, &input.body)?; + conn.execute( + "INSERT INTO notes (id, body, position, created_at, updated_at, dirty) + VALUES (?1, ?2, ?3, ?4, ?4, 1)", + params![id, body, position, ts], + )?; + // The FOLDED body, not the input one: an item can carry a #tag too. + lift_and_sync_tags(conn, &id, &body)?; load_note(conn, &id) } +/// How long one editing session is assumed to last. +/// +/// Inside this window a note's body may be written any number of times and only the +/// FIRST write snapshots. That is what makes an idle-debounced autosave affordable: +/// a write costs a write, not a write plus a revision. +const REVISION_WINDOW_MINUTES: i64 = 10; + +/// Whether a body change earns a snapshot of the pre-edit body. +/// +/// Two conditions. The body must actually differ — re-saving identical text is not a +/// version of anything. And the note must not already carry a revision from this +/// editing session. +/// +/// The session rule is what keeps version history worth reading. Because +/// [`snapshot_revision`] stores the body as it was BEFORE the edit, the first write +/// of a session captures the note as you found it, and every write after it inside +/// the window adds nothing. One revision per sitting falls out of the window on its +/// own — no "commit" the client has to declare, and no protocol surface to carry it, +/// which matters because sync-apply takes this same path. +fn should_snapshot(conn: &Connection, id: &str, new_body: &str) -> rusqlite::Result { + let current: String = + conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0))?; + if current == new_body { + return Ok(false); + } + // String comparison, not date maths: timestamps are RFC3339 UTC with a fixed + // millisecond field (see the module header), so lexical order IS chronological. + let cutoff = (Utc::now() - Duration::minutes(REVISION_WINDOW_MINUTES)) + .to_rfc3339_opts(SecondsFormat::Millis, true); + let recent: i64 = conn.query_row( + "SELECT COUNT(*) FROM note_revisions WHERE note_id = ?1 AND created_at >= ?2", + params![id, cutoff], + |r| r.get(0), + )?; + Ok(recent == 0) +} + fn snapshot_revision(conn: &Connection, id: &str) -> rusqlite::Result<()> { let body: String = conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0))?; @@ -391,9 +485,12 @@ pub fn update_note(conn: &Connection, id: &str, changes: &Value) -> rusqlite::Re .as_object() .ok_or_else(|| rusqlite::Error::InvalidParameterName("changes must be an object".into()))?; - // Snapshot the pre-edit body before changing it (version history). - if obj.contains_key("body") { - snapshot_revision(conn, id)?; + // Snapshot the pre-edit body before changing it (version history) — but only + // once per editing session, and only if it actually changed. See should_snapshot. + if let Some(body) = obj.get("body").and_then(|v| v.as_str()) { + if should_snapshot(conn, id, body)? { + snapshot_revision(conn, id)?; + } } for (k, v) in obj { @@ -404,12 +501,7 @@ pub fn update_note(conn: &Connection, id: &str, changes: &Value) -> rusqlite::Re "UPDATE notes SET body = ?1 WHERE id = ?2", params![body, id], )?; - sync_tags(conn, id, body)?; - } - "color" => { - if let Some(s) = v.as_str() { - conn.execute("UPDATE notes SET color = ?1 WHERE id = ?2", params![s, id])?; - } + lift_and_sync_tags(conn, id, body)?; } "pinned" => { if let Some(b) = v.as_bool() { @@ -508,18 +600,32 @@ pub fn set_labels(conn: &Connection, id: &str, label_ids: &[String]) -> rusqlite load_note(conn, id) } +// ---- checklist items: every one of these is a body edit --------------------- +// +// They keep their own names and signatures because the FFI, the Tauri commands and +// the REST shape all speak in items, and a checklist is still a thing a note HAS. +// What changed is where it is kept. Routing all three through `update_note` rather +// than writing the body directly is what gives them revision snapshotting, `#tag` +// re-derivation and the dirty/updated_at bookkeeping without any of it being +// written a second time here. + +fn note_body(conn: &Connection, id: &str) -> rusqlite::Result { + conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0)) +} + +/// An item's id is its ordinal (see [items_of]). Anything else is a stale id from a +/// UI that has not reloaded, and the right answer to those is to do nothing. +fn item_index(item_id: &str) -> Option { + item_id.parse::().ok() +} + +fn set_body(conn: &Connection, id: &str, body: String) -> rusqlite::Result { + update_note(conn, id, &json!({ "body": body })) +} + pub fn add_item(conn: &Connection, id: &str, text: &str) -> rusqlite::Result { - let pos: i64 = conn.query_row( - "SELECT COALESCE(MAX(position), -1) + 1 FROM checklist_items WHERE note_id = ?1", - [id], - |r| r.get(0), - )?; - conn.execute( - "INSERT INTO checklist_items (id, note_id, text, position) VALUES (?1, ?2, ?3, ?4)", - params![new_id(), id, text, pos], - )?; - touch(conn, id)?; - load_note(conn, id) + let body = note_body(conn, id)?; + set_body(conn, id, derive::append_item(&body, text, false)) } pub fn update_item( @@ -528,29 +634,27 @@ pub fn update_item( item_id: &str, changes: &Value, ) -> rusqlite::Result { + let index = match item_index(item_id) { + Some(i) => i, + None => return load_note(conn, id), + }; + let mut body = note_body(conn, id)?; if let Some(text) = changes.get("text").and_then(Value::as_str) { - conn.execute( - "UPDATE checklist_items SET text = ?1 WHERE id = ?2 AND note_id = ?3", - params![text, item_id, id], - )?; + body = derive::set_item_text(&body, index, text); } if let Some(checked) = changes.get("checked").and_then(Value::as_bool) { - conn.execute( - "UPDATE checklist_items SET checked = ?1 WHERE id = ?2 AND note_id = ?3", - params![checked, item_id, id], - )?; + body = derive::set_item_checked(&body, index, checked); } - touch(conn, id)?; - load_note(conn, id) + set_body(conn, id, body) } pub fn delete_item(conn: &Connection, id: &str, item_id: &str) -> rusqlite::Result { - conn.execute( - "DELETE FROM checklist_items WHERE id = ?1 AND note_id = ?2", - params![item_id, id], - )?; - touch(conn, id)?; - load_note(conn, id) + let index = match item_index(item_id) { + Some(i) => i, + None => return load_note(conn, id), + }; + let body = note_body(conn, id)?; + set_body(conn, id, derive::remove_item(&body, index)) } pub fn delete_attachment(conn: &Connection, id: &str, att_id: &str) -> rusqlite::Result { @@ -668,7 +772,7 @@ pub fn restore_revision(conn: &Connection, id: &str, rev_id: &str) -> rusqlite:: "UPDATE notes SET body = ?1 WHERE id = ?2", params![body, id], )?; - sync_tags(conn, id, &body)?; + lift_and_sync_tags(conn, id, &body)?; touch(conn, id)?; load_note(conn, id) } diff --git a/core/src/sync/compat.rs b/core/src/sync/compat.rs index 42104e1..d2cf77a 100644 --- a/core/src/sync/compat.rs +++ b/core/src/sync/compat.rs @@ -19,11 +19,25 @@ use serde::{Deserialize, Serialize}; /// The sync wire protocol this client speaks. -pub const CLIENT_PROTOCOL_VERSION: u32 = 2; +/// +/// v4 (M315): `color` left the note. NOT a floor raise on either side — see the note +/// on [`MIN_SERVER_PROTOCOL_VERSION`]. +pub const CLIENT_PROTOCOL_VERSION: u32 = 4; /// The oldest server protocol this client can drive — the symmetric half of the /// server's `min_client_protocol_version`. -pub const MIN_SERVER_PROTOCOL_VERSION: u32 = 2; +/// +/// STAYS AT 3 ACROSS v4, and the v2 precedent is the reason to say why rather than +/// leave it looking like an oversight. v2 dropped `kind` and `title` and DID move both +/// floors, on the rule that "dropping a field a client sends and expects back is +/// breaking". `color` fails that test on the second half: a v3 client reading a v4 +/// server gets `"default"` from serde's default and draws the colour it derives +/// locally, which is a board that looks exactly like the one it drew yesterday. A v3 +/// client PUSHING `color` to a v4 server has the key ignored — the server reads its +/// payload key by key and never validates the shape. Neither direction errors, and +/// neither loses anything a person can see; `title` was the note's NAME, and this is a +/// field that no longer renders anywhere. +pub const MIN_SERVER_PROTOCOL_VERSION: u32 = 3; /// Capabilities without which syncing is meaningless, so their absence BLOCKS the /// link rather than degrading it. diff --git a/core/src/sync/pull.rs b/core/src/sync/pull.rs index 977618c..abdace2 100644 --- a/core/src/sync/pull.rs +++ b/core/src/sync/pull.rs @@ -240,13 +240,12 @@ fn upsert_note(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { // `created_at` is deliberately absent from the UPDATE clause: a note's birth time // never changes, and the server's copy is the same value anyway. conn.execute( - "INSERT INTO notes (id, body, color, position, pinned, archived, + "INSERT INTO notes (id, body, position, pinned, archived, trashed, remind_at, recurrence, created_at, updated_at, sync_revision, trashed_at, dirty) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, 0) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, 0) ON CONFLICT(id) DO UPDATE SET body = excluded.body, - color = excluded.color, position = excluded.position, pinned = excluded.pinned, archived = excluded.archived, @@ -260,7 +259,6 @@ fn upsert_note(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { params![ note.id, note.body, - note.color, note.position, note.pinned, note.archived, @@ -277,34 +275,12 @@ fn upsert_note(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { // Children are replaced wholesale: a delta carries the note's FULL current state, // so "what the server sent" IS the complete set. Diffing would be more code and // could leave behind a row the server no longer has. - replace_items(conn, note)?; replace_attachments(conn, note)?; replace_previews(conn, note)?; replace_labels(conn, note)?; Ok(()) } -fn replace_items(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { - conn.execute( - "DELETE FROM checklist_items WHERE note_id = ?1", - params![note.id], - )?; - for (index, item) in note.items.iter().enumerate() { - conn.execute( - "INSERT INTO checklist_items (id, note_id, text, checked, position) - VALUES (?1, ?2, ?3, ?4, ?5)", - params![ - item.id, - note.id, - item.text, - item.checked, - position_of(item.position, index) - ], - )?; - } - Ok(()) -} - fn replace_attachments(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { conn.execute( "DELETE FROM attachments WHERE note_id = ?1", @@ -393,16 +369,6 @@ fn ensure_label_stub(conn: &Connection, label: &wire::NoteLabel) -> rusqlite::Re Ok(()) } -/// Trust an explicit position; fall back to arrival order when the server sent 0 for -/// everything (which is what an unordered list looks like on the wire). -fn position_of(explicit: i64, index: usize) -> i64 { - if explicit > 0 { - explicit - } else { - index as i64 - } -} - /// Loop the feed to exhaustion, starting from the persisted cursor. /// /// NOTE ON ORDERING: the full cycle is push-then-pull (docs/sync.md). Running this @@ -495,7 +461,6 @@ mod tests { wire::Note { id: id.to_string(), body: "Body".into(), - color: "default".into(), position: 0, pinned: false, archived: false, @@ -508,12 +473,22 @@ mod tests { sync_revision: revision, purged_at: None, labels: vec![], - items: vec![], attachments: vec![], previews: vec![], } } + fn attachment(id: &str) -> wire::Attachment { + wire::Attachment { + id: id.to_string(), + url: "/blob/x".into(), + filename: None, + mime: "image/png".into(), + size: None, + sha256: None, + } + } + fn page(notes: Vec, labels: Vec, cursor: i64) -> wire::ChangesPage { wire::ChangesPage { notes, @@ -612,35 +587,20 @@ mod tests { #[test] fn children_are_replaced_not_merged() { + // Was written over checklist items; they are lines of the body now (M304), so + // attachments carry the point instead. It is the same property either way: a + // delta is the note's FULL current state, so a child the server dropped has to + // disappear locally rather than linger. let conn = db(); let mut first = note("n1", 1); - first.items = vec![ - wire::Item { - id: "i1".into(), - text: "one".into(), - checked: false, - position: 0, - }, - wire::Item { - id: "i2".into(), - text: "two".into(), - checked: false, - position: 1, - }, - ]; + first.attachments = vec![attachment("a1"), attachment("a2")]; apply_page(&conn, &page(vec![first], vec![], 1)).expect("apply"); - assert_eq!(count(&conn, "SELECT COUNT(*) FROM checklist_items"), 2); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM attachments"), 2); - // The server dropped an item; the local copy must drop it too. let mut second = note("n1", 2); - second.items = vec![wire::Item { - id: "i1".into(), - text: "one".into(), - checked: true, - position: 0, - }]; + second.attachments = vec![attachment("a1")]; apply_page(&conn, &page(vec![second], vec![], 2)).expect("apply"); - assert_eq!(count(&conn, "SELECT COUNT(*) FROM checklist_items"), 1); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM attachments"), 1); } #[test] @@ -810,23 +770,10 @@ mod tests { fn a_page_that_fails_leaves_the_cursor_untouched() { // Atomicity is the whole resumability story: a cursor committed ahead of its // data would skip those rows forever. Force a failure with a duplicate - // checklist-item id inside one page. + // attachment id inside one page. let conn = db(); let mut n = note("n1", 3); - n.items = vec![ - wire::Item { - id: "dup".into(), - text: "one".into(), - checked: false, - position: 0, - }, - wire::Item { - id: "dup".into(), - text: "two".into(), - checked: false, - position: 1, - }, - ]; + n.attachments = vec![attachment("dup"), attachment("dup")]; assert!(apply_page(&conn, &page(vec![n], vec![], 3)).is_err()); assert_eq!(state::read(&conn).expect("state").last_cursor, 0); assert_eq!(count(&conn, "SELECT COUNT(*) FROM notes"), 0); diff --git a/core/src/sync/push.rs b/core/src/sync/push.rs index 831629e..535d18d 100644 --- a/core/src/sync/push.rs +++ b/core/src/sync/push.rs @@ -64,6 +64,8 @@ pub struct Change { #[serde(skip_serializing_if = "Option::is_none")] pub body: Option, + /// A LABEL's colour. A note has none since M315, so a note change leaves this + /// `None` and the key never reaches the wire. #[serde(skip_serializing_if = "Option::is_none")] pub color: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -79,8 +81,6 @@ pub struct Change { #[serde(skip_serializing_if = "Option::is_none")] pub position: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub items: Option>, - #[serde(skip_serializing_if = "Option::is_none")] pub label_ids: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub created_at: Option, @@ -103,7 +103,6 @@ impl Change { remind_at: None, recurrence: None, position: None, - items: None, label_ids: None, created_at: None, name: None, @@ -111,12 +110,6 @@ impl Change { } } -#[derive(Debug, Serialize)] -pub struct ItemOut { - pub text: String, - pub checked: bool, -} - // --- incoming results -------------------------------------------------------- #[derive(Debug, Deserialize)] @@ -201,7 +194,6 @@ fn collect_labels(conn: &Connection, out: &mut Vec, limit: usize) -> rus remind_at: None, recurrence: None, position: None, - items: None, label_ids: None, created_at: None, }) @@ -230,7 +222,6 @@ fn collect_notes(conn: &Connection, out: &mut Vec, limit: usize) -> rusq /// field-to-column mapping stays readable at the call site. struct NoteRow { body: String, - color: String, position: i64, pinned: bool, archived: bool, @@ -243,22 +234,21 @@ struct NoteRow { fn note_row(conn: &Connection, id: &str) -> rusqlite::Result { conn.query_row( - "SELECT body, color, position, pinned, archived, trashed, + "SELECT body, position, pinned, archived, trashed, remind_at, recurrence, created_at, updated_at FROM notes WHERE id = ?1", params![id], |r| { Ok(NoteRow { body: r.get(0)?, - color: r.get(1)?, - position: r.get(2)?, - pinned: r.get::<_, i64>(3)? != 0, - archived: r.get::<_, i64>(4)? != 0, - trashed: r.get::<_, i64>(5)? != 0, - remind_at: r.get(6)?, - recurrence: r.get(7)?, - created_at: r.get(8)?, - updated_at: r.get(9)?, + position: r.get(1)?, + pinned: r.get::<_, i64>(2)? != 0, + archived: r.get::<_, i64>(3)? != 0, + trashed: r.get::<_, i64>(4)? != 0, + remind_at: r.get(5)?, + recurrence: r.get(6)?, + created_at: r.get(7)?, + updated_at: r.get(8)?, }) }, ) @@ -267,19 +257,6 @@ fn note_row(conn: &Connection, id: &str) -> rusqlite::Result { fn note_change(conn: &Connection, id: &str) -> rusqlite::Result { let row = note_row(conn, id)?; - let items = { - let mut stmt = conn.prepare( - "SELECT text, checked FROM checklist_items WHERE note_id = ?1 ORDER BY position", - )?; - let rows = stmt.query_map(params![id], |r| { - Ok(ItemOut { - text: r.get(0)?, - checked: r.get::<_, i64>(1)? != 0, - }) - })?; - rows.collect::>>()? - }; - // MANUAL memberships only. Tag-sourced ones (`via_tag = 1`) are re-derived by the // server from the body; sending them as label_ids would convert them into manual // assignments that no longer disappear when the #tag is removed from the text. @@ -298,14 +275,14 @@ fn note_change(conn: &Connection, id: &str) -> rusqlite::Result { // server's last-write-wins comparison runs against. edited_at: row.updated_at, body: Some(row.body), - color: Some(row.color), + // A note has no colour to send. See the field on `Change`. + color: None, pinned: Some(row.pinned), archived: Some(row.archived), trashed: Some(row.trashed), remind_at: row.remind_at, recurrence: row.recurrence, position: Some(row.position), - items: Some(items), label_ids: Some(label_ids), created_at: Some(row.created_at), name: None, @@ -521,9 +498,9 @@ mod tests { fn seed_note(conn: &Connection, id: &str, dirty: i64) { conn.execute( - "INSERT INTO notes (id, body, color, position, pinned, archived, + "INSERT INTO notes (id, body, position, pinned, archived, trashed, created_at, updated_at, sync_revision, dirty) - VALUES (?1, 'B', 'default', 0, 0, 0, 0, + VALUES (?1, 'B', 0, 0, 0, 0, '2026-07-26T00:00:00.000Z', '2026-07-26T00:00:00.000Z', 3, ?2)", params![id, dirty], ) diff --git a/core/src/sync/wire.rs b/core/src/sync/wire.rs index 87eb9d9..8ad2d77 100644 --- a/core/src/sync/wire.rs +++ b/core/src/sync/wire.rs @@ -25,8 +25,6 @@ pub struct Note { pub id: String, #[serde(default)] pub body: String, - #[serde(default = "default_color")] - pub color: String, #[serde(default)] pub position: i64, #[serde(default)] @@ -58,8 +56,6 @@ pub struct Note { #[serde(default)] pub labels: Vec, #[serde(default)] - pub items: Vec, - #[serde(default)] pub attachments: Vec, #[serde(default)] pub previews: Vec, @@ -87,17 +83,6 @@ pub struct NoteLabel { pub via_tag: bool, } -#[derive(Debug, Clone, Deserialize)] -pub struct Item { - pub id: String, - #[serde(default)] - pub text: String, - #[serde(default)] - pub checked: bool, - #[serde(default)] - pub position: i64, -} - #[derive(Debug, Clone, Deserialize)] pub struct Attachment { pub id: String, diff --git a/desktop/packaging/arch/README.md b/desktop/packaging/arch/README.md index 78ae1f0..03a6982 100644 --- a/desktop/packaging/arch/README.md +++ b/desktop/packaging/arch/README.md @@ -18,7 +18,7 @@ pacman system: curl -fsSL https://git.fabledsword.com/bvandeusen/thoughtsync/raw/branch/dev/desktop/packaging/install.sh | sh ``` -That installs the newest tagged release. To follow the rolling development +That installs the newest build from `main`. To follow the rolling development channel instead, pass the flag through the pipe: ```sh diff --git a/desktop/packaging/arch/package-prebuilt.sh b/desktop/packaging/arch/package-prebuilt.sh index e9e449a..0d1fa60 100755 --- a/desktop/packaging/arch/package-prebuilt.sh +++ b/desktop/packaging/arch/package-prebuilt.sh @@ -64,7 +64,9 @@ DEPENDS=(webkit2gtk-4.1 gtk3) # this?" question unanswerable. # `|| true` so a miss falls through to the explicit error below rather than # aborting on pipefail with no explanation. -PKGVER="$(sh "$SCRIPT_DIR/../build-version.sh" || true)" +# The ORDERING KEY: pacman compares this, and it must match the filename the +# bundle build produced (write-manifest.sh selects on it). +PKGVER="$(sh "$SCRIPT_DIR/../../../packaging/version.sh" key desktop || true)" [ -n "$PKGVER" ] || { echo "ERROR: could not determine the build version" >&2; exit 1; } # Reproducible-ish: prefer the commit date over "now" so rebuilding the same diff --git a/desktop/packaging/build-version.sh b/desktop/packaging/build-version.sh deleted file mode 100755 index 9f3dd5b..0000000 --- a/desktop/packaging/build-version.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env sh -# -# Echo the version this build should carry. One definition, used in three places -# (both bundle jobs and the manifest writer) — if they ever disagreed, the app would -# compare its own version against a manifest describing a different build, and the -# updater would either offer nothing or loop forever offering the same thing. -# -# WHY DEV BUILDS NEED THEIR OWN VERSION AT ALL: -# an updater decides by comparing semver. Every dev build carries the version in -# Cargo.toml, so without this they'd all be `0.1.0` — an installed build would see a -# manifest advertising the version it already has, conclude it was current, and never -# update. The rolling channel needs a number that actually rises. -# -# The CI run number is that number: monotonic, already unique per build, and it needs -# no state carried between runs. `0.1.0` + run 2932 becomes `0.1.2932`. -# -# Plain semver on purpose, NOT a `-dev.N` prerelease tag: prerelease versions sort -# BELOW the release they qualify (`0.1.0-dev.5` < `0.1.0`), so a tagged build would -# never update to a newer dev one, and Windows installer metadata wants a numeric -# X.Y.Z anyway. Bumping the minor in Cargo.toml still wins over any dev build on the -# old line, which is the ordering you want: 0.2.0 > 0.1.2932. -set -eu - -CARGO_TOML="$(dirname "$0")/../src-tauri/Cargo.toml" -base="$(grep -m1 '^version' "$CARGO_TOML" | sed -E 's/.*"([^"]+)".*/\1/')" - -# Dev builds only. Anything else (a v* tag, main) ships the version as written. -if [ "${GITHUB_REF_NAME:-}" = "dev" ] && [ -n "${GITHUB_RUN_NUMBER:-}" ]; then - printf '%s.%s\n' "${base%.*}" "$GITHUB_RUN_NUMBER" -else - printf '%s\n' "$base" -fi diff --git a/desktop/packaging/install.sh b/desktop/packaging/install.sh index 30cce5c..385fc8a 100755 --- a/desktop/packaging/install.sh +++ b/desktop/packaging/install.sh @@ -5,8 +5,12 @@ # curl -fsSL https://git.fabledsword.com/bvandeusen/thoughtsync/raw/branch/dev/desktop/packaging/install.sh | sh # # Two channels, the SAME two the app's own updater offers (src-tauri/src/update.rs): -# stable (default) — the newest tagged v* release. +# stable (default) — the rolling build from every merge to `main`. # dev — the rolling build from every green push to `dev`. +# Both are fixed-tag releases: the tag never moves and the assets are pruned to the +# current build, so the tag alone names the newest one. `stable` only became one in +# M314 step 3, when `main` started publishing — before that it was a manifest-only +# pointer at whatever `v*` tag somebody had last cut. # Pick one with `--channel dev` or `TS_CHANNEL=dev`. Through a pipe the options go # after a `--`: curl -fsSL | sh -s -- --channel dev # @@ -41,7 +45,7 @@ ThoughtSync desktop installer. install.sh [--channel stable|dev] - --channel stable newest tagged release (default) + --channel stable newest build from main (default) --channel dev rolling build from the latest green push to `dev` -h, --help this text @@ -82,20 +86,23 @@ esac # --- resolve the release for this channel ----------------------------------- say "Finding the latest ThoughtSync build on the $channel channel…" -if [ "$channel" = "dev" ]; then - # A release whose tag never moves and whose assets are pruned to the current - # build — so the tag alone always names the newest dev build. - json="$(curl -fsSL "$API/releases/tags/dev" 2>/dev/null)" || - die "the dev channel has nothing published yet." -else - # Ask the stable channel's own manifest which version is current, then install - # THAT release. This is the same file the in-app updater reads, so the installer - # and the updater can never disagree about what `stable` means. - # - # Not `/releases/latest`: that returns the newest non-prerelease release by date, - # and the `stable` pointer release (manifest only, no bundles — see - # write-manifest.sh) is itself a non-prerelease created moments after the - # versioned one. It would win, and it carries nothing installable. +# ONE lookup for both channels now. Each is a release whose tag never moves and whose +# assets are pruned to the current build, so the tag alone names the newest build on +# that channel — which is exactly what an installer wants and what the in-app updater +# already reads. +json="$(curl -fsSL "$API/releases/tags/$channel" 2>/dev/null)" || + die "the $channel channel has nothing published yet." + +# TRANSITIONAL — delete with the rest of the old scheme (M314 step 7). +# +# `stable` existed before this as a manifest-ONLY pointer: `latest.json` naming a +# version whose bundles lived on a separate `v` release. Between this commit +# and the first merge to `main` it still looks like that, and `stable` is the DEFAULT +# channel — so without this fallback `curl … | sh` is broken for everyone in that +# window. It costs nothing once main has published: the grep finds the bundles and +# this branch never runs again. +if [ "$channel" = "stable" ] && ! printf '%s' "$json" | grep -q "releases/download/stable/[^\"]*\.\(AppImage\|deb\|pkg\.tar\)"; then + say "stable has no bundles of its own yet — falling back to the version its manifest names." manifest="$(curl -fsSL "$INSTANCE/$REPO/releases/download/stable/latest.json" 2>/dev/null || true)" stable_version="$(printf '%s' "$manifest" | grep -oE '"version"[[:space:]]*:[[:space:]]*"[^"]+"' | head -1 | @@ -104,11 +111,8 @@ else json="$(curl -fsSL "$API/releases/tags/v$stable_version" 2>/dev/null)" || die "the stable channel names $stable_version, but there is no v$stable_version release to install." else - # No stable pointer yet — the channel predates the updater. Fall back to the - # newest non-prerelease release, which is what stable meant before there was - # a manifest to ask. json="$(curl -fsSL "$API/releases/latest" 2>/dev/null)" || - die "no stable release published yet — try --channel dev, or ask the maintainer to tag one." + die "no stable build published yet — try --channel dev, or merge to main." fi fi diff --git a/desktop/packaging/publish-release.sh b/desktop/packaging/publish-release.sh index ae27987..0305d11 100755 --- a/desktop/packaging/publish-release.sh +++ b/desktop/packaging/publish-release.sh @@ -118,6 +118,11 @@ first_id() { grep -oE '"id"[[:space:]]*:[[:space:]]*[0-9]+' | head -1 | grep -oE # install.sh defaults to stable, so the rolling dev release must opt in explicitly # — otherwise someone following the instructions here lands on a tagged build and # wonders why the version they were sent isn't what they got. +# +# Both CHANNELS are rolling pointer releases (M314 step 3): `dev` republishes on +# every green push to dev, `stable` on every merge to main. Each says so, because a +# release that prunes its own assets behaves differently from a versioned one and a +# reader deserves to know which they are looking at. if [ "$TAG" = "dev" ]; then INSTALL_TAIL='sh -s -- --channel dev' # Backticks BARE, not `\``. The heredoc below is unquoted, so there the backslash @@ -125,6 +130,10 @@ if [ "$TAG" = "dev" ]; then # Here single quotes already do that job, so a backslash would survive into the # body as `\``, which is not a legal JSON escape: Forgejo answers 422. CHANNEL_NOTE='\n\nThis is the rolling **dev** channel: republished on every green push to `dev`, and pruned to the current build.' +elif [ "$TAG" = "stable" ]; then + # install.sh defaults to stable, so no flag. + INSTALL_TAIL='sh' + CHANNEL_NOTE='\n\nThis is the rolling **stable** channel: republished on every merge to `main`, and pruned to the current build. No tag is required for a build to arrive here.' else INSTALL_TAIL='sh' CHANNEL_NOTE='' @@ -136,6 +145,21 @@ BODY=$(cat < Manifest:" cat "$work/latest.json" -# --- resolve the release the manifest is published TO ------------------------ -if [ "$MANIFEST_TAG" = "$RELEASE_TAG" ]; then - target_id="$release_id" - target_assets="$assets" -else - echo "==> Resolving the $MANIFEST_TAG channel release" - target="$(curl -sS "${AUTH[@]}" "$API/releases/tags/$MANIFEST_TAG")" - target_id="$(printf '%s' "$target" | grep -oE '"id"[[:space:]]*:[[:space:]]*[0-9]+' | head -1 | grep -oE '[0-9]+' || true)" - if [ -z "${target_id:-}" ]; then - # First publish to this channel. A pointer release: no bundles of its own, just - # a permanent tag for the manifest to live under. - echo " creating it (pointer release, manifest only)" - body="{\"tag_name\":\"$MANIFEST_TAG\",\"name\":\"ThoughtSync ($MANIFEST_TAG channel)\",\"draft\":false,\"prerelease\":false,\"body\":\"Update channel pointer. The installable builds live on the versioned releases; this holds only the updater manifest.\"}" - target="$(curl -sS -X POST "${AUTH[@]}" -H "Content-Type: application/json" -d "$body" "$API/releases")" - target_id="$(printf '%s' "$target" | grep -oE '"id"[[:space:]]*:[[:space:]]*[0-9]+' | head -1 | grep -oE '[0-9]+')" - fi - [ -n "${target_id:-}" ] || { echo "ERROR: could not resolve the $MANIFEST_TAG release" >&2; exit 1; } - target_assets="$(curl -sS "${AUTH[@]}" "$API/releases/$target_id/assets")" -fi +# The manifest goes on the same release the bundles were just read from — which is +# also the one `publish-release.sh` created or refreshed moments earlier, so it is +# guaranteed to exist by the time this runs. +target_id="$release_id" +target_assets="$assets" # Replace rather than duplicate: Forgejo rejects a second asset with the same name, # and this file is rewritten on every publish by design. @@ -145,11 +132,11 @@ if [ -n "${old_id:-}" ]; then curl -fsS -X DELETE "${AUTH[@]}" "$API/releases/$target_id/assets/$old_id" >/dev/null fi -echo "==> Uploading latest.json to $MANIFEST_TAG" +echo "==> Uploading latest.json to $RELEASE_TAG" curl -fsS -X POST "${AUTH[@]}" "$API/releases/$target_id/assets?name=latest.json" \ -F "attachment=@$work/latest.json" >/dev/null -echo "==> Done. $MANIFEST_TAG now advertises $APP_VERSION for ${#entries[@]} platform(s)." +echo "==> Done. $RELEASE_TAG now advertises $APP_VERSION for ${#entries[@]} platform(s)." # --- prune superseded builds from a rolling channel --------------------------- # diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index a43e60e..f7720e1 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -1,5 +1,17 @@ [package] name = "thoughtsync-desktop" +# NOT THE SHIPPED VERSION, and bumping it has no effect on anything a user sees. +# +# Cargo requires a version here, and Tauri reads one from `tauri.conf.json` — both +# are overridden per build by `cargo tauri build --config '{"version": ...}'` with +# the value `packaging/version.sh key desktop` derives. See #3144. +# +# It used to matter: the old scheme took its base from this line and appended the CI +# run number on dev, so `0.2.` on dev sat against a bare `0.2.0` on main and +# every dev build outranked every stable one. The remedy was "remember to bump the +# minor before tagging" — documented in a comment, enforced nowhere, and #2183 is +# what that looked like in the field. A scheme needing a human to remember something +# before each release has not removed the decision, only hidden it. version = "0.2.0" description = "ThoughtSync desktop — local-first Keep-style thought capture" authors = ["bvandeusen"] diff --git a/desktop/src-tauri/src/update.rs b/desktop/src-tauri/src/update.rs index c343d4a..ae25bd2 100644 --- a/desktop/src-tauri/src/update.rs +++ b/desktop/src-tauri/src/update.rs @@ -1,10 +1,16 @@ //! In-app updates (M10.9). //! -//! Two channels, because two audiences: `stable` follows tagged `v*` releases, -//! `dev` follows every green push. Each reads a `latest.json` published as an asset -//! on a release whose TAG NEVER CHANGES — verified necessary, because Forgejo has no -//! `/releases/latest/download/` route (it 404s), so "newest" cannot be named -//! in a URL. A fixed tag can. +//! Two channels, because two audiences: `stable` follows every merge to `main`, +//! `dev` follows every green push to `dev`. Each reads a `latest.json` published as +//! an asset on a release whose TAG NEVER CHANGES — verified necessary, because +//! Forgejo has no `/releases/latest/download/` route (it 404s), so "newest" +//! cannot be named in a URL. A fixed tag can. +//! +//! `stable` followed tagged `v*` releases until M314 step 3, and its manifest pointed +//! at bundles living on a different release. It holds its own bundles now, exactly as +//! `dev` always has — so a build reaches stable users with no tag cut anywhere, which +//! is the whole point of the change. NOTHING HERE MOVED: this code only ever read +//! `/latest.json`, and that is still where the manifest lands. //! //! The feed lives on Fabled-Git rather than on a ThoughtSync server, deliberately: //! this app is usable having never linked a server, and an install that can't reach diff --git a/docs/android-distribution.md b/docs/android-distribution.md index e69d836..506ea96 100644 --- a/docs/android-distribution.md +++ b/docs/android-distribution.md @@ -15,13 +15,19 @@ cannot talk to. **Normally: nowhere. It is already in the image.** -CI fetches the newest published Android build into every server image it builds, -so `:dev`, `:latest` and `:` all ship a client. `docker compose pull && -docker compose up -d` delivers a new server and a new client together, and there -is nothing to copy. +CI fetches the published Android build into every server image it builds, so +`:dev` and `:latest` both ship a client. `docker compose pull && docker compose +up -d` delivers a new server and a new client together, and there is nothing to +copy. -A versioned image therefore carries the *newest* client rather than one pinned to -that version. That is deliberate: the two negotiate a sync protocol version +**The channel is a property of the image you run.** A `:dev` image bakes in the +dev-channel APK, `:latest` the stable one — so pointing a phone at a stable +server gets it a stable client, with no second place holding that decision. (Until +M314 step 3 the fetch was hard-wired to the dev release on every branch, so a +stable server served a dev client.) + +An image therefore carries the *newest* client on its channel rather than one +pinned to a version. That is deliberate: the two negotiate a sync protocol version before they link, so a mismatch is caught by the handshake rather than by pinning. diff --git a/docs/sync.md b/docs/sync.md index 709c81f..d9e6e46 100644 --- a/docs/sync.md +++ b/docs/sync.md @@ -52,6 +52,15 @@ syncs everything else. ### The policy - **Any wire change** → bump `SYNC_PROTOCOL_VERSION`. + - v2 (M13): `kind` and `title` left the wire; **floor raised**, because a v1 + client kept pushing both and read back notes carrying neither — and `title` was + the note's NAME, so an old client showed nameless notes. + - v3: attachments/tombstones/revisions. + - v4 (M315): `color` left the note; **floor NOT raised**. Both directions degrade + in silence and neither loses anything visible — an old client reading a v4 note + falls back to the colour it derives locally, and one pushing `color` has the key + ignored. The test is not "did a field leave" but "does either side end up + showing something wrong". - **Additive change** (a new field, a new capability) → add a `sync_features` name. Do **not** raise a minimum. Old clients keep working. - **Breaking change only** → raise `MIN_CLIENT_PROTOCOL_VERSION` (or the client's @@ -190,18 +199,24 @@ Body: `{ "changes": [ ... ] }` (max 1000 per batch). Each change: ```json { "entity": "note", "id": "", "op": "upsert", "edited_at": "", - "title": "...", "body": "...", "color": "blue", "kind": "text", + "body": "...", "pinned": false, "archived": false, "trashed": false, "remind_at": null, - "position": 0, "items": [ {"text": "...", "checked": false} ], + "recurrence": null, "position": 0, "label_ids": ["", ...], "created_at": "" } ``` - **Client-generated ids.** Notes/labels are UUIDs; the client mints the id when it creates the row offline and sends it here. Create-if-absent, else update. - **Whole-note semantics.** A note upsert carries the client's *full* current - state (not a partial patch) — the server overwrites all scalar fields, replaces - items, and sets manual label memberships from `label_ids` (tag-sourced labels - are re-derived from the body). `#tags` are recomputed server-side. + state (not a partial patch) — the server overwrites all scalar fields and sets + manual label memberships from `label_ids` (tag-sourced labels are re-derived + from the body). `#tags` are recomputed server-side. A checklist is `- [ ] ` lines + inside `body` (M304), so there is no separate `items` array. +- **Fields a change may still carry, and the server reads past.** `title` and + `kind` (removed in v2), `items` (M304) and `color` (v4, M315). The server reads + its payload key by key and never validates the shape, which is exactly what lets + an older client keep pushing a field this one has stopped storing — see the + version policy above for why none of those needed a floor raise on their own. - **`op: "delete"`** purges (tombstones) the row. Trashing is just an upsert with `trashed: true`. - **Labels:** `{entity: "label", op: "upsert"|"delete", id, edited_at, name, diff --git a/frontend/src/adapters/repo.ts b/frontend/src/adapters/repo.ts index 3573952..66be22e 100644 --- a/frontend/src/adapters/repo.ts +++ b/frontend/src/adapters/repo.ts @@ -9,7 +9,6 @@ // consume. Client-side logic (list reconciliation, optimistic updates, toasts) // stays in the stores — the repo is data access only. -import type { NoteColor } from "../notes/colors"; import type { Note, NoteFacets, NoteView, NoteRevision } from "../stores/notes"; import type { Label } from "../stores/labels"; import type { SavedFilter } from "../stores/savedFilters"; @@ -33,13 +32,12 @@ export interface NoteListQuery { export interface NoteCreateInput { body: string; - color: NoteColor; items?: string[]; } // The mutable subset of a note (PATCH /api/notes/:id). export type NoteChanges = Partial< - Pick + Pick >; export interface ChecklistItemChanges { diff --git a/frontend/src/adapters/rest.ts b/frontend/src/adapters/rest.ts index 12efa73..651197a 100644 --- a/frontend/src/adapters/rest.ts +++ b/frontend/src/adapters/rest.ts @@ -31,7 +31,6 @@ function notesQuery(q: NoteListQuery): string { if (q.labelId) params.append("label", q.labelId); for (const id of q.facets?.label ?? []) if (id) params.append("label", id); if (q.facets?.q) params.set("q", q.facets.q); - if (q.facets?.color) params.set("color", q.facets.color); if (q.facets?.has_reminder) params.set("has_reminder", "true"); if (q.facets?.has_attachment) params.set("has_attachment", "true"); if (q.facets?.created_after) params.set("created_after", q.facets.created_after); diff --git a/frontend/src/components/AppShell.vue b/frontend/src/components/AppShell.vue index 9db9c1b..1d601ab 100644 --- a/frontend/src/components/AppShell.vue +++ b/frontend/src/components/AppShell.vue @@ -14,7 +14,7 @@ import ImportNotes from "./ImportNotes.vue"; import LabelsModal from "./LabelsModal.vue"; import { isDesktop } from "../desktop/bridge"; import { facetsToQuery } from "../notes/facets"; -import { NOTE_SWATCH_CLASSES, type NoteColor } from "../notes/colors"; +import { NOTE_SWATCH_CLASSES, resolveLabelColor } from "../notes/colors"; const route = useRoute(); const router = useRouter(); @@ -173,8 +173,10 @@ onBeforeUnmount(() => { const currentLabelId = computed(() => (route.name === "label" ? String(route.params.id) : null)); -function labelDot(color: string): string { - return NOTE_SWATCH_CLASSES[color as NoteColor] ?? NOTE_SWATCH_CLASSES.default; +// The drawer's tag list. Same resolution as every other chip and dot — a tag that is +// green on a card must be green here, or the sidebar stops being a way to find it. +function labelDot(label: { name: string; color: string }): string { + return NOTE_SWATCH_CLASSES[resolveLabelColor(label)] ?? NOTE_SWATCH_CLASSES.default; } // The board lenses — the routes a search can happen *within*. Searching while looking @@ -443,7 +445,7 @@ async function signOut() { > {{ lb.name }} diff --git a/frontend/src/components/ColorPicker.vue b/frontend/src/components/ColorPicker.vue deleted file mode 100644 index c6fce03..0000000 --- a/frontend/src/components/ColorPicker.vue +++ /dev/null @@ -1,24 +0,0 @@ - - - diff --git a/frontend/src/components/FilterBar.vue b/frontend/src/components/FilterBar.vue index 9adcbd0..f5f6839 100644 --- a/frontend/src/components/FilterBar.vue +++ b/frontend/src/components/FilterBar.vue @@ -7,7 +7,6 @@ import { useUiStore } from "../stores/ui"; import type { NoteFacets } from "../stores/notes"; import { facetCount, facetsFromQuery, facetsToQuery } from "../notes/facets"; import { addLocalDays, formatLocalDay, parseLocalDate } from "../notes/datetime"; -import { NOTE_COLOR_KEYS, NOTE_COLOR_LABELS, NOTE_SWATCH_CLASSES, type NoteColor } from "../notes/colors"; import Icon from "./Icon.vue"; // A dead-simple facet bar over the board: color + labels + has-reminder @@ -32,9 +31,6 @@ function patch(p: Partial) { function clearAll() { void router.replace({ path: "/", query: {} }); } -function setColor(c: NoteColor) { - patch({ color: facets.value.color === c ? undefined : c }); -} function toggleLabel(id: string) { const cur = facets.value.label ?? []; const next = cur.includes(id) ? cur.filter((x) => x !== id) : [...cur, id]; @@ -111,20 +107,6 @@ const chipOff = "border-neutral-300 text-neutral-600 hover:bg-neutral-100 dark:b v-if="open" class="mt-2 flex flex-col gap-3 rounded-xl border border-neutral-200 p-3 dark:border-neutral-800" > -
- Color -
-
Labels
@@ -140,7 +150,7 @@ async function doMerge(sourceId: string, targetId: string) { > {{ t.name }} diff --git a/frontend/src/components/MarkdownInline.vue b/frontend/src/components/MarkdownInline.vue index 5001909..e5206bd 100644 --- a/frontend/src/components/MarkdownInline.vue +++ b/frontend/src/components/MarkdownInline.vue @@ -1,10 +1,16 @@ + + + + + +
    -
    • +
    -
    2. + 
{{ b.value ?? "" }}
-

+

diff --git a/frontend/src/components/NoteCard.vue b/frontend/src/components/NoteCard.vue index b6ef1bf..f906d9f 100644 --- a/frontend/src/components/NoteCard.vue +++ b/frontend/src/components/NoteCard.vue @@ -1,19 +1,11 @@ - - -
-
diff --git a/frontend/src/components/NoteChecklist.vue b/frontend/src/components/NoteChecklist.vue deleted file mode 100644 index 9401751..0000000 --- a/frontend/src/components/NoteChecklist.vue +++ /dev/null @@ -1,75 +0,0 @@ - - - diff --git a/frontend/src/components/NoteEditor.vue b/frontend/src/components/NoteEditor.vue index e73cfc3..9dfd421 100644 --- a/frontend/src/components/NoteEditor.vue +++ b/frontend/src/components/NoteEditor.vue @@ -1,16 +1,23 @@