Flowlord Updates. proper syncing of sqlite db, better alerting de-escalation

[jbsmith7741]

No description provided.

[qodo-code-review]

Review Summary by Qodo

Graceful HTTP shutdown, tick-based alerts, and SQLite backup sync

✨ Enhancement 🐞 Bug fix

Walkthroughs

Description

• Implement graceful HTTP server shutdown with context-based lifecycle management
• Refactor notification system with tick-based Slack summaries and exponential backoff de-escalation
• Add SQLite database backup synchronization on refresh and daily recycle operations
• Improve backload execution UX with partial-failure handling and detailed error reporting
• Rename slack field to notify throughout codebase for semantic clarity

Diagram

flowchart LR
  A["HTTP Server"] -->|graceful shutdown| B["Context Timeout"]
  C["Notification Tick"] -->|escalate/de-escalate| D["Alert Frequency"]
  E["SQLite Cache"] -->|sync on refresh| F["Backup Path"]
  G["Backload Execute"] -->|partial failure| H["JSON Status Response"]

Loading

File Changes

1. apps/flowlord/handler.go Enhancement, error handling +38/-7

HTTP server lifecycle and notification field refactoring

apps/flowlord/handler.go

---

2. apps/flowlord/handler_test.go 🧪 Tests +2/-2

Update test to use renamed notify field

apps/flowlord/handler_test.go

---

3. apps/flowlord/sqlite/sqlite.go 🐞 Bug fix +7/-8

Unconditional database sync on close operation

apps/flowlord/sqlite/sqlite.go

---

View more (7)

4. apps/flowlord/taskmaster.go ✨ Enhancement +103/-69

Tick-based notification system with backoff logic

apps/flowlord/taskmaster.go

---

5. apps/flowlord/taskmaster_test.go 🧪 Tests +102/-1

Comprehensive unit tests for notification tick behavior

apps/flowlord/taskmaster_test.go

---

6. apps/flowlord/handler/static/backload.js ✨ Enhancement +74/-34

Partial-failure UX and execution results display

apps/flowlord/handler/static/backload.js

---

7. apps/flowlord/handler/static/style.css ✨ Enhancement +6/-0

Execution success status styling

apps/flowlord/handler/static/style.css

---

8. apps/flowlord/handler/backload.tmpl ✨ Enhancement +1/-1

Dynamic preview results heading

apps/flowlord/handler/backload.tmpl

---

9. apps/go.mod Dependencies +1/-2

Update task-tools dependency to v0.32.1

apps/go.mod

---

10. apps/go.sum Dependencies +2/-4

Update checksums and remove unused jsonparser dependency

apps/go.sum

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0)   📘 Rule violations (1)   📎 Requirement gaps (0)   🎨 UX Issues (0)

📘\ ☼ Reliability (1)

1. err.Error() without nil check ☑ 📘 ≡

Description

tm.taskCache.Recycle(...) can return a non-nil err, but the code unconditionally calls
err.Error(), which will panic if err is nil. This violates the requirement to guard
fallible-operation results before dereference/use.

Code

apps/flowlord/taskmaster.go[R269-270]

+				s, err := tm.taskCache.Recycle(time.Now().Add(-tm.taskCache.Retention))
+				log.Printf(" cache recycle:%s %s", s, err.Error())

Evidence

Compliance requires checking/handling errors from fallible operations before using them. The new
code logs err.Error() immediately after Recycle(...) without verifying err != nil, which can
trigger a nil dereference panic.

apps/flowlord/taskmaster.go[269-270]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`err.Error()` is called even when `err` may be nil after `tm.taskCache.Recycle(...)`, which can panic.
## Issue Context
This occurs in the 24-hour DB ticker path inside `Run`, where `Recycle` returns `(string, error)`.
## Fix Focus Areas
- apps/flowlord/taskmaster.go[269-270]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. WAL backup can be stale ☑ 🐞 ☼

Description

SQLite is configured to use WAL journaling, but Sync() copies only the main DB file via io.Copy and
does not checkpoint WAL or copy the -wal/-shm files; the PR adds Sync() calls while the DB is still
open. This can produce backups that miss recent writes (or are inconsistent) even though Sync
reports success.

Code

apps/flowlord/handler.go[R298-301]

+	if err := tm.taskCache.Sync(); err != nil {
+		http.Error(w, "sqlite backup: "+err.Error(), http.StatusInternalServerError)
+		return
+	}

Evidence

The DB is explicitly put into WAL mode, while Sync() ultimately copies just LocalPath to
BackupPath using a single-file stream copy. The PR introduces runtime calls to Sync() (refresh
endpoint and daily tick) while the DB remains open and being written, but there is no WAL
checkpointing and no copying of WAL/SHM companion files, so the backup can lag behind or be
incomplete.

/apps/flowlord/sqlite/sqlite.go[91-114]
/apps/flowlord/sqlite/sqlite.go[128-145]
/apps/flowlord/sqlite/sqlite.go[263-270]
/apps/flowlord/handler.go[285-314]
/apps/flowlord/taskmaster.go[259-273]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SQLite.Sync()` creates a backup by copying only the main DB file, but the DB runs in WAL mode and `Sync()` is now called while the DB is open. Without a WAL checkpoint (or copying `-wal`/`-shm`), backups can be stale/incomplete.
### Issue Context
- WAL is enabled during DB init.
- `Sync()` uses `copyFiles(LocalPath, BackupPath, ...)` which is a single-file `io.Copy`.
- New runtime `Sync()` calls happen in `/refresh` and the daily DB tick.
### Fix Focus Areas
- apps/flowlord/sqlite/sqlite.go[91-114]
- apps/flowlord/sqlite/sqlite.go[128-145]
- apps/flowlord/sqlite/sqlite.go[263-270]
- apps/flowlord/handler.go[285-314]
- apps/flowlord/taskmaster.go[259-273]
### Suggested change (pick one approach)
1) **Checkpoint + copy** (minimal behavior change):
 - In `(*SQLite).Sync()`, take `o.mu` (or otherwise serialize) and execute a WAL checkpoint (e.g., `PRAGMA wal_checkpoint(TRUNCATE);`) before copying.
 - Optionally copy the companion `LocalPath + "-wal"` and `LocalPath + "-shm"` as well if you intend to preserve WAL state.
2) **Snapshot file then upload** (most robust):
 - Create a consistent snapshot to a temporary local file (e.g., via SQLite backup API or `VACUUM INTO` to a temp file), then copy that single snapshot file to `BackupPath`.
Also ensure `Sync()` is concurrency-safe with other DB operations (lock/serialization) so backups aren’t taken mid-write.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Encode() error ignored 📘 ☼

Description

The new error response path discards the return value of json.NewEncoder(w).Encode(...), so
response-write failures are silently ignored. This violates the requirement to check/handle errors
from fallible operations.

Code

apps/flowlord/handler.go[R875-882]

+			_ = json.NewEncoder(w).Encode(map[string]interface{}{
+				"Status": fmt.Sprintf(
+					"%d/%d messages sent: %s",
+					sent,
+					attempted,
+					strings.TrimSpace(errs.Error()),
+				),
+			})

Evidence

The compliance rule requires handling returned errors from fallible operations. The code explicitly
assigns the Encode(...) error to _, so an I/O/write failure would not be surfaced or logged.

apps/flowlord/handler.go[875-882]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`json.NewEncoder(w).Encode(...)` errors are ignored, which can hide response-write failures.
## Issue Context
This is in the `Backloader` execute error path where a JSON status body is returned.
## Fix Focus Areas
- apps/flowlord/handler.go[875-882]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more