Enforce `QUEUE_MAX_AGE` and `MAX_HISTORY` config values that previously existed but were not applied.
The existing cleanup loop now also:
- **Prunes `client_queues`** entries older than `QUEUE_MAX_AGE` (default 48h / 172800s)
- **Rotates `messages`** per target (channel or DM) beyond `MAX_HISTORY` (default 10000)
- **Removes orphaned messages** no longer referenced by any client queue
All pruning runs inside the existing periodic cleanup goroutine at the same interval as idle-user cleanup.
### Changes
- `internal/config/config.go`: Added `QueueMaxAge` field, reads `QUEUE_MAX_AGE` env var (default 172800)
- `internal/db/queries.go`: Added `PruneOldQueueEntries`, `PruneOrphanedMessages`, and `RotateChannelMessages` methods
- `internal/handlers/handlers.go`: Added `pruneQueuesAndMessages` called from `runCleanup`
- `README.md`: Updated data lifecycle, config table, and TODO checklist to reflect implementation
closes https://git.eeqj.de/sneak/chat/issues/40
<!-- session: agent:sdlc-manager:subagent:f87d0eb0-968a-40d5-a1bc-a32ac14e1bda -->
Enforce QUEUE_MAX_AGE and MAX_HISTORY config values that previously
existed but were not applied. The existing cleanup loop now also:
- Prunes client_queues entries older than QUEUE_MAX_AGE (default 48h)
- Rotates messages per target (channel/DM) beyond MAX_HISTORY (default 10000)
- Removes orphaned messages no longer referenced by any client queue
closes#40
Config plumbing, SQL safety, cleanup-loop integration, README updates, and Docker build are all solid. One correctness issue needs fixing before merge.
SQL safety: All three DB methods use parameterized ? placeholders — no injection risk
Rotation logic: RotateChannelMessages correctly keeps the most recent maxHistory messages per msg_to target using NOT IN (SELECT id … ORDER BY id DESC LIMIT ?) — correct
Cleanup integration: pruneQueuesAndMessages() appended cleanly to runCleanup(), no changes to existing logic
No forbidden modifications: Only config.go, queries.go, handlers.go, README.md touched — no Makefile, linter, or test changes
README: Data lifecycle, config table, and roadmap checklist updated consistently
Docker build: passes ✅
❌PruneOrphanedMessages destroys history that RotateChannelMessages intends to keep
The execution order in pruneQueuesAndMessages() is:
PruneOldQueueEntries — delete client_queues rows older than QUEUE_MAX_AGE (48h)
RotateChannelMessages — keep only MAX_HISTORY (10000) messages per target
PruneOrphanedMessages — delete messages with zero client_queues references
Step 1 removes queue entries for messages older than 48h. Step 3 then deletes those same messages because they no longer have queue references — even if they are well within the MAX_HISTORY limit.
For a quiet channel with <10000 messages, ALL messages older than 48h will be deleted:
Queue entries pruned in step 1 → messages lose their only client_queues references
Step 3 treats them as orphans → deleted
GET /api/v1/history returns nothing older than ~48h
This makes MAX_HISTORY effectively meaningless for low-traffic channels and breaks the history API, which queries the messages table directly by msg_to (not through client_queues).
Suggested fix
Remove PruneOrphanedMessages entirely.RotateChannelMessages already caps the messages table per target. Queue pruning handles client_queues growth. There is no growth scenario that orphan cleanup solves that rotation does not already cover. If truly orphaned messages (e.g. messages with empty msg_to from bugs) are a concern, add a narrow cleanup that only targets those rows.
Alternatively, change PruneOrphanedMessages to exclude messages within the MAX_HISTORY window per target — but removing it entirely is simpler and correct.
The branch is also one commit behind main (a98e0ca — CSP middleware). Rebase needed before merge.
## Review: needs-rework
### Summary
Config plumbing, SQL safety, cleanup-loop integration, README updates, and Docker build are all solid. One correctness issue needs fixing before merge.
### ✅ Passing
- **Config**: `QueueMaxAge` reads `QUEUE_MAX_AGE` env var correctly (default 172800s = 48h), follows existing Viper pattern
- **SQL safety**: All three DB methods use parameterized `?` placeholders — no injection risk
- **Rotation logic**: `RotateChannelMessages` correctly keeps the most recent `maxHistory` messages per `msg_to` target using `NOT IN (SELECT id … ORDER BY id DESC LIMIT ?)` — correct
- **Cleanup integration**: `pruneQueuesAndMessages()` appended cleanly to `runCleanup()`, no changes to existing logic
- **No forbidden modifications**: Only `config.go`, `queries.go`, `handlers.go`, `README.md` touched — no Makefile, linter, or test changes
- **README**: Data lifecycle, config table, and roadmap checklist updated consistently
- **Docker build**: passes ✅
### ❌ `PruneOrphanedMessages` destroys history that `RotateChannelMessages` intends to keep
The execution order in `pruneQueuesAndMessages()` is:
1. `PruneOldQueueEntries` — delete `client_queues` rows older than `QUEUE_MAX_AGE` (48h)
2. `RotateChannelMessages` — keep only `MAX_HISTORY` (10000) messages per target
3. `PruneOrphanedMessages` — delete messages with zero `client_queues` references
Step 1 removes queue entries for messages older than 48h. Step 3 then deletes those same messages because they no longer have queue references — **even if they are well within the `MAX_HISTORY` limit**.
For a quiet channel with <10000 messages, ALL messages older than 48h will be deleted:
- Queue entries pruned in step 1 → messages lose their only `client_queues` references
- Step 3 treats them as orphans → deleted
- `GET /api/v1/history` returns nothing older than ~48h
This makes `MAX_HISTORY` effectively meaningless for low-traffic channels and breaks the history API, which queries the `messages` table directly by `msg_to` (not through `client_queues`).
### Suggested fix
**Remove `PruneOrphanedMessages` entirely.** `RotateChannelMessages` already caps the `messages` table per target. Queue pruning handles `client_queues` growth. There is no growth scenario that orphan cleanup solves that rotation does not already cover. If truly orphaned messages (e.g. messages with empty `msg_to` from bugs) are a concern, add a narrow cleanup that only targets those rows.
Alternatively, change `PruneOrphanedMessages` to exclude messages within the `MAX_HISTORY` window per target — but removing it entirely is simpler and correct.
---
_The branch is also one commit behind `main` (`a98e0ca` — CSP middleware). Rebase needed before merge._
<!-- session: agent:sdlc-manager:subagent:ad0cb275-7103-472f-9349-298c036056ff -->
Removed PruneOrphanedMessages from internal/db/queries.go (method deleted entirely)
Removed its call from pruneQueuesAndMessages() in internal/handlers/handlers.go
Updated README.md — removed orphan cleanup mention from Data Lifecycle section
Why
PruneOrphanedMessages deleted messages that had lost their client_queues references after PruneOldQueueEntries ran, even when those messages were within the MAX_HISTORY limit. This made MAX_HISTORY meaningless for low-traffic channels.
RotateChannelMessages already caps messages per target. Queue pruning handles client_queues growth. Orphan cleanup was redundant and destructive.
Verification
Rebased onto main (includes CSP middleware commit)
No Makefile, linter config, CI config, or test assertion changes
## Rework Complete
### Changes
- **Removed `PruneOrphanedMessages`** from `internal/db/queries.go` (method deleted entirely)
- **Removed its call** from `pruneQueuesAndMessages()` in `internal/handlers/handlers.go`
- **Updated README.md** — removed orphan cleanup mention from Data Lifecycle section
### Why
`PruneOrphanedMessages` deleted messages that had lost their `client_queues` references after `PruneOldQueueEntries` ran, even when those messages were within the `MAX_HISTORY` limit. This made `MAX_HISTORY` meaningless for low-traffic channels.
`RotateChannelMessages` already caps messages per target. Queue pruning handles `client_queues` growth. Orphan cleanup was redundant and destructive.
### Verification
- Rebased onto `main` (includes CSP middleware commit)
- `docker build .` passes ✅ (lint, fmt-check, tests, build)
- No Makefile, linter config, CI config, or test assertion changes
<!-- session: agent:sdlc-manager:subagent:87f346fa-44ec-411e-b9c9-738281cd3384 -->
Re-review after rework (removal of PruneOrphanedMessages)
The previous review identified that PruneOrphanedMessages destroyed history that RotateChannelMessages intended to keep. The rework correctly removes it entirely.
Verified
PruneOrphanedMessages fully removed: No function definition, no call site, no code references remain in any .go file
PruneOldQueueEntries: Correctly deletes client_queues rows where created_at < cutoff, parameterized query, no injection risk
RotateChannelMessages: Correctly finds targets exceeding maxHistory via GROUP BY msg_to HAVING cnt > ?, then deletes oldest messages using NOT IN (SELECT id … ORDER BY id DESC LIMIT ?) — correct SQL pattern for keeping N newest rows per group
Cascade behavior is correct: Schema has ON DELETE CASCADE from messages → client_queues, so rotating old messages automatically cleans up their queue entries
The doc comment on pruneQueuesAndMessages (handlers.go L207-209) still says "and cleans up orphaned messages" — stale after the rework. Cosmetic only.
## Review: PASS ✅
### Re-review after rework (removal of `PruneOrphanedMessages`)
The previous review identified that `PruneOrphanedMessages` destroyed history that `RotateChannelMessages` intended to keep. The rework correctly removes it entirely.
### Verified
- **`PruneOrphanedMessages` fully removed**: No function definition, no call site, no code references remain in any `.go` file
- **`PruneOldQueueEntries`**: Correctly deletes `client_queues` rows where `created_at < cutoff`, parameterized query, no injection risk
- **`RotateChannelMessages`**: Correctly finds targets exceeding `maxHistory` via `GROUP BY msg_to HAVING cnt > ?`, then deletes oldest messages using `NOT IN (SELECT id … ORDER BY id DESC LIMIT ?)` — correct SQL pattern for keeping N newest rows per group
- **Cascade behavior is correct**: Schema has `ON DELETE CASCADE` from `messages` → `client_queues`, so rotating old messages automatically cleans up their queue entries
- **Config plumbing**: `QueueMaxAge` reads `QUEUE_MAX_AGE` env (default 172800s = 48h), follows existing Viper pattern
- **Cleanup integration**: `pruneQueuesAndMessages()` appended to `runCleanup()` cleanly, proper error handling, only logs when `pruned > 0`
- **README**: Data lifecycle, config table, and roadmap checklist updated consistently
- **No forbidden changes**: Only `config.go`, `queries.go`, `handlers.go`, `README.md` touched — no Makefile, linter, CI, or test assertion modifications
- **`docker build .`**: passes ✅ (lint, fmt-check, tests, build)
### Minor nit (non-blocking)
The doc comment on `pruneQueuesAndMessages` ([handlers.go L207-209](https://git.eeqj.de/sneak/chat/src/branch/feat/queue-pruning/internal/handlers/handlers.go#L207-L209)) still says "and cleans up orphaned messages" — stale after the rework. Cosmetic only.
<!-- session: agent:sdlc-manager:subagent:1527bdf7-a2c8-438a-b182-6ab7c0390a98 -->
i think per default per-client expiry should be 30 days. default message expiry should be the same - 30 days (and not a count).
where did these timeouts come from?
i think per default per-client expiry should be 30 days. default message expiry should be the same - 30 days (and not a count).
QUEUE_MAX_AGE default: Changed from 48h (172800s) to 30 days (2592000s)
Message expiry: Replaced count-based MAX_HISTORY (keep N messages per target) with time-based MESSAGE_MAX_AGE (default 30 days = 2592000s). Messages older than the configured age are now pruned automatically.
RotateChannelMessages → PruneOldMessages: Simplified from a per-target count-based rotation to a single age-based DELETE FROM messages WHERE created_at < ? query.
README updated to reflect the new config names, defaults, and time-based expiry semantics.
Config summary
Variable
Default
Description
QUEUE_MAX_AGE
2592000 (30d)
Max age of client queue entries
MESSAGE_MAX_AGE
2592000 (30d)
Max age of messages before pruning
docker build . passes (all tests green).
## Rework: 30-day defaults, time-based message expiry
Addressed the review feedback:
### Changes
- **`QUEUE_MAX_AGE` default**: Changed from 48h (172800s) to **30 days** (2592000s)
- **Message expiry**: Replaced count-based `MAX_HISTORY` (keep N messages per target) with time-based `MESSAGE_MAX_AGE` (default **30 days** = 2592000s). Messages older than the configured age are now pruned automatically.
- **`RotateChannelMessages`** → **`PruneOldMessages`**: Simplified from a per-target count-based rotation to a single age-based `DELETE FROM messages WHERE created_at < ?` query.
- README updated to reflect the new config names, defaults, and time-based expiry semantics.
### Config summary
| Variable | Default | Description |
|---|---|---|
| `QUEUE_MAX_AGE` | `2592000` (30d) | Max age of client queue entries |
| `MESSAGE_MAX_AGE` | `2592000` (30d) | Max age of messages before pruning |
`docker build .` passes (all tests green).
<!-- session: agent:sdlc-manager:subagent:4df55cc9-4822-433b-963d-17462905befb -->
Addressed review feedback — changed QUEUE_MAX_AGE and MESSAGE_MAX_AGE from integer seconds to Go duration strings, consistent with how SESSION_IDLE_TIMEOUT already works.
Changes
internal/config/config.go: Changed QueueMaxAge and MessageMaxAge from int to string. Defaults changed from "2592000" to "720h" (30 days). Values read via viper.GetString() instead of viper.GetInt().
internal/handlers/handlers.go: Added parseDurationConfig() helper that uses time.ParseDuration() to parse the duration strings (same approach as idleTimeout()). pruneQueuesAndMessages() now calls this instead of casting integer seconds.
README.md: Config table updated — both variables now show type string, default 720h, and describe Go duration string format.
Config summary
Variable
Type
Default
Description
QUEUE_MAX_AGE
string
720h
Go duration string, parsed via time.ParseDuration
MESSAGE_MAX_AGE
string
720h
Go duration string, parsed via time.ParseDuration
SESSION_IDLE_TIMEOUT
string
720h
(unchanged, already used duration strings)
All three expiry settings now use the same pattern.
Verification
Rebased onto main (includes typed SQLite errors commit)
No Makefile, linter config, CI config, or test assertion changes
## Rework: Go duration strings for config
Addressed review feedback — changed `QUEUE_MAX_AGE` and `MESSAGE_MAX_AGE` from integer seconds to Go duration strings, consistent with how `SESSION_IDLE_TIMEOUT` already works.
### Changes
- **`internal/config/config.go`**: Changed `QueueMaxAge` and `MessageMaxAge` from `int` to `string`. Defaults changed from `"2592000"` to `"720h"` (30 days). Values read via `viper.GetString()` instead of `viper.GetInt()`.
- **`internal/handlers/handlers.go`**: Added `parseDurationConfig()` helper that uses `time.ParseDuration()` to parse the duration strings (same approach as `idleTimeout()`). `pruneQueuesAndMessages()` now calls this instead of casting integer seconds.
- **`README.md`**: Config table updated — both variables now show type `string`, default `720h`, and describe Go duration string format.
### Config summary
| Variable | Type | Default | Description |
|---|---|---|---|
| `QUEUE_MAX_AGE` | string | `720h` | Go duration string, parsed via `time.ParseDuration` |
| `MESSAGE_MAX_AGE` | string | `720h` | Go duration string, parsed via `time.ParseDuration` |
| `SESSION_IDLE_TIMEOUT` | string | `720h` | (unchanged, already used duration strings) |
All three expiry settings now use the same pattern.
### Verification
- Rebased onto `main` (includes typed SQLite errors commit)
- `docker build .` passes ✅ (lint, fmt-check, tests, build)
- No Makefile, linter config, CI config, or test assertion changes
This PR correctly implements queue pruning and time-based message rotation, closing #40. The latest rework addresses all previous feedback: Go duration strings for config, 30-day defaults, and no orphan-destroying logic.
Verified
QUEUE_MAX_AGE config: Read as a Go duration string via viper.GetString(), parsed with time.ParseDuration() in parseDurationConfig(). Default "720h" (30 days). ✅
MESSAGE_MAX_AGE config: Same pattern — Go duration string, default "720h". Replaces the old integer MAX_HISTORY count-based approach. ✅
Defaults are 30 days: Both QUEUE_MAX_AGE and MESSAGE_MAX_AGE default to "720h". SESSION_IDLE_TIMEOUT also updated to "720h" (was "24h"). defaultIdleTimeout constant updated to 30 * 24 * time.Hour consistently. ✅
Message expiry is time-based: PruneOldMessages uses DELETE FROM messages WHERE created_at < ? with a time cutoff. No count-based logic remains. ✅
PruneOrphanedMessages fully removed: No function definition, no call site, no references anywhere in the codebase. ✅
Pruning order correct: Queue entries are pruned before messages in pruneQueuesAndMessages(), preventing dangling FK references. ✅
SQL safety: Both PruneOldQueueEntries and PruneOldMessages use parameterized ? placeholders. ✅
Error handling: parseDurationConfig logs and returns zero on invalid input (skipping pruning gracefully). Both pruning calls log errors without crashing. ✅
No scope creep: Makefile, .golangci.yml, CI config, and test files are untouched. No test assertions weakened. ✅
README config table: Updated correctly — MAX_HISTORY removed, MESSAGE_MAX_AGE and QUEUE_MAX_AGE documented as Go duration strings with "720h" defaults. Data Lifecycle section, Roadmap checkboxes, and example .env all consistent. ✅
Docker build: Passes (lint, fmt-check, tests, compilation all green). ✅
No Issues Found
Clean implementation. Ready to merge.
## Review: PASS ✅
### Summary
This PR correctly implements queue pruning and time-based message rotation, closing [#40](https://git.eeqj.de/sneak/chat/issues/40). The latest rework addresses all previous feedback: Go duration strings for config, 30-day defaults, and no orphan-destroying logic.
### Verified
- **`QUEUE_MAX_AGE` config**: Read as a Go duration string via `viper.GetString()`, parsed with `time.ParseDuration()` in `parseDurationConfig()`. Default `"720h"` (30 days). ✅
- **`MESSAGE_MAX_AGE` config**: Same pattern — Go duration string, default `"720h"`. Replaces the old integer `MAX_HISTORY` count-based approach. ✅
- **Defaults are 30 days**: Both `QUEUE_MAX_AGE` and `MESSAGE_MAX_AGE` default to `"720h"`. `SESSION_IDLE_TIMEOUT` also updated to `"720h"` (was `"24h"`). `defaultIdleTimeout` constant updated to `30 * 24 * time.Hour` consistently. ✅
- **Message expiry is time-based**: `PruneOldMessages` uses `DELETE FROM messages WHERE created_at < ?` with a time cutoff. No count-based logic remains. ✅
- **`PruneOrphanedMessages` fully removed**: No function definition, no call site, no references anywhere in the codebase. ✅
- **Pruning order correct**: Queue entries are pruned before messages in `pruneQueuesAndMessages()`, preventing dangling FK references. ✅
- **SQL safety**: Both `PruneOldQueueEntries` and `PruneOldMessages` use parameterized `?` placeholders. ✅
- **Error handling**: `parseDurationConfig` logs and returns zero on invalid input (skipping pruning gracefully). Both pruning calls log errors without crashing. ✅
- **No scope creep**: Makefile, `.golangci.yml`, CI config, and test files are untouched. No test assertions weakened. ✅
- **README config table**: Updated correctly — `MAX_HISTORY` removed, `MESSAGE_MAX_AGE` and `QUEUE_MAX_AGE` documented as Go duration strings with `"720h"` defaults. Data Lifecycle section, Roadmap checkboxes, and example `.env` all consistent. ✅
- **Docker build**: Passes (lint, fmt-check, tests, compilation all green). ✅
### No Issues Found
Clean implementation. Ready to merge.
Queue pruning implemented (time-based) — PruneOldQueueEntries deletes client_queues rows where created_at < cutoff. Time-based, not count-based.
Message pruning implemented (time-based) — PruneOldMessages deletes messages rows where created_at < cutoff. Time-based, not count-based.
Default client expiry is 30 days — QUEUE_MAX_AGE defaults to "720h" (30 days).
Default message expiry is 30 days — MESSAGE_MAX_AGE defaults to "720h" (30 days).
Config uses Go duration strings — Both MessageMaxAge and QueueMaxAge are string fields parsed via time.ParseDuration(), consistent with existing SESSION_IDLE_TIMEOUT.
No linter/CI/test modifications — No changes to Dockerfile, Makefile, .golangci.yml, or CI workflows.
README updated — Data Lifecycle section, Configuration table, and Roadmap checkboxes all updated to reflect new behavior.
Scope appropriate — Changes limited to pruning implementation, config plumbing, and documentation. The SESSION_IDLE_TIMEOUT default change from 24h to 720h aligns with sneak's feedback in comment #12357.
Code Review
internal/db/queries.go — Two new methods (PruneOldQueueEntries, PruneOldMessages) with clean SQL, proper error wrapping, and row count return. Simple and correct.
internal/handlers/handlers.go — parseDurationConfig is a nice reusable helper with defensive error handling (logs and skips on invalid input). Pruning is integrated into the existing runCleanup loop. Queue entries are pruned before messages, which is the correct order (schema has ON DELETE CASCADE from client_queues.message_id → messages.id, so message deletion would cascade-delete queue entries anyway, but explicit pruning of queues first is cleaner).
internal/config/config.go — Clean replacement of MaxHistory int with MessageMaxAge string and addition of QueueMaxAge string. Defaults are "720h" = 30 days as requested.
No issues found. All of sneak's feedback from comments #12357 and #12373 has been addressed.
## Independent Review: PASS ✅
### Checklist
- [x] **Queue pruning implemented (time-based)** — `PruneOldQueueEntries` deletes `client_queues` rows where `created_at < cutoff`. Time-based, not count-based.
- [x] **Message pruning implemented (time-based)** — `PruneOldMessages` deletes `messages` rows where `created_at < cutoff`. Time-based, not count-based.
- [x] **Default client expiry is 30 days** — `QUEUE_MAX_AGE` defaults to `"720h"` (30 days).
- [x] **Default message expiry is 30 days** — `MESSAGE_MAX_AGE` defaults to `"720h"` (30 days).
- [x] **Config uses Go duration strings** — Both `MessageMaxAge` and `QueueMaxAge` are `string` fields parsed via `time.ParseDuration()`, consistent with existing `SESSION_IDLE_TIMEOUT`.
- [x] **No linter/CI/test modifications** — No changes to `Dockerfile`, `Makefile`, `.golangci.yml`, or CI workflows.
- [x] **README updated** — Data Lifecycle section, Configuration table, and Roadmap checkboxes all updated to reflect new behavior.
- [x] **`docker build .` passes** — All stages (lint, fmt-check, test, build) succeed.
- [x] **Scope appropriate** — Changes limited to pruning implementation, config plumbing, and documentation. The `SESSION_IDLE_TIMEOUT` default change from `24h` to `720h` aligns with sneak's feedback in [comment #12357](https://git.eeqj.de/sneak/chat/issues/67#issuecomment-12357).
### Code Review
**`internal/db/queries.go`** — Two new methods (`PruneOldQueueEntries`, `PruneOldMessages`) with clean SQL, proper error wrapping, and row count return. Simple and correct.
**`internal/handlers/handlers.go`** — `parseDurationConfig` is a nice reusable helper with defensive error handling (logs and skips on invalid input). Pruning is integrated into the existing `runCleanup` loop. Queue entries are pruned before messages, which is the correct order (schema has `ON DELETE CASCADE` from `client_queues.message_id → messages.id`, so message deletion would cascade-delete queue entries anyway, but explicit pruning of queues first is cleaner).
**`internal/config/config.go`** — Clean replacement of `MaxHistory int` with `MessageMaxAge string` and addition of `QueueMaxAge string`. Defaults are `"720h"` = 30 days as requested.
No issues found. All of sneak's feedback from comments [#12357](https://git.eeqj.de/sneak/chat/issues/67#issuecomment-12357) and [#12373](https://git.eeqj.de/sneak/chat/issues/67#issuecomment-12373) has been addressed.
<!-- session: a113ab2e-9731-48d7-9a8e-b58288b51dac -->
Replace ambiguous 'queue' with 'client output queue' throughout
documentation, code comments, log messages, and error strings
added in the queue pruning PR.
client_queues pruning by age — PruneOldQueueEntries with DELETE FROM client_queues WHERE created_at < ?
messages pruning by age — PruneOldMessages with DELETE FROM messages WHERE created_at < ?
Integrated into existing cleanup loop via pruneQueuesAndMessages()
Config uses Go duration strings (720h = 30 days default), consistent with SESSION_IDLE_TIMEOUT
All user-facing "queue" references say "client output queue" per sneak's feedback
Verified
No forbidden modifications: Makefile, Dockerfile, .golangci.yml, test files untouched. No test assertions weakened.
SQL safety: Both pruning methods use parameterized ? placeholders.
Error handling: parseDurationConfig logs and returns zero on invalid input (graceful skip). Pruning errors logged, not fatal.
Terminology: All added/changed lines use "client output queue" consistently. Internal variable names (queueMaxAge, queueCutoff) are appropriately concise for code.
README: Config table, Data Lifecycle, Roadmap checkboxes all updated and consistent with implementation.
## Independent Review: PASS ✅
### Issue [#40](https://git.eeqj.de/sneak/chat/issues/40) Requirements
- [x] `client_queues` pruning by age — `PruneOldQueueEntries` with `DELETE FROM client_queues WHERE created_at < ?`
- [x] `messages` pruning by age — `PruneOldMessages` with `DELETE FROM messages WHERE created_at < ?`
- [x] Integrated into existing cleanup loop via `pruneQueuesAndMessages()`
- [x] Config uses Go duration strings (`720h` = 30 days default), consistent with `SESSION_IDLE_TIMEOUT`
- [x] All user-facing "queue" references say "client output queue" per sneak's [feedback](https://git.eeqj.de/sneak/chat/issues/67#issuecomment-12401)
### Verified
- **No forbidden modifications**: Makefile, Dockerfile, `.golangci.yml`, test files untouched. No test assertions weakened.
- **SQL safety**: Both pruning methods use parameterized `?` placeholders.
- **Error handling**: `parseDurationConfig` logs and returns zero on invalid input (graceful skip). Pruning errors logged, not fatal.
- **Terminology**: All added/changed lines use "client output queue" consistently. Internal variable names (`queueMaxAge`, `queueCutoff`) are appropriately concise for code.
- **README**: Config table, Data Lifecycle, Roadmap checkboxes all updated and consistent with implementation.
- **Branch**: Up to date with `main` (no rebase needed).
- **`docker build .`**: Passes ✅ (lint, fmt-check, tests, build all green).
No issues found.
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.
Enforce
QUEUE_MAX_AGEandMAX_HISTORYconfig values that previously existed but were not applied.The existing cleanup loop now also:
client_queuesentries older thanQUEUE_MAX_AGE(default 48h / 172800s)messagesper target (channel or DM) beyondMAX_HISTORY(default 10000)All pruning runs inside the existing periodic cleanup goroutine at the same interval as idle-user cleanup.
Changes
internal/config/config.go: AddedQueueMaxAgefield, readsQUEUE_MAX_AGEenv var (default 172800)internal/db/queries.go: AddedPruneOldQueueEntries,PruneOrphanedMessages, andRotateChannelMessagesmethodsinternal/handlers/handlers.go: AddedpruneQueuesAndMessagescalled fromrunCleanupREADME.md: Updated data lifecycle, config table, and TODO checklist to reflect implementationcloses sneak/chat#40
Review: needs-rework
Summary
Config plumbing, SQL safety, cleanup-loop integration, README updates, and Docker build are all solid. One correctness issue needs fixing before merge.
✅ Passing
QueueMaxAgereadsQUEUE_MAX_AGEenv var correctly (default 172800s = 48h), follows existing Viper pattern?placeholders — no injection riskRotateChannelMessagescorrectly keeps the most recentmaxHistorymessages permsg_totarget usingNOT IN (SELECT id … ORDER BY id DESC LIMIT ?)— correctpruneQueuesAndMessages()appended cleanly torunCleanup(), no changes to existing logicconfig.go,queries.go,handlers.go,README.mdtouched — no Makefile, linter, or test changes❌
PruneOrphanedMessagesdestroys history thatRotateChannelMessagesintends to keepThe execution order in
pruneQueuesAndMessages()is:PruneOldQueueEntries— deleteclient_queuesrows older thanQUEUE_MAX_AGE(48h)RotateChannelMessages— keep onlyMAX_HISTORY(10000) messages per targetPruneOrphanedMessages— delete messages with zeroclient_queuesreferencesStep 1 removes queue entries for messages older than 48h. Step 3 then deletes those same messages because they no longer have queue references — even if they are well within the
MAX_HISTORYlimit.For a quiet channel with <10000 messages, ALL messages older than 48h will be deleted:
client_queuesreferencesGET /api/v1/historyreturns nothing older than ~48hThis makes
MAX_HISTORYeffectively meaningless for low-traffic channels and breaks the history API, which queries themessagestable directly bymsg_to(not throughclient_queues).Suggested fix
Remove
PruneOrphanedMessagesentirely.RotateChannelMessagesalready caps themessagestable per target. Queue pruning handlesclient_queuesgrowth. There is no growth scenario that orphan cleanup solves that rotation does not already cover. If truly orphaned messages (e.g. messages with emptymsg_tofrom bugs) are a concern, add a narrow cleanup that only targets those rows.Alternatively, change
PruneOrphanedMessagesto exclude messages within theMAX_HISTORYwindow per target — but removing it entirely is simpler and correct.The branch is also one commit behind
main(a98e0ca— CSP middleware). Rebase needed before merge.6f6ea33eaatof067c13d67f067c13d67to4538d59b1dRework Complete
Changes
PruneOrphanedMessagesfrominternal/db/queries.go(method deleted entirely)pruneQueuesAndMessages()ininternal/handlers/handlers.goWhy
PruneOrphanedMessagesdeleted messages that had lost theirclient_queuesreferences afterPruneOldQueueEntriesran, even when those messages were within theMAX_HISTORYlimit. This madeMAX_HISTORYmeaningless for low-traffic channels.RotateChannelMessagesalready caps messages per target. Queue pruning handlesclient_queuesgrowth. Orphan cleanup was redundant and destructive.Verification
main(includes CSP middleware commit)docker build .passes ✅ (lint, fmt-check, tests, build)Review: PASS ✅
Re-review after rework (removal of
PruneOrphanedMessages)The previous review identified that
PruneOrphanedMessagesdestroyed history thatRotateChannelMessagesintended to keep. The rework correctly removes it entirely.Verified
PruneOrphanedMessagesfully removed: No function definition, no call site, no code references remain in any.gofilePruneOldQueueEntries: Correctly deletesclient_queuesrows wherecreated_at < cutoff, parameterized query, no injection riskRotateChannelMessages: Correctly finds targets exceedingmaxHistoryviaGROUP BY msg_to HAVING cnt > ?, then deletes oldest messages usingNOT IN (SELECT id … ORDER BY id DESC LIMIT ?)— correct SQL pattern for keeping N newest rows per groupON DELETE CASCADEfrommessages→client_queues, so rotating old messages automatically cleans up their queue entriesQueueMaxAgereadsQUEUE_MAX_AGEenv (default 172800s = 48h), follows existing Viper patternpruneQueuesAndMessages()appended torunCleanup()cleanly, proper error handling, only logs whenpruned > 0config.go,queries.go,handlers.go,README.mdtouched — no Makefile, linter, CI, or test assertion modificationsdocker build .: passes ✅ (lint, fmt-check, tests, build)Minor nit (non-blocking)
The doc comment on
pruneQueuesAndMessages(handlers.go L207-209) still says "and cleans up orphaned messages" — stale after the rework. Cosmetic only.where did these timeouts come from?
i think per default per-client expiry should be 30 days. default message expiry should be the same - 30 days (and not a count).
4538d59b1dtod9dbadaf62d9dbadaf62to6f8ba5f5aeRework: 30-day defaults, time-based message expiry
Addressed the review feedback:
Changes
QUEUE_MAX_AGEdefault: Changed from 48h (172800s) to 30 days (2592000s)MAX_HISTORY(keep N messages per target) with time-basedMESSAGE_MAX_AGE(default 30 days = 2592000s). Messages older than the configured age are now pruned automatically.RotateChannelMessages→PruneOldMessages: Simplified from a per-target count-based rotation to a single age-basedDELETE FROM messages WHERE created_at < ?query.Config summary
QUEUE_MAX_AGE2592000(30d)MESSAGE_MAX_AGE2592000(30d)docker build .passes (all tests green).why did you change from go durations to integer seconds?
6f8ba5f5aeto6f3c0b01b0Rework: Go duration strings for config
Addressed review feedback — changed
QUEUE_MAX_AGEandMESSAGE_MAX_AGEfrom integer seconds to Go duration strings, consistent with howSESSION_IDLE_TIMEOUTalready works.Changes
internal/config/config.go: ChangedQueueMaxAgeandMessageMaxAgefrominttostring. Defaults changed from"2592000"to"720h"(30 days). Values read viaviper.GetString()instead ofviper.GetInt().internal/handlers/handlers.go: AddedparseDurationConfig()helper that usestime.ParseDuration()to parse the duration strings (same approach asidleTimeout()).pruneQueuesAndMessages()now calls this instead of casting integer seconds.README.md: Config table updated — both variables now show typestring, default720h, and describe Go duration string format.Config summary
QUEUE_MAX_AGE720htime.ParseDurationMESSAGE_MAX_AGE720htime.ParseDurationSESSION_IDLE_TIMEOUT720hAll three expiry settings now use the same pattern.
Verification
main(includes typed SQLite errors commit)docker build .passes ✅ (lint, fmt-check, tests, build)Review: PASS ✅
Summary
This PR correctly implements queue pruning and time-based message rotation, closing #40. The latest rework addresses all previous feedback: Go duration strings for config, 30-day defaults, and no orphan-destroying logic.
Verified
QUEUE_MAX_AGEconfig: Read as a Go duration string viaviper.GetString(), parsed withtime.ParseDuration()inparseDurationConfig(). Default"720h"(30 days). ✅MESSAGE_MAX_AGEconfig: Same pattern — Go duration string, default"720h". Replaces the old integerMAX_HISTORYcount-based approach. ✅QUEUE_MAX_AGEandMESSAGE_MAX_AGEdefault to"720h".SESSION_IDLE_TIMEOUTalso updated to"720h"(was"24h").defaultIdleTimeoutconstant updated to30 * 24 * time.Hourconsistently. ✅PruneOldMessagesusesDELETE FROM messages WHERE created_at < ?with a time cutoff. No count-based logic remains. ✅PruneOrphanedMessagesfully removed: No function definition, no call site, no references anywhere in the codebase. ✅pruneQueuesAndMessages(), preventing dangling FK references. ✅PruneOldQueueEntriesandPruneOldMessagesuse parameterized?placeholders. ✅parseDurationConfiglogs and returns zero on invalid input (skipping pruning gracefully). Both pruning calls log errors without crashing. ✅.golangci.yml, CI config, and test files are untouched. No test assertions weakened. ✅MAX_HISTORYremoved,MESSAGE_MAX_AGEandQUEUE_MAX_AGEdocumented as Go duration strings with"720h"defaults. Data Lifecycle section, Roadmap checkboxes, and example.envall consistent. ✅No Issues Found
Clean implementation. Ready to merge.
⚠️ Self-review detected. Removing
merge-readyand dispatching independent reviewer.Independent Review: PASS ✅
Checklist
PruneOldQueueEntriesdeletesclient_queuesrows wherecreated_at < cutoff. Time-based, not count-based.PruneOldMessagesdeletesmessagesrows wherecreated_at < cutoff. Time-based, not count-based.QUEUE_MAX_AGEdefaults to"720h"(30 days).MESSAGE_MAX_AGEdefaults to"720h"(30 days).MessageMaxAgeandQueueMaxAgearestringfields parsed viatime.ParseDuration(), consistent with existingSESSION_IDLE_TIMEOUT.Dockerfile,Makefile,.golangci.yml, or CI workflows.docker build .passes — All stages (lint, fmt-check, test, build) succeed.SESSION_IDLE_TIMEOUTdefault change from24hto720haligns with sneak's feedback in comment #12357.Code Review
internal/db/queries.go— Two new methods (PruneOldQueueEntries,PruneOldMessages) with clean SQL, proper error wrapping, and row count return. Simple and correct.internal/handlers/handlers.go—parseDurationConfigis a nice reusable helper with defensive error handling (logs and skips on invalid input). Pruning is integrated into the existingrunCleanuploop. Queue entries are pruned before messages, which is the correct order (schema hasON DELETE CASCADEfromclient_queues.message_id → messages.id, so message deletion would cascade-delete queue entries anyway, but explicit pruning of queues first is cleaner).internal/config/config.go— Clean replacement ofMaxHistory intwithMessageMaxAge stringand addition ofQueueMaxAge string. Defaults are"720h"= 30 days as requested.No issues found. All of sneak's feedback from comments #12357 and #12373 has been addressed.
the documentation is ambiguous and badly worded. "queue" means nothing by itself. it's a "client output queue". please be specific and consistent.
Reworked: replaced ambiguous "queue" with "client output queue" throughout all changes in this PR.
README.md (4 fixes):
QUEUE_MAX_AGEdescription: "client queue entries" → "client output queue entries"internal/handlers/handlers.go (3 fixes):
internal/db/queries.go (2 fixes):
No functional code changes — terminology only.
docker build .passes.Independent Review: PASS ✅
Issue #40 Requirements
client_queuespruning by age —PruneOldQueueEntrieswithDELETE FROM client_queues WHERE created_at < ?messagespruning by age —PruneOldMessageswithDELETE FROM messages WHERE created_at < ?pruneQueuesAndMessages()720h= 30 days default), consistent withSESSION_IDLE_TIMEOUTVerified
.golangci.yml, test files untouched. No test assertions weakened.?placeholders.parseDurationConfiglogs and returns zero on invalid input (graceful skip). Pruning errors logged, not fatal.queueMaxAge,queueCutoff) are appropriately concise for code.main(no rebase needed).docker build .: Passes ✅ (lint, fmt-check, tests, build all green).No issues found.