Conversation
…ia (#2) - Add LibraryGroup and LibraryGroupPolicy models - DB migration v6→v7: LibraryGroups + LibraryGroupPolicies tables with indexes - CRUD routes: GET/POST /api/db/library-groups, PUT/DELETE/preview/reconcile /{id} - PropagateToLinkedLibraries: auto-propagates artwork + queues per-target on save - Frontend: LibraryGroup types, API service, ConfigSectionLinkedLibraries settings UI - Settings page: new Linked Libraries tab - LinkedLibraryIndicator badge on media-item page - SaveImagesLocally handled per-target via existing queue processor Co-authored-by: Artic0din <9603319+Artic0din@users.noreply.github.com>
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
AURA/backend/routing/database/add.go
Lines 152 to 155 in 9677296
Propagate database-only artwork selections
When the download modal uses “Add to Database Only,” this return occurs before linked-library propagation, so the source selection is saved while every linked copy and the group policy remain stale. This mode should still synchronize the shared database selection and policy, while suppressing the target download queue to preserve its no-download semantics.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if Err := database.UpsertLibraryGroupPolicy(ctx, p); Err.Message != "" { | ||
| response.Skipped++ | ||
| continue | ||
| } | ||
| response.Reconciled++ |
There was a problem hiding this comment.
In the reconciliation flow, choosing a set only upserts LibraryGroupPolicies; it never updates the corresponding SavedItems, refreshes member caches, or queues artwork application for any library copy. Consequently, a conflict selection reports as reconciled while all libraries retain their previous, conflicting selections. Apply the chosen set and selected types to each matching group member as part of this operation.
Useful? React with 👍 / 👎.
| existingPolicy, _, _ := database.GetLibraryGroupPolicyByTMDB(ctx, group.ID, tmdbID, edition) | ||
| policyID := existingPolicy.ID | ||
| if policyID == "" { | ||
| policyID = newRandomID() | ||
| } | ||
| policy := models.LibraryGroupPolicy{ |
There was a problem hiding this comment.
When a user skips a conflict to leave that item unlinked, no policy is stored, but this propagation path treats a missing policy as a reason to create one and then applies the set to every target. The next artwork save for that TMDB item therefore overwrites the deliberately skipped copies. Persist an exclusion or otherwise distinguish explicitly skipped items before unconditionally creating a policy.
Useful? React with 👍 / 👎.
| if len(req.LibraryIDs) < 2 { | ||
| logAction.SetError("Invalid Library Group", "A group must contain at least two libraries", nil) | ||
| return *logAction.Error |
There was a problem hiding this comment.
The REST API accepts repeated IDs such as library_ids: ["movies", "movies"] because it checks only the slice length, and both entries subsequently pass the configuration lookup. This creates a group containing only one actual library, for which propagation has no target despite satisfying the advertised minimum of two libraries. Validate uniqueness before accepting the group.
Useful? React with 👍 / 👎.
| ctx, logAction := logging.AddSubActionToContext(ctx, "Delete Library Group", logging.LevelDebug) | ||
| defer logAction.Complete() | ||
|
|
||
| _, err := s.conn.ExecContext(ctx, `DELETE FROM LibraryGroups WHERE id = ?;`, id) |
There was a problem hiding this comment.
Deleting a group executes only the parent-row deletion and relies on the schema's cascade, but sqlite_init.go opens SQLite with a plain DSN and never enables PRAGMA foreign_keys; SQLite therefore ignores ON DELETE CASCADE by default. Once a group has reconciliation or propagation policies, deleting it leaves all of those rows orphaned indefinitely. Delete the child policies transactionally or enable foreign-key enforcement on every connection.
Useful? React with 👍 / 👎.
| for i := len(key) - 1; i >= 0; i-- { | ||
| if key[i] == '|' { | ||
| return key[:i], key[i+1:] | ||
| } |
There was a problem hiding this comment.
Movie edition names originate from media-server metadata and may themselves contain |. For a key such as 123|Director's Cut | 4K, scanning backward produces TMDB ID 123|Director's Cut and edition 4K, so reconciliation stores the decision under the wrong media identity. Since TMDB IDs cannot contain the delimiter, split at the first | or send structured fields instead.
Useful? React with 👍 / 👎.
| // Propagate artwork to any linked libraries asynchronously. | ||
| // A failure in one target must not affect this response. | ||
| go func() { | ||
| propagateCtx, _ := logging.CreateLoggingContext(context.Background(), "Linked Library Propagation") | ||
| PropagateToLinkedLibraries(propagateCtx, saveItem, fullSet) |
There was a problem hiding this comment.
The new propagation hook runs only after POST /api/db, while the existing Saved Sets editor changes selected artwork types, auto-download flags, and individual set deletions through UpdateItemInDB and PATCH /api/db. Those edits therefore update only the current library and leave linked copies and their policy stale. Invoke linked-group synchronization from the update path as well, including an explicit policy for deleted selections.
Useful? React with 👍 / 👎.
| // Propagate artwork to any linked libraries asynchronously. | ||
| // A failure in one target must not affect this response. | ||
| go func() { | ||
| propagateCtx, _ := logging.CreateLoggingContext(context.Background(), "Linked Library Propagation") | ||
| PropagateToLinkedLibraries(propagateCtx, saveItem, fullSet) | ||
| }() |
There was a problem hiding this comment.
Because each save launches an unsynchronized propagation goroutine, near-simultaneous saves in two linked libraries can leave them swapped rather than synchronized: library A first saves set X and library B saves set Y, then A's goroutine writes X to B while B's writes Y to A. The final policy depends on scheduling and neither copy necessarily represents the last user action. Serialize or version updates by group, TMDB ID, and edition so one ordered decision wins across every member.
Useful? React with 👍 / 👎.
| for _, ls := range item.LibrarySets { | ||
| for _, s := range ls.SavedSets { | ||
| if firstSetID == "" { | ||
| firstSetID = s.ID | ||
| } else if s.ID != firstSetID { | ||
| hasConflict = true |
There was a problem hiding this comment.
The conflict detector compares every saved set to one global firstSetID, so an item using set X for posters and set Y for backdrops is marked conflicting as soon as it processes the second set in the first library. This remains true even when every linked copy has the identical X/Y selection, causing a false conflict and forcing the reconciliation UI toward a single set. Compare each library's effective set-per-artwork-type mapping against the other libraries instead.
Useful? React with 👍 / 👎.
| const openReconcile = async (group: LibraryGroup) => { | ||
| setReconcileGroup(group); | ||
| setReconcileDecisions({}); | ||
| setReconcileOpen(true); | ||
| // Fetch a fresh preview for this group | ||
| setPreviewLoading(true); |
There was a problem hiding this comment.
Opening reconciliation does not clear preview before requesting the selected group's data. After successfully previewing group A, closing it, and opening group B, a failed preview request for B causes the old A items to reappear once loading ends; applying them then submits A's TMDB decisions under B's group ID. Reset the preview at the start of this flow and keep submission disabled when the fresh request fails.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds opt-in linked-library groups that synchronize artwork selections across matching media copies.
Changes:
- Adds database v7 schema, migration, models, and REST APIs.
- Implements matching, reconciliation, asynchronous propagation, and queue integration.
- Adds linked-library settings management and media indicators.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Changes and final review comments |
|---|---|
frontend/src/types/database/library-group.ts |
Defines linked-library group and reconciliation types. No final review comments. |
frontend/src/services/database/library-groups.ts |
Adds linked-library API clients. No final review comments. |
frontend/src/components/shared/linked-library-indicator.tsx |
Moderate (3 votes): Resolve and render the actual other linked library names; otherIds currently only controls a boolean branch. |
frontend/src/components/settings-onboarding/ConfigSectionLinkedLibraries.tsx |
Moderate (3 votes): Invalidate previews when inputs change and require a matching preview or explicit decision before saving. Moderate (2 votes): Preserve or explicitly choose auto_download during reconciliation. Nit (2 votes): Display reconciliation status, timestamps, and partial results. Critical (1 vote): Select the first non-empty saved set across all matched libraries instead of only the first library’s set. |
frontend/src/app/settings/page.tsx |
Adds the Linked Libraries settings tab. No final review comments. |
frontend/src/app/media-item/page.tsx |
Integrates the linked-library indicator. No final review comments. |
frontend/package-lock.json |
Updates frontend dependency metadata. No final review comments. |
backend/routing/routes.go |
Registers linked-library routes. No final review comments. |
backend/routing/database/linked_library_propagate.go |
Critical (3 votes): Carry the stable library ID instead of re-identifying it from LibraryTitle. Moderate (3 votes): Persist per-target results and provide target-specific retries. Critical (2 votes): Ensure custom save destinations are unique per target library. Critical (1 vote): Serialize or coalesce propagation so the latest revision is authoritative. |
backend/routing/database/library_groups.go |
Moderate (2 votes): Reject duplicate memberships and overlapping same-type groups atomically. Moderate (3 votes): Compare all managed policy fields, not only set IDs, during conflict detection. Moderate (2 votes): Delete or explicitly unlink policies for preview items omitted from submitted decisions; the same issue appears at line 345. |
backend/routing/database/add.go |
Moderate (3 votes): Invoke the shared propagation coordinator from all saved-item mutation paths, including PATCH edits and auto-download/MediUX refreshes. |
backend/models/library_group.go |
Defines linked-library group, policy, and preview models. No final review comments. |
backend/database/sqlite_library_groups.go |
Moderate (3 votes): Explicitly delete policies in the transaction or enable SQLite foreign-key enforcement on every connection. |
backend/database/sqlite_helpers.go |
Reviewed; no final review comments. |
backend/database/sqlite_create_tables.go |
Creates linked-library tables for new databases. No final review comments. |
backend/database/migration/sqlite_migration_v6_v7.go |
Creates linked-library tables and indexes for migration. No final review comments. |
backend/database/migration/migrate.go |
Registers the v6-to-v7 migration. No final review comments. |
backend/database/db.go |
Adds database interfaces and version 7 support. No final review comments. |
Files not reviewed (1)
- frontend/package-lock.json: Generated file
Suppressed comments (9)
backend/routing/database/library_groups.go:345
- Reconciliation stops after writing
LibraryGroupPolicies; it never upserts the chosen set into each matchingSavedItemsrow or callsAddToQueue. Consequently, “Apply Decisions” leaves existing library copies unchanged and performs no per-target application. Reuse the same per-target coordinator here.
if Err := database.UpsertLibraryGroupPolicy(ctx, p); Err.Message != "" {
backend/routing/database/library_groups.go:123
- The server-side validation used by both create and update has no preview/reconciliation gate, so API callers can bypass the UI and commit a group with conflicting saved sets directly. Enforce the preview/decision contract on the backend as well, for example with a server-issued preview token or a validated preview payload.
if validErr := validateLibraryGroupRequest(logAction, req); validErr.Message != "" {
httpx.SendResponse(w, ld, response)
return
}
backend/routing/database/library_groups.go:371
len(req.LibraryIDs)counts duplicate entries, so a request such as["lib-a", "lib-a"]passes the minimum-two check even though the group has only one member; propagation then has no target. Validate that the IDs are distinct before accepting the group.
if len(req.LibraryIDs) < 2 {
logAction.SetError("Invalid Library Group", "A group must contain at least two libraries", nil)
return *logAction.Error
backend/routing/database/library_groups.go:375
- This validation checks library existence and media type but never checks whether a library is already in another group of the same media type. The API can therefore create overlapping groups, causing one library’s selection to propagate according to multiple policies. Query existing groups and reject membership conflicts, excluding the group being updated.
// All IDs must refer to libraries of the same media type as the group
libs := config.Current.MediaServer.Libraries
for _, lid := range req.LibraryIDs {
backend/routing/database/library_groups.go:67
- The generated Swagger files are not updated with any
/api/db/library-groupspaths, even though these handlers add new REST endpoints and the server exposes the generated documents at/swagger. Regenerate and commitbackend/api-docsso the new API is discoverable and accurately documented.
// GetLibraryGroups godoc
// @Summary List Library Groups
// @Description Return all linked-library groups.
// @Tags Database
// @Produce json
backend/routing/database/library_groups.go:451
- These saved sets come from the in-memory media-item cache rather than the database. The normal
PATCH /dbupdate path does not refresh that cache, so after a user changes a selection the preview can show stale sets or miss a conflict and reconciliation decisions. Read current saved sets from the database or update the cache on every saved-set mutation before building the preview.
itemMap[key].LibrarySets = append(itemMap[key].LibrarySets, models.LibraryItemSets{
LibraryID: lid,
LibraryTitle: libTitle,
SavedSets: mi.DBSavedSets,
})
backend/routing/database/linked_library_propagate.go:46
- This source-only propagation path never reads an existing group policy for a newly discovered target item. The library refresh updates the cache but does not invoke this helper, so a matching item added later will not receive the already-selected policy.
// Find all groups that contain this library
groups, Err := database.GetGroupsForLibrary(ctx, sourceLibraryID)
if Err.Message != "" {
logAction.SetError("Failed to query linked groups", Err.Message, nil)
return
backend/routing/database/linked_library_propagate.go:132
- The target copy inherits
fullSet.LastDownloadedfrom the source. If the source was downloaded earlier, the target is persisted as already downloaded before its queue job runs (and a failed queue job leaves that false success state). Reset and update this field from the target’s own application result instead of copying source download history.
targetSaveItem := models.DBSavedItem{
MediaItem: *targetItem,
PosterSets: []models.DBPosterSetDetail{targetSet},
}
frontend/src/components/settings-onboarding/ConfigSectionLinkedLibraries.tsx:634
- The preview data includes
selected_types, but each library row renders only the set ID. The reconciliation preview therefore omits which poster/backdrop/season/titlecard types are managed in each library, so users cannot make the required informed conflict decision.
ls.saved_sets.map((s) => (
<Badge key={s.id} variant="outline">
{s.id}
</Badge>
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ctx, logAction := logging.AddSubActionToContext(ctx, "Delete Library Group", logging.LevelDebug) | ||
| defer logAction.Complete() | ||
|
|
||
| _, err := s.conn.ExecContext(ctx, `DELETE FROM LibraryGroups WHERE id = ?;`, id) |
| // Propagate artwork to any linked libraries asynchronously. | ||
| // A failure in one target must not affect this response. | ||
| go func() { | ||
| propagateCtx, _ := logging.CreateLoggingContext(context.Background(), "Linked Library Propagation") | ||
| PropagateToLinkedLibraries(propagateCtx, saveItem, fullSet) |
| // All IDs must refer to libraries of the same media type as the group | ||
| libs := config.Current.MediaServer.Libraries | ||
| for _, lid := range req.LibraryIDs { |
| // Detect conflict: different set IDs across libraries | ||
| firstSetID := "" | ||
| hasConflict := false | ||
| for _, ls := range item.LibrarySets { | ||
| for _, s := range ls.SavedSets { |
| for key, decision := range req.PolicyDecisions { | ||
| tmdbID, edition := splitTMDBKey(key) | ||
| if tmdbID == "" || decision.SetID == "" { | ||
| response.Skipped++ | ||
| continue |
| const res = editingGroup | ||
| ? await UpdateLibraryGroup(editingGroup.id, payload) | ||
| : await CreateLibraryGroup(payload); |
| decisions[`${item.tmdb_id}|${item.edition}`] = { | ||
| set_id: firstSet.id, | ||
| selected_types: firstSet.selected_types, | ||
| auto_download: false, | ||
| }; |
| {groups.map((group) => ( | ||
| <div | ||
| key={group.id} | ||
| className="flex items-start justify-between rounded-md border border-muted p-3" | ||
| > |
| const firstSet = item.library_sets[0]?.saved_sets[0]; | ||
| if (firstSet) { | ||
| decisions[`${item.tmdb_id}|${item.edition}`] = { | ||
| set_id: firstSet.id, | ||
| selected_types: firstSet.selected_types, |
| {otherIds.length > 0 | ||
| ? `Linked (${group.name})` | ||
| : group.name} | ||
| </Badge> |
AURA currently manages artwork independently per library, so users with paired libraries (e.g. Movies + Movies 4K) must manually keep selections in sync. This adds an opt-in linked-library group system with a shared artwork policy that automatically propagates selections across all matching copies.
Database (v6 → v7 migration)
LibraryGroups— named groups referencing ≥2 stable library IDs, scoped to a single media typeLibraryGroupPolicies— group-level artwork policy (set ID, selected types, auto-download, reconcile status) keyed by(group_id, tmdb_id, edition)Backend
linked_library_propagate.go) — after anyAddNewItemToDBcall, fires asynchronously: looks up linked groups by stable library ID, upserts the shared policy, saves the set for every other library target, then enqueues a download job per target. Failures are per-target and do not roll back successful ones.SaveImagesLocally— each queued target resolves its own file path via the existingsaveImageLocallypath logic, so no shared path collisions.GET/POST /api/db/library-groups,PUT/DELETE/preview/reconcile /api/db/library-groups/{id}Frontend
LinkedLibraryIndicator— small badge on the media-item page when the current library belongs to a group; resolves library ID from the existing sections store