211 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
bvandeusen	abe01da5f7	compose: say which of the three deployment shapes you're in ... The operator asked why `THOUGHTSYNC_BIND` isn't just defaulted to the safe value. Fair question, and the answer exposed that my own advice was incomplete: I told them to set it to 127.0.0.1 without asking where their proxy runs, and for a proxy inside Docker that is the wrong fix. There are three shapes, not two: 1. **LAN, no proxy** — the default. Binds every interface so a phone and a desktop can reach the server. This is why the default is NOT the locked-down value: a server reachable only from the machine it runs on isn't hardened, it's broken, and that is the primary documented use of this app. 2. **Proxy in Docker** — delete the `ports:` block entirely. The proxy reaches the app over the compose network; publishing a host port is a second, unauthenticated way in that bypasses whatever the proxy does about TLS. Safer than 127.0.0.1, because there is no host port to reach even from the host. 3. **Proxy on the host** — `THOUGHTSYNC_BIND=127.0.0.1`. The compose file now spells out all three where the decision is made, and `docs/public-hosting.md` item 4 asks where your proxy runs before telling you what to do, plus how to check: `curl http://<lan-ip>:5000/api/health` from another machine should NOT answer once you're proxied. No default changed. Changing it would silently break every LAN install on the next `docker compose pull` — the phone would just stop syncing, with nothing saying why.	2026-08-23 15:30:01 -04:00
bvandeusen	09b5f874b6	Security values move into the Settings UI ... Operator: *"proxy hops defaults to 1 and should be in the settings UI not in the envs, we need the security values to be in the UI."* Overrules the call I made yesterday, and rule 25 is on your side — I argued deployment-topology, but the operator has to be able to SEE what protects them, and reading a container's environment is not seeing. Six new settings in a **Security** group: trusted proxy hops (default 1), the per-account and per-address sign-in limits with their shared window, and the sign-up limit with its own. `THOUGHTSYNC_TRUSTED_PROXY_HOPS` is gone; the rate limits are no longer hardcoded constants. **The hard part was keeping the throttle cheap.** It consults these BEFORE opening a database connection — deliberately, because a refused attempt is meant to cost nothing, and the hop count is needed to know who is even asking. A query per attempt would undo both. So there is a small cache seeded from the registry defaults (the app works with no database at all, which is what the DB-free unit lane relies on), loaded at boot, and refreshed on every settings save — the same live-update contract `session_ttl_days` already had. `SlidingWindow` now takes its limit and window as SUPPLIERS rather than values, so a saved number applies to the next attempt instead of the next deploy. **Bounds are rejected, not clamped.** A hop count of 99 would trust anything a caller sent; a sign-in limit of 0 would lock every account out permanently. Both now fail validation with a message naming the range, and the number input carries min/max so the browser objects first. Silently storing a different number than the one typed is how somebody ends up believing a protection is set to something it is not. `MAX_BUCKETS` stays a constant on purpose: it protects the limiter from itself rather than the app from a caller, and there is no operator judgment to apply. Two integration tests, because the whole point is the round trip: a dangerous value refused, a legitimate one reaching the cache the throttle reads and persisting; and every Security row reaching the admin payload with bounds and a description that explains itself.	2026-08-23 15:24:15 -04:00
bvandeusen	a85c53ba2c	Trust proxy headers by hop count, and log every credential event ... Operator, before exposing the instance: *"I'd expect that we should have a proxy hops setting for how many proxy hops we should trust a shared real-ip at… and is there any session logging."* Neither existed, and the first one was a real hole. **The address was forgeable.** `client_address()` read the LEFTMOST `X-Forwarded-For` entry — nominally "the original client", and precisely the one a caller controls, because anything they send arrives before what proxies append. So `curl -H "X-Forwarded-For: 1.2.3.4"`, rotated per request, minted a fresh rate-limit bucket every time. Concretely: stuffing ONE account stayed limited (the account key is unforgeable and that is why it exists), but spraying MANY accounts from one source was not — each account got its own budget, and the per-address cap meant to bound the total was defeated by a header. On a LAN that is nothing. It is not nothing on a public host. Now it counts in from the RIGHT by `THOUGHTSYNC_TRUSTED_PROXY_HOPS`, default 1. Each hop appends what it saw, so the rightmost entries are the ones our own infrastructure wrote and a forged prefix lands to the left of them where it can never be selected — proven for the honest, forged, padded, CDN and shorter-than-configured cases. 0 ignores the header entirely; 2 is Cloudflare in front of a proxy. Too high is the dangerous direction, so a header shorter than configured falls back to the socket address rather than reaching further left. `X-Forwarded-Proto` had the same bug and now shares the same rule. Both live in a new `proxy.py` rather than being written twice — two places holding one decision is how issue 2183 happened, and this is the same decision. Env rather than the Settings UI, against rule 25's usual pull: it is deployment topology rather than preference, and the limiter consults it BEFORE opening a database connection, which is the entire point of checking a throttle before doing expensive work. Easy to move if that reads wrong. **And there was no logging at all** — `auth.py` had no logger, and the only record of anything was `device_tokens.last_used_at`. Sign-ins, failures, throttle trips, new accounts and device-token issuance now all log, with the attempted email and the trusted address. Deliberately including the email: it is the operator's own server, and "somebody failed a login" without saying against which account is not actionable. `basicConfig` at INFO in `create_app`, because hypercorn configures its own loggers and leaves the root at WARNING — without it every line above would have gone nowhere, which is a worse failure than not writing them. This is the app log, not an audit table. Not queryable, not retained past log rotation. The table is task 2939; this is what makes the next few days observable.	2026-08-23 15:12:14 -04:00
bvandeusen	2141a0ac45	Registration closes itself once the instance has an owner ... Operator: *"registration should be open only for the first user and they get granted admin privileges. then registration is closed."* The old shape had a window in it. The first account was always allowed and became admin; every account after that was gated by `allow_registration` — which defaulted to ON. So the door stayed open between "my account exists" and "I remembered to turn it off in Settings", and on a public host that gap is the entire exposure: it starts the moment DNS resolves and lasts until someone remembers. Now the door shuts as a CONSEQUENCE of the admin account existing, in the same transaction that creates it. Not "defaults closed" — that would still need the first person to get in somehow. There is no window to remember, because there is no window. Re-opening it is a deliberate act in Settings → Access: turn it on, have the person register, turn it off. Crude, and it is the only mechanism there is — **there is no invite system**, not even a stub. That is real work (a token table, admin create/revoke, a redemption flow, expiry) and is filed as later work rather than smuggled into a release. An integration test covers it, because it is the interaction between two writes in one transaction: first register → 201 and `is_admin: true`; the setting is then false; a second register → 403; re-open deliberately and a third → 201, not admin. **This does not retroactively close an instance that already has users.** The close fires on first-account creation, so a server whose admin predates this keeps whatever the setting was — which was on. `docs/public-hosting.md` now says so explicitly, and step 1 of the checklist is "check" rather than "do" for exactly that reason.	2026-08-23 14:18:58 -04:00
bvandeusen	1aca294b95	Bump to 0.2.0 — a release at 0.1.0 would have been a downgrade ... Found while preparing the release, and it would have quietly defeated the point of cutting one. The version does NOT come from the tag. `desktop/packaging/build-version.sh` reads `desktop/src-tauri/Cargo.toml`, and on `dev` it appends the CI run number (`0.1.269`) while on a tag or `main` it ships the file's value verbatim — which was still `0.1.0`. So tagging today would have published a "release" numbered BELOW every dev build already out there, and below the 0.1.227 on the operator's phone. The desktop updater compares semver: an installed build would have read the stable manifest, seen a version older than its own, and correctly concluded it was already current. The APK would have installed (versionCode is the run number and keeps rising) while displaying a version that reads as going backwards. build-version.sh predicted this in its own comment: "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." Bumped in four places, which is every one that can be read by something: - `desktop/src-tauri/Cargo.toml` — the actual source; everything else derives - `tauri.conf.json` — overridden at build time by `--config`, but a checked-in value that lies is exactly how issue 2183 happened - `pyproject.toml` + `__init__.py` — the server's APP_VERSION fallback when no BUILD_VERSION is injected `core` and `android/ffi` stay at 0.1.0 deliberately: internal library crates whose version reaches no surface, and versioning workspace libs independently of the app is normal. Cargo.lock regenerated with `cargo fetch` per ci-requirements — one line, the version itself. Verified: a tag build now yields 0.2.0 and a dev build 0.2.270, so stable is an upgrade for every existing install and dev stays ahead of stable.	2026-08-23 14:02:02 -04:00
bvandeusen	7033995975	search is a facet on the board, not a place you go ... Operator (note 2930): tags exist so you can *"filter during a search"*. The server has always been able to do that — `GET /api/notes` composes `?q=` with `?label=` and the rest into one AND-ed query. The frontend never reached it. The header search box navigated to `/search`, and that view called a DIFFERENT endpoint — `GET /api/notes/search?q=`, full text only, no facets at all. So the one screen you landed on when you searched was the one screen where you could not narrow by tag. Tag filtering lived on the board's FilterBar, which is where you weren't searching. Two search boxes, two endpoints, and only the hidden one did what tags are for. Now the header box writes `?q=` into the board's URL beside whatever labels are already there, and stays on the lens you're in — searching while looking at Trash searches Trash. The box READS from the URL rather than holding its own copy, so it stays in step with the Filters panel's Clear and with a saved view opened from the sidebar. Deleted: `SearchView.vue`, its route, `GET /api/notes/search`, `repo.notes.search` and both adapter implementations, and the `notes_search` Tauri command whose only caller was the adapter entry. FilterBar loses its own "Search text…" input — it was the same facet, hidden behind a collapsed panel, duplicating a box that is always on screen. Filters now does what its name says: narrowing. The header does searching. `core::store::search` STAYS. Android calls it through the FFI (`search_notes`) and has its own search surface — which has the same no-tag-filter gap the web just lost, and deserves the same fix on its own terms rather than as a rider here.	2026-08-23 10:58:23 -04:00
bvandeusen	de72d27bd4	URLs unfurl on their own, and a lone link becomes the note ... Operator: *"I'd like for URLs to unfurl. To be the whole note when the note is a single URL, and to be a compact slot on the bottom of the note when the URL is inline. We also need to support multiple URLs in a single note."* Less new machinery than it sounds: `unfurl.py` already fetched and parsed OG tags, SSRF-hardened, and `note_link_previews` was already `UNIQUE(note_id, url)` — so several URLs per note has worked at the storage layer all along. What was missing was that it needed a button, had one size, and drew that size in the wrong place. **Automatic, and never in the way.** New `unfurl_queue.py` detects a body's URLs and fetches them on a background task AFTER the note is committed. Capture speed is the product: an unfurl is a five-second timeout against a host nobody controls, and a note has to persist the instant someone stops typing. Scheduled from create, from a body edit, and from a synced push — so a linked desktop or Android client gets previews too, on its next pull. An unlinked one has no server to ask and simply has none, which is the honest consequence of being offline. Safe to call on every save: it re-reads what's cached and does nothing when nothing is new. Capped at five URLs per note, silent on every failure (a link that won't fetch isn't an error the person needs — the note is fine, the link is still there), and it re-checks before storing, so a slow fetch can't resurrect a preview for a URL that was deleted while it was in flight. **Two presentations.** A note whose body is nothing but a URL renders as its preview and nothing else — printing the raw URL under a card that already says where it goes is saying the same thing twice, badly. Until the fetch lands, or if it never does, the URL stands in, so the card is never blank. Anything else gets a compact strip. **And the strip moved.** Previews were rendered ABOVE the body, which put a stranger's headline where the note's own first line should be — worse now that the first line IS the note's name. They sit at the foot of the card now, under the note's own words. The editor's "Preview example.com" button is gone with the manual path; removing an unwanted preview stays, and stays editor-only. Nine tests: three on detection (order, dedupe, sentence-punctuation trimming, non-http rejection) in the unit lane, and three in the integration lane for what only a real database shows — the upsert landing on the right row, a second pass fetching nothing, and a preview NOT being stored for a URL that left the body.	2026-08-23 01:05:37 -04:00
bvandeusen	c99cbb3e14	cards: clamp the web note preview, as Android always has ... M13 step 4 asked for no bold first line, and step 3 already delivered that — removing `note.title` took the card's <h3> and the Android editor's bold field with it. What step 4 also asked for, and hadn't been done, was the other half: "be willing to spend something small on legibility that isn't weight on the first line." The web card rendered the entire body. Android has always clamped to eight lines (`MAX_PREVIEW_LINES`), so one long note produced a card taller than the screen on the web and pushed the rest of the board off it — a real asymmetry between two surfaces that are supposed to be peers. It matters more without a title. The first line used to be what your eye caught; with one weight throughout, an unbounded card is just a wall, and the note beside it is the one you were actually looking for. Clamped in the STRING, not with CSS `line-clamp` — that needs a `-webkit-box` and behaves unreliably around the block elements MarkdownText emits (lists, quotes, fenced code). Doing it before the parse is deterministic, matches Android's semantics exactly, and skips parsing a body the card was never going to show.	2026-08-23 01:00:39 -04:00
bvandeusen	6f21db85a1	ci: an integration lane, so the migrations are finally run by something ... 26 Alembic revisions and none had ever been executed by CI. `alembic upgrade head` ran for the first time when the operator's container started, and the schema the migrations build had never been checked against the models that read it. M13 dropped three columns and rebuilt a STORED GENERATED column with nothing watching but a server boot. Copied from FabledScribe's `integration` job, which had already solved the parts that are easy to get wrong — and which are family rules precisely because they were: a separator-free job key with no `name:` (act_runner derives the service container name from the truncated display name, and the discovery step filters `docker ps` by it), bridge-IP resolution because service hostnames aren't routable on this runner, and a Python readiness wait because `run:` is busybox sh with no `/dev/tcp`. `postgres:16-alpine` to match the production compose. The schema is built by real migrations, never metadata.create_all — that step IS the migration test. Six tests, each pinning something that has only ever been checked by hand: - an ORM insert against the migrated schema, which is the model/migration agreement nothing has verified until now; - `notes.title`, `notes.kind` and `note_revisions.title` are actually gone, and `note_links` with them — a silently no-op migration shows up here; - the rebuilt `search_vector` indexes both the name and the body, which matters because 0026 had to DROP and recreate a generated column rather than alter it; - a note keeps its body AND its items, the shape step 2 made normal; - `_apply_note_items` leaves items alone when a change doesn't mention them — the data-loss path step 2 removed, pinned so its return would be caught; - a note with no body is still named by its first item, the hole that made removing the title unsafe until checklists stopped being their own kind. Runs for visibility; does not gate the build, matching `test` and Scribe. No local equivalent: running it means standing up Postgres on the workstation, which rule 12 reserves for an explicit request. Documented in ci-requirements alongside the Rust, Kotlin and frontend gates.	2026-08-23 00:24:57 -04:00
bvandeusen	924ddb20db	notes: saveEdit still asked for a title ... The one thing step 3 missed, and the typecheck lane caught it: `saveEdit`'s parameter type still declared `title`, so the editor's call — correctly no longer passing one — didn't match. I gated Rust locally and not the frontend. Both are now in ci-requirements, including WHY the frontend one has to be `npm run build` rather than `vue-tsc --noEmit`: the typecheck only reads the script block, so a malformed template sails past it and fails `vite build` in a different workflow, which is exactly how the stray `</div>` got two commits away from where it was written.	2026-08-22 21:38:13 -04:00
bvandeusen	95aa10c2c3	Remove the title field — a note is named by its first line ... Operator (note 2897): "notes shouldn't have a title field." The concept of a NAME stays — search results, export filenames and the command palette all need one — but nothing is typed into it any more. `display_title` is now the first non-empty line of the body, falling back to the first checklist item. That fallback is what step 2 bought, and the reason this could not go first: a checklist had no body to be named from, so the title was its only name. Now every note has a body, and a note that is only a checklist is named by its first item. Gone everywhere: the column and note_revisions.title (0026), the field on the core's Note/NoteCreateInput/NoteRevision and its SQLite columns (user_version 7), `normalize_title`, the wire field, the FFI record and `NoteEdit::Title` / `ClearTitle`, the web editor's "Title (optional)" input and the card's <h3>, and the Android title field in both the compose sheet and the editor. **The search vector had to be rebuilt, not just left alone.** `notes.search_vector` is a STORED GENERATED column whose expression names `title` — Postgres refuses to drop a column another generated column depends on. It is dropped and recreated over `display_title` at weight A, which keeps the original intent: a note's NAME ranks above the rest of its body. **An imported title becomes the note's first body line.** Keep notes carry one, and so does any ThoughtSync export taken before this. Dropping it would silently lose text someone wrote; folding it in puts it exactly where a name now lives, so the note arrives named as it was. Skipped when the body already opens with that line, so re-importing an export this code produced doesn't stack duplicates. Two smaller things fell out. The Android editor loses its bold first field — one weight throughout, because the first line is the note's name but not a different KIND of text, which is most of step 4 arriving early. And `ClearTitle`'s justification comment moved to `ClearRemindAt`, which is now the surviving example of why NoteEdit is a list rather than a struct of options. Protocol note corrected to say what actually shipped: v2 is "no kind, no title", one bump for the pair. Verified with the local Rust gate this time, not by CI: fmt, clippy and 116 tests all green before pushing. It caught four things — orphaned serde attributes where fields were removed, a `wire::Preview.title` I deleted by mistake (a link preview still has one), nine retention fixtures inserting a dropped column, and four rustfmt diffs.	2026-08-22 19:33:57 -04:00
bvandeusen	6d778f26a7	Fix the ktlint and compat-test failures, and start using the Rust gate ... Two more from the step-2 removals: **Two unused Kotlin imports** — `FilterChip` (the Note/List switch) and `Icons.Filled.Create` (the "switch to a note" icon), both orphaned when their callers went. ktlint treats them as errors. **`server_info_tolerates_unknown_and_absent_fields`** pinned `sync_protocol_version: 1` as a literal, so bumping the protocol to v2 made it fail for a reason that has nothing to do with what it tests. It is about unknown FIELDS; the versions now come from `CLIENT_PROTOCOL_VERSION`, like every other test in that file already did. The bigger fix is the habit. `ci-requirements.md` has documented since 2026-08-18 that the operator authorised running fmt/clippy/test against the CI image locally, and I had not been doing it. All three now pass here — 116 tests, clippy clean, fmt clean — and every Rust failure in this milestone so far would have been caught by them in under a minute instead of by CI, several commits downstream. Noted in ci-requirements so the next session doesn't relearn it: a removal is exactly the change that looks too safe to check.	2026-08-22 14:51:38 -04:00
bvandeusen	33e9278975	Fix three breaks the removals left behind ... **`snapshot_revision` was deleted with `create_titled`** (bc22f8e). I sliced the function out by scanning to the next `pub fn`, and the private `fn` sitting between them went too. Nothing in Python or TypeScript compiles Rust, so it sat undetected until the first lane that does. Restored verbatim. **An orphaned serde attribute** in push.rs: removing `pub kind: Option<String>` left its `#[serde(skip_serializing_if)]` behind, which then stacked onto the next field's. That failed the derive, which is why three follow-on errors all said `Change: Serialize is not satisfied` — one cause, four messages. **An unbalanced `</div>`** in NoteEditor.vue, orphaned when the "Links / Linked from" footer was cut. `vue-tsc --noEmit` type-checks the SCRIPT block and never parses the template, so the typecheck lane passed it and `vite build` caught it two workflows later. Worth remembering: a green typecheck says nothing about template structure. I also pushed step 2 without waiting for ad21eac to go terminal, which is what let the Rust break travel a commit further than it should have. Each fix comes with the check that would have caught it: a scan for stacked serde attributes and called-but-undefined fns across every .rs, and a tag balance pass over every .vue. Both are clean.	2026-08-22 13:25:26 -04:00
bvandeusen	c46a4a7709	A checklist is something a note has, not something a note is ... `kind` was never a type. A plain TEXT column with no enum and no CHECK behind it, compared against a hardcoded ("text", "list") tuple in six places; `note_items` was always an ordinary child table keyed by note_id; serialization already emitted `items` whatever the kind; and the Android editor already toggled between the two losslessly, saying so in a comment. The storage has modelled "a body plus optional checkable items" the whole time. This deletes the gates that forbade it. Every surface: the create/PATCH gates, the ?kind= filter and its saved-filter facet, the three import/export branches, the column (alembic 0025); the core's `kind` field, its SQLite column (user_version 6), the sync wire, push and pull; the FFI records and `NoteEdit::Kind`; and on Android `NoteKind.kt`, `DraftKind`, the compose sheet's Note/List switch, and the branches in the card, the editor and the chrome. The editor's note⇄list toggle becomes "Add a checklist" — on both the web and Android. It is not a conversion any more: nothing moves, nothing is swapped, the body stays exactly where it is and the note gains somewhere to put items. The card renders both, in order. Two things that fell out of the merge rather than being aimed at: - The Keep importer was DISCARDING `textContent` whenever a note also had `listContent`, because the target could only hold one. Both survive now, and the test says so. - Markdown export wrote the body OR the checklist. It writes both. Protocol goes to v2, floor included: dropping a field a v1 client sends and expects back is breaking. `title` leaves in step 3 and lands in the same generation, so it needs no further bump. This is the change that will make the 0.1.227 build on the operator's phone refuse to sync — the in-app updater is independent of the handshake and remains the recovery path. The V1 SQLite schema deliberately KEEPS the kind column. V1 is the historical schema and every later block alters it, so removing it there would make a fresh database run V1 without the column and then v6's DROP COLUMN against a column that never existed — "no such column: kind" on every new install.	2026-08-22 12:53:53 -04:00
bvandeusen	229076c82d	sync: stop deleting a note's checklist items because it isn't a "list" ... `_apply_note_items` didn't ignore items on a non-list note — it deleted them. That was survivable only because nothing in the product could produce a note holding both a body and items. M13 makes exactly that the normal shape: a checklist is something a note HAS, not something a note IS. Against that shape this guard is a data-loss path — the first sync after adding a checklist to a note would wipe it. Landing it before the UI that can create the state, so there is never a window where the two disagree. `kind` itself, and the rest of the merge, follow.	2026-08-22 12:45:14 -04:00
bvandeusen	ad21eac5bc	editor: drop the adapter import that went with backlinks ... `repo` reached the editor for exactly two calls — `repo.notes.backlinks` and `repo.notes.linkSearch` — and both left with the linking system. vue-tsc runs with noUnusedLocals, so one stale import failed the whole shared-frontend build and took both desktop lanes down with it (TS6133). My local sweep checked for dangling *references*; it never checked the inverse, that every import still has one. It does now, across all fifteen files that removal touched — `repo` was the only one.	2026-08-22 12:44:33 -04:00
bvandeusen	bc22f8e249	Remove [[wiki-links]], backlinks and the graph ... Operator, 2026-08-22 (note 2897): ThoughtSync is an intermediary surface. You write here because it's easy — a notebook in your pocket — and later you recall the thing and go finish it somewhere else. Recall is the product; organization is secondary. A linking system is organization, and it isn't what this is for. So: `[[wiki-links]]`, backlinks, the `[[` autocomplete, the note_links table, `/api/notes/link-search`, `/api/notes/<id>/backlinks`, the whole graph blueprint and GraphView. Rust core loses `extract_links`, `backlinks`, `link_search` and `create_titled`; the desktop loses the three Tauri commands that exposed them. This subsumes 982d24c rather than reverting it. That commit bound links to a note id so a rename would stop rewriting other notes' bodies — real infra, but infra for a feature that is now gone, and nothing it added survives. Alembic 0023 stays in the chain anyway: it shipped in an image and may already be applied, and deleting an applied revision strands a database's version pointer. 0024 drops the table and takes the column with it. The history stays honest about the fact that it existed for a day. Two things deliberately kept, because they were serving recall and only incidentally serving links: - `/api/notes/titles` and the titles store. The command palette lists them so you can jump to a note by name. `resolve()` — the name→note lookup that only linking needed — is gone. - `display_title`. Every note still has a name for search results and export filenames. What that name is FOR changed; that it exists did not. `notes/links.py` is now `notes/tags.py`, holding the #tag→label reconciliation it always also owned. A file called links.py with no links in it would have been exactly the drift this removal is meant to end. Also swept out on the way: `_escape_like`, whose only caller was link-search, and the `graph` icon. Nothing lost that a person typed — note_links was always derived, and the `[[text]]` is still sitting in every body it was written in.	2026-08-22 12:00:57 -04:00
bvandeusen	982d24c83b	links: bind a [[link]] to a note, not to a string ... A wiki-link was stored only as normalized TEXT, so a note's NAME was the edge. Renaming it broke every inbound link — and the fix that shipped for that (task 1848, option b) was `_rename_inbound_links`: rewrite the `[[Old Name]]` text inside the body of every note that linked to the renamed one. That works while an explicit title exists to hold still. It stops being defensible the moment a note's name is just its first body line, which is where M13 is going: fixing a typo in your opening sentence would silently edit other notes' words, with nothing to opt out to. So this lands first, before the title comes out, and that window never ships. `note_links` gains `target_id`, bound when the link is written. `target_norm` stays and is what an UNRESOLVED link carries — linking to a note that doesn't exist yet is a supported way to create one, so a link has to be able to name a target that isn't there. Resolution reads the id, falling back to the name only where nothing was bound, which is what lets a forward link connect the moment its target appears. `_claim_unresolved_links` then binds it, so the fallback is a transitional state rather than a permanent one. `_rename_inbound_links` and `rewrite_link_title` are gone. What replaced them touches link rows only: a note's text is never modified by something happening to a different note. The client can no longer resolve links for itself, and that is the point. It used to look `[[text]]` up in a client-side name index, which only held together BECAUSE renaming rewrote the text everywhere. Now the written text can name something the target is no longer called, and only the server holds the binding — so each note serializes its resolved links (`norm`, `id`, and the target's name as it stands NOW). A renamed note reads correctly everywhere it is linked from, without a single body having been edited. Unresolved links are simply absent and fall through to the create-on-click affordance that already existed; so does the offline desktop store, which derives links at query time and has no binding to send. The name-fallback join is owner-scoped everywhere it appears. Bound ids were resolved owner-scoped when written, but matching on display_title alone would have let two users who each have a note called "Groceries" see the other's id and name through an unresolved link (rule 47). The new behaviour is all SQL and this suite runs without a database, so the dead helpers' tests are removed rather than replaced. This repo has no integration lane to hold that ground — noted, not papered over.	2026-08-22 11:02:39 -04:00
bvandeusen	bacedea8a3	tests: seed the throttle counters on the clock the routes actually read ... The three route tests stamped their pre-loaded hits at t=0..9 through the injected clock, then called a route that reads `time.monotonic()`. Against a trailing window those hits are fifteen minutes stale on arrival, so they were pruned before they could refuse anything, the request carried on to the database that this suite doesn't have, and the assertion read `500 == 429`. The window's own tests keep the injected clock — they pass the same one to both sides, which is what makes them deterministic and instant. Only the tests that hand off to a route need the real one.	2026-08-21 22:03:19 -04:00
bvandeusen	b6152ec18b	server: harden the surfaces a public deployment leaves exposed ... On a LAN the login form is reachable by people you already trust. Exposed, it is reachable by everyone, and nothing in front of it was counting. Three credential routes — /login, /register and /device-login — now throttle. Every attempt is counted against BOTH the account and the calling address, and either can refuse it. The account key is the one that matters and the one that cannot be forged: it stops stuffing against a known email no matter how many addresses the attempts arrive from. The address key bounds one source spraying many accounts, and is best-effort by nature — behind a proxy it comes from X-Forwarded-For, which a caller can set to anything if the app is exposed directly. That is exactly why it isn't the only key. The check runs BEFORE the password is verified, which is the other half of what this protects. bcrypt is deliberately slow; an unauthenticated caller who can trigger it without limit has a CPU exhaustion primitive as well as a guessing one. Sliding rather than fixed windows, because a fixed one lets twice the limit through across a boundary. Bucket count is capped so a rotating forged header can't turn the limiter into the exhaustion it prevents. A sign-in against an email with no account now spends a real bcrypt against a throwaway hash first. Without it "no such account" returned in microseconds while a wrong password took ~100ms, which is a reliable oracle for which emails are registered here. Every response carries a CSP with script-src 'self', object-src 'none' and frame-ancestors 'none', plus nosniff, a referrer policy and a permissions policy. The app has no inline and no third-party scripts, so this concedes nothing; the exceptions are honest — inline STYLE (Vue writes it itself for v-show and the FLIP), and remote images (a link preview renders the og:image of an arbitrary host, over either scheme, since a LAN install is served over http). HSTS only where the request already arrived over TLS, and scoped to the one host: no includeSubDomains, no preload, neither of which is this app's to commit. X-Forwarded-Proto detection moved into one `_is_https()` — the session cookie's Secure flag and HSTS are the same question, and answering it twice is how the two drift apart. docs/public-hosting.md is the rest of it: the four things only the operator can do (close registration, terminate TLS and forward the scheme, stop publishing the app port, back up the attachment volume as well as the database), and an honest list of what the app does NOT have — no email verification, no password reset, no second factor, no per-user quota, no audit log. Those aren't blockers for an instance whose accounts are people you know. They're the reason not to leave signups open to strangers.	2026-08-21 22:01:37 -04:00
bvandeusen	16f86bef93	web: make the board usable on a phone, not just reachable ... The controls a card carries were always-visible overlays on a touch device — correct as far as it went (task 2697: a finger cannot hover, and the pill is the only way to pin or archive), but they were still absolutely positioned, so they sat ON the note's own title. A card reading "thought sync tauri app" rendered as "ught sync tauri app" with the grip parked over the first three characters, and the four-icon pill covering the right half of the first line. Placement is now CSS's decision. One element each, two placements: where a pointer can hover they lift out of flow into the floating top-corner pills they have always been; where nothing can hover they stay in flow as a footer row, which cannot overlap anything by construction. Keyed on hover rather than width, for the same reason `.hover-reveal` already is — a narrow window on a laptop still hovers, a wide tablet still doesn't. The colour popover moved inside the action set so it follows it, and opens into the card from either end. The header was sharing one phone-width row between a menu button, the logo, the lens name, a search field and four icons; everything in it was truncated, the lens down to "N…" and the search box to an empty pill. It wraps now, so search takes its own line below sm, and account / settings / sign-out move into the drawer where there is room to name them rather than guess at a glyph. One input, moved by CSS — duplicating it would have meant two `searchInput` refs and a `/` shortcut that focuses the wrong one. Also closes the other half of task 2706, which was waiting on a device to look at: `viewport-fit=cover` together with the `env(safe-area-inset-*)` padding that makes it safe (sides on body, top on the sticky header, bottom on the board and the drawer), and `100dvh` behind an @supports so the app box follows the visual viewport when the keyboard opens instead of the layout viewport. Both halves in one change, as that task insisted. And the composer no longer tells a phone to "Press Enter".	2026-08-21 21:55:46 -04:00
bvandeusen	81695fa0c8	android: update the app from the server it syncs with (2727, M12 step 7) ... Closes M12. The phone can now notice that its server has a newer build and install it, instead of the operator copying an APK to a device by hand. **A PackageInstaller session, not an install intent.** The obvious route — ACTION_VIEW on the APK — is exactly what on-device install heuristics are tuned against, and it is what produced the "bypassing Android security" warning on Minstrel (Scribe note 2437). It also never tells the OS that this app is the legitimate updater of its own package, and it returns nothing: a failed install is indistinguishable from someone dismissing the dialog. The session says who is doing what, and on Android 12+ declares no user action required — which, with UPDATE_PACKAGES_WITHOUT_USER_ACTION, removes the confirmation entirely on the UPDATE path. Only there: Android will not let an app quietly put a NEW package on a device, which is right. It also only applies when the new build carries the same signing key as the installed one, which is why signing had to land first. Two things from that research deliberately NOT done: `setRequestUpdateOwnership` was chased and turned out to be a red herring, and REQUEST_INSTALL_PACKAGES is not the differentiator either — Mihon declares it too. The mechanism was the whole difference. **The outcome comes back.** `commit` takes an IntentSender and the result lands at `UpdateReceiver`, so a failure can be shown rather than guessed at, and STATUS_PENDING_USER_ACTION is handled — that is the ordinary path below API 31 and still possible above it, since the OS is entitled to ask anyway. Someone declining is reported as no error at all: calling a deliberate choice a failure is how an app sounds broken when it is not. **The network work stays in Rust.** Two FFI additions — `clientUpdate` and `downloadClientUpdate` — because the device token lives in the core, and pulling it into Kotlin to make an HTTP call would spread the one secret this app holds across two languages for nothing. The core also owns the comparison, so the rule "version CODE decides, never the name" lives in the layer that has to get it right for every surface. The download is streamed to disk, not buffered: 55 MiB in memory on a phone is how an update gets killed halfway through. It lands in `update.apk.part` and is renamed only once size and sha256 both match, so an interrupted download can never be mistaken for a finished one. The digest is not a trust anchor — the signature is, and Android checks it — but it catches a truncated transfer before the installer is bothered with it. The advertised path is joined to the base URL this device is LINKED to rather than followed as given, so a server cannot point the download at a host nobody agreed to. **Updates are linked-only, and it says so.** An unlinked install has no update path, so it gets one sentence explaining where updates come from rather than a Check button that silently finds nothing — the same lesson as the desktop's unlink copy (issue 2110). And the "install unknown apps" grant is asked for BEFORE downloading, so nobody spends 55 MiB to be told no. Every Android API here was read out of `android-36/android.jar` with javap first, and the two new FFI methods out of freshly generated bindings, rather than recalled: `suspend fun clientUpdate(installedVersionCode: Long): ClientUpdate?` and `downloadClientUpdate(destPath: String)`. Also fixes `check-symbols.py`, which reported four false positives on `UpdateOutcome.Result` — its object-member index collected functions and properties but not nested TYPES, and a data class inside an object is an ordinary member.	2026-08-21 08:44:08 -04:00
bvandeusen	0cf77336d4	ci: build the server image after the Android lane, not alongside it ... Baking the newest client into every image left two holes, both raised by the operator. **An Android-only push never rebuilt the image.** `ci.yml` does not trigger on `android/**`, so a new APK could be published and no image would ever pick it up until some unrelated server change came along. **A push touching both raced.** Both workflows start at once; the image build would fetch the PREVIOUS client and there would be no second build to correct it — `:<sha>` is the immutable rollback unit (rule 46), so rebuilding it with different content would make it neither immutable nor a rollback unit. Ordering now runs the other way: the Android lane finishes, then calls the image build. `ci.yml` gains a `gate` job that stands down on any push touching the Android app, and `android.yml` dispatches `ci.yml` when it is done. One image per commit, containing the client from that commit. Cases: - **server only** — ci builds immediately; the newest published client is already the right one. - **Android only** — ci does not trigger at all; the Android lane dispatches it afterwards. - **both** — ci's push run stands down, the Android lane dispatches it. Exactly one image. - **tag** — always builds. The Android lane does not run on tags, so waiting for a call that never comes would mean a release tag with no image. The dispatch is `always()`, so a FAILED Android build still lets the server image through with the previous client. The alternative is a broken Android lane silently blocking server delivery, which is a worse failure than a slightly old APK. Two details that would each have made this quietly wrong: The gate diffs the whole PUSHED RANGE (`event.before..HEAD`, full fetch), not `HEAD^..HEAD`. A three-commit push whose Android change sat in the first would otherwise have looked Android-free and raced anyway — silently, which is the worst version of this bug. The dispatch is `curl -fsS`, not `|| true`. If that call ever stops working the symptom is server images silently never being built for Android pushes, which nobody would notice until wondering why the app stopped updating. The gate's path list has to match android.yml's trigger, and two places holding one decision is the recurring failure in this repo (issues 2181-2183). It is a `git diff` rather than a config precisely so the decision is visible in the log, and both sides carry a comment pointing at the other.	2026-08-20 21:46:05 -04:00
bvandeusen	010e9a2f85	server: bake the newest Android client into every image (operator call) ... Reverses the placement decision made an hour ago. That one put the APK only on the data volume, reasoning that ~55 MiB should not be charged to installs that never touch Android. The operator's call is that ending the manual copy is worth the megabytes, and it is their deployment. CI now fetches the newest published client into the build context immediately before the image build, so `:dev`, `:latest` and `:<version>` all ship one and a `docker compose pull` delivers a new server and a new client together. **Always the rolling `dev` release — the newest build there is.** A versioned image therefore carries the newest client rather than one pinned to that version. Deliberate: the two negotiate a sync protocol version before linking, so a mismatch is caught by the handshake, and pinning would buy nothing the handshake does not already provide. **Fetched by the JOB, never by the Dockerfile.** The release is private, and a token used inside a build ends up in the context or a layer. **It cannot fail the image build.** No release yet, a network blip, a first-ever build — all of them log a warning and produce an image with no client, which is a state the server already supports. Half a pair is cleaned up rather than shipped: a sidecar without its APK is worse than neither, because the server would be describing something it cannot serve. **The volume still wins.** `DATA_DIR/client/` is checked first and the baked copy second, so an operator who deliberately drops a build in gets that build — and a BROKEN drop-in falls through to the image's copy rather than taking the feature offline, which is what makes the copy-order advice survivable instead of load-bearing. Three tests cover the precedence, including that last case. The baked copy lives inside the package, not under DATA_DIR: that path is a volume mount, and anything the image wrote there would disappear behind it the moment one is attached. `client/.keep` is tracked so `COPY client/` cannot fail on a tree where the CI step never ran; the artifacts themselves are gitignored, since a 55 MiB binary does not belong in git history and is re-fetched on every build anyway.	2026-08-20 21:19:47 -04:00
bvandeusen	43ebb6eceb	packaging: the rolling-release prune was eating the Android client ... Run 4092 published `thoughtsync.apk` to the `dev` release. Run 4098 removed it, four minutes later, and both runs were green. `write-manifest.sh` prunes the rolling channel to stop ~100 MB AppImages accumulating forever, keeping `latest.json` and anything whose name contains the current `$APP_VERSION`. The Android assets deliberately have no version in their names — a fixed name is the only addressable URL on a tag that never moves, which is the entire reason the `dev` release exists — so they matched neither rule and were swept. They would have been swept even if they HAD carried a version: Android is a different workflow with its own run number, so its version never equals the desktop's `$APP_VERSION` in this script. The keep-list is now about fixed names rather than about `latest.json` specifically, which is what the rule always meant. A fixed-name asset is self-limiting — each publish replaces the same name — so the accumulation this prune exists to prevent cannot happen to one. Worth noting how this presented: two green runs and a missing file. Nothing failed, and the only way to see it was to ask the release what it actually held rather than trusting that a step named "Publish" had published.	2026-08-20 20:31:03 -04:00
bvandeusen	e6da720e6b	packaging: drop assets that aren't there, instead of trusting nullglob ... `d77a798` added the Android client to publish-release.sh's asset list and broke the desktop lane's publish, which had been working (run 4094, curl exit 26 — "couldn't read local file"). The Android lane published fine, which is what made the shape of the mistake clear. `shopt -s nullglob` drops PATTERNS that match nothing. The two entries I added — `android/dist/thoughtsync.apk` and its sidecar — contain no wildcard, so they are not patterns at all: globbing leaves them in the array verbatim and curl is handed a path to a file that does not exist. In the Android job those files are there, so it worked; in the desktop job they never are, so it did not. Every entry is now filtered on existence, which is what the array has always meant. That covers the literal paths and the globs alike, rather than relying on each future entry containing a `*` to be safe — the trap that just cost a run. Verified both ways before pushing: a literal missing path survives nullglob and is removed by the filter, and an all-empty result still exits cleanly under `set -u`.	2026-08-20 20:20:58 -04:00
bvandeusen	d77a79859c	server: hand out the Android client this server syncs with (2726) ... A self-hoster should not need an account on someone else's forge to get the app for their own notes. The Fabled-Git instance is private — which is why `install.sh` already cannot fetch for anyone but the operator — so a release page is no use as a distribution point. The server holding the notes is something the person already trusts and already reaches. It also keeps the pair in step by construction. Client and server negotiate a sync protocol version before linking, so a server that also serves the client cannot hand out a phone it is unable to talk to. **Two files, and both must be present**: `thoughtsync.apk` and a `thoughtsync-android.json` sidecar carrying `{version_name, version_code, size, sha256}`. The sidecar exists because an APK keeps its version in a binary AXML manifest, which Python cannot read and which is not worth putting `aapt` on a Quart server to reach. CI writes it beside the APK, where the values are already known — including the digest, computed over the same bytes it uploads, so a phone can tell a truncated download from a complete one before handing it to the installer. Not a trust anchor; the signature is that. **Under DATA_DIR, not baked into the image.** Baking charges ~55 MiB to every self-hoster including everyone who never touches Android. `/var/thoughtsync` is already the mounted volume that holds attachments, so a build dropped there survives container recreation. **Absence is an ordinary state, not an error.** No APK means the key is absent from `/api/config` — absent rather than null, so a client testing for it cannot confuse "this server has no client" with "this server predates the field" — the web UI hides the card instead of offering a button that 404s, and the metadata route answers 404. A server whose owner does not use Android is not misconfigured. **A mismatched pair also counts as no client.** If the sidecar's recorded size does not match the file on disk, the two did not arrive together; serving one build while advertising another is worse than serving none, because the phone would compare versions against a promise the bytes do not keep. That makes the copy order in docs/android-distribution.md load-bearing, and it is written down there: APK first, sidecar last. **The version is public, the bytes are not.** An updater has to be able to ask "is there something newer?" cheaply and before it has done anything; 55 MiB is not for anyone who can reach the port. `login_required` already accepts either a session cookie or a device bearer token, so the browser and a linked phone both work with no second auth path. The Android lane now publishes both files to the same rolling `dev` release the desktop bundles use, reusing `publish-release.sh` — its nullglob asset list was already built for several jobs in separate workspaces publishing to one release, which is exactly this. Signed builds only: publishing an unsigned APK would offer people something they cannot install over what they already have. Nine tests, DB-free like the rest of the suite — this lane runs no Postgres, so the advertisement is asserted through `advertisement()` rather than through `/api/config`, whose other half needs a database. Both routes ARE exercised, because neither opens a session.	2026-08-20 20:11:32 -04:00
bvandeusen	6589be2b0f	android: unit tests are a debug-only task, and the artifact name says variant ... Run 4082: `Task 'testReleaseUnitTest' not found`. AGP creates unit-test tasks only for `testBuildType`, which is debug — so pairing the test task with the packaged variant was wrong from the start. It was only paired to stop two Gradle invocations asking for different Cargo profiles and paying the four-minute cross-compile twice. With the profile pinned to debug (#2810) that reason is gone, so the step goes back to `testDebugUnitTest` unconditionally. Costs one extra Kotlin compile and buys the type-check on the variant an emulator build would actually use. Also: the artifact was named from the Cargo profile, which is now always "debug" — so a signed release APK would have been uploaded as `thoughtsync-android-debug-<sha>`. Same word, two different things. It is named from the APK's variant now, and the two outputs are kept separate so they cannot be confused again.	2026-08-20 19:23:53 -04:00
bvandeusen	cae9888eb9	android: build the release APK with a debug-profile .so, for now ... `d0a9c73` switched the lane to a release Cargo profile alongside the release variant. The variant was the point; the profile was mine, and it broke the build (run 4077): `generateUniffiBindings` fails with "No UniFFI metadata found" on the release `.so`. The workspace release profile sets `strip = true`, and uniffi's `--library` mode finds its interface metadata through symbols. That is the obvious suspect and it is recorded as a suspect, not a finding — `lto = true` dropping the metadata statics would print the identical message and the two have not been told apart. Backed out to the debug profile rather than guessing at a fix, because the two halves of that commit are not equally important. Signing and a rising versionCode are what make an install replace the last one instead of wiping the notes; the Rust profile only makes the result faster. The APK this produces is no worse than every previous build, all of which shipped a debug-profile `.so`. Recorded as Scribe #2810 with the four candidate fixes and, more usefully, the instruction to establish the cause on a host build before spending another four-minute cold cross-compile on a guess.	2026-08-20 19:16:03 -04:00
bvandeusen	d0a9c73bf9	android: sign the release build, and give it a version that rises ... Two separate reasons updates were impossible, both fixed here. **Every CI build was signed with a different key** (issue #2803, measured with `apksigner --print-certs` across two runs). No signing config meant AGP's debug keystore, which AGP GENERATES when absent — and every job starts from a fresh container. So no build could ever be installed over another: the only way through was uninstall-then-install, which deletes the app's database and every local note with it. **versionCode was hardcoded to 1.** `build.gradle.kts` has read a `THOUGHTSYNC_VERSION_CODE` property since the skeleton landed; nothing ever passed it. Even with signing fixed, every APK would have claimed to be the same version and nothing could tell a newer one existed. It now comes from `GITHUB_RUN_NUMBER` — the same monotonic counter the desktop's version scheme already uses, needing no state between runs and immune to the shallow checkout that makes a commit count useless here. The version NAME comes from the desktop's `build-version.sh`, so both surfaces report one product version rather than two that can disagree. **The alias is hardcoded, not a secret.** It is fixed for the life of the app and already written into the certificate every install carries; hiding it would buy nothing and stop this file describing its own signing. Two secrets, not three — and PKCS12 cannot hold a key password distinct from the store password anyway, so `keyPassword` is the same value by necessity rather than by shortcut. **The lane now builds RELEASE when it can sign, debug when it cannot.** That is not cosmetic. A debug APK is `debuggable`, which on a phone holding personal notes and a device sync token means anyone with adb can read both. Which meant confronting something the release path would have shipped quietly: `cargoNdkDebug` was hardcoded to the debug Cargo profile and every variant took its `.so` from it, so `assembleRelease` would have packaged an UNOPTIMISED store and sync engine. Now one `cargoNdk` task takes its profile from a property, and the whole run uses one profile. A debug/release task pair would have been the tidier shape and would have made a run that both type-checks and packages pay the four-minute cross-compile twice — this runner has no working Gradle or Cargo cache, so that cost is real on every push. The run prints the signing certificate after assembling, so the fingerprint can be compared against the one recorded at generation. Signing with the wrong key produces a perfectly valid APK that simply refuses to install — a failure that otherwise surfaces on the device, long after the run is green. `.gitignore` learns `*.jks`, `*.keystore`, `*.p12`, `*.b64` first, so generating a keystore anywhere near this tree cannot go wrong. Also corrects the record: the comment this replaces cited "Scribe task 2136" as though it were a standing rule. It is not one — none of the 46 always-on rules mentions signing keys. 2136 is a desktop-updater task whose REASONING got repeated until it sounded like policy. The reasoning holds, and holds harder on Android where a key cannot be rotated without the original, so the practice is unchanged; the citation is now honest about what it is.	2026-08-20 19:03:57 -04:00
bvandeusen	f38864088b	core: a completed recurring reminder advances instead of ending ... `complete_reminder` cleared `remind_at` and said so in its own comment — "(Recurrence advancement is a later refinement.)". So Done on a daily reminder was quietly the last time it ever fired. Reminder notifications made that much easier to hit, because Done is now a button in the notification shade. **The server already had this.** `src/thoughtsync/notes/recurrence.py` has done it correctly all along, which means the web behaved one way and the desktop and Android the other, on the same note, in the same account. This is a port of that file rather than a fresh implementation, kept behaviourally identical rather than merely similar: the same reminder can be completed from a browser or a client, and a disagreement would move it depending on which one you happened to use. The seven new tests in `core/src/local/recur.rs` mirror the Python suite case for case, including the one that matters most in practice — 31 January plus a month is 28 February, and the step after that is 28 March rather than back to the 31st. That clamp is sticky, and it is now asserted on both sides so a future "fix" to either has to change both. Advancement is measured from the reminder's own time, never from now, which is what keeps a 09:00 daily reminder at 09:00 when it is dealt with at 09:47. A phone left in a drawer for a fortnight rolls forward to tomorrow rather than arriving at fourteen pending occurrences of the same thing. Also matched from the server, and a latent bug of its own: the non-recurring branch now clears `recurrence` as well as `remind_at`. Before, completing a note that carried a rule left the rule behind with no reminder attached — invisible in every UI, since they only render recurrence when there is a reminder to recur from, and waiting to surprise whoever next set a time on that note. Documented rather than hidden, and shared with the server: the arithmetic is in UTC and a note carries no timezone, so a daily reminder crossing a DST boundary keeps its UTC time and shifts by an hour locally. Fixing that means a zone per note, which is a wire-format change. Verified in the CI image before pushing: fmt, clippy --all-targets -D warnings, and the full suite — core 89 to 96, ffi 11 to 12. The new FFI test walks the path the notification's Done button actually takes.	2026-08-19 21:22:22 -04:00
bvandeusen	8f13dc2e2c	android: restore the dismiss I deleted, and teach the checker to see it ... `785ebdb` failed at compileDebugKotlin with two `Unresolved reference 'dismiss'`. Splitting the reminder notification code into its own object, I removed `dismiss` from `Reminders` and never pasted it into `ReminderNotification`. The call sites were correctly qualified; the function simply was not there. All four local gates passed it, and `check-symbols.py` passed it for a reason it documented about itself: it only resolved the LEADING segment of a dotted expression, because that is the part a regex can resolve. `ReminderNotification` existed, so `ReminderNotification.dismiss(...)` looked fine. That was a real gap rather than an inherent one, so the checker now indexes the members of every `object` declared in the package and verifies `Foo.bar` against them. Brace-counted, not regex-matched — an object body is full of nested braces from lambdas and apply blocks, and no regex closes correctly over them. Verified by deleting `dismiss` from a copy of the tree again: it reports the same two call sites the Kotlin compiler did. What it still cannot see is narrowed and written down rather than left implied — members of anything declared outside this package, members reached through a variable rather than a type name, and every question about types.	2026-08-19 20:13:43 -04:00
bvandeusen	785ebdba59	android: reminders that actually reach you (M12 step 6) ... Reminders have been settable since the editor landed and have never once gone off. The board showed them overdue in red, which tells you what you already know by the time you are looking at the board. **AlarmManager, not WorkManager.** The background sync is right to be on WorkManager — nobody minds whether it runs at 3:05 or 3:19. A reminder minds very much. WorkManager's periodic floor is fifteen minutes and it batches into maintenance windows, so "remind me at 09:00" would routinely arrive at 09:14, which is not a reminder, it is a rebuke. **One alarm, not one per reminder.** Only the earliest future reminder is ever scheduled; when it fires, everything due is announced and the next is scheduled. A hundred reminders cost one alarm, and there is no incremental bookkeeping to drift — `Reminders.refresh` recomputes the whole picture from the store, and is called from everywhere anything could have changed: an edit, a foreground, a background sync, boot, and an app update. Boot and MY_PACKAGE_REPLACED both matter and both are easy to forget. Pending alarms survive neither, and this app updates by APK from its own server, so without that receiver a phone would silently stop reminding anyone of anything after a restart — the worst kind of failure, because nothing appears wrong. **Neither permission is treated as a prerequisite.** SCHEDULE_EXACT_ALARM, not USE_EXACT_ALARM: the latter is granted at install with no prompt and is reserved for apps whose whole purpose is an alarm clock or a calendar, which this is not. Refusing the former costs precision, not the feature — it falls back to an inexact alarm, because a reminder a few minutes late beats no reminder. POST_NOTIFICATIONS is asked for on the first launch where a reminder actually exists, never at launch on an empty board. Android gives an app essentially one chance at that dialog, and spending it before the person has any idea what this app would send them is spending it on nothing. For anyone who refuses, or who turns notifications off later in system settings, the Reminders view carries a standing notice with a button to the right screen — a feature that silently does nothing is worse than one that is plainly absent. **A first run adopts overdue reminders silently.** The storm case is linking a server and pulling months of history; a hundred notifications the moment someone signs in is a good way to have the feature turned off before it is ever useful. After that, a missed reminder is announced up to a day late — the web uses fifteen minutes because an open tab has been polling every forty-five seconds, but a phone can be switched off all night. Done and Snooze act from the shade without opening the app. The dedupe key is note id plus remind_at, the same one the web store uses, so snoozing produces a new occurrence rather than one already dealt with. Tapping a notification opens that note. The extra is CONSUMED when read: the Activity keeps the intent it was launched with, so without that, rotating the phone would replay it and reopen a note the person had already closed. `Reminders` split into scheduling policy and `ReminderNotification` rendering after detekt counted fourteen functions in one object — it was right, they answer different questions and change for different reasons. `ForegroundTransitions` moves to the ui package; the reminder notice needs it to re-read a permission the person may have just changed in a system screen this app cannot observe. Known gap, pre-existing and shared with every surface: `complete_reminder` in the core clears a reminder without advancing recurrence — its own comment says so. So tapping Done on a daily reminder ends it rather than moving it to tomorrow. Not changed here because it is core behaviour the desktop and web also have, but notifications make it much easier to hit, and it should be next.	2026-08-19 20:06:05 -04:00
bvandeusen	39170b715c	android: leaving the composer keeps the note, and the board loses its dead space ... Two things the operator hit on a real device. **Capture threw work away.** Every exit from the compose sheet except Save discarded it — tapping the board behind, swiping down, back, backgrounding the app, and rotating the phone. That is the wrong default anywhere and the worst possible one here: a sheet that loses a typed thought because you touched outside it teaches people not to trust the app with a thought, and capture is the one place this product cannot afford that. Now every way out saves, which is the shape the editor already settled on. The difference is that capture also has to be abandonable — tapping + and changing your mind is normal — so Discard exists and is the only path that loses anything. It is called Discard rather than Cancel because "cancel" means "undo what I am doing", which is precisely what leaving no longer does; the word would have described the one button it is not attached to. An empty draft needs neither and is simply dropped: a blank note nobody asked for is worse than none. Backgrounding persists but does NOT close an empty sheet. Someone who tapped + and got distracted should find the composer where they left it. Rotation was losing it twice over: the draft was `remember`, and so was the flag saying the sheet is open. Both are `rememberSaveable` now, along with the sync screen's — the editor never had the bug because the note it sits on lives in a view model, and these were the only screen state that did not. `FlushOnStop` moves out of NoteEditorScreen into its own file; the editor and the capture sheet want the identical thing for the identical reason, and it was about to be copied. **The board had a centimetre of nothing above the search field.** `SearchBar` applied `statusBarsPadding()` inside a `Scaffold` whose content padding already carries the system-bar insets — `ScaffoldDefaults.contentWindowInsets` is `systemBarsForVisualComponents`, checked in the material3 sources rather than assumed. So the status bar height was reserved twice on the first screen anyone sees. Insets get consumed once, by whichever component owns the edge.	2026-08-19 19:39:40 -04:00
bvandeusen	5680f046e3	android: name all four permissions WorkManager adds, not one ... The note added with the previous commit said RECEIVE_BOOT_COMPLETED arrives in the merged manifest via WorkManager. True, and incomplete — it brings four: RECEIVE_BOOT_COMPLETED, ACCESS_NETWORK_STATE, WAKE_LOCK and FOREGROUND_SERVICE. A comment whose whole job is "here is why the permission list has entries this file does not declare" fails at that job if it accounts for one of them. Each now says what it is for, checked against the built APK's merged manifest rather than the library's — which is the version a person actually sees.	2026-08-19 19:11:19 -04:00
bvandeusen	452c66c8ef	android: sync without being asked (M12 step 6) ... Until now every sync was a button press. Pull-to-refresh made asking cheaper; it did not stop the app needing to be asked, which on a phone means a note written on the bus reaches the desktop whenever you next happen to open the app. Three moments, and they are deliberately not the same job: * **Coming to the front**, if the last sync is over five minutes old or there is unsent work. Not on every foreground: stepping out to copy a link and stepping back is not a request for fresh notes, and syncing on every app switch spends someone's mobile data telling them what they are looking at. * **Going away with unsent work** — handed to WorkManager rather than run inline, because the process is about to stop being a priority and a sync started there would be killed halfway. This is the one that matters most: it is what gets a note off a phone that then goes into a pocket for the night. * **Every fifteen minutes**, network-constrained. Fifteen is not a preference, it is WorkManager's floor for periodic work; asking for less gets fifteen. **An automatic sync must not raise an error banner.** Someone who pulled the board down is owed an answer; someone who merely opened the app did not ask a question, and answering it with a red banner about an unreachable server makes their own notes look broken when nothing of theirs is. So `syncNow` and `syncQuietly` differ in exactly one thing — whether failure is announced. The quiet channel for a persistent problem is the drawer badge, from `has_pending`, which does not care how the attempt was made. **There is a switch, defaulting to on.** Linking a server IS the consent; a person who paired a device and then had to find a second toggle before anything moved would reasonably call that broken. It lives in SharedPreferences rather than the store: everything else in sync state describes the PAIRING and must survive a reinstall, while this describes how one handset behaves, and someone turning it off on their phone is not asking their laptop to stop. The copy says what "automatically" means in minutes and says that off is not off — a switch next to a Disconnect button invites exactly that misreading. The schedule is DECLARED as a function of (linked, switch) in a LaunchedEffect rather than toggled from the places that change them. There are four routes to "should not be syncing on its own" and a call at each is four chances to leave a phone quietly syncing after it was told to stop. `ON_START`/`ON_STOP`, not resume/pause — the same choice the editor's save-on- leave makes, because pause fires for anything covering the window and a sync per notification-shade pull is not automatic sync, it is a stutter. RECEIVE_BOOT_COMPLETED now appears in the merged manifest. WorkManager contributes it so the schedule survives a restart; commented in AndroidManifest because it shows in the app's permission list and nothing else in that file would explain it. Two things read from artifacts rather than recalled, both of which memory would have got wrong: `work-runtime-ktx` is an empty 6 KB stub as of 2.11 with `CoroutineWorker` and `PeriodicWorkRequestBuilder` moved into `work-runtime`, so the dependency is on the latter alone; and `Switch` is not experimental in material3 1.4.0, so no `@OptIn` — an unnecessary one is itself a warning. Also adds `android/tools/check-strings.py`, after this change added three strings: `R` is generated, so `R.string.typo` type-checks whether or not the string exists. It catches a missing name, `stringResource` on a plural or the reverse, and a format taking more arguments than the call passes. Verified against a tree with one of each fault — its first version counted Kotlin's trailing commas as arguments and called three correct sites broken, which is the failure that teaches you to ignore a tool. Two comments in this change were wrong when written and are corrected here rather than left: the flag check in SyncWorker does NOT avoid opening the store, because Application.onCreate has already run by the time any Worker starts.	2026-08-19 19:04:45 -04:00
bvandeusen	64542ed6cb	android: pull the board down to sync (M12 step 6) ... Every sync so far has been a button press on a screen you have to navigate to. On a phone the gesture for "check if there's anything new" is a pull, and not having it is the kind of absence people read as the app not syncing at all. **The gesture is INERT when this device has no server.** `Modifier.pullToRefresh` takes an `enabled`, which is why the modifier and the indicator are wired by hand instead of using `PullToRefreshBox` — that wrapper is less code and offers no way to turn the gesture off. An unlinked device has nowhere to pull from, and a gesture that always comes back empty is how people learn a control is broken. Same reasoning as the drawer badge staying silent when unlinked: local-only is this app's resting state, not a fault. **A failed refresh reaches the board.** Otherwise the spinner retracts and nothing happens, which is indistinguishable from "you were already up to date" — the one outcome it must not be confused with. It renders as a second banner rather than replacing the store-error one: those are different facts about different halves of the app, and hiding either behind the other reports the wrong problem. Dismissing is honest — the note is still pending, `hasPending` still says so, and the next cycle reports the same fault if it persists. **The empty board is now a `LazyColumn` holding one centred item.** Pull-to- refresh works through nested scroll, and a layout that never scrolls never dispatches any, so on the old plain `Column` the gesture would have been dead on exactly the screen where it matters most: linked, board empty, notes still on the server. Looks identical. The five sync facts the board needs arrive as one `BoardSync` rather than five parameters, for the reason `EditorAction` exists: `summary` and `error` are both `String?` and both about sync, so positionally they could be swapped with nothing to catch it. Still no automatic sync — no background cycle, no sync-on-resume. This is a faster way to ask, not a decision to stop asking. TalkBack users cannot perform a pull; the drawer's Sync → Sync now remains the accessible path, unchanged. Verified against the real artifact rather than from memory, since `material3` resolves through the BOM: 1.4.0's sources confirm `pullToRefresh` has `enabled`, and that none of `pullToRefresh`, `rememberPullToRefreshState`, `Indicator` or `PullToRefreshBox` is `@ExperimentalMaterial3Api` there — only two deprecated members are. So no `@OptIn`, which is what keeps the build at zero warnings.	2026-08-19 16:36:56 -04:00
bvandeusen	65d8f5f9c6	android: the import ktlint and detekt cannot see ... `750d11d` failed CI at `compileDebugKotlin` with `Unresolved reference 'Build'`. `defaultDeviceName()` reads `android.os.Build`, and the import was lost when `SyncPairing.kt` was split out of `SyncScreen.kt`. One line to fix. The interesting part is that ktlint and detekt had both passed it, locally and in CI. Neither resolves symbols — they parse — so a file that cannot compile is indistinguishable to them from one that can. A clean analyzer run is not evidence the code builds, and on this repo `compileDebugKotlin` is the only gate that type-checks at all, since there is no Android SDK on the workstation. So: `android/tools/check-symbols.py`, covering that one blind spot. It flags any capitalised identifier that is neither imported, declared in the same package, a type parameter, nor implicitly available. Not a type checker and not pretending to be — a pre-push filter for the single mistake that survives every other local gate, erring toward false positives. Verified against a known-bad tree rather than trusted on a green: deleting the `Build` import from a copy makes it fail with the same two references the Kotlin compiler reported. That step is not ceremony. An earlier attempt at this check stripped line comments with `re.S`, where `//.*` eats each file from its first comment to EOF — it examined almost nothing and reported everything clean. ci-requirements.md now documents all three Kotlin checks, and its claim that no workflow consumes the Android image yet is gone; the lane has been running since step 5.	2026-08-19 15:51:12 -04:00
bvandeusenandClaude Opus 5	750d11d32e	android: connect a server from the phone (M12 step 6) ... The plumbing has been bound since step 4 — probe, link by password or token, unlink, sync — with nothing on top of it. Until this commit the phone was a good standalone notes app that could not be the SAME notes as the desktop, which is the point of the project. Structurally a port of the desktop's SyncView.vue: same probe-then-link order, same copy wherever the copy was already right. The two surfaces pair with the same servers, and a difference in wording here would read as a difference in behaviour. BEING UNLINKED IS NOT A PROBLEM, and the screen is written around that. It leads with "Working offline on this device" and says what connecting would ADD. A local-first app that frames its resting state as unfinished setup is lying about what it is. The drawer badge follows the same rule: it says nothing at all when unlinked, rather than "Off". Probe before credentials. A typo that reaches a stranger's server should cost a round trip, not a password — so the address is checked first, what answered is shown (name, version, compatibility), and only then does a sign-in form appear. An incompatible server never gets one; the core would refuse the link anyway, and collecting a password to throw away is worse than not asking. CLEARTEXT IS NOW PERMITTED, deliberately and not silently. Android blocks plain http from API 28, and the core explicitly supports a self-hosted server on a LAN — `http://192.168.1.10:8000` is a case it has a test for. The platform default would make this app unusable for exactly the people it is built for, with a transport error they could do nothing about. A network-security-config would be tighter in principle but matches domains and IP literals, not CIDR ranges, so it cannot express "my own network". The other half of the trade is a warning that appears the moment a probed address starts with http:// and BEFORE any credential field: anyone on the same network can read your password and your notes. Credentials never enter the view model. The address, email and device name are `rememberSaveable` so a rotation doesn't cost a retype; the password and the token are plain `remember` on purpose — rememberSaveable persists into the instance-state bundle, and a secret has no business being written there to save four seconds of typing. They reach the core as a `Credentials` sealed type and die with the composable. That sealed type also fixed a bug detekt surfaced by complaining about a six-parameter function: `link_with_token` takes NO device name (the token was already minted against a named device in the web app), so the flat argument list meant the form collected one in token mode and silently dropped it. The field now exists only on the password path. Threading, which differs by call and is easy to get wrong in one direction: probe / linkWithPassword / linkWithToken / unlink / syncNow are Rust async through uniffi, so Kotlin sees suspend functions already driven by tokio and awaits them directly — wrapping them in Dispatchers.IO would park a thread to wait on something that never blocks one. syncStatus and hasPending are ordinary blocking FFI into SQLite and do need it. A sync that changed anything tells the board to reload, because a pull can have rewritten every note it is holding. Wired explicitly at the one place that owns both view models rather than through a shared event bus. A no-op sync deliberately does not, so the board never flashes its loading state for nothing. Sync results are kept RAW in state and turned into sentences in the UI, where stringResource is in scope — the same split Time.kt draws for timestamps. The summary counts what MOVED; batches, pages, noop and cursor are all real numbers and none of them answer "are my notes in step". Rejections are surfaced rather than swallowed: only a person can resolve them. So is a revoke that didn't land — someone disconnecting to retire a phone has to be told a live credential is still out there, and has to still find it when they come back to check, so it is a persistent notice and not a toast. Also here: `Panel`/`Notice` extracted as shared tinted chrome, drawn from the same note palette the cards use rather than Material's errorContainer, so a warning is the same yellow a note can be. `PlainTextField` gained a visual transformation for the password field. `formatReminder` became `formatInstant` now that "last synced" reads it too. Verified locally per ci-requirements.md: ktlint and detekt clean in ci-rust-android:1.97, uniffi bindings generated from a host build and read to confirm ULong on the summary counters, `Compatibility.Ok`/`RevokeOutcome. Unsupported` being objects, and all five sync calls being suspend. Every R.string/R.plurals reference cross-checked for existence, kind and format arity. A symbol-resolution pass over the whole package caught a composable a bad edit had deleted — ktlint and detekt both parse without resolving, so neither could see it. Not done: no automatic sync. The desktop is manual-only too, so this is parity rather than a gap, but pull-to-refresh on the board is the obvious phone-native follow-up. Worth an operator decision, not changed here: allowBackup is still true, so Android's cloud backup now includes a device token as well as the notes. Good for restoring to a new phone, and a wider blast radius than before this commit. Scribe #2777 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 15:41:02 -04:00
bvandeusenandClaude Opus 5	cf0ce382a0	android: the note editor (M12 step 6) ... Tapping a card now opens something. Until this commit the phone could create, find and navigate; it could not change anything. A FULL SCREEN, not a sheet. Capture is a sheet because the board behind it is reassurance that the thought landed; editing is a sustained task with the keyboard up, and a sheet would spend the whole time fighting the IME for the bottom half of the display. Full screen also puts the actions in a bottom bar, which is where a thumb already is. The note's colour paints the whole screen, so opening one reads as the same object growing to fill the display. Text saves ONCE, on close — plus on ON_STOP, so app-switching mid-paragraph doesn't lose it. Not debounced autosave: the core snapshots a revision on every title/body change, so saving per typing pause would fill version history with near-identical entries. A baseline check means opening a note and backing out writes nothing at all, rather than bumping updated_at and marking it dirty for sync. Same shape the web editor settled on, for the same reason. The editor speaks in ACTIONS, not callbacks. The first version passed a bundle of twenty lambdas and the doc comment on it was already worrying about two of the same-shaped ones getting swapped, with nothing to catch it. `EditorAction` plus one `(EditorAction) -> Unit` costs a `when` at the far end and buys exhaustiveness: adding a variant breaks the dispatcher until it is handled. Checklist rows are live here — real checkboxes, editable text, remove, and an add row that keeps focus so a list types straight through. That is the answer to the open question about list entry: the capture sheet stays one-item-per- line because at capture time the list is already in your head and a tap per row is the slow part; the editor is where a list is REVISED, and revising is item-at-a-time. Row text commits on focus loss, not per keystroke — each commit is a store write that reloads the note. Colour, labels and reminders are bottom sheets. Reminders lead with presets (later today / tomorrow / next week) and keep the exact picker one tap down: the web's raw datetime-local is right for a desktop and three taps too many for the common case on a phone. Recurrence only appears once there is a reminder to recur from. The date picker reports UTC midnight of the calendar day tapped and is read back in UTC — reading it in the device zone is the classic off-by-a-day in that control. Pin, labels, archive and delete live in the overflow as WORDS. `material-icons-core` has no pin, archive or label glyph, and the alternatives were pulling in the ~1,000-vector extended set for four icons or pressing unrelated ones into service — a star meaning "pin" is a star meaning "favourite" to everyone who has used another app. The colour button is a dot in the note's current colour, which says what the colour IS as well as what the button does. A trashed note renders read-only. Editing one would silently resurrect work that was meant to be thrown away; Restore and Delete forever are the only things to do with it. Deleting for good is the one irreversible action in the app and gets the one confirmation in it. `#tag` labels are never sent to `set_labels` and get no remove button. They are owned by the body text and the core re-derives them on the next edit, so a cross that undid itself a second later would look broken. FFI additions: delete_note_forever, add_item, set_item_text, set_item_checked, delete_item, complete_reminder, snooze_reminder, set_note_labels, create_label. `set_item_text`/`set_item_checked` are split rather than exposing the core's {text?, checked?} patch, for the same reason NoteEdit is a list — an optional-field struct cannot say "leave this alone" in Kotlin without colliding with "set it to null". Four new tests (11 total in the crate). Found while extracting shared helpers: the card painted EVERY reminder blue, so "you missed this" and "coming up Friday" looked identical. Now red when overdue and neutral otherwise, matching the web card's exact pairs. And the error banner was renderable only by the board — the one screen that needed it, where the writes happen, was the one screen without it. DRY, since three copies each had appeared: PlainTextField (the undecorated field used by capture, editor, checklist rows and the search bar), Time.kt (the RFC3339 seam), NoteKind.kt, ErrorBanner. detekt: LongMethod and LongParameterList now ignore @Composable. Compose breaks those rules' PREMISE, not just their thresholds — a composable's parameters are its UI contract and its length tracks how many elements are on screen, not branching. Two suppressions carry their reasoning at the site instead: onEditorAction is sixty lines because EditorAction has twenty variants, and splitting it would need an `else` that throws away the exhaustiveness; and BoardViewModel stays one class because every editor mutation has to reload the board behind it. Verified locally before pushing, per ci-requirements.md: fmt/clippy/test in ci-tauri:1.97 (89 + 11 + 11 tests, four crates present), ktlint and detekt in ci-rust-android:1.97, uniffi bindings generated from a host build and read to confirm every method and field name the Kotlin calls. Still unbuilt: attachments, link previews, version history, and label management (rename/recolour/delete). Setting up a server from the phone is next. Scribe #2777 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 11:18:24 -04:00
bvandeusenandClaude Opus 5	64e016f32d	android: phone-shaped chrome and the real note card (M12 step 6) ... Two things at once, because they answer one question: what should this look like, and what should it look like ON A PHONE. IDENTITY IS SHARED, INTERACTION IS NOT. The card now renders exactly what the web and desktop render — note colour, checklists, label chips, reminders — using the same palette values, so a note looks like your note on every surface. The chrome does not: the desktop's title bar and sidebar are wrong for a thumb. * NoteTint.kt carries the Tailwind colours from frontend/src/notes/colors.ts VALUE FOR VALUE, generated from tailwindcss 3.4 rather than eyeballed. Dark tints keep the web's alpha (dark:bg-*-950/40) instead of a precomputed blend, because Compose composites translucency over the background exactly as CSS does. * Dynamic colour is GONE. It was the more Android-native choice and it made the app look like a different product — on a stock emulator with no wallpaper it renders as undifferentiated grey, which is what the operator saw. Three peer surfaces share one identity; the brand #F5C518 is the same value the web manifest and the launcher icon already use. * The board is a two-column staggered grid, the Compose equivalent of the CSS multi-column NoteGrid.vue uses. PHONE ERGONOMICS, chosen with the operator: * Search IS the top bar. After writing a note, finding one is the most common thing you do, and burying it behind an icon costs a tap every time. Debounced 180ms and cancelled per keystroke — without that a fast typist queues one full-text query per character and results land out of order. * A + button is the only way in. One obvious target beat a capture bar and a button competing for the same job. * Navigation moved into a drawer behind the search bar's menu icon, which is where archive/trash/labels/reminders now live. They had nowhere to go once search took the top bar, and would otherwise have been unreachable. * The compose sheet asks note-or-list up front. On a phone those are different typing tasks and switching halfway is worse than choosing at the start. A list takes one item per line — fast to type, versus a tap per row. Three new bindings the UI needed: search_notes, reminder_notes, list_labels. Search goes through the CORE so "what matches" cannot drift between surfaces; filtering the loaded list in Kotlin would have been less code and a different product. reminder_notes is its own call because the core models it that way — "has a reminder" cuts across archived and active alike. Empty states are per-destination. "Nothing here yet" is encouraging on an empty board, wrong in Trash, and misleading after a search where the notes exist but did not match. Verified locally before pushing: bindings generated from a host .so and read back, ktlint and detekt clean from the image's pinned CLIs, cargo fmt/clippy/test green (107 tests). Two detekt findings were fixed by extraction rather than by relaxing the rules — this is the first Compose code in the repo and the thresholds should have to earn their exceptions. Still unbuilt: tapping a card does nothing. The editor is next. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 09:23:00 -04:00
bvandeusenandClaude Opus 5	eb3dc3d893	gitignore: don't let a downloaded APK into history ... Debug APKs get pulled into the working tree for emulator testing. They are ~57 MB and come from CI artifacts, so they are never a source — but nothing stopped `git add -A` from committing one permanently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 08:31:44 -04:00
bvandeusenandClaude Opus 5	c8af808432	android: package only the ABIs we actually build for ... The first working APK carried libjnidispatch.so for armeabi, mips and mips64 as well as our four — JNA's .aar still ships those, and AGP packages whatever it finds. Android dropped mips in NDK r17 and armeabi in r17 too; nothing that can install this app can load them, so they are pure payload. abiFilters pins the set to the four the Rust is actually cross-compiled for, so the APK's ABI list matches the build's intent rather than the union of every dependency's history. Found by unpacking the artifact rather than trusting the green: the run said "Upload debug APK ✓", which is true and says nothing about what is inside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 16:24:37 -04:00
bvandeusenandClaude Opus 5	5eab2dd0b3	android: the error enum has to be flat, or the bindings don't compile ... Fifth run cleared ktlint and detekt and failed compiling the GENERATED Kotlin: 'message' hides member of supertype 'Throwable' and needs an 'override' modifier My design, surfacing one layer down. CoreError's variants carried a `message` field, and uniffi turns an error enum into exception classes extending Throwable — which already has `message`. `#[uniffi(flat_error)]` is the right fix rather than renaming the field. Renaming would dodge the collision and leave `e.message` null on the Kotlin side, so every call site would have to know which variant it caught just to read the text. Flat passes the Display string to the Throwable constructor, where Kotlin expects it, and costs nothing that matters: each variant is still its own subclass, so `catch (e: CoreException.NotLinked)` still works and a `when` is still exhaustive. Only the fields stop crossing, and for every variant that has one the field IS the Display string. Confirmed by generating the bindings and reading them: sealed class CoreException(message: String): kotlin.Exception(message) { class NotLinked(message: String) : CoreException(message) class Store(message: String) : CoreException(message) class Network(message: String) : CoreException(message) } That check is worth keeping. thoughtsync-ffi already builds a HOST .so as part of the workspace, and `--library` mode reads metadata straight out of it — so the exact Kotlin the Android lane will compile can be generated and inspected here, with no Android toolchain involved. It also let me verify the app's call sites against the real generated API rather than against my assumptions about uniffi's naming: ThoughtSync(dataDir), createNote(draft), listNotes(query), Note.displayTitle, and NoteDraft/NoteQuery's parameter names all match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 16:15:46 -04:00
bvandeusenandClaude Opus 5	dee71dffb3	android: teach the linters this codebase's conventions, and fix two real nits ... Fourth run got the whole native pipeline through — cargo-ndk built all four ABIs and uniffi generated the Kotlin — and then failed on style. Two genuine mistakes, fixed: * BoardViewModel's constructor parameter needed its own line. * PaddingValues was written fully-qualified inline, which ktlint read as a method chain. Importing it is what the rule was actually asking for, and what the line should have said anyway. The other ten were the tools not knowing this codebase: * @Composable functions are PascalCase by universal Compose convention. Exempted in BOTH .editorconfig (ktlint) and config/detekt.yml — they have to agree or one of them is always wrong. * MagicNumber on `private val Brand = Color(0xFFF5C518)`. The rule asks for a well-named constant; that line IS one. ignorePropertyDeclaration. * TooGenericExceptionCaught in the ViewModel and Application. Deliberate and already commented: a note that fails to save must become a visible error banner rather than a crash, and the store failing to open must still let the app start so it can explain itself. Scoped to those two paths, not disabled globally — everywhere else the rule is right. Verified locally this time, both linters clean, using the SAME pinned CLIs from ci-android:36 that the lane runs. ktlint and detekt are a formatter and a static analyzer — the same category as cargo fmt and clippy, which is the precedent ci-requirements already sets. No build was run locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 16:04:40 -04:00
bvandeusenandClaude Opus 5	5d0de7a682	android: the binding generator gets its own crate, free of the app's deps ... Third Android run got further than either before it — all four ABIs cross-compiled, vendored OpenSSL and all — then the generator died: error: failed to run custom build command for `openssl-sys v0.9.117` That is the HOST build. The generator was a [[bin]] inside thoughtsync-ffi, so building it compiled that crate and therefore the core, reqwest, native-tls and openssl-sys for linux. The vendored-OpenSSL block is scoped to `cfg(target_os = "android")`, so the host build went looking for a system OpenSSL that ci-rust-android has no reason to carry. Adding libssl-dev to the image would have fixed it and been wrong: a code generator has no business linking the app's TLS stack to emit Kotlin. Splitting it into thoughtsync-uniffi-bindgen, whose only dependency is uniffi, removes the entire chain. Verified from the dependency graph rather than from a build that happened to succeed — `cargo tree -p thoughtsync-uniffi-bindgen` contains none of openssl-sys, native-tls, reqwest, thoughtsync-core or rusqlite. It stays a WORKSPACE MEMBER on purpose. Sharing one lockfile is what keeps uniffi here and uniffi linked into the .so at one version; they are two halves of one ABI, and a separate lockfile is precisely how they would drift apart. The cost is that the desktop lane now compiles ~15 generator crates it never runs — cheap next to Tauri, and better than leaving the crate unlinted. Drops the `bindgen` feature and required-features bin from thoughtsync-ffi, which existed only to keep those crates off the desktop lane and now have nothing to gate. Local fmt + clippy + test all green before pushing (107 tests). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 15:54:10 -04:00
bvandeusenandClaude Opus 5	3d3df1beb0	android: register generated sources through the Variant API ... Second Android run failed with AGP 9 refusing the previous fix by name: You cannot add Provider instances to the Android SourceSet API. [...] Instead you should use the Sources interface in the Variant API, in particular SourceDirectories.addGeneratedDirectory AGP cannot tell from a Provider whether the directory holds generated (read-only) or hand-written (read-write) files, which is a distinction the IDE needs. `addGeneratedSourceDirectory` is the supported route and — unlike the plain-path form the error offers as an escape hatch — it carries the task dependency, so Kotlin still cannot compile before the bindings are generated and the APK cannot package a stale .so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 15:43:57 -04:00
bvandeusenandClaude Opus 5	f179928c57	android: run ktlint and detekt from the image, not as Gradle plugins ... First Android run failed at plugin resolution: Plugin [id: 'io.gitlab.arturbosch.detekt', version: '2.0.0-alpha.3'] was not found in any of the following sources That version is published to neither Maven Central nor the plugin portal — the latest detekt anywhere is 1.23.8. It was copied from Minstrel's catalog, where it presumably resolves from a cached artifact; copying a pin without checking it exists is what made it my problem. Rather than chase a working plugin version, the analyzers now run from the CLIs ci-rust-android already ships. That was the point of putting them in the image in step 3, and going through Gradle plugins would have meant a SECOND pinned version of each tool, resolved at build time, kept in lockstep with the image's by hand. One less resolution step, and step 3's decision finally earns its keep. Also replaces the source-ordering hack while here. Kotlin has to compile after the bindings are generated, and the usual `tasks.withType<KotlinCompile>` cannot be written in this build at all — AGP 9's built-in Kotlin means that class is not on the buildscript classpath. Passing the TASK PROVIDERS to srcDir instead lets Gradle read their @OutputDirectory and infer the ordering itself, which is the idiomatic form and removes the dependsOn entirely. Good news from the failed run: the Gradle wrapper check passed, so Gradle 9.1.0 on the image's JDK 25 works — the toolchain decision from step 3 holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 15:35:17 -04:00
bvandeusenandClaude Opus 5	20907abf6e	android: a Kotlin/Compose app that drives the Rust core (M12 step 5) ... The skeleton, and the lane that builds it. Gradle invokes cargo-ndk to cross-compile thoughtsync-ffi for four ABIs, generates the Kotlin bindings from the resulting .so, and packages both. BUILT ON MINSTREL'S TOOLCHAIN, not a fresh guess. Gradle 9.1.0 / AGP 9.0.1 / Kotlin 2.3.21 on JDK 25 is the combination already proven in this family on ci-android, including the JDK 22+ native-access opt-in the launcher JVM needs and the artifact-upload action pinned by SHA (issues 2255 / 2270). It also independently confirms the JDK 25 call made on ci-rust-android in step 3. Gradle wiring worth noting: * ExecOperations, not project.exec — the latter was REMOVED in Gradle 9, and touching `project` at execution time is also what breaks the configuration cache this build enables. * The cargo task's inputs are the Rust SOURCES, not the workspace directory. Declaring the directory would make Gradle hash target/, which is gigabytes. * Bindings are generated with `--library` against the built .so, so they can never describe a different version of the Rust than the one being packaged. * cargo runs --locked, so an Android build cannot silently re-resolve the lockfile the desktop lanes are gated on. JNA is a real dependency, with the @aar classifier. The plain jar builds fine and fails at runtime with UnsatisfiedLinkError, which is the worst way to learn it. R8 keep rules for JNA and the bindings are in for the same reason — that failure would otherwise appear only in a minified release. The UI is a working board, not a debug screen: capture field, note list, empty state, error banner, and an honest failure screen for a store that won't open. Rules 23/24 — a surface ships at quality from the first commit. Capture uses the IME action key because the north star is a thought captured in under a second, and leaves the title empty so the core derives it from the first body line. Every core call runs on Dispatchers.IO: they are blocking FFI into synchronous SQLite, and running them on the main thread is exactly the jank going native was meant to avoid. The launcher icon reuses frontend/public/icon-maskable-512.png as an adaptive foreground on the brand #F5C518 — the same asset and colour the web app already ships, so the three surfaces wear one face. No signing config. A release keystore that has passed through an agent session or shell history is compromised by construction (task 2136); it has to be generated by the operator and reach CI only as a secret. CI builds debug. CI can only prove this BUILDS — a Linux runner cannot execute an APK, so feel and on-device correctness remain an operator pass on an emulator. Scribe #2739. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 15:24:30 -04:00
bvandeusenandClaude Opus 5	e7937ea87e	ci-requirements: the Rust lane can be checked before pushing, not just after ... fmt was already documented here. Operator authorised clippy and test through the same pinned image on 2026-08-18, so the section now covers the whole pre-push loop rather than a third of it. The commands are byte-identical to the workflow's on purpose — a local check that differs from CI is worse than no local check, because it produces confidence without coverage. That this is a faithful proxy is not an assumption: the local test binary hashes matched CI run 3931's exactly (thoughtsync_core-bbaae797…, thoughtsync_desktop_lib-9d162263…, thoughtsync_ffi-fc557b96…). Same image, same lockfile, same compilation units. Also records that target/ persists on the host, which is why the second run costs ~30s rather than several minutes, and that it is gitignored and disposable. Scope note in the text: this authorises fmt/clippy/test only. Not the bundle build, not a local stack. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 13:33:12 -04:00