fix(daemon): enforce exclusive device leases across deploy and RPC workflows; add bounded sharing queue #1864

Activity

1. fastled-project-sync

   added this to FastLED Trackeron Nov 2, 2026
2. fastled-project-sync

   moved this to Triage in FastLED Trackeron Nov 3, 2026
3. zackees commented on Nov 3, 2026

   

   zackees

   on Nov 3, 2026

   MemberAuthor

   More actions

   Steward note: the admission half of this issue is now in PR #1869.

   • DeviceManager::admit_operation(port, lease_id) refuses a mutating
     operation on a port leased by another client with a new
     DeviceLeaseError::ForeignLeaseHeld (names the lease id, the holder and both
     escape hatches).
   • /api/deploy consults it on the resolved port before
     serial_manager.preempt_for_deploy, so a competing checkout can no longer
     close the holder's session or flash over its firmware — closing observation
     2 from the report. DeployRequest/fbuild deploy gain an optional
     --lease-id.
   • /api/reset gets the same gate (a reset would drop the holder
     mid-validation).
   • fbuild device take --reason remains the explicit human override.

   Still open here (not in #1869): bounded wait/queue for competing workflows
   with ordering + cancellation, lease identity on the serial-write/RPC writer
   path, tracked-renumbering coverage beyond the existing move test, and
   lost-lease reporting on daemon restart/disconnect. The two-process
   on-hardware validation still needs a board slot that is not in use for
   FastLED/FastLED#5001; it was not attempted this hour.
4. zackees commented on Nov 3, 2026

   

   zackees

   on Nov 3, 2026

   MemberAuthor

   More actions

   Admission slice of this issue is merged: #1869 (e55b63e3, squash-merged as 13e48205).

   What landed:

   • DeviceManager::admit_operation(port, lease_id) — an exclusive lease admits only the lease id it was acquired under; anything else returns DeviceLeaseError::ForeignLeaseHeld naming the lease, holder, --lease-id <id> and fbuild device release <port> --lease-id <id>.
   • /api/deploy checks admission on the resolved port before preempt_for_deploy, so a competing client can no longer close the holder's session or flash over its image. /api/reset is gated the same way.
   • DeployRequest/ResetRequest carry an optional lease_id; CLI gains fbuild deploy --lease-id <id> (also threaded through debug, mcp tools and the daemon client types).
   • Monitor leases never block; unknown ports admit (emulator/probe routes stay outside the lease path); fbuild device take --reason is still the explicit human override.
   • Tests: 68 lines of device-manager unit tests (unleased admits, foreign-no-lease refused with holder+id, wrong id refused, holder admits, monitor doesn't block, unknown port admits) plus 47 lines of handler tests that drive the real DaemonContext; verified RED first by stubbing the gate.

   Still open on this issue (not in #1869):

   1. Bounded queue / deterministic ordering for competing workflows (sequestration instead of refusal).
   2. Lease identity on the serial-write / RPC writer path (a second writer can still take the port for raw serial writes).
   3. Obtaining a lease by workflow rather than --track-serial heuristics (renumbering test beyond the existing move test).
   4. Lost-lease reporting on daemon restart.
5. zackees commented on Nov 3, 2026

   

   zackees

   on Nov 3, 2026

   MemberAuthor

   More actions

   Second slice of #1864 landed as PR #1871 (unmerged, awaiting review/CI): the
   serial write path now honors a foreign exclusive lease.

   • SerialClientMessage::Attach carries an optional lease_id (omitted by
     older clients → "no lease", the safe reading).
   • handle_serial_ws admits through DeviceManager::admit_operation before
     open_port(), so a refused client never opens/resets/reads the board, and
     re-admits on every write frame so a lease acquired mid-session stops the
     next byte with write_ack{success:false}.
   • fbuild monitor --lease-id <id> lets the holder's own session through.

   Coverage: ws_serial_attach_honors_foreign_exclusive_lease drives the real
   handler over a real WebSocket + pty and the real DeviceManager (refused
   attach with no port opened; holder attaches and writes; mid-session lease
   steal refuses the next write). RED first with the gate stubbed, then green;
   daemon 307p/0f, serial 203p/0f, cli bin 472p/0f, clippy -D warnings clean.

   Remaining acceptance criteria on this issue:

   1. opt-in bounded wait/queue with deterministic ordering, cancellation and
      holder/progress diagnostics (criterion 3);
   2. lease identity across re-enumeration/recovery reattach;
   3. python facade lease_id plumbing (currently refused-by-default, safe);
   4. two real checkouts taking turns on one board (criterion 6, needs an idle
      board).
6. zackees commented on Nov 3, 2026

   

   zackees

   on Nov 3, 2026

   MemberAuthor

   More actions

   Second slice is now merged: PR #1871 — the serial write path honors an
   exclusive workflow lease (merge commit 9f182e4f).

   Shipped:

   • SerialClientMessage::Attach carries an optional lease_id; omitted means
     "present no lease" (older clients), which is refused on a reserved board.
   • handle_serial_ws admits through DeviceManager::admit_operation before
     open_port(), so a refused client never opens/resets/reads the board, and
     re-admits on every write frame — a lease taken by another workflow
     mid-session stops the next byte with write_ack{success:false}.
   • fbuild monitor --lease-id <id> for the holder; docs/reference/cli.md
     workflow-lease section updated.

   Coverage on main: ws_serial_attach_is_refused_under_a_foreign_exclusive_lease
   (portable: refuses no-lease attach without opening the port; holder clears
   admission) and the mid-session steal assertion in
   serial_ws_replies_follow_request_order (real pty). Zero new host-platform
   ledger rows.

   Still open on this issue:

   1. opt-in bounded wait/queue: deterministic ordering, cancellation, holder and
      progress diagnostics, waiting before preemption/reset/open;
   2. lease identity across re-enumeration/recovery reattach (the holder's own
      reattach after PortRenumbered is not yet re-admitted on the new port);
   3. python facade lease_id plumbing (fbuild-python has its own message type;
      today such sessions are correctly refused, which is the safe default);
   4. explicit lost-lease reporting on daemon restart/disconnect;
   5. two real checkouts taking turns on one board (criterion 6) — needs an idle
      board.
7. zackees commented on Nov 3, 2026

   

   zackees

   on Nov 3, 2026

   MemberAuthor

   More actions

   Third slice of #1864 is merged: PR #1873 — the opt-in bounded wait/queue
   (acceptance criterion 3). Merge commit acc3ac55.

   Shipped:

   • DeviceManager admission queue (device_manager/wait_queue.rs):
     enqueue_waiter / leave_queue / queued_client_ids / poll_admission and
     the bounded await_admission.
   • Deterministic ordering: a waiter is admitted only at the head of its
     port's FIFO with no foreign lease, the lease holder is never queued, and a
     fail-fast caller (no ticket) is refused while anyone is queued — so two
     clients that observe a release in the same instant cannot reorder. Queues are
     per device, so competing boards stay independent.
   • Cancellation: QueueGuard owns the ticket, so a dropped/cancelled request
     leaves the queue and promotes the next workflow (no phantom head-of-line
     blocker). A timeout also leaves the queue.
   • fbuild deploy --lease-wait <seconds> / DeployRequest.lease_wait_ms
     (serde default 0 = today's fail-fast behaviour). The wait runs at the same
     admission site as fix(daemon): enforce explicit device leases on deploy and reset admission #1869, on the resolved port, before
     preempt_for_deploy — so a queued deploy never resets, closes or flashes the
     holder's board, and never opens the port.
   • Diagnostics: DeviceLeaseError::LeaseWaitTimedOut names the queue position,
     the holder (lease id, client, description) and the elapsed wait, and states
     the board was not reset/closed/flashed; the refusal without --lease-wait
     now recommends the flag. queued_client_ids/queued_workflows expose queue
     depth for status output.

   Coverage (RED first, behaviourally: stubbing the admission rule gave
   2 passed; 6 failed; restored → green): FIFO head-only ordering, foreign-lease
   blocking + holder naming, unqueued caller cannot jump the queue, unknown ports
   admitted, bounded wait admits after a real release (≥100 ms), timeout leaves the
   queue empty, wait_ms == 0 stays fail-fast, holder never queued, plus
   lease_wait_ms serde defaults. fbuild-daemon --lib 317 passed / 0 failed,
   fbuild-cli --bin fbuild 472 passed / 0 failed, clippy -D warnings rc 0,
   fmt rc 0, boundary rows=44; dylint_rows=43 (ledger untouched).

   Remaining on this issue:

   1. lease identity across re-enumeration/recovery reattach (criterion 4's
      renumbering half);
   2. Python facade lease_id (and now lease_wait_ms) plumbing;
   3. two real checkouts taking turns on one board (criterion 6) — needs a board
      slot not in use for ESP32-S3 LCD_CLOCKLESS: let the application choose the worker task's core (currently created unpinned at priority 23) FastLED#5001.