[vavallee]

Summary

PR 1 of Phase 2 for #2832: the storage for a durable Calibre delivery ledger. No behaviour change. Imports and pushes are untouched, and the repo is not wired into cmd/bindery yet (the next PR does that, since an unused repo would only be dead construction).

Refs #2832

Migration 095 calibre_deliveries. One row per ebook book_files row (UNIQUE(book_file_id)), ON DELETE CASCADE from both books and book_files, with state (pending|delivered|failed|skipped), outcome, attempts, last error and code, the Calibre id in the push target, target library, and next_attempt_at. Table as proposed, with two notes:

• format is the file extension, lower case, no dot (epub). book_files.format is only ebook|audiobook, so it cannot be copied from there.
• Every time column is written by Go in one fixed width shape, 2006-01-02T15:04:05.000000000Z. timeValueArg (RFC3339Nano) trims trailing zeros, so …01.1Z sorts after …01.123Z as text, and CURRENT_TIMESTAMP sorts before any T shape on the same day. Fixed width keeps next_attempt_at <= ? a plain text comparison that the (state, next_attempt_at) index serves. TestCalibreDeliveryDueBatch fails if the layout is switched to RFC3339Nano.

Backfill (SQL, in the migration, like 028/084). A book with calibre_id > 0 that is not Calibre origin gets one delivered row, outcome='backfilled', calibre_id copied, times set to now. The file is the book_files ebook row matching books.ebook_file_path, else the lowest id ebook row (the same order bookColumns / bookCTE use). No ebook row, no ledger row. ON CONFLICT(book_file_id) DO NOTHING makes a re-run a no-op.

Calibre origin is excluded three ways:

1. metadata_provider is calibre (trimmed, any case), and
2. foreign_id starts with calibre:book:. These two are what isCalibreOrigin (internal/calibre/syncer.go:579-584) already uses to refuse overwriting a source calibre_id.
3. A calibre_provenance row with entity_type='book' for the book. The library import writes it for every book it matches (internal/calibre/importer.go:381), including an existing Bindery book adopted by title (path 3, importer.go:905-925), which keeps its own foreign_id and, if it already had one, its provider. Its calibre_id is a source library id, not proof of a push.

Leaving a pushed book out costs one duplicate check on a later push. Recording an unpushed book as delivered would hide it from delivery for good, so doubt excludes. Remaining ambiguity: a book adopted by title before migration 044 (no provenance) whose provider was already set is indistinguishable from a pushed one.

CalibreDeliveryRepo (internal/db/calibre_deliveries.go, model in internal/models/calibre_delivery.go): Enqueue (conflict is a no-op in every state; only Retry re-arms failed/skipped), Get, GetByBookFile, DueBatch, MarkDelivered, MarkFailed (increments attempts; terminal moves to failed, otherwise stays pending with backoff), MarkSkipped, Retry, ClearPending, ResetAll (deletes, so a target change sends nothing until re-enqueued), Summary, List, DeliveredByCalibreID. Every method is one statement. The Mark methods return ErrCalibreDeliveryNotFound when the row vanished under them (cleared, or cascaded with its file).

Migration number. Main's highest was 094. gh pr list --state open with a migrations path filter showed only 086 (#2633) and 090 (#2713). git log --remotes=origin --diff-filter=A for 09[5-9]_* and 1* over every remote branch found nothing. The branch was pushed with only the migration file to claim 095 before the rest was written.

How it was verified

• go build ./... && go vet ./..., GOOS=windows go build ./..., golangci-lint run ./internal/db/... ./internal/models/... (0 issues), and go test ./cmd/... ./internal/... exit 0 (exit code read directly, not piped).
• TestMigrate095BackfillsPushedBooks drops the table and the 095 marker to put the database back at 094, seeds books, runs migrate, then asserts: two pushed books get one delivered row each on the right file (ebook path wins over lower id; lowest ebook id beats a lower id audiobook row), formats mobi/azw3/epub/empty for an extensionless file under a dotted directory; nothing for Calibre origin by foreign_id, by provider (" Calibre "), or by provenance; an author provenance row does not exclude; nothing for audiobook only or never pushed. It then changes a row, clears the marker, re-runs, and checks nothing changed.
• Repo tests cover every method, the conflict rule in all four states, due ordering at sub-second precision, retry filters, summary, paging, cascade on book_files and books delete, and the FK on insert.
• Fail before: with the migration file removed, every new test fails with no such table: calibre_deliveries. That is the expected shape for a new table rather than evidence about a bug.
• Mutations (each tree passes go vet):
  • drop the provenance exclusion: TestMigrate095BackfillsPushedBooks fails, calibre provenance book 6 must not be backfilled
  • drop the foreign_id exclusion: fails, calibre foreign_id book 4 must not be backfilled
  • drop the provider exclusion: fails, calibre provider book 5 must not be backfilled
  • Enqueue re-arming failed/skipped rows on conflict: TestCalibreDeliveryEnqueueLeavesExistingRows/failed and /skipped fail
  • time layout switched to RFC3339Nano: TestCalibreDeliveryDueBatch fails, DueBatch at enqueue time = [1 3], want [2 3]

Checklist

• Every commit carries a Signed-off-by that matches its author
• Changelog fragment changelog.d/2832-calibre-deliveries-table.md
• Tests added
• No new dependency

🤖 Generated with Claude Code

https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9

[github-actions]

Clean storage-only PR. Migration logic, schema, and test coverage are all solid. One correctness issue and two minor observations.

---

MarkDelivered, MarkFailed, MarkSkipped have no state guard — internal/db/calibre_deliveries.go:128–165

All three UPDATE on WHERE id = ? only. A stale worker slot or double-trigger can overwrite a delivered row (resetting its calibre_id, clearing the error record, reverting state to pending), or call MarkFailed on an already-skipped row. The fix is one clause per method, e.g.:

-- MarkDelivered
WHERE id = ? AND state = 'pending'

-- MarkFailed
WHERE id = ? AND state IN ('pending', 'failed')   -- allows re-recording a transient on a pending row

-- MarkSkipped
WHERE id = ? AND state = 'pending'

With these guards the methods return ErrCalibreDeliveryNotFound when the state has already moved on, which is the right signal to the delivery loop. The impact is deferred until the next PR wires this in, but the fix is cheap now and keeps the ledger self-consistent by construction.

---

Minor: delivered_at parse error silently swallowed — calibre_deliveries.go:314

if t, err := parseFlexibleTime(delivered); err == nil {
    d.DeliveredAt = t
}

A malformed delivered_at produces nil with no log, while parseFlexibleTimeValue (used for the other timestamp columns) at least logs a warning. In practice the column is written exclusively by Go with a fixed layout so this will never trigger, but it's inconsistent.

---

Nit: WHERE 1 in the backfill INSERT — 095_calibre_deliveries.sql:55

The WHERE 1 before ON CONFLICT is a no-op tautology; it can be dropped.

— 🤖 Bindery triage bot (automated). Reply to correct me; a human will see it.

[codecov]

Codecov Report

❌ Patch coverage is 84.14634% with 26 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/db/calibre_deliveries.go	84.14%	13 Missing and 13 partials ⚠️

📢 Thoughts on this report? Let us know!