fix(server): expire a host cancel nobody collects - #734
Merged
davidmckayv merged 2 commits intoOct 5, 2026
Merged
davidmckayv merged 2 commits into
davidmckayv merged 2 commits into
Conversation
Every host operation that is failed, stopped, revoked or timed out queues a
cancel for the desktop worker, so a worker that is still holding the command
learns to let it go. That cancel was inserted with expiresAt null and no
timer:
operations.set(cancelOperation.operationId, {
..., expiresAt: null, expiryTimer: null, settled: false,
});
Nothing removed it but resolveDesktopOperation for that exact operationId.
A worker that stopped polling therefore left the entry in operations for
the life of the process: one per abandoned operation, unbounded, and
statusFor reported it to the person as pending forever.
This is the ordinary disconnect case. The cancel exists to tell a desktop
that is there to stop working; when it does not come back there is nothing
to tell, and the entry only misleads.
The cancel now expires on the same clock as any other queued operation, so
a desktop that reconnects is still told to stop, and one that does not is
given up on. Deleted rather than failed, because a cancel has no caller left
to reject and failing it would be indistinguishable from delivering it.
The existing lease-expiry and stop tests both assert the reconnect path and
are unchanged: they collect the cancel immediately.
aniruddhaadak80
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso,
mxmzb and
tylerslaton
as code owners
October 4, 2026 09:43
# Conflicts: # CHANGELOG.md
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
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.
What this changes
When a host operation fails, is stopped, revoked, or times out, the broker queues a
cancelfor the native desktop worker so that a worker still holding the command learns to let it go. That cancel was inserted with no expiry and no timer:Nothing ever removed it except
resolveDesktopOperationfor that exactoperationId(broker.ts:316). So a worker that stopped polling left the entry inoperationsfor the life of the process — one per abandoned operation, unbounded — andstatusForreported it to the person as pending forever (broker.ts:395).This is the ordinary disconnect case, not an edge one. A desktop worker that loses its connection has its lease lapse, every in-flight operation is failed, and each one queues a cancel for a worker that is by definition no longer there. Nothing reaps them, and the Host access panel keeps showing operations that can never finish.
The cancel is still delivered to a desktop that reconnects. That is deliberate and already tested —
server/tests/host-access-broker.test.ts:217("returns cancellation on reconnect") and thestop()test both assert it. So the fix is not to stop queueing an undeliverable cancel, which would have quietly broken those. The cancel now expires on the same clock as any other queued operation (operationTtlMs), which keeps the window open long enough for a reconnecting desktop to be told to stop and gives up on one that never comes back.It is deleted rather than failed, because a cancel has no caller left to reject, and routing it through
failOperationwould make giving up indistinguishable from delivering it.Where it runs
operationsis still an in-process map that already existed and is already documented as per-process host state; this PR adds no new store and does not make an existing in-memory map newly load-bearing. It only stops one entry type from being immortal. I did not move the host-access broker into Postgres — see the honest note below.operationsandgrantswere already per-process maps, so a two-replica deployment already cannot deliver a cancel across processes. This change does not alter that topology, and does not introduce any new cross-replica expectation. If host access is meant to work behind more than one replica, that is a separate and much larger piece of work — flagged below rather than smuggled in here.targetOperationId, which it already did; this adds a timer, not a write.setTimeoutper cancel, which isunref'd exactly like the operation TTL timer already beside it, so it cannot hold the process open.Boundary and audit
Every acting call still goes through the gateway: resolve, decide, audit, then act.
Not an acting call. A cancel is the withdrawal of an action. No
host_*tool call is dispatched, permitted, or refused differently by this change.New refusals and new failures each write a row.
No new refusal and no new failure. The operation it belongs to was already failed and already audited before the cancel was queued; the cancel carries no result and this change does not settle an operation early.
Nothing new is trusted from the client that the server can resolve itself.
Nothing new is read. The expiry is derived from the server's own
operationTtlMsoption, not from anything a client or desktop sent.Changelog
A line in
CHANGELOG.mdunderUnreleased.Added. The deployment-visible difference is that the Host access panel stops listing an operation as pending once it can no longer be delivered.
Proof
bun testhangs on this test file on the machine I work on (Windows), onmainas well as on this branch — it prints onlybun test v1.3.14and never starts reporting. That is pre-existing and unrelated to this change; I confirmed it by stashing and re-running onmain. So I could not usebun testas the instrument here.Instead I drove
createHostAccessBrokerdirectly through a throwaway script (since deleted, never committed) that reproduces exactly what the new test asserts, and ran it againstmainand against this branch.On
main, before the fix:On this branch, after the fix:
The third line is the one I most wanted to keep green, and it is: a desktop that reconnects inside the window is still told to stop.
The committed test is that same check expressed in the file's own style —
a cancel nobody collects does not stay pending for the life of the process, assertingpendingholds onecancelafter the operation times out and is[]afterwards. On theubuntu-latestrunner that CI'stestjob uses, it should go red onmainand green here. I expect it to, but I did not watch it happen, and I would rather say so than claim otherwise.Also run:
bun test server/tests/host-access-tools.test.ts server/tests/host-access-routes.test.ts— 10 pass, 0 fail (the two suites that exercise the broker from outside and were runnable here).bun run --filter server typecheck— exit 0, no diagnostics.bunx biome lint --error-on-warningsandbunx biome formaton both changed files — clean.One thing I deliberately did not do
operationsandgrantsare per-process maps (broker.ts:59-60), whilePOST /grantsawaits a promise only the serving replica can settle and the worker pollsGET /desktop/next, which behind a load balancer can reach any replica. If host access is ever expected to run behind more than one replica, that protocol cannot work as written — a folder grant request would land on replica A and the picker's poll on B.I did not touch that. It is a much larger change than this PR, it depends on whether maintainers consider host access permanently single-host (the token is currently only set by the Tauri desktop app, which spawns a local single-process stack), and guessing wrong would be worse than leaving it. Flagging it rather than bundling it: if it is in scope, it deserves its own issue and PR.