feat!: remove the sync engine and the database's synced copy by datapointchris · Pull Request #20 · datapointchris/projects

datapointchris · 2026-11-08T06:47:21Z

Every desk now works through the projects API, and the one machine that never syncs keeps its database as its only copy. The sync engine kept a database as a copy of an API, so nothing is left for it to do. It still cost a package of its own, three tables, twenty-four queries, a command, a config section and a write refusal. This removes all of it. API mode and database mode are what remain, and both already ran through the same routes.

What to look at

store/sqlite/open.go: dropSyncTables replaces the two migrations that created the sync tables. It refuses while pending_sync holds a row, and otherwise drops all three. Check that it touches nothing a person wrote, and that it runs on a database of any age.
config/retired_variables.go, config/retired_sync.go and main.go: every retired variable refuses from one table, and the sync variables join it. Check that the refusal runs before anything opens.
config/config.go and cli/data_source.go: [sync] loads into Config.RetiredSync, and warnOfRetiredSyncSection warns on stderr. Check that ErrorUnused still refuses every other unknown key.
service/service.go: CreateItem numbers every item, with no option. Check that nothing still builds a service expecting unnumbered items.
tui/app.go: the engine's status bar, pull messages and R path go. The API-mode re-read keeps its tick at apiRereadInterval.
model/undo.go and service/undo.go: the undo result keeps the four fields the API sends. The rest existed for pushing an undo.

How it was verified

go test -short ./... passes. That includes TestRealBinaryFlows, whose two help frames changed only by the R line, which no longer wraps.
A database built by the v1.24.0 binary, under a config holding [sync] enabled = false, held 2 projects, 3 items, 4 memberships, 1 dependency, 2 tasks and 11 undo entries, plus one row in sync_state and sync_origin. This build opened it with a [sync] warning. Every row in the five data tables matched the copy taken before, the undo log still held 11 entries, and the three sync tables were gone. items create then numbered a new item 4.
A v1.24.0 database with two rows in pending_sync refused to open with exit 1, naming 2 edits and the bound API, and every table stayed. After DELETE FROM pending_sync it opened, and the sync tables went.
Every database this was checked against holds an empty pending_sync, so no real database reaches the refusal today.
TestAnAPIRereadWaitsOutAMoveAndLandsOnTheNextTick fails when the gate is dropped from the tick's handler, and passes with it.
PROJECTS_SYNC_URL=… projects version, TODOUI_SYNC=1 projects version and TODOUI_DB=… projects version each exit 2 with their refusal. projects sync is an unknown command.

What changes

projects sync is gone, and so are --adopt and --force.
A config holding [sync] loads. Every command that opens data prints a warning: line naming the file, until the section is removed.
PROJECTS_SYNC, PROJECTS_SYNC_URL, PROJECTS_SYNC_TOKEN_COMMAND and PROJECTS_SYNC_INTERVAL, and their TODOUI_ twins, stop every command with exit 2 while set.
A database that was a synced copy opens as a standalone database. Its writes stay on that machine.
A database whose push queue still holds edits refuses to open, with exit 1. The message names how many, the API they were queued for, and the two ways out: projects sync from v1.24.2, or DELETE FROM pending_sync.
A failed open names the database path rather than "database".
Through an API, the board re-reads every apiRereadInterval, and nothing configures that. R re-reads the data in database mode too, where it used to flash a sync-disabled message.
The status bar shows LOCAL or API. The sync states are gone.
store/sqlite/generated moves to store/sqlite/internal/generated.

Decisions, and what they rejected

The sync variables refuse, while [sync] only warns. On a desk the config file holds api. An ignored PROJECTS_SYNC_URL would leave that api in charge, so a dev run meant for the dev stack would reach the production API. A warning was rejected for the variables for that reason. The config section warns because a refusal would stop the tool on a machine until its next config change.
The drop ships in this release, guarded, rather than in a later one. A table's drop usually ships a release after the code stops reading it, so a rollback still finds it. Here a reverted binary recreates all three tables empty and opens. The one table whose rows matter is the push queue, and the open refuses while it holds any. A later release for the drop was rejected as a release carrying nothing.
The re-read interval is a constant, not a new key. No known config set interval. A new key for one knob was rejected as surface nobody asked for.
Every service numbers its items. An option existed so a synced copy could leave numbering to its API. That case is gone, so the option, its branch and the Postgres store's guard for it go too. RestoreItem keeps its nil check, because its number comes from an undo record on disk.
Unnumbered items keep no number. No database checked held one, and leaving them is what the earlier migration already did.
Smallest change that would work — deleting sync/ and the projects sync command alone. The schema, the queries, the config section and the write refusal would then describe an engine that no longer exists.

Risk and rollback

A release ships the binary, the API image and the web image under one version. The API's numbering is unchanged, since it always numbered. The web app does not change. The first open of a database by this binary drops the three sync tables, unless the push queue holds a row. A revert brings back code that recreates them empty, so a reverted binary opens fine. Its database is then bound to no API until a projects sync --adopt.

The review

Round 1: #20 (comment)

G1.1 fixed — dropSyncTables refuses while pending_sync holds a row, naming the count and the bound API. sync_state and sync_origin drop unconditionally. Run by hand rather than pinned by a test, because no database checked holds a queued row.
system-design.1 fixed — same as G1.1.
failure-paths.1 fixed — same as G1.1.
contracts.1 fixed — same as G1.1.
G1.2 rejected — a test of a migration pins a transition, which this suite leaves out and deletes where a change touches one. The drop, the refusal and the null number were each proved on a database built by v1.24.0.
G1.3 fixed — the sentence says what the test reads, and that a PROJECTS_SYNC* line left in .envrc stops every run.
code-architecture.1 fixed — the option, its field, its branch and the CreateItem guard are gone. RestoreItem keeps its guard, because its number comes from an undo record.
code-architecture.2 fixed — the help says the board reads the API again on a timer, and the README names apiRereadInterval.
code-architecture.3 fixed — config/retired_variables.go holds the one table. Each retirement adds its rows from an init in its own file, and main.go calls RetiredVariableRefusal alone.
useless-test.1 fixed — one test drives the tick through Update in API mode, replacing the predicate test and the re-arm test. It fails with the gate dropped.
contracts.2 fixed — the one config that held [sync] drops it, in the repo that installs it, and its header says what the file is now. v1.24.x reads the missing section as sync off, so it landed ahead of this release.
stale-prose.1 fixed — the MIGRATION marker lists dropSyncTables, which carries a line naming the marker, and CLAUDE.md says the leftovers retire together under it.
stale-prose.2 fixed — CLAUDE.md names the tables by role. The caller list in service/service.go went with the option.
stale-prose.3 fixed — both sentences rewritten as the finding proposed.
stale-prose.4 fixed — the section opens on what each leftover does, and its heading says the same.
synthesis.1 fixed — stale-prose.2, stale-prose.3 and code-architecture.2 are each fixed.
synthesis.2 fixed — stale-prose.1, contracts.2 and code-architecture.3 are each fixed.
synthesis.3 fixed — useless-test.1 is fixed, and the migration half is rejected as G1.2 says.

Session 43e2375e-0270-4942-8076-19e8ab6a2337 wrote this change.

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

datapointchris

PR review — system-design

Used: Read the diff of open.go, retired_sync.go, main.go and the deploy files. Did not build or run the migration.
Standards: No standards searched. The one finding is a data-loss path, and no rule was needed to state it.
Findings: 1 correctness · 0 design · 0 prose

Correctness

system-design.1 store/sqlite/open.go:80 · store/sqlite/open.go:137

Reach: A desk runs the old binary in sync mode while offline or the API is failing. Its writes land in the local database and wait in pending_sync, unpushed. The desk upgrades to this build and opens the database. migrate() runs dropSyncTables and drops pending_sync before anything else looks at it. The desk is then set to API mode. The queued writes never reach the API, and nothing says they were dropped. If the desk stays in database mode, the rows stay local but the API never gets them.

dropSyncTables drops pending_sync without reading it. The PR says no person-written row is touched. That holds for the five data tables. The queue is the only record of which local writes the API has not seen. Dropping it loses those writes for the API, and the PR does not mention this.

The verification used a database with one row in pending_sync. It checked only that the table was gone. It did not check that the row had been pushed.

Fix: before the drop, count the rows in pending_sync. If the count is above zero, refuse to open and name the count. Tell the user to run the previous release's projects sync first. Alternatively, print the pending operations to stderr or save them to a file before dropping. Drop the table only when it is empty.

Findings as JSON

{
  "used": "Read the diff of open.go, retired_sync.go, main.go and the deploy files. Did not build or run the migration.",
  "standards": "No standards searched. The one finding is a data-loss path, and no rule was needed to state it.",
  "findings": [
    {
      "number": "system-design.1",
      "section": "correctness",
      "sites": [
        "store/sqlite/open.go:80",
        "store/sqlite/open.go:137"
      ],
      "reach": "A desk runs the old binary in sync mode while offline or the API is failing. Its writes land in the local database and wait in pending_sync, unpushed. The desk upgrades to this build and opens the database. migrate() runs dropSyncTables and drops pending_sync before anything else looks at it. The desk is then set to API mode. The queued writes never reach the API, and nothing says they were dropped. If the desk stays in database mode, the rows stay local but the API never gets them.",
      "body": "`dropSyncTables` drops `pending_sync` without reading it. The PR says no person-written row is touched. That holds for the five data tables. The queue is the only record of which local writes the API has not seen. Dropping it loses those writes for the API, and the PR does not mention this.\n\nThe verification used a database with one row in `pending_sync`. It checked only that the table was gone. It did not check that the row had been pushed.\n\nFix: before the drop, count the rows in `pending_sync`. If the count is above zero, refuse to open and name the count. Tell the user to run the previous release's `projects sync` first. Alternatively, print the pending operations to stderr or save them to a file before dropping. Drop the table only when it is empty."
    }
  ]
}

This review does not gate merge.

datapointchris

PR review — code-architecture

Used: Cloned the repo and checked out PR 20 head 67e8fa1. Read the diff of main...HEAD and the touched modules: config/, main.go, cli/data_source.go, store/sqlite/open.go, service/, tui/app.go. Searched the whole repo with rg for other sites that decide the same facts. Did not build or run anything.
Standards: code-quality.md/A guard behind a filter is answered by the filter, not by the guard (search "dead branch flag every caller passes the same value remove the option"). documentation.md/A doc points at the source instead of restating its content or constants (search "constant restated in help text drifts"). help.md/Help text never carries a count, version, path or list of what is supported (same search).
Findings: 0 correctness · 3 design · 0 prose

Design and architecture

code-architecture.1 service/service.go:38 · service/service.go:535 · cli/data_source.go:63 · api/main.go:106 · store/postgres/store.go:269 · service/service_test.go:40 · tui/realdata_test.go:43

Reach: Every new place that builds a service has to pass the option. This repo already has six that do not: test helpers in service/service_test.go (lines 40, 901 and 941), service/project_status_test.go:300 and tui/realdata_test.go:43. Each of these builds a service with no item numbers, a setup no command runs. The Postgres store carries its own check, errItemNeedsNumber, only to catch a service built without the option.

Each place that builds a service decides whether it numbers items, and only one answer is left

After this change, AssigningItemNumbers has a single valid value. Both entry points pass it: cli.LocalService at cli/data_source.go:63 and api/main.go:106. The PR body and the new comment at service/service.go:38 both say so. The reason the option existed was a synced database, where the API assigned the numbers. This PR removes that case.

The off branch is still there. service/service.go:535 skips numbering when the option is missing. store/postgres/store.go:269 keeps errItemNeedsNumber to refuse a service built without it. That is a guard for a setup nothing in the product builds. The rule code-quality.md § "A guard behind a filter is answered by the filter, not by the guard" calls this a predicate that cannot return both answers.

The PR rejected making numbering unconditional because it would "reach the Postgres store's refusal and the tests built without it". Those two things are the cost described here. They are not a reason to keep it.

Fix: make numbering what service.New always does. Then delete:

the AssigningItemNumbers option and the assignsItemNumbers field;
the if at service/service.go:535;
errItemNeedsNumber in the Postgres store.

The test helpers that build services without numbering would then number their items, as every real command already does.

code-architecture.2 tui/app.go:266 · cli/tui.go:50 · README.md:186

Reach: The next change to apiRereadInterval must also edit the projects tui --help text and the README. This PR is already that change once: it turned [sync] interval into a constant and rewrote both prose copies alongside it.

The re-read interval is written in three places: one constant and two sentences

tui/app.go:266 sets apiRereadInterval = 2 * time.Minute. The projects tui long help at cli/tui.go:50 says "every two minutes". So does README.md:186. Nothing ties the two sentences to the constant.

Two rules cover this:

documentation.md § "A doc points at the source instead of restating its content or constants";
help.md § "Help text never carries a count, version, path or list of what is supported, which the next release makes stale".

Fix: describe the behaviour without the number, for example "In API mode the board reads the API again on a timer, and R reads it now". The README can point at apiRereadInterval. Another option is to build the help string from the constant with fmt.Sprint(apiRereadInterval). That keeps one owner, but it needs the constant exported from tui.

code-architecture.3 config/former_name.go:22 · config/former_name.go:30 · config/retired_sync.go:11 · config/retired_sync.go:25 · main.go:12

Reach: This PR is the second retirement. Like the todoui rename, it adds its own list and function and another entry in main.go's slice of refusals. The next retired variable (a renamed PROJECTS_* key, or a dropped PROJECTS_DB) has two lists to choose from and a third pattern to start. Each choice also means another edit to main.go.

Two lists decide which retired variables stop a run, each with its own loop and its own check in `main.go`

config/former_name.go:22 has retiredVariables and RetiredVariableRefusal, for the variables renamed from todoui. config/retired_sync.go:11 adds retiredSyncVariables and RetiredSyncVariableRefusal. Both answer the same question: "is a variable set that this tool no longer reads?" They use the same loop over os.Getenv and return the same kind of string. main.go:12 now loops over a hand-written slice of both functions to decide exit 2.

The only difference is the wording of the message. One names a replacement variable. The other says to unset it.

Fix: keep one table in config with one entry per variable: the retired name, a replacement (empty for none) and the remedy. Give it one refusal function, and have main.go call that alone. Each retirement can still own its rows in its own MIGRATION file, through a package-level append or a separate slice concatenated in one place. Deleting a retirement then removes its rows and never touches main.go.

Findings as JSON

{
  "used": "Cloned the repo and checked out PR 20 head 67e8fa1. Read the diff of `main...HEAD` and the touched modules: `config/`, `main.go`, `cli/data_source.go`, `store/sqlite/open.go`, `service/`, `tui/app.go`. Searched the whole repo with `rg` for other sites that decide the same facts. Did not build or run anything.",
  "standards": "code-quality.md/A guard behind a filter is answered by the filter, not by the guard (search \"dead branch flag every caller passes the same value remove the option\"). documentation.md/A doc points at the source instead of restating its content or constants (search \"constant restated in help text drifts\"). help.md/Help text never carries a count, version, path or list of what is supported (same search).",
  "findings": [
    {
      "number": "code-architecture.1",
      "section": "design",
      "sites": [
        "service/service.go:38",
        "service/service.go:535",
        "cli/data_source.go:63",
        "api/main.go:106",
        "store/postgres/store.go:269",
        "service/service_test.go:40",
        "tui/realdata_test.go:43"
      ],
      "reach": "Every new place that builds a service has to pass the option. This repo already has six that do not: test helpers in `service/service_test.go` (lines 40, 901 and 941), `service/project_status_test.go:300` and `tui/realdata_test.go:43`. Each of these builds a service with no item numbers, a setup no command runs. The Postgres store carries its own check, `errItemNeedsNumber`, only to catch a service built without the option.",
      "body": "### Each place that builds a service decides whether it numbers items, and only one answer is left\n\nAfter this change, `AssigningItemNumbers` has a single valid value. Both entry points pass it: `cli.LocalService` at `cli/data_source.go:63` and `api/main.go:106`. The PR body and the new comment at `service/service.go:38` both say so. The reason the option existed was a synced database, where the API assigned the numbers. This PR removes that case.\n\nThe off branch is still there. `service/service.go:535` skips numbering when the option is missing. `store/postgres/store.go:269` keeps `errItemNeedsNumber` to refuse a service built without it. That is a guard for a setup nothing in the product builds. The rule `code-quality.md` § \"A guard behind a filter is answered by the filter, not by the guard\" calls this a predicate that cannot return both answers.\n\nThe PR rejected making numbering unconditional because it would \"reach the Postgres store's refusal and the tests built without it\". Those two things are the cost described here. They are not a reason to keep it.\n\n**Fix:** make numbering what `service.New` always does. Then delete:\n- the `AssigningItemNumbers` option and the `assignsItemNumbers` field;\n- the `if` at `service/service.go:535`;\n- `errItemNeedsNumber` in the Postgres store.\n\nThe test helpers that build services without numbering would then number their items, as every real command already does."
    },
    {
      "number": "code-architecture.2",
      "section": "design",
      "sites": [
        "tui/app.go:266",
        "cli/tui.go:50",
        "README.md:186"
      ],
      "reach": "The next change to `apiRereadInterval` must also edit the `projects tui --help` text and the README. This PR is already that change once: it turned `[sync] interval` into a constant and rewrote both prose copies alongside it.",
      "body": "### The re-read interval is written in three places: one constant and two sentences\n\n`tui/app.go:266` sets `apiRereadInterval = 2 * time.Minute`. The `projects tui` long help at `cli/tui.go:50` says \"every two minutes\". So does `README.md:186`. Nothing ties the two sentences to the constant.\n\nTwo rules cover this:\n- `documentation.md` § \"A doc points at the source instead of restating its content or constants\";\n- `help.md` § \"Help text never carries a count, version, path or list of what is supported, which the next release makes stale\".\n\n**Fix:** describe the behaviour without the number, for example \"In API mode the board reads the API again on a timer, and R reads it now\". The README can point at `apiRereadInterval`. Another option is to build the help string from the constant with `fmt.Sprint(apiRereadInterval)`. That keeps one owner, but it needs the constant exported from `tui`."
    },
    {
      "number": "code-architecture.3",
      "section": "design",
      "sites": [
        "config/former_name.go:22",
        "config/former_name.go:30",
        "config/retired_sync.go:11",
        "config/retired_sync.go:25",
        "main.go:12"
      ],
      "reach": "This PR is the second retirement. Like the `todoui` rename, it adds its own list and function and another entry in `main.go`'s slice of refusals. The next retired variable (a renamed `PROJECTS_*` key, or a dropped `PROJECTS_DB`) has two lists to choose from and a third pattern to start. Each choice also means another edit to `main.go`.",
      "body": "### Two lists decide which retired variables stop a run, each with its own loop and its own check in `main.go`\n\n`config/former_name.go:22` has `retiredVariables` and `RetiredVariableRefusal`, for the variables renamed from `todoui`. `config/retired_sync.go:11` adds `retiredSyncVariables` and `RetiredSyncVariableRefusal`. Both answer the same question: \"is a variable set that this tool no longer reads?\" They use the same loop over `os.Getenv` and return the same kind of string. `main.go:12` now loops over a hand-written slice of both functions to decide exit 2.\n\nThe only difference is the wording of the message. One names a replacement variable. The other says to unset it.\n\n**Fix:** keep one table in `config` with one entry per variable: the retired name, a replacement (empty for none) and the remedy. Give it one refusal function, and have `main.go` call that alone. Each retirement can still own its rows in its own MIGRATION file, through a package-level `append` or a separate slice concatenated in one place. Deleting a retirement then removes its rows and never touches `main.go`."
    }
  ]
}

This review does not gate merge.

datapointchris

PR review — G1

Used: I built the PR head and its base (v1.24.0 shape) into my own directory. With the base binary I made a database and added one pending_sync row. I gave it a [sync] enabled = true config and opened it with the new binary under a scratch XDG_CONFIG_HOME/HOME. I also ran the PROJECTS_SYNC, TODOUI_SYNC_URL and TODOUI_DB refusals and go test -short ./..., which passed.
Standards: I cited data.md § "A destructive migration ships as its own release, after the release that stopped reading" and configuration.md § "A retired environment variable that picked a target is refused while it is still set", which the change satisfies. Searches on dropping undelivered queued writes found no rule.
Findings: 1 correctness · 1 design · 1 prose

Correctness

G1.1 store/sqlite/open.go:137 · store/sqlite/open.go:80

Reach: Every existing database opened by this binary in database mode runs dropSyncTables. Before this PR, the dev dev.db ran with PROJECTS_SYNC=true from .envrc, and the PR body's work-box config still holds [sync]. Any TUI edit made on those machines while their API was unreachable sits in pending_sync. Unproven: whether any machine's pending_sync holds rows today. SELECT count(*) FROM pending_sync on each synced database would settle it.

The first open silently drops edits that were queued but never pushed.

I made a database with the base binary and inserted one pending_sync row. I wrote a [sync] enabled = true config and ran this branch's projects list. It printed the [sync] warning and exited 0. After that run .tables showed no pending_sync. Neither the warning nor anything else said that a queued edit had never reached the API.

Before this PR the status bar showed PENDING (n), and the README said such a change "is sent once it comes back". After this PR the edit lives only in a database the PR calls standalone. The desk then moves to api, so nothing reads that database again. The edit is stranded.

The PR's own rollback makes this worse. The rollback recreates the tables empty and leaves the database "bound to no API until a projects sync --adopt". The old README says a pull "deletes every local row the server did not return". So the adopt step deletes exactly the rows whose pushes were dropped.

The drop ships in the same release as the code that stops reading the tables, which data.md § "A destructive migration ships as its own release, after the release that stopped reading" exists to prevent.

Fix: in dropSyncTables, count pending_sync first. When it holds rows, refuse to open. Name the count and tell the user to run projects sync with the previous release before upgrading. The other two tables hold nothing a person wrote and can drop unconditionally. Add a test that seeds a pending row and asserts the refusal.

Design and architecture

G1.2 store/sqlite/open.go:140 · store/sqlite/open.go:299 · store/sqlite/open_test.go

Reach: The MIGRATION marker in config/retired_sync.go plans the next change to migrate(), which is retiring dropSyncTables. That change and any new migration reorder or edit this chain with nothing pinning two of its steps.

The diff deletes the only test of a live migration and adds none for the new one.

The diff deletes TestMigrate_AddsNumberColumnToExistingDatabase. migrateItemNumber still runs on every database older than the number column. The README still says such databases exist ("a database from before numbers existed"). The deleted test seeded a pre-number row and asserted three things: the title survived, number stayed null, and a second migrate was a no-op. Only its comment mentioned sync. No remaining test in open_test.go builds a table without number.

dropSyncTables has no test at all. The PR body asks reviewers to check that it "touches nothing a person wrote, and that it runs on a database of any age". Only a hand run checks that, and a hand run does not repeat on the next change.

Fix: restore TestMigrate_AddsNumberColumnToExistingDatabase with its comment reworded to the null-number rule. Add one test that builds a database with the three sync tables plus a project, an item, a membership and an undo entry. Run migrate twice, then assert that the sync tables are gone and every data row is unchanged.

Prose

G1.3 CLAUDE.md:28

The sentence about .envrc reads as its opposite.

The sentence is "It points PROJECTS_DB at dev.db for TestRealDatabaseSurvivesEverySizeAndProject, and keeps no run off the API the config file names." "Keeps no run off the API" parses as a double negative. A reader can take it to mean that .envrc guards against the API, which is the opposite of the warning two lines above it.

The file also gives no instruction for an .envrc that still exports PROJECTS_SYNC=true, which the old version of this file told every machine to hold. That variable now stops every go run with exit 2.

Rewrite: "It sets PROJECTS_DB to dev.db for TestRealDatabaseSurvivesEverySizeAndProject and sets nothing else, so a run with the machine's config file still goes to the API that file's api names."

Findings as JSON

{
  "used": "I built the PR head and its base (v1.24.0 shape) into my own directory. With the base binary I made a database and added one `pending_sync` row. I gave it a `[sync] enabled = true` config and opened it with the new binary under a scratch `XDG_CONFIG_HOME`/`HOME`. I also ran the `PROJECTS_SYNC`, `TODOUI_SYNC_URL` and `TODOUI_DB` refusals and `go test -short ./...`, which passed.",
  "standards": "I cited `data.md` § \"A destructive migration ships as its own release, after the release that stopped reading\" and `configuration.md` § \"A retired environment variable that picked a target is refused while it is still set\", which the change satisfies. Searches on dropping undelivered queued writes found no rule.",
  "findings": [
    {
      "number": "G1.1",
      "section": "correctness",
      "sites": [
        "store/sqlite/open.go:137",
        "store/sqlite/open.go:80"
      ],
      "reach": "Every existing database opened by this binary in database mode runs `dropSyncTables`. Before this PR, the dev `dev.db` ran with `PROJECTS_SYNC=true` from `.envrc`, and the PR body's work-box config still holds `[sync]`. Any TUI edit made on those machines while their API was unreachable sits in `pending_sync`. Unproven: whether any machine's `pending_sync` holds rows today. `SELECT count(*) FROM pending_sync` on each synced database would settle it.",
      "body": "**The first open silently drops edits that were queued but never pushed.**\n\nI made a database with the base binary and inserted one `pending_sync` row. I wrote a `[sync] enabled = true` config and ran this branch's `projects list`. It printed the `[sync]` warning and exited 0. After that run `.tables` showed no `pending_sync`. Neither the warning nor anything else said that a queued edit had never reached the API.\n\nBefore this PR the status bar showed `PENDING (n)`, and the README said such a change \"is sent once it comes back\". After this PR the edit lives only in a database the PR calls standalone. The desk then moves to `api`, so nothing reads that database again. The edit is stranded.\n\nThe PR's own rollback makes this worse. The rollback recreates the tables empty and leaves the database \"bound to no API until a `projects sync --adopt`\". The old README says a pull \"deletes every local row the server did not return\". So the adopt step deletes exactly the rows whose pushes were dropped.\n\nThe drop ships in the same release as the code that stops reading the tables, which `data.md` § \"A destructive migration ships as its own release, after the release that stopped reading\" exists to prevent.\n\nFix: in `dropSyncTables`, count `pending_sync` first. When it holds rows, refuse to open. Name the count and tell the user to run `projects sync` with the previous release before upgrading. The other two tables hold nothing a person wrote and can drop unconditionally. Add a test that seeds a pending row and asserts the refusal."
    },
    {
      "number": "G1.2",
      "section": "design",
      "sites": [
        "store/sqlite/open.go:140",
        "store/sqlite/open.go:299",
        "store/sqlite/open_test.go"
      ],
      "reach": "The `MIGRATION` marker in `config/retired_sync.go` plans the next change to `migrate()`, which is retiring `dropSyncTables`. That change and any new migration reorder or edit this chain with nothing pinning two of its steps.",
      "body": "**The diff deletes the only test of a live migration and adds none for the new one.**\n\nThe diff deletes `TestMigrate_AddsNumberColumnToExistingDatabase`. `migrateItemNumber` still runs on every database older than the number column. The README still says such databases exist (\"a database from before numbers existed\"). The deleted test seeded a pre-number row and asserted three things: the title survived, `number` stayed null, and a second `migrate` was a no-op. Only its comment mentioned sync. No remaining test in `open_test.go` builds a table without `number`.\n\n`dropSyncTables` has no test at all. The PR body asks reviewers to check that it \"touches nothing a person wrote, and that it runs on a database of any age\". Only a hand run checks that, and a hand run does not repeat on the next change.\n\nFix: restore `TestMigrate_AddsNumberColumnToExistingDatabase` with its comment reworded to the null-number rule. Add one test that builds a database with the three sync tables plus a project, an item, a membership and an undo entry. Run `migrate` twice, then assert that the sync tables are gone and every data row is unchanged."
    },
    {
      "number": "G1.3",
      "section": "prose",
      "sites": [
        "CLAUDE.md:28"
      ],
      "reach": "",
      "body": "**The sentence about `.envrc` reads as its opposite.**\n\nThe sentence is \"It points `PROJECTS_DB` at `dev.db` for `TestRealDatabaseSurvivesEverySizeAndProject`, and keeps no run off the API the config file names.\" \"Keeps no run off the API\" parses as a double negative. A reader can take it to mean that `.envrc` guards against the API, which is the opposite of the warning two lines above it.\n\nThe file also gives no instruction for an `.envrc` that still exports `PROJECTS_SYNC=true`, which the old version of this file told every machine to hold. That variable now stops every `go run` with exit 2.\n\nRewrite: \"It sets `PROJECTS_DB` to `dev.db` for `TestRealDatabaseSurvivesEverySizeAndProject` and sets nothing else, so a run with the machine's config file still goes to the API that file's `api` names.\""
    }
  ]
}

This review does not gate merge.

datapoints-fleet-automation

PR review — failure-paths

Used: Checked out PR 20 head. Ran a throwaway Go test on store/sqlite: seeded a database with a pending_sync row, reopened it with Open, and the table was gone with exit 0. Test file removed afterwards.
Standards: Searched "destructive migration drops table holding unsynced pending writes". It surfaced data.md "A destructive migration ships as its own release, after the release that stopped reading" and data.md "Migrations are backward-compatible". I did not read either rule in full.
Findings: 1 correctness · 0 design · 0 prose

Correctness

failure-paths.1 store/sqlite/open.go:137 · store/sqlite/open.go:80

Reach: Any machine whose dev or production database has unpushed rows in pending_sync. These are writes made while the API was unreachable, or since the last push in the TUI. Upgrading and running any command that opens the database triggers the drop. I ran it: a database with a pending_sync row, reopened through Open, returned 'no such table: pending_sync' and Open returned no error.

dropSyncTables runs DROP TABLE IF EXISTS pending_sync on every existing database at open. It does this without checking whether the queue still holds rows. The comment says 'every row a person wrote stays'. That is wrong for the queue. Those rows are the only record of writes the API never received. The local projects and items tables may already carry the change, but the API never got it. After the drop, nothing says the API is behind, and the API stays that way for good. Exit is 0 and nothing is printed.

Fix: before dropping, count the rows in pending_sync. If any exist, refuse with an error that names the count and tells the person to run the previous release's push, or to export the rows. Alternatively, rename the table to pending_sync_retired and warn on stderr. This also matches data.md § "A destructive migration ships as its own release, after the release that stopped reading". Drop the table in a later release.

Findings as JSON

{
  "used": "Checked out PR 20 head. Ran a throwaway Go test on store/sqlite: seeded a database with a pending_sync row, reopened it with Open, and the table was gone with exit 0. Test file removed afterwards.",
  "standards": "Searched \"destructive migration drops table holding unsynced pending writes\". It surfaced data.md \"A destructive migration ships as its own release, after the release that stopped reading\" and data.md \"Migrations are backward-compatible\". I did not read either rule in full.",
  "findings": [
    {
      "number": "failure-paths.1",
      "section": "correctness",
      "sites": [
        "store/sqlite/open.go:137",
        "store/sqlite/open.go:80"
      ],
      "reach": "Any machine whose dev or production database has unpushed rows in pending_sync. These are writes made while the API was unreachable, or since the last push in the TUI. Upgrading and running any command that opens the database triggers the drop. I ran it: a database with a pending_sync row, reopened through Open, returned 'no such table: pending_sync' and Open returned no error.",
      "body": "`dropSyncTables` runs `DROP TABLE IF EXISTS pending_sync` on every existing database at open. It does this without checking whether the queue still holds rows. The comment says 'every row a person wrote stays'. That is wrong for the queue. Those rows are the only record of writes the API never received. The local projects and items tables may already carry the change, but the API never got it. After the drop, nothing says the API is behind, and the API stays that way for good. Exit is 0 and nothing is printed.\n\nFix: before dropping, count the rows in `pending_sync`. If any exist, refuse with an error that names the count and tells the person to run the previous release's push, or to export the rows. Alternatively, rename the table to `pending_sync_retired` and warn on stderr. This also matches data.md § \"A destructive migration ships as its own release, after the release that stopped reading\". Drop the table in a later release."
    }
  ]
}

This review does not gate merge.

datapoints-fleet-automation

PR review — contracts

Used: I built the PR head (67e8fa1) in my clone. I ran it under a private HOME and XDG dirs, using the work box's real config, copied from dotfiles configs/trust/nonfleet/.config/projects/config.toml. Commands run: list --json, sync, --help and version with PROJECTS_SYNC=false set, and list on a database where I had added a pending_sync row. Callers were searched with gh search code --owner datapointchris (rate limit reached partway) and an rg of a shallow dotfiles clone.
Standards: Cited data.md § "A destructive migration ships as its own release, after the release that stopped reading" and configuration.md § "A retired environment variable that picked a target is refused while it is still set". The search "retired config key section warning removed" found no rule about a retired config section.
Findings: 1 correctness · 1 design · 0 prose

Correctness

contracts.1 store/sqlite/open.go:137 · store/sqlite/open.go:80

Reach: unproven. Every binary self-updates through goselfupdate (cli/root.go:147), so the first run on a machine whose database still synced drops its queue. No config in dotfiles sets [sync] enabled = true today. One check would settle it: count pending_sync rows on each desk's projects.db before the release ships.

The upgrade deletes unpushed writes without a word. dropSyncTables runs DROP TABLE IF EXISTS pending_sync on the first open. It does not look at whether the queue still holds rows. Those rows are writes made on the board while the API was unreachable, and they never reached the API. After the drop nothing can push them, and nothing reports that they existed.

What I ran: I made a database with the head binary, then added pending_sync and inserted one create item row. I then ran projects list. It exited 0, printed only the [sync] warning and the empty list, and afterwards sqlite_master held no table matching %sync%. The row was gone and nothing said so.

The PR checks that every row in the five data tables survives. It does not check this table. data.md § "A destructive migration ships as its own release, after the release that stopped reading" covers the shape: this one release both stops reading the tables and drops them. The probe-based migrations mean a reverted binary does reopen the database, but the queue does not come back.

Fix: refuse to drop a non-empty pending_sync, and name the count and the database path. The message should say how to finish the push with the previous release (projects tui under v1.24.x), or how to discard the queue on purpose. Another route is to stop reading the tables in this release and drop them in a later one.

Design and architecture

contracts.2 config/retired_sync.go:25 · cli/data_source.go:115 · dotfiles:configs/trust/nonfleet/.config/projects/config.toml:20

Reach: dotfiles:configs/trust/nonfleet/.config/projects/config.toml:20 holds [sync] enabled = false / api_url = …, and it is the only config in the fleet that still has the section. Each data command on the work box will print the warning until dotfiles drops it. The MIGRATION note in config/retired_sync.go cannot close until then.

The only [sync] caller lives in dotfiles, and this PR leaves it there. I ran the head against that file verbatim. projects list --json printed warning: [sync] in …/projects/config.toml is ignored, because projects keeps no synced copy of an API. Remove the section… on stderr, then [], and exited 0. Stdout is clean, so a script that parses --json still works. Still, every command on the work box warns from now on.

The file's own comment says "Sync is off here and on in the other copy". The fleet copy (config.toml.tmpl) now holds api and has no [sync], so that comment is also stale.

The PR's MIGRATION comment makes its removal depend on "no machine's config holds [sync]". The one machine that holds it gets its config from this file.

Fix: send the dotfiles change that deletes the [sync] block and its comment, together with this release. Link it from the PR so the retirement has a named end.

Findings as JSON

{
  "used": "I built the PR head (67e8fa1) in my clone. I ran it under a private HOME and XDG dirs, using the work box's real config, copied from dotfiles configs/trust/nonfleet/.config/projects/config.toml. Commands run: `list --json`, `sync`, `--help` and `version` with PROJECTS_SYNC=false set, and `list` on a database where I had added a pending_sync row. Callers were searched with `gh search code --owner datapointchris` (rate limit reached partway) and an rg of a shallow dotfiles clone.",
  "standards": "Cited data.md § \"A destructive migration ships as its own release, after the release that stopped reading\" and configuration.md § \"A retired environment variable that picked a target is refused while it is still set\". The search \"retired config key section warning removed\" found no rule about a retired config section.",
  "findings": [
    {
      "number": "contracts.1",
      "section": "correctness",
      "sites": [
        "store/sqlite/open.go:137",
        "store/sqlite/open.go:80"
      ],
      "reach": "unproven. Every binary self-updates through goselfupdate (cli/root.go:147), so the first run on a machine whose database still synced drops its queue. No config in dotfiles sets `[sync] enabled = true` today. One check would settle it: count pending_sync rows on each desk's projects.db before the release ships.",
      "body": "**The upgrade deletes unpushed writes without a word.** `dropSyncTables` runs `DROP TABLE IF EXISTS pending_sync` on the first open. It does not look at whether the queue still holds rows. Those rows are writes made on the board while the API was unreachable, and they never reached the API. After the drop nothing can push them, and nothing reports that they existed.\n\nWhat I ran: I made a database with the head binary, then added `pending_sync` and inserted one `create item` row. I then ran `projects list`. It exited 0, printed only the `[sync]` warning and the empty list, and afterwards `sqlite_master` held no table matching `%sync%`. The row was gone and nothing said so.\n\nThe PR checks that every row in the five data tables survives. It does not check this table. data.md § \"A destructive migration ships as its own release, after the release that stopped reading\" covers the shape: this one release both stops reading the tables and drops them. The probe-based migrations mean a reverted binary does reopen the database, but the queue does not come back.\n\nFix: refuse to drop a non-empty `pending_sync`, and name the count and the database path. The message should say how to finish the push with the previous release (`projects tui` under v1.24.x), or how to discard the queue on purpose. Another route is to stop reading the tables in this release and drop them in a later one."
    },
    {
      "number": "contracts.2",
      "section": "design",
      "sites": [
        "config/retired_sync.go:25",
        "cli/data_source.go:115",
        "dotfiles:configs/trust/nonfleet/.config/projects/config.toml:20"
      ],
      "reach": "dotfiles:configs/trust/nonfleet/.config/projects/config.toml:20 holds `[sync] enabled = false / api_url = …`, and it is the only config in the fleet that still has the section. Each data command on the work box will print the warning until dotfiles drops it. The MIGRATION note in config/retired_sync.go cannot close until then.",
      "body": "**The only `[sync]` caller lives in dotfiles, and this PR leaves it there.** I ran the head against that file verbatim. `projects list --json` printed `warning: [sync] in …/projects/config.toml is ignored, because projects keeps no synced copy of an API. Remove the section…` on stderr, then `[]`, and exited 0. Stdout is clean, so a script that parses `--json` still works. Still, every command on the work box warns from now on.\n\nThe file's own comment says \"Sync is off here and on in the other copy\". The fleet copy (`config.toml.tmpl`) now holds `api` and has no `[sync]`, so that comment is also stale.\n\nThe PR's MIGRATION comment makes its removal depend on \"no machine's config holds [sync]\". The one machine that holds it gets its config from this file.\n\nFix: send the dotfiles change that deletes the `[sync]` block and its comment, together with this release. Link it from the PR so the retirement has a named end."
    }
  ]
}

This review does not gate merge.

datapoints-fleet-automation

PR review — useless-test

Used: I cloned the PR at 67e8fa1 and read every edited test beside the code it covers: config/config_test.go, former_name_test.go, service/service_test.go, service/number_test.go, store/sqlite/open_test.go, tui/app_test.go, tui/realdata_test.go and real_binary_flows_test.go. go test -short ./tui/ ./config/ ./service/ passes. I then changed !m.safeToReread() in the rereadTickMsg case of tui/app.go to false && ... and both reread tests still passed. I restored the file and the clone is clean.
Standards: I applied three rules: testing.md § "Test behavior the user cares about, not what this commit happened to change" (the shape where a test asserts a value one internal function hands another), pull-requests.md § "Tests the diff adds are checked for whether they test the program or the change", and testing.md § "Never assert on rendered output — assert the value it was built from".
Findings: 0 correctness · 1 design · 0 prose

Design and architecture

useless-test.1 tui/app_test.go:303

Reach: The diff renames safeToAutoPull to safeToReread and rewrites the rereadTickMsg case in tui/app.go that calls it. In API mode, Init arms the reread tick every two minutes, and the gate is the only thing that stops a read from landing during a grab or a text entry.

TestRereadOnlyRunsInNormalMode checks the gate's predicate but not that the tick obeys it

Delete tui/app_test.go TestRereadOnlyRunsInNormalMode.

The test calls app.safeToReread() directly in each mode. That function is the one-liner m.appMode == modeNormal. Its only caller is the rereadTickMsg case of Update. The test checks the value one internal function hands another. It never checks what the tick does with that value.

I made Update ignore the gate (if false && !m.safeToReread()), so every tick reads during a grab, an add-item prompt or the reader. With that change, go test -short ./tui/ -run Reread still printed PASS for this test and for TestRereadTickRearmsEvenWhenItSkipsTheRead. The behavior a user reaches broke, and no test failed.

This PR also deletes TestSyncTickReconcilesWithTheServer. That was the end-to-end test that sent a tick in modeAddItem, counted zero reads, then sent a tick in modeNormal and saw the read land. No test pins the gate now.

Keep TestRereadTickRearmsEvenWhenItSkipsTheRead. It drives Update and pins a real behavior: the timer survives a skipped tick.

Replacement test: build the app as in TestRInAPIModeShowsAnItemAnotherMachineWrote, with Where{APIHost: ...}. Create an item through the backend after launch. Set appMode = modeMove, send rereadTickMsg{}, drain the command, and assert app.items still lacks the new item. Then set appMode = modeNormal, send the tick again, and assert the item appears. That test fails when the gate is dropped from Update. It also survives renaming or inlining safeToReread.

Findings as JSON

{
  "used": "I cloned the PR at 67e8fa1 and read every edited test beside the code it covers: config/config_test.go, former_name_test.go, service/service_test.go, service/number_test.go, store/sqlite/open_test.go, tui/app_test.go, tui/realdata_test.go and real_binary_flows_test.go. `go test -short ./tui/ ./config/ ./service/` passes. I then changed `!m.safeToReread()` in the rereadTickMsg case of tui/app.go to `false && ...` and both reread tests still passed. I restored the file and the clone is clean.",
  "standards": "I applied three rules: testing.md § \"Test behavior the user cares about, not what this commit happened to change\" (the shape where a test asserts a value one internal function hands another), pull-requests.md § \"Tests the diff adds are checked for whether they test the program or the change\", and testing.md § \"Never assert on rendered output — assert the value it was built from\".",
  "findings": [
    {
      "number": "useless-test.1",
      "section": "design",
      "sites": [
        "tui/app_test.go:303"
      ],
      "reach": "The diff renames `safeToAutoPull` to `safeToReread` and rewrites the `rereadTickMsg` case in tui/app.go that calls it. In API mode, `Init` arms the reread tick every two minutes, and the gate is the only thing that stops a read from landing during a grab or a text entry.",
      "body": "## TestRereadOnlyRunsInNormalMode checks the gate's predicate but not that the tick obeys it\n\nDelete `tui/app_test.go` `TestRereadOnlyRunsInNormalMode`.\n\nThe test calls `app.safeToReread()` directly in each mode. That function is the one-liner `m.appMode == modeNormal`. Its only caller is the `rereadTickMsg` case of `Update`. The test checks the value one internal function hands another. It never checks what the tick does with that value.\n\nI made `Update` ignore the gate (`if false && !m.safeToReread()`), so every tick reads during a grab, an add-item prompt or the reader. With that change, `go test -short ./tui/ -run Reread` still printed `PASS` for this test and for `TestRereadTickRearmsEvenWhenItSkipsTheRead`. The behavior a user reaches broke, and no test failed.\n\nThis PR also deletes `TestSyncTickReconcilesWithTheServer`. That was the end-to-end test that sent a tick in `modeAddItem`, counted zero reads, then sent a tick in `modeNormal` and saw the read land. No test pins the gate now.\n\nKeep `TestRereadTickRearmsEvenWhenItSkipsTheRead`. It drives `Update` and pins a real behavior: the timer survives a skipped tick.\n\nReplacement test: build the app as in `TestRInAPIModeShowsAnItemAnotherMachineWrote`, with `Where{APIHost: ...}`. Create an item through the backend after launch. Set `appMode = modeMove`, send `rereadTickMsg{}`, drain the command, and assert `app.items` still lacks the new item. Then set `appMode = modeNormal`, send the tick again, and assert the item appears. That test fails when the gate is dropped from `Update`. It also survives renaming or inlining `safeToReread`."
    }
  ]
}

This review does not gate merge.

datapoints-fleet-automation

PR review — stale-prose

Used: I cloned the repo and checked out PR 20 at 67e8fa1. I built the binary into my own directory and gave it its own HOME, XDG_CONFIG_HOME, XDG_DATA_HOME and XDG_STATE_HOME. First I ran projects list with a config holding [sync]: it printed the warning and exited 0. Next I ran PROJECTS_SYNC_INTERVAL=5m projects version: exit 2, with the refusal. Then I ran TODOUI_SYNC=1 projects list: exit 2. Last, I used rg to check every added comment, doc line and message against the code at head.
Standards: I applied the rule pull-requests.md/Sentences the diff adds are checked for what goes stale, and the finding carries the rewrite. fleet standards search turned up three more: documentation.md/A comment describes the code as it is, never the change or history that produced it, documentation.md/A count, version or measurement pinned in a comment or doc names what made it true and how it was measured, or goes stale as a constant, and documentation.md/A rename, move or behavior change sweeps every doc and comment that describes it, from above every repo.
Findings: 0 correctness · 0 design · 4 prose

Prose

stale-prose.1 CLAUDE.md:39 · CLAUDE.md:44 · config/retired_sync.go:8

False. The sync leftovers sit in four files. The retirement list leaves one of them out.

CLAUDE.md:39: "The synced copy's leftovers are handled in one file too." The same paragraph names four files: config/retired_sync.go, Config.RetiredSync in config/config.go, warnOfRetiredSyncSection in cli/data_source.go, and dropSyncTables in store/sqlite/open.go.
CLAUDE.md:44: "The file's MIGRATION marker says when it retires." This sentence follows the dropSyncTables sentence, so it reads as covering dropSyncTables. It does not.
config/retired_sync.go:8-10: "Delete this file, Config.RetiredSync, its call in main.go and the warning in cli/data_source.go once…". rg -n MIGRATION finds only this marker and the one in former_name.go. Nothing tells whoever retires the sync code that dropSyncTables in store/sqlite/open.go goes too.

Rewrite for CLAUDE.md:39: "The synced copy's leftovers retire together, under the MIGRATION marker in config/retired_sync.go." Add dropSyncTables in store/sqlite/open.go to the marker's delete list. Or put a MIGRATION: retires with config/retired_sync.go line on dropSyncTables.

stale-prose.2 CLAUDE.md:43-44 · service/service.go:39-40

A value the code holds. Two comments copy out lists the code already defines.

CLAUDE.md:43-44: "dropSyncTables in store/sqlite/open.go drops pending_sync, sync_state and sync_origin on the first open." The three table names live in the drop constant in open.go. That function's own comment already describes them by role: the push queue, the pull times, and the bound API. Rewrite: "dropSyncTables in store/sqlite/open.go drops the synced copy's own tables on the first open, and no row a person wrote."
service/service.go:39-40: "Both entry points pass it: database mode in cli.LocalService, and projects-api over Postgres." This is a count and a list of callers. rg -n 'AssigningItemNumbers\(\)' also finds api/routes/routes_test.go:48, tui/app_test.go:42, store/postgres/store_test.go:23 and service/service_test.go:33. The next entry point makes "Both" wrong. Rewrite: "Every running service passes it. A Service built without it creates unnumbered items." That second sentence says what the option is for, which a list of callers cannot show.

stale-prose.3 CLAUDE.md:24-27 · config/config_test.go:35-36

Will go stale. Two sentences describe files outside the repo, and nothing in the repo keeps them true.

CLAUDE.md:24-27: ".envrc is machine-local and is not in this repo. … It points PROJECTS_DB at dev.db for TestRealDatabaseSurvivesEverySizeAndProject, and keeps no run off the API the config file names." The paragraph itself says the file is not in the repo, so the repo cannot hold its contents level. Rewrite: ".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." Keep the last sentence only if the skip is real.
config/config_test.go:35-36: "The [sync] section below it is the work box's file, which has to load rather than stop the tool." The test writes [sync]\nenabled = false. That is not anyone's file, and another machine's config can change without this test changing. Rewrite: "The [sync] section below it has to load, so a machine whose config still holds one keeps running."

stale-prose.4 README.md:256-258

The change narrated. The README describes what the tool used to be.

README.md:257-258: "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." "Once" and "now" describe the change, not the tool. The diff and the release notes carry the change. The upgrade bullets below this sentence say what a reader with a leftover [sync] section or PROJECTS_SYNC* variable needs to know.

Rewrite: "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." Keep the three bullets as they are.

Findings as JSON

{
  "used": "I cloned the repo and checked out PR 20 at 67e8fa1. I built the binary into my own directory and gave it its own HOME, XDG_CONFIG_HOME, XDG_DATA_HOME and XDG_STATE_HOME. First I ran `projects list` with a config holding `[sync]`: it printed the warning and exited 0. Next I ran `PROJECTS_SYNC_INTERVAL=5m projects version`: exit 2, with the refusal. Then I ran `TODOUI_SYNC=1 projects list`: exit 2. Last, I used rg to check every added comment, doc line and message against the code at head.",
  "standards": "I applied the rule `pull-requests.md/Sentences the diff adds are checked for what goes stale, and the finding carries the rewrite`. `fleet standards search` turned up three more: `documentation.md/A comment describes the code as it is, never the change or history that produced it`, `documentation.md/A count, version or measurement pinned in a comment or doc names what made it true and how it was measured, or goes stale as a constant`, and `documentation.md/A rename, move or behavior change sweeps every doc and comment that describes it, from above every repo`.",
  "findings": [
    {
      "number": "stale-prose.1",
      "section": "prose",
      "sites": [
        "CLAUDE.md:39",
        "CLAUDE.md:44",
        "config/retired_sync.go:8"
      ],
      "reach": "",
      "body": "**False. The sync leftovers sit in four files. The retirement list leaves one of them out.**\n\n- `CLAUDE.md:39`: \"**The synced copy's leftovers are handled in one file too.**\" The same paragraph names four files: `config/retired_sync.go`, `Config.RetiredSync` in `config/config.go`, `warnOfRetiredSyncSection` in `cli/data_source.go`, and `dropSyncTables` in `store/sqlite/open.go`.\n- `CLAUDE.md:44`: \"The file's `MIGRATION` marker says when it retires.\" This sentence follows the `dropSyncTables` sentence, so it reads as covering `dropSyncTables`. It does not.\n- `config/retired_sync.go:8-10`: \"Delete this file, Config.RetiredSync, its call in main.go and the warning in cli/data_source.go once…\". `rg -n MIGRATION` finds only this marker and the one in `former_name.go`. Nothing tells whoever retires the sync code that `dropSyncTables` in `store/sqlite/open.go` goes too.\n\nRewrite for `CLAUDE.md:39`: \"**The synced copy's leftovers retire together, under the `MIGRATION` marker in `config/retired_sync.go`.**\" Add `dropSyncTables in store/sqlite/open.go` to the marker's delete list. Or put a `MIGRATION: retires with config/retired_sync.go` line on `dropSyncTables`."
    },
    {
      "number": "stale-prose.2",
      "section": "prose",
      "sites": [
        "CLAUDE.md:43-44",
        "service/service.go:39-40"
      ],
      "reach": "",
      "body": "**A value the code holds. Two comments copy out lists the code already defines.**\n\n- `CLAUDE.md:43-44`: \"`dropSyncTables` in `store/sqlite/open.go` drops `pending_sync`, `sync_state` and `sync_origin` on the first open.\" The three table names live in the `drop` constant in `open.go`. That function's own comment already describes them by role: the push queue, the pull times, and the bound API. Rewrite: \"`dropSyncTables` in `store/sqlite/open.go` drops the synced copy's own tables on the first open, and no row a person wrote.\"\n- `service/service.go:39-40`: \"Both entry points pass it: database mode in cli.LocalService, and projects-api over Postgres.\" This is a count and a list of callers. `rg -n 'AssigningItemNumbers\\(\\)'` also finds `api/routes/routes_test.go:48`, `tui/app_test.go:42`, `store/postgres/store_test.go:23` and `service/service_test.go:33`. The next entry point makes \"Both\" wrong. Rewrite: \"Every running service passes it. A Service built without it creates unnumbered items.\" That second sentence says what the option is for, which a list of callers cannot show."
    },
    {
      "number": "stale-prose.3",
      "section": "prose",
      "sites": [
        "CLAUDE.md:24-27",
        "config/config_test.go:35-36"
      ],
      "reach": "",
      "body": "**Will go stale. Two sentences describe files outside the repo, and nothing in the repo keeps them true.**\n\n- `CLAUDE.md:24-27`: \"`.envrc` is machine-local and is not in this repo. … It points `PROJECTS_DB` at `dev.db` for `TestRealDatabaseSurvivesEverySizeAndProject`, and keeps no run off the API the config file names.\" The paragraph itself says the file is not in the repo, so the repo cannot hold its contents level. Rewrite: \"`.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.\" Keep the last sentence only if the skip is real.\n- `config/config_test.go:35-36`: \"The [sync] section below it is the work box's file, which has to load rather than stop the tool.\" The test writes `[sync]\\nenabled = false`. That is not anyone's file, and another machine's config can change without this test changing. Rewrite: \"The [sync] section below it has to load, so a machine whose config still holds one keeps running.\""
    },
    {
      "number": "stale-prose.4",
      "section": "prose",
      "sites": [
        "README.md:256-258"
      ],
      "reach": "",
      "body": "**The change narrated. The README describes what the tool used to be.**\n\n- `README.md:257-258`: \"`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.\" \"Once\" and \"now\" describe the change, not the tool. The diff and the release notes carry the change. The upgrade bullets below this sentence say what a reader with a leftover `[sync]` section or `PROJECTS_SYNC*` variable needs to know.\n\nRewrite: \"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.\" Keep the three bullets as they are."
    }
  ]
}

This review does not gate merge.

datapoints-fleet-automation

PR review — fleet-patterns

Used: I checked out PR 20 in a clone and read its diff for config/retired_sync.go, config/former_name.go, main.go, config/config.go, cli/data_source.go, store/sqlite/open.go and tui/app.go. I ran gh search code --owner datapointchris for five things: the retired-variable refusal, PROJECTS_SYNC, imports of projects/sync, tea.Tick re-read loops and [sync] configs. Searching for DROP TABLE IF EXISTS across Go repos found no other repo. I read ichrisbirch's retiredAPIBase refusal and the dotfiles projects config. GitHub's search rate limit cut off one last query, which looked for other repos setting api_url. A [sync] config written outside dotfiles would show up there.
Standards: The change follows configuration.md § "A retired environment variable that picked a target is refused while it is still set". The search "config file key no longer read unknown key ErrorUnused" found no rule for a retired config-file section.
Findings: 0 correctness · 0 design · 0 prose

No findings.

Findings as JSON

{
  "used": "I checked out PR 20 in a clone and read its diff for `config/retired_sync.go`, `config/former_name.go`, `main.go`, `config/config.go`, `cli/data_source.go`, `store/sqlite/open.go` and `tui/app.go`. I ran `gh search code --owner datapointchris` for five things: the retired-variable refusal, `PROJECTS_SYNC`, imports of `projects/sync`, `tea.Tick` re-read loops and `[sync]` configs. Searching for `DROP TABLE IF EXISTS` across Go repos found no other repo. I read ichrisbirch's `retiredAPIBase` refusal and the dotfiles projects config. GitHub's search rate limit cut off one last query, which looked for other repos setting `api_url`. A `[sync]` config written outside dotfiles would show up there.",
  "standards": "The change follows `configuration.md` § \"A retired environment variable that picked a target is refused while it is still set\". The search \"config file key no longer read unknown key ErrorUnused\" found no rule for a retired config-file section.",
  "findings": []
}

This review does not gate merge.

datapointchris

PR review — synthesis

Used: the 15 findings of round 1, from 8 review(s), and nothing else
Standards: none, because this step reads only the findings
Findings: 0 correctness · 3 design · 0 prose

Design and architecture

synthesis.1 explains stale-prose.2 · stale-prose.3 · code-architecture.2

Prose states a list, count or file content that the code or an outside file already owns, so it goes stale. Sites: CLAUDE.md:43-44 and service/service.go:39-40 (stale-prose.2), CLAUDE.md:24-27 and config/config_test.go:35-36 (stale-prose.3), the interval written in cli/tui.go:50 and README.md:186 (code-architecture.2).

synthesis.2 explains stale-prose.1 · contracts.2 · code-architecture.3

The sync retirement leaves loose ends with no single owner: the dropSyncTables retirement is not tied to the MIGRATION marker (stale-prose.1), the dotfiles config still holds [sync] (contracts.2), and the retired-variable lists are split across two files with separate checks in main.go (code-architecture.3).

synthesis.3 explains G1.2 · useless-test.1

The PR deletes tests of live behavior and leaves the replacements unwritten. The deleted tests are TestMigrate_AddsNumberColumnToExistingDatabase and TestSyncTickReconcilesWithTheServer. Nothing pins dropSyncTables or the reread gate, and the one remaining gate test checks only the predicate.

One defect, raised more than once

G1.1 · system-design.1 · failure-paths.1 · contracts.1: dropSyncTables in store/sqlite/open.go drops pending_sync on first open without checking for unpushed rows, so queued edits are lost silently.

Shapes as JSON

{
  "shapes": [
    {
      "number": "synthesis.1",
      "section": "design",
      "explains": [
        "stale-prose.2",
        "stale-prose.3",
        "code-architecture.2"
      ],
      "body": "Prose states a list, count or file content that the code or an outside file already owns, so it goes stale. Sites: CLAUDE.md:43-44 and service/service.go:39-40 (stale-prose.2), CLAUDE.md:24-27 and config/config_test.go:35-36 (stale-prose.3), the interval written in cli/tui.go:50 and README.md:186 (code-architecture.2)."
    },
    {
      "number": "synthesis.2",
      "section": "design",
      "explains": [
        "stale-prose.1",
        "contracts.2",
        "code-architecture.3"
      ],
      "body": "The sync retirement leaves loose ends with no single owner: the dropSyncTables retirement is not tied to the MIGRATION marker (stale-prose.1), the dotfiles config still holds `[sync]` (contracts.2), and the retired-variable lists are split across two files with separate checks in main.go (code-architecture.3)."
    },
    {
      "number": "synthesis.3",
      "section": "design",
      "explains": [
        "G1.2",
        "useless-test.1"
      ],
      "body": "The PR deletes tests of live behavior and leaves the replacements unwritten. The deleted tests are TestMigrate_AddsNumberColumnToExistingDatabase and TestSyncTickReconcilesWithTheServer. Nothing pins dropSyncTables or the reread gate, and the one remaining gate test checks only the predicate."
    }
  ],
  "duplicates": [
    {
      "defect": "dropSyncTables in store/sqlite/open.go drops pending_sync on first open without checking for unpushed rows, so queued edits are lost silently.",
      "findings": [
        "G1.1",
        "system-design.1",
        "failure-paths.1",
        "contracts.1"
      ]
    }
  ]
}

This review does not gate merge.

datapointchris · 2026-11-08T06:57:43Z

PR review round 1

Ran: G1, useless-test, code-architecture, fleet-patterns, system-design, stale-prose, failure-paths, contracts, synthesis
Did not run: none
Rules: 1 proposed, filed into the fleet inbox as proposals.

reviewer	model	effort	duration	uncached input	cache write	cache read	output	findings	review
G1	opus	high	2m 30s	38	60,228	828,084	10,354	3	review
useless-test	opus	high	4m 12s	18	51,269	295,591	5,359	1	review
code-architecture	opus	medium	1m 13s	24	35,704	323,221	5,725	3	review
fleet-patterns	opus	medium	1m 03s	18	24,940	204,439	2,613	0	review
system-design	sonnet	medium	0m 26s	10	10,559	68,087	2,158	1	review
stale-prose	opus	medium	3m 31s	16	34,707	207,170	7,522	4	review
failure-paths	sonnet	medium	2m 52s	16	24,179	158,844	2,872	1	review
contracts	opus	medium	4m 00s	38	39,636	579,402	8,034	2	review
synthesis	sonnet	medium	0m 06s	2	9,461	1,433	619	3	review
total				180	290,683	2,666,271	45,256	18

Round record

{
  "grammar": 6,
  "round": 1,
  "repo": "datapointchris/projects",
  "pr": 20,
  "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
  "experiment": "none",
  "dispatch": "20261108T065057Z-pr-review",
  "roster": "1ee22e0694913fcfb159f68268741c1851bff2a5",
  "harness": "1b0df0090dc84b9bf846796f6afafba9a2ef8e48",
  "machine": "archlinux",
  "started": "2026-11-08T06:51:19Z",
  "finished": "2026-11-08T06:57:42Z",
  "reviewers": [
    {
      "label": "G1",
      "agent": "pr-reviewer",
      "model": "opus",
      "effort": "high",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/general.md",
      "prompt_sha": "a0f85e3b82b4",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 150,
      "tokens": {
        "input": 38,
        "cache_write": 60228,
        "cache_read": 828084,
        "output": 10354
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 38,
          "cache_write": 60228,
          "cache_read": 828084,
          "output": 10354
        }
      },
      "session": "01a1b9ec-e572-7b79-a87b-82ec81700830",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816963727",
      "review_id": 5816963727,
      "findings": {
        "correctness": 1,
        "design": 1,
        "prose": 1
      },
      "numbers": [
        "G1.1",
        "G1.2",
        "G1.3"
      ],
      "rules": {
        "proposed": 1,
        "keys": [
          "rule/data/a-migration-that-drops-a-table-refuses-while-the-table-holds-rows-nothing-else-carries"
        ],
        "filed": true,
        "why": "",
        "path": "/home/chris/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records/G1.rules.json"
      },
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:51:19Z",
      "finished": "2026-11-08T06:53:53Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    },
    {
      "label": "useless-test",
      "agent": "pr-useless-test-reviewer",
      "model": "opus",
      "effort": "high",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/useless-test.md",
      "prompt_sha": "3700d72236e3",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 252,
      "tokens": {
        "input": 18,
        "cache_write": 51269,
        "cache_read": 295591,
        "output": 5359
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 18,
          "cache_write": 51269,
          "cache_read": 295591,
          "output": 5359
        }
      },
      "session": "01a1b9ec-eaa2-7b41-bffc-eceea4d4fe83",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816968840",
      "review_id": 5816968840,
      "findings": {
        "correctness": 0,
        "design": 1,
        "prose": 0
      },
      "numbers": [
        "useless-test.1"
      ],
      "rules": {},
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:51:47Z",
      "finished": "2026-11-08T06:56:05Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "queue"
    },
    {
      "label": "code-architecture",
      "agent": "pr-code-architecture-reviewer",
      "model": "opus",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "d9be86eb4324",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 73,
      "tokens": {
        "input": 24,
        "cache_write": 35704,
        "cache_read": 323221,
        "output": 5725
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 24,
          "cache_write": 35704,
          "cache_read": 323221,
          "output": 5725
        }
      },
      "session": "01a1b9ec-e7bc-7f51-98f5-62651c64cdc4",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816960834",
      "review_id": 5816960834,
      "findings": {
        "correctness": 0,
        "design": 3,
        "prose": 0
      },
      "numbers": [
        "code-architecture.1",
        "code-architecture.2",
        "code-architecture.3"
      ],
      "rules": {},
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:51:20Z",
      "finished": "2026-11-08T06:52:35Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    },
    {
      "label": "fleet-patterns",
      "agent": "pr-fleet-patterns-reviewer",
      "model": "opus",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "eae343d1d8d5",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 63,
      "tokens": {
        "input": 18,
        "cache_write": 24940,
        "cache_read": 204439,
        "output": 2613
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 18,
          "cache_write": 24940,
          "cache_read": 204439,
          "output": 2613
        }
      },
      "session": "01a1b9f0-cca0-79a6-a771-e2d163919053",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816970879",
      "review_id": 5816970879,
      "findings": {
        "correctness": 0,
        "design": 0,
        "prose": 0
      },
      "numbers": [],
      "rules": {},
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:55:53Z",
      "finished": "2026-11-08T06:57:00Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "queue"
    },
    {
      "label": "system-design",
      "agent": "pr-system-design-reviewer",
      "model": "sonnet",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "c284e39c894a",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 26,
      "tokens": {
        "input": 10,
        "cache_write": 10559,
        "cache_read": 68087,
        "output": 2158
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-sonnet-5-5": {
          "input": 10,
          "cache_write": 10559,
          "cache_read": 68087,
          "output": 2158
        }
      },
      "session": "01a1b9ec-e6dd-79a2-ae86-4ca9bdf81198",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816959089",
      "review_id": 5816959089,
      "findings": {
        "correctness": 1,
        "design": 0,
        "prose": 0
      },
      "numbers": [
        "system-design.1"
      ],
      "rules": {},
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:51:20Z",
      "finished": "2026-11-08T06:51:48Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    },
    {
      "label": "stale-prose",
      "agent": "pr-stale-prose-reviewer",
      "model": "opus",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "50506a05f936",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 211,
      "tokens": {
        "input": 16,
        "cache_write": 34707,
        "cache_read": 207170,
        "output": 7522
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 16,
          "cache_write": 34707,
          "cache_read": 207170,
          "output": 7522
        }
      },
      "session": "01a1b9ed-5a46-7906-8afe-f24e09768f57",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816969534",
      "review_id": 5816969534,
      "findings": {
        "correctness": 0,
        "design": 0,
        "prose": 4
      },
      "numbers": [
        "stale-prose.1",
        "stale-prose.2",
        "stale-prose.3",
        "stale-prose.4"
      ],
      "rules": {},
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:52:38Z",
      "finished": "2026-11-08T06:56:23Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "queue"
    },
    {
      "label": "failure-paths",
      "agent": "pr-failure-paths-reviewer",
      "model": "sonnet",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "56bdd640764b",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 172,
      "tokens": {
        "input": 16,
        "cache_write": 24179,
        "cache_read": 158844,
        "output": 2872
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-sonnet-5-5": {
          "input": 16,
          "cache_write": 24179,
          "cache_read": 158844,
          "output": 2872
        }
      },
      "session": "01a1b9ed-534a-73ae-a9d1-0efbe23e113c",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816967692",
      "review_id": 5816967692,
      "findings": {
        "correctness": 1,
        "design": 0,
        "prose": 0
      },
      "numbers": [
        "failure-paths.1"
      ],
      "rules": {},
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:52:38Z",
      "finished": "2026-11-08T06:55:35Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "queue"
    },
    {
      "label": "contracts",
      "agent": "pr-contracts-reviewer",
      "model": "opus",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "88b65626bceb",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 240,
      "tokens": {
        "input": 38,
        "cache_write": 39636,
        "cache_read": 579402,
        "output": 8034
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 38,
          "cache_write": 39636,
          "cache_read": 579402,
          "output": 8034
        }
      },
      "session": "01a1b9ed-24a1-70f8-937d-72dd4e8f18e3",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816968641",
      "review_id": 5816968641,
      "findings": {
        "correctness": 1,
        "design": 1,
        "prose": 0
      },
      "numbers": [
        "contracts.1",
        "contracts.2"
      ],
      "rules": {},
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:51:55Z",
      "finished": "2026-11-08T06:56:00Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "queue"
    },
    {
      "label": "synthesis",
      "agent": null,
      "model": "sonnet",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/synthesis.md",
      "prompt_sha": "40dcb5a50df0",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 6,
      "tokens": {
        "input": 2,
        "cache_write": 9461,
        "cache_read": 1433,
        "output": 619
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-sonnet-5-5": {
          "input": 2,
          "cache_write": 9461,
          "cache_read": 1433,
          "output": 619
        }
      },
      "session": "01a1b9f2-981a-7a58-ae0e-b102a491bd48",
      "worklog": "2407.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/projects/pull/20#pullrequestreview-5816972406",
      "review_id": 5816972406,
      "findings": {
        "correctness": 0,
        "design": 3,
        "prose": 0
      },
      "numbers": [
        "synthesis.1",
        "synthesis.2",
        "synthesis.3"
      ],
      "rules": {},
      "head": "67e8fa13a26f748959a489e2ef35ee84c662a4fd",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-projects-pr20-round1-20261108T065057Z/records",
      "started": "2026-11-08T06:57:33Z",
      "finished": "2026-11-08T06:57:42Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    }
  ],
  "rules": {
    "proposed": 1,
    "keys": [
      "rule/data/a-migration-that-drops-a-table-refuses-while-the-table-holds-rows-nothing-else-carries"
    ],
    "unfiled": []
  },
  "kept": []
}

…dits 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

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

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

…ext 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

…t 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

datapointchris commented Nov 8, 2026

View reviewed changes

datapoints-fleet-automation Bot reviewed Nov 8, 2026

View reviewed changes

datapointchris commented Nov 8, 2026

View reviewed changes

datapointchris added 5 commits November 8, 2026 02:06

datapointchris merged commit 29e5ee3 into main Nov 8, 2026
6 checks passed

datapointchris deleted the projects-sync-removal branch November 8, 2026 07:15

Navigation Menu

Repository navigation

feat!: remove the sync engine and the database's synced copy - #20

feat!: remove the sync engine and the database's synced copy#20
datapointchris merged 6 commits into
mainfrom
projects-sync-removal

datapointchris commented Nov 8, 2026 •

edited

Loading

Uh oh!

datapointchris left a comment

Uh oh!

datapointchris left a comment

Uh oh!

datapointchris left a comment

Uh oh!

datapoints-fleet-automation Bot left a comment

Uh oh!

datapoints-fleet-automation Bot left a comment

Uh oh!

datapoints-fleet-automation Bot left a comment

Uh oh!

datapoints-fleet-automation Bot left a comment

Uh oh!

datapoints-fleet-automation Bot left a comment

Uh oh!

datapointchris left a comment

Uh oh!

datapointchris commented Nov 8, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

datapointchris commented Nov 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What to look at

How it was verified

What changes

Decisions, and what they rejected

Risk and rollback

The review

Uh oh!

datapointchris left a comment

Choose a reason for hiding this comment

PR review — system-design

Correctness

Uh oh!

datapointchris left a comment

Choose a reason for hiding this comment

PR review — code-architecture

Design and architecture

Each place that builds a service decides whether it numbers items, and only one answer is left

The re-read interval is written in three places: one constant and two sentences

Two lists decide which retired variables stop a run, each with its own loop and its own check in main.go

Uh oh!

datapointchris left a comment

Choose a reason for hiding this comment

PR review — G1

Correctness

Design and architecture

Prose

Uh oh!

datapoints-fleet-automation Bot left a comment

Choose a reason for hiding this comment

PR review — failure-paths

Correctness

Uh oh!

datapoints-fleet-automation Bot left a comment

Choose a reason for hiding this comment

PR review — contracts

Correctness

Design and architecture

Uh oh!

datapoints-fleet-automation Bot left a comment

Choose a reason for hiding this comment

PR review — useless-test

Design and architecture

TestRereadOnlyRunsInNormalMode checks the gate's predicate but not that the tick obeys it

Uh oh!

datapoints-fleet-automation Bot left a comment

Choose a reason for hiding this comment

PR review — stale-prose

Prose

Uh oh!

datapoints-fleet-automation Bot left a comment

Choose a reason for hiding this comment

PR review — fleet-patterns

Uh oh!

datapointchris left a comment

Choose a reason for hiding this comment

PR review — synthesis

Design and architecture

One defect, raised more than once

Uh oh!

datapointchris commented Nov 8, 2026

PR review round 1

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

datapointchris commented Nov 8, 2026 •

edited

Loading

Two lists decide which retired variables stop a run, each with its own loop and its own check in `main.go`