fbuild: absorb post-deploy serial wedge recovery into SerialMonitor; deprecate client pyserial

Summary

The post-pyocd CDC VCOM wedge that #565 tracks is conceptually a serial-layer problem, not an LPC-deploy problem. The fix belongs in fbuild-python's SerialMonitor (and its Rust backing in fbuild-serial), not in client orchestration scripts that wrap their own pyserial around fbuild's port-management work.

This issue proposes a two-part move:

Land the architected wedge recovery in fbuild::SerialMonitor -- detect a wedged composite-USB-CDC port on open, recover it (close + re-enumerate request + reopen), and present the recovery as a normal SerialMonitor.__enter__ result to the caller. No client code change required.
Make SerialMonitor the only blessed serial path for fbuild clients. Direct pyserial use by clients becomes a lint failure (parallel to how bare cargo use is blocked by tool_guard.py today). Migration target: every client open of a board's CDC port goes through fbuild.api.SerialMonitor.

Why the wedge fix belongs in fbuild, not in autoresearch

Investigated in the refined-root-cause comment on FastLED/fbuild#565. The wedge isn't triggered by pyocd's reset semantics; it's triggered by two pyocd subprocesses opening + closing the CMSIS-DAP HID interface back-to-back on a composite USB device. Once usbccgp.sys emits a function-level reset on the second HID open, Windows' usbser.sys drops the CDC sibling to error 31 and only physical replug recovers.

The fix lives at the deploy-then-monitor seam, which is exactly fbuild's territory:

fbuild's LpcDeployer (feat(deploy/lpc): LpcDeployer Phase 1 — lpc21isp ISP-over-UART (closes #551, sidesteps #565) #595) is the one issuing the pyocd calls. It can collapse two subprocesses into one, or it can be replaced by lpc21isp (which never opens the HID at all -- the architected long-term answer).
fbuild's SerialMonitor is the one handed to the client immediately after the deploy. It's the layer that sees the wedge first, and is the layer that has all the context to attempt recovery (the deployer just told it the device should be alive; it can detect error 31 on open, and either retry with re-enumeration or surface a structured WedgeDetected error rather than a serial timeout).

Pushing this fix into every client (autoresearch, fbuild-cli's monitor, any future test harness) means every client has to know about the CMSIS-DAP composite quirk, the Windows usbccgp.sys reset semantics, and the recovery sequence. That's not the abstraction SerialMonitor is supposed to provide.

Architectural commitment

From ~/dev/fbuild/CLAUDE.md (already in the repo):

Daemon-centric: All serial access routes through SharedSerialManager in the daemon. No OS-level port locks. Multiple readers (broadcast), exclusive writer (condition variable).

Deploy preemption: Deploy forcibly closes serial sessions → flash via esptool → 2s USB re-enumeration delay → monitors auto-reconnect. Monitors with auto_reconnect=true pause during preemption.

That contract is exactly right -- but it doesn't generalize past ESP32. For LPC845-BRK (composite CDC + CMSIS-DAP HID), the "2s USB re-enumeration delay" isn't an actual re-enumeration; it's just a sleep that doesn't help because Windows error 31 doesn't clear on its own. The deploy-preemption protocol needs a per-platform wedge-recovery hook that the deployer registers with SharedSerialManager.

Concrete shape:

// crates/fbuild-deploy/src/lib.rs (or similar)
pub trait Deployer: Send + Sync {
    fn deploy(&self, ...) -> Result<DeploymentResult>;

    // NEW: per-platform recovery sequence. Called by SharedSerialManager
    // after deploy completes, before the first SerialMonitor open.
    // Default impl is a 2-second sleep (the ESP32 path).
    fn post_deploy_recovery(&self, port: &str) -> Result<()> {
        std::thread::sleep(Duration::from_secs(2));
        Ok(())
    }
}

// LPC override:
impl Deployer for LpcDeployer {
    // ... existing deploy ...

    fn post_deploy_recovery(&self, port: &str) -> Result<()> {
        if cfg!(windows) && self.protocol == "pyocd" {
            // 1. Detect error 31 on CreateFile() against the CDC port
            // 2. Issue CM_Reenumerate_DevNode on the parent hub port
            // 3. Wait for device arrival
            // 4. Retry open
            recover_wedged_cdc(port)
        } else {
            std::thread::sleep(Duration::from_millis(200));
            Ok(())
        }
    }
}

recover_wedged_cdc is the actual Windows-specific Setupapi/CM API dance. Doing it once, in fbuild-deploy, with proper plumbing through SharedSerialManager, is dramatically better than every client re-implementing it (or, today, every client bypassing fbuild entirely to dodge the wedge).

Migration plan

Phase 0: ship the wedge recovery in fbuild (closes #565)

Implement Deployer::post_deploy_recovery default + LPC override.
Wire it into SharedSerialManager's deploy-preemption flow so the first SerialMonitor open after a deploy goes through the recovery hook.
PyO3 binding: SerialMonitor.__enter__ invokes the recovery transparently. Existing Python contract unchanged.
Land the cheap unified-pyocd-call collapse from LPC845-BRK: deploy must always unwedge the shared VCOM (pyocd/SWD flash leaves CDC in Windows error 31) #565's Path A as a defensive layer that reduces wedge incidence even when recovery fails.

Phase 1: deprecate direct pyserial in FastLED clients

Walked the FastLED tree -- 9 sites bypass SerialMonitor for pyserial today:

Site	Reason given	Right fix
`ci/autoresearch/driver_sweep.py:214`	"avoids fbuild daemon port locking issues"	port-locking is what `SharedSerialManager` was built to solve; this comment is a smoke signal
`ci/autoresearch/gpio.py:82, 280`	DTR-reset for ESP32 boot	needs a `SerialMonitor.dtr_reset()` method; trivial PyO3 add
`ci/autoresearch/ota.py:276-284`	`with pyserial.Serial(...)` as a probe	same: `SerialMonitor` with a short timeout
`ci/autoresearch/phases.py:981, 1005, 1748`	inline imports, "hot path"	hot path is fine; just import the right symbol
`ci/autoresearch/test_flexio_rx_objectfled.py:41`	top of test script	move to fbuild adapter
`ci/autoresearch/test_flexio_rx_squarewave.py:40`	top of test script	move to fbuild adapter
`ci/autoresearch/test_lpc_pin_toggle_rx.py:45`	top of test script	move to fbuild adapter

Tasks:

Add the SerialMonitor methods clients need (dtr_reset, probe, short-timeout open).
PR sweep: replace all 9 sites with from fbuild.api import SerialMonitor.
Add a lint rule (parallel to existing ban_std_pathbuf-style dylint) that fails CI when ci/ or examples/ directly imports serial as pyserial / import serial / from serial import.
Document the architectural commitment in fbuild's CLAUDE.md ("Clients must use fbuild.api.SerialMonitor. Direct pyserial use is a tool_guard failure.").

Phase 2: extend to the Rust side

fbuild-cli's monitor subcommand is already routed through the daemon; no change needed.
Any in-tree Rust caller using serialport-rs directly outside fbuild-serial becomes a crate-gate.yml failure.

Acceptance

This closes when:

LPC845-BRK: deploy must always unwedge the shared VCOM (pyocd/SWD flash leaves CDC in Windows error 31) #565's "deploy must always unwedge" criterion is met by virtue of SerialMonitor running the recovery transparently -- no caller has to opt in.
grep -r 'import serial' ci/ examples/ in FastLED returns empty (modulo a 1-2 line exemption in the fbuild_python shim itself for the actual pyserial-as-backend implementation, which is fbuild-serial's internal business).
The tool_guard equivalent for pyserial is wired and CI-enforced.
CLAUDE.md documents the rule.

Refs

LPC845-BRK: deploy must always unwedge the shared VCOM (pyocd/SWD flash leaves CDC in Windows error 31) #565 -- the wedge symptom this issue's Phase 0 closes.
feat(deploy/lpc): LpcDeployer Phase 1 — lpc21isp ISP-over-UART (closes #551, sidesteps #565) #595 -- the LPC deployer Phase 1; the merge that landed two days ago made fbuild the right owner for this whole post-deploy serial story.
design: restore broker-mediated serial proxy + finish running-process broker (port custody, disconnect events, multi-client singleton, cross-version device lease) #592 -- the FbuildSerialAdapter.close() reusability fix that reflected a similar "serial abstraction edge case shouldn't leak to callers" pattern.
fix(autoresearch/lpc): move pinToggleRx + ws2812SctTest edge buffers off the stack (closes #3125) FastLED#3133 -- the RAM-bomb fix that closed lpc845: pinToggleRx + ws2812SctTest handlers stack-overflow at runtime (8 KB stack buffer on 16 KB SRAM) FastLED#3125; with this issue's recovery in place, meta(lpc845-brk): bench-validate the three on-hand-hardware checklists (deploy + #565-sidestep + SCT-RX phases) FastLED#3102 Section 1's bench would be unblockable end-to-end.

Happy to take Phase 0 in a single PR if you confirm the Deployer::post_deploy_recovery shape is what you'd want. Phase 1 sweep is its own PR (or maybe two -- one for the new SerialMonitor methods, one for the call-site migration).

Filed via Claude Code on behalf of zachvorhies@protonmail.com.

Follow-up research — corrections, concrete API surface, and the actual gap table

Filing this as a self-contained follow-up since the original body had two structural errors and was light on the specific fbuild internals. Cited file:line throughout so the implementation review doesn't have to re-walk the workspace.

Correction 1: `Deployer` already exists; this is a default-method extension, not a new trait

The original body proposed introducing Deployer::post_deploy_recovery as if Deployer were new. It isn't. The trait already lives at crates/fbuild-deploy/src/lib.rs:92-101 and currently has exactly one method:

fn deploy(
    &self,
    project_dir: &Path,
    env_name: &str,
    firmware_path: &Path,
    port: Option<&str>,
) -> Result<DeploymentResult>;

The proposal stays the same in shape — add a post_deploy_recovery default method — but it's an additive change to an existing trait, not a green-field introduction. That makes the impact smaller than the original body suggested and avoids the need for any migration of existing impls beyond LpcDeployer.

Correction 2: fbuild's `LpcDeployer` doesn't use pyocd

The original body framed the wedge as a problem the LpcDeployer itself caused. Reading crates/fbuild-deploy/src/lpc.rs:74-247 shows otherwise:

// lpc.rs:166-246 — deploy() spawns:
//   lpc21isp -control -wipe -hex <firmware> <port> <baud> <xtal_kHz>

fbuild's LPC path is already lpc21isp-over-UART. lpc21isp never opens the CMSIS-DAP HID interface, so the CMSIS-DAP composite-reset wedge cannot fire on fbuild-driven LPC deploys today. The wedge bug is entirely a property of autoresearch's pyocd path (ci/autoresearch/phases.py:1684-1716 in FastLED), which is a separate code path that bypasses fbuild's deploy layer.

This shifts the issue's positioning:

The wedge fix isn't urgently needed for LpcDeployer.deploy() itself. It's needed for autoresearch, which sidesteps fbuild's deploy layer when it directly spawns pyocd.
The cleaner long-term fix is: route autoresearch's LPC flashing through fbuild deploy lpc845brk (i.e., through LpcDeployer via lpc21isp), which side-steps the wedge by virtue of not touching the HID interface.
The post_deploy_recovery hook is still right as a defense-in-depth layer for any future Deployer impl that does touch HID (e.g. an eventual PyocdDeployer for SWD-only LPC variants, or a CMSIS-DAP-flashed STM32 path). But it's a "general protocol" change, not the load-bearing fix for LPC845-BRK: deploy must always unwedge the shared VCOM (pyocd/SWD flash leaves CDC in Windows error 31) #565.

The actual deploy → monitor seam (refined from "2s sleep")

Original body said "2s USB re-enumeration delay". The real code at crates/fbuild-daemon/src/handlers/operations/deploy.rs:740-781 is a 100ms-interval fast-poll loop with a 3s deadline:

deploy.rs:369  ctx.serial_manager.preempt_for_deploy(p, ...).await;
                ... firmware write happens here ...
deploy.rs:759  ctx.serial_manager.clear_preemption(p).await;
deploy.rs:762  spawn_blocking() polling loop:
                  loop until deadline (3s):
                      try serialport::new(p).open()
                      if ok → break
                      sleep 100ms
                  (skipped entirely if outcome is VerifySkip)

Preemption tracker: crates/fbuild-serial/src/preemption.rs:1-65 — lock-based, no sleep logic of its own.

So the post_deploy_recovery hook lands cleanly at one specific site: replace (or wrap) the spawn_blocking() poll at deploy.rs:762-779 with self.deployer.post_deploy_recovery(p) and let the deployer choose between the existing fast-poll (default impl) and a platform-specific wedge-recovery sequence (LPC + CMSIS-DAP override, once that path exists).

Current `SerialMonitor` PyO3 surface

From crates/fbuild-python/src/serial_monitor.rs:1-499 and python/fbuild/api/__init__.py:1-14 (the latter is a thin re-export of the PyO3 type with no additional Python-side wrapping):

Method	Present?	Notes
`__new__(port, baud_rate=115200, hooks=None, auto_reconnect=True, verbose=False)`	✅	Constructor
`__enter__` / `__exit__`	✅	Context manager
`last_line` (getter)	✅
`read_lines(timeout=30.0)`	✅	Batched read, dispatches hooks
`write(data: str)`	✅	Base64-over-WS, awaits `WriteAck`
`run_until(condition, timeout=30.0)`	✅
`write_json_rpc(request, timeout=5.0)`	✅	Waits for `REMOTE:` prefix
`reset_device(board=None, wait_for_output=False, timeout=5.0)`	✅	POSTs to daemon `/api/reset`
`in_waiting` / `bytes_available`	❌	gap
`reset_input_buffer()`	❌	gap
`reset_output_buffer()`	❌	gap
`flush()`	❌	gap (implicit drain on `write` may cover)
`set_dtr` / `set_rts`	❌	covered by `reset_device()` at coarser grain
`send_break`	❌	not needed by any current bypass site
`try_read` (short-timeout)	❌	`read_lines(timeout=…)` is the substitute

The actual pyserial API gap from autoresearch's 9 bypass sites

Enumerated the 9 sites in FastLED ci/autoresearch/ and categorized what each one actually calls. The gap is narrower than I implied in the original body:

Site	What pyserial features it uses	Class
`driver_sweep.py:214`	`create_serial_interface(use_pyserial=True)` — already goes through the `SerialInterface` Protocol	(A) just flip the flag
`gpio.py:82, 280`	`_pyserial_dtr_reset(port)` → opens `Serial()`, toggles `dtr`/`rts` for hardware reset	(B) covered by `SerialMonitor.reset_device()` once `FbuildSerialAdapter` is wired everywhere
`ota.py:276`, `phases.py:981, 1005`	`with serial.Serial(port, 115200, timeout=0.1):` — port-availability probe, immediately closes	(C) needs a small `SerialMonitor.probe_open()` helper or just `__enter__` + `__exit__` round-trip wrapped
`phases.py:1748`, `test_lpc_pin_toggle_rx.py`, `test_flexio_rx_objectfled.py`, `test_flexio_rx_squarewave.py`	`Serial()` + `.dtr`/`.rts` + `.reset_input_buffer()` + `.write()` + `.flush()` + `.in_waiting` + `.read()`	(D) the real PyO3 add — needs `in_waiting` + `reset_input_buffer()` exposed on `SerialMonitor` to support timeout-bounded RPC response collection

Minimum PyO3 add needed to delete all 9 sites:

SerialMonitor.in_waiting (getter, usize) — exposes the daemon's buffered-byte count for the current session.
SerialMonitor.reset_input_buffer() (method, returns ()) — sends a ClearBuffer message to the daemon so the monitor's view of the line buffer is dropped to empty.
(Optional) SerialMonitor.probe_open() — a one-shot "can I open the port right now?" check that returns bool without holding the port. Could also be done via a quick __enter__/__exit__ cycle, but a dedicated method is cheap and clearer.

That's it. The DTR/RTS-toggle sites collapse to reset_device(). Three new PyO3 methods (one with broker-side support for ClearBuffer) plus rewiring _pyserial_dtr_reset to call reset_device() is the whole shape of Phase 1.

Lint mechanism for the "no pyserial in clients" rule

fbuild's existing custom-lint pattern is dylint, not clippy.toml. Reference impl: dylints/ban_raw_subprocess/ with workspace registration at Cargo.toml:25-26 and CI enforcement at .github/workflows/dylint.yml:1-102.

But dylint is Rust-side. The new rule is Python-side, so the right parallel is FastLED's existing tool_guard.py mechanism (already blocks bare cargo, bare pio, etc.). Specifically: extend tool_guard.py with an AST check that flags import serial / from serial import / import serial as ... in ci/ and examples/, with a single allowlist exemption for the actual pyserial-backing implementation inside fbuild-serial's own glue.

Related: this fits inside fbuild#592's umbrella

Found fbuild#592 — "design: restore broker-mediated serial proxy + finish running-process broker (port custody, disconnect events, multi-client singleton, cross-version device lease)". The work proposed here (SerialMonitor API growth + Deployer::post_deploy_recovery + dylint-style rule against client pyserial) is a natural concrete deliverable of #592's broker-completion design. Suggest closing this as "implements part of #592" rather than as its own freestanding work — or relabeling it as a sub-issue if #592 gets an explicit sub-task breakdown.

Windows wedge-recovery API confirmation

For the LPC / CMSIS-DAP composite case where #605's post_deploy_recovery would actually do work, the API choice in the original body (CM_Reenumerate_DevNode) is correct. Microsoft documents CM_Reenumerate_DevNode_Ex (cfgmgr32.h) as the supported call to force re-enumeration of a device subtree, and Microsoft Q&A directly confirms the failure mode this issue is recovering from:

Problem When USB CDC Device(Usbser.sys) poor contact, serialport may can not Read — "the issue appears to occur when a user application is still connected to the port while the device is replugged."

Two implementation alternatives for the recovery sequence:

A. CM_Reenumerate_DevNode on the parent hub devnode. Cleaner; targeted at the right level.
B. CM_Disable_DevNode + CM_Enable_DevNode cycle on the composite parent. Heavier-handed but more aggressive against stuck driver state.

Recommend A as the first attempt with a B fallback after a configurable timeout. Both APIs are available via windows-sys's Win32::Devices::DeviceAndDriverInstallation module — no FFI hand-rolling needed.

Revised acceptance for #605

Folding the corrections in, the acceptance gates become:

Deployer::post_deploy_recovery added as a default method on the existing trait at crates/fbuild-deploy/src/lib.rs:92-101. Default impl is the current spawn_blocking 3s fast-poll lifted from deploy.rs:762-779.
crates/fbuild-daemon/src/handlers/operations/deploy.rs:759 switches from inline fast-poll to self.deployer.post_deploy_recovery(port).await.
SerialMonitor PyO3 surface grows in_waiting (getter) and reset_input_buffer() (method); daemon protocol gains a ClearBuffer message.
FastLED's ci/util/serial_interface.py::FbuildSerialAdapter is wired to surface those methods.
FastLED PR migrates all 9 ci/autoresearch/ bypass sites onto the adapter.
FastLED tool_guard.py gains a ban_client_pyserial rule; CI-enforced.
One row in fbuild/CLAUDE.md documents the new architectural commitment.
No LPC + CMSIS-DAP-driven Deployer exists yet, so the wedge recovery itself is documented as a future LPC-specific override of post_deploy_recovery, not part of this PR's scope. The hook just has to exist and be reachable so an autoresearch-side pyocd path can later override it. (If autoresearch migrates to lpc21isp via fbuild deploy lpc845brk first, the LPC override may never need to be written.)

Happy to take any subset as a single PR. Phase 0 (trait + daemon rewire + the two PyO3 method adds) is mechanically small; Phase 1 (the 9-site sweep + lint) is the higher-LOC follow-up. Let me know which subset you'd like sequenced first.

Filed via Claude Code on behalf of zachvorhies@protonmail.com.

Sources for the Windows API + failure-mode citation:

feat(serial): #605 Phase 0 — Deployer::post_deploy_recovery + SerialM…

chore(release): bump 2.2.29 -> 2.2.30 (#608)

feat(serial): HandoffTiming per BoardFamily + matrix doc table (#691) (…

Navigation Menu

Repository navigation

fbuild: absorb post-deploy serial wedge recovery into SerialMonitor; deprecate client pyserial #605

Summary

Why the wedge fix belongs in fbuild, not in autoresearch

Architectural commitment

Migration plan

Phase 0: ship the wedge recovery in fbuild (closes #565)

Phase 1: deprecate direct pyserial in FastLED clients

Phase 2: extend to the Rust side

Acceptance

Refs

Follow-up research — corrections, concrete API surface, and the actual gap table

Correction 1: `Deployer` already exists; this is a default-method extension, not a new trait

Correction 2: fbuild's `LpcDeployer` doesn't use pyocd

The actual deploy → monitor seam (refined from "2s sleep")

Current `SerialMonitor` PyO3 surface

The actual pyserial API gap from autoresearch's 9 bypass sites

Lint mechanism for the "no pyserial in clients" rule

Related: this fits inside fbuild#592's umbrella

Windows wedge-recovery API confirmation

Revised acceptance for #605

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

fbuild: absorb post-deploy serial wedge recovery into SerialMonitor; deprecate client pyserial #605

Description

Summary

Why the wedge fix belongs in fbuild, not in autoresearch

Architectural commitment

Migration plan

Phase 0: ship the wedge recovery in fbuild (closes #565)

Phase 1: deprecate direct pyserial in FastLED clients

Phase 2: extend to the Rust side

Acceptance

Refs

Activity

zackees commented on Jun 16, 2026

Follow-up research — corrections, concrete API surface, and the actual gap table

Correction 1: Deployer already exists; this is a default-method extension, not a new trait

Correction 2: fbuild's LpcDeployer doesn't use pyocd

The actual deploy → monitor seam (refined from "2s sleep")

Current SerialMonitor PyO3 surface

The actual pyserial API gap from autoresearch's 9 bypass sites

Lint mechanism for the "no pyserial in clients" rule

Related: this fits inside fbuild#592's umbrella

Windows wedge-recovery API confirmation

Revised acceptance for #605

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Correction 1: `Deployer` already exists; this is a default-method extension, not a new trait

Correction 2: fbuild's `LpcDeployer` doesn't use pyocd

Current `SerialMonitor` PyO3 surface