Repository navigation
refactor(ipc): serve search from the daemon - #140
Merged
Merged
Conversation
find was the only operation that bypassed the daemon. That was an implementation convenience - it avoided touching the gob protocol - not a decision, and it is the wrong home for what is coming: making a Cloudflare-protected tracker work needs a logged-in session, a browser session held open by an external solver, and a result cache. None of that can live in a TUI you quit and reopen. The daemon is the only part that lives long enough to hold it. So the daemon builds the Searcher and answers Search and SearchSources, and both clients ask it. The searcher is injected into ipc.Serve the way previewURL already is, and nil - no indexer configured - is the ordinary case rather than a failure, answered by saying which keys to set. Two things about the protocol had to give first, and both would have been discovered the hard way otherwise. A search can take a minute: a browser page load and a challenge solve at the far end. Requests on a connection were answered strictly in order, so every refresh the TUI makes would have queued behind one and the interface would have looked frozen. Search is now answered on its own goroutine. That is safe because replies already carry their sequence number and the client matches on it, and because the send closure already holds the write mutex the event pump uses. The test for it fails without the change, blocking a concurrent List for the full five seconds. The client's 30s timeout is also too short for that worst case, so Search gets its own 120s one. Raising the shared constant instead would mean a genuinely hung daemon took two minutes to report. ipc now imports indexer as well as engine, to carry []indexer.Result on the wire. The rule it bends exists to prevent cycles and keep the daemon lean; indexer is a stdlib-only leaf like engine, so it does neither, and Response.Snapshot []engine.TorrentSnapshot is the same pattern already. CLAUDE.md says so now, along with the note about the daemon not linking an HTTP client, which this deliberately reverses. The TUI takes a narrow local interface rather than indexer.Searcher: all a client needs is the one call, *ipc.Client satisfies it, and the existing test double keeps working. What "configured" means moves with it - the client is always present once attached, so it says nothing, and the daemon naming at least one source is the real test. Per-source warnings are rendered under the results, so a source being down does not hide what the others found. The CLI loses its daemon-free path, which it had and nothing else did. --check now names what the daemon will search rather than reporting a health probe, which is the more useful answer: it separates "nothing configured" from "searched and found nothing". Verified against a stand-in indexer with a six second delay, not just in tests: find through a spawned daemon in 6.0s, and in the TUI the loading state, esc closing the overlay mid-search, the late reply correctly not reopening it, and enter adding a torrent that went on to download.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
findwas the only operation that bypassed the daemon. That was an implementationconvenience - it avoided touching the gob protocol - not a decision, and it is the
wrong home for what is coming: making a Cloudflare-protected tracker work needs a
logged-in session, a browser session held open by an external solver, and a result
cache. None of that can live in a TUI you quit and reopen.
So the daemon builds the
Searcherand answersSearchandSearchSources, andboth clients ask it. The searcher is injected into
ipc.Servethe waypreviewURLalready is, and nil - no indexer configured - is the ordinary case rather than a
failure, answered by naming the keys to set.
Two protocol problems had to be solved first
A slow search would have frozen the TUI. Requests on a connection were answered
strictly in order, and a search can take a minute - a browser page load and a
challenge solve at the far end - so every refresh would have queued behind one.
Search is now answered on its own goroutine. Safe because replies already carry
their sequence number and the client matches on it, and because
sendalreadyholds the write mutex the event pump uses.
The test for this fails without the change:
The 30s client timeout is too short for that worst case.
Searchgets its own120s one via a new
callFor. Raising the shared constant instead would mean agenuinely hung daemon took two minutes to report.
A rule bent on purpose
ipcnow importsindexeras well asengine, to carry[]indexer.Resultonthe wire. The rule exists to prevent cycles and keep the daemon lean;
indexerisa stdlib-only leaf like
engine, so it does neither, andResponse.Snapshot []engine.TorrentSnapshotis the same pattern already. CLAUDE.md records it, alongwith the note about the daemon "never linking an HTTP client", which this
deliberately reverses.
Client changes
The TUI takes a narrow local interface rather than
indexer.Searcher- all aclient needs is the one call,
*ipc.Clientsatisfies it, and the existing testdouble keeps working. What "configured" means moves with it: the client is always
present once attached, so its presence says nothing, and the daemon naming at least
one source is the real test (
canFind).Per-source warnings render under the results, so a source being down does not hide
what the others found.
Response.Errsalready existed for AddBatch's partialsuccess and is reused verbatim.
The CLI loses its daemon-free path, which it had and nothing else did.
--checknow names what the daemon will search (searching: jackett) rather thanreporting a health probe - the more useful answer, since it separates "nothing
configured" from "searched and found nothing".
Verified beyond the tests
Against a stand-in indexer with a deliberate six-second delay:
findthrough a spawned daemon returned in 6.0s - the full server delay, nolonger near any timeout.
(so the interface stayed responsive), the reply landing six seconds later
correctly did not reopen it (the sequence guard, observed live rather than
only unit-tested), and enter added a torrent that went on to download.
make check,make e2e(18/18) andmake docs-buildall clean. Next in thisseries: per-source
[search.*]config with multi-source merge, theninternal/flaresolverr, then the rutracker plugin.