Repository navigation
Phase 0 and Phase 1: self-host skeleton, Lists and Items - #18
Merged
Merged
Conversation
Flat config on ESLint 9: typescript-eslint, eslint-plugin-react on the automatic JSX runtime, and react-hooks. eslint-config-prettier goes last so formatting stays in .prettierrc and the two never disagree. eslint-plugin-react-hooks v7 ships its config in ESLint 10's plugin-array form, so the plugin is registered by hand and only its rules are spread. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers every exported function, including the branches that are easy to get wrong: map over an err is a no-op, andThen short-circuits without calling the next step, and fromPromise hands the thrown value to the caller's mapper untouched. Red against a stub: 16 failing, 0 passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ok/err/isOk/isErr/map/andThen/unwrapOr/fromPromise. E is unconstrained so a boundary can carry a raw error before it has settled on its variants; andThen widens to E | F so a chained step can add a failure the first could not produce. fromPromise is the wrapper for code that throws, and takes the mapper from the caller because only the caller knows what a failure means. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`can()` and the REST schemas, as types and signatures only, so the tests that specify them typecheck. The implementing agent fills them in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Written from issue #2 alone, before any implementation exists, so the tests define the contract rather than describe it. All 102 fail with "not implemented". Covers ADR-0005 (Owner holds every Permission, Editor every Item Permission), ADR-0004 (an Ownerless List keeps working, minus the Owner-only Permissions), and the rule that an Account with no Membership cannot see a List at all. Every schema is strict, so no schema admits a `position`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner and Editor are spelled out as separate Permission sets rather than Editor being derived from Owner, so adding a Permission forces a decision about Editor instead of silently widening it. An Ownerless List has no holder of the Owner-only Permissions and no fallback (ADR-0004). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every schema is strict, which is what keeps a `position` out of the wire format. A unit may not arrive without a quantity to measure, and an update must change something. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Better Auth runs on the application's single Bun.sql pool through kysely-postgres-js (ADR-0007). postgres.js stays out of the tree despite the dialect's peer dependency, verified in the Phase 0 spike. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tests come first and against a real Postgres, never a mock (CONVENTIONS.md). docker-compose.test.yml provides the server; each test takes its own database so a migration runner under test is free to create and drop whatever it likes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each file runs in its own transaction and is recorded in _migrations with a checksum, so a half-applied file leaves no trace and an applied file that changed on disk stops the boot. A session advisory lock serialises two instances booting at once (ADR-0007, ADR-0008). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A visitor is a real server-side Account from their first request (ADR-0003): GET / creates one and returns the session cookie. Better Auth shares the application's pool through kysely-postgres-js and its schema is CLI-generated as migration 0002 (ADR-0007). PUBLIC_URL is the only origin source (ADR-0008) — it sets the cookie's Secure flag and the WebSocket URL, and nothing reads the request Host header. The signing secret comes from the environment, or is generated on first boot and persisted so sessions survive a restart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copy .env.example to .env and `docker compose up` gives a working app (ADR-0008): a pinned Postgres major on a named volume, a healthcheck gating the app's start, and migrations completing before the server binds. The image pins oven/bun:1.4 — pre-1.4 bun:sql could return one query's rows to another (oven-sh/bun#32772), which an auth adapter hits routinely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The UI clears a note by emptying the text input and saving, so an empty or whitespace-only note is accepted and normalised, not rejected: to absent on create, to cleared on update. `null` stays valid on update as the explicit alternative. The assertions are on the parsed output, because the property that matters is that exactly one representation of "no note" reaches the database — two would push the ambiguity into the display and sort code. Not extended to `unit`: doing so would contradict two existing tests. Raised separately rather than settled here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rule that an empty note means no note now covers `unit`, and on create `null` joins `""` as a spelling of nothing. Four cases asserted the opposite and are changed here, on their own, before any test that depends on the new behaviour is written. Deleted: - createItemSchema > rejects an empty or whitespace-only unit — an emptied unit input is now accepted and normalised, not a 400. - the two empty-unit assertions in updateItemSchema > rejects an empty unit or one over the maximum length, which is now "rejects a unit over the maximum length". The over-length half still rejects. Relaxed: - createItemSchema > rejects a unit that is not a string — `unit: null` moved from reject to accept; `true` covers the not-a-string case in its place. - createItemSchema > rejects a note that is not a string, or is over the maximum length — `note: null` likewise. `quantity` is untouched: it has no empty-string spelling to collapse, and `null` still clears it on update and still rejects on create. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the note rule over `unit`, and adds `null` as a create-side spelling of
nothing for both fields, so one payload shape serves create and update.
The coupling rule makes `unit` sharper than `note`: an empty unit must be
normalised away BEFORE "a unit needs a quantity" is checked. `{name, unit: ""}`
— rename an Item and clear its unit in one save — asserts that ordering, and is
not satisfied by dropping the `.min(1)` alone; with the normalisation after the
refine it still fails on the coupling error. `{quantity, unit: ""}` isolates the
normalisation, since that one passes the coupling check either way, so the two
together say which half is wrong.
`{quantity: null, unit: ""}` clears both. `{quantity: null, unit: "kg"}` still
rejects: clearing a quantity while naming a real unit is incoherent, emptying
both fields is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No `position` column on items: Items have no inherent order and the server never orders them, so there is nothing for a column to hold. The database refuses a unit with no quantity, the same rule the Zod schemas apply at the edge, and an Ownerless List is a valid state (ADR-0004). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each endpoint has a happy path and a denied permission. Denial has two shapes: an Account with no Membership is told 404 because it cannot see the List at all, a Member lacking the Permission is told 403. The domain functions are signature stubs, so the red is behavioural — 404 where a 201 belongs — rather than a missing import. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three gaps, each red for a behavioural reason: an unreadable .sql file throws EISDIR straight out of a Promise<Result<…>>; a lock release that fails replaces the returned migration_failed with a driver error; and an applied migration whose file is gone is accepted silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three layers: handlers parse and map, domain functions decide, query functions hold the SQL. Every action passes `authoriseList`, the one caller of `can()`, which reports a List the Account has no Membership on as missing rather than forbidden — a 403 would confirm it exists. Clearing an Item's quantity clears its unit with it: a unit measures nothing on its own, which is what the schemas and the check constraint already say. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading a .sql file sat outside the try that guarded the directory listing, so a file that is unreadable or that vanishes between the two threw past a Promise<Result<…>> and killed the boot with a raw stack instead of the refusal a self-hoster can act on (CONVENTIONS.md, "Errors are values"). The error now also names the file it could not read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A throw inside the finally replaced the returned migration_failed, so the operator saw a driver error instead of the file that failed, and the reserved connection was never returned to the pool. The unlock is now best effort and the release is unconditional. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The drift check iterated the files on disk, so a _migrations row with no file — a deleted or renamed migration, or an older image against a newer database — passed silently while the comment claimed the check caught any disagreement between the repo and the database. It now walks the applied rows, and the comment describes what it does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Instant actions — tick, delete, clear checked, uncheck all — fire on the click and reconcile with the server's answer. Edited fields (name, quantity, unit, note, and the List's name) sit behind Save and Cancel: the draft lives in the editing component, so Cancel sends nothing at all and nothing is ever debounced (ADR-0002). Sorting is client-side, un-Checked first and alphabetical within each group; `byCheckedThenName` joins the pure core with its own unit tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The UI clears a note or a unit by emptying its input and saving, which is what a text input naturally sends. Absent, "", whitespace and null now collapse to one representation — absent on create, null on update — so display, sorting and SQL only ever meet one shape of "no note". The pipeline order is the contract: trim, collapse emptiness, then the coupling check, then the lengths. Checking the coupling first would make "rename this Item and clear its unit" impossible, since an emptied unit would still look like a unit with nothing to measure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The route table becomes a module so the whole server can be mounted in a test, and the bootstrap moves to the client so every path — deep links included — gets a session (ADR-0003). The bootstrap is the old server-side logic ported as it was, so the tests are red on exactly the bug the review found: a 503 from the session check signs in and mints a second Anonymous Account instead of failing, a 200 that is not a session does the same, and two bootstraps racing on one page mint two Accounts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rule is now uniform: the server orders nothing, Items and Lists alike, so there is one story about where ordering lives when realtime refetching starts returning rows in whatever order the database felt like. `byName` joins the pure core and keeps the visible order identical to the `order by lower(name)` it replaces. The integration test no longer asserts the order the endpoint came back in, only which Lists it returned — it was asserting something we no longer promise. It now creates two Lists and compares them order-insensitively, which is a stronger test than the single-List one that could not have noticed either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A failed session check is not an answer: a 5xx, a network error, or a 200 that is not a session now surface as session_check_failed and leave the cookie alone. An Anonymous Account lives in one cookie on one device, so minting a new one over a blip strands that visitor's Lists with no recovery path (ADR-0003). One bootstrap is shared while it is in flight, so StrictMode's double effect and an impatient retry cannot race the cookie into two Accounts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every path now serves the bundle directly and creates nothing. The old handler ran on the exact path `/` only, so a bookmarked or refreshed `/lists/abc` opened the app with no Account and 401ed on every call. It also fetched its own page back over loopback unwrapped, serving a 500 when that rejected, and left `/__shell` publicly routable. SessionGate resolves the session before the app renders, shows a brief starting state, and on failure offers a retry rather than a fresh start. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Visibility was decided by the index query's `where` clause, so `can()` was never consulted — a capability decision living in SQL. Today every Membership implies `list:read` and the two agree; the day a Role appears that does not, the endpoint would have gone on returning Lists nobody may read, and Phase 3 would have had to rewrite the one place the ticket was written to keep whole. The seam: the query scopes the candidates and returns each List with its Memberships, the domain filters them with `can(actor, "list:read", list)`. `listsVisibleTo` is pure, so its tests hand it candidates the query should never produce — a List the Account has no Membership on — and a gate that passed the query's answer through fails them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"The creator of a List starts as its Owner" is a rule about who holds what, so it belongs with the other decisions rather than as a string literal in SQL. The query function now takes a `Role`, which a typo could not be, and `createList` has a decision of its own instead of delegating outright. The transaction stays in the query function, where it is correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One limit per refinement. Checking both together could only report one path, so an over-long unit was blamed on the note and the client's error message pointed at the wrong input. Also records why the quantity/unit coupling is judged against the payload rather than the resulting Item: keeping it payload-local keeps the check in the pure core, and it is strictly stricter than the database constraint, so nothing it rejects could have been stored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Better Auth throws, and an unwrapped throw became a bare 500 that never reached `statusFor` — the one place a failure is supposed to become a status code. It now enters as a value and answers 503 `session_unavailable`, which is deliberately not `unauthenticated`: the session could not be checked at all, and telling a visitor they are signed out is a lie that costs them their Anonymous Account if they act on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The server stopped ordering Lists and nothing took over, so the picker showed whatever order Postgres returned and would have churned as rows updated. `byName` at the render, exactly as `byCheckedThenName` already orders the Items. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A tick that 403s or a delete that 404s left the screen asserting something the server had rejected, behind an error banner, until the user reloaded. The instant actions now refetch the List when their request fails, so the server's answer replaces the guess — not an inverse computed locally, which would be a second guess about what the row says. If the List itself has gone, the index becomes the truth and the selection clears. ADR-0002 accepts last-write-wins; it does not accept a UI showing state the server refused. The ADR's "failed requests retry while the tab is open" is NOT implemented here and is not implemented anywhere yet — see the follow-up note in the report; blind retries of a refused request would be worse than none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"2kg" in the quantity box parsed to null, which silently cleared the quantity and — through the unit's coupling to it — the unit as well, and told the person nothing. Both editors now refuse the save, keep the draft on screen and say which value was not a number, so the typo is still there to correct and nothing is sent. The two character-identical normalisations are now one pure function in `src/lib/item-draft.ts`, unit-tested, rather than a helper exported from a row component and imported by its parent. Save and Cancel are unchanged: Cancel still sends nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Capture the decisions taken during the work that the plan did not predict, because they change what a later phase will find: the session is bootstrapped by the client rather than a page handler, the server orders Lists as well as Items nowhere, and an empty note or unit collapses to a single representation of "no value". Also record the Phase 1 process-experiment verdict, which Phase 2's entry requires before Phase 2 starts: the split is worth keeping, with the weakness to carry forward named. Closes #1 Closes #2 GitHub will not auto-close from an unmerged branch; both issues are closed by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Delivers issues #1 (Phase 0) and #2 (Phase 1), both now closed. 41 commits;
mainfast-forwards cleanly.What this delivers
Phase 0 —
docker compose upon a fresh clone gives a working install:app+ pinnedpostgres:17-alpinewith a healthcheck gating startup, a transactional migration runner tracked in_migrationsthat completes before the server binds, Better Auth with the Anonymous plugin, and a signing secret that persists across restarts.Phase 1 — a usable single-user app:
lists/items/memberships, all eleven REST endpoints, and the React UI with instant actions firing immediately and edited fields behind explicit save/cancel.Decisions taken during the work
Recorded in
docs/PLAN.md; these differ from or extend what the tickets described./*serves the bundle and does no session work; the app signs in anonymously on boot when it finds no session. A server-side shell handler only covered exact/, so deep links and refreshes got no Account. A failed or errored session check never triggers sign-in — only a definitive "no session" does, because minting a new Anonymous Account over a network blip strands that visitor's Lists with no recovery (ADR-0003).noteandunit: on create, absent /""/ whitespace /nullall collapse to the key being absent; on update, to key-present-null. Exactly one representation of "no value" reaches the database.namestill rejects empty.The spike
Better Auth runs on one shared
Bun.sqlpool viakysely-postgres-js'sPostgresJSDialect. All four checks passed —postgres.jsnever resolves despite the peer dependency, the CLI schema applies through the pool, and 600 concurrent interleaved queries returned their own rows on Bun 1.4.2 (oven-sh/bun#32772). Thepgfallback was not needed.Verification
bun test— 226 pass / 0 fail across 13 files, against real Postgres. No mocks; there is no mocking framework in the repo.bunx tsc --noEmitandbunx eslint .— clean.docker compose upon a fresh volume, not only in tests.One expected stack trace appears in the suite output: a test deliberately kills the connection pool to prove the
session_unavailable→ 503 path.Review
Both phases were reviewed by an agent along Standards and Spec axes, and every must-fix was addressed. The substantive one:
GET /api/listsdecided visibility in SQL without ever callingcan()— a capability decision rather than a Role read, which is why grep-based checks missed it, and the one thing Phase 3 would have had to rewrite. Fixed ine6c1067.Deferred findings are filed as #8–#17 (five
ready-for-agent, fiveready-for-human).Caveat worth stating plainly: every review so far has been by an agent. No human has read this diff.
🤖 Generated with Claude Code