remote: the poll loop has no upper bound, so a stuck job hangs the caller forever

[otsobide]

What happens

collapse-remote uploads the bytes, then polls GET /jobs/{id} until the job
leaves queued/compressing. Nothing bounds that loop: no deadline, no maximum
number of polls, no per-request timeout. A server that keeps answering
compressing keeps the client waiting for as long as the process lives.

Observed against a stub server that answers 202 to POST /compress and then
reports compressing on every poll, driven with the release binary at v0.7.0:

still polling after 60s, killed by the harness
polls issued: 293
client output so far: b''

The client printed nothing, wrote nothing, and exited only because the harness
killed it. 293 polls is the 200 ms interval doing its job correctly; the loop
itself simply never ends.

The same is true one layer down. Against a server that accepts the TCP
connection and then never answers at all, the CLI was still waiting when the
harness gave up:

still waiting after 45s, killed by the harness: b''

ureq is used with its defaults, and it sets no read timeout, so a connection
that stalls mid-response waits as long as the peer holds it open.

Reproduction (the whole stub, minus the assertions):

class Handler(BaseHTTPRequestHandler):
    def do_POST(self):                      # 202 Accepted, job queued
        self._json(202, {"job_id": "1111", "status": "queued",
                         "archive_name": "notes.txt.zip"})
    def do_GET(self):                       # forever compressing
        self._json(200, {"job_id": "1111", "status": "compressing",
                         "archive_name": "notes.txt.zip"})

collapse compress notes.txt -o notes.txt.zip --server http://127.0.0.1:<port>

Why it matters

How bad this is depends entirely on which front end is waiting.

• CLI: Ctrl+C works, and nothing was written, so the cost is a puzzled user
  and a job left on the server.
• Desktop: there is no way out. compress() sets processing.value = true
  and awaits the invoke (apps/desktop/src/App.vue:138), the command is a
  synchronous Tauri command blocking a worker thread
  (apps/desktop/src-tauri/src/commands.rs:92), and the UI offers no cancel
  button. Quitting the app is the only exit.
• Web app: same loop in JavaScript, same absence of a bound; the tab has to
  be reloaded.

The trigger is uncommon but real, and it is not only "a hostile server". The
most reachable one is on the server itself: when the worker cannot record a
status change, it logs and moves on (apps/server-backend/src/queue.rs:31), so
a registry write that fails (a full disk, for example) leaves the row saying
compressing after the archive is finished. Nothing later corrects it: the
reaper only collects terminal jobs, and reconciliation only runs at startup. A
genuinely enormous tree at level 5 produces the same experience without anything
being wrong at all, which is the harder half of the problem: the client cannot
tell a slow job from a dead one, because the server reports no progress beyond
the status name.

Nothing is lost when it happens. The archive, if there is one, stays on the
server until the TTL collects it.

Where it is

apps/remote/src/client.rs:126

/// Poll `GET /jobs/{id}` until the job is ready (Ok) or gives up (Err).
fn wait_for_completion(base: &str, job_id: &str) -> Result<(), RemoteError> {
    loop {
        let response = ureq::get(&format!("{base}/jobs/{job_id}"))
            .call()
            .map_err(|e| remote_error(base, e))?;

        match protocol::progress_of(&parse_json(response)?)? {
            Progress::Ready => return Ok(()),
            Progress::Waiting => std::thread::sleep(POLL_INTERVAL),
        }
    }
}

POLL_INTERVAL is 200 ms (apps/remote/src/client.rs:18). Every ureq call in
the file (create_job, wait_for_completion, download, check_health) uses
the crate defaults, so none of them carries a read timeout either.

The web app repeats the shape independently, apps/server-frontend/src/api.js:78:

  let last = null
  for (;;) {
    const polled = await fetcher(`/jobs/${job.job_id}`)
    ...
    if (progressOf(current) === 'ready') break
    await sleep(POLL_INTERVAL)
  }

Why it is like that

The decision that was made here was the narrow one, and it was right: only
queued and compressing mean "wait" (apps/remote/src/protocol.rs:36), so a
server answering an unknown status or no status at all is an error rather than a
reason to keep polling. That closed the case where a foreign server made the
client spin forever on nonsense. What it deliberately did not do is put a limit
on the legitimate case, because there is no defensible constant: compressing a
large tree at level 5 can take hours, and a client that gives up at ten minutes
would be wrong more often than the hang it prevents.

docs/architecture.md lists "an upper bound on how long a client waits for a
job" under Roadmap as remaining work, so this is known and unresolved rather
than overlooked. The roadmap says it is tracked in the issues, which it is not
yet.

What a fix looks like

The pieces are separable, and the first two are worth doing regardless of what
is decided about the third.

1. Per-request timeouts. Build one ureq::Agent with
   timeout_read/timeout_write (and a connect timeout) instead of the bare
   ureq::get/ureq::post helpers, and reuse it across the flow. That bounds a
   stalled socket without saying anything about how long a job may take. The
   download needs a generous value or none: a large archive over a slow link is
   a legitimate long read, and this is the same tail the container stop already
   costs (see docs/server.md).
2. Cancellation, which is what the desktop actually needs. Even a perfect
   deadline leaves a user with a frozen window for its duration. The shape that
   fits: compress_path takes something it can check between polls (an
   AtomicBool, or a channel), the Tauri command owns it, and the UI grows a
   cancel button that also issues DELETE /jobs/{id}. This changes
   collapse-remote's public signature, so it wants doing at the same time as
   any other change to it.
3. A bound on the wait itself. Three candidates, in rising order of effort:
   • A flat deadline, off by default: --job-timeout-minutes on the CLI, a
     field per server in the desktop's settings. Honest and predictable, but the
     user has to guess a number for work whose duration they do not know.
   • A stall detector: give up only when the status has not changed for N
     minutes. Better shaped, but with today's protocol compressing is a single
     state with no progress inside it, so a two hour compression looks exactly
     like a stuck one. This only becomes meaningful if the server starts
     reporting progress (bytes read, entries done), which is a server change and
     a wire contract change.
   • Bound only the states the server cannot legitimately sit in for long:
     queued behind a single-consumer worker is one of them, and a job that
     stays queued for an hour on a server nobody else is using means something
     is wrong. Narrow, and it catches the failed-status-write case above only
     when it happens before the flip to compressing.

Whatever is chosen, keep it in collapse-remote so the CLI and the desktop
inherit it together, and mirror it in apps/server-frontend/src/api.js, which is
a separate implementation of the same loop.

How to know it is fixed

• apps/remote/tests/client.rs gains a test that serves a stub answering
  compressing forever and asserts compress_path returns an error within the
  configured bound instead of blocking (the suite already serves a real backend
  in-process, so a stub handler is a small addition).
• A test that a server which accepts and never answers produces a
  RemoteError::Unreachable rather than a hang.
• apps/cli/tests/remote.rs asserts the user-facing message names the timeout
  and that no partial archive is written.
• apps/server-frontend/tests/ covers the same decision in api.js.
• If cancellation lands: a desktop test that a cancelled compression leaves no
  output file, and the Vitest suite covers the button.
• docs/architecture.md's Roadmap line is updated or removed.

Related

• remote: a small file waits 200 ms for a job that is already done #48 lives in this exact loop (the 200 ms floor on small files). Both changes
  touch wait_for_completion, so doing them together is cheaper than twice.
• docs/architecture.md, Roadmap: "an upper bound on how long a client waits
  for a job".
• docs/server.md, "Known limitations": a job can outlive its client's
  patience, from the server's side of the same coin.
• apps/remote/tests/protocol.rs pins progress_of's decision table, including
  that an unknown status is an error; that behaviour stays as it is.

[otsobide]

Reopened. This was closed by mistake, by me.

Closes #71 was in a commit message (9c40daf), not only in a PR body, so when that commit reached main GitHub honoured it. I had removed the keyword from the release PR's description after an audit flagged it, which turned out to address the wrong surface entirely: the commit carries it, and editing the PR body does nothing.

What actually landed in v0.9.0

Item 1, per-request timeouts. Done. One shared ureq::Agent with connect 10 s and read/write 30 s. A server that accepts the connection and then goes silent is now given up on with a distinct RemoteError::Unresponsive, whose message says the job may still be running on the far side.

What did not, and why

The headline case in this issue's title is still live, and that is now a decision rather than an omission. A server that answers compressing on every poll still keeps the client waiting indefinitely. Confirmed against a stub on the v0.9.0 build: it polled until a 45 s guard killed it.

The limit was deliberately put on the server's answers, not on the job, because a real compression may legitimately run for hours and a total deadline would cut off exactly the users who need a remote server most. apps/remote/src/waiting.rs says so at the wait_for doc comment, and there is a test (a_long_job_is_never_cut_short_while_the_server_keeps_answering) that fails if anyone adds a total deadline.

So item 3, a bound on the wait itself, is closed as "will not do" in the form this issue proposed. If a server that lies about its own progress is worth defending against, that is a different mechanism (progress reporting, or a no-change-in-status bound), and worth its own issue rather than this one.

Item 2, cancellation, is untouched, and the issue itself calls it "what the desktop actually needs". The desktop still has no way out of a long compression: processing.value stays true and there is nothing to click.

Proposal

Narrow this issue to item 2, cancellation, and note item 3 as declined. Item 1 shipped in v0.9.0.