From 67e8fa13a26f748959a489e2ef35ee84c662a4fd Mon Sep 17 00:00:00 2001 From: Chris Birch Date: Sun, 8 Nov 2026 01:46:13 -0500 Subject: [PATCH 1/6] feat!: remove the sync engine and the database's synced copy Every desk now works through the projects API, and the one machine that never syncs keeps its database as its only copy. Nothing is left to keep a copy of, so the engine, `projects sync`, the [sync] section and the refusal of command-line writes beside sync all go. - A database from before opens as it is. The first open drops pending_sync, sync_state and sync_origin, and keeps every project, item, task, dependency, membership and undo entry. - A config still holding [sync] loads, and every command that opens data warns on stderr naming the file. A refusal would stop the tool on a machine until its next config apply. - A PROJECTS_SYNC* or TODOUI_SYNC* variable set to a value stops every command with exit 2. PROJECTS_SYNC_URL chose an API, so ignoring it would send a run wherever the config file points. - The board re-reads the API every two minutes, fixed, where sync.interval used to set it. R re-reads the data in either mode. - The undo result keeps the four fields the API sends. The rows it also carried were for pushing an undo, and nothing else read them. - store/sqlite's generated queries move under internal/, so only the store can reach them now that the engine no longer does. Agent-Name: claude-code-unattended Agent-Session: 43e2375e-0270-4942-8076-19e8ab6a2337 Agent-Run: 01a1b9cf-185b-7680-9738-f82275bfa5f7 Worklog: 2407.1 --- CLAUDE.md | 170 +-- README.md | 116 +- backend/backend.go | 4 +- cli/auth.go | 18 - cli/data_source.go | 130 +- cli/item_create.go | 2 +- cli/item_dependencies.go | 7 +- cli/item_memberships.go | 4 +- cli/item_tasks.go | 6 +- cli/items.go | 8 +- cli/projects.go | 6 +- cli/root.go | 2 +- cli/schemas/dependency-tree.schema.json | 2 +- cli/sync.go | 67 - cli/tables.go | 3 +- cli/tui.go | 11 +- config.example.toml | 24 +- config/config.go | 86 +- config/config_test.go | 145 +-- config/former_name.go | 6 +- config/retired_sync.go | 33 + former_name_test.go | 14 +- main.go | 8 +- model/project.go | 2 +- model/project_item.go | 14 +- model/undo.go | 48 +- real_binary_flows_test.go | 2 +- service/number_test.go | 19 - service/references.go | 4 +- service/service.go | 15 +- service/service_test.go | 27 +- service/undo.go | 84 +- store/sqlite/convert.go | 2 +- store/sqlite/{ => internal}/generated/db.go | 0 .../sqlite/{ => internal}/generated/models.go | 23 - .../{ => internal}/generated/queries.sql.go | 430 +------ store/sqlite/open.go | 90 +- store/sqlite/open_test.go | 125 -- store/sqlite/queries.sql | 123 +- store/sqlite/schema.sql | 45 +- store/sqlite/sqlc.yaml | 2 +- store/sqlite/store.go | 2 +- sync/backend.go | 550 --------- sync/engine.go | 276 ----- sync/engine_test.go | 1074 ----------------- sync/operations.go | 41 - sync/origin.go | 171 --- sync/origin_test.go | 266 ---- sync/pull.go | 446 ------- sync/push.go | 221 ---- sync/status.go | 9 - sync/token.go | 153 --- sync/token_test.go | 257 ---- .../from-the-item-pane.style | 18 +- .../from-the-item-pane.txt | 4 +- .../from-the-project-pane.style | 12 +- .../from-the-project-pane.txt | 4 +- tui/app.go | 149 +-- tui/app_test.go | 155 +-- tui/editor.go | 2 +- tui/realdata_test.go | 3 +- tui/styles.go | 12 - 62 files changed, 320 insertions(+), 5432 deletions(-) delete mode 100644 cli/sync.go create mode 100644 config/retired_sync.go rename store/sqlite/{ => internal}/generated/db.go (100%) rename store/sqlite/{ => internal}/generated/models.go (77%) rename store/sqlite/{ => internal}/generated/queries.sql.go (73%) delete mode 100644 sync/backend.go delete mode 100644 sync/engine.go delete mode 100644 sync/engine_test.go delete mode 100644 sync/operations.go delete mode 100644 sync/origin.go delete mode 100644 sync/origin_test.go delete mode 100644 sync/pull.go delete mode 100644 sync/push.go delete mode 100644 sync/status.go delete mode 100644 sync/token.go delete mode 100644 sync/token_test.go diff --git a/CLAUDE.md b/CLAUDE.md index e99d2be..2debbac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,47 +6,25 @@ the browser page over that API, in Vue, and nothing in Go imports it. ## How to run and test -**A hand run is `direnv exec . go run .` from the repo root, against the dev -database. Do not build a throwaway binary to `/tmp` for one, and do not seed a -synthetic database for one.** A test fixture is allowed, and it needs at least one -long note. - -`.envrc` pairs a dedicated dev database with the sync API's local dev stack, so -nothing run here touches production data. This is the whole file: +**A hand run is `go run .` from the repo root, against the dev database, with the +machine's config kept out. Do not build a throwaway binary to `/tmp` for one, and +do not seed a synthetic database for one.** A test fixture is allowed, and it needs +at least one long note. ```bash -export PROJECTS_DB="$HOME/.local/share/projects/dev.db" -export PROJECTS_SYNC=true -export PROJECTS_SYNC_URL="https://api.docker.localhost" +XDG_CONFIG_HOME="$(mktemp -d)" PROJECTS_DB="$HOME/.local/share/projects/dev.db" go run . list +XDG_CONFIG_HOME="$(mktemp -d)" PROJECTS_DB="$HOME/.local/share/projects/dev.db" go run . tui ``` -The dev API needs no token, because its proxy injects the user header. The -server's own repo starts it, and the first sync binds `dev.db` to it. Its seed -data carries notes one line long. A change about notes needs a real-length -markdown note written into the dev API first. - -⚠️ **`.envrc` is machine-local and is not in this repo. Check it exists before -running anything here.** Without it nothing is overridden, so `go run . tui` opens -the production database against the production API — which is the accident the -coupled-variables rule below exists to prevent, arrived at from the other -direction. `task setup`, which `worktree new` runs, links the primary -checkout's `.envrc` into a worktree. - -**A shell without direnv's hook ignores `.envrc`, and Claude Code's tool shell is -one.** There, a bare `go run . list` reads production even with the file in place. -Load it explicitly: - -```bash -direnv exec . go run . tui # the board against dev.db -direnv exec . go run . list # a command against dev.db -direnv exec . go run . sync # one reconcile against the dev API -direnv exec . env | rg '^PROJECTS' # nothing printed: no .envrc here -``` +⚠️ **Name both, every time.** A desk's config file holds `api`, which puts every +command in API mode against the production API and ignores `PROJECTS_DB`. So +`PROJECTS_DB` alone writes to production. An empty `XDG_CONFIG_HOME` leaves no +`api`, and the run opens the database `PROJECTS_DB` names. -`.envrc` turns sync on, so a command that writes refuses there, naming `api`. Reads -answer from `dev.db`. A write is checked in the board, or through `both_modes_test.go`. -A config file that sets `api` beside the `PROJECTS_SYNC=true` from `.envrc` refuses -every command, naming both, so the pair cannot cross into production that way either. +`.envrc` is machine-local and is not in this repo. `task setup`, which `worktree +new` runs, links the primary checkout's copy into a worktree. It points +`PROJECTS_DB` at `dev.db` for `TestRealDatabaseSurvivesEverySizeAndProject`, and +keeps no run off the API the config file names. **The former name is handled in one file.** `config/former_name.go` refuses a retired variable that is still set, and a config still at the former path. It moves the old data directory on the first run @@ -58,6 +36,13 @@ marker there says when the file retires. Anything that opens the database stays `MoveDataUnderFormerName` in `openDatabase` in `cli/data_source.go`, because opening first creates an empty database, and the move refuses to run over one. +**The synced copy's leftovers are handled in one file too.** `config/retired_sync.go` refuses a set +`PROJECTS_SYNC*` or `TODOUI_SYNC*` variable with exit 2, because `PROJECTS_SYNC_URL` chose an API. +`Config.RetiredSync` keeps a `[sync]` section loading, and `warnOfRetiredSyncSection` in +`cli/data_source.go` warns of it rather than refusing, because a machine's config changes only at its +next apply. `dropSyncTables` in `store/sqlite/open.go` drops `pending_sync`, `sync_state` and +`sync_origin` on the first open. The file's `MIGRATION` marker says when it retires. + Build, vet and unit tests check code correctness, not TUI correctness. A TUI change is verified by driving the built binary, which the flows below do with nobody at the keyboard. @@ -80,8 +65,7 @@ go test -run TestTheCommandsAnswerTheSameInBothModes . # the commands, in both m ``` - **`seedFixtureDatabase` writes through `service.Service`**, the path the TUI writes through, so - the fixture holds only what the tool's own rules allow. There is no stand-in of the sync API, and sync - stays off because no `PROJECTS_SYNC` reaches the program. + the fixture holds only what the tool's own rules allow. - **`XDG_CONFIG_HOME` points under the test's temporary directory**, so the machine's own config file never reaches the program. - **The API is `routes.New` over a seeded SQLite database in an `httptest.Server`.** `serveOverHTTP` @@ -117,9 +101,9 @@ Ports and adapters at the storage seam, so a second store runs the same rules an ```text cli commands tui api/main.go │ apiclient.Client │ backend.Backend, which │ - │ │ sync.SyncBackend and │ - ▼ │ apiclient.Backend also ▼ -api/routes, in process │ satisfy api/routes + │ │ apiclient.Backend also │ + ▼ │ satisfies ▼ +api/routes, in process │ api/routes │ in database mode; │ │ *service.Service, scoped │ across HTTP to │ │ to each request by │ projects-api otherwise │ │ WithContext @@ -132,7 +116,7 @@ store/postgres the same, with goose migrations and an audit trigger ``` - **`store/sqlite` opens the database and holds no decision.** It owns the schema, the migrations, - `queries.sql` and sqlc's `generated`. It turns the driver's no-row answer, and each unique answer + `queries.sql` and sqlc's `internal/generated`, which nothing outside it can import. It turns the driver's no-row answer, and each unique answer `store/store.go` promises a sentinel for, into `model` sentinels, so `service` matches no driver text and imports no `database/sql`. - **`service.Service` satisfies `backend.Backend`**, so the TUI reaches every rule through that @@ -157,19 +141,12 @@ store/postgres the same, with goose migrations and an audit trigger `store/store.go` states each on its method: a new row's position, an item's `updated_at`, and a restore's handling of a row already there. Each store states them in its own `queries.sql`. -Two carve-outs are declared here rather than assumed: +One carve-out is declared here rather than assumed: -- **`sync` reaches `store/sqlite/generated` directly.** A pull rewrites rows wholesale, with no undo entry - and none of the rules, and the push queue and origin binding belong to no rule. Routing that through - `service` would give the rules a surface only the engine calls. A pull writes only the columns it has - always carried, so on a synced database `kind` and `completed_at` are what that database set, and neither - arrives from elsewhere. It ends when the sync engine is deleted. `generated` can then move under - `store/sqlite/internal`, where the compiler enforces the seam. - **Database mode links `service` and `api/routes` directly.** A client normally depends on the wire contract and never on a server's types, because importing them couples deploy order. Database mode serves itself, so that reason does not reach it. `apiclient` speaks `api/wire` and converts to - `model` for the board, and imports no route, service or store. The sync engine keeps its own lean - structs. + `model` for the board, and imports no route, service or store. ## `projects` is a command line, and `projects tui` is the board @@ -180,12 +157,6 @@ builds the tree. Help renders through `gotermstyle/cobrahelp`. - **The `api` key, or `--api`, chooses the mode, and neither means the database.** `decideMode` in `cli/data_source.go` reads goclilogin's parse of the key, so the mode and the login read one setting. It runs in each command's `RunE`, so `help`, `completion` and `version` open nothing. -- **`api` beside sync refuses**, through `chooseMode`, the one check every command, `projects sync` - and `auth login` take. The two are two authorities for the same rows. The refusal names - `PROJECTS_SYNC` when the environment turned sync on, and the config file's `[sync]` otherwise. -- **With sync on, a command that writes refuses before it asks anything.** `openForWriting` holds the - sentence, and `refusingWrites` answers 409 with it for any write that reaches the in-process routes - without it. A write there would land in the copy with nothing queued, and the next pull deletes it. - **Exit codes are 0, 1 for refused or failed, 2 for usage, and 4 for no answer.** `exitCodeFor` in `cli/root.go` maps them, and `explain` in `cli/api_errors.go` words them. A 422 is usage. A refused credential is 1. @@ -209,17 +180,13 @@ builds the tree. Help renders through `gotermstyle/cobrahelp`. an API serves tags written from every machine. `repoRegistry` on the data source answers an empty registry in API mode. -An item shows its **number**, the handle the API assigns and answers with. The UUID is the sync key +An item shows its **number**, the handle the API assigns and answers with. The UUID is the key and stops there. -The number is assigned by whichever database is the authority. Through an API, the API numbers it as -it creates it. With sync on that is the API, and -`pushCreatedItem` writes the number from the create response into the local row, so the TUI shows -it once the push lands rather than after the next pull. With sync off, `AssigningItemNumbers` makes -`projects` allocate `max+1` itself, which cannot collide because that database is a disjoint universe. - -An item created while the API is unreachable has no number until its push lands, and shows its UUID -tail until then (`itemHandle` in `tui/app.go`). +`AssigningItemNumbers` numbers each item as it is created. Through an API, the API numbers it, from +the `item_numbers` sequence. In database mode `projects` allocates `max+1` itself. An item with no +number, which only a database from before numbers or from the synced copy holds, shows its UUID tail +(`itemHandle` in `tui/app.go`). ## A project name is bounded work, never a repo @@ -284,75 +251,8 @@ adding columns, because SQLite attaches the old column-level `UNIQUE(name)` as an implicit index no DDL can drop. Foreign keys are off across the swap: `project_item_memberships` cascades off `projects`, so dropping the old table with them on would take every membership. Do not simplify it into three -`ALTER TABLE ADD COLUMN`s — leaving the global constraint in place fails the -entire pull transaction the first time the server returns a done project beside -a live one sharing its name. - -## A database and its API are a pair — never mix a dev half with a prod half - -`.envrc` exports two coupled variables, `PROJECTS_DB` and `PROJECTS_SYNC_URL`, and -`PROJECTS_SYNC_TOKEN_COMMAND` where the dev API wants a token. They name one -environment. A URL from the environment never takes the config file's -`token_command` (`config.Load`), so the dev URL cannot carry the production -token. Overriding one of them names an environment that does not exist, and -for this tool that is destructive rather than merely wrong: a pull deletes -every local row the server did not return, so -a production database reconciled against the dev API is emptied of everything -dev has never heard of. A sync run that way once deleted 42 projects and 286 -items and printed `Synced.` - -**To run against production data, run from outside this repo**, where direnv has -loaded nothing. Un-setting your way out is how the accident happened: `env -u` -on the variable you remembered still leaves the one you did not. - -`sync_origin` is the enforcement, because a rule only prose enforces is -decoration. The first pull records which API the database belongs to, and every -pull and push after that refuses any other (`OriginMismatch`). `--adopt` is the -only way the pairing changes. A database that has pulled before but carries no -origin — anything predating this — refuses to guess and asks to be bound, since -inferring it from whichever environment is loaded *is* the mistake. - -`guardSweep` is the second layer, for when the API is right but its answer is -not. A pull that would delete ten or more local projects, and more than half of -them, is refused, and items are guarded the same way. A truncated response, an -auth failure that still returns 200, and a genuine mass deletion are the same -event seen from here. `--force` allows it. The floor counts rows deleted, not -rows held, so 8 deletions out of 15 pass. It keeps the guard silent on small -databases, where losing most of it is a handful of rows; a guard that fires on -ordinary days gets forced past by reflex and stops guarding anything. - -Neither override is reachable from the background timers. A guard something -automatic can waive is not a guard. - -## Sync is automatic; `projects sync` is a convenience, not a requirement - -`sync.interval` (default 2m, floored at 15s) is the single knob for how stale `projects` may ever be, -and it drives two loops: - -- **TUI** — `syncPullTickMsg` re-arms itself every interval, and always reschedules, even on the - ticks it skips: a tick that returns no command kills background sync for the session. - `safeToAutoPull` gates the reconcile to `modeNormal`, because a pull rewrites items, memberships - and ordering wholesale and would move the ground under a grab or a text entry. -- **Push** — `pushLoop` carries a retry ticker alongside `Notify`, which fires only on a local - mutation, so a push that failed while the API was down retries without another edit. - -An automatic pull neither flashes on success nor claims the status bar on failure; failure surfaces -through the engine status (`SYNC ERR`). Local-first means an unreachable API degrades to local -data, never an error. - -`Pull` records a pending-sync high-water mark before it fetches, then clears only the ops at or -below it and spares entities queued above it from the "deleted upstream" sweeps. Without that mark, -an item typed mid-pull is deleted by the same pull and its queued create dropped with it. Do not -restore `DeleteAllPendingSync` here. - -**Absent is not empty, and that is what keeps deploy order between this repo and its server -irrelevant.** The item list embeds memberships, dependencies and tasks; when the server omits -`dependency_ids`/`tasks`, the pull falls back to the per-item endpoints — decode into slices and -branch on nil, never on empty, or the first pull against an older API deletes every local task and -dependency. The project pull asks for `?status=all` because the sweep deletes any local project the -server did not return, so an active-only response would read "completed" as "deleted" and cascade -through the memberships. A server sending no project status is read as `active`. Do not drop either -fallback just because the API has shipped. +`ALTER TABLE ADD COLUMN`s — leaving the global constraint in place refuses a +done project beside a live one sharing its name. ## The API is `service` over Postgres, and a handler holds no rule diff --git a/README.md b/README.md index 4509f85..5402c04 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,7 @@ empty database at the old path, and `projects` then stops until that file is gon Move `~/.config/todoui/config.toml` to `~/.config/projects/config.toml` first. While only the old file exists, every run that opens a database stops with exit 1, naming both paths. Reading no config would -turn sync off, and the next pull deletes what was written with it off. +drop its `api` and send every command to a database on this machine. The first run that opens the default database moves everything in `$XDG_DATA_HOME/todoui` to `$XDG_DATA_HOME/projects`. `todoui.db` becomes `projects.db`, and its `-wal` and `-shm` files go with @@ -169,7 +169,7 @@ j / k move through items and their tasks as one list J / K jump item to Tab switch pane Enter read it in full a add a project, or an item to this project space mark done T dependency browser / search every project -u undo the last change R sync, or read the API, now +u undo the last change R read the data again now ? every key the pane you are in takes q quit ``` @@ -183,7 +183,7 @@ items it waits on. The reader also names the repo the item is for. The board has that tag. `projects items edit --repo ` sets it. Through an API, the status bar reads `API` and the API's host. The board reads the API again every -`sync.interval`, and `R` reads it at once. +two minutes, and `R` reads it at once. `Enter` opens the reader, a full screen holding one item or one project. An item shows its fields, what blocks it, what it blocks, its tasks, and its notes rendered as markdown. A project shows its @@ -247,70 +247,29 @@ next round of them. ## An item's handle is its number, and its key is a UUID An item shows its number, the same handle the API answers with. The number is not the primary key. -`projects` creates every id as a UUIDv7, so an item can be created offline and pushed without a round -trip to claim one. +`projects` creates every id as a UUIDv7. -Through an API, the API numbers the item as it creates it. With sync on, the number comes from the -API's response to the create, and the TUI shows it once the push lands. In database mode with sync -off, `projects` allocates the next number itself. An item created while a synced API is unreachable -shows the last 8 characters of its id until its push lands and earns it a number. +Through an API, the API numbers the item as it creates it. In database mode, `projects` allocates the +next number itself. An item with no number shows the last 8 characters of its id. Only a database +from before numbers existed, or one kept as a synced copy of an API, can hold such an item. -## Sync keeps a database a copy of an API, and only the board writes to the copy +## A config or database from the synced copy still works, and its sync variables refuse -`[sync] enabled = true` keeps a database on this machine as a -copy of `sync.api_url`, and `projects tui` edits the copy and pushes each edit. With `api` set there -is no copy, so a config file holding both `api` and `[sync] enabled = true` stops every command, -naming both keys. +`projects` once kept a database as a copy of an API, set under `[sync]`. It keeps no copy now, and +`api` is how a machine works through an API. -With sync on, every read on the command line answers from the copy. A command that writes refuses -with a sentence naming `api`. Its write would land in the copy with nothing queued to push, and the -next pull would delete it. `projects sync` refuses when `api` is set, because there is nothing to -reconcile. - -With sync on, every edit in the board is queued and pushed as you make it. The TUI pulls on launch, then again -every `sync.interval` (default two minutes) for as long as it is open. A pull waits while a text -input, an overlay, the reader or a move is open, and runs on the next tick instead. - -Queued pushes retry on a timer. A change made while the API was unreachable is sent once it comes -back, with no further edit to trigger it. - -The status bar names the API's URL beside one of `SYNCING...`, `PENDING (n)`, `SYNC ERR` or -`SYNCED`. An unreachable API shows `SYNC ERR` and leaves the local data in place. `R` in the TUI or -`projects sync` forces a reconcile. - -A pull is two requests, because the item list embeds memberships, dependencies and sub-tasks. -Against an API that does not embed them, it falls back to two more requests per item. Lower -`interval` to sync more eagerly. It is floored at 15 seconds. - -## A database belongs to one API, and a pull that would empty it stops - -```bash -projects sync # push anything queued, then pull, however recently the last pull ran -projects sync --adopt # bind this database to the configured API, replacing the old binding -projects sync --force # allow a pull that would delete most of the projects or items here -``` - -A pull deletes every local row the server did not return, so a database and its API are a pair. -The first pull records which API the database belongs to. Every pull and push after that refuses any -other. A database pointed at the wrong API would lose everything that API has never heard of, and -the reconcile cannot tell that from a real deletion. - -A database that has pulled before but records no API refuses as well. It asks for `--adopt` rather -than guessing from whatever is configured, because guessing from the loaded configuration is the -mistake the binding exists to stop. - -The second guard is for the right API giving a wrong answer. A pull that would delete ten or more -projects, and more than half of them, stops and asks for `--force`. Items are guarded the same way. -A truncated response, an authentication failure that still returns `200`, and a genuine mass -deletion look identical from the client. The floor keeps the guard quiet on a small database, where -losing most of it is a handful of rows. - -Neither flag is reachable from the background timers. +- A config file still holding `[sync]` loads. Every command that opens data warns on stderr, naming + the file, and ignores the section. +- A database from then opens as it is. The first open drops its queue of pushes, its pull times and + the API it was bound to. Every project, item, task, dependency and membership stays. +- `PROJECTS_SYNC`, `PROJECTS_SYNC_URL`, `PROJECTS_SYNC_TOKEN_COMMAND` or `PROJECTS_SYNC_INTERVAL`, or + its `TODOUI_` twin, set to a value stops every command with exit 2. `PROJECTS_SYNC_URL` chose an + API, so ignoring it would send the run wherever the config file points. ## Configuration — one TOML file, every key overridable from the environment `projects` reads `$XDG_CONFIG_HOME/projects/config.toml`, else `~/.config/projects/config.toml`, on macOS -as well as Linux. No config file is a valid configuration: a local database, with sync off. +as well as Linux. No config file is a valid configuration: a local database. [config.example.toml](config.example.toml) is an annotated copy. ```toml @@ -324,43 +283,16 @@ as well as Linux. No config file is a valid configuration: a local database, wit [local] # db_path = "/custom/path/projects.db" # default: ~/.local/share/projects/projects.db - -[sync] -enabled = false -# api_url = "https://example.com" # the server's host; the tool adds /api/v1 -# token_command = " auth token" # prints a bearer token for api_url -# interval = "2m" # longest it goes without reconciling; floored at 15s ``` -`enabled = true` without an `api_url` is an error. So is `enabled = true` beside `api`. - -The bare `token_command` belongs to `api`, and is an argument list printing a token response. The -`[sync]` one belongs to `api_url`, and is a string printing a bare token. The two never stand in for -each other, and either written in the other's shape is refused, naming the shape it takes. - -`[sync]`'s `token_command` is how `projects` authenticates to the API without holding a secret itself. It is split -on spaces and run without a shell or stdin. Whatever it prints becomes the bearer token. A command -that has not finished in 10 seconds fails. `projects` keeps the token until the API refuses it, then -runs the command once more and retries once. A failing command's first line of stderr is what the -status bar shows. Leave it unset for an API that needs no token, such as a dev stack that sets its -own identity. - -The command belongs to the URL beside it. When `PROJECTS_SYNC_URL` supplies the URL, the config -file's `token_command` is ignored and `PROJECTS_SYNC_TOKEN_COMMAND` is the only source. Overriding the -URL never sends one API's token to another. - Each variable overrides the key beside it: | Variable | Overrides | | -------- | --------- | | `PROJECTS_DB` | `local.db_path` | -| `PROJECTS_SYNC` | `sync.enabled` (`true` or `false`) | -| `PROJECTS_SYNC_URL` | `sync.api_url` | -| `PROJECTS_SYNC_TOKEN_COMMAND` | `sync.token_command` | -| `PROJECTS_SYNC_INTERVAL` | `sync.interval`, as a duration such as `30s` or `5m` | | `PROJECTS_REPOS_REGISTRY` | `repos_registry` | -`api`, `issuer`, `client_id` and the bare `token_command` take no variable. `--api` outranks `api` for +`api`, `issuer`, `client_id` and `token_command` take no variable. `--api` outranks `api` for one run. `PROJECTS_CLIENT_SECRET` switches the login to the client-credentials grant. ## Every command speaks HTTP to the API's routes, served in the process or across the network @@ -375,8 +307,8 @@ projects projects tui v v apiclient.Client backend.Backend | | - +-- database mode +-- database mode: service.Service, - | InProcess -> api/routes | in SyncBackend when sync is on + +-- database mode +-- database mode: service.Service + | InProcess -> api/routes | | -> service.Service | | -> store/sqlite +-- api: apiclient.Backend | -> apiclient.Client @@ -384,14 +316,10 @@ apiclient.Client backend.Backend -> projects-api ``` -The sync engine, when it is on, sits in the board's path alone. It pushes each queued write over -HTTP and pulls the API's state into the local database. - The data model is projects, items, the memberships joining them many to many, dependencies between items, and sub-tasks. -In database mode, the database at `db_path` holds the data, the queue of pending pushes, and the API -the database is bound to. The update check keeps its last result under `$XDG_STATE_HOME/projects/`, +In database mode, the database at `db_path` holds the data and the undo log. The update check keeps its last result under `$XDG_STATE_HOME/projects/`, which defaults to `~/.local/state/projects/`. `auth login` is the only command that writes the config file, and it adds its keys without rewriting the rest. The repo registry is read and never written. diff --git a/backend/backend.go b/backend/backend.go index 31518c5..2367cd2 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -2,8 +2,8 @@ package backend import "github.com/datapointchris/projects/model" -// Backend defines the interface for all data operations. -// service.Service holds the rules over a store; SyncBackend wraps it with background push/pull. +// Backend defines the interface for all data operations. service.Service holds +// the rules over a store, and apiclient.Backend reaches them through the API. type Backend interface { // Projects // ListProjectsByStatus takes a status name or model.StatusAll. diff --git a/cli/auth.go b/cli/auth.go index 340c85c..6404228 100644 --- a/cli/auth.go +++ b/cli/auth.go @@ -1,7 +1,6 @@ package cli import ( - "cmp" "context" "errors" "fmt" @@ -14,8 +13,6 @@ import ( "github.com/pkg/browser" "github.com/spf13/cobra" "golang.org/x/oauth2" - - "github.com/datapointchris/projects/config" ) // loginTimeout is a backstop. The device code carries its own expiry, and the @@ -76,9 +73,6 @@ func newAuthLoginCommand() *cobra.Command { } url = args[0] } - if err := refuseLoginBesideSync(url); err != nil { - return err - } ctx, cancel := context.WithTimeout(cmd.Context(), loginTimeout) defer cancel() @@ -130,18 +124,6 @@ func newAuthLoginCommand() *cobra.Command { return cmd } -// refuseLoginBesideSync refuses a login that would write api beside a synced -// database, because every command after it would refuse the pair. -func refuseLoginBesideSync(url string) error { - cfg, err := config.Load() - if err != nil { - return err - } - _, err = chooseMode(cfg, cmp.Or(url, "the API it logs in to"), - "logging in writes api to "+homeAsTilde(configFilePath())) - return err -} - func newAuthLogoutCommand() *cobra.Command { return &cobra.Command{ Use: "logout", diff --git a/cli/data_source.go b/cli/data_source.go index 26fd23a..d0a7954 100644 --- a/cli/data_source.go +++ b/cli/data_source.go @@ -3,7 +3,6 @@ package cli import ( "context" "database/sql" - "encoding/json" "errors" "fmt" "io" @@ -21,14 +20,12 @@ import ( "github.com/datapointchris/projects/api/requestlog" "github.com/datapointchris/projects/api/routes" - "github.com/datapointchris/projects/api/wire" "github.com/datapointchris/projects/apiclient" "github.com/datapointchris/projects/backend" "github.com/datapointchris/projects/config" "github.com/datapointchris/projects/repos" "github.com/datapointchris/projects/service" "github.com/datapointchris/projects/store/sqlite" - "github.com/datapointchris/projects/sync" ) // inProcessBase is the address the client is given when the routes run inside @@ -58,11 +55,6 @@ type dataSource struct { // Database mode. database *sql.DB local *service.Service - engine *sync.Engine - started bool - - // writeRefusal is set where a write through the routes would be lost. - writeRefusal string } // LocalService is the service over a database this machine owns outright, which @@ -81,7 +73,7 @@ const ( // openDataSource opens where this run's data lives, as decideMode answers it. func openDataSource(cmd *cobra.Command) (*dataSource, error) { - cfg, deployment, mode, err := decideMode(cmd.Context()) + cfg, deployment, mode, err := decideMode(cmd) if err != nil { return nil, err } @@ -92,9 +84,10 @@ func openDataSource(cmd *cobra.Command) (*dataSource, error) { } // decideMode reads the config file and the login settings in it, and answers -// which store this run uses. goclilogin's reading of api is the one the mode -// follows, because it is the one every request is then sent with. -func decideMode(ctx context.Context) (*config.Config, goclilogin.Deployment, runMode, error) { +// which store this run uses: the API api names, or this machine's database when +// it names none. goclilogin's reading of api is the one the mode follows, because +// it is the one every request is then sent with. +func decideMode(cmd *cobra.Command) (*config.Config, goclilogin.Deployment, runMode, error) { if err := config.RefuseConfigUnderFormerName(); err != nil { return nil, goclilogin.Deployment{}, databaseMode, err } @@ -102,10 +95,11 @@ func decideMode(ctx context.Context) (*config.Config, goclilogin.Deployment, run if err != nil { return nil, goclilogin.Deployment{}, databaseMode, err } - deployment, loadErr := loadDeployment(ctx, apiOverride) - mode, err := chooseMode(cfg, deployment.API.Value, apiPhrase(deployment.API)) - if err != nil { - return nil, deployment, mode, err + warnOfRetiredSyncSection(cmd, cfg) + deployment, loadErr := loadDeployment(cmd.Context(), apiOverride) + mode := databaseMode + if deployment.API.Value != "" { + mode = apiMode } // With no api, the deployment's only error worth stopping for is a file it // could not read: a broken api line must not fall back to the database. @@ -115,52 +109,16 @@ func decideMode(ctx context.Context) (*config.Config, goclilogin.Deployment, run return cfg, deployment, mode, nil } -// chooseMode is the one answer to where a run's data lives: the API api names, or -// this machine's database when it names none. A synced database beside an API is -// refused, because the two are authorities for the same rows. apiPhrase says -// what named the API. -func chooseMode(cfg *config.Config, api, apiPhrase string) (runMode, error) { - if api == "" { - return databaseMode, nil +// warnOfRetiredSyncSection names a [sync] section the run ignores. It warns rather +// than refuses, because a machine's config is rewritten only by its next apply, +// and a refusal would stop the tool there until then. +func warnOfRetiredSyncSection(cmd *cobra.Command, cfg *config.Config) { + if cfg.RetiredSync == nil { + return } - if cfg.Sync.Enabled { - chosenBy, remedy := syncChosenBy() - return apiMode, fmt.Errorf("%s, and %s keeps a synced database, which are two authorities for the same rows; "+ - "%s to work through the API", apiPhrase, chosenBy, remedy) - } - return apiMode, nil -} - -// apiPhrase names what chose the API, as the first thing a person checks. -func apiPhrase(api goclilogin.Setting) string { - if api.Source == goclilogin.SourceCommandLine { - return "--api sends this command to " + api.Value - } - return fmt.Sprintf("api in %s sends every command to %s", homeAsTilde(api.From), api.Value) -} - -// syncChosenBy names what turned sync on and how to turn it off. The variable -// outranks the file, so it is named whenever it is set. -func syncChosenBy() (chosenBy, remedy string) { - if value := os.Getenv(config.SyncVariable); value != "" { - return fmt.Sprintf("%s=%s in the environment", config.SyncVariable, value), "unset " + config.SyncVariable - } - path := homeAsTilde(configFilePath()) - return "[sync] enabled = true in " + path, "remove [sync] from " + path -} - -// openForWriting is openDataSource for a command that writes, refused before it -// asks anything where the write would be lost. -func openForWriting(cmd *cobra.Command) (*dataSource, error) { - source, err := openDataSource(cmd) - if err != nil { - return nil, err - } - if source.writeRefusal != "" { - _ = source.Close() - return nil, errors.New(source.writeRefusal) - } - return source, nil + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "warning: [sync] in %s is ignored, because projects keeps no synced copy "+ + "of an API. Remove the section. `projects auth login ` writes api, which sends every command to an API.\n", + homeAsTilde(configFilePath())) } func openAPI(ctx context.Context, cfg *config.Config, deployment goclilogin.Deployment) (*dataSource, error) { @@ -231,63 +189,24 @@ func openDatabase(cfg *config.Config) (*dataSource, error) { if err != nil { return nil, fmt.Errorf("opening database: %w", err) } - source := &dataSource{cfg: cfg, database: database} - if cfg.Sync.Enabled { - source.local = service.New(sqlite.New(database), service.RefusingRepoNames(repos.DefaultPath())) - source.engine = sync.New(database, cfg.Sync.APIURL, cfg.Sync.TokenCommand) - chosenBy, remedy := syncChosenBy() - source.writeRefusal = fmt.Sprintf("%s keeps this database a copy of %s, and a write from a command would "+ - "never reach it; make the change in `projects tui`, or %s and set api in %s to write through the projects API", - chosenBy, cfg.Sync.APIURL, remedy, homeAsTilde(configFilePath())) - } else { - source.local = LocalService(database, repos.DefaultPath()) - } + source := &dataSource{cfg: cfg, database: database, local: LocalService(database, repos.DefaultPath())} // A fault in a route is logged with its cause, which the answer only names by // request id. The request line itself is for a server's log, not a terminal. faults := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) quiet := slog.New(slog.NewTextHandler(io.Discard, nil)) handler := requestlog.Middleware(quiet, routes.New(source.local, faults)) - if source.writeRefusal != "" { - handler = refusingWrites(handler, source.writeRefusal) - } source.client = apiclient.New(inProcessBase, &http.Client{Transport: apiclient.InProcess(handler)}) return source, nil } -// refusingWrites answers every request but a read with the sentence saying why. -// It backs up openForWriting for any write a command reaches without it. -func refusingWrites(next http.Handler, sentence string) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet { - next.ServeHTTP(w, r) - return - } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusConflict) - _ = json.NewEncoder(w).Encode(wire.Error{Error: http.StatusText(http.StatusConflict), Message: sentence}) - }) -} - -// startSync starts the background push loop, for the commands that sync. -func (s *dataSource) startSync() { - if s.engine != nil && !s.started { - s.engine.Start() - s.started = true - } -} - // boardBackend is what the TUI reads and writes: the API over the client, or the -// database's own service, wrapped to queue each write when sync is on. +// database's own service. func (s *dataSource) boardBackend() backend.Backend { - switch { - case s.deployment != nil: + if s.deployment != nil { return apiclient.NewBackend(s.client) - case s.engine != nil: - return sync.NewSyncBackend(s.local, s.engine) - default: - return s.local } + return s.local } // repoRegistry is what a repo tag is checked against: this machine's registry in @@ -312,9 +231,6 @@ func (s *dataSource) apiHost() string { } func (s *dataSource) Close() error { - if s.engine != nil && s.started { - s.engine.Stop() - } if s.database != nil { return s.database.Close() } diff --git a/cli/item_create.go b/cli/item_create.go index d567421..2841a3c 100644 --- a/cli/item_create.go +++ b/cli/item_create.go @@ -49,7 +49,7 @@ func newItemsCreateCommand() *cobra.Command { return goclikit.UsageError(fmt.Errorf("%s required — pass %s, or run from a terminal to be asked", strings.Join(missing, " and "), pronoun)) } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } diff --git a/cli/item_dependencies.go b/cli/item_dependencies.go index b4fe0e1..bb16f91 100644 --- a/cli/item_dependencies.go +++ b/cli/item_dependencies.go @@ -48,7 +48,7 @@ func newDependenciesAddCommand() *cobra.Command { if dependsOn == "" { return goclikit.UsageError(errors.New("--depends-on is required")) } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -83,7 +83,7 @@ func newDependenciesRemoveCommand() *cobra.Command { if dependsOn == "" { return goclikit.UsageError(errors.New("--depends-on is required")) } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -316,8 +316,7 @@ func crossProjectTag(item, parent wire.ProjectItem, hasParent bool) string { // drawing, so a consumer wanting another shape parses no box-drawing. // cli/schemas/dependency-tree.schema.json is its shape. An edge runs from an item // to what it depends on whichever way the drawing ran, and only roots follows -// --invert. Edges and roots name items by id, because an item made on a synced -// database has no number until its create reaches the API. +// --invert. Edges and roots name items by id, because an item may have no number. type treeShown struct { SchemaVersion int `json:"schema_version"` Nodes []treeNode `json:"nodes"` diff --git a/cli/item_memberships.go b/cli/item_memberships.go index 7287fa0..f4c0fc4 100644 --- a/cli/item_memberships.go +++ b/cli/item_memberships.go @@ -36,7 +36,7 @@ func newMembershipsAddCommand() *cobra.Command { if project == "" { return goclikit.UsageError(errors.New("--project is required")) } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -71,7 +71,7 @@ func newMembershipsRemoveCommand() *cobra.Command { if project == "" { return goclikit.UsageError(errors.New("--project is required")) } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } diff --git a/cli/item_tasks.go b/cli/item_tasks.go index ece2b78..cb20ba6 100644 --- a/cli/item_tasks.go +++ b/cli/item_tasks.go @@ -78,7 +78,7 @@ func newTasksAddCommand() *cobra.Command { if cmd.Flags().Changed("position") { input.Position = &position } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -154,7 +154,7 @@ func newTasksEditCommand() *cobra.Command { } func runTaskUpdate(cmd *cobra.Command, itemRef, taskID string, edit apiclient.TaskChanges, asJSON bool) error { - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -178,7 +178,7 @@ func newTasksRemoveCommand() *cobra.Command { Example: " projects items tasks remove 118 019a2b3c-… --yes", Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } diff --git a/cli/items.go b/cli/items.go index a5ee85a..3427913 100644 --- a/cli/items.go +++ b/cli/items.go @@ -328,7 +328,7 @@ func newItemsEditCommand() *cobra.Command { if edit == (apiclient.ItemChanges{}) { return goclikit.UsageError(errors.New("nothing to change — pass --title, --notes or --repo")) } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -372,7 +372,7 @@ func newItemStateCommand(verb, short, done string, edit apiclient.ItemChanges) * } func runItemUpdate(cmd *cobra.Command, ref string, edit apiclient.ItemChanges, asJSON bool, done string) error { - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -401,7 +401,7 @@ func newItemsDeleteCommand() *cobra.Command { Example: " projects items delete 118 --yes", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -449,7 +449,7 @@ func newItemsReorderCommand() *cobra.Command { if !cmd.Flags().Changed("position") { return goclikit.UsageError(errors.New("--position is required")) } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } diff --git a/cli/projects.go b/cli/projects.go index c602bc1..8bc6d16 100644 --- a/cli/projects.go +++ b/cli/projects.go @@ -168,7 +168,7 @@ func newProjectsCreateCommand() *cobra.Command { if cmd.Flags().Changed("position") { input.Position = &position } - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -332,7 +332,7 @@ func newProjectStatusCommand(verb statusVerb) *cobra.Command { } func runProjectUpdate(cmd *cobra.Command, ref string, edit apiclient.ProjectChanges, asJSON bool, done string) error { - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } @@ -359,7 +359,7 @@ func newProjectsDeleteCommand() *cobra.Command { Example: " projects delete \"Kitchen remodel\"\n projects delete \"Kitchen remodel\" --yes", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - source, err := openForWriting(cmd) + source, err := openDataSource(cmd) if err != nil { return err } diff --git a/cli/root.go b/cli/root.go index 670498a..a5814ee 100644 --- a/cli/root.go +++ b/cli/root.go @@ -137,7 +137,7 @@ func NewRootCommand() *cobra.Command { )...) root.AddCommand(inGroup(groupItems, withAPIFlag(newItemsCommand()))...) root.AddCommand(inGroup(groupScreen, withAPIFlag(newTUICommand()))...) - root.AddCommand(inGroup(groupSetup, newAuthCommand(), newSyncCommand(), VersionCmd(), UpdateCmd())...) + root.AddCommand(inGroup(groupSetup, newAuthCommand(), VersionCmd(), UpdateCmd())...) return root } diff --git a/cli/schemas/dependency-tree.schema.json b/cli/schemas/dependency-tree.schema.json index 5399199..cd5ada3 100644 --- a/cli/schemas/dependency-tree.schema.json +++ b/cli/schemas/dependency-tree.schema.json @@ -35,7 +35,7 @@ "id": { "type": "string", "format": "uuid" }, "number": { "type": ["integer", "null"], - "description": "The item's handle. Null only on a synced database whose API has not assigned one yet." + "description": "The item's handle. Null on an item from before numbers existed, or one a database kept as a copy of an API created before the API numbered it." }, "title": { "type": "string" }, "status": { "type": "string", "enum": ["open", "completed", "archived"] }, diff --git a/cli/sync.go b/cli/sync.go deleted file mode 100644 index 6f5ac02..0000000 --- a/cli/sync.go +++ /dev/null @@ -1,67 +0,0 @@ -package cli - -import ( - "errors" - "fmt" - - "github.com/spf13/cobra" - - "github.com/datapointchris/projects/sync" -) - -func newSyncCommand() *cobra.Command { - var adoptOrigin, forceSweep bool - cmd := &cobra.Command{ - Use: "sync", - Short: "Reconcile a synced database with its API now", - Long: "For a database with [sync] enabled = true in the config file. `projects tui` on\n" + - "such a database pushes each edit as it is made and pulls on every interval, so\n" + - "this is for when the API's state is wanted now: it pushes anything queued, then\n" + - "pulls.\n" + - "\n" + - "A pull deletes everything the server did not return, so the database is bound to\n" + - "the API it was first pulled from and refuses to reconcile against any other.\n" + - "--adopt rebinds it; --force allows a pull that would delete most of what is here.\n" + - "\n" + - "With api in the config file there is no copy to reconcile, and this refuses.", - Example: " projects sync push queued edits, then pull from the API now\n" + - " projects sync --adopt bind this database to the configured API, then sync\n" + - " projects sync --force sync even when the pull would delete most local rows", - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - cfg, deployment, mode, err := decideMode(cmd.Context()) - if err != nil { - return err - } - if mode == apiMode { - return fmt.Errorf("%s, so there is no database copy to sync", apiPhrase(deployment.API)) - } - source, err := openDatabase(cfg) - if err != nil { - return err - } - defer func() { _ = source.Close() }() - if source.engine == nil { - return errors.New("sync is disabled — enable it under [sync] in " + homeAsTilde(configFilePath())) - } - // Named on the way in: --adopt rewrites which API owns this database, - // and doing that silently is the mistake the binding exists to prevent. - if adoptOrigin { - _, _ = fmt.Fprintf(cmd.OutOrStdout(), "Binding %s to %s\n", source.cfg.Local.DBPath, source.engine.APIURL()) - } - if err := source.engine.PullWith(cmd.Context(), sync.PullOptions{ - Adopt: adoptOrigin, - AllowLargeSweep: forceSweep, - }); err != nil { - return err - } - _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Synced.") - return nil - }, - } - cmd.Flags().BoolVar(&adoptOrigin, "adopt", false, - "bind this database to the configured API, replacing whatever it was bound to") - cmd.Flags().BoolVar(&forceSweep, "force", false, - "allow a pull that would delete most of the local projects or items") - return cmd -} diff --git a/cli/tables.go b/cli/tables.go index ef1bd59..0622059 100644 --- a/cli/tables.go +++ b/cli/tables.go @@ -171,8 +171,7 @@ func statusMark(status string) string { } } -// handle is an item's number, or the tail of its id while it has none: an item -// made on a synced database before its create reached the API. +// handle is an item's number, or the tail of its id where it has none. func handle(number *int, id string) string { if number != nil { return strconv.Itoa(*number) diff --git a/cli/tui.go b/cli/tui.go index 2c36e0a..cbe8485 100644 --- a/cli/tui.go +++ b/cli/tui.go @@ -46,9 +46,9 @@ func newTUICommand() *cobra.Command { Use: "tui", Short: "Open the board: projects on the left, the selected one's items on the right", Long: "Everything the commands do can be done here by key; ? lists the keys. The status\n" + - "line names where the data lives: LOCAL and the database's path, API and its host,\n" + - "or the sync state of a synced database. In API mode the board reads the API again\n" + - "every [sync] interval, and R reads it now.\n" + + "line names where the data lives: LOCAL and the database's path, or API and its\n" + + "host. In API mode the board reads the API again every two minutes, and R reads\n" + + "it now.\n" + "\n" + "--once prints one frame at COLUMNS by LINES, else at the terminal's size, else at\n" + "120 by 40, and --json what that frame is drawn from, so an agent reads what a\n" + @@ -74,10 +74,7 @@ func newTUICommand() *cobra.Command { } defer func() { _ = source.Close() }() where := tui.Where{DatabasePath: source.cfg.Local.DBPath, APIHost: source.apiHost()} - if !printing { - source.startSync() - } - app := tui.NewApp(source.boardBackend(), source.engine, where, source.cfg.Sync.Interval) + app := tui.NewApp(source.boardBackend(), where) if !printing { _, err := tea.NewProgram(app).Run() return err diff --git a/config.example.toml b/config.example.toml index d4f43b3..05ce4ce 100644 --- a/config.example.toml +++ b/config.example.toml @@ -1,7 +1,7 @@ # projects configuration # Place at ~/.config/projects/config.toml -# The keys under [local] and [sync], and repos_registry, can be overridden with -# environment variables. Bare keys must stay above the first [section]. +# db_path and repos_registry can be overridden with environment variables. Bare +# keys must stay above the first [section]. # Repo registry, whose names a project may not take in database mode. # Set this when the registry is maintained outside the tool's data directory and @@ -22,26 +22,6 @@ # Set, no login runs. For an API that needs no real token, print a stand-in. # token_command = ["printf", '{"access_token":"dev","token_type":"Bearer"}'] -[sync] -# Keep the database a copy of an API, for the board. Refused beside api. -# With it on, a command that writes refuses: make the change in `projects tui`. -# Env: PROJECTS_SYNC -enabled = false -# The API server's host, required when enabled. The tool adds /api/v1 itself. -# Env: PROJECTS_SYNC_URL -# api_url = "https://example.com" -# Command printing a bearer token for api_url, split on spaces and run without a -# shell. Unset sends no token. A URL from PROJECTS_SYNC_URL takes its command from -# PROJECTS_SYNC_TOKEN_COMMAND alone, never from this file. -# Env: PROJECTS_SYNC_TOKEN_COMMAND -# token_command = " auth token" -# Longest the tool goes without reconciling: the TUI's background pull period. -# Through api, how often the board reads the API again. -# A duration such as "30s" or "5m". Anything below 15s is raised to 15s. -# Env: PROJECTS_SYNC_INTERVAL -# Default: 2m -# interval = "2m" - [local] # Path to SQLite database # Env: PROJECTS_DB diff --git a/config/config.go b/config/config.go index 43b12ce..fc6eebb 100644 --- a/config/config.go +++ b/config/config.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "strings" - "time" "github.com/go-viper/mapstructure/v2" "github.com/spf13/viper" @@ -14,7 +13,10 @@ import ( // Config holds the application configuration loaded from TOML and env vars. type Config struct { Local LocalConfig `mapstructure:"local"` - Sync SyncConfig `mapstructure:"sync"` + + // RetiredSync holds a [sync] section, which the command layer warns is ignored. + // It is declared because ErrorUnused refuses a key the struct has no home for. + RetiredSync map[string]any `mapstructure:"sync"` // ReposRegistry names a registry maintained outside the tool's data directory, // which is how a machine sharing one between tools declares it. Empty, or no @@ -34,22 +36,6 @@ type Config struct { TokenCommand []string `mapstructure:"token_command"` } -// SyncConfig holds settings for background sync with the remote API. -type SyncConfig struct { - Enabled bool `mapstructure:"enabled"` - APIURL string `mapstructure:"api_url"` - - // TokenCommand prints a bearer token for APIURL, the way a CLI's - // `auth token` does. It is split on spaces and run without a shell. Empty - // sends no Authorization header, which a dev API that sets its own identity - // wants. - TokenCommand string `mapstructure:"token_command"` - - // Interval is the longest the tool will go without reconciling: the TUI's - // background pull period. - Interval time.Duration `mapstructure:"interval"` -} - // LocalConfig holds settings for the local embedded mode. type LocalConfig struct { DBPath string `mapstructure:"db_path"` @@ -60,10 +46,7 @@ type LocalConfig struct { func Load() (*Config, error) { v := newViper() - // Defaults. Every pull is a full reconcile, so the interval trades API load - // against staleness. Two minutes keeps an unattended TUI current. v.SetDefault("local.db_path", defaultDBPath()) - v.SetDefault("sync.interval", 2*time.Minute) if err := v.ReadInConfig(); err != nil { if _, ok := err.(viper.ConfigFileNotFoundError); !ok { @@ -74,10 +57,6 @@ func Load() (*Config, error) { // Env var overrides v.SetEnvPrefix("PROJECTS") _ = v.BindEnv("local.db_path", "PROJECTS_DB") - _ = v.BindEnv("sync.enabled", SyncVariable) - _ = v.BindEnv("sync.api_url", "PROJECTS_SYNC_URL") - _ = v.BindEnv("sync.token_command", "PROJECTS_SYNC_TOKEN_COMMAND") - _ = v.BindEnv("sync.interval", "PROJECTS_SYNC_INTERVAL") // ErrorUnused, so a key the tool does not read is refused rather than dropped. // viper decodes the merged settings map, and every default set above maps to a @@ -88,66 +67,30 @@ func Load() (*Config, error) { // naming its registry under a key the tool does not read would fall back to the // data directory, find no registry there, and lift the ban with nothing // reporting why. - if err := refuseTokenCommandInTheOtherShape(v); err != nil { + if err := refuseTokenCommandAsOneString(v); err != nil { return nil, err } var cfg Config - if err := v.Unmarshal(&cfg, viper.DecodeHook(mapstructure.StringToTimeDurationHookFunc()), func(dc *mapstructure.DecoderConfig) { + if err := v.Unmarshal(&cfg, func(dc *mapstructure.DecoderConfig) { dc.ErrorUnused = true }); err != nil { return nil, fmt.Errorf("parsing config: %w", err) } - - if cfg.Sync.Enabled && cfg.Sync.APIURL == "" { - return nil, fmt.Errorf("sync.api_url is required when sync is enabled (set in config or PROJECTS_SYNC_URL)") - } - - // A token command answers for the URL beside it. A URL from the environment - // names an API the config file does not describe, so the file's command - // never pairs with it: a dev .envrc overriding the URL would otherwise send a - // production token to the dev API. PROJECTS_SYNC_TOKEN_COMMAND is then the only - // source, and unset means no token. - if os.Getenv("PROJECTS_SYNC_URL") != "" { - cfg.Sync.TokenCommand = os.Getenv("PROJECTS_SYNC_TOKEN_COMMAND") - } - if cfg.Sync.TokenCommand != "" && strings.TrimSpace(cfg.Sync.TokenCommand) == "" { - return nil, fmt.Errorf("sync.token_command names no command") - } - - // time.NewTicker panics on a non-positive duration, and a config typo must not - // take the TUI down. Disabling automatic sync is what sync.enabled is for. - if cfg.Sync.Interval < minSyncInterval { - cfg.Sync.Interval = minSyncInterval - } - return &cfg, nil } -// SyncVariable turns sync on or off from the environment, over the file's -// sync.enabled. -const SyncVariable = "PROJECTS_SYNC" - -// refuseTokenCommandInTheOtherShape refuses a token_command written the way the -// other key takes it. Beside api it is an argument list printing a JSON token -// response; under [sync] it is one string printing a bare token. A command moved -// from one to the other would otherwise fail on a type error naming neither. -func refuseTokenCommandInTheOtherShape(v *viper.Viper) error { +// refuseTokenCommandAsOneString refuses a token_command written as one string. It +// is an argument list printing a JSON token response, and a string would otherwise +// fail on a type error naming neither. +func refuseTokenCommandAsOneString(v *viper.Viper) error { if command, isString := v.Get("token_command").(string); isString { return fmt.Errorf("token_command beside api is a list of arguments, run without a shell, that prints a JSON "+ - "token response: token_command = [\"%s\"]; one string is [sync]'s form", + "token response: token_command = [\"%s\"]", strings.Join(strings.Fields(command), `", "`)) } - if _, isList := v.Get("sync.token_command").([]any); isList { - return fmt.Errorf("[sync] token_command is one string, split on spaces, that prints a bare token, " + - "such as token_command = \" auth token\"; a list is the form beside api") - } return nil } -// minSyncInterval floors the reconcile period. A full pull is 2+2N requests, so -// anything shorter is a self-inflicted denial of service on the API. -const minSyncInterval = 15 * time.Second - // newViper points a fresh viper at the tool's config file. Shared by both readers // here so they cannot drift on where that file is. // @@ -168,10 +111,9 @@ func newViper() *viper.Viper { // should not have to hold a file saying so. // // A read of its own rather than a field off Load's result, because Load -// validates. Sync enabled without an api_url is an error there and has nothing -// to do with where the registry lives, so resolving through it would turn that -// unrelated failure into an empty registry — which bans nothing and reports -// nothing. +// refuses a key it does not read. That has nothing to do with where the registry +// lives, so resolving through it would turn an unrelated failure into an empty +// registry, which bans nothing and reports nothing. func ConfiguredReposRegistry() string { v := newViper() if err := v.ReadInConfig(); err != nil { diff --git a/config/config_test.go b/config/config_test.go index 03f7ddf..8ee263a 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -4,33 +4,11 @@ import ( "os" "path/filepath" "testing" - "time" ) -func TestLoad_Defaults(t *testing.T) { - // No config file, no env vars — should use defaults - t.Setenv("XDG_CONFIG_HOME", t.TempDir()) - t.Setenv("PROJECTS_SYNC", "") - t.Setenv("PROJECTS_SYNC_URL", "") - t.Setenv("PROJECTS_DB", "") - - cfg, err := Load() - if err != nil { - t.Fatalf("Load: %v", err) - } - - if cfg.Sync.Enabled { - t.Error("expected sync disabled by default") - } - if cfg.Local.DBPath == "" { - t.Error("expected non-empty default db_path") - } -} - func TestLoad_EnvOverrides(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) t.Setenv("PROJECTS_DB", "/tmp/test.db") - t.Setenv("PROJECTS_SYNC", "true") - t.Setenv("PROJECTS_SYNC_URL", "https://example.com") cfg, err := Load() if err != nil { @@ -40,23 +18,6 @@ func TestLoad_EnvOverrides(t *testing.T) { if cfg.Local.DBPath != "/tmp/test.db" { t.Errorf("expected db_path '/tmp/test.db', got %q", cfg.Local.DBPath) } - if !cfg.Sync.Enabled { - t.Error("expected sync enabled via env") - } - if cfg.Sync.APIURL != "https://example.com" { - t.Errorf("expected sync url 'https://example.com', got %q", cfg.Sync.APIURL) - } -} - -func TestLoad_SyncEnabledWithoutURL(t *testing.T) { - t.Setenv("XDG_CONFIG_HOME", t.TempDir()) - t.Setenv("PROJECTS_SYNC", "true") - t.Setenv("PROJECTS_SYNC_URL", "") - - _, err := Load() - if err == nil { - t.Fatal("expected error when sync enabled without api_url") - } } func TestDefaultDBPath_XDGOverride(t *testing.T) { @@ -69,37 +30,10 @@ func TestDefaultDBPath_XDGOverride(t *testing.T) { } } -// The interval is the TUI's background pull period, so a missing default would -// mean no automatic sync at all. -func TestLoad_SyncIntervalDefault(t *testing.T) { - t.Setenv("XDG_CONFIG_HOME", t.TempDir()) - t.Setenv("PROJECTS_SYNC_INTERVAL", "") - - cfg, err := Load() - if err != nil { - t.Fatalf("Load: %v", err) - } - if cfg.Sync.Interval != 2*time.Minute { - t.Errorf("expected a 2m default interval, got %v", cfg.Sync.Interval) - } -} - -func TestLoad_SyncIntervalFromEnv(t *testing.T) { - t.Setenv("XDG_CONFIG_HOME", t.TempDir()) - t.Setenv("PROJECTS_SYNC_INTERVAL", "45s") - - cfg, err := Load() - if err != nil { - t.Fatalf("Load: %v", err) - } - if cfg.Sync.Interval != 45*time.Second { - t.Errorf("expected 45s from the environment, got %v", cfg.Sync.Interval) - } -} - // repos_registry is a bare top-level key, which TOML requires above the first // [section] header — declared under one it would belong to that section and -// unmarshal into nothing. +// unmarshal into nothing. The [sync] section below it is the work box's file, +// which has to load rather than stop the tool. func TestReposRegistryIsReadFromTheConfigFile(t *testing.T) { const declared = "/declared/repos.json" writeConfig(t, "repos_registry = \""+declared+"\"\n\n[sync]\nenabled = false\n") @@ -115,6 +49,9 @@ func TestReposRegistryIsReadFromTheConfigFile(t *testing.T) { if cfg.ReposRegistry != declared { t.Errorf("cfg.ReposRegistry = %q, want %q", cfg.ReposRegistry, declared) } + if cfg.RetiredSync == nil { + t.Error("the [sync] section was dropped, so nothing warns that it is ignored") + } } // A machine keeping its registry where the tool expects it holds no config at @@ -144,73 +81,3 @@ func writeConfig(t *testing.T, body string) { } t.Setenv("XDG_CONFIG_HOME", dir) } - -const syncedConfig = "[sync]\nenabled = true\napi_url = \"https://api.example\"\ntoken_command = \"print-token --for api\"\n" - -func TestTheTokenCommandIsReadFromTheConfigFile(t *testing.T) { - writeConfig(t, syncedConfig) - t.Setenv("PROJECTS_SYNC_URL", "") - t.Setenv("PROJECTS_SYNC_TOKEN_COMMAND", "") - - cfg, err := Load() - if err != nil { - t.Fatalf("Load: %v", err) - } - if cfg.Sync.TokenCommand != "print-token --for api" { - t.Errorf("token command %q, want the file's", cfg.Sync.TokenCommand) - } -} - -// A dev .envrc points the URL at another API. The file's command was written for -// the file's URL, and must not hand its token to that one. -func TestAURLFromTheEnvironmentTakesNoTokenCommandFromTheFile(t *testing.T) { - writeConfig(t, syncedConfig) - t.Setenv("PROJECTS_SYNC_URL", "https://dev.example") - t.Setenv("PROJECTS_SYNC_TOKEN_COMMAND", "") - - cfg, err := Load() - if err != nil { - t.Fatalf("Load: %v", err) - } - if cfg.Sync.TokenCommand != "" { - t.Errorf("token command %q paired with the environment's URL, want none", cfg.Sync.TokenCommand) - } - - t.Setenv("PROJECTS_SYNC_TOKEN_COMMAND", "print-token --for dev") - cfg, err = Load() - if err != nil { - t.Fatalf("Load: %v", err) - } - if cfg.Sync.TokenCommand != "print-token --for dev" { - t.Errorf("token command %q, want the environment's", cfg.Sync.TokenCommand) - } -} - -// Split on spaces, a blank command is nothing to run, and silently sending no -// token would read as the API refusing a good one. -func TestABlankTokenCommandIsRefused(t *testing.T) { - writeConfig(t, "[sync]\nenabled = true\napi_url = \"https://api.example\"\ntoken_command = \" \"\n") - t.Setenv("PROJECTS_SYNC_URL", "") - t.Setenv("PROJECTS_SYNC_TOKEN_COMMAND", "") - - if _, err := Load(); err == nil { - t.Fatal("a blank token_command loaded") - } -} - -// time.NewTicker panics on a non-positive duration, so a typo here would take -// the TUI down on launch rather than degrading to a slower sync. -func TestLoad_SyncIntervalIsFloored(t *testing.T) { - t.Setenv("XDG_CONFIG_HOME", t.TempDir()) - - for _, given := range []string{"0s", "-5m", "1s"} { - t.Setenv("PROJECTS_SYNC_INTERVAL", given) - cfg, err := Load() - if err != nil { - t.Fatalf("Load(%s): %v", given, err) - } - if cfg.Sync.Interval < minSyncInterval { - t.Errorf("interval %q left below the floor: %v", given, cfg.Sync.Interval) - } - } -} diff --git a/config/former_name.go b/config/former_name.go index a880fc1..0a686de 100644 --- a/config/former_name.go +++ b/config/former_name.go @@ -21,10 +21,6 @@ const formerName = "todoui" var retiredVariables = []struct{ retired, replacement string }{ {"TODOUI_DB", "PROJECTS_DB"}, - {"TODOUI_SYNC", "PROJECTS_SYNC"}, - {"TODOUI_SYNC_URL", "PROJECTS_SYNC_URL"}, - {"TODOUI_SYNC_TOKEN_COMMAND", "PROJECTS_SYNC_TOKEN_COMMAND"}, - {"TODOUI_SYNC_INTERVAL", "PROJECTS_SYNC_INTERVAL"}, {"TODOUI_REPOS_REGISTRY", "PROJECTS_REPOS_REGISTRY"}, {"TODOUI_NO_AUTO_UPDATE", "PROJECTS_NO_AUTO_UPDATE"}, } @@ -43,7 +39,7 @@ func RetiredVariableRefusal() string { } // RefuseConfigUnderFormerName stops a run while the config sits only at the former path. -// Reading none would turn sync off, and the next pull deletes what was written meanwhile. +// Reading none would drop its api and send every command to a database on this machine. func RefuseConfigUnderFormerName() error { former := filepath.Join(userConfigDir(), formerName, "config.toml") current := filepath.Join(userConfigDir(), toolName, "config.toml") diff --git a/config/retired_sync.go b/config/retired_sync.go new file mode 100644 index 0000000..80ec8bc --- /dev/null +++ b/config/retired_sync.go @@ -0,0 +1,33 @@ +package config + +import ( + "fmt" + "os" +) + +// MIGRATION: projects once kept a database as a synced copy of an API, set under [sync] and by +// the variables below. Delete this file, Config.RetiredSync, its call in main.go and the warning +// in cli/data_source.go once no machine's config holds [sync] and no .envrc exports one of these. +var retiredSyncVariables = []string{ + "PROJECTS_SYNC", + "PROJECTS_SYNC_URL", + "PROJECTS_SYNC_TOKEN_COMMAND", + "PROJECTS_SYNC_INTERVAL", + "TODOUI_SYNC", + "TODOUI_SYNC_URL", + "TODOUI_SYNC_TOKEN_COMMAND", + "TODOUI_SYNC_INTERVAL", +} + +// RetiredSyncVariableRefusal names the first sync variable set to a value, or "" where none is. +// PROJECTS_SYNC_URL chose an API, and ignoring it would send the run wherever the config file +// points, which on a machine whose config names the production API is production. +func RetiredSyncVariableRefusal() string { + for _, name := range retiredSyncVariables { + if os.Getenv(name) != "" { + return fmt.Sprintf("$%s is set, and projects keeps no synced copy of an API. Unset it. "+ + "api in the config file, written by `projects auth login `, sends every command to an API.", name) + } + } + return "" +} diff --git a/former_name_test.go b/former_name_test.go index 817e955..5616468 100644 --- a/former_name_test.go +++ b/former_name_test.go @@ -17,8 +17,8 @@ import ( ) // Each case runs the built binary under a scratch home, because the exit code and the -// order against opening the database belong to main. `sync` reaches the move and then -// stops at sync being off, so no case needs a terminal. +// order against opening the database belong to main. `list` reaches the move and opens +// the database, and needs no terminal. func TestFormerNameOnFirstRun(t *testing.T) { binary := tuidrive.Build(t, ".") @@ -28,8 +28,8 @@ func TestFormerNameOnFirstRun(t *testing.T) { seedFormerDatabaseWithUnflushedWAL(t, former) writeFile(t, filepath.Join(former, "dev.db"), "dev") - if output, code := home.run(t, binary); code != 1 { - t.Fatalf("exit %d, output %q, want it to reach the disabled sync", code, output) + if output, code := home.run(t, binary); code != 0 { + t.Fatalf("exit %d, output %q, want the list to answer", code, output) } if _, err := os.Lstat(former); !errors.Is(err, os.ErrNotExist) { @@ -81,11 +81,11 @@ func TestFormerNameOnFirstRun(t *testing.T) { } }) - t.Run("a config left at the former path refuses before sync silently turns off", func(t *testing.T) { + t.Run("a config left at the former path refuses before it is silently ignored", func(t *testing.T) { home := newScratchHome(t) formerConfig := filepath.Join(home.root, "config", "todoui", "config.toml") formerDB := filepath.Join(home.dataDir("todoui"), "todoui.db") - writeFile(t, formerConfig, "[sync]\nenabled = true\n") + writeFile(t, formerConfig, "api = \"https://projects.example\"\n") writeFile(t, formerDB, "former") output, code := home.run(t, binary) @@ -158,7 +158,7 @@ func (h scratchHome) dataDir(name string) string { func (h scratchHome) run(t *testing.T, binary string, env ...string) (string, int) { t.Helper() - cmd := exec.Command(binary, "sync") + cmd := exec.Command(binary, "list") cmd.Dir = h.root cmd.Env = append([]string{ "HOME=" + h.root, diff --git a/main.go b/main.go index 22c8851..605cf01 100644 --- a/main.go +++ b/main.go @@ -9,9 +9,11 @@ import ( ) func main() { - if refusal := config.RetiredVariableRefusal(); refusal != "" { - fmt.Fprintf(os.Stderr, "error: %s\n", refusal) - os.Exit(2) + for _, refusal := range []string{config.RetiredVariableRefusal(), config.RetiredSyncVariableRefusal()} { + if refusal != "" { + fmt.Fprintf(os.Stderr, "error: %s\n", refusal) + os.Exit(2) + } } os.Exit(cli.Execute()) } diff --git a/model/project.go b/model/project.go index 6c845ee..a3fa9e7 100644 --- a/model/project.go +++ b/model/project.go @@ -15,7 +15,7 @@ import ( const ( StatusActive = "active" // StatusCompleted and not "done". The API stores this value behind a foreign - // key, so a push carrying any other word is rejected. It also matches the + // key, so a write carrying any other word is rejected. It also matches the // `completed` boolean an item carries, so a project and its own items spell // one concept one way. StatusCompleted = "completed" diff --git a/model/project_item.go b/model/project_item.go index 76a6965..ac63097 100644 --- a/model/project_item.go +++ b/model/project_item.go @@ -16,9 +16,10 @@ const ( // ProjectItem is the base representation of an item in the system. type ProjectItem struct { ID string `json:"id"` - // Number is the handle — short, server-assigned, and the only one of the two - // identifiers meant to be read or typed. Nil for an item created while sync - // was unreachable, which has no number until its create is pushed. + // Number is the handle — short, assigned as the item is created, and the only + // one of the two identifiers meant to be read or typed. Nil on an item from + // before numbers existed, or one a database once kept as a copy of an API + // created before the API numbered it. Number *int `json:"number,omitempty"` Title string `json:"title"` Notes *string `json:"notes,omitempty"` @@ -47,11 +48,8 @@ func (i ProjectItem) Status() string { // ProjectItemDetail is a ProjectItem with everything filed against it. // -// DependencyIDs and Tasks decode to nil when a server omits the field and to an -// empty slice when it sends one, which is the only way a pull can tell a server -// predating the embed from an item that genuinely has neither. Keep them as plain -// slices and keep the API sending `[]`; anything that collapses nil and empty -// makes an old server look like an instruction to delete every task. +// DependencyIDs and Tasks hold an empty slice rather than nil where an item has +// none, so the API answers `[]` for them. type ProjectItemDetail struct { ProjectItem Projects []Project `json:"projects"` diff --git a/model/undo.go b/model/undo.go index f4dfc88..bc5b22c 100644 --- a/model/undo.go +++ b/model/undo.go @@ -33,56 +33,10 @@ type UndoEntry struct { CreatedAt time.Time `json:"created_at"` } -// UndoResult describes what an undo actually did. A sync layer needs more than -// a display string: the pending queue still holds operations describing the -// state the undo just reversed, and reconciling them requires knowing which -// entity changed and what it looks like now. +// UndoResult describes what an undo reversed. type UndoResult struct { Description string Action string EntityType string EntityID string - - // Restored is the local row after the undo, or nil when the undo removed it. - // Exactly one of Restored / RestoredProject can be set, matching EntityType. - Restored *ProjectItem - RestoredProject *Project - - // Rows that cascaded off a delete and came back with it. Sync has to push - // these, not just the row: a pull replaces memberships and dependencies - // wholesale from server state and drops tasks the server does not know - // about, so anything restored only locally is erased on the next pull. - RestoredProjectIDs []string // projects an item was restored into - RestoredItemIDs []string // items a restored project held - RestoredTasks []ProjectItemTask - RestoredDependencies []ItemDependency - - // Detail is set for entity types other than item and project. - Detail *UndoDetail -} - -// ItemDependency is one edge of the blocking graph: ItemID is blocked until -// DependsOnID is complete. -type ItemDependency struct { - ItemID string `json:"item_id"` - DependsOnID string `json:"depends_on_id"` -} - -// UndoDetail describes a reversal of anything other than an item or project -// row — a task, a membership, a dependency, or a reorder. Only the fields -// relevant to the result's EntityType are set. The sync layer needs it to -// push the same reversal rather than guessing from the entity ID alone. -type UndoDetail struct { - // Removed reports whether the undo took the row away rather than putting - // it back. It is what separates reversing an add from reversing a remove, - // which are otherwise indistinguishable at the sync layer. - Removed bool - - ItemID string // owning item: task, membership, dependency, item reorder - ProjectID string // membership and item reorder - DependsOnID string // dependency - Position int // reorder - - // Task is the row as it stands after the undo, nil when the undo removed it. - Task *ProjectItemTask } diff --git a/real_binary_flows_test.go b/real_binary_flows_test.go index 19db595..43adf91 100644 --- a/real_binary_flows_test.go +++ b/real_binary_flows_test.go @@ -108,7 +108,7 @@ func writeAPIConfig(t *testing.T, dir, api string) { } } -// seedFixtureDatabase writes through the backend the TUI writes through with sync off, +// seedFixtureDatabase writes through the backend the TUI writes through in database mode, // so the fixture holds what the tool's own rules allow. The export note runs long, // because a one-line note hides layout bugs. func seedFixtureDatabase(t *testing.T, path string) { diff --git a/service/number_test.go b/service/number_test.go index fd19f60..f4a852b 100644 --- a/service/number_test.go +++ b/service/number_test.go @@ -44,22 +44,3 @@ func TestCreateItem_AssignsNumbersWhenThisDatabaseIsTheAuthority(t *testing.T) { t.Errorf("numbers = %v, want %v — the first item in a fresh database is 1", numbers, want) } } - -// With sync on the server owns the numbering. Guessing one locally would hand -// the user a handle that changes the moment the push comes back. -func TestCreateItem_LeavesTheNumberToTheServerWhenSyncIsOn(t *testing.T) { - b := service.New(sqlite.New(newNumberTestDB(t))) - project, err := b.CreateProject(model.CreateProject{Name: "Synced"}) - if err != nil { - t.Fatalf("CreateProject: %v", err) - } - - item, err := b.CreateItem(model.CreateProjectItem{Title: "Waits for a number", ProjectIDs: []string{project.ID}}) - if err != nil { - t.Fatalf("CreateItem: %v", err) - } - - if item.Number != nil { - t.Errorf("Number = %d, want nil — the server assigns it and the two would disagree", *item.Number) - } -} diff --git a/service/references.go b/service/references.go index f87e0ec..adeb673 100644 --- a/service/references.go +++ b/service/references.go @@ -159,8 +159,8 @@ func parseItemNumber(ref string) (int, bool) { return number, err == nil } -// itemHandle is what a refusal calls an item. An item created while sync was -// unreachable has no number yet, so it goes by its id. +// itemHandle is what a refusal calls an item: its number, or its id where it +// has none. func itemHandle(item model.ProjectItem) string { if item.Number != nil { return strconv.Itoa(*item.Number) diff --git a/service/service.go b/service/service.go index 4521393..7252d21 100644 --- a/service/service.go +++ b/service/service.go @@ -23,8 +23,7 @@ const itemListSegment = "/items" // Service holds every rule about projects, items, memberships, dependencies, tasks and undo. type Service struct { store store.Store - // assignsItemNumbers is set when nothing upstream will hand one out, which - // is exactly the sync-disabled case. See AssigningItemNumbers. + // assignsItemNumbers is set by AssigningItemNumbers. assignsItemNumbers bool // repoRegistryPath enables the repo-named-project ban. Empty means no // registry and no ban. See RefusingRepoNames. @@ -36,12 +35,9 @@ type Service struct { // Option configures a Service at construction. type Option func(*Service) -// AssigningItemNumbers makes this database the authority for item numbers. -// -// Pass it when sync is off, because that database is a disjoint universe no -// server will ever number. With sync on the server assigns instead, because a -// number guessed locally and a number assigned upstream would disagree and the -// handle would change under a user who had already written it down. +// AssigningItemNumbers numbers each item as it is created, from the store's +// NextItemNumber. Both entry points pass it: database mode in cli.LocalService, +// and projects-api over Postgres. func AssigningItemNumbers() Option { return func(s *Service) { s.assignsItemNumbers = true } } @@ -997,8 +993,7 @@ func (s *Service) CreateTask(itemRef string, input model.CreateProjectItemTask) return fmt.Errorf("creating task: %w", err) } created = t - // The row, not nil: undoing a task create still has to know which item - // owned it so the reversal can be pushed. + // The row, not nil: undoTask refuses a task entry with no previous state. return logUndo(ctx, tx, model.UndoActionCreate, model.UndoEntityTask, id, taskRecordOf(t)) }) if err != nil { diff --git a/service/service_test.go b/service/service_test.go index 8096cd4..022f52e 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -404,10 +404,6 @@ func TestUndoCreateItem(t *testing.T) { if result.EntityID != item.ID { t.Errorf("expected undo result for %s, got %s", item.ID, result.EntityID) } - if result.Restored != nil { - t.Error("undoing a create removes the row, so nothing should be restored") - } - _, err = b.GetItem(item.ID) if !errors.Is(err, model.ErrNotFound) { t.Errorf("expected ErrNotFound after undo, got %v", err) @@ -446,15 +442,15 @@ func TestUndoRestoresDeletedProject(t *testing.T) { t.Fatalf("deleting project: %v", err) } - result, err := b.Undo() - if err != nil { + if _, err := b.Undo(); err != nil { t.Fatalf("undoing: %v", err) } - if result.RestoredProject == nil || result.RestoredProject.Name != "doomed" { - t.Fatalf("expected the deleted project restored, got %+v", result.RestoredProject) + restored, err := b.GetProject(p.ID) + if err != nil { + t.Fatalf("expected project back after undo: %v", err) } - if _, err := b.GetProject(p.ID); err != nil { - t.Errorf("expected project back after undo: %v", err) + if restored.Name != "doomed" { + t.Errorf("restored project is named %q, want doomed", restored.Name) } } @@ -879,8 +875,7 @@ func TestUndoPreservesDeletedItemTimestamps(t *testing.T) { } } -// An edit in the TUI reaches this while the sync engine's pull writes, and two -// running copies reach it on one file. Each rule reads and then writes, and SQLite +// Two running copies reach this on one file, such as the board and a command. Each rule reads and then writes, and SQLite // refuses that upgrade once another writer has committed unless the transaction // began with the write lock. func TestAnEditSurvivesASecondWriter(t *testing.T) { @@ -963,12 +958,12 @@ func TestUndoAcceptsPreSnapshotDeleteEntries(t *testing.T) { `"Completed":0,"Archived":0,"CreatedAt":"2026-01-02 03:04:05","UpdatedAt":"2026-01-02 03:04:05"}`, item.ID)) - result, err := b.Undo() - if err != nil { + if _, err := b.Undo(); err != nil { t.Fatalf("undoing: %v", err) } - if result.Restored == nil || result.Restored.Title != "legacy" { - t.Fatalf("expected the item restored from a pre-snapshot entry, got %+v", result.Restored) + restored, err := b.GetItem(item.ID) + if err != nil || restored.Title != "legacy" { + t.Fatalf("expected the item restored from a pre-snapshot entry, got %+v, %v", restored, err) } } diff --git a/service/undo.go b/service/undo.go index ccfdc2d..3b25a33 100644 --- a/service/undo.go +++ b/service/undo.go @@ -36,19 +36,19 @@ func (s *Service) Undo() (*model.UndoResult, error) { var undoErr error switch entry.EntityType { case model.UndoEntityProject: - undoErr = undoProject(ctx, tx, entry, result) + undoErr = undoProject(ctx, tx, entry) case model.UndoEntityItem: - undoErr = undoItem(ctx, tx, entry, result) + undoErr = undoItem(ctx, tx, entry) case model.UndoEntityTask: - undoErr = undoTask(ctx, tx, entry, result) + undoErr = undoTask(ctx, tx, entry) case model.UndoEntityMembership: - undoErr = undoMembership(ctx, tx, entry, result) + undoErr = undoMembership(ctx, tx, entry) case model.UndoEntityDependency: - undoErr = undoDependency(ctx, tx, entry, result) + undoErr = undoDependency(ctx, tx, entry) case model.UndoEntityItemPosition: - undoErr = undoItemPosition(ctx, tx, entry, result) + undoErr = undoItemPosition(ctx, tx, entry) case model.UndoEntityProjectPosition: - undoErr = undoProjectPosition(ctx, tx, entry, result) + undoErr = undoProjectPosition(ctx, tx, entry) default: return fmt.Errorf("undoing unknown entity type %q", entry.EntityType) } @@ -68,7 +68,7 @@ func (s *Service) Undo() (*model.UndoResult, error) { return result, nil } -func undoItem(ctx context.Context, tx store.Operations, entry model.UndoEntry, result *model.UndoResult) error { +func undoItem(ctx context.Context, tx store.Operations, entry model.UndoEntry) error { switch entry.Action { case model.UndoActionCreate: if err := tx.DeleteItem(ctx, entry.EntityID); err != nil { @@ -82,11 +82,9 @@ func undoItem(ctx context.Context, tx store.Operations, entry model.UndoEntry, r } item := prev.toModel() item.ID = entry.EntityID - restored, err := tx.UpdateItem(ctx, item) - if err != nil { + if _, err := tx.UpdateItem(ctx, item); err != nil { return fmt.Errorf("restoring previous state: %w", err) } - result.Restored = &restored } case model.UndoActionDelete: if entry.PreviousState != nil { @@ -97,20 +95,6 @@ func undoItem(ctx context.Context, tx store.Operations, entry model.UndoEntry, r if err := restoreItemSnapshot(ctx, tx, snapshot); err != nil { return err } - item := snapshot.Item.toModel() - result.Restored = &item - for _, membership := range snapshot.Memberships { - result.RestoredProjectIDs = append(result.RestoredProjectIDs, membership.ProjectID) - } - for _, dependency := range snapshot.Dependencies { - result.RestoredDependencies = append(result.RestoredDependencies, model.ItemDependency{ - ItemID: dependency.ItemID, - DependsOnID: dependency.DependsOnID, - }) - } - for _, task := range snapshot.Tasks { - result.RestoredTasks = append(result.RestoredTasks, task.toModel()) - } } default: return fmt.Errorf("undoing unknown item action %q", entry.Action) @@ -118,7 +102,7 @@ func undoItem(ctx context.Context, tx store.Operations, entry model.UndoEntry, r return nil } -func undoTask(ctx context.Context, tx store.Operations, entry model.UndoEntry, result *model.UndoResult) error { +func undoTask(ctx context.Context, tx store.Operations, entry model.UndoEntry) error { if entry.PreviousState == nil { return fmt.Errorf("task undo entry %d has no previous state", entry.ID) } @@ -127,28 +111,22 @@ func undoTask(ctx context.Context, tx store.Operations, entry model.UndoEntry, r return fmt.Errorf("unmarshaling previous task state: %w", err) } - detail := &model.UndoDetail{ItemID: prev.ItemID} - result.Detail = detail - if entry.Action == model.UndoActionCreate { if err := tx.DeleteTask(ctx, entry.EntityID); err != nil { return fmt.Errorf("undoing task create: %w", err) } - detail.Removed = true return nil } // Update and delete both reverse to the recorded row. Restore covers both: // the row is still there after an update, and gone after a delete. - task := prev.toModel() - if err := tx.RestoreTask(ctx, task); err != nil { + if err := tx.RestoreTask(ctx, prev.toModel()); err != nil { return fmt.Errorf("restoring task: %w", err) } - detail.Task = &task return nil } -func undoMembership(ctx context.Context, tx store.Operations, entry model.UndoEntry, result *model.UndoResult) error { +func undoMembership(ctx context.Context, tx store.Operations, entry model.UndoEntry) error { if entry.PreviousState == nil { return fmt.Errorf("membership undo entry %d has no previous state", entry.ID) } @@ -157,12 +135,6 @@ func undoMembership(ctx context.Context, tx store.Operations, entry model.UndoEn return fmt.Errorf("unmarshaling previous membership state: %w", err) } - result.Detail = &model.UndoDetail{ - ItemID: prev.ItemID, - ProjectID: prev.ProjectID, - Removed: entry.Action == model.UndoActionCreate, - } - if entry.Action == model.UndoActionCreate { if err := tx.RemoveItemFromProject(ctx, prev.ItemID, prev.ProjectID); err != nil { return fmt.Errorf("undoing membership create: %w", err) @@ -176,7 +148,7 @@ func undoMembership(ctx context.Context, tx store.Operations, entry model.UndoEn return nil } -func undoDependency(ctx context.Context, tx store.Operations, entry model.UndoEntry, result *model.UndoResult) error { +func undoDependency(ctx context.Context, tx store.Operations, entry model.UndoEntry) error { if entry.PreviousState == nil { return fmt.Errorf("dependency undo entry %d has no previous state", entry.ID) } @@ -185,12 +157,6 @@ func undoDependency(ctx context.Context, tx store.Operations, entry model.UndoEn return fmt.Errorf("unmarshaling previous dependency state: %w", err) } - result.Detail = &model.UndoDetail{ - ItemID: prev.ItemID, - DependsOnID: prev.DependsOnID, - Removed: entry.Action == model.UndoActionCreate, - } - if entry.Action == model.UndoActionCreate { if err := tx.RemoveDependency(ctx, model.Dependency(prev)); err != nil { return fmt.Errorf("undoing dependency create: %w", err) @@ -204,7 +170,7 @@ func undoDependency(ctx context.Context, tx store.Operations, entry model.UndoEn return nil } -func undoItemPosition(ctx context.Context, tx store.Operations, entry model.UndoEntry, result *model.UndoResult) error { +func undoItemPosition(ctx context.Context, tx store.Operations, entry model.UndoEntry) error { if entry.PreviousState == nil { return fmt.Errorf("item reorder undo entry %d has no previous state", entry.ID) } @@ -221,16 +187,10 @@ func undoItemPosition(ctx context.Context, tx store.Operations, entry model.Undo return fmt.Errorf("restoring a sibling's position: %w", err) } } - - result.Detail = &model.UndoDetail{ - ItemID: prev.ItemID, - ProjectID: prev.ProjectID, - Position: int(prev.Position), - } return nil } -func undoProjectPosition(ctx context.Context, tx store.Operations, entry model.UndoEntry, result *model.UndoResult) error { +func undoProjectPosition(ctx context.Context, tx store.Operations, entry model.UndoEntry) error { if entry.PreviousState == nil { return fmt.Errorf("project reorder undo entry %d has no previous state", entry.ID) } @@ -242,13 +202,11 @@ func undoProjectPosition(ctx context.Context, tx store.Operations, entry model.U if err := tx.UpdateProjectPosition(ctx, prev.ID, int(prev.Position)); err != nil { return fmt.Errorf("restoring project position: %w", err) } - - result.Detail = &model.UndoDetail{Position: int(prev.Position)} return nil } // undoProject reverses a logged project mutation. -func undoProject(ctx context.Context, tx store.Operations, entry model.UndoEntry, result *model.UndoResult) error { +func undoProject(ctx context.Context, tx store.Operations, entry model.UndoEntry) error { switch entry.Action { case model.UndoActionCreate: if err := tx.DeleteProject(ctx, entry.EntityID); err != nil { @@ -266,11 +224,6 @@ func undoProject(ctx context.Context, tx store.Operations, entry model.UndoEntry if err := tx.RestoreProject(ctx, prev.toModel()); err != nil { return fmt.Errorf("restoring previous project state: %w", err) } - restored, err := tx.GetProject(ctx, entry.EntityID) - if err != nil { - return fmt.Errorf("reading back the restored project: %w", err) - } - result.RestoredProject = &restored } case model.UndoActionDelete: if entry.PreviousState != nil { @@ -281,11 +234,6 @@ func undoProject(ctx context.Context, tx store.Operations, entry model.UndoEntry if err := restoreProjectSnapshot(ctx, tx, snapshot); err != nil { return err } - project := snapshot.Project.toModel() - result.RestoredProject = &project - for _, membership := range snapshot.Memberships { - result.RestoredItemIDs = append(result.RestoredItemIDs, membership.ItemID) - } } default: return fmt.Errorf("undoing unknown project action %q", entry.Action) diff --git a/store/sqlite/convert.go b/store/sqlite/convert.go index dd7df27..9979282 100644 --- a/store/sqlite/convert.go +++ b/store/sqlite/convert.go @@ -5,7 +5,7 @@ import ( "time" "github.com/datapointchris/projects/model" - "github.com/datapointchris/projects/store/sqlite/generated" + "github.com/datapointchris/projects/store/sqlite/internal/generated" ) func parseTime(s string) time.Time { diff --git a/store/sqlite/generated/db.go b/store/sqlite/internal/generated/db.go similarity index 100% rename from store/sqlite/generated/db.go rename to store/sqlite/internal/generated/db.go diff --git a/store/sqlite/generated/models.go b/store/sqlite/internal/generated/models.go similarity index 77% rename from store/sqlite/generated/models.go rename to store/sqlite/internal/generated/models.go index 98361e1..52817fb 100644 --- a/store/sqlite/generated/models.go +++ b/store/sqlite/internal/generated/models.go @@ -8,17 +8,6 @@ import ( "database/sql" ) -type PendingSync struct { - ID int64 - Operation string - EntityType string - EntityID string - Payload string - CreatedAt string - Attempts int64 - LastError sql.NullString -} - type Project struct { ID string Name string @@ -73,18 +62,6 @@ type ProjectStatus struct { Name string } -type SyncOrigin struct { - ID int64 - ApiUrl string - AdoptedAt string -} - -type SyncState struct { - EntityType string - LastPullAt string - LastPushAt string -} - type UndoLog struct { ID int64 Action string diff --git a/store/sqlite/generated/queries.sql.go b/store/sqlite/internal/generated/queries.sql.go similarity index 73% rename from store/sqlite/generated/queries.sql.go rename to store/sqlite/internal/generated/queries.sql.go index 19d2e3d..ba8f98e 100644 --- a/store/sqlite/generated/queries.sql.go +++ b/store/sqlite/internal/generated/queries.sql.go @@ -40,43 +40,6 @@ func (q *Queries) AddItemToProject(ctx context.Context, arg AddItemToProjectPara return err } -const countItems = `-- name: CountItems :one -SELECT COUNT(*) FROM project_items -` - -func (q *Queries) CountItems(ctx context.Context) (int64, error) { - row := q.db.QueryRowContext(ctx, countItems) - var count int64 - err := row.Scan(&count) - return count, err -} - -const countPendingSync = `-- name: CountPendingSync :one -SELECT COUNT(*) FROM pending_sync -` - -func (q *Queries) CountPendingSync(ctx context.Context) (int64, error) { - row := q.db.QueryRowContext(ctx, countPendingSync) - var count int64 - err := row.Scan(&count) - return count, err -} - -const countProjects = `-- name: CountProjects :one - -SELECT COUNT(*) FROM projects -` - -// Sync: state tracking -// Counted rather than listed: the caller only needs to know whether this -// database has anything to lose. -func (q *Queries) CountProjects(ctx context.Context) (int64, error) { - row := q.db.QueryRowContext(ctx, countProjects) - var count int64 - err := row.Scan(&count) - return count, err -} - const countUndoLogs = `-- name: CountUndoLogs :one SELECT COUNT(*) FROM undo_log ` @@ -202,33 +165,6 @@ func (q *Queries) CreateTask(ctx context.Context, arg CreateTaskParams) (Project return i, err } -const deleteAllDependencies = `-- name: DeleteAllDependencies :exec -DELETE FROM project_item_dependencies -` - -func (q *Queries) DeleteAllDependencies(ctx context.Context) error { - _, err := q.db.ExecContext(ctx, deleteAllDependencies) - return err -} - -const deleteAllMemberships = `-- name: DeleteAllMemberships :exec -DELETE FROM project_item_memberships -` - -func (q *Queries) DeleteAllMemberships(ctx context.Context) error { - _, err := q.db.ExecContext(ctx, deleteAllMemberships) - return err -} - -const deleteAllPendingSync = `-- name: DeleteAllPendingSync :exec -DELETE FROM pending_sync -` - -func (q *Queries) DeleteAllPendingSync(ctx context.Context) error { - _, err := q.db.ExecContext(ctx, deleteAllPendingSync) - return err -} - const deleteItem = `-- name: DeleteItem :exec DELETE FROM project_items WHERE id = ? ` @@ -238,36 +174,6 @@ func (q *Queries) DeleteItem(ctx context.Context, id string) error { return err } -const deletePendingSync = `-- name: DeletePendingSync :exec -DELETE FROM pending_sync WHERE id = ? -` - -func (q *Queries) DeletePendingSync(ctx context.Context, id int64) error { - _, err := q.db.ExecContext(ctx, deletePendingSync, id) - return err -} - -const deletePendingSyncByEntity = `-- name: DeletePendingSyncByEntity :execrows -DELETE FROM pending_sync WHERE entity_id = ? -` - -func (q *Queries) DeletePendingSyncByEntity(ctx context.Context, entityID string) (int64, error) { - result, err := q.db.ExecContext(ctx, deletePendingSyncByEntity, entityID) - if err != nil { - return 0, err - } - return result.RowsAffected() -} - -const deletePendingSyncUpTo = `-- name: DeletePendingSyncUpTo :exec -DELETE FROM pending_sync WHERE id <= ? -` - -func (q *Queries) DeletePendingSyncUpTo(ctx context.Context, id int64) error { - _, err := q.db.ExecContext(ctx, deletePendingSyncUpTo, id) - return err -} - const deleteProject = `-- name: DeleteProject :exec DELETE FROM projects WHERE id = ? ` @@ -504,26 +410,6 @@ func (q *Queries) GetMembership(ctx context.Context, arg GetMembershipParams) (P return i, err } -const getOldestPendingSync = `-- name: GetOldestPendingSync :one -SELECT id, operation, entity_type, entity_id, payload, created_at, attempts, last_error FROM pending_sync ORDER BY id ASC LIMIT 1 -` - -func (q *Queries) GetOldestPendingSync(ctx context.Context) (PendingSync, error) { - row := q.db.QueryRowContext(ctx, getOldestPendingSync) - var i PendingSync - err := row.Scan( - &i.ID, - &i.Operation, - &i.EntityType, - &i.EntityID, - &i.Payload, - &i.CreatedAt, - &i.Attempts, - &i.LastError, - ) - return i, err -} - const getProject = `-- name: GetProject :one SELECT id, name, description, kind, status, status_reason, closed_at, position, created_at FROM projects WHERE id = ? ` @@ -572,17 +458,6 @@ func (q *Queries) GetProjectMemberships(ctx context.Context, projectID string) ( return items, nil } -const getSyncOrigin = `-- name: GetSyncOrigin :one -SELECT api_url FROM sync_origin WHERE id = 1 -` - -func (q *Queries) GetSyncOrigin(ctx context.Context) (string, error) { - row := q.db.QueryRowContext(ctx, getSyncOrigin) - var api_url string - err := row.Scan(&api_url) - return api_url, err -} - const getTask = `-- name: GetTask :one SELECT id, item_id, title, completed, completed_at, position, created_at FROM project_item_tasks WHERE id = ? ` @@ -602,43 +477,6 @@ func (q *Queries) GetTask(ctx context.Context, id string) (ProjectItemTask, erro return i, err } -const hasPulledBefore = `-- name: HasPulledBefore :one -SELECT COUNT(*) FROM sync_state WHERE last_pull_at > '1970-01-01T00:00:00.000Z' -` - -// Whether this database has ever reconciled with an API. A database that has -// pulled was bound to something, even if nothing recorded what. -func (q *Queries) HasPulledBefore(ctx context.Context) (int64, error) { - row := q.db.QueryRowContext(ctx, hasPulledBefore) - var count int64 - err := row.Scan(&count) - return count, err -} - -const insertPendingSync = `-- name: InsertPendingSync :exec - -INSERT INTO pending_sync (operation, entity_type, entity_id, payload) -VALUES (?, ?, ?, ?) -` - -type InsertPendingSyncParams struct { - Operation string - EntityType string - EntityID string - Payload string -} - -// Sync: pending operations -func (q *Queries) InsertPendingSync(ctx context.Context, arg InsertPendingSyncParams) error { - _, err := q.db.ExecContext(ctx, insertPendingSync, - arg.Operation, - arg.EntityType, - arg.EntityID, - arg.Payload, - ) - return err -} - const insertUndoLog = `-- name: InsertUndoLog :exec INSERT INTO undo_log (action, entity_type, entity_id, previous_state) @@ -947,69 +785,6 @@ func (q *Queries) ListItemsInProject(ctx context.Context, projectID string) ([]L return items, nil } -const listPendingSync = `-- name: ListPendingSync :many -SELECT id, operation, entity_type, entity_id, payload, created_at, attempts, last_error FROM pending_sync ORDER BY id ASC -` - -func (q *Queries) ListPendingSync(ctx context.Context) ([]PendingSync, error) { - rows, err := q.db.QueryContext(ctx, listPendingSync) - if err != nil { - return nil, err - } - defer rows.Close() - var items []PendingSync - for rows.Next() { - var i PendingSync - if err := rows.Scan( - &i.ID, - &i.Operation, - &i.EntityType, - &i.EntityID, - &i.Payload, - &i.CreatedAt, - &i.Attempts, - &i.LastError, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - -const listPendingSyncEntityIDsAfter = `-- name: ListPendingSyncEntityIDsAfter :many -SELECT DISTINCT entity_id FROM pending_sync WHERE id > ? -` - -func (q *Queries) ListPendingSyncEntityIDsAfter(ctx context.Context, id int64) ([]string, error) { - rows, err := q.db.QueryContext(ctx, listPendingSyncEntityIDsAfter, id) - if err != nil { - return nil, err - } - defer rows.Close() - var items []string - for rows.Next() { - var entity_id string - if err := rows.Scan(&entity_id); err != nil { - return nil, err - } - items = append(items, entity_id) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const listProjectItemFacts = `-- name: ListProjectItemFacts :many SELECT m.project_id, pi.completed, pi.archived, pi.repo FROM project_item_memberships m @@ -1143,24 +918,10 @@ func (q *Queries) ListTasksByItem(ctx context.Context, itemID string) ([]Project return items, nil } -const maxPendingSyncID = `-- name: MaxPendingSyncID :one -SELECT CAST(COALESCE(MAX(id), 0) AS INTEGER) AS max_id FROM pending_sync -` - -func (q *Queries) MaxPendingSyncID(ctx context.Context) (int64, error) { - row := q.db.QueryRowContext(ctx, maxPendingSyncID) - var max_id int64 - err := row.Scan(&max_id) - return max_id, err -} - const nextItemNumber = `-- name: NextItemNumber :one SELECT COALESCE(MAX(number), 0) + 1 FROM project_items ` -// Only used when sync is off and this database assigns its own numbers. With -// sync on the server is the authority and this would hand out a number it is -// about to disagree with. func (q *Queries) NextItemNumber(ctx context.Context) (int64, error) { row := q.db.QueryRowContext(ctx, nextItemNumber) var column_1 int64 @@ -1284,7 +1045,7 @@ type RestoreProjectParams struct { CreatedAt string } -// Undo: a restore carries each column a pull leaves alone. +// Undo: a restore inserts the whole row, or, where it is still there, rewrites what an edit can change. func (q *Queries) RestoreProject(ctx context.Context, arg RestoreProjectParams) error { _, err := q.db.ExecContext(ctx, restoreProject, arg.ID, @@ -1378,35 +1139,6 @@ func (q *Queries) SearchItems(ctx context.Context, arg SearchItemsParams) ([]Pro return items, nil } -const setItemNumber = `-- name: SetItemNumber :exec -UPDATE project_items SET number = ? WHERE id = ? -` - -type SetItemNumberParams struct { - Number sql.NullInt64 - ID string -} - -func (q *Queries) SetItemNumber(ctx context.Context, arg SetItemNumberParams) error { - _, err := q.db.ExecContext(ctx, setItemNumber, arg.Number, arg.ID) - return err -} - -const setSyncOrigin = `-- name: SetSyncOrigin :exec -INSERT INTO sync_origin (id, api_url, adopted_at) -VALUES (1, ?, strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) -ON CONFLICT(id) DO UPDATE SET - api_url = excluded.api_url, - adopted_at = excluded.adopted_at -` - -// Adoption is deliberate and always overwrites: the caller has already decided -// this database belongs to this API. -func (q *Queries) SetSyncOrigin(ctx context.Context, apiUrl string) error { - _, err := q.db.ExecContext(ctx, setSyncOrigin, apiUrl) - return err -} - const updateItem = `-- name: UpdateItem :one UPDATE project_items SET title = ?, @@ -1471,20 +1203,6 @@ func (q *Queries) UpdateItemPosition(ctx context.Context, arg UpdateItemPosition return err } -const updatePendingSyncError = `-- name: UpdatePendingSyncError :exec -UPDATE pending_sync SET attempts = attempts + 1, last_error = ? WHERE id = ? -` - -type UpdatePendingSyncErrorParams struct { - LastError sql.NullString - ID int64 -} - -func (q *Queries) UpdatePendingSyncError(ctx context.Context, arg UpdatePendingSyncErrorParams) error { - _, err := q.db.ExecContext(ctx, updatePendingSyncError, arg.LastError, arg.ID) - return err -} - const updateProject = `-- name: UpdateProject :one UPDATE projects SET name = ?, @@ -1602,149 +1320,3 @@ func (q *Queries) UpsertDependency(ctx context.Context, arg UpsertDependencyPara _, err := q.db.ExecContext(ctx, upsertDependency, arg.ItemID, arg.DependsOnID) return err } - -const upsertItem = `-- name: UpsertItem :exec -INSERT INTO project_items (id, number, title, notes, repo, completed, archived, created_at, updated_at) -VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) -ON CONFLICT(id) DO UPDATE SET - number = excluded.number, - title = excluded.title, - notes = excluded.notes, - repo = excluded.repo, - completed = excluded.completed, - archived = excluded.archived, - updated_at = excluded.updated_at -` - -type UpsertItemParams struct { - ID string - Number sql.NullInt64 - Title string - Notes sql.NullString - Repo sql.NullString - Completed int64 - Archived int64 - CreatedAt string - UpdatedAt string -} - -func (q *Queries) UpsertItem(ctx context.Context, arg UpsertItemParams) error { - _, err := q.db.ExecContext(ctx, upsertItem, - arg.ID, - arg.Number, - arg.Title, - arg.Notes, - arg.Repo, - arg.Completed, - arg.Archived, - arg.CreatedAt, - arg.UpdatedAt, - ) - return err -} - -const upsertMembership = `-- name: UpsertMembership :exec -INSERT INTO project_item_memberships (item_id, project_id, position) -VALUES (?, ?, ?) -ON CONFLICT(item_id, project_id) DO UPDATE SET - position = excluded.position -` - -type UpsertMembershipParams struct { - ItemID string - ProjectID string - Position int64 -} - -func (q *Queries) UpsertMembership(ctx context.Context, arg UpsertMembershipParams) error { - _, err := q.db.ExecContext(ctx, upsertMembership, arg.ItemID, arg.ProjectID, arg.Position) - return err -} - -const upsertProject = `-- name: UpsertProject :exec - -INSERT INTO projects (id, name, description, status, status_reason, closed_at, position, created_at) -VALUES (?, ?, ?, ?, ?, ?, ?, ?) -ON CONFLICT(id) DO UPDATE SET - name = excluded.name, - description = excluded.description, - status = excluded.status, - status_reason = excluded.status_reason, - closed_at = excluded.closed_at, - position = excluded.position -` - -type UpsertProjectParams struct { - ID string - Name string - Description sql.NullString - Status string - StatusReason sql.NullString - ClosedAt sql.NullString - Position int64 - CreatedAt string -} - -// Sync: pull reconciliation (upserts) -func (q *Queries) UpsertProject(ctx context.Context, arg UpsertProjectParams) error { - _, err := q.db.ExecContext(ctx, upsertProject, - arg.ID, - arg.Name, - arg.Description, - arg.Status, - arg.StatusReason, - arg.ClosedAt, - arg.Position, - arg.CreatedAt, - ) - return err -} - -const upsertSyncState = `-- name: UpsertSyncState :exec -INSERT INTO sync_state (entity_type, last_pull_at, last_push_at) -VALUES (?, ?, ?) -ON CONFLICT(entity_type) DO UPDATE SET - last_pull_at = CASE WHEN excluded.last_pull_at > sync_state.last_pull_at THEN excluded.last_pull_at ELSE sync_state.last_pull_at END, - last_push_at = CASE WHEN excluded.last_push_at > sync_state.last_push_at THEN excluded.last_push_at ELSE sync_state.last_push_at END -` - -type UpsertSyncStateParams struct { - EntityType string - LastPullAt string - LastPushAt string -} - -func (q *Queries) UpsertSyncState(ctx context.Context, arg UpsertSyncStateParams) error { - _, err := q.db.ExecContext(ctx, upsertSyncState, arg.EntityType, arg.LastPullAt, arg.LastPushAt) - return err -} - -const upsertTask = `-- name: UpsertTask :exec -INSERT INTO project_item_tasks (id, item_id, title, completed, position, created_at) -VALUES (?, ?, ?, ?, ?, ?) -ON CONFLICT(id) DO UPDATE SET - title = excluded.title, - completed = excluded.completed, - position = excluded.position -` - -type UpsertTaskParams struct { - ID string - ItemID string - Title string - Completed int64 - Position int64 - CreatedAt string -} - -func (q *Queries) UpsertTask(ctx context.Context, arg UpsertTaskParams) error { - _, err := q.db.ExecContext(ctx, upsertTask, - arg.ID, - arg.ItemID, - arg.Title, - arg.Completed, - arg.Position, - arg.CreatedAt, - ) - return err -} diff --git a/store/sqlite/open.go b/store/sqlite/open.go index bf1963d..658ba68 100644 --- a/store/sqlite/open.go +++ b/store/sqlite/open.go @@ -23,7 +23,7 @@ var lookupRows string // `PRAGMA foreign_keys=ON` through db.Exec sets it on whichever single // connection served that call and leaves every other one with SQLite's default, // which is off — so the ON DELETE CASCADE on every child table was inert, and a -// project deleted by a sync sweep left its memberships behind as orphans. +// deleted project left its memberships behind as orphans. // // A transaction takes the write lock when it begins, because a rule reads inside // it and then writes. SQLite refuses a deferred transaction's upgrade from read to @@ -67,7 +67,7 @@ func migrate(db *sql.DB) error { var name string err := db.QueryRow("SELECT name FROM sqlite_master WHERE type='table' AND name='project_items'").Scan(&name) if err == sql.ErrNoRows { - // Fresh database — apply full schema (includes sync tables) + // Fresh database — apply full schema if _, err := db.Exec(schema); err != nil { return fmt.Errorf("applying schema: %w", err) } @@ -77,10 +77,7 @@ func migrate(db *sql.DB) error { } // Existing database — apply incremental migrations - if err := migrateSyncTables(db); err != nil { - return err - } - if err := migrateSyncOrigin(db); err != nil { + if err := dropSyncTables(db); err != nil { return err } if err := migrateOrphanedChildRows(db); err != nil { @@ -137,40 +134,28 @@ CREATE TABLE IF NOT EXISTS project_statuses (name TEXT PRIMARY KEY);` return nil } -// migrateSyncOrigin adds the table that pairs a database with its API. Left -// empty for an existing database: which API it belongs to is not derivable from -// anything in it, so the first pull is what records it. -func migrateSyncOrigin(db *sql.DB) error { - var name string - err := db.QueryRow("SELECT name FROM sqlite_master WHERE type='table' AND name='sync_origin'").Scan(&name) - if err == nil { - return nil - } else if err != sql.ErrNoRows { - return fmt.Errorf("checking sync_origin: %w", err) - } - - const create = ` -CREATE TABLE sync_origin ( - id INTEGER PRIMARY KEY CHECK (id = 1), - api_url TEXT NOT NULL, - adopted_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) -);` - if _, err := db.Exec(create); err != nil { - return fmt.Errorf("creating sync_origin: %w", err) +// dropSyncTables removes what a database kept while it was a copy of an API: the +// queue of pushes, the pull times, and the API it was bound to. None of it is a +// project, an item or anything hanging off one, so every row a person wrote stays. +func dropSyncTables(db *sql.DB) error { + const drop = ` +DROP TABLE IF EXISTS pending_sync; +DROP TABLE IF EXISTS sync_state; +DROP TABLE IF EXISTS sync_origin;` + if _, err := db.Exec(drop); err != nil { + return fmt.Errorf("dropping the sync tables: %w", err) } return nil } // migrateOrphanedChildRows deletes rows the cascades would never have allowed: // memberships, dependencies and tasks whose parent is gone. Every database -// written before foreign keys were actually enforced has them — a sync sweep -// that deleted a project left its memberships pointing at nothing, and a real -// database had accumulated 46 of them. +// written before foreign keys were actually enforced has them — a deleted +// project left its memberships pointing at nothing. // // Deleting is not a data loss decision. A membership whose project does not -// exist names no project, so nothing can reach it and nothing can render it; -// were the project to come back, the item pull rebuilds the membership from the -// server's own answer. It runs before migrateProjectStatus because that +// exist names no project, so nothing can reach it and nothing can render it. It +// runs before migrateProjectStatus because that // migration's foreign key check is what these rows fail. func migrateOrphanedChildRows(db *sql.DB) error { sweeps := []struct { @@ -209,9 +194,8 @@ func migrateOrphanedChildRows(db *sql.DB) error { // // The unique constraint is why this is a table rebuild rather than three ALTER // TABLE ADD COLUMNs. SQLite attaches it as an implicit index that no DDL can -// drop, and leaving it would fail the entire pull transaction the first time the -// server returned a completed project beside a live one sharing its name — -// exactly the case the partial index exists to allow. +// drop, and leaving it would refuse a completed project beside a live one +// sharing its name — exactly the case the partial index exists to allow. // // Foreign keys are off for the swap because project_item_memberships references // projects ON DELETE CASCADE, so dropping the old table with them on would take @@ -289,9 +273,8 @@ ALTER TABLE projects_rebuilt RENAME TO projects;` } // migrateItemNumber adds project_items.number to databases created before the -// column existed. Left null rather than backfilled: with sync on the server -// owns the numbering and the next pull fills every row in, and with sync off -// the first create allocates from an empty column, which starts at 1. +// column existed. Left null rather than backfilled, so the first create +// allocates from an empty column, which starts at 1. // // The UNIQUE constraint lives in indexes.sql because ALTER TABLE ADD COLUMN // cannot carry one. @@ -311,7 +294,7 @@ func migrateItemNumber(db *sql.DB) error { // migrateItemRepo adds project_items.repo to databases created before the // column existed. Detected by column presence rather than a version counter, -// matching how migrateSyncTables probes for its tables. +// as every migration here is. func migrateItemRepo(db *sql.DB) error { var name string err := db.QueryRow("SELECT name FROM pragma_table_info('project_items') WHERE name = 'repo'").Scan(&name) @@ -325,32 +308,3 @@ func migrateItemRepo(db *sql.DB) error { } return nil } - -func migrateSyncTables(db *sql.DB) error { - var name string - err := db.QueryRow("SELECT name FROM sqlite_master WHERE type='table' AND name='pending_sync'").Scan(&name) - if err == sql.ErrNoRows { - const syncMigration = ` -CREATE TABLE pending_sync ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - operation TEXT NOT NULL, - entity_type TEXT NOT NULL, - entity_id TEXT NOT NULL, - payload TEXT NOT NULL, - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - attempts INTEGER NOT NULL DEFAULT 0, - last_error TEXT -); -CREATE TABLE sync_state ( - entity_type TEXT PRIMARY KEY, - last_pull_at TEXT NOT NULL DEFAULT '1970-01-01T00:00:00.000Z', - last_push_at TEXT NOT NULL DEFAULT '1970-01-01T00:00:00.000Z' -);` - if _, err := db.Exec(syncMigration); err != nil { - return fmt.Errorf("applying sync migration: %w", err) - } - } else if err != nil { - return fmt.Errorf("checking sync tables: %w", err) - } - return nil -} diff --git a/store/sqlite/open_test.go b/store/sqlite/open_test.go index 3b228f1..4b8ffd3 100644 --- a/store/sqlite/open_test.go +++ b/store/sqlite/open_test.go @@ -115,76 +115,6 @@ func mustExec(t *testing.T, database *sql.DB, query string) { } } -func TestOpen_IncrementalMigration(t *testing.T) { - // Simulate an existing database without sync tables - database, err := sql.Open("sqlite", ":memory:") - if err != nil { - t.Fatalf("opening raw db: %v", err) - } - database.SetMaxOpenConns(1) - - // Create only the core tables (no sync tables) - _, err = database.Exec(` - CREATE TABLE projects ( - id TEXT PRIMARY KEY, - name TEXT NOT NULL UNIQUE, - description TEXT, - position INTEGER NOT NULL DEFAULT 0, - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) - ); - CREATE TABLE project_items ( - id TEXT PRIMARY KEY, - title TEXT NOT NULL, - completed INTEGER NOT NULL DEFAULT 0, - archived INTEGER NOT NULL DEFAULT 0, - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) - ); - `) - if err != nil { - t.Fatalf("creating core tables: %v", err) - } - - // Insert test data - _, err = database.Exec("INSERT INTO projects (id, name) VALUES ('p1', 'existing')") - if err != nil { - t.Fatalf("inserting test data: %v", err) - } - - // Run migration — should add sync tables without losing data - if err := migrate(database); err != nil { - t.Fatalf("migrate: %v", err) - } - - // Verify sync tables exist - var name string - err = database.QueryRow( - "SELECT name FROM sqlite_master WHERE type='table' AND name='pending_sync'", - ).Scan(&name) - if err != nil { - t.Error("pending_sync table not created by migration") - } - - err = database.QueryRow( - "SELECT name FROM sqlite_master WHERE type='table' AND name='sync_state'", - ).Scan(&name) - if err != nil { - t.Error("sync_state table not created by migration") - } - - // Verify existing data survived - var projectName string - err = database.QueryRow("SELECT name FROM projects WHERE id='p1'").Scan(&projectName) - if err != nil { - t.Fatalf("existing data lost: %v", err) - } - if projectName != "existing" { - t.Errorf("expected project name 'existing', got %q", projectName) - } - - _ = database.Close() -} - func TestMigrate_AddsRepoColumnToExistingDatabase(t *testing.T) { database, err := sql.Open("sqlite", ":memory:") if err != nil { @@ -270,61 +200,6 @@ func TestOpen_ExistingDatabaseGainsTheUniquenessRules(t *testing.T) { } } -func TestMigrate_AddsNumberColumnToExistingDatabase(t *testing.T) { - database, err := sql.Open("sqlite", ":memory:") - if err != nil { - t.Fatalf("opening raw db: %v", err) - } - database.SetMaxOpenConns(1) - - _, err = database.Exec(` - CREATE TABLE projects ( - id TEXT PRIMARY KEY, - name TEXT NOT NULL UNIQUE, - description TEXT, - position INTEGER NOT NULL DEFAULT 0, - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) - ); - CREATE TABLE project_items ( - id TEXT PRIMARY KEY, - title TEXT NOT NULL, - notes TEXT, - repo TEXT, - completed INTEGER NOT NULL DEFAULT 0, - archived INTEGER NOT NULL DEFAULT 0, - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) - ); - INSERT INTO project_items (id, title) VALUES ('i1', 'pre-number item'); - `) - if err != nil { - t.Fatalf("creating pre-number table: %v", err) - } - - if err := migrate(database); err != nil { - t.Fatalf("migrate: %v", err) - } - - // Deliberately not backfilled: with sync on the pull supplies every number, - // and with sync off the first create allocates from an empty column. - var number sql.NullInt64 - var title string - row := database.QueryRow("SELECT title, number FROM project_items WHERE id = 'i1'") - if err := row.Scan(&title, &number); err != nil { - t.Fatalf("selecting migrated row: %v", err) - } - if title != "pre-number item" { - t.Errorf("existing row was disturbed: title = %q", title) - } - if number.Valid { - t.Errorf("number should be null for a pre-existing item, got %d", number.Int64) - } - - if err := migrate(database); err != nil { - t.Fatalf("second migrate: %v", err) - } -} - // TestMigrate_ReplacesTheGlobalNameConstraint is the one migration that rebuilds // a table rather than adding a column, because SQLite attaches a column-level // UNIQUE as an index no DDL can drop. Two things have to survive it: the rows, diff --git a/store/sqlite/queries.sql b/store/sqlite/queries.sql index 092e942..90c60ae 100644 --- a/store/sqlite/queries.sql +++ b/store/sqlite/queries.sql @@ -55,15 +55,9 @@ INSERT INTO project_items (id, number, title, notes, repo) VALUES (?, ?, ?, ?, ?) RETURNING *; --- Only used when sync is off and this database assigns its own numbers. With --- sync on the server is the authority and this would hand out a number it is --- about to disagree with. -- name: NextItemNumber :one SELECT COALESCE(MAX(number), 0) + 1 FROM project_items; --- name: SetItemNumber :exec -UPDATE project_items SET number = ? WHERE id = ?; - -- name: UpdateItem :one UPDATE project_items SET title = ?, @@ -175,77 +169,7 @@ RETURNING *; -- name: DeleteTask :exec DELETE FROM project_item_tasks WHERE id = ?; --- Sync: pending operations - --- name: InsertPendingSync :exec -INSERT INTO pending_sync (operation, entity_type, entity_id, payload) -VALUES (?, ?, ?, ?); - --- name: ListPendingSync :many -SELECT * FROM pending_sync ORDER BY id ASC; - --- name: GetOldestPendingSync :one -SELECT * FROM pending_sync ORDER BY id ASC LIMIT 1; - --- name: DeletePendingSync :exec -DELETE FROM pending_sync WHERE id = ?; - --- name: DeletePendingSyncByEntity :execrows -DELETE FROM pending_sync WHERE entity_id = ?; - --- name: UpdatePendingSyncError :exec -UPDATE pending_sync SET attempts = attempts + 1, last_error = ? WHERE id = ?; - --- name: CountPendingSync :one -SELECT COUNT(*) FROM pending_sync; - --- name: DeleteAllPendingSync :exec -DELETE FROM pending_sync; - --- name: MaxPendingSyncID :one -SELECT CAST(COALESCE(MAX(id), 0) AS INTEGER) AS max_id FROM pending_sync; - --- name: DeletePendingSyncUpTo :exec -DELETE FROM pending_sync WHERE id <= ?; - --- name: ListPendingSyncEntityIDsAfter :many -SELECT DISTINCT entity_id FROM pending_sync WHERE id > ?; - --- Sync: state tracking - --- Counted rather than listed: the caller only needs to know whether this --- database has anything to lose. --- name: CountProjects :one -SELECT COUNT(*) FROM projects; - --- name: CountItems :one -SELECT COUNT(*) FROM project_items; - --- Whether this database has ever reconciled with an API. A database that has --- pulled was bound to something, even if nothing recorded what. --- name: HasPulledBefore :one -SELECT COUNT(*) FROM sync_state WHERE last_pull_at > '1970-01-01T00:00:00.000Z'; - --- name: GetSyncOrigin :one -SELECT api_url FROM sync_origin WHERE id = 1; - --- Adoption is deliberate and always overwrites: the caller has already decided --- this database belongs to this API. --- name: SetSyncOrigin :exec -INSERT INTO sync_origin (id, api_url, adopted_at) -VALUES (1, ?, strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) -ON CONFLICT(id) DO UPDATE SET - api_url = excluded.api_url, - adopted_at = excluded.adopted_at; - --- name: UpsertSyncState :exec -INSERT INTO sync_state (entity_type, last_pull_at, last_push_at) -VALUES (?, ?, ?) -ON CONFLICT(entity_type) DO UPDATE SET - last_pull_at = CASE WHEN excluded.last_pull_at > sync_state.last_pull_at THEN excluded.last_pull_at ELSE sync_state.last_pull_at END, - last_push_at = CASE WHEN excluded.last_push_at > sync_state.last_push_at THEN excluded.last_push_at ELSE sync_state.last_push_at END; - --- Undo: a restore carries each column a pull leaves alone. +-- Undo: a restore inserts the whole row, or, where it is still there, rewrites what an edit can change. -- name: RestoreProject :exec INSERT INTO projects (id, name, description, kind, status, status_reason, closed_at, position, created_at) @@ -281,55 +205,10 @@ ON CONFLICT(id) DO UPDATE SET completed_at = excluded.completed_at, position = excluded.position; --- Sync: pull reconciliation (upserts) - --- name: UpsertProject :exec -INSERT INTO projects (id, name, description, status, status_reason, closed_at, position, created_at) -VALUES (?, ?, ?, ?, ?, ?, ?, ?) -ON CONFLICT(id) DO UPDATE SET - name = excluded.name, - description = excluded.description, - status = excluded.status, - status_reason = excluded.status_reason, - closed_at = excluded.closed_at, - position = excluded.position; - --- name: UpsertItem :exec -INSERT INTO project_items (id, number, title, notes, repo, completed, archived, created_at, updated_at) -VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) -ON CONFLICT(id) DO UPDATE SET - number = excluded.number, - title = excluded.title, - notes = excluded.notes, - repo = excluded.repo, - completed = excluded.completed, - archived = excluded.archived, - updated_at = excluded.updated_at; - --- name: UpsertTask :exec -INSERT INTO project_item_tasks (id, item_id, title, completed, position, created_at) -VALUES (?, ?, ?, ?, ?, ?) -ON CONFLICT(id) DO UPDATE SET - title = excluded.title, - completed = excluded.completed, - position = excluded.position; - --- name: UpsertMembership :exec -INSERT INTO project_item_memberships (item_id, project_id, position) -VALUES (?, ?, ?) -ON CONFLICT(item_id, project_id) DO UPDATE SET - position = excluded.position; - -- name: UpsertDependency :exec INSERT OR IGNORE INTO project_item_dependencies (item_id, depends_on_id) VALUES (?, ?); --- name: DeleteAllMemberships :exec -DELETE FROM project_item_memberships; - --- name: DeleteAllDependencies :exec -DELETE FROM project_item_dependencies; - -- name: ListAllProjectsRaw :many SELECT * FROM projects ORDER BY position, name; diff --git a/store/sqlite/schema.sql b/store/sqlite/schema.sql index f9277f1..c8120a8 100644 --- a/store/sqlite/schema.sql +++ b/store/sqlite/schema.sql @@ -13,9 +13,7 @@ CREATE TABLE project_statuses ( CREATE TABLE projects ( id TEXT PRIMARY KEY, -- Unique among ACTIVE projects only, via the partial index in indexes.sql. - -- A global UNIQUE would make a completed `clisteno` own the name forever, - -- and would fail the whole pull the moment the server returns two projects - -- that shared one. + -- A global UNIQUE would make a completed `clisteno` own the name forever. name TEXT NOT NULL, description TEXT, -- A name in project_kinds, checked by the rule that writes it. @@ -23,8 +21,7 @@ CREATE TABLE projects ( -- active | someday | completed | dropped. A project is a finite effort, so -- completing it is what hides it — there is no separate archived flag the way -- items have one. `dropped` requires a reason, which is what stops it reading - -- as deferred. A name in project_statuses, checked by the rule that writes it - -- rather than by a CHECK, so a pull mirrors what a server tells it. + -- as deferred. A name in project_statuses, checked by the rule that writes it. status TEXT NOT NULL DEFAULT 'active', status_reason TEXT, closed_at TEXT, @@ -34,10 +31,9 @@ CREATE TABLE projects ( CREATE TABLE project_items ( id TEXT PRIMARY KEY, - -- The handle: short enough to type, unlike the UUID key. Assigned by the API - -- when sync is on, and locally when it is off and this database is the only - -- authority. Null in the window between creating an item offline and the push - -- that earns it a number. + -- The handle: short enough to type, unlike the UUID key. Assigned as each + -- item is created. Null on an item from before the column, and on one a + -- database once kept as a copy of an API created before the API numbered it. number INTEGER, title TEXT NOT NULL, notes TEXT, @@ -45,8 +41,8 @@ CREATE TABLE project_items ( -- all — home projects, things to sell, errands. repo TEXT, completed INTEGER NOT NULL DEFAULT 0, - -- Null where the completion time is unknown: never finished, finished before - -- this column existed, or finished on a server a pull mirrors. + -- Null where the completion time is unknown: never finished, or finished + -- before this column existed. completed_at TEXT, archived INTEGER NOT NULL DEFAULT 0, created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), @@ -77,33 +73,6 @@ CREATE TABLE project_item_tasks ( created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) ); -CREATE TABLE pending_sync ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - operation TEXT NOT NULL, - entity_type TEXT NOT NULL, - entity_id TEXT NOT NULL, - payload TEXT NOT NULL, - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - attempts INTEGER NOT NULL DEFAULT 0, - last_error TEXT -); - -CREATE TABLE sync_state ( - entity_type TEXT PRIMARY KEY, - last_pull_at TEXT NOT NULL DEFAULT '1970-01-01T00:00:00.000Z', - last_push_at TEXT NOT NULL DEFAULT '1970-01-01T00:00:00.000Z' -); - --- Which API this database belongs to. A pull deletes everything the server did --- not return, so a database and its API are a pair and pointing one at the other --- one is a wipe rather than a sync. Recording the pairing is what lets a pull --- refuse instead of obeying. Single row, enforced by the CHECK. -CREATE TABLE sync_origin ( - id INTEGER PRIMARY KEY CHECK (id = 1), - api_url TEXT NOT NULL, - adopted_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) -); - CREATE TABLE undo_log ( id INTEGER PRIMARY KEY AUTOINCREMENT, action TEXT NOT NULL, diff --git a/store/sqlite/sqlc.yaml b/store/sqlite/sqlc.yaml index 2c189f6..0b92fbb 100644 --- a/store/sqlite/sqlc.yaml +++ b/store/sqlite/sqlc.yaml @@ -6,4 +6,4 @@ sql: gen: go: package: "generated" - out: "generated" + out: "internal/generated" diff --git a/store/sqlite/store.go b/store/sqlite/store.go index dc01986..e0d41b5 100644 --- a/store/sqlite/store.go +++ b/store/sqlite/store.go @@ -11,7 +11,7 @@ import ( "github.com/datapointchris/projects/model" "github.com/datapointchris/projects/store" - "github.com/datapointchris/projects/store/sqlite/generated" + "github.com/datapointchris/projects/store/sqlite/internal/generated" ) // Store is store.Store over a database Open returned. diff --git a/sync/backend.go b/sync/backend.go deleted file mode 100644 index 3536f6f..0000000 --- a/sync/backend.go +++ /dev/null @@ -1,550 +0,0 @@ -package sync - -import ( - "fmt" - - "github.com/datapointchris/projects/backend" - "github.com/datapointchris/projects/model" - "github.com/datapointchris/projects/service" -) - -// SyncBackend wraps the service and queues sync operations after mutations. -// Read methods delegate directly; write methods delegate then queue + notify. -type SyncBackend struct { - local *service.Service - engine *Engine -} - -// Compile-time check that SyncBackend implements Backend. -var _ backend.Backend = (*SyncBackend)(nil) - -// NewSyncBackend creates a backend that writes locally and syncs to the remote API. -func NewSyncBackend(local *service.Service, engine *Engine) *SyncBackend { - return &SyncBackend{local: local, engine: engine} -} - -// --- Read methods: pass through to local --- - -func (s *SyncBackend) ListProjectsByStatus(status string) ([]model.ProjectWithItemCount, error) { - return s.local.ListProjectsByStatus(status) -} - -func (s *SyncBackend) GetProject(id string) (*model.ProjectWithItemCount, error) { - return s.local.GetProject(id) -} - -func (s *SyncBackend) ListAllItemsIncludingArchived() ([]model.ProjectItem, error) { - return s.local.ListAllItemsIncludingArchived() -} - -func (s *SyncBackend) ListItemsByProject(projectID string) ([]model.ProjectItemInProject, error) { - return s.local.ListItemsByProject(projectID) -} - -func (s *SyncBackend) GetItem(id string) (*model.ProjectItemDetail, error) { - return s.local.GetItem(id) -} - -func (s *SyncBackend) GetItemProjects(itemID string) ([]model.Project, error) { - return s.local.GetItemProjects(itemID) -} - -func (s *SyncBackend) GetBlockers(itemID string) ([]model.ProjectItem, error) { - return s.local.GetBlockers(itemID) -} - -func (s *SyncBackend) ListDependencies() ([]model.Dependency, error) { - return s.local.ListDependencies() -} - -func (s *SyncBackend) ListMemberships() ([]model.Membership, error) { - return s.local.ListMemberships() -} - -func (s *SyncBackend) ListTasks(itemID string) ([]model.ProjectItemTask, error) { - return s.local.ListTasks(itemID) -} - -func (s *SyncBackend) ListAllTasks() ([]model.ProjectItemTask, error) { - return s.local.ListAllTasks() -} - -func (s *SyncBackend) Search(query string) ([]model.ProjectItem, error) { - return s.local.Search(query) -} - -func (s *SyncBackend) ListBlocked() ([]model.ProjectItem, error) { - return s.local.ListBlocked() -} - -func (s *SyncBackend) ListArchived(projectID string) ([]model.ProjectItemInProject, error) { - return s.local.ListArchived(projectID) -} - -func (s *SyncBackend) CanUndo() (bool, error) { - return s.local.CanUndo() -} - -// --- Write methods: delegate to local, then queue + notify --- - -func (s *SyncBackend) CreateProject(input model.CreateProject) (*model.Project, error) { - result, err := s.local.CreateProject(input) - if err != nil { - return nil, err - } - _ = s.engine.QueueOp(OpCreateProject, result.ID, createProjectPayload{ - ID: result.ID, - Name: input.Name, - Description: input.Description, - Status: result.Status, - Reason: result.StatusReason, - }) - s.engine.Notify() - return result, nil -} - -func (s *SyncBackend) UpdateProject(id string, input model.UpdateProject) (*model.Project, error) { - result, err := s.local.UpdateProject(id, input) - if err != nil { - return nil, err - } - _ = s.engine.QueueOp(OpUpdateProject, id, input) - s.engine.Notify() - return result, nil -} - -// SetProjectStatus queues a PATCH rather than a complete/drop/reopen action -// endpoint, matching the API: one write path, so the transition is validated in -// one place. closed_at is left out of the payload because the server derives it -// from the status, exactly as the local statement does. -func (s *SyncBackend) SetProjectStatus(id, status string, reason *string) (*model.Project, error) { - result, err := s.local.SetProjectStatus(id, status, reason) - if err != nil { - return nil, err - } - _ = s.engine.QueueOp(OpUpdateProject, id, projectStatusPayload{ - Status: status, - Reason: result.StatusReason, - }) - s.engine.Notify() - return result, nil -} - -func (s *SyncBackend) DeleteProject(id string) error { - if err := s.local.DeleteProject(id); err != nil { - return err - } - _ = s.engine.QueueOp(OpDeleteProject, id, nil) - s.engine.Notify() - return nil -} - -func (s *SyncBackend) ReorderProject(projectID string, newPosition int) error { - if err := s.local.ReorderProject(projectID, newPosition); err != nil { - return err - } - _ = s.engine.QueueOp(OpReorderProject, projectID, projectReorderPayload{ - Position: newPosition, - }) - s.engine.Notify() - return nil -} - -func (s *SyncBackend) CreateItem(input model.CreateProjectItem) (*model.ProjectItemDetail, error) { - result, err := s.local.CreateItem(input) - if err != nil { - return nil, err - } - _ = s.engine.QueueOp(OpCreateItem, result.ID, createItemPayload{ - ID: result.ID, - Title: input.Title, - Notes: input.Notes, - Repo: input.Repo, - ProjectIDs: input.ProjectIDs, - }) - s.engine.Notify() - return result, nil -} - -func (s *SyncBackend) UpdateItem(id string, input model.UpdateProjectItem) (*model.ProjectItem, error) { - result, err := s.local.UpdateItem(id, input) - if err != nil { - return nil, err - } - _ = s.engine.QueueOp(OpUpdateItem, id, input) - s.engine.Notify() - return result, nil -} - -func (s *SyncBackend) DeleteItem(id string) error { - if err := s.local.DeleteItem(id); err != nil { - return err - } - _ = s.engine.QueueOp(OpDeleteItem, id, nil) - s.engine.Notify() - return nil -} - -func (s *SyncBackend) ReorderItem(itemID, projectID string, newPosition int) error { - if err := s.local.ReorderItem(itemID, projectID, newPosition); err != nil { - return err - } - _ = s.engine.QueueOp(OpReorderItem, itemID, reorderPayload{ - ProjectID: projectID, - Position: newPosition, - }) - s.engine.Notify() - return nil -} - -func (s *SyncBackend) AddToProject(itemID, projectID string) error { - if err := s.local.AddToProject(itemID, projectID); err != nil { - return err - } - _ = s.engine.QueueOp(OpAddToProject, itemID, projectIDPayload{ProjectID: projectID}) - s.engine.Notify() - return nil -} - -func (s *SyncBackend) RemoveFromProject(itemID, projectID string) error { - if err := s.local.RemoveFromProject(itemID, projectID); err != nil { - return err - } - _ = s.engine.QueueOp(OpRemoveFromProject, itemID, projectIDPayload{ProjectID: projectID}) - s.engine.Notify() - return nil -} - -func (s *SyncBackend) AddDependency(itemID, dependsOn string) error { - if err := s.local.AddDependency(itemID, dependsOn); err != nil { - return err - } - _ = s.engine.QueueOp(OpAddDependency, itemID, depPayload{DependsOnID: dependsOn}) - s.engine.Notify() - return nil -} - -func (s *SyncBackend) RemoveDependency(itemID, dependsOn string) error { - if err := s.local.RemoveDependency(itemID, dependsOn); err != nil { - return err - } - _ = s.engine.QueueOp(OpRemoveDependency, itemID, depPayload{DependsOnID: dependsOn}) - s.engine.Notify() - return nil -} - -func (s *SyncBackend) CreateTask(itemID string, input model.CreateProjectItemTask) (*model.ProjectItemTask, error) { - result, err := s.local.CreateTask(itemID, input) - if err != nil { - return nil, err - } - _ = s.engine.QueueOp(OpCreateTask, result.ID, taskPayload{ - ID: result.ID, - ItemID: itemID, - Title: input.Title, - }) - s.engine.Notify() - return result, nil -} - -func (s *SyncBackend) UpdateTask(itemID, taskID string, input model.UpdateProjectItemTask) (*model.ProjectItemTask, error) { - result, err := s.local.UpdateTask(itemID, taskID, input) - if err != nil { - return nil, err - } - _ = s.engine.QueueOp(OpUpdateTask, taskID, taskUpdatePayload{ - ItemID: itemID, - Title: input.Title, - Completed: input.Completed, - Position: input.Position, - }) - s.engine.Notify() - return result, nil -} - -func (s *SyncBackend) DeleteTask(itemID, taskID string) error { - if err := s.local.DeleteTask(itemID, taskID); err != nil { - return err - } - _ = s.engine.QueueOp(OpDeleteTask, taskID, taskRefPayload{ItemID: itemID}) - s.engine.Notify() - return nil -} - -// Undo reverses the local change and then reconciles the sync queue with it. -// Treating undo as local-only leaves the queued operations for that entity in -// place, so the next push recreates on the server exactly what was just undone. -func (s *SyncBackend) Undo() (*model.UndoResult, error) { - result, err := s.local.Undo() - if err != nil { - return nil, err - } - - // Memberships, dependencies, and item reorders all record the item as their - // entity ID, so dropping by entity ID would take unrelated queued edits to - // that item with them. Queue the inverse operation instead — a push that - // finds nothing to reverse gets a 404 or 409, which the push loop already - // treats as settled. - switch result.EntityType { - case "membership", "dependency", "item_position", "project_position": - s.queueUndoInverse(result) - s.engine.Notify() - return result, nil - } - - // Anything still queued describes the state the undo just reversed. - dropped, dropErr := s.engine.DropOpsForEntity(result.EntityID) - if dropErr != nil { - return result, fmt.Errorf("reconciling sync queue: %w", dropErr) - } - - if result.EntityType == "task" { - s.queueTaskUndo(result, dropped) - s.engine.Notify() - return result, nil - } - - // Undoing a delete when the delete had already been pushed means the row - // is gone from the server, so it has to be recreated rather than updated — - // and recreated with everything that cascaded off it, since a pull rebuilds - // memberships and dependencies from server state alone. - deleteWasPushed := result.Action == "delete" && dropped == 0 - - if result.EntityType == "project" { - switch { - case result.RestoredProject != nil && deleteWasPushed: - restored := result.RestoredProject - _ = s.engine.QueueOp(OpCreateProject, result.EntityID, createProjectPayload{ - ID: restored.ID, - Name: restored.Name, - Description: restored.Description, - Status: restored.Status, - Reason: restored.StatusReason, - }) - for _, itemID := range result.RestoredItemIDs { - _ = s.engine.QueueOp(OpAddToProject, itemID, projectIDPayload{ProjectID: result.EntityID}) - } - case result.RestoredProject != nil: - restored := result.RestoredProject - // One PATCH carrying the whole restored row, status included: the - // undo may have reversed a complete or a drop, and a body that - // omitted status would leave the server showing the project as - // closed after it came back locally. - _ = s.engine.QueueOp(OpUpdateProject, result.EntityID, projectRestorePayload{ - Name: restored.Name, - Description: restored.Description, - Position: restored.Position, - Status: restored.Status, - Reason: restored.StatusReason, - }) - case dropped == 0: - _ = s.engine.QueueOp(OpDeleteProject, result.EntityID, nil) - } - s.engine.Notify() - return result, nil - } - - switch { - case result.Restored != nil && deleteWasPushed: - restored := result.Restored - _ = s.engine.QueueOp(OpCreateItem, result.EntityID, createItemPayload{ - ID: restored.ID, - Title: restored.Title, - Notes: restored.Notes, - Repo: restored.Repo, - ProjectIDs: result.RestoredProjectIDs, - }) - s.queueRestoredChildren(result) - case result.Restored != nil: - // The row is back; push its restored state. Idempotent whether or not - // the reversed change had already been pushed. - restored := result.Restored - unlinked := "" - repo := restored.Repo - if repo == nil { - // A nil Repo is omitted from the payload and would leave a link the - // undo removed. Empty string is how this API unlinks one. - repo = &unlinked - } - _ = s.engine.QueueOp(OpUpdateItem, result.EntityID, model.UpdateProjectItem{ - Title: &restored.Title, - Notes: restored.Notes, - Repo: repo, - Completed: &restored.Completed, - Archived: &restored.Archived, - }) - case dropped == 0: - // The row is gone locally and its create had already been pushed, so - // the server still has it. With a queued create there is nothing on the - // server to delete. - _ = s.engine.QueueOp(OpDeleteItem, result.EntityID, nil) - } - - s.engine.Notify() - return result, nil -} - -// queueUndoInverse pushes the reversal of a membership, dependency, or reorder -// undo. Detail carries which way the reversal went; Removed means the undo took -// the row away rather than putting it back. -func (s *SyncBackend) queueUndoInverse(result *model.UndoResult) { - detail := result.Detail - if detail == nil { - return - } - - switch result.EntityType { - case "membership": - if detail.Removed { - _ = s.engine.QueueOp(OpRemoveFromProject, detail.ItemID, projectIDPayload{ProjectID: detail.ProjectID}) - return - } - _ = s.engine.QueueOp(OpAddToProject, detail.ItemID, projectIDPayload{ProjectID: detail.ProjectID}) - - case "dependency": - if detail.Removed { - _ = s.engine.QueueOp(OpRemoveDependency, detail.ItemID, depPayload{DependsOnID: detail.DependsOnID}) - return - } - _ = s.engine.QueueOp(OpAddDependency, detail.ItemID, depPayload{DependsOnID: detail.DependsOnID}) - - case "item_position": - _ = s.engine.QueueOp(OpReorderItem, detail.ItemID, reorderPayload{ - ProjectID: detail.ProjectID, - Position: detail.Position, - }) - - case "project_position": - _ = s.engine.QueueOp(OpReorderProject, result.EntityID, projectReorderPayload{Position: detail.Position}) - } -} - -// queueTaskUndo pushes the reversal of a task undo. dropped counts operations -// still queued for the task, so a non-zero count means the change being undone -// never reached the server and needs no reversal there. -func (s *SyncBackend) queueTaskUndo(result *model.UndoResult, dropped int64) { - detail := result.Detail - if detail == nil { - return - } - - switch { - case detail.Removed: - if dropped == 0 { - _ = s.engine.QueueOp(OpDeleteTask, result.EntityID, taskRefPayload{ItemID: detail.ItemID}) - } - - case result.Action == "delete": - if dropped > 0 || detail.Task == nil { - return - } - task := detail.Task - _ = s.engine.QueueOp(OpCreateTask, task.ID, taskPayload{ID: task.ID, ItemID: task.ItemID, Title: task.Title}) - if task.Completed { - _ = s.engine.QueueOp(OpCompleteTask, task.ID, taskRefPayload{ItemID: task.ItemID}) - } - - case detail.Task != nil: - task := detail.Task - position := task.Position - _ = s.engine.QueueOp(OpUpdateTask, task.ID, taskUpdatePayload{ - ItemID: task.ItemID, - Title: &task.Title, - Completed: &task.Completed, - Position: &position, - }) - } -} - -// queueRestoredChildren pushes the tasks and dependencies that came back with -// a restored item. The item's own create carries its memberships. -func (s *SyncBackend) queueRestoredChildren(result *model.UndoResult) { - for _, task := range result.RestoredTasks { - _ = s.engine.QueueOp(OpCreateTask, task.ID, taskPayload{ - ID: task.ID, - ItemID: task.ItemID, - Title: task.Title, - }) - if task.Completed { - _ = s.engine.QueueOp(OpCompleteTask, task.ID, taskRefPayload{ItemID: task.ItemID}) - } - } - for _, dependency := range result.RestoredDependencies { - _ = s.engine.QueueOp(OpAddDependency, dependency.ItemID, depPayload{ - DependsOnID: dependency.DependsOnID, - }) - } -} - -// --- Payload types for JSON serialization into pending_sync --- - -type createProjectPayload struct { - ID string `json:"id"` - Name string `json:"name"` - Description *string `json:"description,omitempty"` - // Sent even when active so a project completed offline does not come back - // alive on the push that first tells the server it exists. - Status string `json:"status"` - Reason *string `json:"status_reason,omitempty"` -} - -// projectStatusPayload is the PATCH body for a transition. Reason is whatever -// the local statement settled on — the value sent when dropping, and null after -// a reopen cleared it — so the two databases agree without a second round trip. -type projectStatusPayload struct { - Status string `json:"status"` - Reason *string `json:"status_reason"` -} - -// projectRestorePayload is an undo's PATCH: the whole row as it was, so one -// request reverses whichever field the undone mutation had changed. -type projectRestorePayload struct { - Name string `json:"name"` - Description *string `json:"description"` - Position int `json:"position"` - Status string `json:"status"` - Reason *string `json:"status_reason"` -} - -type createItemPayload struct { - ID string `json:"id"` - Title string `json:"title"` - Notes *string `json:"notes,omitempty"` - Repo *string `json:"repo,omitempty"` - ProjectIDs []string `json:"project_ids"` -} - -type reorderPayload struct { - ProjectID string `json:"project_id"` - Position int `json:"position"` -} - -type projectReorderPayload struct { - Position int `json:"position"` -} - -type projectIDPayload struct { - ProjectID string `json:"project_id"` -} - -type depPayload struct { - DependsOnID string `json:"depends_on_id"` -} - -// taskPayload carries the local id, because a later update names the task by it. -type taskPayload struct { - ID string `json:"id"` - ItemID string `json:"item_id"` - Title string `json:"title"` -} - -type taskUpdatePayload struct { - ItemID string `json:"item_id"` - Title *string `json:"title,omitempty"` - Completed *bool `json:"completed,omitempty"` - Position *int `json:"position,omitempty"` -} - -type taskRefPayload struct { - ItemID string `json:"item_id"` -} diff --git a/sync/engine.go b/sync/engine.go deleted file mode 100644 index f8a2869..0000000 --- a/sync/engine.go +++ /dev/null @@ -1,276 +0,0 @@ -package sync - -import ( - "context" - "database/sql" - "encoding/json" - "errors" - "fmt" - "log" - "net/http" - "strings" - gosync "sync" - "time" - - "github.com/datapointchris/projects/store/sqlite/generated" -) - -// apiPrefix is the API version every path and decoder here is written against. -const apiPrefix = "/api/v1" - -// Engine orchestrates push and pull synchronization with the remote API. -type Engine struct { - db *sql.DB - q *generated.Queries - client *http.Client - apiURL string - base string - - mu gosync.Mutex - status SyncStatus - syncDepth int - - // drainMu serializes drainPendingOps. It reads the oldest queued op, pushes - // it, and only then deletes it, so two drains at once both read the same op - // and both push it — a 201 followed by a 409 for every create, with the - // server burning an item number on the insert that lost. Two can overlap - // easily: Notify wakes the push loop on the same mutation that a caller then - // flushes for, and the retry ticker fires regardless of either. - drainMu gosync.Mutex - - pushCh chan struct{} - retryInterval time.Duration - ctx context.Context - cancel context.CancelFunc - done chan struct{} -} - -// Option customizes an Engine at construction. -type Option func(*Engine) - -// WithPushRetryInterval overrides how often the push loop re-attempts a queue it -// failed to drain. Tests use it to avoid waiting out the production interval. -func WithPushRetryInterval(d time.Duration) Option { - return func(e *Engine) { e.retryInterval = d } -} - -// New creates a sync engine. Call Start() to launch the background push loop. -// A non-empty tokenCommand is split on spaces and run without a shell, and every -// request carries the bearer token it prints (tokenTransport). Empty sends no -// Authorization header. -func New(db *sql.DB, apiURL, tokenCommand string, opts ...Option) *Engine { - ctx, cancel := context.WithCancel(context.Background()) - client := &http.Client{Timeout: 10 * time.Second} - if argv := strings.Fields(tokenCommand); len(argv) > 0 { - client.Transport = newTokenTransport(argv, http.DefaultTransport) - } - e := &Engine{ - db: db, - q: generated.New(db), - client: client, - apiURL: apiURL, - base: strings.TrimRight(apiURL, "/") + apiPrefix, - pushCh: make(chan struct{}, 1), - retryInterval: defaultPushRetryInterval, - ctx: ctx, - cancel: cancel, - done: make(chan struct{}), - } - for _, opt := range opts { - opt(e) - } - return e -} - -// APIURL returns the configured address of the server this engine syncs with. -func (e *Engine) APIURL() string { return e.apiURL } - -// Start launches the background push loop goroutine. -func (e *Engine) Start() { - go func() { - defer close(e.done) - e.pushLoop() - }() -} - -// Stop cancels the background loop and waits for it to finish. Callers close the -// database right after this returns, so returning early leaves the loop mid-query -// against a closed handle, which fails with "sql: database is closed" on exit. -// The timeout keeps an in-flight HTTP push from hanging exit. -func (e *Engine) Stop() { - e.cancel() - select { - case <-e.done: - case <-time.After(2 * time.Second): - } -} - -// QueueOp inserts a pending sync operation into the database. -func (e *Engine) QueueOp(op OpType, entityID string, payload any) error { - data, err := json.Marshal(payload) - if err != nil { - return fmt.Errorf("marshaling sync payload: %w", err) - } - return e.q.InsertPendingSync(e.ctx, generated.InsertPendingSyncParams{ - Operation: string(op), - EntityType: op.entityType(), - EntityID: entityID, - Payload: string(data), - }) -} - -// DropOpsForEntity removes every queued operation for one entity and reports how -// many were dropped. Used when a local change is reversed: the queued operations -// describe a state that no longer exists, and pushing them would recreate it on -// the server. A non-zero count also means nothing for that entity reached the -// server yet, since operations are deleted once pushed. -func (e *Engine) DropOpsForEntity(entityID string) (int64, error) { - return e.q.DeletePendingSyncByEntity(e.ctx, entityID) -} - -// Notify signals the push loop to wake up and process pending operations. -// Non-blocking: if the push loop is already signaled, this is a no-op. -func (e *Engine) Notify() { - select { - case e.pushCh <- struct{}{}: - default: - } -} - -// Status returns the current sync status (thread-safe). -func (e *Engine) Status() SyncStatus { - e.mu.Lock() - defer e.mu.Unlock() - - count, err := e.q.CountPendingSync(e.ctx) - if err != nil { - return e.status - } - e.status.PendingCount = int(count) - return e.status -} - -func (e *Engine) setStatus(fn func(s *SyncStatus)) { - e.mu.Lock() - defer e.mu.Unlock() - fn(&e.status) -} - -// beginSync/endSync track sync depth rather than a bare flag: Pull calls -// drainPendingOps inside its own window, and a bool would clear on the inner -// call's return, reporting SYNCED while a full pull was still in flight. -func (e *Engine) beginSync() { - e.mu.Lock() - defer e.mu.Unlock() - e.syncDepth++ - e.status.Syncing = true -} - -func (e *Engine) endSync() { - e.mu.Lock() - defer e.mu.Unlock() - e.syncDepth-- - if e.syncDepth <= 0 { - e.syncDepth = 0 - e.status.Syncing = false - } -} - -// defaultPushRetryInterval is how often the push loop re-attempts a queue it -// failed to drain. Notify only fires on a local mutation, so without this a push -// that failed while the API was unreachable stayed queued until the user -// happened to edit something else — the queue never healed on its own. -const defaultPushRetryInterval = 30 * time.Second - -func (e *Engine) pushLoop() { - retry := time.NewTicker(e.retryInterval) - defer retry.Stop() - - for { - select { - case <-e.ctx.Done(): - return - case <-e.pushCh: - e.drainPendingOps() - case <-retry.C: - e.retryPendingOps() - } - } -} - -// retryPendingOps drains only when something is actually queued. An unconditional -// drain would report Connected on an empty queue without having reached the -// network, clearing a pull failure the status bar should still be showing. -func (e *Engine) retryPendingOps() { - count, err := e.q.CountPendingSync(e.ctx) - if err != nil || count == 0 { - return - } - e.drainPendingOps() -} - -func (e *Engine) drainPendingOps() { - e.drainMu.Lock() - defer e.drainMu.Unlock() - - e.beginSync() - defer e.endSync() - - // The push loop runs unattended, so this is the only thing standing between - // a mispointed engine and one database's work landing in another's API. - // Nothing is lost by stopping: the ops stay queued for a correctly pointed - // engine to drain. - if err := e.checkOriginMatch(e.ctx); err != nil { - e.setStatus(func(s *SyncStatus) { - s.Connected = false - s.LastError = err.Error() - }) - return - } - - for { - if e.ctx.Err() != nil { - return - } - - op, err := e.q.GetOldestPendingSync(e.ctx) - if err == sql.ErrNoRows { - e.setStatus(func(s *SyncStatus) { - s.Connected = true - s.LastError = "" - }) - return - } - if err != nil { - // Shutdown cancels the context mid-query. That is the normal exit - // path, not a fault worth printing on every invocation. - if !errors.Is(err, context.Canceled) { - log.Printf("sync: reading pending op: %v", err) - } - return - } - - if err := e.executePush(op); err != nil { - e.setStatus(func(s *SyncStatus) { - s.Connected = false - s.LastError = err.Error() - }) - _ = e.q.UpdatePendingSyncError(e.ctx, generated.UpdatePendingSyncErrorParams{ - LastError: sql.NullString{String: err.Error(), Valid: true}, - ID: op.ID, - }) - // Back off briefly, then exit loop. Will retry on next Notify. - select { - case <-time.After(2 * time.Second): - case <-e.ctx.Done(): - } - return - } - - _ = e.q.DeletePendingSync(e.ctx, op.ID) - e.setStatus(func(s *SyncStatus) { - s.Connected = true - s.LastError = "" - }) - } -} diff --git a/sync/engine_test.go b/sync/engine_test.go deleted file mode 100644 index 6eb0357..0000000 --- a/sync/engine_test.go +++ /dev/null @@ -1,1074 +0,0 @@ -package sync_test - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "strings" - goSync "sync" - "testing" - "time" - - "github.com/datapointchris/projects/model" - "github.com/datapointchris/projects/service" - "github.com/datapointchris/projects/store/sqlite" - "github.com/datapointchris/projects/sync" -) - -func setupSync(t *testing.T, handler http.Handler) (*sync.SyncBackend, *sync.Engine) { - t.Helper() - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - t.Cleanup(func() { _ = database.Close() }) - - ts := httptest.NewServer(handler) - t.Cleanup(ts.Close) - - local := service.New(sqlite.New(database)) - engine := sync.New(database, ts.URL, "") - engine.Start() - t.Cleanup(engine.Stop) - - sb := sync.NewSyncBackend(local, engine) - return sb, engine -} - -func TestSyncBackend_ReadsDelegateToLocal(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - }) - sb, _ := setupSync(t, handler) - - // Create a project - _, err := sb.CreateProject(model.CreateProject{Name: "LocalRead"}) - if err != nil { - t.Fatalf("CreateProject: %v", err) - } - - // Read should come from local DB immediately - projects, err := sb.ListProjectsByStatus(model.StatusActive) - if err != nil { - t.Fatalf("ListProjectsByStatus: %v", err) - } - if len(projects) != 1 { - t.Fatalf("got %d projects, want 1", len(projects)) - } - if projects[0].Name != "LocalRead" { - t.Errorf("got name %q, want LocalRead", projects[0].Name) - } -} - -func TestSyncBackend_PushRetryOnNetworkError(t *testing.T) { - attempts := 0 - handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - attempts++ - if attempts <= 1 { - w.WriteHeader(http.StatusInternalServerError) - return - } - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - }) - sb, engine := setupSync(t, handler) - - _, err := sb.CreateProject(model.CreateProject{Name: "Retry"}) - if err != nil { - t.Fatalf("CreateProject: %v", err) - } - - // First push will fail (500), wait for backoff + retry - time.Sleep(4 * time.Second) - - // Manually trigger a retry - engine.Notify() - time.Sleep(500 * time.Millisecond) - - status := engine.Status() - if status.PendingCount != 0 { - t.Errorf("expected 0 pending after retry, got %d", status.PendingCount) - } -} - -func TestSyncBackend_PushDropsOn409(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusConflict) - _, _ = w.Write([]byte(`{"detail":"duplicate"}`)) - }) - sb, engine := setupSync(t, handler) - - _, err := sb.CreateProject(model.CreateProject{Name: "Conflict"}) - if err != nil { - t.Fatalf("CreateProject: %v", err) - } - - // 409 should be treated as "drop" — pending count goes to 0 - time.Sleep(200 * time.Millisecond) - - status := engine.Status() - if status.PendingCount != 0 { - t.Errorf("expected 0 pending (409 should drop), got %d", status.PendingCount) - } -} - -// callRecorder captures what a test's mock server received. The push loop -// runs on its own goroutine, so every read and write of the log needs the -// lock — asserting on an unguarded slice is a data race. -type callRecorder struct { - mu goSync.Mutex - calls []string -} - -func (r *callRecorder) handler() http.HandlerFunc { - return func(w http.ResponseWriter, req *http.Request) { - r.mu.Lock() - r.calls = append(r.calls, req.Method+" "+req.URL.Path) - r.mu.Unlock() - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - } -} - -func (r *callRecorder) snapshot() []string { - r.mu.Lock() - defer r.mu.Unlock() - return append([]string(nil), r.calls...) -} - -func (r *callRecorder) count() int { - r.mu.Lock() - defer r.mu.Unlock() - return len(r.calls) -} - -func TestSyncBackend_ItemLifecycle(t *testing.T) { - recorder := &callRecorder{} - sb, _ := setupSync(t, recorder.handler()) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Items"}) - item, err := sb.CreateItem(model.CreateProjectItem{ - Title: "Test", - ProjectIDs: []string{p.ID}, - }) - if err != nil { - t.Fatalf("CreateItem: %v", err) - } - - done := true - _, err = sb.UpdateItem(item.ID, model.UpdateProjectItem{Completed: &done}) - if err != nil { - t.Fatalf("UpdateItem: %v", err) - } - - if err := sb.DeleteItem(item.ID); err != nil { - t.Fatalf("DeleteItem: %v", err) - } - - // Wait for push loop to drain - time.Sleep(500 * time.Millisecond) - - // Verify the HTTP calls were made in order - if recorder.count() < 4 { - t.Fatalf("expected at least 4 HTTP calls, got %d: %v", recorder.count(), recorder.snapshot()) - } -} - -func TestSyncBackend_TaskOperations(t *testing.T) { - recorder := &callRecorder{} - sb, _ := setupSync(t, recorder.handler()) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Tasks"}) - item, _ := sb.CreateItem(model.CreateProjectItem{ - Title: "WithTasks", - ProjectIDs: []string{p.ID}, - }) - - task, err := sb.CreateTask(item.ID, model.CreateProjectItemTask{Title: "Sub"}) - if err != nil { - t.Fatalf("CreateTask: %v", err) - } - - done := true - if _, err := sb.UpdateTask(item.ID, task.ID, model.UpdateProjectItemTask{Completed: &done}); err != nil { - t.Fatalf("UpdateTask: %v", err) - } - - if err := sb.DeleteTask(item.ID, task.ID); err != nil { - t.Fatalf("DeleteTask: %v", err) - } - - time.Sleep(500 * time.Millisecond) - - if recorder.count() < 5 { - t.Fatalf("expected at least 5 HTTP calls, got %d: %v", recorder.count(), recorder.snapshot()) - } -} - -// A person adds a task and checks it off before the next pull. The check names the -// local id, so the server has to hold the task under it, or a 404 drops the check and -// the pull brings the task back unchecked. -func TestATaskCheckedOffBeforeThePullReachesTheServerUnderItsOwnID(t *testing.T) { - createdAs := make(chan string, 8) - recorder := &callRecorder{} - sb, _ := setupSync(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/tasks") { - var body struct { - ID string `json:"id"` - } - _ = json.NewDecoder(r.Body).Decode(&body) - createdAs <- body.ID - } - recorder.handler()(w, r) - })) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Tasks"}) - item, _ := sb.CreateItem(model.CreateProjectItem{Title: "WithTasks", ProjectIDs: []string{p.ID}}) - task, err := sb.CreateTask(item.ID, model.CreateProjectItemTask{Title: "Sub"}) - if err != nil { - t.Fatalf("CreateTask: %v", err) - } - - select { - case got := <-createdAs: - if got != task.ID { - t.Errorf("the server was asked to create task %q, and the TUI holds it as %q", got, task.ID) - } - case <-time.After(5 * time.Second): - t.Fatal("the task's create never reached the server") - } -} - -func TestSyncBackend_UndoAfterPushDeletesRemotely(t *testing.T) { - recorder := &callRecorder{} - sb, _ := setupSync(t, recorder.handler()) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Undo"}) - item, _ := sb.CreateItem(model.CreateProjectItem{ - Title: "Undoable", - ProjectIDs: []string{p.ID}, - }) - - // Let the create reach the server, so the server holds a row the undo removes. - time.Sleep(300 * time.Millisecond) - - if _, err := sb.Undo(); err != nil { - t.Fatalf("Undo: %v", err) - } - time.Sleep(300 * time.Millisecond) - - want := "DELETE /api/v1/project-items/" + item.ID - calls := recorder.snapshot() - for _, call := range calls { - if call == want { - return - } - } - t.Errorf("expected %q after undoing an already-pushed create, got %v", want, calls) -} - -func TestSyncBackend_UndoBeforePushDropsQueuedCreate(t *testing.T) { - // Every push fails, so operations stay queued and nothing reaches the server. - handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusInternalServerError) - }) - sb, engine := setupSync(t, handler) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Undo"}) - item, _ := sb.CreateItem(model.CreateProjectItem{ - Title: "Undoable", - ProjectIDs: []string{p.ID}, - }) - - if _, err := sb.Undo(); err != nil { - t.Fatalf("Undo: %v", err) - } - - // The queued create described a row that no longer exists. Nothing should be - // left for this item — not the create, and not a delete for a row the server - // never received. - remaining, err := engine.DropOpsForEntity(item.ID) - if err != nil { - t.Fatalf("checking queue: %v", err) - } - if remaining != 0 { - t.Errorf("expected no queued operations for the undone item, found %d", remaining) - } -} - -func TestPull_Reconciles(t *testing.T) { - // Mock API server that returns test data - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/api/v1/projects": - _ = json.NewEncoder(w).Encode([]model.Project{ - {ID: "proj-1", Name: "Pulled", Position: 0, CreatedAt: time.Now()}, - }) - case "/api/v1/project-items": - _ = json.NewEncoder(w).Encode([]model.ProjectItem{ - {ID: "01900000-0000-7000-8000-00000000a001", Title: "Pulled Item", CreatedAt: time.Now(), UpdatedAt: time.Now()}, - }) - case "/api/v1/project-items/01900000-0000-7000-8000-00000000a001": - _ = json.NewEncoder(w).Encode(model.ProjectItemDetail{ - ProjectItem: model.ProjectItem{ID: "01900000-0000-7000-8000-00000000a001", Title: "Pulled Item", CreatedAt: time.Now(), UpdatedAt: time.Now()}, - Projects: []model.Project{{ID: "proj-1", Name: "Pulled"}}, - }) - case "/api/v1/project-items/01900000-0000-7000-8000-00000000a001/tasks": - _ = json.NewEncoder(w).Encode([]model.ProjectItemTask{ - {ID: "task-1", ItemID: "01900000-0000-7000-8000-00000000a001", Title: "Pulled Task", CreatedAt: time.Now()}, - }) - default: - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - } - }) - - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - defer func() { _ = database.Close() }() - - ts := httptest.NewServer(handler) - defer ts.Close() - - engine := sync.New(database, ts.URL, "") - - // Pull should reconcile server data into local DB - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("Pull: %v", err) - } - - // Verify data was pulled into local DB - local := service.New(sqlite.New(database)) - - projects, err := local.ListProjectsByStatus(model.StatusActive) - if err != nil { - t.Fatalf("ListProjectsByStatus: %v", err) - } - if len(projects) != 1 || projects[0].Name != "Pulled" { - t.Errorf("expected 1 project 'Pulled', got %v", projects) - } - - item, err := local.GetItem("01900000-0000-7000-8000-00000000a001") - if err != nil { - t.Fatalf("GetItem: %v", err) - } - if item.Title != "Pulled Item" { - t.Errorf("got title %q, want 'Pulled Item'", item.Title) - } - - tasks, err := local.ListTasks("01900000-0000-7000-8000-00000000a001") - if err != nil { - t.Fatalf("ListTasks: %v", err) - } - if len(tasks) != 1 || tasks[0].Title != "Pulled Task" { - t.Errorf("expected 1 task 'Pulled Task', got %v", tasks) - } -} - -// Undoing an already-pushed delete has to recreate the row on the server, not -// update one the server no longer has — and recreate its memberships, tasks, -// and dependencies with it, because a pull rebuilds those from server state -// and would otherwise erase the restore. -func TestSyncBackend_UndoOfPushedDeleteRecreatesRemotely(t *testing.T) { - recorder := &callRecorder{} - sb, _ := setupSync(t, recorder.handler()) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Undo"}) - item, _ := sb.CreateItem(model.CreateProjectItem{ - Title: "Doomed", - ProjectIDs: []string{p.ID}, - }) - if _, err := sb.CreateTask(item.ID, model.CreateProjectItemTask{Title: "step"}); err != nil { - t.Fatalf("CreateTask: %v", err) - } - if err := sb.DeleteItem(item.ID); err != nil { - t.Fatalf("DeleteItem: %v", err) - } - - // Let the delete reach the server before undoing it, then look only at - // what the undo itself pushed — the original create hit the same paths. - time.Sleep(400 * time.Millisecond) - beforeUndo := recorder.count() - - if _, err := sb.Undo(); err != nil { - t.Fatalf("Undo: %v", err) - } - time.Sleep(400 * time.Millisecond) - - calls := recorder.snapshot()[beforeUndo:] - - var sawItemCreate, sawTaskCreate bool - for _, call := range calls { - switch call { - case "POST /api/v1/project-items": - sawItemCreate = true - case "POST /api/v1/project-items/" + item.ID + "/tasks": - sawTaskCreate = true - } - } - if !sawItemCreate { - t.Errorf("expected the restored item recreated on the server, got %v", calls) - } - if !sawTaskCreate { - t.Errorf("expected the restored sub-task recreated on the server, got %v", calls) - } -} - -// Membership, dependency, and reorder undos all record the item as their -// entity ID. Reconciling them by dropping queued ops for that ID would take -// unrelated edits with it, and falling through to the item branch would push -// a delete for an item that is still there. -func TestSyncBackend_UndoOfMembershipDoesNotDeleteTheItem(t *testing.T) { - recorder := &callRecorder{} - sb, _ := setupSync(t, recorder.handler()) - - work, _ := sb.CreateProject(model.CreateProject{Name: "work"}) - personal, _ := sb.CreateProject(model.CreateProject{Name: "personal"}) - item, _ := sb.CreateItem(model.CreateProjectItem{ - Title: "shared", - ProjectIDs: []string{work.ID}, - }) - if err := sb.AddToProject(item.ID, personal.ID); err != nil { - t.Fatalf("AddToProject: %v", err) - } - - time.Sleep(400 * time.Millisecond) - beforeUndo := recorder.count() - - if _, err := sb.Undo(); err != nil { - t.Fatalf("Undo: %v", err) - } - time.Sleep(400 * time.Millisecond) - - calls := recorder.snapshot()[beforeUndo:] - wantRemove := "DELETE /api/v1/project-items/" + item.ID + "/projects/" + personal.ID - itemDelete := "DELETE /api/v1/project-items/" + item.ID - - var sawRemove bool - for _, call := range calls { - if call == itemDelete { - t.Fatalf("undoing a membership add deleted the item on the server: %v", calls) - } - if call == wantRemove { - sawRemove = true - } - } - if !sawRemove { - t.Errorf("expected %q after undoing the membership add, got %v", wantRemove, calls) - } -} - -func TestSyncBackend_UndoOfTaskCompletionPushesTheReversal(t *testing.T) { - recorder := &callRecorder{} - sb, _ := setupSync(t, recorder.handler()) - - p, _ := sb.CreateProject(model.CreateProject{Name: "work"}) - item, _ := sb.CreateItem(model.CreateProjectItem{Title: "item", ProjectIDs: []string{p.ID}}) - task, err := sb.CreateTask(item.ID, model.CreateProjectItemTask{Title: "step"}) - if err != nil { - t.Fatalf("CreateTask: %v", err) - } - done := true - if _, err := sb.UpdateTask(item.ID, task.ID, model.UpdateProjectItemTask{Completed: &done}); err != nil { - t.Fatalf("UpdateTask: %v", err) - } - - time.Sleep(400 * time.Millisecond) - beforeUndo := recorder.count() - - if _, err := sb.Undo(); err != nil { - t.Fatalf("Undo: %v", err) - } - time.Sleep(400 * time.Millisecond) - - calls := recorder.snapshot()[beforeUndo:] - want := "PATCH /api/v1/project-items/" + item.ID + "/tasks/" + task.ID - for _, call := range calls { - if call == want { - return - } - } - t.Errorf("expected %q after undoing the completion, got %v", want, calls) -} - -// logging wraps a handler so a test can count requests without reimplementing -// the routing. -func logging(recorder *callRecorder, next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - recorder.mu.Lock() - recorder.calls = append(recorder.calls, r.Method+" "+r.URL.Path) - recorder.mu.Unlock() - next.ServeHTTP(w, r) - }) -} - -// A pull takes seconds — 2+2N requests — and the user keeps working through it. -// Anything created in that window postdates the server response, so the -// reconciliation must not read it as "deleted upstream" and erase it, nor drop -// the queued operation that is the only record of it. -func TestPull_KeepsWorkDoneWhileItWasRunning(t *testing.T) { - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - t.Cleanup(func() { _ = database.Close() }) - - local := service.New(sqlite.New(database)) - - // The mid-pull create needs a project to attach to, and the server has to - // report that same project or the pull deletes it and cascades the item away. - project, err := local.CreateProject(model.CreateProject{Name: "Server"}) - if err != nil { - t.Fatalf("seeding project: %v", err) - } - - var sb *sync.SyncBackend - var once goSync.Once - raced := make(chan string, 1) - - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Every write fails, so the op queued below is guaranteed to still be - // pending when the assertion runs. - if r.Method != http.MethodGet { - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/api/v1/projects": - _ = json.NewEncoder(w).Encode([]model.Project{ - {ID: project.ID, Name: "Server", Position: 0, CreatedAt: time.Now()}, - }) - case "/api/v1/project-items": - _ = json.NewEncoder(w).Encode([]model.ProjectItem{ - {ID: "01900000-0000-7000-8000-00000000a001", Title: "Server Item", CreatedAt: time.Now(), UpdatedAt: time.Now()}, - }) - case "/api/v1/project-items/01900000-0000-7000-8000-00000000a001": - // Mid-pull: the user adds an item the server response cannot know about. - once.Do(func() { - created, err := sb.CreateItem(model.CreateProjectItem{ - Title: "Typed During Pull", - ProjectIDs: []string{project.ID}, - }) - if err != nil { - t.Errorf("creating item mid-pull: %v", err) - return - } - raced <- created.ID - }) - _ = json.NewEncoder(w).Encode(model.ProjectItemDetail{ - ProjectItem: model.ProjectItem{ID: "01900000-0000-7000-8000-00000000a001", Title: "Server Item", CreatedAt: time.Now(), UpdatedAt: time.Now()}, - Projects: []model.Project{{ID: project.ID, Name: "Server"}}, - }) - default: - _ = json.NewEncoder(w).Encode([]model.ProjectItemTask{}) - } - }) - - ts := httptest.NewServer(handler) - t.Cleanup(ts.Close) - - engine := sync.New(database, ts.URL, "") - t.Cleanup(engine.Stop) - sb = sync.NewSyncBackend(local, engine) - - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("Pull: %v", err) - } - - racedID := <-raced - if _, err := local.GetItem(racedID); err != nil { - t.Fatalf("item created during the pull was erased by it: %v", err) - } - if got := engine.Status().PendingCount; got == 0 { - t.Error("the queued create was dropped, so nothing will ever push it") - } -} - -// Notify only fires on a local mutation. Without a retry ticker a push that -// failed while the API was down stayed queued until the user happened to edit -// something else, which is indistinguishable from sync being broken. -func TestPush_RetriesWithoutAFurtherMutation(t *testing.T) { - var mu goSync.Mutex - failing := true - handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - mu.Lock() - down := failing - mu.Unlock() - if down { - w.WriteHeader(http.StatusInternalServerError) - return - } - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - }) - - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - t.Cleanup(func() { _ = database.Close() }) - - ts := httptest.NewServer(handler) - t.Cleanup(ts.Close) - - engine := sync.New(database, ts.URL, "", sync.WithPushRetryInterval(100*time.Millisecond)) - engine.Start() - t.Cleanup(engine.Stop) - sb := sync.NewSyncBackend(service.New(sqlite.New(database)), engine) - - if _, err := sb.CreateProject(model.CreateProject{Name: "Queued"}); err != nil { - t.Fatalf("CreateProject: %v", err) - } - - // The first push fails and backs off; the op must still be waiting. - time.Sleep(500 * time.Millisecond) - if engine.Status().PendingCount == 0 { - t.Fatal("op vanished while the API was failing") - } - - mu.Lock() - failing = false - mu.Unlock() - - // No Notify, no further mutation — only the ticker can clear this. - deadline := time.Now().Add(5 * time.Second) - for time.Now().Before(deadline) { - if engine.Status().PendingCount == 0 { - return - } - time.Sleep(50 * time.Millisecond) - } - t.Error("queue never healed on its own after the API recovered") -} - -// pullFixture serves a two-item dataset. When embedded is true the list carries -// dependency_ids and tasks the way the current API does; when false it omits -// them the way a server predating the embed does, which is what forces the -// per-item fallback. -func pullFixture(embedded bool) http.HandlerFunc { - now := time.Now() - base := func(id, title string) map[string]any { - return map[string]any{ - "id": id, "title": title, "notes": nil, "repo": nil, - "completed": false, "archived": false, - "created_at": now.Format(time.RFC3339Nano), - "updated_at": now.Format(time.RFC3339Nano), - "projects": []map[string]any{{"id": "proj-1", "name": "Remote", "position": 0, "created_at": now.Format(time.RFC3339Nano)}}, - } - } - task := map[string]any{ - "id": "task-1", "item_id": "01900000-0000-7000-8000-00000000a002", "title": "Embedded Task", - "completed": false, "position": 0, "created_at": now.Format(time.RFC3339Nano), - } - - return func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/api/v1/projects": - _ = json.NewEncoder(w).Encode([]model.Project{{ID: "proj-1", Name: "Remote", Position: 0, CreatedAt: now}}) - case "/api/v1/project-items": - blocker := base("01900000-0000-7000-8000-00000000a001", "Blocker") - blocked := base("01900000-0000-7000-8000-00000000a002", "Blocked") - if embedded { - blocker["dependency_ids"] = []string{} - blocker["tasks"] = []map[string]any{} - blocked["dependency_ids"] = []string{"01900000-0000-7000-8000-00000000a001"} - blocked["tasks"] = []map[string]any{task} - } - _ = json.NewEncoder(w).Encode([]map[string]any{blocker, blocked}) - case "/api/v1/project-items/01900000-0000-7000-8000-00000000a001": - _ = json.NewEncoder(w).Encode(base("01900000-0000-7000-8000-00000000a001", "Blocker")) - case "/api/v1/project-items/01900000-0000-7000-8000-00000000a002": - detail := base("01900000-0000-7000-8000-00000000a002", "Blocked") - detail["dependency_ids"] = []string{"01900000-0000-7000-8000-00000000a001"} - _ = json.NewEncoder(w).Encode(detail) - case "/api/v1/project-items/01900000-0000-7000-8000-00000000a002/tasks": - _ = json.NewEncoder(w).Encode([]map[string]any{task}) - default: - _ = json.NewEncoder(w).Encode([]map[string]any{}) - } - } -} - -func pulledInto(t *testing.T, handler http.HandlerFunc) (*callRecorder, *service.Service) { - t.Helper() - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - t.Cleanup(func() { _ = database.Close() }) - - recorder := &callRecorder{} - ts := httptest.NewServer(logging(recorder, handler)) - t.Cleanup(ts.Close) - - engine := sync.New(database, ts.URL, "") - t.Cleanup(engine.Stop) - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("Pull: %v", err) - } - return recorder, service.New(sqlite.New(database)) -} - -// The whole point of embedding: a reconcile is two requests regardless of how -// many items exist, instead of two more per item. -func TestPull_UsesEmbeddedDetailAndStopsAtTwoRequests(t *testing.T) { - recorder, local := pulledInto(t, pullFixture(true)) - - if got := recorder.snapshot(); len(got) != 2 { - t.Errorf("expected 2 requests, got %d: %v", len(got), got) - } - - tasks, err := local.ListTasks("01900000-0000-7000-8000-00000000a002") - if err != nil { - t.Fatalf("ListTasks: %v", err) - } - if len(tasks) != 1 || tasks[0].Title != "Embedded Task" { - t.Errorf("embedded task did not reconcile: %v", tasks) - } - blockers, err := local.GetBlockers("01900000-0000-7000-8000-00000000a002") - if err != nil { - t.Fatalf("GetBlockers: %v", err) - } - if len(blockers) != 1 || blockers[0].ID != "01900000-0000-7000-8000-00000000a001" { - t.Errorf("embedded dependency did not reconcile: %v", blockers) - } -} - -// Deploy order between this tool and its server must not matter. Against a server -// that omits the fields, treating absent as empty would delete every task and -// dependency locally, so the pull has to fall back to the per-item endpoints. -func TestPull_FallsBackWhenTheServerOmitsEmbeddedFields(t *testing.T) { - recorder, local := pulledInto(t, pullFixture(false)) - - if got := recorder.count(); got <= 2 { - t.Errorf("expected per-item fallback requests, got only %d", got) - } - - tasks, err := local.ListTasks("01900000-0000-7000-8000-00000000a002") - if err != nil { - t.Fatalf("ListTasks: %v", err) - } - if len(tasks) != 1 || tasks[0].Title != "Embedded Task" { - t.Errorf("fallback lost the item's tasks: %v", tasks) - } - blockers, err := local.GetBlockers("01900000-0000-7000-8000-00000000a002") - if err != nil { - t.Fatalf("GetBlockers: %v", err) - } - if len(blockers) != 1 || blockers[0].ID != "01900000-0000-7000-8000-00000000a001" { - t.Errorf("fallback lost the item's dependencies: %v", blockers) - } -} - -// pendingAfterPush waits for the push loop a mutation's Notify woke, and returns -// how many ops are still queued once the queue empties or the wait runs out. -func pendingAfterPush(engine *sync.Engine) int { - deadline := time.Now().Add(2 * time.Second) - for { - pending := engine.Status().PendingCount - if pending == 0 || time.Now().After(deadline) { - return pending - } - time.Sleep(10 * time.Millisecond) - } -} - -// The number is the item's handle in the TUI, and the create response is the -// first place it exists. Waiting for the next pull would leave the item unnamed -// for up to a full sync interval after it was made. -func TestPush_RecordsTheNumberTheServerAssigned(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && r.URL.Path == "/api/v1/project-items" { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusCreated) - _, _ = w.Write([]byte(`{"number": 231}`)) - return - } - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - }) - sb, engine := setupSync(t, handler) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Numbered"}) - item, err := sb.CreateItem(model.CreateProjectItem{Title: "Earns a number", ProjectIDs: []string{p.ID}}) - if err != nil { - t.Fatalf("CreateItem: %v", err) - } - if item.Number != nil { - t.Fatalf("Number = %d before the push, want nil", *item.Number) - } - - if pending := pendingAfterPush(engine); pending != 0 { - t.Fatalf("pending ops = %d, want the create pushed", pending) - } - - pushed, err := sb.GetItem(item.ID) - if err != nil { - t.Fatalf("GetItem: %v", err) - } - if pushed.Number == nil { - t.Fatal("Number is still nil after the push — the create response was discarded") - } - if *pushed.Number != 231 { - t.Errorf("Number = %d, want 231", *pushed.Number) - } -} - -// A create that reached the server is applied whether or not its response -// decodes. Failing here would re-queue it and file the item twice. -func TestPush_AnUndecodableCreateResponseIsNotAFailure(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && r.URL.Path == "/api/v1/project-items" { - w.WriteHeader(http.StatusCreated) - _, _ = w.Write([]byte("not json")) - return - } - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - }) - sb, engine := setupSync(t, handler) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Garbled"}) - item, err := sb.CreateItem(model.CreateProjectItem{Title: "Still filed once", ProjectIDs: []string{p.ID}}) - if err != nil { - t.Fatalf("CreateItem: %v", err) - } - - if pending := pendingAfterPush(engine); pending != 0 { - t.Errorf("pending ops = %d, want 0 — the create landed and must not be retried", pending) - } - stored, err := sb.GetItem(item.ID) - if err != nil { - t.Fatalf("GetItem: %v", err) - } - if stored.Number != nil { - t.Errorf("Number = %d, want nil — the next pull supplies it", *stored.Number) - } -} - -// Two drains overlapping pushed the same queued op twice — a 201 followed by a -// 409, and a server-side item number burned on the insert that lost the race. -// Notify wakes the push loop on a create, and a pull drains the queue before it -// fetches, so a create landing just before a pull is drained from both sides. -func TestPush_AQueuedCreateIsPushedExactlyOnce(t *testing.T) { - var mu goSync.Mutex - creates := 0 - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && r.URL.Path == "/api/v1/project-items" { - mu.Lock() - creates++ - mu.Unlock() - time.Sleep(20 * time.Millisecond) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusCreated) - _, _ = w.Write([]byte(`{"number": 9}`)) - return - } - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - }) - sb, engine := setupSync(t, handler) - - p, _ := sb.CreateProject(model.CreateProject{Name: "Once"}) - if _, err := sb.CreateItem(model.CreateProjectItem{Title: "Pushed once", ProjectIDs: []string{p.ID}}); err != nil { - t.Fatalf("CreateItem: %v", err) - } - - // The create already woke the push loop. The pull's own drain races one - // that is very likely still in flight, and its fetch then fails on the - // handler's "{}", which this test does not care about. - _ = engine.Pull(t.Context()) - time.Sleep(300 * time.Millisecond) - - mu.Lock() - defer mu.Unlock() - if creates != 1 { - t.Errorf("POST /api/v1/project-items called %d times, want exactly 1", creates) - } -} - -// The server assembles /projects/ and each item's embedded Projects list -// separately, and nothing makes the two agree, so an item can name a project -// the projects endpoint omitted. The pull drops that link and carries on. -// Inserting it would fail the pull on the foreign key, and one inconsistent -// link would cost every other change in the reconcile. -func TestPull_DropsLinksToEntitiesTheServerDidNotSend(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/api/v1/projects": - _ = json.NewEncoder(w).Encode([]model.Project{ - {ID: "proj-1", Name: "Real", Position: 0, CreatedAt: time.Now()}, - }) - case "/api/v1/project-items": - _ = json.NewEncoder(w).Encode([]model.ProjectItemDetail{{ - ProjectItem: model.ProjectItem{ID: "01900000-0000-7000-8000-00000000a001", Title: "Kept", CreatedAt: time.Now(), UpdatedAt: time.Now()}, - Projects: []model.Project{ - {ID: "proj-1", Name: "Real"}, - {ID: "proj-missing", Name: "Never sent by /projects/"}, - }, - DependencyIDs: []string{"item-missing"}, - Tasks: []model.ProjectItemTask{ - {ID: "task-1", ItemID: "01900000-0000-7000-8000-00000000a001", Title: "Kept task", CreatedAt: time.Now()}, - }, - }}) - default: - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - } - }) - - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - defer func() { _ = database.Close() }() - - ts := httptest.NewServer(handler) - defer ts.Close() - - engine := sync.New(database, ts.URL, "") - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("one unlinkable membership failed the whole pull: %v", err) - } - - local := service.New(sqlite.New(database)) - projects, err := local.GetItemProjects("01900000-0000-7000-8000-00000000a001") - if err != nil { - t.Fatalf("GetItemProjects: %v", err) - } - if len(projects) != 1 || projects[0].ID != "proj-1" { - t.Errorf("expected only the membership that has a project, got %v", projects) - } - - var violations int - if err := database.QueryRow("SELECT COUNT(*) FROM pragma_foreign_key_check").Scan(&violations); err != nil { - t.Fatalf("checking foreign keys: %v", err) - } - if violations != 0 { - t.Errorf("the pull left %d dangling rows behind", violations) - } - - tasks, err := local.ListTasks("01900000-0000-7000-8000-00000000a001") - if err != nil { - t.Fatalf("ListTasks: %v", err) - } - if len(tasks) != 1 { - t.Errorf("the good task went with the bad link: %v", tasks) - } -} - -// idx_items_number is unique and the upsert writes row by row, so two items -// swapping numbers upstream means one of them briefly needs a number the other -// still holds. Writing the numbers after every row has been cleared makes the -// pass order-independent — otherwise any renumbering upstream fails the entire -// pull and the tool never syncs again. -func TestPull_SurvivesItemsSwappingNumbers(t *testing.T) { - numbers := map[string]int{"01900000-0000-7000-8000-00000000a00a": 1, "01900000-0000-7000-8000-00000000a00b": 2} - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - if r.URL.Path != "/api/v1/project-items" { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("[]")) - return - } - out := make([]model.ProjectItemDetail, 0, 2) - for _, id := range []string{"01900000-0000-7000-8000-00000000a00a", "01900000-0000-7000-8000-00000000a00b"} { - n := numbers[id] - out = append(out, model.ProjectItemDetail{ - ProjectItem: model.ProjectItem{ - ID: id, Number: &n, Title: id, - CreatedAt: time.Now(), UpdatedAt: time.Now(), - }, - DependencyIDs: []string{}, - Tasks: []model.ProjectItemTask{}, - }) - } - _ = json.NewEncoder(w).Encode(out) - }) - - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - defer func() { _ = database.Close() }() - - ts := httptest.NewServer(handler) - defer ts.Close() - - engine := sync.New(database, ts.URL, "") - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - - numbers["01900000-0000-7000-8000-00000000a00a"], numbers["01900000-0000-7000-8000-00000000a00b"] = 2, 1 - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("a swap upstream failed the pull: %v", err) - } - - local := service.New(sqlite.New(database)) - item, err := local.GetItem("01900000-0000-7000-8000-00000000a00a") - if err != nil { - t.Fatalf("GetItem: %v", err) - } - if item.Number == nil || *item.Number != 2 { - t.Errorf("01900000-0000-7000-8000-00000000a00a number = %v, want 2", item.Number) - } -} - -// Both list fetches ask for every status, because the sweep in Pull deletes any -// local row the server did not return. An unqualified fetch is therefore not a -// narrower read — it is a delete. /project-items/ defaults to open items, so an -// unqualified fetch would delete every completed item here. -func TestPull_RequestsEveryStatusFromBothListEndpoints(t *testing.T) { - var mu goSync.Mutex - asked := map[string]string{} - record := func(r *http.Request) { - mu.Lock() - defer mu.Unlock() - asked[r.URL.Path] = r.URL.Query().Get("status") - } - - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/api/v1/projects": - record(r) - _ = json.NewEncoder(w).Encode([]model.Project{}) - case "/api/v1/project-items": - record(r) - _ = json.NewEncoder(w).Encode([]model.ProjectItemDetail{}) - default: - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("{}")) - } - }) - - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - defer func() { _ = database.Close() }() - - ts := httptest.NewServer(handler) - defer ts.Close() - - engine := sync.New(database, ts.URL, "") - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("pull: %v", err) - } - - mu.Lock() - defer mu.Unlock() - for _, path := range []string{"/api/v1/projects", "/api/v1/project-items"} { - if asked[path] != "all" { - t.Errorf("%s fetched with status=%q, want all — the sweep deletes whatever the server omits", path, asked[path]) - } - } -} diff --git a/sync/operations.go b/sync/operations.go deleted file mode 100644 index 46d9d92..0000000 --- a/sync/operations.go +++ /dev/null @@ -1,41 +0,0 @@ -package sync - -// OpType identifies the kind of sync operation queued in pending_sync. -type OpType string - -const ( - OpCreateProject OpType = "create_project" - OpUpdateProject OpType = "update_project" - OpDeleteProject OpType = "delete_project" - OpReorderProject OpType = "reorder_project" - OpCreateItem OpType = "create_item" - OpUpdateItem OpType = "update_item" - OpDeleteItem OpType = "delete_item" - OpReorderItem OpType = "reorder_item" - OpAddToProject OpType = "add_to_project" - OpRemoveFromProject OpType = "remove_from_project" - OpCreateTask OpType = "create_task" - OpUpdateTask OpType = "update_task" - OpDeleteTask OpType = "delete_task" - OpCompleteTask OpType = "complete_task" - OpAddDependency OpType = "add_dependency" - OpRemoveDependency OpType = "remove_dependency" -) - -// entityType returns the entity category for the pending_sync table. -func (o OpType) entityType() string { - switch o { - case OpCreateProject, OpUpdateProject, OpDeleteProject, OpReorderProject: - return "project" - case OpCreateItem, OpUpdateItem, OpDeleteItem, OpReorderItem: - return "item" - case OpAddToProject, OpRemoveFromProject: - return "membership" - case OpCreateTask, OpUpdateTask, OpDeleteTask, OpCompleteTask: - return "task" - case OpAddDependency, OpRemoveDependency: - return "dependency" - default: - return "unknown" - } -} diff --git a/sync/origin.go b/sync/origin.go deleted file mode 100644 index 351858e..0000000 --- a/sync/origin.go +++ /dev/null @@ -1,171 +0,0 @@ -package sync - -import ( - "context" - "database/sql" - "errors" - "fmt" - "strings" -) - -// A pull deletes every local row the server did not return, which makes a -// database and the API it reconciles against a pair. Point one at the other -// one and the pull is not a sync but a wipe: it faithfully deletes everything -// the other API has never heard of. That is not a hypothetical — a production -// database reconciled against a development API lost 42 projects and 286 items -// in a single command that reported success. -// -// Nothing in a database says which API it belongs to, so nothing could refuse. -// The pairing is recorded on first pull and checked before every push and pull -// after that. - -// OriginMismatch is returned when a database is asked to reconcile against an -// API other than the one it was adopted by. -type OriginMismatch struct { - Recorded string - Configured string -} - -func (e *OriginMismatch) Error() string { - return fmt.Sprintf( - "this database belongs to %s but sync is configured for %s — "+ - "reconciling would delete everything the other API has never seen. "+ - "Run `projects sync --adopt` if the move is deliberate", - e.Recorded, e.Configured) -} - -// normalizeAPIURL makes the comparison indifferent to the things that do not -// change which server is being addressed. -func normalizeAPIURL(url string) string { - return strings.TrimRight(strings.TrimSpace(url), "/") -} - -// OriginUnclaimed is returned when a database that has reconciled before has no -// record of which API it did that with. It was bound to something; adopting it -// silently to whatever is configured now is exactly the step that turns a -// misconfiguration into a wipe, so it is the user who says. -type OriginUnclaimed struct { - Configured string - Projects int - Items int -} - -func (e *OriginUnclaimed) Error() string { - return fmt.Sprintf( - "this database holds %d projects and %d items and has synced before, but has no record of "+ - "which API it belongs to — reconciling against %s would delete everything that API has "+ - "never seen. Run `projects sync --adopt` to bind it", - e.Projects, e.Items, e.Configured) -} - -// checkOriginMatch refuses only an outright mismatch. It is what the push path -// asks, because a push does not destroy local data: it only has to be stopped -// from delivering one database's work to another's API. -func (e *Engine) checkOriginMatch(ctx context.Context) error { - recorded, err := e.q.GetSyncOrigin(ctx) - if errors.Is(err, sql.ErrNoRows) { - return nil - } - if err != nil { - return fmt.Errorf("reading the database's sync origin: %w", err) - } - if configured := normalizeAPIURL(e.apiURL); recorded != configured { - return &OriginMismatch{Recorded: recorded, Configured: configured} - } - return nil -} - -// checkOrigin is the pull's stricter question: may this reconcile happen, and -// should it be the one to record the pairing. -// -// A database that has never pulled adopts on this one — that is every new -// install, whatever it has typed in locally, since a pull spares rows queued for -// push anyway. A database that has pulled before was bound to some API already, -// and if nothing recorded which, that has to be said out loud rather than -// inferred from whichever environment happens to be loaded. -func (e *Engine) checkOrigin(ctx context.Context) (adopt bool, err error) { - configured := normalizeAPIURL(e.apiURL) - - recorded, err := e.q.GetSyncOrigin(ctx) - switch { - case err == nil: - if recorded != configured { - return false, &OriginMismatch{Recorded: recorded, Configured: configured} - } - return false, nil - case !errors.Is(err, sql.ErrNoRows): - return false, fmt.Errorf("reading the database's sync origin: %w", err) - } - - pulledBefore, err := e.q.HasPulledBefore(ctx) - if err != nil { - return false, fmt.Errorf("checking whether this database has synced before: %w", err) - } - if pulledBefore == 0 { - return true, nil - } - - projects, err := e.q.CountProjects(ctx) - if err != nil { - return false, fmt.Errorf("counting projects: %w", err) - } - items, err := e.q.CountItems(ctx) - if err != nil { - return false, fmt.Errorf("counting items: %w", err) - } - return false, &OriginUnclaimed{ - Configured: configured, - Projects: int(projects), - Items: int(items), - } -} - -// AdoptOrigin binds this database to the engine's API, overwriting whatever it -// was bound to. It is what `projects sync --adopt` calls: the one place the -// pairing can change, and only because someone said so. -func (e *Engine) AdoptOrigin(ctx context.Context) error { - return e.q.SetSyncOrigin(ctx, normalizeAPIURL(e.apiURL)) -} - -// A pull deletes whatever the server did not return, so a truncated response -// and a genuine mass deletion are the same event seen from here. Scale is the -// only thing that separates them, and the cost of guessing wrong is asymmetric: -// a refused pull costs one flag, an obeyed one costs the database. The floor -// keeps the guard out of the way of small databases, where losing "most of it" -// is a handful of rows and routine. -const ( - sweepFloor = 10 - sweepShare = 0.5 -) - -// SweepRefused is returned when a pull would delete implausibly much. -type SweepRefused struct { - Entity string - Deleting int - Local int -} - -func (e *SweepRefused) Error() string { - return fmt.Sprintf( - "refusing to delete %d of %d %s in one pull — the API returned almost nothing this database has, "+ - "which is a truncated response or the wrong API far more often than it is a real deletion. "+ - "Run `projects sync --force` if it is genuinely what happened", - e.Deleting, e.Local, e.Entity) -} - -func guardSweep(entity string, deleting, local int, allowed bool) error { - if allowed || deleting < sweepFloor || float64(deleting) <= float64(local)*sweepShare { - return nil - } - return &SweepRefused{Entity: entity, Deleting: deleting, Local: local} -} - -// Origin returns the API this database is bound to, or "" if it has never -// pulled. -func (e *Engine) Origin(ctx context.Context) string { - recorded, err := e.q.GetSyncOrigin(ctx) - if err != nil { - return "" - } - return recorded -} diff --git a/sync/origin_test.go b/sync/origin_test.go deleted file mode 100644 index ac26a54..0000000 --- a/sync/origin_test.go +++ /dev/null @@ -1,266 +0,0 @@ -package sync_test - -import ( - "database/sql" - "encoding/json" - "errors" - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/datapointchris/projects/model" - "github.com/datapointchris/projects/service" - "github.com/datapointchris/projects/store/sqlite" - "github.com/datapointchris/projects/sync" -) - -// apiWith serves a fixed set of projects and items, which is all a pull reads. -func apiWith(t *testing.T, projects []model.Project, items []model.ProjectItemDetail) *httptest.Server { - t.Helper() - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/api/v1/projects": - _ = json.NewEncoder(w).Encode(projects) - case "/api/v1/project-items": - _ = json.NewEncoder(w).Encode(items) - default: - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("[]")) - } - }) - ts := httptest.NewServer(handler) - t.Cleanup(ts.Close) - return ts -} - -func openTestDB(t *testing.T) *sql.DB { - t.Helper() - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening database: %v", err) - } - t.Cleanup(func() { _ = database.Close() }) - return database -} - -func seededAPI(t *testing.T, projectCount, itemCount int) *httptest.Server { - t.Helper() - projects := make([]model.Project, 0, projectCount) - for i := range projectCount { - projects = append(projects, model.Project{ - ID: "proj-" + itoa(i), Name: "Project " + itoa(i), CreatedAt: time.Now(), - }) - } - items := make([]model.ProjectItemDetail, 0, itemCount) - for i := range itemCount { - items = append(items, model.ProjectItemDetail{ - ProjectItem: model.ProjectItem{ - ID: "item-" + itoa(i), Title: "Item " + itoa(i), - CreatedAt: time.Now(), UpdatedAt: time.Now(), - }, - Projects: []model.Project{{ID: "proj-0"}}, - DependencyIDs: []string{}, - Tasks: []model.ProjectItemTask{}, - }) - } - return apiWith(t, projects, items) -} - -func itoa(i int) string { - if i == 0 { - return "0" - } - var digits []byte - for i > 0 { - digits = append([]byte{byte('0' + i%10)}, digits...) - i /= 10 - } - return string(digits) -} - -// A fresh install must not need a ceremony: its first pull records which API it -// belongs to and gets on with it. -func TestFirstPullAdoptsTheAPI(t *testing.T) { - database := openTestDB(t) - ts := seededAPI(t, 2, 3) - - engine := sync.New(database, ts.URL, "") - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - if got := engine.Origin(t.Context()); got != ts.URL { - t.Errorf("origin = %q, want %q", got, ts.URL) - } -} - -// The incident this exists to prevent: a database that belongs to one API is -// pointed at another, and the pull faithfully deletes everything the second API -// has never heard of. A production database lost 42 projects and 286 items this -// way, and the command printed "Synced." -func TestPullRefusesADifferentAPI(t *testing.T) { - database := openTestDB(t) - real := seededAPI(t, 20, 40) - - if err := sync.New(database, real.URL, "").Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - - other := seededAPI(t, 1, 2) - err := sync.New(database, other.URL, "").Pull(t.Context()) - - var mismatch *sync.OriginMismatch - if !errors.As(err, &mismatch) { - t.Fatalf("pull against a different API returned %v, want an OriginMismatch", err) - } - if mismatch.Recorded != real.URL || mismatch.Configured != other.URL { - t.Errorf("mismatch names %s → %s, want %s → %s", - mismatch.Recorded, mismatch.Configured, real.URL, other.URL) - } - - local := service.New(sqlite.New(database)) - projects, err := local.ListProjectsByStatus(model.StatusActive) - if err != nil { - t.Fatalf("ListProjectsByStatus: %v", err) - } - if len(projects) != 20 { - t.Errorf("the refused pull still took data: %d projects left of 20", len(projects)) - } -} - -// The refusal has to survive the trailing slash, or it fires on a difference -// that addresses the same server. -func TestOriginIgnoresATrailingSlash(t *testing.T) { - database := openTestDB(t) - ts := seededAPI(t, 2, 3) - - if err := sync.New(database, ts.URL, "").Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - if err := sync.New(database, ts.URL+"/", "").Pull(t.Context()); err != nil { - t.Errorf("a trailing slash was treated as a different API: %v", err) - } -} - -// --adopt is the deliberate override, for an API that genuinely moved. -func TestAdoptRebindsTheDatabase(t *testing.T) { - database := openTestDB(t) - first := seededAPI(t, 2, 3) - if err := sync.New(database, first.URL, "").Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - - moved := seededAPI(t, 2, 3) - engine := sync.New(database, moved.URL, "") - if err := engine.PullWith(t.Context(), sync.PullOptions{Adopt: true, AllowLargeSweep: true}); err != nil { - t.Fatalf("adopting pull: %v", err) - } - if got := engine.Origin(t.Context()); got != moved.URL { - t.Errorf("origin = %q, want %q", got, moved.URL) - } -} - -// A database carried over from before the binding existed has reconciled with -// something, and nothing recorded what. Guessing from whichever environment -// happens to be loaded is the mistake itself. -func TestPullRefusesToGuessForADatabaseThatSyncedBeforeTheBinding(t *testing.T) { - database := openTestDB(t) - ts := seededAPI(t, 20, 40) - if err := sync.New(database, ts.URL, "").Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - if _, err := database.Exec("DELETE FROM sync_origin"); err != nil { - t.Fatalf("clearing the origin: %v", err) - } - - err := sync.New(database, ts.URL, "").Pull(t.Context()) - - var unclaimed *sync.OriginUnclaimed - if !errors.As(err, &unclaimed) { - t.Fatalf("pull returned %v, want an OriginUnclaimed", err) - } - if unclaimed.Projects != 20 || unclaimed.Items != 40 { - t.Errorf("the refusal reports %d projects and %d items, want 20 and 40", - unclaimed.Projects, unclaimed.Items) - } - - if err := sync.New(database, ts.URL, "").PullWith(t.Context(), sync.PullOptions{Adopt: true}); err != nil { - t.Errorf("--adopt did not resolve it: %v", err) - } -} - -// A truncated response and a real mass deletion are the same event from here, -// and only one of them should be obeyed without being asked. -func TestPullRefusesToDeleteMostOfTheDatabase(t *testing.T) { - database := openTestDB(t) - full := seededAPI(t, 20, 40) - if err := sync.New(database, full.URL, "").Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - - // Same API, now answering with almost nothing — a bad deploy, an auth - // failure that still returns 200, a paginated endpoint read as complete. - truncated := apiWith(t, []model.Project{{ID: "proj-0", Name: "Project 0", CreatedAt: time.Now()}}, nil) - engine := sync.New(database, truncated.URL, "") - if err := engine.PullWith(t.Context(), sync.PullOptions{Adopt: true}); err == nil { - t.Fatal("a pull deleting 19 of 20 projects was allowed") - } else { - var refused *sync.SweepRefused - if !errors.As(err, &refused) { - t.Fatalf("got %v, want a SweepRefused", err) - } - } - - local := service.New(sqlite.New(database)) - projects, err := local.ListProjectsByStatus(model.StatusActive) - if err != nil { - t.Fatalf("ListProjectsByStatus: %v", err) - } - if len(projects) != 20 { - t.Errorf("the refused pull still deleted: %d projects left of 20", len(projects)) - } - - if err := engine.PullWith(t.Context(), sync.PullOptions{Adopt: true, AllowLargeSweep: true}); err != nil { - t.Fatalf("--force did not allow it: %v", err) - } - projects, err = local.ListProjectsByStatus(model.StatusActive) - if err != nil { - t.Fatalf("ListProjectsByStatus: %v", err) - } - if len(projects) != 1 { - t.Errorf("--force did not apply the sweep: %d projects left", len(projects)) - } -} - -// The guard must not fire on ordinary tidying, or it trains you to pass --force -// by reflex and stops guarding anything. -func TestPullAllowsAnOrdinaryDeletion(t *testing.T) { - database := openTestDB(t) - full := seededAPI(t, 20, 40) - if err := sync.New(database, full.URL, "").Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - - fewer := seededAPI(t, 18, 32) - engine := sync.New(database, fewer.URL, "") - if err := engine.PullWith(t.Context(), sync.PullOptions{Adopt: true}); err != nil { - t.Fatalf("deleting 2 of 20 projects and 8 of 40 items was refused: %v", err) - } -} - -// A small database loses "most of itself" on any ordinary day, so the guard has -// a floor — otherwise it fires constantly and gets forced past on reflex. -func TestSweepGuardHasAFloorForSmallDatabases(t *testing.T) { - database := openTestDB(t) - full := seededAPI(t, 4, 6) - if err := sync.New(database, full.URL, "").Pull(t.Context()); err != nil { - t.Fatalf("first pull: %v", err) - } - - empty := apiWith(t, nil, nil) - engine := sync.New(database, empty.URL, "") - if err := engine.PullWith(t.Context(), sync.PullOptions{Adopt: true}); err != nil { - t.Errorf("a 4-project database was guarded: %v", err) - } -} diff --git a/sync/pull.go b/sync/pull.go deleted file mode 100644 index 67587f5..0000000 --- a/sync/pull.go +++ /dev/null @@ -1,446 +0,0 @@ -package sync - -import ( - "context" - "database/sql" - "encoding/json" - "fmt" - "io" - "net/http" - "time" - - "github.com/datapointchris/projects/model" - "github.com/datapointchris/projects/store/sqlite/generated" -) - -// Pull fetches all data from the remote API and reconciles it with the local database. -// It attempts to push pending ops first (so the server has our changes), then does a -// full pull. Server state wins on conflicts. -// -// A failure is recorded on the status so the TUI's indicator reflects it. Pull runs -// unattended on a timer, and an error only a foreground caller can see would leave -// the bar reading SYNCED while nothing had reconciled for hours. -func (e *Engine) Pull(ctx context.Context) error { - return e.PullWith(ctx, PullOptions{}) -} - -// PullOptions carries the deliberate overrides of the two guards that stand -// between a misconfigured reconcile and an emptied database. Neither is -// reachable from the background timers — a guard something automatic can waive -// is not a guard. -type PullOptions struct { - // Adopt binds the database to this API before reconciling, which is the - // only way the pairing ever changes. `projects sync --adopt`. - Adopt bool - // AllowLargeSweep permits a pull that would delete most of what is here. - // `projects sync --force`. - AllowLargeSweep bool -} - -func (e *Engine) PullWith(ctx context.Context, opts PullOptions) error { - e.beginSync() - defer e.endSync() - - err := e.pull(ctx, opts) - e.setStatus(func(s *SyncStatus) { - s.Connected = err == nil - if err != nil { - s.LastError = err.Error() - } else { - s.LastError = "" - } - }) - return err -} - -func (e *Engine) pull(ctx context.Context, opts PullOptions) error { - if opts.Adopt { - if err := e.AdoptOrigin(ctx); err != nil { - return fmt.Errorf("adopting %s: %w", e.apiURL, err) - } - } - // Before the push, not after: draining first would send this database's - // queued work to an API that is not its own. - adopt, err := e.checkOrigin(ctx) - if err != nil { - return err - } - if adopt { - if err := e.AdoptOrigin(ctx); err != nil { - return fmt.Errorf("recording %s as this database's origin: %w", e.apiURL, err) - } - } - - // Push first: try to drain pending ops so server has our changes - e.drainPendingOps() - - // Everything queued from here on describes a local change the fetch below - // cannot know about. Recording the high-water mark lets the reconciliation - // clear only what it actually superseded — see the delete calls at the end. - pushedThrough, err := e.q.MaxPendingSyncID(ctx) - if err != nil { - return fmt.Errorf("reading pending sync high-water mark: %w", err) - } - - // Fetch all data from the server. - // - // status=all because the sweep below deletes any local project the server - // did not return, cascading through to its memberships. Once /projects - // defaults to active-only, an omitted-because-completed project reads as - // deleted-upstream and the row goes. Terminal projects are filtered for - // display locally instead. The param is ignored by an API predating it, - // which is what lets this ship ahead of the server change. - projects, err := fetchJSON[[]model.Project](ctx, e.client, e.base, "/projects?status=all") - if err != nil { - return fmt.Errorf("pulling projects: %w", err) - } - - // status=all for the same reason as projects. /project-items defaults to - // open items, so without it every completed item is absent from the response - // and the sweep below deletes it. - items, err := fetchJSON[[]model.ProjectItemDetail](ctx, e.client, e.base, "/project-items?status=all") - if err != nil { - return fmt.Errorf("pulling items: %w", err) - } - - type itemExtra struct { - detail *model.ProjectItemDetail - tasks []model.ProjectItemTask - } - extras := make(map[string]itemExtra, len(items)) - - for _, item := range items { - // The list embeds memberships, dependencies, and tasks, so the whole - // reconcile is two requests. The fallback below costs two more per item: - // 122 serial round trips and 6.15s at 60 items. - if item.DependencyIDs != nil && item.Tasks != nil { - extras[item.ID] = itemExtra{detail: &item, tasks: item.Tasks} - continue - } - - // A server predating the embed omits both fields. Falling back rather - // than trusting the zero value matters: treating absent as empty would - // erase every task and dependency locally on the first pull against an - // older API, so deploy order between this and ichrisbirch cannot lose data. - detail, err := fetchJSON[*model.ProjectItemDetail](ctx, e.client, e.base, fmt.Sprintf("/project-items/%s", item.ID)) - if err != nil { - return fmt.Errorf("pulling item %s detail: %w", item.ID, err) - } - - tasks, err := fetchJSON[[]model.ProjectItemTask](ctx, e.client, e.base, fmt.Sprintf("/project-items/%s/tasks", item.ID)) - if err != nil { - return fmt.Errorf("pulling item %s tasks: %w", item.ID, err) - } - - extras[item.ID] = itemExtra{detail: detail, tasks: tasks} - } - - // Reconcile in a transaction - tx, err := e.db.Begin() - if err != nil { - return fmt.Errorf("beginning sync transaction: %w", err) - } - defer func() { _ = tx.Rollback() }() - qtx := e.q.WithTx(tx) - - // Entities changed locally while the fetch was in flight. The server response - // predates them, so the sweeps below would read them as "deleted upstream" and - // erase a row the user created seconds ago. Ids are UUIDv7 and unique across - // tables, so one set covers projects, items, and tasks alike. - pendingIDs, err := qtx.ListPendingSyncEntityIDsAfter(ctx, pushedThrough) - if err != nil { - return fmt.Errorf("listing in-flight sync entities: %w", err) - } - queuedLocally := make(map[string]bool, len(pendingIDs)) - for _, id := range pendingIDs { - queuedLocally[id] = true - } - - // Upsert projects - serverProjectIDs := make(map[string]bool, len(projects)) - for _, p := range projects { - serverProjectIDs[p.ID] = true - // A server predating the status column sends none, and an empty status - // would violate NOT NULL and fail the whole pull. Absent means active, - // which is what every project was before the column existed. - status := p.Status - if status == "" { - status = model.StatusActive - } - if err := qtx.UpsertProject(ctx, generated.UpsertProjectParams{ - ID: p.ID, - Name: p.Name, - Description: nullStr(p.Description), - Status: status, - StatusReason: nullStr(p.StatusReason), - ClosedAt: nullTime(p.ClosedAt), - Position: int64(p.Position), - CreatedAt: p.CreatedAt.Format(time.RFC3339Nano), - }); err != nil { - return fmt.Errorf("upserting project %s: %w", p.ID, err) - } - } - - // Delete local projects not on server - localProjects, err := qtx.ListAllProjectsRaw(ctx) - if err != nil { - return fmt.Errorf("listing local projects: %w", err) - } - // Which projects a membership may legally point at once this loop is done: - // what the server sent, plus anything created locally that the sweep spares. - projectExists := make(map[string]bool, len(serverProjectIDs)) - for id := range serverProjectIDs { - projectExists[id] = true - } - var staleProjects []string - for _, lp := range localProjects { - if !serverProjectIDs[lp.ID] && !queuedLocally[lp.ID] { - staleProjects = append(staleProjects, lp.ID) - continue - } - projectExists[lp.ID] = true - } - if err := guardSweep("projects", len(staleProjects), len(localProjects), opts.AllowLargeSweep); err != nil { - return err - } - for _, id := range staleProjects { - if err := qtx.DeleteProject(ctx, id); err != nil { - return fmt.Errorf("deleting stale project %s: %w", id, err) - } - } - - // Upsert items, unnumbered. - // - // idx_items_number is unique and the upsert writes row by row, so an item - // taking a number another row has not given up yet fails the whole pull — - // which is what any renumbering upstream looks like from here. Clearing - // every server item to unnumbered first makes the pass order-independent; - // the index is partial over non-null numbers, so any number of rows may sit - // unnumbered in between. - serverItemIDs := make(map[string]bool, len(items)) - for _, item := range items { - serverItemIDs[item.ID] = true - if err := qtx.UpsertItem(ctx, generated.UpsertItemParams{ - ID: item.ID, - Title: item.Title, - Notes: nullStr(item.Notes), - Repo: nullStr(item.Repo), - Completed: boolToInt(item.Completed), - Archived: boolToInt(item.Archived), - CreatedAt: item.CreatedAt.Format(time.RFC3339Nano), - UpdatedAt: item.UpdatedAt.Format(time.RFC3339Nano), - }); err != nil { - return fmt.Errorf("upserting item %s: %w", item.ID, err) - } - } - // Delete local items not on server - localItems, err := qtx.ListAllItemsRaw(ctx) - if err != nil { - return fmt.Errorf("listing local items: %w", err) - } - itemExists := make(map[string]bool, len(serverItemIDs)) - for id := range serverItemIDs { - itemExists[id] = true - } - var staleItems []string - for _, li := range localItems { - if !serverItemIDs[li.ID] && !queuedLocally[li.ID] { - staleItems = append(staleItems, li.ID) - continue - } - itemExists[li.ID] = true - } - if err := guardSweep("items", len(staleItems), len(localItems), opts.AllowLargeSweep); err != nil { - return err - } - for _, id := range staleItems { - if err := qtx.DeleteItem(ctx, id); err != nil { - return fmt.Errorf("deleting stale item %s: %w", id, err) - } - } - - // Numbers go on last of all, once the rows that are leaving have left: an - // item deleted upstream still holds its number until the sweep above runs, - // and the server reissuing that number to something else is exactly the - // collision this ordering avoids. - for _, item := range items { - if item.Number == nil { - continue - } - if err := qtx.SetItemNumber(ctx, generated.SetItemNumberParams{ - ID: item.ID, - Number: nullInt(item.Number), - }); err != nil { - return fmt.Errorf("numbering item %s: %w", item.ID, err) - } - } - - // Replace all memberships with server state - if err := qtx.DeleteAllMemberships(ctx); err != nil { - return fmt.Errorf("clearing memberships: %w", err) - } - for _, extra := range extras { - if extra.detail == nil { - continue - } - for _, proj := range extra.detail.Projects { - // An item detail can name a project the projects endpoint did not - // return — the two answers are assembled separately server-side and - // nothing makes them agree. The link is dropped rather than - // inserted: it has no project to point at, and failing the pull over - // it would take the whole reconcile down with it, which is the - // opposite of local-first. - if !projectExists[proj.ID] || !itemExists[extra.detail.ID] { - continue - } - if err := qtx.UpsertMembership(ctx, generated.UpsertMembershipParams{ - ItemID: extra.detail.ID, - ProjectID: proj.ID, - Position: 0, // server doesn't return per-project position in detail - }); err != nil { - return fmt.Errorf("upserting membership: %w", err) - } - } - } - - // Replace all dependencies with server state - if err := qtx.DeleteAllDependencies(ctx); err != nil { - return fmt.Errorf("clearing dependencies: %w", err) - } - for _, extra := range extras { - if extra.detail == nil { - continue - } - for _, depID := range extra.detail.DependencyIDs { - // Same reasoning as memberships: a blocker the server no longer - // lists has no row to reference. - if !itemExists[depID] || !itemExists[extra.detail.ID] { - continue - } - if err := qtx.UpsertDependency(ctx, generated.UpsertDependencyParams{ - ItemID: extra.detail.ID, - DependsOnID: depID, - }); err != nil { - return fmt.Errorf("upserting dependency: %w", err) - } - } - } - - // Upsert tasks and delete stale ones - serverTaskIDs := make(map[string]bool) - for _, extra := range extras { - for _, task := range extra.tasks { - if !itemExists[task.ItemID] { - continue - } - serverTaskIDs[task.ID] = true - if err := qtx.UpsertTask(ctx, generated.UpsertTaskParams{ - ID: task.ID, - ItemID: task.ItemID, - Title: task.Title, - Completed: boolToInt(task.Completed), - Position: int64(task.Position), - CreatedAt: task.CreatedAt.Format(time.RFC3339Nano), - }); err != nil { - return fmt.Errorf("upserting task %s: %w", task.ID, err) - } - } - } - - localTasks, err := qtx.ListAllTasks(ctx) - if err != nil { - return fmt.Errorf("listing local tasks: %w", err) - } - for _, lt := range localTasks { - if !serverTaskIDs[lt.ID] && !queuedLocally[lt.ID] { - if err := qtx.DeleteTask(ctx, lt.ID); err != nil { - return fmt.Errorf("deleting stale task %s: %w", lt.ID, err) - } - } - } - - // Clear only the ops this reconciliation superseded. Anything queued after the - // high-water mark postdates the server response and still has to be pushed. - if err := qtx.DeletePendingSyncUpTo(ctx, pushedThrough); err != nil { - return fmt.Errorf("clearing pending sync: %w", err) - } - - // Update sync state - now := time.Now().UTC().Format(time.RFC3339Nano) - for _, entityType := range []string{"project", "item", "task"} { - if err := qtx.UpsertSyncState(ctx, generated.UpsertSyncStateParams{ - EntityType: entityType, - LastPullAt: now, - LastPushAt: now, - }); err != nil { - return fmt.Errorf("updating sync state: %w", err) - } - } - - if err := tx.Commit(); err != nil { - return fmt.Errorf("committing sync: %w", err) - } - - // Ops that survived the truncation were queued mid-pull and are still unsent. - if len(queuedLocally) > 0 { - e.Notify() - } - return nil -} - -// fetchJSON performs a GET request and decodes the JSON response into T. -func fetchJSON[T any](ctx context.Context, client *http.Client, baseURL, path string) (T, error) { - var zero T - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, baseURL+path, nil) - if err != nil { - return zero, err - } - - resp, err := client.Do(req) - if err != nil { - return zero, friendlyNetErr(err) - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode >= 400 { - body, _ := io.ReadAll(resp.Body) - return zero, refusal("API error", resp.StatusCode, body) - } - - var result T - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return zero, fmt.Errorf("decoding response: %w", err) - } - return result, nil -} - -func nullStr(s *string) sql.NullString { - if s == nil { - return sql.NullString{} - } - return sql.NullString{String: *s, Valid: true} -} - -// nullTime stores a nullable timestamp as RFC3339Nano, which trims trailing zeros. -// These columns sort as text, so a pulled .34Z sorts after a local .343Z. -func nullTime(t *time.Time) sql.NullString { - if t == nil { - return sql.NullString{} - } - return sql.NullString{String: t.Format(time.RFC3339Nano), Valid: true} -} - -func nullInt(n *int) sql.NullInt64 { - if n == nil { - return sql.NullInt64{} - } - return sql.NullInt64{Int64: int64(*n), Valid: true} -} - -func boolToInt(b bool) int64 { - if b { - return 1 - } - return 0 -} diff --git a/sync/push.go b/sync/push.go deleted file mode 100644 index 4cf4ff8..0000000 --- a/sync/push.go +++ /dev/null @@ -1,221 +0,0 @@ -package sync - -import ( - "bytes" - "database/sql" - "encoding/json" - "errors" - "fmt" - "io" - "net/http" - "strings" - - "github.com/datapointchris/projects/store/sqlite/generated" -) - -// executePush translates a pending sync operation to an HTTP request. -// Returns nil on success, or on server rejection (404/409) which means -// the operation should be dropped (server wins). -// Returns non-nil error on network/transient failures that should be retried. -func (e *Engine) executePush(op generated.PendingSync) error { - opType := OpType(op.Operation) - - switch opType { - case OpCreateProject: - return e.pushJSON(http.MethodPost, "/projects", op.Payload) - case OpUpdateProject: - return e.pushJSON(http.MethodPatch, fmt.Sprintf("/projects/%s", op.EntityID), op.Payload) - case OpDeleteProject: - return e.pushDelete(fmt.Sprintf("/projects/%s", op.EntityID)) - case OpReorderProject: - // The payload is {"position": n}, which a project's own PATCH takes. - return e.pushJSON(http.MethodPatch, fmt.Sprintf("/projects/%s", op.EntityID), op.Payload) - - case OpCreateItem: - return e.pushCreatedItem(op.EntityID, op.Payload) - case OpUpdateItem: - return e.pushJSON(http.MethodPatch, fmt.Sprintf("/project-items/%s", op.EntityID), op.Payload) - case OpDeleteItem: - return e.pushDelete(fmt.Sprintf("/project-items/%s", op.EntityID)) - - case OpReorderItem: - return e.pushJSON(http.MethodPatch, fmt.Sprintf("/project-items/%s/reorder", op.EntityID), op.Payload) - - case OpAddToProject: - return e.pushJSON(http.MethodPost, fmt.Sprintf("/project-items/%s/projects", op.EntityID), op.Payload) - case OpRemoveFromProject: - var p struct { - ProjectID string `json:"project_id"` - } - _ = json.Unmarshal([]byte(op.Payload), &p) - return e.pushDelete(fmt.Sprintf("/project-items/%s/projects/%s", op.EntityID, p.ProjectID)) - - case OpCreateTask: - var p struct { - ItemID string `json:"item_id"` - } - _ = json.Unmarshal([]byte(op.Payload), &p) - return e.pushJSON(http.MethodPost, fmt.Sprintf("/project-items/%s/tasks", p.ItemID), op.Payload) - case OpUpdateTask: - var p struct { - ItemID string `json:"item_id"` - } - _ = json.Unmarshal([]byte(op.Payload), &p) - return e.pushJSON(http.MethodPatch, fmt.Sprintf("/project-items/%s/tasks/%s", p.ItemID, op.EntityID), op.Payload) - case OpDeleteTask: - var p struct { - ItemID string `json:"item_id"` - } - _ = json.Unmarshal([]byte(op.Payload), &p) - return e.pushDelete(fmt.Sprintf("/project-items/%s/tasks/%s", p.ItemID, op.EntityID)) - case OpCompleteTask: - var p struct { - ItemID string `json:"item_id"` - } - _ = json.Unmarshal([]byte(op.Payload), &p) - return e.pushJSON(http.MethodPatch, fmt.Sprintf("/project-items/%s/tasks/%s", p.ItemID, op.EntityID), `{"completed":true}`) - - case OpAddDependency: - return e.pushJSON(http.MethodPost, fmt.Sprintf("/project-items/%s/dependencies", op.EntityID), op.Payload) - case OpRemoveDependency: - var p struct { - DependsOnID string `json:"depends_on_id"` - } - _ = json.Unmarshal([]byte(op.Payload), &p) - return e.pushDelete(fmt.Sprintf("/project-items/%s/dependencies/%s", op.EntityID, p.DependsOnID)) - - default: - return fmt.Errorf("unknown sync operation: %s", op.Operation) - } -} - -// pushCreatedItem posts a queued item create and records the number the server -// assigned to it. -// -// The number is the item's handle, and until the server answers there is none — -// so without this the item would stay unnamed until the next pull, up to a full -// sync interval after the command that created it returned. Long enough that -// the thing you just filed cannot be referred to by the handle it already has. -func (e *Engine) pushCreatedItem(itemID, payload string) error { - var created struct { - Number *int `json:"number"` - } - if err := e.pushJSONInto(http.MethodPost, "/project-items", payload, &created); err != nil { - return err - } - if created.Number == nil { - return nil - } - return e.q.SetItemNumber(e.ctx, generated.SetItemNumberParams{ - Number: sql.NullInt64{Int64: int64(*created.Number), Valid: true}, - ID: itemID, - }) -} - -// pushJSON sends a JSON request and handles the response. -// 404/409 are treated as "drop this op" (returns nil). -// Network errors and 5xx are retryable (returns error). -func (e *Engine) pushJSON(method, path string, payload string) error { - return e.pushJSONInto(method, path, payload, nil) -} - -// pushJSONInto is pushJSON with the response decoded into out when the server -// accepted the request. A body that will not decode is not an error: the write -// itself landed, and failing here would re-queue a create the server has -// already applied. The missing field arrives on the next pull instead. -func (e *Engine) pushJSONInto(method, path string, payload string, out any) error { - var body io.Reader - if payload != "" { - body = bytes.NewReader([]byte(payload)) - } - - req, err := http.NewRequestWithContext(e.ctx, method, e.base+path, body) - if err != nil { - return err - } - if body != nil { - req.Header.Set("Content-Type", "application/json") - } - - resp, err := e.client.Do(req) - if err != nil { - return friendlyNetErr(err) - } - defer func() { _ = resp.Body.Close() }() - responseBody, _ := io.ReadAll(resp.Body) - - if err := classifyResponse(resp.StatusCode, responseBody); err != nil { - return err - } - if out != nil && resp.StatusCode < 400 { - _ = json.Unmarshal(responseBody, out) - } - return nil -} - -// pushDelete sends a DELETE request. -func (e *Engine) pushDelete(path string) error { - req, err := http.NewRequestWithContext(e.ctx, http.MethodDelete, e.base+path, nil) - if err != nil { - return err - } - - resp, err := e.client.Do(req) - if err != nil { - return friendlyNetErr(err) - } - defer func() { _ = resp.Body.Close() }() - responseBody, _ := io.ReadAll(resp.Body) - - return classifyResponse(resp.StatusCode, responseBody) -} - -// classifyResponse decides whether a status code means success, drop, or retry. -func classifyResponse(status int, body []byte) error { - switch { - case status < 400: - return nil // success - case status == http.StatusNotFound, status == http.StatusConflict: - return nil // drop: server wins, next pull reconciles - case status >= 500: - return refusal("server error", status, body) - default: - return refusal("push rejected", status, body) - } -} - -// refusal names the status and the reason the API gave in its error body's -// `message`, or the body as it arrived when it carries none. -func refusal(prefix string, status int, body []byte) error { - reason := strings.TrimSpace(string(body)) - var answer struct { - Message string `json:"message"` - } - if json.Unmarshal(body, &answer) == nil && answer.Message != "" { - reason = answer.Message - } - return fmt.Errorf("%s (%d %s): %s", prefix, status, http.StatusText(status), reason) -} - -// friendlyNetErr translates raw network errors into user-readable messages. -// -// A token command's failure arrives through the same call, wrapped as a network -// error. Its own message says what to run, and its timeout is not the API's, so -// it is returned as it is. -func friendlyNetErr(err error) error { - var tokenErr *TokenCommandError - if errors.As(err, &tokenErr) { - return tokenErr - } - msg := err.Error() - switch { - case strings.Contains(msg, "connection refused"): - return fmt.Errorf("cannot reach API server (connection refused)") - case strings.Contains(msg, "no such host"): - return fmt.Errorf("cannot reach API server (DNS lookup failed)") - case strings.Contains(msg, "Timeout") || strings.Contains(msg, "deadline exceeded"): - return fmt.Errorf("API server did not respond (timeout)") - default: - return fmt.Errorf("network error: %w", err) - } -} diff --git a/sync/status.go b/sync/status.go deleted file mode 100644 index dfd3338..0000000 --- a/sync/status.go +++ /dev/null @@ -1,9 +0,0 @@ -package sync - -// SyncStatus represents the current state of the sync engine for TUI display. -type SyncStatus struct { - Connected bool - PendingCount int - LastError string - Syncing bool -} diff --git a/sync/token.go b/sync/token.go deleted file mode 100644 index 73796dd..0000000 --- a/sync/token.go +++ /dev/null @@ -1,153 +0,0 @@ -package sync - -import ( - "bytes" - "context" - "errors" - "fmt" - "io" - "net/http" - "os/exec" - "strings" - gosync "sync" - "time" -) - -// tokenCommandTimeout bounds one run of sync.token_command. A command that -// refreshes its own login reaches an identity provider, which answers well -// inside this when it answers at all. -const tokenCommandTimeout = 10 * time.Second - -// TokenCommandError is sync.token_command failing to print a token. Reason is -// the first line the command wrote to stderr where it wrote one, because that is -// where a CLI says its login has lapsed and what to run. -type TokenCommandError struct { - Command string - Reason string -} - -func (e *TokenCommandError) Error() string { - return fmt.Sprintf("sync.token_command %q: %s", e.Command, e.Reason) -} - -// tokenTransport sends the bearer token sync.token_command prints. -// -// The command runs on the first request and again only after the API refuses -// the token held, so a pull of many requests costs one run. A refused token is -// replaced once and the request sent once more; a second refusal is the API's -// answer, and so is a refusal of a token printed for this request. -type tokenTransport struct { - argv []string - base http.RoundTripper - - mu gosync.Mutex - token string -} - -func newTokenTransport(argv []string, base http.RoundTripper) *tokenTransport { - return &tokenTransport{argv: argv, base: base} -} - -func (t *tokenTransport) RoundTrip(req *http.Request) (*http.Response, error) { - token, printedNow, err := t.held(req.Context()) - if err != nil { - closeBody(req) - return nil, err - } - - resp, err := t.send(req, req.Body, token) - if err != nil || resp.StatusCode != http.StatusUnauthorized || printedNow { - return resp, err - } - // A body cannot be sent twice unless the request can produce it again, which - // http.NewRequest arranges for every in-memory body. - if req.Body != nil && req.GetBody == nil { - return resp, nil - } - - replacement, err := t.replace(req.Context(), token) - if err != nil { - _ = resp.Body.Close() - return nil, err - } - var body io.ReadCloser - if req.GetBody != nil { - if body, err = req.GetBody(); err != nil { - return resp, nil - } - } - _ = resp.Body.Close() - return t.send(req, body, replacement) -} - -func (t *tokenTransport) send(req *http.Request, body io.ReadCloser, token string) (*http.Response, error) { - out := req.Clone(req.Context()) - out.Body = body - out.Header.Set("Authorization", "Bearer "+token) - return t.base.RoundTrip(out) -} - -// held returns the token to send, running the command when none is held. -// printedNow reports that the command printed it for this request. -func (t *tokenTransport) held(ctx context.Context) (token string, printedNow bool, err error) { - t.mu.Lock() - defer t.mu.Unlock() - if t.token != "" { - return t.token, false, nil - } - if t.token, err = runTokenCommand(ctx, t.argv); err != nil { - return "", false, err - } - return t.token, true, nil -} - -// replace runs the command again for a refused token. Requests in flight -// together are refused together, so the first to arrive replaces it and the -// rest take its replacement rather than each running the command. -func (t *tokenTransport) replace(ctx context.Context, refused string) (string, error) { - t.mu.Lock() - defer t.mu.Unlock() - if t.token != "" && t.token != refused { - return t.token, nil - } - token, err := runTokenCommand(ctx, t.argv) - t.token = token - return token, err -} - -// runTokenCommand runs argv without a shell and with no stdin, so a command -// that would prompt fails rather than waiting on a terminal the TUI owns. -func runTokenCommand(ctx context.Context, argv []string) (string, error) { - command := strings.Join(argv, " ") - ctx, cancel := context.WithTimeout(ctx, tokenCommandTimeout) - defer cancel() - - cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) - var stderr bytes.Buffer - cmd.Stderr = &stderr - out, err := cmd.Output() - if err != nil { - return "", &TokenCommandError{Command: command, Reason: failureReason(ctx, err, stderr.String())} - } - token := strings.TrimSpace(string(out)) - if token == "" { - return "", &TokenCommandError{Command: command, Reason: "printed no token"} - } - return token, nil -} - -func failureReason(ctx context.Context, err error, stderr string) string { - if errors.Is(ctx.Err(), context.DeadlineExceeded) { - return fmt.Sprintf("did not finish within %s", tokenCommandTimeout) - } - if line, _, _ := strings.Cut(strings.TrimSpace(stderr), "\n"); line != "" { - return line - } - return err.Error() -} - -func closeBody(req *http.Request) { - if req.Body != nil { - _ = req.Body.Close() - } -} diff --git a/sync/token_test.go b/sync/token_test.go deleted file mode 100644 index ad0f59a..0000000 --- a/sync/token_test.go +++ /dev/null @@ -1,257 +0,0 @@ -package sync_test - -import ( - "errors" - "io" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "strconv" - "strings" - goSync "sync" - "testing" - - "github.com/datapointchris/projects/sync" -) - -// seenRequest is one request as the API received it. -type seenRequest struct { - Authorization, Method, Path, Body string -} - -// bearerAPI answers a pull with empty lists for a request whose Authorization -// header is the accepted one, and 401 for any other. It records every request. -type bearerAPI struct { - *httptest.Server - mu goSync.Mutex - accepted string - seen []seenRequest -} - -func newBearerAPI(t *testing.T, accepted string) *bearerAPI { - t.Helper() - api := &bearerAPI{accepted: accepted} - api.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, _ := io.ReadAll(r.Body) - api.mu.Lock() - authorization := r.Header.Get("Authorization") - api.seen = append(api.seen, seenRequest{authorization, r.Method, r.URL.Path, string(body)}) - ok := authorization == api.accepted - api.mu.Unlock() - if !ok { - w.WriteHeader(http.StatusUnauthorized) - return - } - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte("[]")) - })) - t.Cleanup(api.Close) - return api -} - -func (a *bearerAPI) accept(authorization string) { - a.mu.Lock() - defer a.mu.Unlock() - a.accepted = authorization -} - -func (a *bearerAPI) requests() []seenRequest { - a.mu.Lock() - defer a.mu.Unlock() - return append([]seenRequest(nil), a.seen...) -} - -// writeTokenScript writes body as an executable shell script and returns its -// path, which holds no space, so it survives being split into a command. -func writeTokenScript(t *testing.T, dir, body string) string { - t.Helper() - path := filepath.Join(dir, "print-token") - if err := os.WriteFile(path, []byte("#!/bin/sh\n"+body), 0o700); err != nil { - t.Fatalf("writing token script: %v", err) - } - return path -} - -// tokenCommand returns a command printing tokens[n-1] on its nth run, and the -// last token on every run after, with a count of its runs so far. -func tokenCommand(t *testing.T, tokens ...string) (string, func() int) { - t.Helper() - dir := t.TempDir() - if err := os.WriteFile(filepath.Join(dir, "tokens"), []byte(strings.Join(tokens, "\n")+"\n"), 0o600); err != nil { - t.Fatalf("writing tokens: %v", err) - } - script := writeTokenScript(t, dir, `dir=$(dirname "$0") -runs=$(( $(cat "$dir/runs" 2>/dev/null || echo 0) + 1 )) -echo "$runs" > "$dir/runs" -total=$(wc -l < "$dir/tokens") -[ "$runs" -gt "$total" ] && line=$total || line=$runs -sed -n "${line}p" "$dir/tokens" -`) - runs := func() int { - data, err := os.ReadFile(filepath.Join(dir, "runs")) - if err != nil { - return 0 - } - n, _ := strconv.Atoi(strings.TrimSpace(string(data))) - return n - } - return script, runs -} - -func mustPull(t *testing.T, engine *sync.Engine) { - t.Helper() - if err := engine.Pull(t.Context()); err != nil { - t.Fatalf("pull: %v", err) - } -} - -// A pull is several requests, and the command runs for the first alone. -func TestEveryRequestCarriesTheTokenTheCommandPrints(t *testing.T) { - command, runs := tokenCommand(t, "tok-1") - api := newBearerAPI(t, "Bearer tok-1") - engine := sync.New(openTestDB(t), api.URL, command) - - mustPull(t, engine) - mustPull(t, engine) - - seen := api.requests() - if len(seen) < 4 { - t.Fatalf("two pulls made %d requests, want at least 4", len(seen)) - } - for _, r := range seen { - if r.Authorization != "Bearer tok-1" { - t.Errorf("%s %s carried %q, want the printed token", r.Method, r.Path, r.Authorization) - } - } - if got := runs(); got != 1 { - t.Errorf("the command ran %d times for %d requests, want once", got, len(seen)) - } -} - -// An access token expires while the TUI is open. The next request is refused, -// and the command runs once more for a token the API takes. -func TestAnExpiredTokenIsReplacedAndTheRequestSentAgain(t *testing.T) { - command, runs := tokenCommand(t, "tok-1", "tok-2") - api := newBearerAPI(t, "Bearer tok-1") - engine := sync.New(openTestDB(t), api.URL, command) - mustPull(t, engine) - - api.accept("Bearer tok-2") - mustPull(t, engine) - - if got := runs(); got != 2 { - t.Errorf("the command ran %d times, want 2", got) - } - seen := api.requests() - if last := seen[len(seen)-1]; last.Authorization != "Bearer tok-2" { - t.Errorf("the last request carried %q, want the replacement", last.Authorization) - } -} - -// Running the command again for a token it printed a moment ago would print the -// same refusal, so the refusal is the answer. -func TestATokenRefusedAsSoonAsItIsPrintedIsTheAnswer(t *testing.T) { - command, runs := tokenCommand(t, "tok-1", "tok-2") - api := newBearerAPI(t, "Bearer nothing-it-prints") - - if err := sync.New(openTestDB(t), api.URL, command).Pull(t.Context()); err == nil { - t.Fatal("a pull whose every request is refused succeeded") - } - if got := runs(); got != 1 { - t.Errorf("the command ran %d times, want once", got) - } -} - -func TestARefusedReplacementIsTheAnswer(t *testing.T) { - command, runs := tokenCommand(t, "tok-1", "tok-2", "tok-3") - api := newBearerAPI(t, "Bearer tok-1") - engine := sync.New(openTestDB(t), api.URL, command) - mustPull(t, engine) - - api.accept("Bearer nothing-it-prints") - if err := engine.Pull(t.Context()); err == nil { - t.Fatal("a pull whose every request is refused succeeded") - } - if got := runs(); got != 2 { - t.Errorf("the command ran %d times, want 2: the first token and one replacement", got) - } -} - -// A create refused for an expired token has not reached the server, so it is -// sent again whole. -func TestAPushRefusedForAnExpiredTokenIsResentWithItsBody(t *testing.T) { - command, _ := tokenCommand(t, "tok-1", "tok-2") - api := newBearerAPI(t, "Bearer tok-1") - engine := sync.New(openTestDB(t), api.URL, command) - mustPull(t, engine) - - api.accept("Bearer tok-2") - if err := engine.QueueOp(sync.OpCreateProject, "proj-1", map[string]string{"id": "proj-1", "name": "Boxes"}); err != nil { - t.Fatalf("queueing a create: %v", err) - } - mustPull(t, engine) - - var posts []seenRequest - for _, r := range api.requests() { - if r.Method == http.MethodPost && r.Path == "/api/v1/projects" { - posts = append(posts, r) - } - } - if len(posts) != 2 { - t.Fatalf("the create was posted %d times, want 2: refused, then resent", len(posts)) - } - if posts[1].Authorization != "Bearer tok-2" { - t.Errorf("the resend carried %q, want the replacement token", posts[1].Authorization) - } - if posts[1].Body == "" || posts[1].Body != posts[0].Body { - t.Errorf("the resend's body %q, want the original %q", posts[1].Body, posts[0].Body) - } -} - -// A CLI whose login has lapsed says so on stderr, and that line is what the -// status bar needs, rather than a network error. -func TestAFailingCommandSaysWhatItPrinted(t *testing.T) { - script := writeTokenScript(t, t.TempDir(), "echo 'Not logged in. Run the login command.' >&2\nexit 1\n") - api := newBearerAPI(t, "Bearer tok-1") - - err := sync.New(openTestDB(t), api.URL, script).Pull(t.Context()) - - var tokenErr *sync.TokenCommandError - if !errors.As(err, &tokenErr) { - t.Fatalf("pull error %v, want a TokenCommandError", err) - } - if tokenErr.Reason != "Not logged in. Run the login command." { - t.Errorf("reason %q, want the command's stderr", tokenErr.Reason) - } - if strings.Contains(err.Error(), "network error") { - t.Errorf("pull error %q calls the command's failure a network error", err) - } - if seen := api.requests(); len(seen) != 0 { - t.Errorf("%d requests reached the API without a token", len(seen)) - } -} - -func TestACommandPrintingNothingIsAFailure(t *testing.T) { - script := writeTokenScript(t, t.TempDir(), "exit 0\n") - api := newBearerAPI(t, "Bearer tok-1") - - err := sync.New(openTestDB(t), api.URL, script).Pull(t.Context()) - - var tokenErr *sync.TokenCommandError - if !errors.As(err, &tokenErr) || tokenErr.Reason != "printed no token" { - t.Fatalf("pull error %v, want a TokenCommandError saying it printed no token", err) - } -} - -// A dev API sets its own identity and needs no header. -func TestNoTokenCommandSendsNoAuthorization(t *testing.T) { - api := newBearerAPI(t, "") - mustPull(t, sync.New(openTestDB(t), api.URL, "")) - - for _, r := range api.requests() { - if r.Authorization != "" { - t.Errorf("%s %s carried %q, want no Authorization header", r.Method, r.Path, r.Authorization) - } - } -} diff --git a/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-item-pane.style b/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-item-pane.style index 3cdd220..262a194 100644 --- a/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-item-pane.style +++ b/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-item-pane.style @@ -8,25 +8,25 @@ 4: 76-76 fg=4 5: 25-25 fg=4 5: 76-76 fg=4 -6: 25-76 fg=4 -7: 25-25 fg=4 -7: 76-76 fg=4 +6: 25-25 fg=4 +6: 76-76 fg=4 +7: 25-76 fg=4 8: 25-25 fg=4 8: 76-76 fg=4 9: 25-25 fg=4 9: 76-76 fg=4 -10: 25-76 fg=4 -11: 25-25 fg=4 -11: 76-76 fg=4 +10: 25-25 fg=4 +10: 76-76 fg=4 +11: 25-76 fg=4 12: 25-25 fg=4 12: 76-76 fg=4 13: 25-25 fg=4 13: 76-76 fg=4 14: 25-25 fg=4 14: 76-76 fg=4 -15: 25-76 fg=4 -16: 25-25 fg=4 -16: 76-76 fg=4 +15: 25-25 fg=4 +15: 76-76 fg=4 +16: 25-76 fg=4 17: 25-25 fg=4 17: 76-76 fg=4 18: 25-25 fg=4 diff --git a/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-item-pane.txt b/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-item-pane.txt index 826cee9..ce36295 100644 --- a/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-item-pane.txt +++ b/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-item-pane.txt @@ -1,3 +1,4 @@ + │ p Manage project membership │ │ b Link dependency (blocked by) │ │ B Unlink dependency │ │ T Dependency browser (map + │ @@ -20,8 +21,7 @@ │ 2 Filter: all + archived │ │ (toggle) │ │ 0 Filter: reset │ - │ R Sync now (runs automatically │ - │ too) │ + │ R Read the data again now │ │ q Ctrl+C Quit │ │ ? Toggle this help │ │ │ diff --git a/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-project-pane.style b/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-project-pane.style index 406fbc2..4ead506 100644 --- a/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-project-pane.style +++ b/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-project-pane.style @@ -8,9 +8,9 @@ 4: 76-76 fg=4 5: 25-25 fg=4 5: 76-76 fg=4 -6: 25-76 fg=4 -7: 25-25 fg=4 -7: 76-76 fg=4 +6: 25-25 fg=4 +6: 76-76 fg=4 +7: 25-76 fg=4 8: 25-25 fg=4 8: 76-76 fg=4 9: 25-25 fg=4 @@ -25,9 +25,9 @@ 13: 76-76 fg=4 14: 25-25 fg=4 14: 76-76 fg=4 -15: 25-76 fg=4 -16: 25-25 fg=4 -16: 76-76 fg=4 +15: 25-25 fg=4 +15: 76-76 fg=4 +16: 25-76 fg=4 17: 25-25 fg=4 17: 76-76 fg=4 18: 25-25 fg=4 diff --git a/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-project-pane.txt b/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-project-pane.txt index f3ac5a1..f3552a4 100644 --- a/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-project-pane.txt +++ b/testdata/TestRealBinaryFlows/help-taller-than-the-window/from-the-project-pane.txt @@ -1,3 +1,4 @@ + │ a Add project │ │ m Move/reorder project │ │ C Show/hide shelved and closed │ │ projects │ @@ -20,8 +21,7 @@ │ 2 Filter: all + archived │ │ (toggle) │ │ 0 Filter: reset │ - │ R Sync now (runs automatically │ - │ too) │ + │ R Read the data again now │ │ q Ctrl+C Quit │ │ ? Toggle this help │ │ │ diff --git a/tui/app.go b/tui/app.go index c644b0b..e6b9e3a 100644 --- a/tui/app.go +++ b/tui/app.go @@ -2,7 +2,6 @@ package tui import ( "cmp" - "context" "fmt" "slices" "strconv" @@ -15,7 +14,6 @@ import ( "github.com/datapointchris/projects/backend" "github.com/datapointchris/projects/model" - "github.com/datapointchris/projects/sync" ) // App is the top-level Bubble Tea model for the TUI. @@ -121,25 +119,18 @@ type App struct { errorMsg string // transient error shown in status bar loading bool // true while an async operation is in-flight - // Sync - syncEngine *sync.Engine - syncStatus sync.SyncStatus - syncInterval time.Duration - where Where } // Where names the data the board shows, for its status line: a database on this -// machine, or the API at APIHost. A database with sync on is named by its engine. +// machine, or the API at APIHost. type Where struct { DatabasePath string APIHost string } -// NewApp creates a new TUI application backed by the given Backend. syncEngine -// is nil unless the database syncs. syncInterval is how often the board rereads -// what it shows: a pull with sync on, and a read of the API in API mode. -func NewApp(b backend.Backend, syncEngine *sync.Engine, where Where, syncInterval time.Duration) *App { +// NewApp creates a new TUI application backed by the given Backend. +func NewApp(b backend.Backend, where Where) *App { ti := textinput.New() ti.CharLimit = 200 ti.SetStyles(inputStyles()) @@ -155,8 +146,6 @@ func NewApp(b backend.Backend, syncEngine *sync.Engine, where Where, syncInterva itemBlockers: make(map[string][]model.ProjectItem), titleInput: ti, depFilter: df, - syncEngine: syncEngine, - syncInterval: syncInterval, where: where, } } @@ -270,18 +259,11 @@ type errMsg struct{ error } // moved flashSeq on, so an older flash's timer leaves it on screen. type clearStatusMsg struct{ seq int } -// Sync messages. A pull carries whether the user asked for it: an automatic one -// runs every syncInterval, so announcing it would flash the status bar forever -// and a transient failure would pin an error banner over a working app. -type ( - syncStatusMsg sync.SyncStatus - syncPullTickMsg struct{} - syncPullDoneMsg struct{ manual bool } - syncPullErrMsg struct { - error - manual bool - } -) +// rereadTickMsg reads the API again, since another client may have written. +type rereadTickMsg struct{} + +// apiRereadInterval is how stale the board may grow through an API. +const apiRereadInterval = 2 * time.Minute const statusFlashDuration = 3 * time.Second @@ -295,9 +277,8 @@ func (m *App) flash(msg string) tea.Cmd { } // itemHandle names an item the way it is typed on the command line: its number, -// or the UUID tail while it is still waiting for one. An item created here -// while the API was unreachable has no number until its create is pushed, and -// showing nothing in that window would leave the row unnameable. +// or the UUID tail for an item that has none, which a database once kept as a +// copy of an API may still hold. func itemHandle(item model.ProjectItem) string { if item.Number != nil { return strconv.Itoa(*item.Number) @@ -327,34 +308,12 @@ func truncate(s string, maxWidth int) string { return string(runes[:maxWidth-1]) + "…" } -// --- Sync commands --- - -func syncPullCmd(e *sync.Engine, manual bool) tea.Cmd { - return func() tea.Msg { - if err := e.Pull(context.Background()); err != nil { - return syncPullErrMsg{error: err, manual: manual} - } - return syncPullDoneMsg{manual: manual} - } -} - -func syncStatusTickCmd(e *sync.Engine) tea.Cmd { - return tea.Tick(2*time.Second, func(_ time.Time) tea.Msg { - return syncStatusMsg(e.Status()) - }) -} - -func syncPullTickCmd(d time.Duration) tea.Cmd { - return tea.Tick(d, func(_ time.Time) tea.Msg { - return syncPullTickMsg{} +func rereadTickCmd() tea.Cmd { + return tea.Tick(apiRereadInterval, func(_ time.Time) tea.Msg { + return rereadTickMsg{} }) } -// safeToAutoPull reports whether a background reconcile can run without pulling -// the ground out from under the user. A pull rewrites items, memberships and -// ordering wholesale, which would corrupt a grab in progress or refresh a -// reader into a snapshot of something else. Every other mode defers to -// the next tick rather than skipping the pull outright. // projectStatusFilter is what the project pane is currently asking for. Closing // a project is what takes it out of the list, so the default hides the terminal // ones and `C` is how you get at them. @@ -365,7 +324,11 @@ func (m *App) projectStatusFilter() string { return model.StatusActive } -func (m *App) safeToAutoPull() bool { +// safeToReread reports whether a background read can land without pulling the +// ground out from under the user. A read replaces the lists and their order, +// which would corrupt a grab in progress or refresh a reader into a snapshot of +// something else. Every other mode defers to the next tick. +func (m *App) safeToReread() bool { return m.appMode == modeNormal } @@ -993,16 +956,8 @@ func (m *App) groupHeaderAt(idx int) string { } func (m *App) Init() tea.Cmd { - if m.syncEngine != nil { - return tea.Batch( - fetchProjectsCmd(m.backend, m.projectStatusFilter()), - syncPullCmd(m.syncEngine, false), - syncStatusTickCmd(m.syncEngine), - syncPullTickCmd(m.syncInterval), - ) - } if m.where.APIHost != "" { - return tea.Batch(fetchProjectsCmd(m.backend, m.projectStatusFilter()), syncPullTickCmd(m.syncInterval)) + return tea.Batch(fetchProjectsCmd(m.backend, m.projectStatusFilter()), rereadTickCmd()) } return fetchProjectsCmd(m.backend, m.projectStatusFilter()) } @@ -1252,40 +1207,14 @@ func (m *App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.errorMsg = msg.Error() return m, nil - case syncPullTickMsg: - next := syncPullTickCmd(m.syncInterval) - switch { - case !m.safeToAutoPull(): + case rereadTickMsg: + // A tick re-arms even when it skips the read. One that returned nothing + // would end the rereads for the rest of the session. + next := rereadTickCmd() + if !m.safeToReread() { return m, next - case m.syncEngine != nil: - return m, tea.Batch(syncPullCmd(m.syncEngine, false), next) - case m.where.APIHost != "": - // Another client may have written since the last read. - return m, tea.Batch(fetchProjectsCmd(m.backend, m.projectStatusFilter()), next) - } - return m, next - - case syncPullDoneMsg: - if !msg.manual { - return m, fetchProjectsCmd(m.backend, m.projectStatusFilter()) - } - return m, tea.Batch(fetchProjectsCmd(m.backend, m.projectStatusFilter()), m.flash("Synced with server")) - - case syncPullErrMsg: - // An automatic pull failing is not an error the user did anything to - // cause. The status bar already reads SYNC ERR off the engine status; - // hijacking the bar every interval would bury real messages. - if msg.manual { - m.errorMsg = fmt.Sprintf("Sync: %v", msg.error) } - return m, nil - - case syncStatusMsg: - m.syncStatus = sync.SyncStatus(msg) - if m.syncEngine != nil { - return m, syncStatusTickCmd(m.syncEngine) - } - return m, nil + return m, tea.Batch(fetchProjectsCmd(m.backend, m.projectStatusFilter()), next) } return m, nil @@ -1626,14 +1555,11 @@ func (m *App) handleNormalKey(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { // --- Phase 6: Advanced --- case "R": - switch { - case m.syncEngine != nil: - return m, syncPullCmd(m.syncEngine, true) - case m.where.APIHost != "": - return m, tea.Batch(fetchProjectsCmd(m.backend, m.projectStatusFilter()), m.flash("Read again from "+m.where.APIHost)) + source := m.where.APIHost + if source == "" { + source = m.where.DatabasePath } - disabled := m.flash("Sync is disabled — running local-only") - return m, disabled + return m, tea.Batch(fetchProjectsCmd(m.backend, m.projectStatusFilter()), m.flash("Read again from "+source)) case "?": m.appMode = modeHelp @@ -2581,7 +2507,7 @@ func (m *App) renderHelpOverlay() string { 1 Filter: blocked only (toggle) 2 Filter: all + archived (toggle) 0 Filter: reset - R Sync now (runs automatically too) + R Read the data again now q Ctrl+C Quit ? Toggle this help` @@ -3324,19 +3250,6 @@ func clipHints(hints string, maxWidth int) string { func (m *App) renderStatusBar() string { var modeStr string switch { - case m.syncEngine != nil: - target := dimStyle.Render(" " + m.syncEngine.APIURL()) - s := m.syncStatus - switch { - case s.Syncing: - modeStr = syncingStyle.Render("SYNCING...") + target - case s.LastError != "": - modeStr = syncErrorStyle.Render("SYNC ERR") + target - case s.PendingCount > 0: - modeStr = syncPendingStyle.Render(fmt.Sprintf("PENDING (%d)", s.PendingCount)) + target - default: - modeStr = syncOKStyle.Render("SYNCED") + target - } case m.where.APIHost != "": modeStr = modeAPIStyle.Render("API") + dimStyle.Render(" "+m.where.APIHost) default: @@ -3365,7 +3278,7 @@ func (m *App) renderStatusBar() string { // rather than wrapped, and truncated before styling so no escape // sequence is cut in half. The 5 is the two spaces this function adds, // the style's own left and right padding, and the one column of gap - // that always separates the hints from the sync status. + // that always separates the hints from the mode on the right. left = clipHints(m.statusBarHints(), m.width-modeWidth-5) } diff --git a/tui/app_test.go b/tui/app_test.go index c7ac2dc..6b84ebc 100644 --- a/tui/app_test.go +++ b/tui/app_test.go @@ -1,14 +1,9 @@ package tui import ( - "encoding/json" - "errors" - "net/http" - "net/http/httptest" "slices" "strconv" "strings" - goSync "sync" "testing" "time" @@ -18,7 +13,6 @@ import ( "github.com/datapointchris/projects/model" "github.com/datapointchris/projects/service" "github.com/datapointchris/projects/store/sqlite" - "github.com/datapointchris/projects/sync" ) // newTestApp returns an app already showing one project's items, sized to @@ -44,8 +38,7 @@ func newTestAppWithItems(t *testing.T, width, height int, inputs []model.CreateP } t.Cleanup(func() { _ = database.Close() }) - // AssigningItemNumbers because the harness passes a nil sync engine, which - // is the sync-off case: nothing upstream will number these items. + // AssigningItemNumbers, as database mode builds it. b := service.New(sqlite.New(database), service.AssigningItemNumbers()) project, err := b.CreateProject(model.CreateProject{Name: "work"}) if err != nil { @@ -58,7 +51,7 @@ func newTestAppWithItems(t *testing.T, width, height int, inputs []model.CreateP } } - app := NewApp(b, nil, Where{DatabasePath: ":memory:"}, 2*time.Minute) + app := NewApp(b, Where{DatabasePath: ":memory:"}) send(t, app, tea.WindowSizeMsg{Width: width, Height: height}) send(t, app, app.Init()()) send(t, app, app.fetchItems()()) @@ -98,7 +91,7 @@ func drain(t *testing.T, app *App, cmd tea.Cmd, depth int) { } // runCmd abandons a command that does not produce a message promptly — -// flash and sync-status commands are tea.Tick timers that would otherwise +// flash and reread commands are tea.Tick timers that would otherwise // block the test for their full duration. func runCmd(cmd tea.Cmd) tea.Msg { result := make(chan tea.Msg, 1) @@ -289,56 +282,39 @@ func TestCreatedTaskAppearsAsRow(t *testing.T) { } } -// The auto-pull timer re-arms itself from its own message. A tick that returns -// no command ends background sync for the rest of the session, so the skipped -// ticks have to re-arm too — otherwise opening one modal at the wrong moment -// silently reverts the tool to manual syncing. -func TestSyncTickRearmsEvenWhenItSkipsThePull(t *testing.T) { +// The reread timer re-arms itself from its own message. A tick that returns no +// command ends the rereads for the rest of the session, so the skipped ticks +// have to re-arm too — otherwise opening one modal at the wrong moment leaves the +// board showing what the API held when it opened. +func TestRereadTickRearmsEvenWhenItSkipsTheRead(t *testing.T) { app := newTestApp(t, 80, 24, "Alpha") for _, mode := range []appMode{modeNormal, modeAddItem, modeMove, modeHelp} { app.appMode = mode - _, cmd := app.Update(syncPullTickMsg{}) + _, cmd := app.Update(rereadTickMsg{}) if cmd == nil { - t.Errorf("tick in mode %v returned no command: the sync timer is now dead", mode) + t.Errorf("tick in mode %v returned no command: the reread timer is now dead", mode) } } } -// safeToAutoPull gates the reconcile, not the timer. A pull rewrites items and -// ordering wholesale, which would move the ground under a grab or a text entry. -func TestAutoPullOnlyRunsInNormalMode(t *testing.T) { +// safeToReread gates the read, not the timer. A read replaces the lists and +// their order, which would move the ground under a grab or a text entry. +func TestRereadOnlyRunsInNormalMode(t *testing.T) { app := newTestApp(t, 80, 24, "Alpha") app.appMode = modeNormal - if !app.safeToAutoPull() { - t.Error("normal mode must allow the background reconcile") + if !app.safeToReread() { + t.Error("normal mode must allow the background read") } for _, mode := range []appMode{modeAddItem, modeExternalEdit, modeMove, modeMoveProject, modeReader} { app.appMode = mode - if app.safeToAutoPull() { - t.Errorf("mode %v must defer the reconcile to the next tick", mode) + if app.safeToReread() { + t.Errorf("mode %v must defer the read to the next tick", mode) } } } -// An automatic pull lands every syncInterval. Announcing it would keep the -// status bar permanently flashing and bury the messages the user acted to see. -func TestAutomaticPullIsSilentAndManualPullIsNot(t *testing.T) { - app := newTestApp(t, 80, 24, "Alpha") - - app.statusMsg = "" - send(t, app, syncPullDoneMsg{manual: false}) - if app.statusMsg != "" { - t.Errorf("automatic pull announced itself: %q", app.statusMsg) - } - - send(t, app, syncPullDoneMsg{manual: true}) - if app.statusMsg == "" { - t.Error("a pull the user asked for should confirm it happened") - } -} - // Each flash lasts its full duration. A second flash raised before the first // one's timer fires must outlive that timer. func TestOlderFlashTimerLeavesNewerFlashShowing(t *testing.T) { @@ -359,101 +335,6 @@ func TestOlderFlashTimerLeavesNewerFlashShowing(t *testing.T) { } } -// A failing automatic pull is not something the user did. The status bar already -// reads SYNC ERR off the engine; pinning an error banner every interval would -// make a briefly unreachable API look like a broken app. -func TestAutomaticPullFailureDoesNotHijackTheStatusBar(t *testing.T) { - app := newTestApp(t, 80, 24, "Alpha") - - send(t, app, syncPullErrMsg{error: errors.New("connection refused"), manual: false}) - if app.errorMsg != "" { - t.Errorf("automatic pull failure took over the status bar: %q", app.errorMsg) - } - - send(t, app, syncPullErrMsg{error: errors.New("connection refused"), manual: true}) - if app.errorMsg == "" { - t.Error("a pull the user asked for must report why it failed") - } -} - -// End-to-end for the background timer: a tick in normal mode must actually -// reconcile with the server and re-arm, and a tick in a modal must do neither -// the pull nor lose the timer. This is the behavior that removes the need to -// drop out of the TUI and run `projects sync` by hand. -func TestSyncTickReconcilesWithTheServer(t *testing.T) { - var mu goSync.Mutex - var hits int - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - mu.Lock() - hits++ - mu.Unlock() - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/api/v1/projects": - _ = json.NewEncoder(w).Encode([]model.Project{ - {ID: "proj-1", Name: "Remote", Position: 0, CreatedAt: time.Now()}, - }) - case "/api/v1/project-items": - _ = json.NewEncoder(w).Encode([]model.ProjectItem{ - {ID: "01900000-0000-7000-8000-00000000a001", Title: "Added Elsewhere", CreatedAt: time.Now(), UpdatedAt: time.Now()}, - }) - case "/api/v1/project-items/01900000-0000-7000-8000-00000000a001": - _ = json.NewEncoder(w).Encode(model.ProjectItemDetail{ - ProjectItem: model.ProjectItem{ID: "01900000-0000-7000-8000-00000000a001", Title: "Added Elsewhere", CreatedAt: time.Now(), UpdatedAt: time.Now()}, - Projects: []model.Project{{ID: "proj-1", Name: "Remote"}}, - }) - default: - _ = json.NewEncoder(w).Encode([]model.ProjectItemTask{}) - } - }) - ts := httptest.NewServer(handler) - t.Cleanup(ts.Close) - - database, err := sqlite.Open(":memory:") - if err != nil { - t.Fatalf("opening test db: %v", err) - } - t.Cleanup(func() { _ = database.Close() }) - - engine := sync.New(database, ts.URL, "") - t.Cleanup(engine.Stop) - app := NewApp(service.New(sqlite.New(database)), engine, Where{DatabasePath: ":memory:"}, 20*time.Millisecond) - send(t, app, tea.WindowSizeMsg{Width: 80, Height: 24}) - - countHits := func() int { - mu.Lock() - defer mu.Unlock() - return hits - } - - app.appMode = modeAddItem - send(t, app, syncPullTickMsg{}) - if countHits() != 0 { - t.Fatalf("a tick during text entry reconciled anyway: %d requests", countHits()) - } - - app.appMode = modeNormal - send(t, app, syncPullTickMsg{}) - if countHits() == 0 { - t.Fatal("a tick in normal mode never reached the server") - } - - // An item that only ever existed on the server has to land in the TUI - // without the user asking for it. - if _, err := app.backend.GetItem("01900000-0000-7000-8000-00000000a001"); err != nil { - t.Fatalf("background pull did not bring the remote item local: %v", err) - } - found := false - for _, p := range app.projects { - if p.Name == "Remote" { - found = true - } - } - if !found { - t.Errorf("pulled project never reached the rendered model: %v", app.projects) - } -} - // The row is where the handle has to be legible: the id column was 8 characters // of hex tail, which resolves but which nobody can read back off the screen. func TestItemRowShowsTheNumber(t *testing.T) { @@ -467,7 +348,7 @@ func TestItemRowShowsTheNumber(t *testing.T) { } for _, item := range items { if item.Number == nil { - t.Fatalf("item %q has no number in a sync-off database", item.Title) + t.Fatalf("item %q has no number in a database", item.Title) } if !strings.Contains(view, strconv.Itoa(*item.Number)) { t.Errorf("view is missing the handle %d for %q", *item.Number, item.Title) diff --git a/tui/editor.go b/tui/editor.go index b1a2864..3700146 100644 --- a/tui/editor.go +++ b/tui/editor.go @@ -80,7 +80,7 @@ func projectDescriptionFileName(p model.Project) string { // openInEditor hands the terminal to the editor on a temporary file holding // text. The app sits in modeExternalEdit meanwhile, which keeps the background -// pull from rewriting the row the save is about to update. +// read from replacing the row the save is about to update. func (m *App) openInEditor(target editTarget, fileName, text string) tea.Cmd { dir, err := os.MkdirTemp("", "projects-") if err != nil { diff --git a/tui/realdata_test.go b/tui/realdata_test.go index 26c2e2f..6d36a0d 100644 --- a/tui/realdata_test.go +++ b/tui/realdata_test.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "testing" - "time" tea "charm.land/bubbletea/v2" @@ -41,7 +40,7 @@ func TestRealDatabaseSurvivesEverySizeAndProject(t *testing.T) { for _, size := range sizes { width, height := size[0], size[1] - app := NewApp(service.New(sqlite.New(database)), nil, Where{DatabasePath: path}, 2*time.Minute) + app := NewApp(service.New(sqlite.New(database)), Where{DatabasePath: path}) send(t, app, tea.WindowSizeMsg{Width: width, Height: height}) send(t, app, app.Init()()) app.activePane = itemPane diff --git a/tui/styles.go b/tui/styles.go index 911e327..6ba07fe 100644 --- a/tui/styles.go +++ b/tui/styles.go @@ -122,18 +122,6 @@ var ( modeAPIStyle = lipgloss.NewStyle(). Foreground(lipgloss.Color("4")). Bold(true) - syncOKStyle = lipgloss.NewStyle(). - Foreground(lipgloss.Color("2")). - Bold(true) - syncingStyle = lipgloss.NewStyle(). - Foreground(lipgloss.Color("3")). - Bold(true) - syncPendingStyle = lipgloss.NewStyle(). - Foreground(lipgloss.Color("3")). - Bold(true) - syncErrorStyle = lipgloss.NewStyle(). - Foreground(lipgloss.Color("1")). - Bold(true) // Titles paneTitleStyle = lipgloss.NewStyle(). From 69c12ab0b100ffd78d0ae4b8b7e277e1389f6bfa Mon Sep 17 00:00:00 2001 From: Chris Birch Date: Sun, 8 Nov 2026 02:06:19 -0500 Subject: [PATCH 2/6] fix(store): refuse to open a synced database whose push queue holds edits A row in pending_sync is an edit made on a synced copy that never reached its API, and the queue is the only record of it. Dropping the queue on the first open lost that record with no word. The open now refuses while the queue holds a row, naming how many and the API they were for, with the two ways out: push them with v1.24.2's `projects sync`, or discard them. An empty queue drops as before, with sync_state and sync_origin, which hold nothing a person wrote. A failed open names the database it was reading, so the refusal says which file to act on. Agent-Name: claude-code-unattended Agent-Session: 43e2375e-0270-4942-8076-19e8ab6a2337 Agent-Run: 01a1b9ec-c540-7532-8ef9-925d1983656e Worklog: 2407.1 --- cli/data_source.go | 2 +- store/sqlite/open.go | 31 +++++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/cli/data_source.go b/cli/data_source.go index d0a7954..38abcae 100644 --- a/cli/data_source.go +++ b/cli/data_source.go @@ -187,7 +187,7 @@ func openDatabase(cfg *config.Config) (*dataSource, error) { } database, err := sqlite.Open(cfg.Local.DBPath) if err != nil { - return nil, fmt.Errorf("opening database: %w", err) + return nil, fmt.Errorf("opening %s: %w", homeAsTilde(cfg.Local.DBPath), err) } source := &dataSource{cfg: cfg, database: database, local: LocalService(database, repos.DefaultPath())} diff --git a/store/sqlite/open.go b/store/sqlite/open.go index 658ba68..a49da41 100644 --- a/store/sqlite/open.go +++ b/store/sqlite/open.go @@ -135,9 +135,27 @@ CREATE TABLE IF NOT EXISTS project_statuses (name TEXT PRIMARY KEY);` } // dropSyncTables removes what a database kept while it was a copy of an API: the -// queue of pushes, the pull times, and the API it was bound to. None of it is a -// project, an item or anything hanging off one, so every row a person wrote stays. +// queue of pushes, the pull times, and the API it was bound to. It refuses while +// the queue holds a row, because that edit never reached the API and the queue is +// the only record saying so. The other two hold nothing a person wrote. +// +// MIGRATION: retires with config/retired_sync.go. func dropSyncTables(db *sql.DB) error { + var name string + err := db.QueryRow("SELECT name FROM sqlite_master WHERE type='table' AND name='pending_sync'").Scan(&name) + if err != nil && err != sql.ErrNoRows { + return fmt.Errorf("checking pending_sync: %w", err) + } + if err == nil { + var queued int + if err := db.QueryRow("SELECT COUNT(*) FROM pending_sync").Scan(&queued); err != nil { + return fmt.Errorf("counting pending_sync: %w", err) + } + if queued > 0 { + return fmt.Errorf("%d edits here were queued for %s and never reached it. Push them with `projects sync` "+ + "from v1.24.2, or discard them with DELETE FROM pending_sync", queued, boundAPI(db)) + } + } const drop = ` DROP TABLE IF EXISTS pending_sync; DROP TABLE IF EXISTS sync_state; @@ -148,6 +166,15 @@ DROP TABLE IF EXISTS sync_origin;` return nil } +// boundAPI is the API a synced database was bound to, as its refusal names it. +func boundAPI(db *sql.DB) string { + var url string + if err := db.QueryRow("SELECT api_url FROM sync_origin WHERE id = 1").Scan(&url); err != nil { + return "the API this database synced with" + } + return url +} + // migrateOrphanedChildRows deletes rows the cascades would never have allowed: // memberships, dependencies and tasks whose parent is gone. Every database // written before foreign keys were actually enforced has them — a deleted From bcc34497ea4e59a94ba4433c4efed41ca2c7ffa4 Mon Sep 17 00:00:00 2001 From: Chris Birch Date: Sun, 8 Nov 2026 02:06:50 -0500 Subject: [PATCH 3/6] refactor(service): number every item the service creates AssigningItemNumbers existed so a synced copy could leave numbering to its API. With the copy gone, both entry points passed it, and only test helpers built a service without it. Numbering is now what CreateItem always does, from the store's NextItemNumber. The Postgres store's nil-number check on CreateItem goes with the option, because nothing can reach it. RestoreItem keeps it, because its number comes from an undo record on disk. Agent-Name: claude-code-unattended Agent-Session: 43e2375e-0270-4942-8076-19e8ab6a2337 Agent-Run: 01a1b9ec-c540-7532-8ef9-925d1983656e Worklog: 2407.1 --- api/main.go | 2 +- api/routes/routes_test.go | 2 +- cli/data_source.go | 5 ++--- service/number_test.go | 9 ++++----- service/service.go | 21 ++++----------------- service/service_test.go | 4 ++-- store/postgres/store.go | 9 +++------ store/postgres/store_test.go | 2 +- tui/app_test.go | 3 +-- 9 files changed, 19 insertions(+), 38 deletions(-) diff --git a/api/main.go b/api/main.go index b0e95d4..c399051 100644 --- a/api/main.go +++ b/api/main.go @@ -103,7 +103,7 @@ func serve(ctx context.Context, log *slog.Logger) error { log.WarnContext(ctx, "WEB_CLIENT_SECRET is unset, so the API logs no browser in and only a bearer token reaches it") } - svc := service.New(postgresStore, service.AssigningItemNumbers()) + svc := service.New(postgresStore) mux := routes.New(svc, log, checks...) if loaded.PublicURL != "" { authentication.Metadata(mux, loaded.PublicURL, loaded.OIDCIssuer) diff --git a/api/routes/routes_test.go b/api/routes/routes_test.go index 9b4a06b..a9a79ee 100644 --- a/api/routes/routes_test.go +++ b/api/routes/routes_test.go @@ -45,7 +45,7 @@ func newAPI(t *testing.T) api { provider := authtest.NewProvider(t) config := authentication.Config(provider.URL) verifier := provider.Verifier(t, config.PersonPrefix, config.ServicePrefix) - mux := routes.New(service.New(sqlite.New(database), service.AssigningItemNumbers()), log) + mux := routes.New(service.New(sqlite.New(database)), log) authentication.Metadata(mux, "https://projects.example.com", provider.URL) protected := authentication.Protect(mux, routes.ServiceScopes, verifier, nil, "", log, routes.PublicPaths...) diff --git a/cli/data_source.go b/cli/data_source.go index 38abcae..3509b57 100644 --- a/cli/data_source.go +++ b/cli/data_source.go @@ -57,10 +57,9 @@ type dataSource struct { local *service.Service } -// LocalService is the service over a database this machine owns outright, which -// numbers items itself because nothing upstream will. +// LocalService is the service over a database this machine owns outright. func LocalService(database *sql.DB, reposRegistryPath string) *service.Service { - return service.New(sqlite.New(database), service.AssigningItemNumbers(), service.RefusingRepoNames(reposRegistryPath)) + return service.New(sqlite.New(database), service.RefusingRepoNames(reposRegistryPath)) } // runMode is where a run's data lives. diff --git a/service/number_test.go b/service/number_test.go index f4a852b..68ff988 100644 --- a/service/number_test.go +++ b/service/number_test.go @@ -19,10 +19,9 @@ func newNumberTestDB(t *testing.T) *sql.DB { return database } -// The work machine is never allowed to reach the API, so nothing upstream will -// ever number its items. Its database is the authority instead. -func TestCreateItem_AssignsNumbersWhenThisDatabaseIsTheAuthority(t *testing.T) { - b := service.New(sqlite.New(newNumberTestDB(t)), service.AssigningItemNumbers()) +// An item's number is the handle a person types, so a fresh database starts it at 1. +func TestCreateItem_NumbersAFreshDatabaseFromOne(t *testing.T) { + b := service.New(sqlite.New(newNumberTestDB(t))) project, err := b.CreateProject(model.CreateProject{Name: "Work"}) if err != nil { t.Fatalf("CreateProject: %v", err) @@ -35,7 +34,7 @@ func TestCreateItem_AssignsNumbersWhenThisDatabaseIsTheAuthority(t *testing.T) { t.Fatalf("CreateItem(%s): %v", title, err) } if item.Number == nil { - t.Fatalf("CreateItem(%s) left the number unset — nothing else will assign it here", title) + t.Fatalf("CreateItem(%s) left the number unset", title) } numbers = append(numbers, *item.Number) } diff --git a/service/service.go b/service/service.go index 7252d21..a9822ac 100644 --- a/service/service.go +++ b/service/service.go @@ -23,8 +23,6 @@ const itemListSegment = "/items" // Service holds every rule about projects, items, memberships, dependencies, tasks and undo. type Service struct { store store.Store - // assignsItemNumbers is set by AssigningItemNumbers. - assignsItemNumbers bool // repoRegistryPath enables the repo-named-project ban. Empty means no // registry and no ban. See RefusingRepoNames. repoRegistryPath string @@ -35,13 +33,6 @@ type Service struct { // Option configures a Service at construction. type Option func(*Service) -// AssigningItemNumbers numbers each item as it is created, from the store's -// NextItemNumber. Both entry points pass it: database mode in cli.LocalService, -// and projects-api over Postgres. -func AssigningItemNumbers() Option { - return func(s *Service) { s.assignsItemNumbers = true } -} - // RefusingRepoNames bans naming a project after a repo, reading the registry at // path on the create and rename paths only. // @@ -531,19 +522,15 @@ func (s *Service) CreateItem(input model.CreateProjectItem) (*model.ProjectItemD } } - var number *int - if s.assignsItemNumbers { - next, err := tx.NextItemNumber(ctx) - if err != nil { - return fmt.Errorf("allocating item number: %w", err) - } - number = &next + number, err := tx.NextItemNumber(ctx) + if err != nil { + return fmt.Errorf("allocating item number: %w", err) } id := idOrNew(input.ID) item, err := tx.CreateItem(ctx, model.ProjectItem{ ID: id, - Number: number, + Number: &number, Title: input.Title, Notes: input.Notes, Repo: emptyRepoUnlinks(input.Repo), diff --git a/service/service_test.go b/service/service_test.go index 022f52e..0dffcac 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -30,7 +30,7 @@ func runsOnPostgres() bool { func newTestBackend(t *testing.T) *Service { t.Helper() if runsOnPostgres() { - return New(postgres.New(postgrestest.Pool(t)), AssigningItemNumbers()) + return New(postgres.New(postgrestest.Pool(t))) } database, err := sqlite.Open(":memory:") if err != nil { @@ -888,7 +888,7 @@ func TestAnEditSurvivesASecondWriter(t *testing.T) { t.Fatalf("connecting: %v", err) } t.Cleanup(pool.Close) - return New(postgres.New(pool), AssigningItemNumbers()) + return New(postgres.New(pool)) } } else { path := filepath.Join(t.TempDir(), "projects.db") diff --git a/store/postgres/store.go b/store/postgres/store.go index afd58dd..7dedfe9 100644 --- a/store/postgres/store.go +++ b/store/postgres/store.go @@ -266,14 +266,11 @@ func (o operations) NextItemNumber(ctx context.Context) (int, error) { return int(next), nil } -// errItemNeedsNumber is a service built without AssigningItemNumbers, because this -// schema is its own authority for numbers and the column holds no null. -var errItemNeedsNumber = errors.New("an item on Postgres needs a number from NextItemNumber") +// errItemNeedsNumber is an undo record restoring an item with no number, which +// the column cannot hold. +var errItemNeedsNumber = errors.New("an item on Postgres needs a number") func (o operations) CreateItem(ctx context.Context, item model.ProjectItem) (model.ProjectItem, error) { - if item.Number == nil { - return model.ProjectItem{}, errItemNeedsNumber - } row, err := o.q.CreateItem(ctx, generated.CreateItemParams{ ID: item.ID, ItemNumber: int64(*item.Number), diff --git a/store/postgres/store_test.go b/store/postgres/store_test.go index 57fd63c..a184be4 100644 --- a/store/postgres/store_test.go +++ b/store/postgres/store_test.go @@ -20,7 +20,7 @@ func TestEveryWriteIsAuditedWithItsWriter(t *testing.T) { ctx := store.WithWriter(context.Background(), store.Writer{ Actor: "a-person", RequestID: "a-request", JobRunID: "a-run", Traceparent: "a-trace", Baggage: "a-baggage", }) - svc := service.New(postgres.New(pool), service.AssigningItemNumbers()).WithContext(ctx) + svc := service.New(postgres.New(pool)).WithContext(ctx) project, err := svc.CreateProject(model.CreateProject{Name: "audited"}) if err != nil { diff --git a/tui/app_test.go b/tui/app_test.go index 6b84ebc..c8cdbf6 100644 --- a/tui/app_test.go +++ b/tui/app_test.go @@ -38,8 +38,7 @@ func newTestAppWithItems(t *testing.T, width, height int, inputs []model.CreateP } t.Cleanup(func() { _ = database.Close() }) - // AssigningItemNumbers, as database mode builds it. - b := service.New(sqlite.New(database), service.AssigningItemNumbers()) + b := service.New(sqlite.New(database)) project, err := b.CreateProject(model.CreateProject{Name: "work"}) if err != nil { t.Fatalf("creating project: %v", err) From 873bc7976d65c0f6ddbbde5a69c6a7474cbf12c8 Mon Sep 17 00:00:00 2001 From: Chris Birch Date: Sun, 8 Nov 2026 02:07:15 -0500 Subject: [PATCH 4/6] refactor(config): refuse every retired variable from one table The todoui rename and the synced copy each kept a list of retired variables, a loop over it, and a call in main.go. Both answer one question, so retiredVariables is now the one table, and main.go asks RetiredVariableRefusal alone. Each retirement adds its rows from an init in its own file, so deleting that file takes its rows with it and leaves main.go alone. The synced copy's MIGRATION marker now lists dropSyncTables, so the whole retirement has one owner. Agent-Name: claude-code-unattended Agent-Session: 43e2375e-0270-4942-8076-19e8ab6a2337 Agent-Run: 01a1b9ec-c540-7532-8ef9-925d1983656e Worklog: 2407.1 --- config/former_name.go | 28 ++++++++++------------- config/retired_sync.go | 45 +++++++++++++++++-------------------- config/retired_variables.go | 29 ++++++++++++++++++++++++ main.go | 8 +++---- 4 files changed, 64 insertions(+), 46 deletions(-) create mode 100644 config/retired_variables.go diff --git a/config/former_name.go b/config/former_name.go index 0a686de..0becc35 100644 --- a/config/former_name.go +++ b/config/former_name.go @@ -14,28 +14,24 @@ import ( // MIGRATION: the tool was called todoui. A machine that upgrades still holds data, a config // and variables under that name, and to this tool they read as if never written. Delete this -// file, its calls in main.go, former_name_test.go, and the passages on it in README and -// CLAUDE.md once no machine holds a todoui data or config directory and no checkout's .envrc +// file, its calls in cli/data_source.go, former_name_test.go, and the passages on it in README +// and CLAUDE.md once no machine holds a todoui data or config directory and no checkout's .envrc // exports a TODOUI_* variable. const formerName = "todoui" -var retiredVariables = []struct{ retired, replacement string }{ - {"TODOUI_DB", "PROJECTS_DB"}, - {"TODOUI_REPOS_REGISTRY", "PROJECTS_REPOS_REGISTRY"}, - {"TODOUI_NO_AUTO_UPDATE", "PROJECTS_NO_AUTO_UPDATE"}, -} - -// RetiredVariableRefusal names the first retired variable set to a value, or "" where none is. // Ignoring TODOUI_DB would open the default database where it chose another, and the others // are refused so a half-renamed environment never runs. -func RetiredVariableRefusal() string { - for _, variable := range retiredVariables { - if os.Getenv(variable.retired) != "" { - return fmt.Sprintf("$%s is set, and projects reads $%s. Move the value there, or unset it.", - variable.retired, variable.replacement) - } +func init() { + for _, renamed := range []struct{ retired, replacement string }{ + {"TODOUI_DB", "PROJECTS_DB"}, + {"TODOUI_REPOS_REGISTRY", "PROJECTS_REPOS_REGISTRY"}, + {"TODOUI_NO_AUTO_UPDATE", "PROJECTS_NO_AUTO_UPDATE"}, + } { + retiredVariables = append(retiredVariables, retiredVariable{ + name: renamed.retired, + remedy: fmt.Sprintf("and projects reads $%s. Move the value there, or unset it.", renamed.replacement), + }) } - return "" } // RefuseConfigUnderFormerName stops a run while the config sits only at the former path. diff --git a/config/retired_sync.go b/config/retired_sync.go index 80ec8bc..5f2a612 100644 --- a/config/retired_sync.go +++ b/config/retired_sync.go @@ -1,33 +1,28 @@ package config -import ( - "fmt" - "os" -) - // MIGRATION: projects once kept a database as a synced copy of an API, set under [sync] and by -// the variables below. Delete this file, Config.RetiredSync, its call in main.go and the warning -// in cli/data_source.go once no machine's config holds [sync] and no .envrc exports one of these. -var retiredSyncVariables = []string{ - "PROJECTS_SYNC", - "PROJECTS_SYNC_URL", - "PROJECTS_SYNC_TOKEN_COMMAND", - "PROJECTS_SYNC_INTERVAL", - "TODOUI_SYNC", - "TODOUI_SYNC_URL", - "TODOUI_SYNC_TOKEN_COMMAND", - "TODOUI_SYNC_INTERVAL", -} +// the variables below. Delete this file, Config.RetiredSync, warnOfRetiredSyncSection in +// cli/data_source.go, dropSyncTables in store/sqlite/open.go, and the passages on them in README +// and CLAUDE.md once no machine's config holds [sync], no .envrc exports one of these, and no +// machine's database still holds pending_sync. -// RetiredSyncVariableRefusal names the first sync variable set to a value, or "" where none is. // PROJECTS_SYNC_URL chose an API, and ignoring it would send the run wherever the config file // points, which on a machine whose config names the production API is production. -func RetiredSyncVariableRefusal() string { - for _, name := range retiredSyncVariables { - if os.Getenv(name) != "" { - return fmt.Sprintf("$%s is set, and projects keeps no synced copy of an API. Unset it. "+ - "api in the config file, written by `projects auth login `, sends every command to an API.", name) - } +func init() { + for _, name := range []string{ + "PROJECTS_SYNC", + "PROJECTS_SYNC_URL", + "PROJECTS_SYNC_TOKEN_COMMAND", + "PROJECTS_SYNC_INTERVAL", + "TODOUI_SYNC", + "TODOUI_SYNC_URL", + "TODOUI_SYNC_TOKEN_COMMAND", + "TODOUI_SYNC_INTERVAL", + } { + retiredVariables = append(retiredVariables, retiredVariable{ + name: name, + remedy: "and projects keeps no synced copy of an API. Unset it. api in the config file, " + + "written by `projects auth login `, sends every command to an API.", + }) } - return "" } diff --git a/config/retired_variables.go b/config/retired_variables.go new file mode 100644 index 0000000..022bcd4 --- /dev/null +++ b/config/retired_variables.go @@ -0,0 +1,29 @@ +package config + +import ( + "fmt" + "os" +) + +// retiredVariable is an environment variable projects refuses rather than reads. +// Each chose where a run read or wrote, so ignoring it would send the run +// somewhere else. +type retiredVariable struct { + name string + // remedy follows "$ is set, " in the refusal. + remedy string +} + +// retiredVariables holds every retirement's rows. Each retirement adds its own +// from the file that holds it, so deleting that file takes its rows with it. +var retiredVariables []retiredVariable + +// RetiredVariableRefusal names the first retired variable set to a value, or "" where none is. +func RetiredVariableRefusal() string { + for _, variable := range retiredVariables { + if os.Getenv(variable.name) != "" { + return fmt.Sprintf("$%s is set, %s", variable.name, variable.remedy) + } + } + return "" +} diff --git a/main.go b/main.go index 605cf01..22c8851 100644 --- a/main.go +++ b/main.go @@ -9,11 +9,9 @@ import ( ) func main() { - for _, refusal := range []string{config.RetiredVariableRefusal(), config.RetiredSyncVariableRefusal()} { - if refusal != "" { - fmt.Fprintf(os.Stderr, "error: %s\n", refusal) - os.Exit(2) - } + if refusal := config.RetiredVariableRefusal(); refusal != "" { + fmt.Fprintf(os.Stderr, "error: %s\n", refusal) + os.Exit(2) } os.Exit(cli.Execute()) } From 96a9b54527427a125077c055dd2733414e143b1b Mon Sep 17 00:00:00 2001 From: Chris Birch Date: Sun, 8 Nov 2026 02:07:38 -0500 Subject: [PATCH 5/6] test(tui): pin that an API reread waits out a move and lands on the next tick The gate test called safeToReread directly, so dropping the gate from the tick's handler failed nothing. One test now drives the tick through Update in API mode: a tick mid-move re-arms and reads nothing, and the next tick in normal mode shows an item another client wrote. It replaces both the predicate test and the re-arm test. Agent-Name: claude-code-unattended Agent-Session: 43e2375e-0270-4942-8076-19e8ab6a2337 Agent-Run: 01a1b9ec-c540-7532-8ef9-925d1983656e Worklog: 2407.1 --- tui/app_test.go | 53 +++++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/tui/app_test.go b/tui/app_test.go index c8cdbf6..a525b26 100644 --- a/tui/app_test.go +++ b/tui/app_test.go @@ -281,36 +281,37 @@ func TestCreatedTaskAppearsAsRow(t *testing.T) { } } -// The reread timer re-arms itself from its own message. A tick that returns no -// command ends the rereads for the rest of the session, so the skipped ticks -// have to re-arm too — otherwise opening one modal at the wrong moment leaves the -// board showing what the API held when it opened. -func TestRereadTickRearmsEvenWhenItSkipsTheRead(t *testing.T) { - app := newTestApp(t, 80, 24, "Alpha") - - for _, mode := range []appMode{modeNormal, modeAddItem, modeMove, modeHelp} { - app.appMode = mode - _, cmd := app.Update(rereadTickMsg{}) - if cmd == nil { - t.Errorf("tick in mode %v returned no command: the reread timer is now dead", mode) - } +// In API mode a tick reads the API again. One arriving mid-move waits for the +// next tick, because a read replaces the rows being moved. It still re-arms, or +// the board stops reading the API for the rest of the session. +func TestAnAPIRereadWaitsOutAMoveAndLandsOnTheNextTick(t *testing.T) { + app := newTestApp(t, 80, 24, "here at launch") + app.where = Where{APIHost: "projects.example.com"} + projects, err := app.backend.ListProjectsByStatus(model.StatusActive) + if err != nil || len(projects) == 0 { + t.Fatalf("listing projects: %v", err) + } + if _, err := app.backend.CreateItem(model.CreateProjectItem{Title: "written elsewhere", ProjectIDs: []string{projects[0].ID}}); err != nil { + t.Fatal(err) + } + shown := func() bool { + return slices.ContainsFunc(app.items, func(item model.ProjectItemInProject) bool { return item.Title == "written elsewhere" }) } -} -// safeToReread gates the read, not the timer. A read replaces the lists and -// their order, which would move the ground under a grab or a text entry. -func TestRereadOnlyRunsInNormalMode(t *testing.T) { - app := newTestApp(t, 80, 24, "Alpha") + app.appMode = modeMove + _, cmd := app.Update(rereadTickMsg{}) + if cmd == nil { + t.Fatal("a tick during a move returned no command, so the rereads stopped") + } + drain(t, app, cmd, 16) + if shown() { + t.Fatal("a tick during a move read the API under the rows being moved") + } app.appMode = modeNormal - if !app.safeToReread() { - t.Error("normal mode must allow the background read") - } - for _, mode := range []appMode{modeAddItem, modeExternalEdit, modeMove, modeMoveProject, modeReader} { - app.appMode = mode - if app.safeToReread() { - t.Errorf("mode %v must defer the read to the next tick", mode) - } + send(t, app, rereadTickMsg{}) + if !shown() { + t.Error("the tick after the move did not read the API") } } From 14b6ccf543fd4e846a6cae673c316a71422397bb Mon Sep 17 00:00:00 2001 From: Chris Birch Date: Sun, 8 Nov 2026 02:07:47 -0500 Subject: [PATCH 6/6] docs: say what the synced copy's leftovers do, and stop restating what code holds - README's section on a synced copy's config and database states how each behaves, including the refusal of an unpushed queue, rather than narrating what the tool was. - The board's re-read interval is named once, as apiRereadInterval. The help says the board reads the API again on a timer. - CLAUDE.md describes the one retired-variable table, ties every sync leftover to the MIGRATION marker that retires it, and lists no table names or callers the code already holds. Its .envrc note says what the test reads and that a PROJECTS_SYNC* line stops every run. Agent-Name: claude-code-unattended Agent-Session: 43e2375e-0270-4942-8076-19e8ab6a2337 Agent-Run: 01a1b9ec-c540-7532-8ef9-925d1983656e Worklog: 2407.1 --- CLAUDE.md | 38 +++++++++++++++++++++----------------- README.md | 15 +++++++++------ cli/tui.go | 3 +-- config/config_test.go | 4 ++-- 4 files changed, 33 insertions(+), 27 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2debbac..be6d24d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,13 +21,17 @@ command in API mode against the production API and ignores `PROJECTS_DB`. So `PROJECTS_DB` alone writes to production. An empty `XDG_CONFIG_HOME` leaves no `api`, and the run opens the database `PROJECTS_DB` names. -`.envrc` is machine-local and is not in this repo. `task setup`, which `worktree -new` runs, links the primary checkout's copy into a worktree. It points -`PROJECTS_DB` at `dev.db` for `TestRealDatabaseSurvivesEverySizeAndProject`, and -keeps no run off the API the config file names. +`.envrc` is machine-local. `task setup`, which `worktree new` runs, links the +primary checkout's copy into a worktree. `TestRealDatabaseSurvivesEverySizeAndProject` +reads `PROJECTS_DB`, and skips without it. A `PROJECTS_SYNC*` line left in an +`.envrc` stops every run with exit 2. -**The former name is handled in one file.** `config/former_name.go` refuses a retired variable that is -still set, and a config still at the former path. It moves the old data directory on the first run +**A retired variable refuses from one table.** `config/retired_variables.go` holds it, `main.go` +refuses any variable in it that is set before anything runs, and each retirement adds its rows from +its own file. + +**The former name is handled in one file.** `config/former_name.go` adds the `TODOUI_*` variables to +that table, and refuses a config still at the former path. It moves the old data directory on the first run that opens the default database. A run naming its database moves nothing, so a run from `.envrc` leaves the real data where it is. An `.envrc` still exporting the old names is refused rather than ignored, which keeps it from falling through to the production database. On a machine upgrading, run `projects` @@ -36,12 +40,12 @@ marker there says when the file retires. Anything that opens the database stays `MoveDataUnderFormerName` in `openDatabase` in `cli/data_source.go`, because opening first creates an empty database, and the move refuses to run over one. -**The synced copy's leftovers are handled in one file too.** `config/retired_sync.go` refuses a set -`PROJECTS_SYNC*` or `TODOUI_SYNC*` variable with exit 2, because `PROJECTS_SYNC_URL` chose an API. -`Config.RetiredSync` keeps a `[sync]` section loading, and `warnOfRetiredSyncSection` in -`cli/data_source.go` warns of it rather than refusing, because a machine's config changes only at its -next apply. `dropSyncTables` in `store/sqlite/open.go` drops `pending_sync`, `sync_state` and -`sync_origin` on the first open. The file's `MIGRATION` marker says when it retires. +**The synced copy's leftovers retire together, under the `MIGRATION` marker in +`config/retired_sync.go`.** Its rows refuse a set `PROJECTS_SYNC*` or `TODOUI_SYNC*` variable with +exit 2, because `PROJECTS_SYNC_URL` chose an API. `Config.RetiredSync` keeps a `[sync]` section +loading, and `warnOfRetiredSyncSection` in `cli/data_source.go` warns of it rather than refusing, +because a machine's config changes only at its next apply. `dropSyncTables` in `store/sqlite/open.go` +drops the synced copy's own tables on the first open, and refuses while its push queue holds a row. Build, vet and unit tests check code correctness, not TUI correctness. A TUI change is verified by driving the built binary, which the flows below do with nobody at the keyboard. @@ -183,10 +187,10 @@ builds the tree. Help renders through `gotermstyle/cobrahelp`. An item shows its **number**, the handle the API assigns and answers with. The UUID is the key and stops there. -`AssigningItemNumbers` numbers each item as it is created. Through an API, the API numbers it, from -the `item_numbers` sequence. In database mode `projects` allocates `max+1` itself. An item with no -number, which only a database from before numbers or from the synced copy holds, shows its UUID tail -(`itemHandle` in `tui/app.go`). +`service.CreateItem` numbers each item as it is created, from the store's `NextItemNumber`: the +`item_numbers` sequence on Postgres, and `max+1` on SQLite. An item with no number, which only a +database from before numbers or from the synced copy holds, shows its UUID tail (`itemHandle` in +`tui/app.go`). ## A project name is bounded work, never a repo @@ -300,7 +304,7 @@ per test. Set `TEST_DATABASE_URL` to use a running server. `-short` skips them, `lookup_rows.sql`. `postgres.Migrate` upserts Postgres's from `model`, and `TestEachStoreHoldsTheKnownKindsAndStatuses` holds both stores to it. The project schemas' `enum` and the web app's `types.ts` list them too, and a test holds each to `model`. -- **Item numbers come from the `item_numbers` sequence**, through `AssigningItemNumbers`. A database +- **Item numbers come from the `item_numbers` sequence**, through `NextItemNumber`. A database loaded from elsewhere needs the sequence set from the old sequence's own counter, because numbers it already handed out can outrun the rows left. - **`.sqlfluffignore` keeps the SQLite hook off `store/postgres`**, and a second hook in diff --git a/README.md b/README.md index 5402c04..a611086 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,7 @@ items it waits on. The reader also names the repo the item is for. The board has that tag. `projects items edit --repo ` sets it. Through an API, the status bar reads `API` and the API's host. The board reads the API again every -two minutes, and `R` reads it at once. +`apiRereadInterval` in `tui/app.go`, and `R` reads it at once. `Enter` opens the reader, a full screen holding one item or one project. An item shows its fields, what blocks it, what it blocks, its tasks, and its notes rendered as markdown. A project shows its @@ -253,15 +253,18 @@ Through an API, the API numbers the item as it creates it. In database mode, `pr next number itself. An item with no number shows the last 8 characters of its id. Only a database from before numbers existed, or one kept as a synced copy of an API, can hold such an item. -## A config or database from the synced copy still works, and its sync variables refuse +## A synced copy's config and database still load, while a sync variable or an unpushed edit refuses -`projects` once kept a database as a copy of an API, set under `[sync]`. It keeps no copy now, and -`api` is how a machine works through an API. +A config holding `[sync]`, a `PROJECTS_SYNC*` variable, or a database that was a synced copy of an +API behaves as follows. `api` is how a machine works through an API. - A config file still holding `[sync]` loads. Every command that opens data warns on stderr, naming the file, and ignores the section. -- A database from then opens as it is. The first open drops its queue of pushes, its pull times and - the API it was bound to. Every project, item, task, dependency and membership stays. +- A database that was a synced copy opens as it is. The first open drops its queue of pushes, its pull + times and the API it was bound to. Every project, item, task, dependency and membership stays. +- A queue still holding an edit stops that open with exit 1, naming how many and the API they were + for, because those edits never reached it. `projects sync` from v1.24.2 pushes them, and + `DELETE FROM pending_sync` discards them. - `PROJECTS_SYNC`, `PROJECTS_SYNC_URL`, `PROJECTS_SYNC_TOKEN_COMMAND` or `PROJECTS_SYNC_INTERVAL`, or its `TODOUI_` twin, set to a value stops every command with exit 2. `PROJECTS_SYNC_URL` chose an API, so ignoring it would send the run wherever the config file points. diff --git a/cli/tui.go b/cli/tui.go index cbe8485..8339c78 100644 --- a/cli/tui.go +++ b/cli/tui.go @@ -47,8 +47,7 @@ func newTUICommand() *cobra.Command { Short: "Open the board: projects on the left, the selected one's items on the right", Long: "Everything the commands do can be done here by key; ? lists the keys. The status\n" + "line names where the data lives: LOCAL and the database's path, or API and its\n" + - "host. In API mode the board reads the API again every two minutes, and R reads\n" + - "it now.\n" + + "host. In API mode the board reads the API again on a timer, and R reads it now.\n" + "\n" + "--once prints one frame at COLUMNS by LINES, else at the terminal's size, else at\n" + "120 by 40, and --json what that frame is drawn from, so an agent reads what a\n" + diff --git a/config/config_test.go b/config/config_test.go index 8ee263a..c3f94e8 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -32,8 +32,8 @@ func TestDefaultDBPath_XDGOverride(t *testing.T) { // repos_registry is a bare top-level key, which TOML requires above the first // [section] header — declared under one it would belong to that section and -// unmarshal into nothing. The [sync] section below it is the work box's file, -// which has to load rather than stop the tool. +// unmarshal into nothing. The [sync] section below it has to load, so a machine +// whose config still holds one keeps running. func TestReposRegistryIsReadFromTheConfigFile(t *testing.T) { const declared = "/declared/repos.json" writeConfig(t, "repos_registry = \""+declared+"\"\n\n[sync]\nenabled = false\n")