Also adds Database.Hydrate() as a generic method to inject the DB reference into any model.
Verified
All 8 tables created on fresh DB
0 lint issues
Builds clean
Implements the full database schema from the README spec and corresponding Go models with relation methods.
## Schema (002_tables.sql)
| Table | Purpose |
|-------|--------|
| `users` | Accounts with nick, bcrypt password hash, timestamps |
| `auth_tokens` | Per-device auth tokens with optional expiry |
| `channels` | Chat rooms with topic and IRC-style mode flags |
| `channel_members` | Membership join table with per-user modes (+o, +v) |
| `messages` | Channel/DM history with JSON meta for extensibility |
| `message_queue` | Per-user pending delivery queue (unread messages) |
| `sessions` | Server-held session state with idle timeout |
| `server_links` | Federation peer configuration |
All IDs are UUIDs (TEXT), not auto-increment. Appropriate indexes on foreign keys and common query patterns.
## Models (internal/models/)
All models embed `Base` which provides `GetDB()` for relation methods:
- `User.Channels(ctx)` → channels the user belongs to
- `User.QueuedMessages(ctx)` → undelivered messages
- `Channel.Members(ctx)` → members with nicks
- `Channel.RecentMessages(ctx, limit)` → message history
- `ChannelMember.User(ctx)` / `ChannelMember.Channel(ctx)` → resolve relations
- `AuthToken.User(ctx)` / `Session.User(ctx)` → token/session owner
Also adds `Database.Hydrate()` as a generic method to inject the DB reference into any model.
## Verified
- All 8 tables created on fresh DB
- 0 lint issues
- Builds clean
sneak
was assigned by clawbot2026-02-09 23:54:51 +01:00
AuthToken.User(), Session.User(), ChannelMember.User(), and ChannelMember.Channel() all have the same SELECT query copy-pasted. Should extract db.GetUserByID(ctx, id) and db.GetChannelByID(ctx, id) lookup methods, then call those from the relation methods.
2. Relation methods return nil on empty results, not empty slices
User.Channels(), Channel.Members(), etc. return nil when there are no rows (because var channels []*Channel starts nil). Callers might get surprised by nil vs [] when marshaling to JSON. Should initialize with make() or []*Channel{}.
3. No timestamps populated on Create methods
CreateUser, CreateChannel, etc. return model structs but don't populate CreatedAt/UpdatedAt — they rely on SQL defaults. The returned model has zero-value times. Should either SELECT back after insert or set time.Now() explicitly.
4. No transactions in the migration runner
Each migration's SQL + recording should be wrapped in a transaction. If a migration partially applies and the INSERT into schema_migrations fails, the database is left in an inconsistent state with no way to retry.
5. QueueMessage ignores LastInsertId error
entryID,_:=res.LastInsertId()
Should check the error.
6. No dequeue/ack for message queue
There's QueueMessage but no DequeueMessages or AckMessages to remove entries after delivery. Needed for the message polling flow.
7. time.Sleep in tests
TestUserQueuedMessages and TestChannelRecentMessages use time.Sleep(10ms) to ensure timestamp ordering. Fragile on slow CI. Could use explicit timestamps or sequential insert ordering instead.
8. Missing lookup methods needed for handlers
GetUserByNick() — needed for login
GetUserByToken() — needed for auth middleware
DeleteAuthToken() — needed for logout
UpdateUserLastSeen() — needed for presence tracking
These will be needed as soon as we build the auth handlers.
9. SQLite foreign keys not enabled
SQLite doesn't enforce foreign keys by default. Should run PRAGMA foreign_keys = ON on every new connection. Without this, the ON DELETE CASCADE clauses in the schema are decorative.
10. Overall
The architecture is solid — embedded DB pattern works well, test coverage is good, migration system is clean. Items 1, 3, 4, and 9 are the most impactful to fix before merge.
## Code Review
### 1. Duplicated "find by ID" queries
`AuthToken.User()`, `Session.User()`, `ChannelMember.User()`, and `ChannelMember.Channel()` all have the same SELECT query copy-pasted. Should extract `db.GetUserByID(ctx, id)` and `db.GetChannelByID(ctx, id)` lookup methods, then call those from the relation methods.
### 2. Relation methods return nil on empty results, not empty slices
`User.Channels()`, `Channel.Members()`, etc. return `nil` when there are no rows (because `var channels []*Channel` starts nil). Callers might get surprised by nil vs `[]` when marshaling to JSON. Should initialize with `make()` or `[]*Channel{}`.
### 3. No timestamps populated on Create methods
`CreateUser`, `CreateChannel`, etc. return model structs but don't populate `CreatedAt`/`UpdatedAt` — they rely on SQL defaults. The returned model has zero-value times. Should either SELECT back after insert or set `time.Now()` explicitly.
### 4. No transactions in the migration runner
Each migration's SQL + recording should be wrapped in a transaction. If a migration partially applies and the INSERT into `schema_migrations` fails, the database is left in an inconsistent state with no way to retry.
### 5. `QueueMessage` ignores `LastInsertId` error
```go
entryID, _ := res.LastInsertId()
```
Should check the error.
### 6. No dequeue/ack for message queue
There's `QueueMessage` but no `DequeueMessages` or `AckMessages` to remove entries after delivery. Needed for the message polling flow.
### 7. `time.Sleep` in tests
`TestUserQueuedMessages` and `TestChannelRecentMessages` use `time.Sleep(10ms)` to ensure timestamp ordering. Fragile on slow CI. Could use explicit timestamps or sequential insert ordering instead.
### 8. Missing lookup methods needed for handlers
- `GetUserByNick()` — needed for login
- `GetUserByToken()` — needed for auth middleware
- `DeleteAuthToken()` — needed for logout
- `UpdateUserLastSeen()` — needed for presence tracking
These will be needed as soon as we build the auth handlers.
### 9. SQLite foreign keys not enabled
SQLite doesn't enforce foreign keys by default. Should run `PRAGMA foreign_keys = ON` on every new connection. Without this, the `ON DELETE CASCADE` clauses in the schema are decorative.
### 10. Overall
The architecture is solid — embedded DB pattern works well, test coverage is good, migration system is clean. Items 1, 3, 4, and 9 are the most impactful to fix before merge.
I've created a follow-up PR with fixes for the code review feedback: sneak/chat#6
It addresses items 1-6 and 8-10 from the review.
I've created a follow-up PR with fixes for the code review feedback: https://git.eeqj.de/sneak/chat/pulls/6
It addresses items 1-6 and 8-10 from the review.
- Item 1: Extract GetUserByID/GetChannelByID lookup methods, use from relation methods
- Item 2: Initialize slices with literals so JSON gets [] not null
- Item 3: Populate CreatedAt/UpdatedAt with time.Now() on all Create methods
- Item 4: Wrap each migration's SQL + recording in a transaction
- Item 5: Check error from res.LastInsertId() in QueueMessage
- Item 6: Add DequeueMessages and AckMessages methods
- Item 8: Add GetUserByNick, GetUserByToken, DeleteAuthToken, UpdateUserLastSeen
- Item 9: Run PRAGMA foreign_keys = ON on every new connection
- Item 10: Builds clean, all tests pass
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Implements the full database schema from the README spec and corresponding Go models with relation methods.
Schema (002_tables.sql)
usersauth_tokenschannelschannel_membersmessagesmessage_queuesessionsserver_linksAll IDs are UUIDs (TEXT), not auto-increment. Appropriate indexes on foreign keys and common query patterns.
Models (internal/models/)
All models embed
Basewhich providesGetDB()for relation methods:User.Channels(ctx)→ channels the user belongs toUser.QueuedMessages(ctx)→ undelivered messagesChannel.Members(ctx)→ members with nicksChannel.RecentMessages(ctx, limit)→ message historyChannelMember.User(ctx)/ChannelMember.Channel(ctx)→ resolve relationsAuthToken.User(ctx)/Session.User(ctx)→ token/session ownerAlso adds
Database.Hydrate()as a generic method to inject the DB reference into any model.Verified
Code Review
1. Duplicated "find by ID" queries
AuthToken.User(),Session.User(),ChannelMember.User(), andChannelMember.Channel()all have the same SELECT query copy-pasted. Should extractdb.GetUserByID(ctx, id)anddb.GetChannelByID(ctx, id)lookup methods, then call those from the relation methods.2. Relation methods return nil on empty results, not empty slices
User.Channels(),Channel.Members(), etc. returnnilwhen there are no rows (becausevar channels []*Channelstarts nil). Callers might get surprised by nil vs[]when marshaling to JSON. Should initialize withmake()or[]*Channel{}.3. No timestamps populated on Create methods
CreateUser,CreateChannel, etc. return model structs but don't populateCreatedAt/UpdatedAt— they rely on SQL defaults. The returned model has zero-value times. Should either SELECT back after insert or settime.Now()explicitly.4. No transactions in the migration runner
Each migration's SQL + recording should be wrapped in a transaction. If a migration partially applies and the INSERT into
schema_migrationsfails, the database is left in an inconsistent state with no way to retry.5.
QueueMessageignoresLastInsertIderrorShould check the error.
6. No dequeue/ack for message queue
There's
QueueMessagebut noDequeueMessagesorAckMessagesto remove entries after delivery. Needed for the message polling flow.7.
time.Sleepin testsTestUserQueuedMessagesandTestChannelRecentMessagesusetime.Sleep(10ms)to ensure timestamp ordering. Fragile on slow CI. Could use explicit timestamps or sequential insert ordering instead.8. Missing lookup methods needed for handlers
GetUserByNick()— needed for loginGetUserByToken()— needed for auth middlewareDeleteAuthToken()— needed for logoutUpdateUserLastSeen()— needed for presence trackingThese will be needed as soon as we build the auth handlers.
9. SQLite foreign keys not enabled
SQLite doesn't enforce foreign keys by default. Should run
PRAGMA foreign_keys = ONon every new connection. Without this, theON DELETE CASCADEclauses in the schema are decorative.10. Overall
The architecture is solid — embedded DB pattern works well, test coverage is good, migration system is clean. Items 1, 3, 4, and 9 are the most impactful to fix before merge.
fix all of these except for number 7 on another branch based on this one and make a new PR.
I've created a follow-up PR with fixes for the code review feedback: sneak/chat#6
It addresses items 1-6 and 8-10 from the review.