Skip to content

fix(archon,paroche): persist want and release rows before enqueue, on both surfaces - #668

Merged
forkwright merged 5 commits into
mainfrom
fix/harmonia-wave2
Jul 28, 2026
Merged

fix(archon,paroche): persist want and release rows before enqueue, on both surfaces#668
forkwright merged 5 commits into
mainfrom
fix/harmonia-wave2

Conversation

@forkwright

Copy link
Copy Markdown
Owner

Two issues, and the more interesting result is that #644's premise did not survive reading the code.

#651 — releases and wants were never persisted before enqueue

handle_enqueue (MCP) resolved release_id against eksetasis's in-memory cache and minted a random want_id when the caller omitted one, then enqueued — without either row reaching the database. syntaxis::DownloadQueue::enqueue only writes download_queue. So the happy path downloaded bytes and then deterministically failed at import, where ImportAdapter requires both rows to exist.

The HTTP route paroche::routes::download::enqueue_download had the identical defect, taking client-supplied ids at face value. An issue that names one surface does not mean only one surface carries the defect, so both are fixed here.

Both now require an existing want_id, validated against apotheke::repo::want, and idempotently upsert the releases row before calling queue.enqueue().

The shared rule lives in one place — paroche::routes::download, the lower point in the dependency graph, since archon already depends on paroche and not the reverse. What stays separate is only the error rendering: HTTP maps to ParocheError (404/409/500), MCP to tool_error text. That mapping is genuinely surface-specific; the persistence rule now has exactly one definition.

For the self-supplied-URL path there is no server-side resolve to draw metadata from, so magnet dn/xl parameters are parsed where present and a placeholder is used otherwise.

#644 — the issue's literal fix would have left the feature dead

HarmoniaEvent::PlexNotifyRequired has no production sender anywhere in the tree. Every occurrence outside the enum definition is inside syndesmos's own tests. The real import pipeline (kathodos) emits only ImportCompleted, and nothing bridges the two. Widening the event and "threading it through the senders", as the issue asks, would have threaded it through test-only senders and changed nothing at runtime.

What was fixed is the part that is real: the event now carries media_type, notify_plex_import takes it, and the wasteful refresh_all_plex_sections — which refreshed every configured Plex section on every import — is replaced by a targeted notify_library_scan. notify_library_scan is decoupled from &PlexClient in favour of the section map it actually holds, and notify_library_scan_by_section drops to private as the issue asked.

Deciding whether kathodos should fire PlexNotifyRequired, or syndesmos should subscribe to ImportCompleted, or a translator should exist, is an architecture call outside this issue's scope. Plex notification remains unreachable end-to-end until that is settled — stated plainly rather than left implied by a green diff.

Verification

The ordering tests are the load-bearing ones: each records whether the releases row existed at the instant enqueue() ran, so they fail if the persist is ever moved after the enqueue rather than merely checking the end state. That was confirmed by temporarily moving the call and watching them fail.

Also fixed, both pre-existing: archon/tests/mcp_acquisition_acceptance.rs did not compile under --all-targets, and acquisition_integration.rs's helper minted an unpersisted want_id that would now 404 across six tests.

On the build box, CI-exact: fmt clean, clippy --all-targets -D warnings clean, 2285/2285 tests passed, doctests passing.

Refs #651
Refs #644

admin added 5 commits July 28, 2026 12:07
The MCP acquisition bridge's harmonia_enqueue_download resolved a
release_id against eksetasis's in-memory results cache and, when the
caller omitted want_id, minted a fresh UUIDv7 for it — then enqueued
against paroche::state::EnqueueItem without either identity ever
touching the `wants`/`releases` tables. The production ImportAdapter
requires both rows to exist once a download completes, so the happy
path (search, enqueue by release_id, wait) downloaded bytes and then
deterministically failed at import with WantNotFound/ReleaseNotFound.

handle_enqueue now: requires want_id (no longer inventing one) and
validates it references an existing `wants` row; and, before calling
queue.enqueue(), upserts a `releases` row from the cache-resolved
search hit (or, on the magnet arm, from the magnet URI's own dn/xl
metadata params) so release_id is a durable identifier by the time the
transfer completes. A retry against an already-persisted (release,
want) pair is idempotent; the tool's advertised JSON schema now marks
want_id required.

ResolvedRelease (both the eksetasis and paroche mirrors) gained
title/size_bytes/indexer_id so the enqueue path has what it needs to
build the Release row without a second cache lookup.

Refs #651
…a_type

HarmoniaEvent::PlexNotifyRequired carried only a MediaId, so
ScrobbleClient::notify_plex_import couldn't resolve which Plex section
to refresh and fell back to refreshing every configured section on
every import (refresh_all_plex_sections). notify_library_scan — the
targeted, media_type-keyed entry point — was unreachable in non-test
builds and held behind an untracked #[expect(dead_code)] (kanon
WORKFLOW/unwired-dead-code-untracked, harmonia#632/#633).

PlexNotifyRequired now carries media_type: MediaType, threaded through
the event senders in syndesmos/src/events.rs. notify_plex_import takes
media_type and calls notify_library_scan directly against the
service's plex_sections map, so only the section for the imported
item's own media type is touched. refresh_all_plex_sections is
removed (no longer has a reason to exist); notify_library_scan_by_section
drops to a private helper — notify_library_scan is now its sole
caller, matching the issue's intent.

notify_library_scan's own signature no longer takes a `&PlexClient`
(ScrobbleClient never held a concrete one — only the type-erased
Arc<dyn PlexApi> plus a section HashMap kept separately for
testability) — it takes the section map directly, which is both what
the only caller actually has and removes a needless coupling.

Adds notify_plex_refreshes_only_the_matching_section, which fails
under the old refresh-everything behavior: two sections configured,
only the one matching the notified media_type may be touched.

Note: HarmoniaEvent::PlexNotifyRequired has no production sender
anywhere in this tree — the real import pipeline (kathodos) only
emits ImportCompleted, which already carries media_type but nothing
subscribes it for Plex purposes. This leaves Plex notification
unreachable end-to-end in production even after this fix; deciding
where that should be wired (kathodos firing PlexNotifyRequired
directly vs. syndesmos subscribing ImportCompleted vs. a translator)
is a design call outside this issue's scope and is called out
separately.

Refs #644
notify_library_scan now resolves sections from the map it already holds,
leaving PlexClient::section_id_for with no consumer. Removed rather than
suppressed — a second way to resolve a section id can silently diverge
from the one actually used. Its MediaType import went with it.
paroche::routes::download::enqueue_download accepted client-supplied
want_id/release_id at face value and enqueued directly, with no check
that want_id referenced an existing wants row and no releases row ever
written — the identical gap the MCP acquisition bridge's
handle_enqueue carried before its own fix (#651): bytes download, then
syntaxis::ImportService's ImportAdapter deterministically fails to
resolve want_id/release_id against apotheke::repo::want.

Factors the persist-before-enqueue invariant out of the MCP bridge and
into paroche::routes::download (want existence check + idempotent
releases upsert, plus the magnet dn/xl metadata extraction and the
manual-indexer-id sentinel), since paroche has no dependency on archon
and archon already depends on paroche — the shared logic lives at the
lower point in the dependency graph. Both enqueue_download and the MCP
bridge's handle_enqueue now call persist_release_before_enqueue; each
surface maps its ReleasePersistError variants to its own error shape
(ParocheError vs. tool_error text) since that rendering is surface-
specific, but the persistence rule itself has exactly one definition.

The HTTP route's self-supplied-URL arm (no server-side resolve) had no
existing metadata source for indexer_id/title/size_bytes, unlike the
by-reference resolve arm — mirrors the MCP bridge's manual-magnet arm:
a magnet's own dn/xl query params when present, a neutral placeholder
title otherwise.

Test rigor matches the MCP bridge's own #651 test: an ordering probe
records whether the releases row already existed at the INSTANT
queue.enqueue() ran, so it fails under a reordering even though the
end state would look identical (verified: temporarily moving the
persist call after enqueue() flips the probe to false and fails the
test, then reverted).

Also repairs two pre-existing wave1 test breakages surfaced while
getting this compiling/green: mcp_acquisition_acceptance.rs never
updated its ResolvedRelease literal or its enqueue call for the MCP
bridge's own want_id requirement (wave1 didn't compile with
--all-targets), and acquisition_integration.rs's enqueue_via_api
minted a want_id no test ever persisted (every enqueue_via_api-based
HTTP test would now 404 on WantNotFound) — both now seed a real wants
row via the migrator's default quality profile, matching the pattern
already used elsewhere in these suites.

Refs #651
@forkwright
forkwright merged commit ebc998f into main Jul 28, 2026
15 checks passed
@forkwright
forkwright deleted the fix/harmonia-wave2 branch July 28, 2026 20:09
forkwright pushed a commit that referenced this pull request Jul 28, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.2.1](v0.2.0...v0.2.1)
(2026-07-28)


### Bug Fixes

* **archon,paroche:** persist want and release rows before enqueue, on
both surfaces
([#668](#668))
([ebc998f](ebc998f))
* **archon:** tag EngineAdapter's dead_code suppression with a held
marker ([#633](#633))
([#662](#662))
([cafee58](cafee58))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant