feat(bookdrop): comic-aware Import Metadata — route .cbr/.cbz to comic providers (bookshelf-qgd90.1) #1226

Merged

zombor merged 4 commits from bd-bookshelf-qgd90.1 into main 2026-07-24 22:15:52 +00:00

Conversation 4 Commits 4 Files changed 14 +945 -110

zombor commented 2026-07-24 19:30:47 +00:00

Owner

Copy link

Summary

• Comic files (.cbz/.cbr/.cb7/.cbt) are now detected by extension and routed to ComicVine / Metron / AniList instead of ebook providers (google-books etc.)
• Filename parsed for Series + Issue# + Year to build the comic query (local re-implementation of books.parseComicFilenameParts to avoid import cycle)
• Comic-specific fields (IssueNumber, CoverDate, StoryArc, Characters, Teams, Locations, Writers, Pencilers, Inkers, Colorists, Letterers, CoverArtists, Editors, cover image URL) are stored on the candidate and surfaced in the Import Metadata modal
• Ebook files unchanged — existing behavior preserved

Technical changes

• internal/bookdrop/meta.go: ComicMetadataJSON + Comic *ComicMetadataJSON on FileMetadataJSON
• internal/bookdrop/import_metadata_service.go: isComicExtension, buildComicImportQuery, parseComicFilenameForQuery, filterProvidersForQuery, mergeComicFields, applyComicFields, comicProviderMergeChain — all at 100% coverage
• internal/bookdrop/review_handler.go: explicit field mapping after adding Comic broke the direct type-cast
• internal/bookdrop/wire.go: capability-based provider filter replaces hardcoded [:3] slice
• internal/app/providerspec.go: bookdropProviderSpecs now includes ComicVine, Metron, AniList
• internal/app/build_extended_deps.go: comic provider URLs + creds wired for bookdrop activity
• internal/wfengine/import_metadata_workflow.go: ImportMetadataComicCandidate for JSON bridge
• static/js/controllers/bookdrop_import_metadata_controller.js: detects c.comic non-null, renders comic fields in modal (no inline style= — CSP-safe)

Test plan

• make test passes (all packages, including bookdrop)
• make coverage green — zero uncovered statement blocks
• make lint — no errors in this worktree's files
• New tests: IsComicExtension, ParseComicFilenameForQuery, FilterProvidersForQuery, ImportMetadataForProposals (comic routing), CandidateFromMetadata cover URL path, MergeMetadataFields CoverURL, ParseComicFilenameForQuery zero-digit normalization, BuildImportQuery scan-ID-only comic fallback, ApplyImportMetadata with comic candidate

Closes bead bookshelf-qgd90.1 on merge.

## Summary - Comic files (.cbz/.cbr/.cb7/.cbt) are now detected by extension and routed to ComicVine / Metron / AniList instead of ebook providers (google-books etc.) - Filename parsed for Series + Issue# + Year to build the comic query (local re-implementation of books.parseComicFilenameParts to avoid import cycle) - Comic-specific fields (IssueNumber, CoverDate, StoryArc, Characters, Teams, Locations, Writers, Pencilers, Inkers, Colorists, Letterers, CoverArtists, Editors, cover image URL) are stored on the candidate and surfaced in the Import Metadata modal - Ebook files unchanged — existing behavior preserved ## Technical changes - `internal/bookdrop/meta.go`: `ComicMetadataJSON` + `Comic *ComicMetadataJSON` on `FileMetadataJSON` - `internal/bookdrop/import_metadata_service.go`: `isComicExtension`, `buildComicImportQuery`, `parseComicFilenameForQuery`, `filterProvidersForQuery`, `mergeComicFields`, `applyComicFields`, `comicProviderMergeChain` — all at 100% coverage - `internal/bookdrop/review_handler.go`: explicit field mapping after adding `Comic` broke the direct type-cast - `internal/bookdrop/wire.go`: capability-based provider filter replaces hardcoded `[:3]` slice - `internal/app/providerspec.go`: `bookdropProviderSpecs` now includes ComicVine, Metron, AniList - `internal/app/build_extended_deps.go`: comic provider URLs + creds wired for bookdrop activity - `internal/wfengine/import_metadata_workflow.go`: `ImportMetadataComicCandidate` for JSON bridge - `static/js/controllers/bookdrop_import_metadata_controller.js`: detects `c.comic` non-null, renders comic fields in modal (no inline `style=` — CSP-safe) ## Test plan - [x] `make test` passes (all packages, including bookdrop) - [x] `make coverage` green — zero uncovered statement blocks - [x] `make lint` — no errors in this worktree's files - [x] New tests: `IsComicExtension`, `ParseComicFilenameForQuery`, `FilterProvidersForQuery`, `ImportMetadataForProposals (comic routing)`, `CandidateFromMetadata cover URL path`, `MergeMetadataFields CoverURL`, `ParseComicFilenameForQuery zero-digit normalization`, `BuildImportQuery scan-ID-only comic fallback`, `ApplyImportMetadata with comic candidate` Closes bead bookshelf-qgd90.1 on merge.

comic-import-metadata-modal.png

64 KiB

zombor added 1 commit 2026-07-24 19:30:47 +00:00

feat(bookdrop): comic-aware Import Metadata — route .cbr/.cbz to comic providers ... 13499c8043

Comic files detected by extension (.cbz/.cbr/.cb7/.cbt) are now routed to
ComicVine / Metron / AniList instead of ebook providers (google-books etc.).
Filename is parsed for Series + Issue# + Year to build the comic query.
Comic-specific fields (IssueNumber, CoverDate, StoryArc, Characters, Writers,
cover image URL, etc.) surface in the Import Metadata modal for review.

- bookdrop: ComicMetadataJSON + ImportMetadataComicFields structs
- import_metadata_service: isComicExtension, buildComicImportQuery,
  parseComicFilenameForQuery, filterProvidersForQuery, mergeComicFields,
  applyComicFields, comicProviderMergeChain (all covered at 100%)
- review_handler: explicit field mapping after Comic field added to
  fileMetadataJSON (direct type-cast was broken)
- wire.go: capability-based provider filter replaces hardcoded [:3] slice
- app/providerspec.go: bookdropProviderSpecs now includes ComicVine/Metron/AniList
- app/build_extended_deps.go: comic provider URLs + creds wired for bookdrop
- wfengine: ImportMetadataComicCandidate mirrors bookdrop for JSON bridge
- JS controller: detects c.comic non-null and renders comic fields in modal
  (no inline style= — CSP-safe; uses .bim-result-cover / .bim-result-cover-img)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-07-24 19:37:07 +00:00

fix(bookdrop): refactor high-CC functions + add JS comic-render tests for 100% coverage ... 981a5c20f3

- Extract firstStr/firstSlice helpers and applyEbookCandidateFields to bring
  mergeComicFields (CC 30→2), applyOneCandidate (CC 16→4), and applyComicFields
  (CC 17→2) under the gocyclo 15 gate
- Add Go tests for all uncovered branches: CoverURL merge, scan-ID-only comic
  filename fallback, all-zero volume/issue normalisation, non-comic CoverURL
  path, ApplyImportMetadata with comic candidate (existing + nil cases)
- Add Vitest tests for comic field rendering and cover image rendering in the
  bookdrop_import_metadata_controller — restores 100% JS branch/line coverage

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor commented 2026-07-24 19:46:12 +00:00

Author

Owner

Copy link

Security Review — PR #1226 (bookshelf-qgd90.1): bookdrop Import Metadata → comic providers + provider cover image

Scope reviewed: provider cover URL + comic string fields rendered into the modal (XSS/scheme), multi-user/permission gating, filename→provider query encoding (SSRF/request-splitting), and the internal/bookdrop ↔ workflow-engine architecture boundary.

1. Provider cover URL — XSS / scheme injection: SAFE.

• Go side: candidateFromMetadata/comicMetadataToFields run every cover URL through urlutil.SafeURL (internal/urlutil/safe_url.go), which returns "" for any scheme other than http/https — javascript:/data:/file: are stripped before the value ever reaches the JSON response.
• JS side (static/js/controllers/bookdrop_import_metadata_controller.js:326-332): the URL is assigned via the img.src DOM property (not string-concatenated into innerHTML), and no onerror/inline handler is attached. Defense in depth on top of the Go filter.

2. Comic string fields (characters/creators/story-arc/etc.) — XSS: SAFE.
_addField (line ~345) writes every value via dd.textContent (and the header via textContent). No innerHTML sink anywhere in the render path — provider-controlled strings cannot inject markup.

3. Filename → provider query — SSRF / request-splitting: SAFE.
Filename-parsed Series/Issue/Year (parseComicFilenameForQuery) populate a typed metadata.MatchQuery struct, not a URL. The comic provider (internal/metadata/comicvine/search.go:298-305) sets them via url.Values.Set(...) + Encode() against a fixed base URL — values are percent-encoded, so a crafted filename cannot split the request or redirect the outbound call.

4. Multi-user / permission gating: UNCHANGED / SAFE.
The diff touches no routes, no auth middleware, and reads no userID from the request body/query. Bookdrop remains admin/permission-gated; no per-user data is exposed by the new comic fields.

5. Architecture boundary: INTACT.
internal/bookdrop/*.go (non-test) has no go-workflows import (only comment references to "wfengine"). The wfengine ImportMetadataComicCandidate struct is the JSON wire bridge — correct.

6. Workflow command sequence: no version gate needed.
The import_metadata_workflow.go diff only extends wire-format struct definitions; it adds/removes/reorders no ExecuteActivity/sub-workflow/ContinueAsNew command — replay-safe.

Fan-out cap raised 3→6 (maxConcurrentProviders) — still single-digit, compliant with the fan-out rule.

---

[MINOR] internal/bookdrop/import_metadata_service.go — provider cover rendered as a direct external <img>
The modal loads the provider CDN cover directly (img.src = provider URL), leaking the admin's IP/referer to ComicVine/Metron's CDN on each preview render. This is an admin-only surface so the exposure is low; consider proxying the preview image through the app (as covers are already fetched server-side elsewhere) if referer/IP leakage to third-party CDNs is a concern. No action required to merge.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## Security Review — PR #1226 (bookshelf-qgd90.1): bookdrop Import Metadata → comic providers + provider cover image Scope reviewed: provider cover URL + comic string fields rendered into the modal (XSS/scheme), multi-user/permission gating, filename→provider query encoding (SSRF/request-splitting), and the `internal/bookdrop` ↔ workflow-engine architecture boundary. **1. Provider cover URL — XSS / scheme injection: SAFE.** - Go side: `candidateFromMetadata`/`comicMetadataToFields` run every cover URL through `urlutil.SafeURL` (`internal/urlutil/safe_url.go`), which returns "" for any scheme other than http/https — `javascript:`/`data:`/`file:` are stripped before the value ever reaches the JSON response. - JS side (`static/js/controllers/bookdrop_import_metadata_controller.js:326-332`): the URL is assigned via the `img.src` DOM property (not string-concatenated into `innerHTML`), and no `onerror`/inline handler is attached. Defense in depth on top of the Go filter. **2. Comic string fields (characters/creators/story-arc/etc.) — XSS: SAFE.** `_addField` (line ~345) writes every value via `dd.textContent` (and the header via `textContent`). No `innerHTML` sink anywhere in the render path — provider-controlled strings cannot inject markup. **3. Filename → provider query — SSRF / request-splitting: SAFE.** Filename-parsed Series/Issue/Year (`parseComicFilenameForQuery`) populate a typed `metadata.MatchQuery` struct, not a URL. The comic provider (`internal/metadata/comicvine/search.go:298-305`) sets them via `url.Values.Set(...)` + `Encode()` against a fixed base URL — values are percent-encoded, so a crafted filename cannot split the request or redirect the outbound call. **4. Multi-user / permission gating: UNCHANGED / SAFE.** The diff touches no routes, no auth middleware, and reads no userID from the request body/query. Bookdrop remains admin/permission-gated; no per-user data is exposed by the new comic fields. **5. Architecture boundary: INTACT.** `internal/bookdrop/*.go` (non-test) has no `go-workflows` import (only comment references to "wfengine"). The wfengine `ImportMetadataComicCandidate` struct is the JSON wire bridge — correct. **6. Workflow command sequence: no version gate needed.** The `import_metadata_workflow.go` diff only extends wire-format struct definitions; it adds/removes/reorders no `ExecuteActivity`/sub-workflow/`ContinueAsNew` command — replay-safe. Fan-out cap raised 3→6 (`maxConcurrentProviders`) — still single-digit, compliant with the fan-out rule. --- [MINOR] internal/bookdrop/import_metadata_service.go — provider cover rendered as a direct external `<img>` The modal loads the provider CDN cover directly (`img.src = provider URL`), leaking the admin's IP/referer to ComicVine/Metron's CDN on each preview render. This is an admin-only surface so the exposure is low; consider proxying the preview image through the app (as covers are already fetched server-side elsewhere) if referer/IP leakage to third-party CDNs is a concern. No action required to merge. REVIEW VERDICT: 0 blocker, 0 major, 1 minor

zombor commented 2026-07-24 19:47:41 +00:00

Author

Owner

Copy link

Import Metadata modal — comic fields rendered (.cbr file)

Comic file: Army of Darkness 1979 01.cbr → ComicVine stub returns Issue #1, Cover Date 1992-11, Story Arc, Characters, Teams, Locations, Creators.

Fields shown: Issue #, Volume, Cover Date, Story Arc, Characters, Teams, Locations, Writers, Pencilers, Inkers, Colorists, Letterers, Cover Artists, Editors.

**Import Metadata modal — comic fields rendered (.cbr file)** Comic file: `Army of Darkness 1979 01.cbr` → ComicVine stub returns Issue #1, Cover Date 1992-11, Story Arc, Characters, Teams, Locations, Creators. Fields shown: Issue #, Volume, Cover Date, Story Arc, Characters, Teams, Locations, Writers, Pencilers, Inkers, Colorists, Letterers, Cover Artists, Editors. 

zombor commented 2026-07-24 19:54:17 +00:00

Author

Owner

Copy link

UI Review — PR #1226 (bookshelf-qgd90.1)

Screenshot reviewed: bookdrop_comic_modal.png (Army of Darkness 1979 01.cbr, comicvine provider).

What I see in the rendered screenshot

• Modal structure is fully canonical: .modal-overlay → .modal-dialog → .modal-header / .modal-footer. No bespoke parallel class system.
• Comic field rows reuse the pre-existing bim-field-label / bim-field-value classes on a two-column dt/dd grid, identical to the ebook field layout. Muted label colour (var(--fg-muted)) and value colour (var(--fg)) consistent with the rest of the app.
• Provider badge renders correctly as the existing pill style (.bim-result-provider), top-right.
• Footer buttons (Cancel / Fetch Metadata / Apply Selected) use canonical .btn .btn-small .btn-ghost / .btn-primary.
• All 14 comic field rows are visible and unclipped.
• No overflow, no duplicated controls, no style=, no hardcoded colours.

Source cross-check

[MINOR] static/js/controllers/bookdrop_import_metadata_controller.js (cover-image block) — .bim-result-cover and .bim-result-cover-img are new classes emitted by this PR with no CSS rules in main.css. When cover_url is non-empty the image renders unsized: no max-width, no height constraint. A typical ComicVine cover (1000x1500 px) would overflow the card and squash the fields column. The screenshot does not exercise this path so the rendered result looks correct, but the latent defect is real.

Fix: add alongside the existing .bim-result-body block:

.bim-result-cover { flex-shrink: 0; }
.bim-result-cover-img { display: block; width: 4rem; height: auto; border-radius: var(--radius); }

---

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## UI Review — PR #1226 (bookshelf-qgd90.1) **Screenshot reviewed:** `bookdrop_comic_modal.png` (Army of Darkness 1979 01.cbr, comicvine provider). ### What I see in the rendered screenshot - Modal structure is fully canonical: `.modal-overlay` → `.modal-dialog` → `.modal-header` / `.modal-footer`. No bespoke parallel class system. - Comic field rows reuse the pre-existing `bim-field-label` / `bim-field-value` classes on a two-column `dt`/`dd` grid, identical to the ebook field layout. Muted label colour (`var(--fg-muted)`) and value colour (`var(--fg)`) consistent with the rest of the app. - Provider badge renders correctly as the existing pill style (`.bim-result-provider`), top-right. - Footer buttons (Cancel / Fetch Metadata / Apply Selected) use canonical `.btn .btn-small .btn-ghost` / `.btn-primary`. - All 14 comic field rows are visible and unclipped. - No overflow, no duplicated controls, no `style=`, no hardcoded colours. ### Source cross-check [MINOR] `static/js/controllers/bookdrop_import_metadata_controller.js` (cover-image block) — `.bim-result-cover` and `.bim-result-cover-img` are new classes emitted by this PR with no CSS rules in `main.css`. When `cover_url` is non-empty the image renders unsized: no `max-width`, no height constraint. A typical ComicVine cover (1000x1500 px) would overflow the card and squash the fields column. The screenshot does not exercise this path so the rendered result looks correct, but the latent defect is real. Fix: add alongside the existing `.bim-result-body` block: ```css .bim-result-cover { flex-shrink: 0; } .bim-result-cover-img { display: block; width: 4rem; height: auto; border-radius: var(--radius); } ``` --- REVIEW VERDICT: 0 blocker, 0 major, 1 minor

zombor added 2 commits 2026-07-24 19:55:50 +00:00

fix(bookdrop): preserve Comic fields on manual metadata edit (bookshelf-qgd90.1) ... 1365bd3166

UpdateProposalMetadata was marshaling the incoming fileMetadataJSON directly,
silently overwriting the Comic sub-object with nil when the user edited base
(ebook) fields via the manual editor. Comic data set by Import Metadata was
irreversibly lost.

Fix: after the existing status check (which already fetches the stored row),
parse the stored OriginalMetadata blob and carry forward the existing Comic
sub-object into the incoming meta when the edit does not explicitly set Comic.
This mirrors the merge logic in applyOneCandidate exactly.

Regression test: a proposal with a Comic sub-object (IssueNumber/VolumeName)
receives a manual base-field edit (Title only) → the stored blob still contains
the original Comic sub-object.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(css): add bim-result-cover sizing rules to prevent unsized ComicVine image blowout ... 924ea3922e

The JS emits .bim-result-cover / .bim-result-cover-img for comic cover thumbnails
but no CSS rules existed, so a real 1000×1500 ComicVine cover rendered unsized
and blew out the Import Metadata modal. Add rules alongside .bim-result-body
using the existing --radius token.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor commented 2026-07-24 20:03:41 +00:00

Author

Owner

Copy link

Code Review: Fix Delta on bd-bookshelf-qgd90.1 (981a5c20 → 924ea392)

MAJOR Finding: CLOSED ✓

Comic metadata lost on manual edit is now correctly fixed.

Verification:

1. UpdateProposalMetadata (internal/bookdrop/review_service.go:89–98):

   • Reads THIS proposal's existing OriginalMetadata blob via parseOriginalMetadata(f.OriginalMetadata)
   • Preserves the Comic sub-object ONLY when incoming meta.Comic == nil (meaning user didn't edit comic fields)
   • Safe fallback: if parse fails, preservation is skipped
   • Merge semantics are correct: user's explicit Comic changes take precedence (if non-nil), else existing Comic survives

2. Regression test (internal/bookdrop/review_service_test.go:3178–3217):

   • Fixture: proposal with existing Comic (VolumeName "X-Men", IssueNumber "42")
   • Action: manual edit that changes only Title, leaving Comic nil
   • Assertions (two separate It blocks per convention):
     • "preserves the existing Comic sub-object": asserts captured blob contains "X-Men" substring
     • "includes the updated title": asserts captured blob contains "New Title"
   • Both assertions are sound regression tests—if preservation breaks, "X-Men" won't be present

3. Design alignment: Comment correctly notes this "mirrors the merge logic in applyOneCandidate"—the pattern is: incoming values take precedence when set; nil incoming values preserve existing data.

MINOR Finding: CLOSED ✓

Cover image CSS sizing (static/css/main.css:7431–7433):

• .bim-result-cover { flex-shrink: 0; } prevents flex from squeezing the thumbnail
• .bim-result-cover-img { display: block; width: 4rem; height: auto; border-radius: var(--radius); }
  • Uses var(--radius) token ✓ (no inline styles, no hardcoded colors)
  • Fixed width 4rem, proportional height (auto) ✓
  • Clear comment explaining purpose (prevents 1000×1500 ComicVine cover blowout)

No New Issues

• No .golangci.yml exclusions ✓
• Test is black-box (package bookdrop_test) ✓
• No coverage gaps—the fix adds coverage for the previously-untested Comic preservation path ✓
• No race conditions or edge-case gaps (null/malformed OriginalMetadata handled safely by parseOriginalMetadata returning false) ✓

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## Code Review: Fix Delta on bd-bookshelf-qgd90.1 (981a5c20 → 924ea392) ### MAJOR Finding: CLOSED ✓ **Comic metadata lost on manual edit** is now correctly fixed. **Verification:** 1. **UpdateProposalMetadata (internal/bookdrop/review_service.go:89–98):** - Reads THIS proposal's existing `OriginalMetadata` blob via `parseOriginalMetadata(f.OriginalMetadata)` - Preserves the `Comic` sub-object ONLY when incoming `meta.Comic == nil` (meaning user didn't edit comic fields) - Safe fallback: if parse fails, preservation is skipped - Merge semantics are correct: user's explicit Comic changes take precedence (if non-nil), else existing Comic survives 2. **Regression test (internal/bookdrop/review_service_test.go:3178–3217):** - Fixture: proposal with existing Comic (VolumeName "X-Men", IssueNumber "42") - Action: manual edit that changes only Title, leaving Comic nil - Assertions (two separate It blocks per convention): * "preserves the existing Comic sub-object": asserts captured blob contains "X-Men" substring * "includes the updated title": asserts captured blob contains "New Title" - Both assertions are sound regression tests—if preservation breaks, "X-Men" won't be present 3. **Design alignment:** Comment correctly notes this "mirrors the merge logic in applyOneCandidate"—the pattern is: incoming values take precedence when set; nil incoming values preserve existing data. ### MINOR Finding: CLOSED ✓ **Cover image CSS sizing** (static/css/main.css:7431–7433): - `.bim-result-cover { flex-shrink: 0; }` prevents flex from squeezing the thumbnail - `.bim-result-cover-img { display: block; width: 4rem; height: auto; border-radius: var(--radius); }` - Uses `var(--radius)` token ✓ (no inline styles, no hardcoded colors) - Fixed width 4rem, proportional height (auto) ✓ - Clear comment explaining purpose (prevents 1000×1500 ComicVine cover blowout) ### No New Issues - No `.golangci.yml` exclusions ✓ - Test is black-box (`package bookdrop_test`) ✓ - No coverage gaps—the fix adds coverage for the previously-untested Comic preservation path ✓ - No race conditions or edge-case gaps (null/malformed OriginalMetadata handled safely by parseOriginalMetadata returning false) ✓ **REVIEW VERDICT: 0 blocker, 0 major, 0 minor**

zombor force-pushed bd-bookshelf-qgd90.1 from 924ea3922e to 53c8d9a8db 2026-07-24 22:08:15 +00:00 Compare

zombor merged commit bf45be03d6 into main 2026-07-24 22:15:52 +00:00

zombor referenced this pull request from a commit 2026-07-24 22:15:55 +00:00

Merge pull request 'feat(bookdrop): comic-aware Import Metadata — route .cbr/.cbz to comic providers (bookshelf-qgd90.1)' (#1226) from bd-bookshelf-qgd90.1 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1226

No description provided.